diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go index c7106553043b1..fa2785aacecc3 100644 --- a/src/crypto/x509/x509.go +++ b/src/crypto/x509/x509.go @@ -1186,7 +1186,7 @@ func buildCertExtensions(template *Certificate, subjectIsEmpty bool, authorityKe n++ } - if len(template.PolicyIdentifiers) > 0 && + if (len(template.PolicyIdentifiers) > 0 || len(template.Policies) > 0) && !oidInExtensions(oidExtensionCertificatePolicies, template.ExtraExtensions) { ret[n], err = marshalCertificatePolicies(template.Policies, template.PolicyIdentifiers) if err != nil { @@ -1378,14 +1378,27 @@ func marshalCertificatePolicies(policies []OID, policyIdentifiers []asn1.ObjectI b := cryptobyte.NewBuilder(make([]byte, 0, 128)) b.AddASN1(cryptobyte_asn1.SEQUENCE, func(child *cryptobyte.Builder) { + // added is used to track OIDs which are duplicated in both Policies and PolicyIdentifiers + // so they can be skipped. Note that this explicitly doesn't check for duplicate OIDs in + // Policies or in PolicyIdentifiers themselves, as this would be considered breaking behavior. + added := map[string]bool{} for _, v := range policies { child.AddASN1(cryptobyte_asn1.SEQUENCE, func(child *cryptobyte.Builder) { child.AddASN1(cryptobyte_asn1.OBJECT_IDENTIFIER, func(child *cryptobyte.Builder) { + oidStr := v.String() + added[oidStr] = true + if len(v.der) == 0 { + child.SetError(errors.New("invalid policy object identifier")) + return + } child.AddBytes(v.der) }) }) } for _, v := range policyIdentifiers { + if added[v.String()] { + continue + } child.AddASN1(cryptobyte_asn1.SEQUENCE, func(child *cryptobyte.Builder) { child.AddASN1ObjectIdentifier(v) }) @@ -1534,6 +1547,7 @@ var emptyASN1Subject = []byte{0x30, 0} // - PermittedIPRanges // - PermittedURIDomains // - PolicyIdentifiers +// - Policies // - SerialNumber // - SignatureAlgorithm // - Subject @@ -1557,6 +1571,9 @@ var emptyASN1Subject = []byte{0x30, 0} // // If SubjectKeyId from template is empty and the template is a CA, SubjectKeyId // will be generated from the hash of the public key. +// +// If both PolicyIdentifiers and Policies are populated, any OID which appears +// in both slices will only be added to the certificate policies extension once. func CreateCertificate(rand io.Reader, template, parent *Certificate, pub, priv any) ([]byte, error) { key, ok := priv.(crypto.Signer) if !ok { diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go index bdc03216bcbc6..f32c39090023a 100644 --- a/src/crypto/x509/x509_test.go +++ b/src/crypto/x509/x509_test.go @@ -3929,6 +3929,7 @@ func TestCertificateOIDPolicies(t *testing.T) { NotAfter: time.Unix(100000, 0), PolicyIdentifiers: []asn1.ObjectIdentifier{[]int{1, 2, 3}}, Policies: []OID{ + mustNewOIDFromInts(t, []uint64{1, 2, 3}), mustNewOIDFromInts(t, []uint64{1, 2, 3, 4, 5}), mustNewOIDFromInts(t, []uint64{1, 2, 3, math.MaxInt32}), mustNewOIDFromInts(t, []uint64{1, 2, 3, math.MaxUint32, math.MaxUint64}), @@ -3936,16 +3937,16 @@ func TestCertificateOIDPolicies(t *testing.T) { } var expectPolicyIdentifiers = []asn1.ObjectIdentifier{ + []int{1, 2, 3}, []int{1, 2, 3, 4, 5}, []int{1, 2, 3, math.MaxInt32}, - []int{1, 2, 3}, } var expectPolicies = []OID{ + mustNewOIDFromInts(t, []uint64{1, 2, 3}), mustNewOIDFromInts(t, []uint64{1, 2, 3, 4, 5}), mustNewOIDFromInts(t, []uint64{1, 2, 3, math.MaxInt32}), mustNewOIDFromInts(t, []uint64{1, 2, 3, math.MaxUint32, math.MaxUint64}), - mustNewOIDFromInts(t, []uint64{1, 2, 3}), } certDER, err := CreateCertificate(rand.Reader, &template, &template, rsaPrivateKey.Public(), rsaPrivateKey) @@ -3966,3 +3967,19 @@ func TestCertificateOIDPolicies(t *testing.T) { t.Errorf("cert.Policies = %v, want: %v", cert.Policies, expectPolicies) } } + +func TestInvalidPolicyOID(t *testing.T) { + template := Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "Cert"}, + NotBefore: time.Now(), + NotAfter: time.Now().Add(time.Hour), + PolicyIdentifiers: []asn1.ObjectIdentifier{[]int{1, 2, 3}}, + Policies: []OID{OID{}}, + } + _, err := CreateCertificate(rand.Reader, &template, &template, rsaPrivateKey.Public(), rsaPrivateKey) + expected := "invalid policy object identifier" + if err.Error() != expected { + t.Fatalf("CreateCertificate() unexpected error: %v, want: %v", err, expected) + } +}