diff --git a/coderd/azureidentity/azureidentity.go b/coderd/azureidentity/azureidentity.go index e4da9e54fc..7a06ccbc51 100644 --- a/coderd/azureidentity/azureidentity.go +++ b/coderd/azureidentity/azureidentity.go @@ -68,6 +68,20 @@ func Validate(ctx context.Context, signature string, options Options) (string, e options.Intermediates.AddCert(cert) } } + // Set Roots explicitly so we never fall back to the platform's system + // verifier (notably Apple's Security framework on macOS/iOS), which + // enforces stricter standards-compliance checks than Go's pure-Go + // verifier and rejects some otherwise valid Azure leaf certificates + // with errors like: + // x509: "metadata.azure.com" certificate is not standards compliant + // See https://github.com/coder/coder/issues/12978. + if options.Roots == nil { + roots, err := rootCertPool() + if err != nil { + return "", xerrors.Errorf("load roots: %w", err) + } + options.Roots = roots + } _, err = signer.Verify(options.VerifyOptions) if err != nil { @@ -115,6 +129,104 @@ func Validate(ctx context.Context, signature string, options Options) (string, e return metadata.VMID, nil } +// Roots are the root CAs that Azure instance-identity certificates chain to. +// These are embedded so verification works deterministically on all +// platforms, including macOS where the system verifier would otherwise be +// used and may reject otherwise valid Azure certificates due to stricter +// standards-compliance checks. See https://github.com/coder/coder/issues/12978. +var Roots = []string{ + // DigiCert Global Root G2 + `-----BEGIN CERTIFICATE----- +MIIDjjCCAnagAwIBAgIQAzrx5qcRqaC7KGSxHQn65TANBgkqhkiG9w0BAQsFADBh +MQswCQYDVQQGEwJVUzEVMBMGA1UEChMMRGlnaUNlcnQgSW5jMRkwFwYDVQQLExB3 +d3cuZGlnaWNlcnQuY29tMSAwHgYDVQQDExdEaWdpQ2VydCBHbG9iYWwgUm9vdCBH +MjAeFw0xMzA4MDExMjAwMDBaFw0zODAxMTUxMjAwMDBaMGExCzAJBgNVBAYTAlVT +MRUwEwYDVQQKEwxEaWdpQ2VydCBJbmMxGTAXBgNVBAsTEHd3dy5kaWdpY2VydC5j +b20xIDAeBgNVBAMTF0RpZ2lDZXJ0IEdsb2JhbCBSb290IEcyMIIBIjANBgkqhkiG +9w0BAQEFAAOCAQ8AMIIBCgKCAQEAuzfNNNx7a8myaJCtSnX/RrohCgiN9RlUyfuI +2/Ou8jqJkTx65qsGGmvPrC3oXgkkRLpimn7Wo6h+4FR1IAWsULecYxpsMNzaHxmx +1x7e/dfgy5SDN67sH0NO3Xss0r0upS/kqbitOtSZpLYl6ZtrAGCSYP9PIUkY92eQ +q2EGnI/yuum06ZIya7XzV+hdG82MHauVBJVJ8zUtluNJbd134/tJS7SsVQepj5Wz +tCO7TG1F8PapspUwtP1MVYwnSlcUfIKdzXOS0xZKBgyMUNGPHgm+F6HmIcr9g+UQ +vIOlCsRnKPZzFBQ9RnbDhxSJITRNrw9FDKZJobq7nMWxM4MphQIDAQABo0IwQDAP +BgNVHRMBAf8EBTADAQH/MA4GA1UdDwEB/wQEAwIBhjAdBgNVHQ4EFgQUTiJUIBiV +5uNu5g/6+rkS7QYXjzkwDQYJKoZIhvcNAQELBQADggEBAGBnKJRvDkhj6zHd6mcY +1Yl9PMWLSn/pvtsrF9+wX3N3KjITOYFnQoQj8kVnNeyIv/iPsGEMNKSuIEyExtv4 +NeF22d+mQrvHRAiGfzZ0JFrabA0UWTW98kndth/Jsw1HKj2ZL7tcu7XUIOGZX1NG +Fdtom/DzMNU+MeKNhJ7jitralj41E6Vf8PlwUHBHQRFXGU7Aj64GxJUTFy8bJZ91 +8rGOmaFvE7FBcf6IKshPECBV1/MUReXgRPTqh5Uykw7+U0b6LJ3/iyK5S9kJRaTe +pLiaWN0bfVKfjllDiIGknibVb63dDcY3fe0Dkhvld1927jyNxF1WW6LZZm6zNTfl +MrY= +-----END CERTIFICATE-----`, + // DigiCert Global Root G3 + `-----BEGIN CERTIFICATE----- +MIICPzCCAcWgAwIBAgIQBVVWvPJepDU1w6QP1atFcjAKBggqhkjOPQQDAzBhMQsw +CQYDVQQGEwJVUzEVMBMGA1UEChMMRGlnaUNlcnQgSW5jMRkwFwYDVQQLExB3d3cu +ZGlnaWNlcnQuY29tMSAwHgYDVQQDExdEaWdpQ2VydCBHbG9iYWwgUm9vdCBHMzAe +Fw0xMzA4MDExMjAwMDBaFw0zODAxMTUxMjAwMDBaMGExCzAJBgNVBAYTAlVTMRUw +EwYDVQQKEwxEaWdpQ2VydCBJbmMxGTAXBgNVBAsTEHd3dy5kaWdpY2VydC5jb20x +IDAeBgNVBAMTF0RpZ2lDZXJ0IEdsb2JhbCBSb290IEczMHYwEAYHKoZIzj0CAQYF +K4EEACIDYgAE3afZu4q4C/sLfyHS8L6+c/MzXRq8NOrexpu80JX28MzQC7phW1FG +fp4tn+6OYwwX7Adw9c+ELkCDnOg/QW07rdOkFFk2eJ0DQ+4QE2xy3q6Ip6FrtUPO +Z9wj/wMco+I+o0IwQDAPBgNVHRMBAf8EBTADAQH/MA4GA1UdDwEB/wQEAwIBhjAd +BgNVHQ4EFgQUs9tIpPmhxdiuNkHMEWNpYim8S8YwCgYIKoZIzj0EAwMDaAAwZQIx +AK288mw/EkrRLTnDCgmXc/SINoyIJ7vmiI1Qhadj+Z4y3maTD/HMsQmP3Wyr+mt/ +oAIwOWZbwmSNuJ5Q3KjVSaLtx9zRSX8XAbjIho9OjIgrqJqpisXRAL34VOKa5Vt8 +sycX +-----END CERTIFICATE-----`, + // Baltimore CyberTrust Root. + // Required for chains rooted here, e.g. "Microsoft RSA TLS CA 01/02". + // Expired 2025-05-12 but kept so callers that pass a CurrentTime + // before the expiry can still verify historical signatures. + `-----BEGIN CERTIFICATE----- +MIIDdzCCAl+gAwIBAgIEAgAAuTANBgkqhkiG9w0BAQUFADBaMQswCQYDVQQGEwJJ +RTESMBAGA1UEChMJQmFsdGltb3JlMRMwEQYDVQQLEwpDeWJlclRydXN0MSIwIAYD +VQQDExlCYWx0aW1vcmUgQ3liZXJUcnVzdCBSb290MB4XDTAwMDUxMjE4NDYwMFoX +DTI1MDUxMjIzNTkwMFowWjELMAkGA1UEBhMCSUUxEjAQBgNVBAoTCUJhbHRpbW9y +ZTETMBEGA1UECxMKQ3liZXJUcnVzdDEiMCAGA1UEAxMZQmFsdGltb3JlIEN5YmVy +VHJ1c3QgUm9vdDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAKMEuyKr +mD1X6CZymrV51Cni4eiVgLGw41uOKymaZN+hXe2wCQVt2yguzmKiYv60iNoS6zjr +IZ3AQSsBUnuId9Mcj8e6uYi1agnnc+gRQKfRzMpijS3ljwumUNKoUMMo6vWrJYeK +mpYcqWe4PwzV9/lSEy/CG9VwcPCPwBLKBsua4dnKM3p31vjsufFoREJIE9LAwqSu +XmD+tqYF/LTdB1kC1FkYmGP1pWPgkAx9XbIGevOF6uvUA65ehD5f/xXtabz5OTZy +dc93Uk3zyZAsuT3lySNTPx8kmCFcB5kpvcY67Oduhjprl3RjM71oGDHweI12v/ye +jl0qhqdNkNwnGjkCAwEAAaNFMEMwHQYDVR0OBBYEFOWdWTCCR1jMrPoIVDaGezq1 +BE3wMBIGA1UdEwEB/wQIMAYBAf8CAQMwDgYDVR0PAQH/BAQDAgEGMA0GCSqGSIb3 +DQEBBQUAA4IBAQCFDF2O5G9RaEIFoN27TyclhAO992T9Ldcw46QQF+vaKSm2eT92 +9hkTI7gQCvlYpNRhcL0EYWoSihfVCr3FvDB81ukMJY2GQE/szKN+OMY3EU/t3Wgx +jkzSswF07r51XgdIGn9w/xZchMB5hbgF/X++ZRGjD8ACtPhSNzkE1akxehi/oCr0 +Epn3o0WC4zxe9Z2etciefC7IpJ5OCBRLbf1wbWsaY71k5h+3zvDyny67G7fyUIhz +ksLi4xaNmjICq44Y3ekQEe5+NauQrz4wlHrQMz2nZQ/1/I6eYs9HRCwBXbsdtTLS +R9I4LtD+gdwyah617jzV/OeBHRnDJELqYzmp +-----END CERTIFICATE-----`, +} + +// rootCertPool returns a CertPool containing the root CAs that Azure +// instance-identity certificates ultimately chain to. We embed these so +// callers do not have to populate Roots themselves, and so we never +// implicitly fall back to the platform's system verifier (notably Apple's +// Security framework on macOS/iOS) which enforces stricter standards- +// compliance checks than Go's pure-Go verifier and rejects some otherwise +// valid Azure leaf certificates. +var rootCertPool = sync.OnceValues(func() (*x509.CertPool, error) { + pool := x509.NewCertPool() + for _, pemCert := range Roots { + block, rest := pem.Decode([]byte(pemCert)) + if block == nil { + return nil, xerrors.New("root: failed to decode PEM block") + } + if len(rest) != 0 { + return nil, xerrors.Errorf("root: invalid certificate, %d bytes remain", len(rest)) + } + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return nil, xerrors.Errorf("root: parse certificate: %w", err) + } + pool.AddCert(cert) + } + return pool, nil +}) + // Certificates are manually downloaded from Azure, then processed with OpenSSL // and added here. See: https://learn.microsoft.com/en-us/azure/security/fundamentals/azure-ca-details // diff --git a/coderd/azureidentity/azureidentity_test.go b/coderd/azureidentity/azureidentity_test.go index bd94f836be..93627ff927 100644 --- a/coderd/azureidentity/azureidentity_test.go +++ b/coderd/azureidentity/azureidentity_test.go @@ -4,7 +4,6 @@ import ( "context" "crypto/x509" "encoding/pem" - "runtime" "testing" "time" @@ -15,10 +14,6 @@ import ( func TestValidate(t *testing.T) { t.Parallel() - if runtime.GOOS == "darwin" { - // This test fails on MacOS for some reason. See https://github.com/coder/coder/issues/12978 - t.Skip() - } mustTime := func(layout string, value string) time.Time { ti, err := time.Parse(layout, value) @@ -61,6 +56,40 @@ func TestValidate(t *testing.T) { } } +// TestEmbeddedRoots ensures the package's embedded root certificates parse +// successfully. The Roots are used by Validate to avoid falling back to the +// platform's system verifier (notably Apple's Security framework on macOS), +// which previously caused TestValidate/regular to fail on macOS with +// `x509: "metadata.azure.com" certificate is not standards compliant`. +// See https://github.com/coder/coder/issues/12978. +func TestEmbeddedRoots(t *testing.T) { + t.Parallel() + require.NotEmpty(t, azureidentity.Roots, "embedded roots must not be empty") + seen := map[string]bool{} + for _, pemCert := range azureidentity.Roots { + block, rest := pem.Decode([]byte(pemCert)) + require.NotNil(t, block, "PEM block should decode") + require.Zero(t, len(rest), "no trailing data after PEM block") + cert, err := x509.ParseCertificate(block.Bytes) + require.NoError(t, err) + // Each root must be self-signed (issuer == subject). + require.Equal(t, cert.Issuer.String(), cert.Subject.String(), + "root certificate must be self-signed: %s", cert.Subject.CommonName) + require.False(t, seen[cert.Subject.CommonName], + "duplicate embedded root: %s", cert.Subject.CommonName) + seen[cert.Subject.CommonName] = true + } + // Verify the three roots Azure instance-identity chains ultimately + // terminate at are all present. + for _, name := range []string{ + "DigiCert Global Root G2", + "DigiCert Global Root G3", + "Baltimore CyberTrust Root", + } { + require.True(t, seen[name], "missing embedded root %q", name) + } +} + func TestExpiresSoon(t *testing.T) { t.Parallel() // TODO (@kylecarbs): It's unknown why Microsoft does not have new certificates live...