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

SqlSetup fails to run after configuring ForcedEncryption in SqlSecureConnection #1893

Closed
Kreby opened this issue Mar 31, 2023 · 9 comments · Fixed by #1900
Closed

SqlSetup fails to run after configuring ForcedEncryption in SqlSecureConnection #1893

Kreby opened this issue Mar 31, 2023 · 9 comments · Fixed by #1900
Labels
bug The issue is a bug.

Comments

@Kreby
Copy link
Contributor

Kreby commented Mar 31, 2023

Problem description

The SqlSetup resource fails to execute on subsequent runs after an initial install if a SqlSecureConnection resource has been processed and has ForceEncryption = $true. The issue is related to the lack of support for FQDN's, much like in #1888.

This specifics of this issue is two fold

Firstly the ServerName property for SqlSetup is assumed and obtained using the Get-ComputerName function. This only becomes a problem after Encryption is forced.

Secondly, the root of the first issue, is the function Get-ComputerName from the DscResource.Common module will only ever provide a hostname and is not capable of returning a FQDN.

The ServerName variable is then passed to a number of other cmdlets which in turn call Connect-Sql and we've seen before in, #1888, the use of a FQDN is required in order to connect.

Verbose logs

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 ---

DSC configuration

It's not really relevant to the issue but can provide if needed.

Suggested solution

The best option would probably be to add a ServerName property to the SqlSetup resource which defaults to just using the Get-ComputerName it is already assuming when the argument isn't supplied.

If adding a ServerName property is not acceptable then trying to resolve the hostname via DNS is an option but I feel that might be more problematic. I too was using $env:COMPUTERNAME previously but currently I am working around that by relying on DNS

$serverName = [System.Net.Dns]::GetHostByName($env:COMPUTERNAME).HostName 

I do plan to go back to fix to pass the FQDN around but I haven't done that work yet.

I am currently only testing the DNS lookup on Server 2022 so I can't speak to it's support on other OS's as well older versions of Windows or PowerShell at the moment. I've not looked up to see what versions of Dot Net use this.

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 Apr 2, 2023

This get the information on current node on macOS so it might work on all OS'es, although it doesn't return FQDN there, but it is not connected to an AD domain either, so not a good example. 🙂

[System.Net.Dns]::GetHostByName($null)

I think Get-ComputerName returns what is expected, it could possible be extended with a switch parameter FQDN that runs the above command to return the fully qualified domain name . 🤔 That would help solve most (or all) resource by adding that parameter to Get-ComputerName.

But it would not help if the network name to connect to is different than the hostname. For example when using AG or failover clustering. For failover clustering there is the parameter FailoverClusterNetworkName, but after creation the dns name is gotten from the cluster. But I wonder if we still need a ServerName parameter to handle AG's or other scenrios where network name is different from hostname. 🤔

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

johlju commented Apr 2, 2023

Seems GetHostByName() is deprecated, instead we should use this to return the entry for the current node:

[System.Net.Dns]::GetHostEntry('')

@johlju johlju changed the title SqlSetup fails to run after configuring ForcedEncryption in SqlSecureConnection. SqlSetup fails to run after configuring ForcedEncryption in SqlSecureConnection Apr 2, 2023
@Kreby
Copy link
Contributor Author

Kreby commented Apr 2, 2023

I'm not currently running an AG in the configuration I'm testing with and that is something I hadn't yet thought about.

I did consider proposing a FQDN switch for the Get-ComputerName. That might certainly be helpful in general, but then how do you decide when to use the switch in the DSC resources? I'm not sure if you want to default to using it or not. I imagine that could have a number of unexpected outcomes for people who expect/want just the hostname?

I guess I should switch over to using GetHostEntry() on my side of things even if it is temporary.

I did consider that the DNS lookup could fail. I didn't have a better option when I started testing. This would also require the SQL server to have a network connection and a reachable DNS server (assuming that's how GetHostEntry actually works). It is a bit naive to think that will always be the case, if I'm being honest. So I don't know if there is a better way than to add the ServerName property to the resource and let the configuration author decide what needs to be passed.

@johlju
Copy link
Member

johlju commented Apr 6, 2023

I think safest bet is to add ServerName parameter which is used instead of Get-ComputerName. If not provided it defaults to Get-ComputerName.

@johlju
Copy link
Member

johlju commented Apr 8, 2023

@Kreby I made a suggested fix in PR #1900 using ServerName. Could you verify that it works?

@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 Apr 8, 2023
johlju added a commit that referenced this issue Apr 8, 2023
- SqlSetup
  - Added new parameter `ServerName` that will be used as the host name when
    evaluating the SQL Server instance. If using a secure connection the
    specified value should be the same name that is used in the certificate
    (issue #1893).
@johlju johlju removed the in progress The issue is being actively worked on by someone. label Apr 8, 2023
@johlju
Copy link
Member

johlju commented Apr 9, 2023

@Kreby I merged the suggested fix. If there are any issue or if it doesn't work out as expected let me know and we make a change before next full release is released.

@Kreby
Copy link
Contributor Author

Kreby commented Apr 9, 2023

@johlju Thanks for the turn around on that. The newly released SqlServer module has caused me a bit of a headache elsewhere so I've had to focus my efforts on addressing some other issues first. I hope to be able to test this tomorrow. I'll let you know how it goes.

@Kreby
Copy link
Contributor Author

Kreby commented Apr 10, 2023

@johlju I was able to test it this morning and verify that everything was working as expected now. Subsequent runs are no longer failing. I did it before you released the latest 16.2.0 version so my tests were just using the main branch.

Thanks again for the help.

@johlju
Copy link
Member

johlju commented Apr 10, 2023

Yes I went ahead and released the new version so we would get the support for SqlServer 22.0.59. Happy it worked as expected though, thanks for testing and reporting back. Then we squashed that one too. 😃

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