From 7683c495fcbc1dd46e97e82f59fe9dc0fb9c29a3 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Mon, 4 Dec 2023 10:00:41 -0800 Subject: [PATCH] crypto/x509: revert Policies marshaling behavior Don't marshal Policies field. Updates #64248 Change-Id: I7e6d8b9ff1b3698bb4f585fa82fc4050eff3ae4d Reviewed-on: https://go-review.googlesource.com/c/go/+/546915 LUCI-TryBot-Result: Go LUCI Reviewed-by: Damien Neil --- src/crypto/x509/x509.go | 30 +++--------------------------- src/crypto/x509/x509_test.go | 27 --------------------------- 2 files changed, 3 insertions(+), 54 deletions(-) diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go index fa2785aacecc3b..29d8e6bff7cd32 100644 --- a/src/crypto/x509/x509.go +++ b/src/crypto/x509/x509.go @@ -1186,9 +1186,9 @@ func buildCertExtensions(template *Certificate, subjectIsEmpty bool, authorityKe n++ } - if (len(template.PolicyIdentifiers) > 0 || len(template.Policies) > 0) && + if len(template.PolicyIdentifiers) > 0 && !oidInExtensions(oidExtensionCertificatePolicies, template.ExtraExtensions) { - ret[n], err = marshalCertificatePolicies(template.Policies, template.PolicyIdentifiers) + ret[n], err = marshalCertificatePolicies(template.PolicyIdentifiers) if err != nil { return nil, err } @@ -1373,32 +1373,12 @@ func marshalBasicConstraints(isCA bool, maxPathLen int, maxPathLenZero bool) (pk return ext, err } -func marshalCertificatePolicies(policies []OID, policyIdentifiers []asn1.ObjectIdentifier) (pkix.Extension, error) { +func marshalCertificatePolicies(policyIdentifiers []asn1.ObjectIdentifier) (pkix.Extension, error) { ext := pkix.Extension{Id: oidExtensionCertificatePolicies} 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) }) @@ -1547,7 +1527,6 @@ var emptyASN1Subject = []byte{0x30, 0} // - PermittedIPRanges // - PermittedURIDomains // - PolicyIdentifiers -// - Policies // - SerialNumber // - SignatureAlgorithm // - Subject @@ -1571,9 +1550,6 @@ 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 f32c39090023a9..47cddceacfd21c 100644 --- a/src/crypto/x509/x509_test.go +++ b/src/crypto/x509/x509_test.go @@ -3928,25 +3928,14 @@ func TestCertificateOIDPolicies(t *testing.T) { NotBefore: time.Unix(1000, 0), 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}), - }, } var expectPolicyIdentifiers = []asn1.ObjectIdentifier{ []int{1, 2, 3}, - []int{1, 2, 3, 4, 5}, - []int{1, 2, 3, math.MaxInt32}, } 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}), } certDER, err := CreateCertificate(rand.Reader, &template, &template, rsaPrivateKey.Public(), rsaPrivateKey) @@ -3967,19 +3956,3 @@ 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) - } -}