- Notifications
You must be signed in to change notification settings - Fork311
Tests | Fix RemoteCertificateNameMismatchErrorTest (ActiveIssue 31754)#3059
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Do not automatically trust the Certificate in the Test
/azp run |
| Azure Pipelines successfully started running 2 pipeline(s). |
codecovbot commentedMar 19, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@## main #3059 +/- ##==========================================- Coverage 72.68% 68.18% -4.50%========================================== Files 283 315 +32 Lines 58975 76136 +17161 ==========================================+ Hits 42864 51911 +9047- Misses 16111 24225 +8114
Flags with carried forward coverage won't be shown.Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
/azp run |
| Azure Pipelines successfully started running 2 pipeline(s). |
@edwardneal I know you were working on certificates and tests, can you give a bit of feedback on this one? |
| Set-ItemProperty "HKLM:\SOFTWARE\Microsoft\Microsoft SQL Server\$($_.Value)\MSSQLServer\SuperSocketNetLib" -Name Certificate -Value $certificate.Thumbprint.ToLower() | ||
| # Grant read access to Private Key for SQL Service Account | ||
| if ($($_.Name) -eq "MSSQLSERVER") { | ||
| icacls $machineKeyPath /grant "NT Service\MSSQLSERVER:R" | ||
| } else { | ||
| icacls $machineKeyPath /grant "NT Service\MSSQL`$$($_.Name):R" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| Set-ItemProperty "HKLM:\SOFTWARE\Microsoft\Microsoft SQL Server\$($_.Value)\MSSQLServer\SuperSocketNetLib" -Name Certificate -Value $certificate.Thumbprint.ToLower() | |
| # Grant read access to Private Key for SQL Service Account | |
| if ($($_.Name) -eq "MSSQLSERVER") { | |
| icacls $machineKeyPath /grant "NT Service\MSSQLSERVER:R" | |
| } else { | |
| icacls $machineKeyPath /grant "NT Service\MSSQL`$$($_.Name):R" | |
| } | |
| $serviceAccount = (Get-ItemProperty "HKLM:\SYSTEM\CurrentControlSet\Services\$($_.Name)" -Name ObjectName).ObjectName | |
| Set-ItemProperty "HKLM:\SOFTWARE\Microsoft\Microsoft SQL Server\$($_.Value)\MSSQLServer\SuperSocketNetLib" -Name Certificate -Value $certificate.Thumbprint.ToLower() | |
| # Grant read access to Private Key for SQL Service Account | |
| icacls $machineKeyPath /grant "$serviceAccount:R" |
This will handle the case where SQL Server runs under a non-default account (CONTOSO\srv-mssql, etc.)
It'd be a little more PowerShell-centric to use the built-inGet-Acl andSet-Acl rather than shelling out toicacls, but that doesn't do anything different - the approach is fine.
| $store.open("MaxAllowed") | ||
| $store.add($certificate) | ||
| $store.close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We're going to add a new certificate to the Computer and to the Trusted Root Certificate Authorities certificate stores on every CI run. Depending how often the test agents are reprovisioned, we might bump intoKB2801679.
Another approach might be to iterate over the list of instances, then lazily generate a computer certificate if one is missing, then add the certificate to the Root store, then to add permissions over its machine key path.
7663524 intodotnet:mainUh oh!
There was an error while loading.Please reload this page.
This pull request includes significant changes to the configuration and testing of SQL Server certificates. The most important changes involve adding a PowerShell script to configure SQL Server certificates on Windows and updating tests to reflect these changes.
Configuration changes:
eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml: Added a PowerShell script to create a self-signed certificate, add it to trusted roots, and configure SQL Server instances to use the certificate. This script also grants read access to the private key for the SQL Service Account.Test updates:
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs: Removed theActiveIssueattribute from theRemoteCertificateNameMismatchErrorTestmethod to enable the test.src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs: Updated theRemoteCertificateNameMismatchErrorTestmethod to setTrustServerCertificatetofalse, ensuring the test checks for certificate name mismatches.This is in addition to#3012 which activates other tests in this Class