go-sqlcmd icon indicating copy to clipboard operation
go-sqlcmd copied to clipboard

Generated connection strings are incorrect

Open yorek opened this issue 2 years ago • 9 comments

When generating the connection strings via sqlcmd config connection-strings, the ADO.NET doesn't work as is:

  • The connection string contains spaces that make it hard to just copy-and-paste it into an application/configuration file
  • The "Encode" property doesn't exist (it should be "Encrypt"?)
  • The "Persist Security options" doesn't exist (it should be "Persist Security Info"?)

Reference to connection string keywords is here: https://learn.microsoft.com/en-us/dotnet/api/system.data.sqlclient.sqlconnection.connectionstring?view=dotnet-plat-ext-7.0

Also, the connection string comes with the default for TrustServerCertificate to false, which make it hard to use it with a local SQL Server. sqlcmd should detect if the connection string is generated for a local machine and automatically set that property to true, to make usage frictionless.

yorek avatar Feb 24 '23 18:02 yorek

Great feedback, does this look right (addresses all 4 issues above):

C:\src\go-sqlcmd\cmd\modern>sqlcmd config cs ADO.NET: Server=tcp:localhost,1435;Initial Catalog=master;Persist Security Info=False;User ID=username;Password=REDACTED;MultipleActiveResultSets=False;Encrypt=True;TrustServerCertificate=True;Connection Timeout=30; JDBC: jdbc:sqlserver://localhost:1435;database=master;user=username;password=REDACTED;encrypt=true;trustServerCertificate=true;loginTimeout=30; ODBC: Driver={ODBC Driver 13 for SQL Server};Server=tcp:localhost,1435;Database=master;Uid=username;Pwd=REDACTED;Encode=yes;TrustServerCertificate=yes;Connection Timeout=30; SQLCMD: SET "SQLCMDPASSWORD=REDACTED" & sqlcmd -S localhost,1435 -U username

I was outputting it as YAML, which was putting the newlines in the output, which I didn't know it did. Now just outputting as text.

stuartpa avatar Feb 25 '23 11:02 stuartpa

how about adding a string for go-mssqldb? We'd prefer the url syntax for the Go driver because the pseudo-ODBC and pseudo-ADO syntax support don't support passwords with escaped separator characters etc.

Which reminds me - we should make sure to properly escape such characters in the ado and odbc strings, since it's not as straightforward as escaping a URL component.

shueybubbles avatar Feb 25 '23 15:02 shueybubbles

Great feedback, does this look right (addresses all 4 issues above):

Everything looks good at first glance, I'll try the ADO.NET and ODBC connection string ASAP. I'm not an expert in Java so let's make sure someone can test it properly too.

yorek avatar Feb 25 '23 19:02 yorek

Another suggestion. Can you make the TrustServerCertificate parameter set to false only if connecting to an Azure SQL Database? (you can take a look at the server name. If it ends with .database.windows.net then set that option to false) The goal is to make the experience frustration free.

yorek avatar Feb 25 '23 19:02 yorek

how about adding a string for go-mssqldb? We'd prefer the url syntax for the Go driver because the pseudo-ODBC and pseudo-ADO syntax support don't support passwords with escaped separator characters etc.

Which reminds me - we should make sure to properly escape such characters in the ado and odbc strings, since it's not as straightforward as escaping a URL component.

Added GO:

C:\src\go-sqlcmd\cmd\modern>sqlcmd config cs ADO.NET: Server=tcp:localhost,1435;Initial Catalog=master;Persist Security Info=False;User ID=username;Password=REDACTED;MultipleActiveResultSets=False;Encrypt=True;TrustServerCertificate=True;Connection Timeout=30; JDBC: jdbc:sqlserver://localhost:1435;database=master;user=username;password=REDACTED;encrypt=true;trustServerCertificate=true;loginTimeout=30; ODBC: Driver={ODBC Driver 18 for SQL Server};Server=tcp:localhost,1435;Database=master;Uid=username;Pwd=REDACTED;Encrypt=yes;TrustServerCertificate=yes;Connection Timeout=30; GO: sqlserver://username:REDACTED@localhost,1435?database=master;encrypt=true;trustServerCertificate=true;loginTimeout=30 SQLCMD: SET "SQLCMDPASSWORD=REDACTED" & sqlcmd -S localhost,1435 -U username

stuartpa avatar Feb 25 '23 19:02 stuartpa

Another suggestion. Can you make the TrustServerCertificate parameter set to false only if connecting to an Azure SQL Database? (you can take a look at the server name. If it ends with .database.windows.net then set that option to false) The goal is to make the experience frustration free.

Done

stuartpa avatar Feb 25 '23 19:02 stuartpa

go-mssqldb doesn't use the same names for a bunch of the parameters. eg "logintimeout" => "dial timeout" There is an adoSynonyms list we can edit to match them up though. https://github.com/microsoft/go-mssqldb/blob/f481f922043187c6b33c43e32683cd03989d9d04/msdsn/conn_str.go#L399

shueybubbles avatar Feb 26 '23 04:02 shueybubbles

What value do we get from setting trust server cert to false for Azure? Azure connections are always encrypted and always use a root cert that's trusted by most OS distributions. I could understand omitting the parameter but actively setting it to false is not needed.

shueybubbles avatar Feb 27 '23 16:02 shueybubbles

Yep, I'm fine not adding the TrustServerCertificate property in the connection string for Azure SQL connections. Omitting it means it is false by default.

yorek avatar Feb 27 '23 19:02 yorek