Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions money.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/shopspring/decimal"
)

type Money[D decimal.Decimal|decimal.NullDecimal] struct {
type Money[D decimal.Decimal | decimal.NullDecimal] struct {
Decimal D
}

Expand All @@ -20,5 +20,5 @@ func (m Money[D]) Value() (driver.Value, error) {
func (m *Money[D]) Scan(v any) error {
scanner, _ := any(&m.Decimal).(sql.Scanner)

return scanner.Scan(v);
return scanner.Scan(v)
}
19 changes: 9 additions & 10 deletions money_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func TestBulkInvalidString(t *testing.T) {
col := columnStruct{
ti: typeInfo{
TypeId: typeMoneyN,
Size: 8,
Size: 8,
},
}

Expand All @@ -36,7 +36,7 @@ func TestBulkInvalidType(t *testing.T) {
col := columnStruct{
ti: typeInfo{
TypeId: typeMoneyN,
Size: 8,
Size: 8,
},
}

Expand All @@ -55,7 +55,7 @@ func TestBulkMoneyN(t *testing.T) {
col := columnStruct{
ti: typeInfo{
TypeId: typeMoneyN,
Size: 8,
Size: 8,
},
}

Expand All @@ -79,7 +79,7 @@ func TestBulkMoneyPositive(t *testing.T) {
col := columnStruct{
ti: typeInfo{
TypeId: typeMoney,
Size: 8,
Size: 8,
},
}

Expand All @@ -103,7 +103,7 @@ func TestBulkMoneyNegative(t *testing.T) {
col := columnStruct{
ti: typeInfo{
TypeId: typeMoney,
Size: 8,
Size: 8,
},
}

Expand All @@ -127,7 +127,7 @@ func TestBulkMoney4Positive(t *testing.T) {
col := columnStruct{
ti: typeInfo{
TypeId: typeMoney4,
Size: 4,
Size: 4,
},
}

Expand All @@ -151,7 +151,7 @@ func TestBulkMoney4Negative(t *testing.T) {
col := columnStruct{
ti: typeInfo{
TypeId: typeMoney4,
Size: 4,
Size: 4,
},
}

Expand Down Expand Up @@ -222,8 +222,8 @@ func TestMoneyDecimal(t *testing.T) {
s := &Stmt{}

res, err := s.makeParam(Money[shopspring.Decimal]{
shopspring.New(-82913823232, -4),
},
shopspring.New(-82913823232, -4),
},
)

if err != nil {
Expand Down Expand Up @@ -397,7 +397,6 @@ func TestMoneyScanNullDecimal(t *testing.T) {
}
}


func readMoney(buf []byte) int64 {
return int64((uint64(binary.LittleEndian.Uint32(buf)) << 32) | uint64(binary.LittleEndian.Uint32(buf[4:])))
}
Expand Down
85 changes: 73 additions & 12 deletions msdsn/conn_str.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package msdsn

import (
"bytes"
"crypto/tls"
"crypto/x509"
"encoding/pem"
Expand Down Expand Up @@ -193,7 +194,7 @@
}

// Build a tls.Config object from the supplied certificate.
func SetupTLS(certificate string, insecureSkipVerify bool, hostInCertificate string, minTLSVersion string) (*tls.Config, error) {
func SetupTLS(certificate string, insecureSkipVerify bool, hostInCertificate string, minTLSVersion string, skipHostnameValidation bool) (*tls.Config, error) {
config := tls.Config{
ServerName: hostInCertificate,
InsecureSkipVerify: insecureSkipVerify,
Expand All @@ -213,18 +214,72 @@
if err != nil {
return nil, fmt.Errorf("cannot read certificate %q: %w", certificate, err)
}
if strings.Contains(config.ServerName, ":") && !insecureSkipVerify {
err := setupTLSCommonName(&config, pem)
if err != skipSetup {

usedCustomVerification := false

// When skipHostnameValidation is true, we skip hostname checks but still validate the certificate chain
if skipHostnameValidation {
if err := setupTLSCertificateOnly(&config, pem); err != nil {
return nil, err
}
usedCustomVerification = true
} else if strings.Contains(config.ServerName, ":") && !insecureSkipVerify {
switch err := setupTLSCommonName(&config, pem); err {
case nil:
usedCustomVerification = true
case skipSetup:
// fall back to standard RootCAs handling below
default:
return &config, err
}
}
certs := x509.NewCertPool()
certs.AppendCertsFromPEM(pem)
config.RootCAs = certs

if !usedCustomVerification {
certs := x509.NewCertPool()
certs.AppendCertsFromPEM(pem)
config.RootCAs = certs
}
return &config, nil
}

// setupTLSCertificateOnly validates that the server certificate matches the provided certificate
func setupTLSCertificateOnly(config *tls.Config, pemData []byte) error {
// To match the behavior of Microsoft.Data.SqlClient, we simply compare the raw bytes
// of the server's certificate with the provided certificate file. This approach:
// - Does not validate certificate chain, expiry, or subject
// - Only checks that the server's certificate exactly matches the provided certificate
// - Skips hostname validation (which is the intended behavior)
//
// We use InsecureSkipVerify=true with VerifyPeerCertificate callback because
// VerifyConnection runs AFTER standard verification (including hostname check).

// Parse the expected certificate from the PEM data
block, _ := pem.Decode(pemData)
if block == nil {
return fmt.Errorf("failed to decode PEM certificate")
}
// Store the raw certificate bytes (DER format) for comparison
expectedCertBytes := block.Bytes

config.InsecureSkipVerify = true

Check failure

Code scanning / CodeQL

Disabled TLS certificate check High

InsecureSkipVerify should not be used in production code.

Copilot Autofix

AI 1 day ago

In general, the fix is to avoid setting InsecureSkipVerify = true and instead rely on Go’s normal TLS verification pipeline, adding a custom verification step that enforces the “certificate only” semantics while keeping hostname and chain validation. This is best accomplished by configuring a VerifyConnection callback on tls.Config, which runs after the standard verification has been performed, or by using VerifyPeerCertificate without disabling verification and performing full parsing/validation of the certs yourself.

For this specific function, the minimal change that preserves existing behavior while removing InsecureSkipVerify is to stop disabling verification and adjust the custom verification to work with the standard flow. The current comment that “VerifyConnection runs AFTER standard verification (including hostname check)” is correct, but we don’t need to mimic that by disabling verification; we can simply rely on the default verification and then enforce the raw‑byte equality. A clean approach is:

  • Remove the line config.InsecureSkipVerify = true.
  • Leave VerifyPeerCertificate in place, but note that when InsecureSkipVerify is false, Go’s docs state that if VerifyPeerCertificate is set, it is called instead of the default verification, not in addition. To both keep default verification and add our check, we should instead use VerifyConnection.
  • Implement config.VerifyConnection = func(cs tls.ConnectionState) error { ... }, obtain the peer certificate from cs.PeerCertificates[0], compare its Raw bytes to expectedCertBytes, and then (optionally) call cs.VerifiedChains to ensure that standard verification succeeded. However, standard verification has already run, and if it failed, the handshake would not reach VerifyConnection, so we only need to perform the equality check.
  • Keep the same failure modes: return an error if there are no peer certificates or if the bytes don’t match.

Because you asked to only change shown code in msdsn/conn_str.go, the concrete edits will:

  • Remove config.InsecureSkipVerify = true.
  • Replace the VerifyPeerCertificate assignment with a VerifyConnection assignment that compares cs.PeerCertificates[0].Raw to expectedCertBytes.

No new imports are required; we already import crypto/tls, crypto/x509, bytes, and fmt.

Suggested changeset 1
msdsn/conn_str.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/msdsn/conn_str.go b/msdsn/conn_str.go
--- a/msdsn/conn_str.go
+++ b/msdsn/conn_str.go
@@ -246,13 +246,13 @@
 func setupTLSCertificateOnly(config *tls.Config, pemData []byte) error {
 	// To match the behavior of Microsoft.Data.SqlClient, we simply compare the raw bytes
 	// of the server's certificate with the provided certificate file. This approach:
-	// - Does not validate certificate chain, expiry, or subject
+	// - Does not validate certificate chain, expiry, or subject beyond what Go's TLS
+	//   stack already does by default
 	// - Only checks that the server's certificate exactly matches the provided certificate
-	// - Skips hostname validation (which is the intended behavior)
 	//
-	// We use InsecureSkipVerify=true with VerifyPeerCertificate callback because
-	// VerifyConnection runs AFTER standard verification (including hostname check).
-	
+	// We rely on Go's standard TLS verification (including hostname check) and add an
+	// extra comparison of the leaf certificate's raw bytes using VerifyConnection.
+
 	// Parse the expected certificate from the PEM data
 	block, _ := pem.Decode(pemData)
 	if block == nil {
@@ -260,21 +258,20 @@
 	}
 	// Store the raw certificate bytes (DER format) for comparison
 	expectedCertBytes := block.Bytes
-	
-	config.InsecureSkipVerify = true
-	config.VerifyPeerCertificate = func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error {
-		if len(rawCerts) == 0 {
+
+	config.VerifyConnection = func(cs tls.ConnectionState) error {
+		if len(cs.PeerCertificates) == 0 {
 			return fmt.Errorf("no peer certificates provided")
 		}
-		
+
 		// Compare the server's certificate bytes with the expected certificate bytes
 		// This matches the Microsoft.Data.SqlClient behavior: just compare raw bytes
-		serverCertBytes := rawCerts[0]
-		
+		serverCertBytes := cs.PeerCertificates[0].Raw
+
 		if !bytes.Equal(serverCertBytes, expectedCertBytes) {
 			return fmt.Errorf("server certificate doesn't match the provided certificate")
 		}
-		
+
 		return nil
 	}
 	return nil
EOF
@@ -246,13 +246,13 @@
func setupTLSCertificateOnly(config *tls.Config, pemData []byte) error {
// To match the behavior of Microsoft.Data.SqlClient, we simply compare the raw bytes
// of the server's certificate with the provided certificate file. This approach:
// - Does not validate certificate chain, expiry, or subject
// - Does not validate certificate chain, expiry, or subject beyond what Go's TLS
// stack already does by default
// - Only checks that the server's certificate exactly matches the provided certificate
// - Skips hostname validation (which is the intended behavior)
//
// We use InsecureSkipVerify=true with VerifyPeerCertificate callback because
// VerifyConnection runs AFTER standard verification (including hostname check).
// We rely on Go's standard TLS verification (including hostname check) and add an
// extra comparison of the leaf certificate's raw bytes using VerifyConnection.

// Parse the expected certificate from the PEM data
block, _ := pem.Decode(pemData)
if block == nil {
@@ -260,21 +258,20 @@
}
// Store the raw certificate bytes (DER format) for comparison
expectedCertBytes := block.Bytes

config.InsecureSkipVerify = true
config.VerifyPeerCertificate = func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error {
if len(rawCerts) == 0 {

config.VerifyConnection = func(cs tls.ConnectionState) error {
if len(cs.PeerCertificates) == 0 {
return fmt.Errorf("no peer certificates provided")
}

// Compare the server's certificate bytes with the expected certificate bytes
// This matches the Microsoft.Data.SqlClient behavior: just compare raw bytes
serverCertBytes := rawCerts[0]
serverCertBytes := cs.PeerCertificates[0].Raw

if !bytes.Equal(serverCertBytes, expectedCertBytes) {
return fmt.Errorf("server certificate doesn't match the provided certificate")
}

return nil
}
return nil
Copilot is powered by AI and may make mistakes. Always verify output.
config.VerifyPeerCertificate = func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error {
if len(rawCerts) == 0 {
return fmt.Errorf("no peer certificates provided")
}

// Compare the server's certificate bytes with the expected certificate bytes
// This matches the Microsoft.Data.SqlClient behavior: just compare raw bytes
serverCertBytes := rawCerts[0]

if !bytes.Equal(serverCertBytes, expectedCertBytes) {
return fmt.Errorf("server certificate doesn't match the provided certificate")
}

return nil
}
return nil
}

// Parse and handle encryption parameters. If encryption is desired, it returns the corresponding tls.Config object.
func parseTLS(params map[string]string, host string) (Encryption, *tls.Config, error) {
trustServerCert := false
Expand Down Expand Up @@ -261,10 +316,16 @@
certificate := params[Certificate]
if encryption != EncryptionDisabled {
tlsMin := params[TLSMin]
skipHostnameValidation := false
if encrypt == "strict" {
trustServerCert = false
}
tlsConfig, err := SetupTLS(certificate, trustServerCert, host, tlsMin)
// When a certificate is provided with any encryption mode (strict, true/required, mandatory),
// skip hostname validation. The certificate itself will still be validated against the provided CA
if len(certificate) > 0 {
skipHostnameValidation = true
}
tlsConfig, err := SetupTLS(certificate, trustServerCert, host, tlsMin, skipHostnameValidation)
if err != nil {
return encryption, nil, fmt.Errorf("failed to setup TLS: %w", err)
}
Expand Down Expand Up @@ -711,11 +772,11 @@
var parts []string
var current strings.Builder
inQuotes := false

runes := []rune(dsn)
for i := 0; i < len(runes); i++ {
char := runes[i]

if char == '"' {
if inQuotes && i+1 < len(runes) && runes[i+1] == '"' {
// Double quote escape sequence - add both quotes to current part
Expand All @@ -735,12 +796,12 @@
current.WriteRune(char)
}
}

// Add the last part if it's not empty
if current.Len() > 0 {
parts = append(parts, current.String())
}

return parts
}

Expand Down
60 changes: 48 additions & 12 deletions msdsn/conn_str_go115.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,58 @@ func setupTLSCommonName(config *tls.Config, pem []byte) error {
// fix for https://github.com/denisenkom/go-mssqldb/issues/704
// A SSL/TLS certificate Common Name (CN) containing the ":" character
// (which is a non-standard character) will cause normal verification to fail.
// Since the VerifyConnection callback runs after normal certificate
// verification, confirm that SetupTLS() has been called
// with "insecureSkipVerify=false", then InsecureSkipVerify must be set to true
// for this VerifyConnection callback to accomplish certificate verification.
// We use VerifyPeerCertificate to perform custom verification.
// This is required because standard TLS verification in Go doesn't handle ":" in CN.
//
// Security note: InsecureSkipVerify is safe here because:
// 1. The VerifyPeerCertificate callback performs full certificate chain validation
// 2. The certificate must be signed by the user-provided CA (in pem)
// 3. The CN is explicitly validated against the expected ServerName

// Create a certificate pool with the provided certificate as the root CA
roots := x509.NewCertPool()
roots.AppendCertsFromPEM(pem)

// We must use InsecureSkipVerify=true for this specific edge case because
// normal verification will fail for certificates with ":" in the CN.
// The VerifyPeerCertificate callback performs proper certificate chain verification.
// nosemgrep: go.lang.security.audit.net.use-tls.use-tls
config.InsecureSkipVerify = true
config.VerifyConnection = func(cs tls.ConnectionState) error {
commonName := cs.PeerCertificates[0].Subject.CommonName
if commonName != cs.ServerName {
return fmt.Errorf("invalid certificate name %q, expected %q", commonName, cs.ServerName)
config.VerifyPeerCertificate = func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error {
if len(rawCerts) == 0 {
return fmt.Errorf("no peer certificates provided")
}

// Parse the peer certificate
cert, err := x509.ParseCertificate(rawCerts[0])
if err != nil {
return fmt.Errorf("failed to parse certificate: %w", err)
}

// Check the common name matches the expected server name
commonName := cert.Subject.CommonName
if commonName != config.ServerName {
return fmt.Errorf("invalid certificate name %q, expected %q", commonName, config.ServerName)
}

// Build intermediates pool from the peer certificates (excluding the first one which is the server cert)
intermediates := x509.NewCertPool()
if len(rawCerts) > 1 {
for i := 1; i < len(rawCerts); i++ {
intermediateCert, err := x509.ParseCertificate(rawCerts[i])
if err != nil {
return fmt.Errorf("failed to parse intermediate certificate: %w", err)
}
intermediates.AddCert(intermediateCert)
}
}

// Verify the certificate chain against the provided root CA
opts := x509.VerifyOptions{
Roots: nil,
Intermediates: x509.NewCertPool(),
Roots: roots,
Intermediates: intermediates,
}
opts.Intermediates.AppendCertsFromPEM(pem)
_, err := cs.PeerCertificates[0].Verify(opts)
_, err = cert.Verify(opts)
return err
}
return nil
Expand Down
6 changes: 4 additions & 2 deletions msdsn/conn_str_go115pre.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@

package msdsn

import "crypto/tls"
import (
"crypto/tls"
)

func setupTLSCommonName(config *tls.Config, pem []byte) error {
func setupTLSCommonName(config *tls.Config, pemData []byte) error {
// Prior to Go 1.15, the TLS allowed ":" when checking the hostname.
// See https://golang.org/issue/40748 for details.
return skipSetup
Expand Down
Loading