fix: add actionable guidance for TLS certificate failures

[David Levy (dlevy-msft-sql)]

Problem

TLS handshake failures caused by invalid certificates or obsolete TLS configuration provide little remediation guidance:

• Certificates with negative serial numbers do not comply with RFC 5280. SQL Server Docker images may generate these certificates. (Go 1.23: Unable to connect to SQL Server 2022 docker image with TLS error #217)
• Servers using SHA-1 signatures do not comply with current TLS requirements in RFC 9155. (Go 1.25 "TLS Handshake failed: cannot read handshake packet: EOF" #302)

Root cause

The driver returned the underlying TLS error without identifying the server-side certificate or TLS configuration change needed to resolve it.

Solution

Recognize these certificate and TLS failures and put permanent server-side remediation first. The errors recommend replacing invalid certificates or updating obsolete TLS configuration. Where supported, they also identify GODEBUG settings as temporary compatibility options.

Changes

File	Change
tds.go	Add wrapTLSError, apply it at both TLS handshake sites, and preserve wrapped errors with %w
tds_unit_test.go	Cover each recognized failure, fallback behavior, error unwrapping, and both handshake paths

Testing

• go build ./...
• go test -count=1 -run 'TestWrapTLSError|TestGetTLSConnHandshakeError|TestConnectNonStrictTLSHandshakeError' .
• CI covers the full SQL Server and Go version matrix

Related issues

Closes #302
Closes #217

[Codecov Comments Bot (codecov-commenter)]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.31%. Comparing base (60674a5) to head (abea722).

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #356       +/-   ##
===========================================
+ Coverage   81.66%   96.31%   +14.65%     
===========================================
  Files          34       91       +57     
  Lines        7056    75317    +68261     
===========================================
+ Hits         5762    72539    +66777     
- Misses       1027     2758     +1731     
+ Partials      267       20      -247     

Flag	Coverage Δ
unittests	96.31% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
tds.go	81.31% <100.00%> (+6.89%)	⬆️

... and 74 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

Improves the driver’s TLS handshake failure diagnostics by wrapping handshake errors with actionable guidance (including relevant GODEBUG workarounds) for known Go crypto/tls policy changes, helping users self-remediate certificate-related failures.

Changes:

• Add wrapTLSError() in tds.go to recognize specific TLS/cert failure strings and append remediation guidance.
• Apply the wrapper at both TLS handshake sites (getTLSConn() and the standard encryption path in connect()), ensuring proper error wrapping (%w).
• Add unit tests covering the known patterns and default behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
tds.go	Introduces wrapTLSError() and uses it at both TLS handshake call sites to enrich errors with guidance.
tds_unit_test.go	Adds unit tests validating wrapping behavior, message contents, and errors.Is unwrapping.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Copilot]

🔵 Needs a closer look

SHA-1 matching misses Go’s hyphenated SHA-1 format, leaving the intended guidance unreachable.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[David Levy (dlevy-msft-sql)]

Refuted: Go 1.25's crypto/x509.InsecureAlgorithmError formats SHA-1 algorithms as SHA1-RSA and ECDSA-SHA1, as asserted in the standard library's X.509 tests. Both lowercase to strings containing sha1, so the current matcher reaches this branch for both standard forms. The unit test constructs x509.InsecureAlgorithmError(x509.SHA1WithRSA), which exercises the actual formatter rather than synthetic text.

[Copilot]

Copilot review overview

🔵 Needs a closer look

SHA-1 errors are not recognized correctly, causing the related test and remediation guidance to fail.

Review effort: Lite
Findings: None

[Copilot]

Copilot review overview

🔵 Needs a closer look

The SHA-1 guidance branch is unreachable for the reported standard-library error format.

Review effort: Lite
Findings: None

[Copilot]

Copilot review overview

🟢 Approval recommended

The changes are covered by tests and introduce no unresolved blocking issues.

Review effort: Lite
Findings: None

[Copilot]

Copilot review overview

🔵 Needs a closer look

SHA-1 matching misses Go’s SHA-1 spelling, so the intended guidance is not emitted.

Review effort: Lite
Findings: None

[Copilot]

Copilot review overview

🔵 Needs a closer look

SHA-1 matching misses Go’s SHA-1 spelling, causing the expected guidance test to fail.

Review effort: Lite
Findings: None

[David Levy (dlevy-msft-sql)]

The benchmarks check fails on ConvertAssign_BoolToBool (+23.60%, p=0.000). I investigated it instead of re-running past it. It is a code-layout artifact, not a performance change from this PR.

Evidence

wrapTLSError is a new package-level function in tds.go, called from exactly two places, both TLS handshake error paths. convertAssign lives in convert.go and has no call path to anything this PR touches. The diff is 253 lines across tds.go and tds_unit_test.go.

I built the test binary from origin/main and from this branch, then disassembled convertAssign in both:

	main	PR
instruction count	1533	1533
symbol address	0x14047bac0	0x14047bfc0
differing instruction lines	205 of 1533

Same instruction count, same sequence, same mnemonics. Every one of the 205 differing lines is a relocation operand: 80 LEAQ, 78 CALL, 27 MOVQ, 18 CMPL, 1 MOVZX. Those are call displacements and PC-relative addresses of symbols that moved. Adding 37 lines to tds.go pushes everything after it by 1280 bytes. The compiled body of convertAssign is unchanged.

Local A/B on those same two binaries, -cpu=4 -benchtime=1s -count=10:

	ns/op
main	19.5
PR	19.1

No regression locally. The PR side is marginally faster.

Why CI sees it

The job builds the baseline from a separate origin/main worktree, so the two sides are different binaries with different symbol placement. ConvertAssign_BoolToBool is a 13ns operation carrying one allocation, small enough that x86 code alignment moves the number by 20% or more. Both failing runs show ±0-1% variance within each side and a stable gap between sides, which is the shape of a deterministic layout difference rather than runner drift. Everything else in the comparison is ~, geomean -0.35%.

Runs: job 107153657614 (+22.32%) and job 107347674431 (+23.60%). Same benchmark, same magnitude, so re-running again will not clear it.

What I did not change

I did not widen the 15% threshold and did not add ConvertAssign_BoolToBool to the exclusion list. Either would make the check pass without making the measurement any more trustworthy. That decision is yours. The gate is behaving as written, and changing pr-validation.yml is outside this PR's scope.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Fix SHA-1 error matching and add strict-path coverage for recognized TLS failures.

Review effort: Lite
Findings: None

[David Levy (dlevy-msft-sql)]

Third reproduction, now on head abea722 (run job 107514971808):

ConvertAssign_BoolToBool-4    13.77n ± 1%   16.79n ± 0%  +22.01% (p=0.000 n=10)
geomean                       265.9n        264.6n        -0.50%

Same benchmark, same magnitude (+22.32%, +23.60%, +22.01% across three runs on three different heads), everything else ~, geomean still negative. This confirms the prediction in my earlier comment: re-running will not clear it, because the cause is deterministic code placement, not runner drift.

Nothing in this PR touches convertAssign or anything it calls. I have not changed the benchmark, the gate threshold, or the exclusion list. The call is yours on whether to widen the gate, exclude this benchmark, or override.