mirror of
https://github.com/coder/coder.git
synced 2026-06-06 22:48:19 +00:00
Kyle Carberry
e3db203011
Fixes [CODAGT-372](https://linear.app/codercom/issue/CODAGT-372/coderdazureidentity-testvalidateregular-fails-on-macos). Closes coder/internal#101. ## Problem `coderd/azureidentity TestValidate/regular` fails on macOS with: ``` verify signature: github.com/coder/coder/v2/coderd/azureidentity.Validate /Users/runner/work/coder/coder/coderd/azureidentity/azureidentity.go:75 - x509: “metadata.azure.com” certificate is not standards compliant ``` When `crypto/x509.VerifyOptions.Roots` is `nil`, Go's verifier on macOS/iOS falls back to the system verifier (`systemVerify` in `crypto/x509/root_darwin.go`), which delegates to Apple's `SecTrustEvaluateWithError`. Apple's framework enforces stricter standards-compliance checks than Go's pure-Go verifier and rejects some otherwise valid Azure instance-identity leaf certificates with `errSecCertificateIsNotStandardsCompliant`, surfaced as the `not standards compliant` error. The test had been skipped on darwin since #12979 (April 2024) as a workaround. ## Fix - Embed the three root CAs that Azure instance-identity certificates ultimately chain to: - DigiCert Global Root G2 - DigiCert Global Root G3 - Baltimore CyberTrust Root (kept for historical chains via `Microsoft RSA TLS CA 01/02`) - In `Validate`, populate `options.Roots` from those embedded roots when the caller does not supply its own pool. Because `Roots != nil`, Go no longer takes the `systemVerify` path on darwin and uses the pure-Go verifier on all platforms. - Remove the `runtime.GOOS == "darwin"` skip from `TestValidate`. - Add `TestEmbeddedRoots` to guard against future regressions in the embedded root list (parses each PEM, asserts self-signed, requires all three named roots). The caller's existing `Intermediates` handling is unchanged. Tests that pass their own `Roots` (e.g. `coderdtest.NewAzureInstanceIdentity`) are unaffected. ## Verification On Linux: ``` $ go test ./coderd/azureidentity/ -race -count=1 -v === RUN TestValidate === RUN TestValidate/regular === RUN TestValidate/govcloud === RUN TestValidate/rsa --- PASS: TestValidate (0.00s) --- PASS: TestValidate/regular (0.00s) --- PASS: TestValidate/rsa (0.00s) --- PASS: TestValidate/govcloud (0.00s) === RUN TestEmbeddedRoots --- PASS: TestEmbeddedRoots (0.00s) === RUN TestExpiresSoon --- SKIP: TestExpiresSoon (0.00s) PASS ok github.com/coder/coder/v2/coderd/azureidentity 1.020s ``` The `test-go-pg` job on `macos-latest` in CI is the authoritative confirmation of the fix on macOS; previously it would have failed `TestValidate/regular` had the skip been removed. <details> <summary>Why this is the correct fix</summary> From `/usr/local/go/src/crypto/x509/verify.go`: ```go // Use platform verifiers, where available, if Roots is from SystemCertPool. if runtime.GOOS == "windows" || runtime.GOOS == "darwin" || runtime.GOOS == "ios" { systemPool := systemRootsPool() if opts.Roots == nil && (systemPool == nil || systemPool.systemPool) { return c.systemVerify(&opts) } ... } ``` Setting `opts.Roots` to any non-nil, non-system pool deterministically routes verification through Go's pure-Go verifier, bypassing Apple's stricter compliance checks. The embedded roots are sufficient to validate every chain we currently care about, since every intermediate in `Certificates` ultimately issues to one of the three embedded roots. </details> > Generated by Coder Agents. Reviewed manually.