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

TDS8 Add Server Certificate Support #1822

Merged
merged 25 commits into from
Nov 10, 2022

Conversation

lcheunglci
Copy link
Contributor

This adds the parameter Server Certificate to the connection string. The server certificate is a local path to the SSL Certificate that has been download from the server. This certificate will be manually validated during the SSL Handshake. When HostNameInCertificate is specified with ServerCertificate, HNIC will take precedence. I added very basic functional test to check for the connection string parameter. Additional manual tests will be added in a later PR.

@codecov
Copy link

codecov bot commented Nov 1, 2022

Codecov Report

Base: 69.25% // Head: 69.22% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (1c26fff) compared to base (2a54bc5).
Patch coverage: 54.44% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1822      +/-   ##
==========================================
- Coverage   69.25%   69.22%   -0.03%     
==========================================
  Files         292      292              
  Lines       61604    61727     +123     
==========================================
+ Hits        42662    42733      +71     
- Misses      18942    18994      +52     
Flag Coverage Δ
addons 92.38% <ø> (ø)
netcore 73.30% <55.23%> (-0.04%) ⬇️
netfx 67.62% <61.95%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...core/src/Interop/SNINativeMethodWrapper.Windows.cs 98.83% <ø> (ø)
...core/src/Microsoft/Data/SqlClient/SNI/SNICommon.cs 55.12% <0.00%> (-14.23%) ⬇️
...c/Microsoft/Data/SqlClient/TdsParserStateObject.cs 23.30% <ø> (+0.08%) ⬆️
...a.SqlClient/netcore/src/Resources/StringsHelper.cs 26.66% <ø> (ø)
...c/Microsoft/Data/Interop/SNINativeMethodWrapper.cs 57.14% <ø> (ø)
.../Microsoft/Data/Common/DbConnectionStringCommon.cs 64.05% <ø> (ø)
...oft/Data/SqlClient/TdsParserSafeHandles.Windows.cs 93.20% <ø> (ø)
...e/src/Microsoft/Data/SqlClient/SNI/SNITcpHandle.cs 62.62% <45.45%> (-0.87%) ⬇️
...nt/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs 70.84% <52.70%> (-0.21%) ⬇️
.../netcore/src/Microsoft/Data/SqlClient/TdsParser.cs 73.33% <55.35%> (-0.03%) ⬇️
... and 17 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@DavoudEshtehari DavoudEshtehari added this to the 5.1.0-preview2 milestone Nov 1, 2022
@cheenamalhotra cheenamalhotra added the 🆕 Public API Issues/PRs that introduce new APIs to the driver. label Nov 2, 2022
@JRahnama
Copy link
Contributor

JRahnama commented Nov 7, 2022

/azurepipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JRahnama
Copy link
Contributor

the failure seems not relevant .Connection Open() without retries took longer than expected (2 seconds) on Mac machine.

@lcheunglci
Copy link
Contributor Author

The mac failures seem to intermittently occur on the pipeline in all the PRs recently, so I just have to re-run it until it succeeds.

@lcheunglci lcheunglci merged commit f0f36f2 into dotnet:main Nov 10, 2022
@lcheunglci lcheunglci deleted the TDS8-Add-ServerCertificate-Support branch November 10, 2022 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🆕 Public API Issues/PRs that introduce new APIs to the driver.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants