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

SqlSecureConnection: Unable to finish successfully with ForcedEncryption #1888

Closed
Kreby opened this issue Mar 27, 2023 · 11 comments · Fixed by #1892
Closed

SqlSecureConnection: Unable to finish successfully with ForcedEncryption #1888

Kreby opened this issue Mar 27, 2023 · 11 comments · Fixed by #1892
Labels
bug The issue is a bug.

Comments

@Kreby
Copy link
Contributor

Kreby commented Mar 27, 2023

Problem description

I am currently trying to implement a SqlSecureConnection resource with a certificate and forced encryption but have run into an issue with it not being able to execute successfully. I think I have narrowed the problem down to two issues.

The first is this line. The problem is with the fact that the ServerName is hardcoded to localhost which once the thumbprint of the certificate is set results in the Connect-SQL cmdlet throwing the following exception.

Connect-Sql : System.InvalidOperationException: Failed to connect to SQL instance 'localhost'. (SQLCOMMON0019) --->
System.Management.Automation.MethodInvocationException: Exception calling "Connect" with "0" argument(s): "Failed to
connect to server localhost." ---> Microsoft.SqlServer.Management.Common.ConnectionFailureException: Failed to connect
to server localhost. ---> Microsoft.Data.SqlClient.SqlException: A connection was successfully established with the
server, but then an error occurred during the login process. (provider: SSL Provider, error: 0 - The target principal
name is incorrect.) ---> System.ComponentModel.Win32Exception: The target principal name is incorrect
   --- End of inner exception stack trace ---

The Connect-SQL cmdlet is working as expected but this becomes an issue here when waiting for the restart. The ErrorActionPrefernce is masking that the Connect-SQL cmdlet is throwing an exception about the SSL connection which will never resolve itself. This results in $testConnectionServerObject always being null which ends up waiting for a loop control condition that will never occur.

The first issue led me to the second issue which is with the Stopwatch timer. The intention is that the loop end after 120 seconds, assuming the default timeout value, but it's not because the control statement is incorrect. The loop control should be looking at TotalSeconds not Elapsed.Seconds because Elapsed.Seconds will never be greater than 60.

I've included these in the same issue as they are related but I can submit the incorrect loop control as a separate issue if you'd prefer.

Verbose logs

There is nothing of interest in the logs as the  is set to ```SilentlyContinue```.

DSC configuration

SqlSecureConnection 'ForceSecureConnection'
{
    InstanceName    = $SqlInstanceName
    Thumbprint      = $CertificateThumbprint
    ForceEncryption = $ForceEncryption
    Ensure          = 'Present'
    ServiceAccount  = $SqlServiceCredential.UserName
}

Suggested solution

For the first issue with the hardcoded localhost value for the ServerName I'm not sure if you'd want to do anything other than
add ServerName to the resource and then pass that through to the Restart-SqlServerice cmdlet. Once the certificate is set and encryption is forced the only way to connect is by using the FQDN. There might be another way to get the data by just trying to figure out what the FQDN should be or using localhost as a default but I don't think that would be easier than just adding ServerName property to the resource and defaulting to a value of localhost if it's not supplied.

For the issue with the loop control this simply just modify the Elapsed.Seconds to Elapse.TotalSeconds

SQL Server edition and version

Microsoft SQL Server 2022 (RTM) - 16.0.1000.6 (X64)   Oct  8 2022 05:58:25   Copyright (C) 2022 Microsoft Corporation  Standard Edition (64-bit) on Windows Server 2022 Datacenter 10.0 <X64> (Build 20348: ) (Hypervisor)

SQL Server PowerShell modules

Name            Version    Path
----            -------    ----
SqlServer       22.0.49    C:\Program Files\WindowsPowerShell\Modules\SqlServer\22.0.49\SqlServer.psd1
SqlServer       21.1.18256 C:\Program Files\WindowsPowerShell\Modules\SqlServer\21.1.18256\SqlServer.psd1
SqlServer       21.1.18221 C:\Program Files\WindowsPowerShell\Modules\SqlServer\21.1.18221\SqlServer.psd1
SQLPS           16.0       C:\Program Files (x86)\Microsoft SQL Server\160\Tools\PowerShell\Modules\SQLPS\SQLPS.psd1

Operating system

OsName               : Microsoft Windows Server 2022 Datacenter
OsOperatingSystemSKU : DatacenterServerEdition
OsArchitecture       : 64-bit
WindowsVersion       : 2009
WindowsBuildLabEx    : 20348.1.amd64fre.fe_release.210507-1500
OsLanguage           : en-US
OsMuiLanguages       : {en-US}

PowerShell version

Name                           Value
----                           -----
PSVersion                      5.1.20348.1366
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.20348.1366
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

SqlServerDsc version

Name         Version Path
----         ------- ----
SqlServerDsc 16.1.0  C:\Program Files\WindowsPowerShell\Modules\SqlServerDsc\16.1.0\SqlServerDsc.psd1
@johlju
Copy link
Member

johlju commented Mar 28, 2023

The first issue led me to the second issue which is with the Stopwatch timer. The intention is that the loop end after 120 seconds, assuming the default timeout value, but it's not because the control statement is incorrect. The loop control should be looking at TotalSeconds not Elapsed.Seconds because Elapsed.Seconds will never be greater than 60.

Confirmed. Good catch! I will create a separate issue for this.

@johlju
Copy link
Member

johlju commented Mar 28, 2023

The Connect-SQL cmdlet is working as expected but this becomes an issue here when waiting for the restart. The ErrorActionPrefernce is masking that the Connect-SQL cmdlet is throwing an exception about the SSL connection which will never resolve itself. This results in $testConnectionServerObject always being null which ends up waiting for a loop control condition that will never occur.

Creating a new issue for this.

@johlju
Copy link
Member

johlju commented Mar 28, 2023

Please see #1891 (comment) and see if you can help getting the output to catch the error you mentioned.

@johlju johlju added bug The issue is a bug. help wanted The issue is up for grabs for anyone in the community. labels Mar 28, 2023
@johlju
Copy link
Member

johlju commented Mar 28, 2023

For the first issue with the hardcoded localhost value for the ServerName I'm not sure if you'd want to do anything other than
add ServerName to the resource and then pass that through to the Restart-SqlServerice cmdlet. Once the certificate is set and encryption is forced the only way to connect is by using the FQDN. There might be another way to get the data by just trying to figure out what the FQDN should be or using localhost as a default but I don't think that would be easier than just adding ServerName property to the resource and defaulting to a value of localhost if it's not supplied.

An optional parameter ServerName with the default value of 'localhost' sounds like the easiest non-breaking change solution.

I was thinking we could evaluate the property Subject and Subject Alternate Name (the first DNS Name that exist in SAN) on the certificate that would be returned as the ServerName. 🤔
A new local function similar to Get-CertificateAcl that returns the name to use for ServerName, or use Find-Certificate from DscResource.Common (currently in PR https://github.com/dsccommunity/DscResource.Common/pull/101/files) could help getting the certificate.
But that will probably be a breaking change.

@johlju
Copy link
Member

johlju commented Mar 28, 2023

I must also say that the work you put into the details in this issue was very much appreciated.

@johlju
Copy link
Member

johlju commented Mar 28, 2023

First issue #1889 squashed inmerged PR #1890.

@johlju
Copy link
Member

johlju commented Mar 28, 2023

@Kreby can you look at the change in the PR #1892. Will it solve this issue?

@johlju johlju added in progress The issue is being actively worked on by someone. and removed help wanted The issue is up for grabs for anyone in the community. labels Mar 28, 2023
@Kreby
Copy link
Contributor Author

Kreby commented Mar 28, 2023

@johlju, I will try to take a look at this afternoon. I've got a few meetings but hopefully I can get to it, if not I might not be able to get to it until Thursday. I should just be able to merge in the changes to what I'm running locally now and see if it works.

Thanks for the quick response on this and the complement. 😃 Always glad to help out when I can.

@Kreby
Copy link
Contributor Author

Kreby commented Mar 30, 2023

I had a chance to test today and I think it will work. My config got further but it is still erroring out, however I don't think it is related to this change. I will have more time tomorrow to double check and then get back to you for sure.

@Kreby
Copy link
Contributor Author

Kreby commented Mar 30, 2023

The errors came from the SqlDatabaseMail resource. They ran after the SqlSecureConnection, but I don't think they are related to the addition of the ServerName property to the SqlSecureConnection resource. My previously failing SqlSecureConnection resource executed successfully and didn't get stuck in the Restart-SqlService loop. I think that's all there is to check at the moment, in regards to #1892.

@johlju
Copy link
Member

johlju commented Apr 1, 2023

Cool, lets merge the PR then.

johlju added a commit that referenced this issue Apr 1, 2023
- `SqlSecureConnection`
  - Added new parameter `ServerName` that will be used as the host name when
    restarting the SQL Server instance. The specified value should be the same
    name that is used in the certificate (issue #1888).
@johlju johlju removed the in progress The issue is being actively worked on by someone. label Apr 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants