Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update mssql driver to support verify-ca and verify-full #1198

Merged
merged 3 commits into from
Apr 16, 2020

Conversation

doodlesbykumbi
Copy link
Contributor

@doodlesbykumbi doodlesbykumbi commented Apr 15, 2020

Closes #1165
Closes #1199

mssql driver only supported full verification out of the box, which corresponds to sslmode=verify-full.

  1. sslmode=verify-full has been added, with addition parameter sslhost that overrides the host used for verification.
  2. sslmode=verify-ca has been fixed. custom verification logic has been added to ignore hostname verification.

Checklist before merge:

@doodlesbykumbi doodlesbykumbi requested a review from a team as a code owner April 15, 2020 21:19
@doodlesbykumbi doodlesbykumbi changed the title Update mssql driver to support verify-ca Update mssql driver to support verify-ca and verify-full Apr 15, 2020
@doodlesbykumbi doodlesbykumbi force-pushed the mssql-verify-ca branch 3 times, most recently from 8f460c1 to 80f61d3 Compare April 15, 2020 21:36
Copy link
Contributor

@BradleyBoutcher BradleyBoutcher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@BradleyBoutcher BradleyBoutcher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, now with driver support!

mssql driver only supported full verification out of the box. Custom verification logic is necessary to ignore hostname verification.
@codeclimate
Copy link

codeclimate bot commented Apr 16, 2020

Code Climate has analyzed commit 369043a and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 48.4% (0.1% change).

View more on Code Climate.

Copy link
Contributor

@BradleyBoutcher BradleyBoutcher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@doodlesbykumbi doodlesbykumbi merged commit 8fea23c into master Apr 16, 2020
@doodlesbykumbi doodlesbykumbi deleted the mssql-verify-ca branch April 16, 2020 20:18
@doodlesbykumbi doodlesbykumbi self-assigned this Apr 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

MSSQL: Support for sslmode=verify-full MSSQL: Support for sslmode=verify-ca
2 participants