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 <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>

diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go
index fa2785aacecc3bbb9f14bec5fc2c17e5ecb8445f..29d8e6bff7cd32365df9e5e0401e81b5869c2fd4 100644 (file)
--- 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 f32c39090023a9b66f4b4d587f34fec44fa0410a..47cddceacfd21c37612fdc624c170eaae778d74a 100644 (file)
--- 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)
-       }
-}