-
Notifications
You must be signed in to change notification settings - Fork 91
Validate server certificate by byte comparison when certificate is provided #305
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
base: main
Are you sure you want to change the base?
Changes from 8 commits
244a007
c0a2a2e
8b0692d
c6eb984
afa3e49
edfd563
1c8901e
688fe04
dc5d261
cce0e41
4fa15a5
fac055e
415045c
99b21d1
768525c
b1647fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -111,12 +111,12 @@ func TestValidConnectionString(t *testing.T) { | |||||||||||||||||||||||||||||||
| {"MultiSubnetFailover=false", func(p Config) bool { return !p.MultiSubnetFailover }}, | ||||||||||||||||||||||||||||||||
| {"timezone=Asia/Shanghai", func(p Config) bool { return p.Encoding.Timezone.String() == "Asia/Shanghai" }}, | ||||||||||||||||||||||||||||||||
| {"Pwd=placeholder", func(p Config) bool { return p.Password == "placeholder" }}, | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // ADO connection string tests with double-quoted values containing semicolons | ||||||||||||||||||||||||||||||||
| {"server=test;password=\"pass;word\"", func(p Config) bool { return p.Host == "test" && p.Password == "pass;word" }}, | ||||||||||||||||||||||||||||||||
| {"password=\"[2+R2B6O:fF/[;]cJsr\"", func(p Config) bool { return p.Password == "[2+R2B6O:fF/[;]cJsr" }}, | ||||||||||||||||||||||||||||||||
| {"server=host;user id=user;password=\"complex;pass=word\"", func(p Config) bool { | ||||||||||||||||||||||||||||||||
| return p.Host == "host" && p.User == "user" && p.Password == "complex;pass=word" | ||||||||||||||||||||||||||||||||
| {"server=host;user id=user;password=\"complex;pass=word\"", func(p Config) bool { | ||||||||||||||||||||||||||||||||
| return p.Host == "host" && p.User == "user" && p.Password == "complex;pass=word" | ||||||||||||||||||||||||||||||||
| }}, | ||||||||||||||||||||||||||||||||
| {"password=\"value with \"\"quotes\"\" inside\"", func(p Config) bool { return p.Password == "value with \"quotes\" inside" }}, | ||||||||||||||||||||||||||||||||
| {"server=test;password=\"simple\"", func(p Config) bool { return p.Host == "test" && p.Password == "simple" }}, | ||||||||||||||||||||||||||||||||
|
|
@@ -125,19 +125,19 @@ func TestValidConnectionString(t *testing.T) { | |||||||||||||||||||||||||||||||
| return p.Host == "sql.database.windows.net" && p.Database == "MyDatabase" && p.User == "[email protected]" && p.Password == "[2+R2B6O:fF/[;]cJsr" | ||||||||||||||||||||||||||||||||
| }}, | ||||||||||||||||||||||||||||||||
| // Additional edge cases for double-quoted values | ||||||||||||||||||||||||||||||||
| {"password=\"\"", func(p Config) bool { return p.Password == "" }}, // Empty quoted password | ||||||||||||||||||||||||||||||||
| {"password=\";\"", func(p Config) bool { return p.Password == ";" }}, // Just a semicolon | ||||||||||||||||||||||||||||||||
| {"password=\";;\"", func(p Config) bool { return p.Password == ";;" }}, // Multiple semicolons | ||||||||||||||||||||||||||||||||
| {"password=\"\"", func(p Config) bool { return p.Password == "" }}, // Empty quoted password | ||||||||||||||||||||||||||||||||
| {"password=\";\"", func(p Config) bool { return p.Password == ";" }}, // Just a semicolon | ||||||||||||||||||||||||||||||||
| {"password=\";;\"", func(p Config) bool { return p.Password == ";;" }}, // Multiple semicolons | ||||||||||||||||||||||||||||||||
| {"server=\"host;name\";password=\"pass;word\"", func(p Config) bool { return p.Host == "host;name" && p.Password == "pass;word" }}, // Multiple quoted values | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Test cases with multibyte UTF-8 characters | ||||||||||||||||||||||||||||||||
| {"password=\"пароль;test\"", func(p Config) bool { return p.Password == "пароль;test" }}, // Cyrillic characters with semicolon | ||||||||||||||||||||||||||||||||
| {"password=\"пароль;test\"", func(p Config) bool { return p.Password == "пароль;test" }}, // Cyrillic characters with semicolon | ||||||||||||||||||||||||||||||||
| {"server=\"服务器;name\";password=\"密码;word\"", func(p Config) bool { return p.Host == "服务器;name" && p.Password == "密码;word" }}, // Chinese characters | ||||||||||||||||||||||||||||||||
| {"password=\"🔐;secret;🗝️\"", func(p Config) bool { return p.Password == "🔐;secret;🗝️" }}, // Emoji characters with semicolons | ||||||||||||||||||||||||||||||||
| {"user id=\"用户名\";password=\"пароль\"", func(p Config) bool { return p.User == "用户名" && p.Password == "пароль" }}, // Mixed multibyte chars | ||||||||||||||||||||||||||||||||
| {"password=\"测试\"\"密码\"\"\"", func(p Config) bool { return p.Password == "测试\"密码\"" }}, // Chinese chars with escaped quotes | ||||||||||||||||||||||||||||||||
| {"password=\"café;naïve;résumé\"", func(p Config) bool { return p.Password == "café;naïve;résumé" }}, // Accented characters | ||||||||||||||||||||||||||||||||
| {"password=\"🔐;secret;🗝️\"", func(p Config) bool { return p.Password == "🔐;secret;🗝️" }}, // Emoji characters with semicolons | ||||||||||||||||||||||||||||||||
| {"user id=\"用户名\";password=\"пароль\"", func(p Config) bool { return p.User == "用户名" && p.Password == "пароль" }}, // Mixed multibyte chars | ||||||||||||||||||||||||||||||||
| {"password=\"测试\"\"密码\"\"\"", func(p Config) bool { return p.Password == "测试\"密码\"" }}, // Chinese chars with escaped quotes | ||||||||||||||||||||||||||||||||
| {"password=\"café;naïve;résumé\"", func(p Config) bool { return p.Password == "café;naïve;résumé" }}, // Accented characters | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // those are supported currently, but maybe should not be | ||||||||||||||||||||||||||||||||
| {"someparam", func(p Config) bool { return true }}, | ||||||||||||||||||||||||||||||||
| {";;=;", func(p Config) bool { return true }}, | ||||||||||||||||||||||||||||||||
|
|
@@ -361,3 +361,52 @@ func TestReadCertificate(t *testing.T) { | |||||||||||||||||||||||||||||||
| assert.NotNil(t, err, "Expected error while reading certificate, found nil") | ||||||||||||||||||||||||||||||||
| assert.Nil(t, cert, "Expected certificate to be nil, found %v", cert) | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // TestStrictEncryptionWithCertificate tests that hostname validation is skipped | ||||||||||||||||||||||||||||||||
| // when a certificate is provided with encrypt=strict | ||||||||||||||||||||||||||||||||
| func TestStrictEncryptionWithCertificate(t *testing.T) { | ||||||||||||||||||||||||||||||||
| // Create a temporary certificate file for testing | ||||||||||||||||||||||||||||||||
| // This is a minimal self-signed certificate for testing purposes | ||||||||||||||||||||||||||||||||
| pemCert := `-----BEGIN CERTIFICATE----- | ||||||||||||||||||||||||||||||||
| MIIBkTCB+wIJAKHHCgVZU1tZMA0GCSqGSIb3DQEBBQUAMBExDzANBgNVBAMMBnNl | ||||||||||||||||||||||||||||||||
| cnZlcjAeFw0yMjA0MDQxMTIxNTNaFw0zMjA0MDExMTIxNTNaMBExDzANBgNVBAMM | ||||||||||||||||||||||||||||||||
| BnNlcnZlcjCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEAuTU1euiQCmLQG0z8 | ||||||||||||||||||||||||||||||||
| b/5pXNlWM6gGAMJklwO9jN8vGiWQGbQXPOMPqK8xDQqLOQnVEXrKJSfF2blHRneC | ||||||||||||||||||||||||||||||||
| qVmMNL7YSUEMxWdVaW3mQ4MzC6JgmWsxVrJeQEDZLdYVbQPXMGh5YtH5Ih8qTqJy | ||||||||||||||||||||||||||||||||
| e4MJwPMXEKlYVPJ3LE3E8pD6vXkCAwEAATANBgkqhkiG9w0BAQUFAAOBgQBHCqVT | ||||||||||||||||||||||||||||||||
| tZhWYXPHQFQgbKh6yvmhZfF8ZXHgZMhQQQwvqc0i5mvFpJpCQUQXAOkPGNPJANcV | ||||||||||||||||||||||||||||||||
| QSkVdAJg8mHKYGNZ6pIYMFr7RoBLGqMnKLPMYn3VqFvMccPx7A0hKQFJBR/qV8lh | ||||||||||||||||||||||||||||||||
| f0kGHKQEAFYGJLqJdK4KsGQDKLfZr9fqvXCCAA== | ||||||||||||||||||||||||||||||||
| -----END CERTIFICATE-----` | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| pemfile, _ := os.CreateTemp("", "*.pem") | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
| pemfile, _ := os.CreateTemp("", "*.pem") | |
| pemfile, err := os.CreateTemp("", "*.pem") | |
| if err != nil { | |
| t.Fatalf("failed to create temporary certificate file: %v", err) | |
| } | |
| ``` #Closed |
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.
Fixed in commit 415045c. Added error handling for os.CreateTemp.
Outdated
Copilot
AI
Jan 9, 2026
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.
Error handling is ignored when writing to the certificate file. If WriteString fails, the file may be incomplete and cause unexpected test failures. Consider checking the error: if _, err := pemfile.WriteString(pemCert); err != nil { t.Fatal(err) }
| pemfile, _ := os.CreateTemp("", "*.pem") | |
| defer os.Remove(pemfile.Name()) | |
| pemfile.WriteString(pemCert) | |
| pemfile.Close() | |
| pemfile, err := os.CreateTemp("", "*.pem") | |
| if err != nil { | |
| t.Fatal(err) | |
| } | |
| defer os.Remove(pemfile.Name()) | |
| if _, err := pemfile.WriteString(pemCert); err != nil { | |
| t.Fatal(err) | |
| } | |
| if err := pemfile.Close(); err != nil { | |
| t.Fatal(err) | |
| } |
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.
Fixed in commit 415045c. Added error handling for WriteString and Close.
Outdated
Copilot
AI
Jan 9, 2026
•
edited by shueybubbles
Loading
edited by shueybubbles
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.
Test 3 should verify that VerifyConnection is NOT set when encrypt=true (EncryptionRequired) with a certificate. Since skipHostnameValidation is only true for encrypt=strict, certificates provided with encrypt=true should go through normal hostname validation. The test should assert that config3.TLSConfig.VerifyConnection is nil to confirm hostname validation is not being skipped. #Closed
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.
Updated in commit 415045c. Test 3 now verifies that VerifyConnection IS set when encrypt=true with a certificate, since hostname validation is now skipped for all encryption modes when a certificate is provided (per @shueybubbles' request).
Uh oh!
There was an error while loading. Please reload this page.