diff --git a/signer/local/local_test.go b/signer/local/local_test.go index 1cb36fba1..914763689 100644 --- a/signer/local/local_test.go +++ b/signer/local/local_test.go @@ -1636,3 +1636,307 @@ func TestLint(t *testing.T) { }) } } + +// createCSRWithExtensions generates a CSR containing the given raw X.509v3 +// extensions. This simulates an attacker injecting extensions into a CSR. +func createCSRWithExtensions(t *testing.T, extraExts []pkix.Extension) []byte { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("generating key: %v", err) + } + + tmpl := &x509.CertificateRequest{ + Subject: pkix.Name{CommonName: "test.example.com"}, + DNSNames: []string{"test.example.com"}, + ExtraExtensions: extraExts, + } + + csrDER, err := x509.CreateCertificateRequest(rand.Reader, tmpl, key) + if err != nil { + t.Fatalf("creating CSR: %v", err) + } + + return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrDER}) +} + +// marshalKeyUsage encodes a KeyUsage bitstring into the DER value expected +// inside an X.509v3 KeyUsage extension (OID 2.5.29.15). +func marshalKeyUsage(t *testing.T, ku x509.KeyUsage) []byte { + t.Helper() + // KeyUsage is a bit string where bits are numbered from the MSB. + // Go's x509.KeyUsage constants are already bit positions from MSB. + var b [2]byte + b[0] = byte(ku >> 8) // not used for standard usages below bit 8 + // Standard key usages fit in one byte for the common ones: + // digitalSignature(0), contentCommitment(1), keyEncipherment(2), + // dataEncipherment(3), keyAgreement(4), keyCertSign(5), cRLSign(6), + // encipherOnly(7) + b[0] = 0 + b[1] = 0 + if ku&x509.KeyUsageDigitalSignature != 0 { + b[0] |= 0x80 + } + if ku&x509.KeyUsageContentCommitment != 0 { + b[0] |= 0x40 + } + if ku&x509.KeyUsageKeyEncipherment != 0 { + b[0] |= 0x20 + } + if ku&x509.KeyUsageDataEncipherment != 0 { + b[0] |= 0x10 + } + if ku&x509.KeyUsageKeyAgreement != 0 { + b[0] |= 0x08 + } + if ku&x509.KeyUsageCertSign != 0 { + b[0] |= 0x04 + } + if ku&x509.KeyUsageCRLSign != 0 { + b[0] |= 0x02 + } + if ku&x509.KeyUsageEncipherOnly != 0 { + b[0] |= 0x01 + } + if ku&x509.KeyUsageDecipherOnly != 0 { + b[1] |= 0x80 + } + + // Determine padding bits: find the lowest set bit + var usedBytes int + var padBits int + if b[1] != 0 { + usedBytes = 2 + for i := 0; i < 8; i++ { + if b[1]&(1<= 5 && t[3] <= 11) || t[3] == 17)) } +// caMangedExtensionOIDs is the set of X.509v3 extension OIDs whose values are +// authoritatively determined by the CA's signing profile (via FillTemplate) and +// must never be copied from a CSR. Allowing a CSR to supply these via +// ExtraExtensions would silently override the profile because Go's +// x509.CreateCertificate gives ExtraExtensions precedence over struct fields +// for the same OID. +// +// See: https://pkg.go.dev/crypto/x509#Certificate (ExtraExtensions field). +var caManagedExtensionOIDs = map[string]bool{ + // Key Usage (RFC 5280, 4.2.1.3) — set by profile.Usages() + asn1.ObjectIdentifier{2, 5, 29, 15}.String(): true, + // Extended Key Usage (RFC 5280, 4.2.1.12) — set by profile.Usages() + asn1.ObjectIdentifier{2, 5, 29, 37}.String(): true, + // Basic Constraints (RFC 5280, 4.2.1.9) — already handled specially above, + // but included for defense-in-depth + asn1.ObjectIdentifier{2, 5, 29, 19}.String(): true, + // Subject Key Identifier (RFC 5280, 4.2.1.2) — computed by FillTemplate + asn1.ObjectIdentifier{2, 5, 29, 14}.String(): true, + // Authority Key Identifier (RFC 5280, 4.2.1.1) — set by CreateCertificate + // from the issuer + asn1.ObjectIdentifier{2, 5, 29, 35}.String(): true, + // Authority Info Access / OCSP (RFC 5280, 4.2.2.1) — set by profile OCSP URL + asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 1}.String(): true, + // CRL Distribution Points (RFC 5280, 4.2.1.13) — set by profile CRL URL + asn1.ObjectIdentifier{2, 5, 29, 31}.String(): true, + // Certificate Policies (RFC 5280, 4.2.1.4) — set by profile Policies + asn1.ObjectIdentifier{2, 5, 29, 32}.String(): true, + // Name Constraints (RFC 5280, 4.2.1.10) — CA-controlled + asn1.ObjectIdentifier{2, 5, 29, 30}.String(): true, + // Subject Alternative Name (RFC 5280, 4.2.1.6) — handled via Hosts/SANs + asn1.ObjectIdentifier{2, 5, 29, 17}.String(): true, + // Issuer Alternative Name (RFC 5280, 4.2.1.7) — CA-controlled + asn1.ObjectIdentifier{2, 5, 29, 18}.String(): true, +} + +// isCAManagedExtension reports whether the given OID is for an extension whose +// value is authoritatively set by the CA and must not be copied from a CSR. +func isCaManagedExtension(oid asn1.ObjectIdentifier) bool { + return caManagedExtensionOIDs[oid.String()] +} + // ParseCertificateRequest takes an incoming certificate request and // builds a certificate template from it. func ParseCertificateRequest(s Signer, p *config.SigningProfile, csrBytes []byte) (template *x509.Certificate, err error) { @@ -247,8 +289,16 @@ func ParseCertificateRequest(s Signer, p *config.SigningProfile, csrBytes []byte } else if val.Id.Equal(helpers.DelegationUsage) { template.ExtraExtensions = append(template.ExtraExtensions, val) } else { - // If the profile has 'copy_extensions' to true then lets add it + // If the profile has 'copy_extensions' to true then copy the + // extension, but never copy CA-managed extensions (KeyUsage, + // ExtKeyUsage, SKI, AKI, etc.) whose values are set by the + // signing profile. Allowing them through would let a CSR + // override the CA's policy via ExtraExtensions precedence. if p.CopyExtensions { + if isCaManagedExtension(val.Id) { + log.Warningf("copy_extensions: skipping CA-managed extension OID %s from CSR", val.Id) + continue + } template.ExtraExtensions = append(template.ExtraExtensions, val) } } diff --git a/signer/signer_test.go b/signer/signer_test.go index bab6ef745..acdea1ee2 100644 --- a/signer/signer_test.go +++ b/signer/signer_test.go @@ -107,6 +107,38 @@ func TestAddPoliciesWithQualifiers(t *testing.T) { } } +func TestIsCaManagedExtension(t *testing.T) { + tests := []struct { + name string + oid asn1.ObjectIdentifier + want bool + }{ + {"KeyUsage", asn1.ObjectIdentifier{2, 5, 29, 15}, true}, + {"ExtKeyUsage", asn1.ObjectIdentifier{2, 5, 29, 37}, true}, + {"BasicConstraints", asn1.ObjectIdentifier{2, 5, 29, 19}, true}, + {"SubjectKeyIdentifier", asn1.ObjectIdentifier{2, 5, 29, 14}, true}, + {"AuthorityKeyIdentifier", asn1.ObjectIdentifier{2, 5, 29, 35}, true}, + {"AuthorityInfoAccess", asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 1}, true}, + {"CRLDistributionPoints", asn1.ObjectIdentifier{2, 5, 29, 31}, true}, + {"CertificatePolicies", asn1.ObjectIdentifier{2, 5, 29, 32}, true}, + {"NameConstraints", asn1.ObjectIdentifier{2, 5, 29, 30}, true}, + {"SubjectAltName", asn1.ObjectIdentifier{2, 5, 29, 17}, true}, + {"IssuerAltName", asn1.ObjectIdentifier{2, 5, 29, 18}, true}, + {"private use OID", asn1.ObjectIdentifier{1, 2, 3, 4, 5}, false}, + {"DelegationUsage", asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 44363, 44}, false}, + {"CT Poison", asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 11129, 2, 4, 3}, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := isCaManagedExtension(tt.oid) + if got != tt.want { + t.Errorf("isCaManagedExtension(%v) = %v, want %v", tt.oid, got, tt.want) + } + }) + } +} + func TestName(t *testing.T) { sub := &Subject{ CN: "foobar",