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 <[email protected]>
Reviewed-by: Damien Neil <[email protected]>

Author: rolandshoemaker

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 {

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)
-	}
-}