Skip to content

Commit

Permalink
crypto/x509: revert Policies marshaling behavior
Browse files Browse the repository at this point in the history
Don't marshal Policies field.

Updates golang#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]>
  • Loading branch information
rolandshoemaker authored and ezz-no committed Feb 17, 2024
1 parent ffe3a52 commit 7683c49
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 54 deletions.
30 changes: 3 additions & 27 deletions src/crypto/x509/x509.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
})
Expand Down Expand Up @@ -1547,7 +1527,6 @@ var emptyASN1Subject = []byte{0x30, 0}
// - PermittedIPRanges
// - PermittedURIDomains
// - PolicyIdentifiers
// - Policies
// - SerialNumber
// - SignatureAlgorithm
// - Subject
Expand All @@ -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 {
Expand Down
27 changes: 0 additions & 27 deletions src/crypto/x509/x509_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
}

0 comments on commit 7683c49

Please sign in to comment.