Skip to content

Commit a11a885

Browse files
rolandshoemakergopherbot
authored andcommitted
crypto/x509: reject duplicate extensions
When parsing certificates and CSRs, reject duplicate extensions (and additionally duplicate requested extensions in CSRs.) Fixes #50988 Change-Id: I531e932cfcdde78f64c106e747a68270bd4f1d80 Reviewed-on: https://go-review.googlesource.com/c/go/+/383215 Reviewed-by: Damien Neil <[email protected]> Run-TryBot: Roland Shoemaker <[email protected]> Auto-Submit: Roland Shoemaker <[email protected]> Reviewed-by: Roland Shoemaker <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent c5edd5f commit a11a885

File tree

3 files changed

+66
-0
lines changed

3 files changed

+66
-0
lines changed

src/crypto/x509/parser.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -930,6 +930,7 @@ func parseCertificate(der []byte) (*Certificate, error) {
930930
return nil, errors.New("x509: malformed extensions")
931931
}
932932
if present {
933+
seenExts := make(map[string]bool)
933934
if !extensions.ReadASN1(&extensions, cryptobyte_asn1.SEQUENCE) {
934935
return nil, errors.New("x509: malformed extensions")
935936
}
@@ -942,6 +943,11 @@ func parseCertificate(der []byte) (*Certificate, error) {
942943
if err != nil {
943944
return nil, err
944945
}
946+
oidStr := ext.Id.String()
947+
if seenExts[oidStr] {
948+
return nil, errors.New("x509: certificate contains duplicate extensions")
949+
}
950+
seenExts[oidStr] = true
945951
cert.Extensions = append(cert.Extensions, ext)
946952
}
947953
err = processExtensions(cert)

src/crypto/x509/x509.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1825,12 +1825,18 @@ func parseCSRExtensions(rawAttributes []asn1.RawValue) ([]pkix.Extension, error)
18251825
}
18261826

18271827
var ret []pkix.Extension
1828+
seenExts := make(map[string]bool)
18281829
for _, rawAttr := range rawAttributes {
18291830
var attr pkcs10Attribute
18301831
if rest, err := asn1.Unmarshal(rawAttr.FullBytes, &attr); err != nil || len(rest) != 0 || len(attr.Values) == 0 {
18311832
// Ignore attributes that don't parse.
18321833
continue
18331834
}
1835+
oidStr := attr.Id.String()
1836+
if seenExts[oidStr] {
1837+
return nil, errors.New("x509: certificate request contains duplicate extensions")
1838+
}
1839+
seenExts[oidStr] = true
18341840

18351841
if !attr.Id.Equal(oidExtensionRequest) {
18361842
continue
@@ -1840,6 +1846,14 @@ func parseCSRExtensions(rawAttributes []asn1.RawValue) ([]pkix.Extension, error)
18401846
if _, err := asn1.Unmarshal(attr.Values[0].FullBytes, &extensions); err != nil {
18411847
return nil, err
18421848
}
1849+
requestedExts := make(map[string]bool)
1850+
for _, ext := range extensions {
1851+
oidStr := ext.Id.String()
1852+
if requestedExts[oidStr] {
1853+
return nil, errors.New("x509: certificate request contains duplicate requested extensions")
1854+
}
1855+
requestedExts[oidStr] = true
1856+
}
18431857
ret = append(ret, extensions...)
18441858
}
18451859

src/crypto/x509/x509_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3661,3 +3661,49 @@ func TestCreateNegativeSerial(t *testing.T) {
36613661
t.Errorf("CreateCertificate returned unexpected error: want %q, got %q", expectedErr, err)
36623662
}
36633663
}
3664+
3665+
const dupExtCert = `-----BEGIN CERTIFICATE-----
3666+
MIIBrjCCARegAwIBAgIBATANBgkqhkiG9w0BAQsFADAPMQ0wCwYDVQQDEwR0ZXN0
3667+
MCIYDzAwMDEwMTAxMDAwMDAwWhgPMDAwMTAxMDEwMDAwMDBaMA8xDTALBgNVBAMT
3668+
BHRlc3QwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAMiFchnHms9l9NninAIz
3669+
SkY9acwl9Bk2AtmJrNCenFpiA17AcOO5q8DJYwdXi6WPKlVgcyH+ysW8XMWkq+CP
3670+
yhtF/+LMzl9odaUF2iUy3vgTC5gxGLWH5URVssx21Und2Pm2f4xyou5IVxbS9dxy
3671+
jLvV9PEY9BIb0H+zFthjhihDAgMBAAGjFjAUMAgGAioDBAIFADAIBgIqAwQCBQAw
3672+
DQYJKoZIhvcNAQELBQADgYEAlhQ4TQQKIQ8GUyzGiN/75TCtQtjhMGemxc0cNgre
3673+
d9rmm4DjydH0t7/sMCB56lQrfhJNplguzsbjFW4l245KbNKHfLiqwEGUgZjBNKur
3674+
ot6qX/skahLtt0CNOaFIge75HVKe/69OrWQGdp18dkay/KS4Glu8YMKIjOhfrUi1
3675+
NZA=
3676+
-----END CERTIFICATE-----`
3677+
3678+
func TestDuplicateExtensionsCert(t *testing.T) {
3679+
b, _ := pem.Decode([]byte(dupExtCert))
3680+
if b == nil {
3681+
t.Fatalf("couldn't decode test certificate")
3682+
}
3683+
_, err := ParseCertificate(b.Bytes)
3684+
if err == nil {
3685+
t.Fatal("ParseCertificate should fail when parsing certificate with duplicate extensions")
3686+
}
3687+
}
3688+
3689+
const dupExtCSR = `-----BEGIN CERTIFICATE REQUEST-----
3690+
MIIBczCB3QIBADAPMQ0wCwYDVQQDEwR0ZXN0MIGfMA0GCSqGSIb3DQEBAQUAA4GN
3691+
ADCBiQKBgQC5PbxMGVJ8aLF9lq/EvGObXTRMB7ieiZL9N+DJZg1n/ECCnZLIvYrr
3692+
ZmmDV7YZsClgxKGfjJB0RQFFyZElFM9EfHEs8NJdidDKCRdIhDXQWRyhXKevHvdm
3693+
CQNKzUeoxvdHpU/uscSkw6BgUzPyLyTx9A6ye2ix94z8Y9hGOBO2DQIDAQABoCUw
3694+
IwYJKoZIhvcNAQkOMRYwFDAIBgIqAwQCBQAwCAYCKgMEAgUAMA0GCSqGSIb3DQEB
3695+
CwUAA4GBAHROEsE7URk1knXmBnQtIHwoq663vlMcX3Hes58pUy020rWP8QkocA+X
3696+
VF18/phg3p5ILlS4fcbbP2bEeV0pePo2k00FDPsJEKCBAX2LKxbU7Vp2OuV2HM2+
3697+
VLOVx0i+/Q7fikp3hbN1JwuMTU0v2KL/IKoUcZc02+5xiYrnOIt5
3698+
-----END CERTIFICATE REQUEST-----`
3699+
3700+
func TestDuplicateExtensionsCSR(t *testing.T) {
3701+
b, _ := pem.Decode([]byte(dupExtCSR))
3702+
if b == nil {
3703+
t.Fatalf("couldn't decode test certificate")
3704+
}
3705+
_, err := ParseCertificateRequest(b.Bytes)
3706+
if err == nil {
3707+
t.Fatal("ParseCertificate should fail when parsing certificate with duplicate extensions")
3708+
}
3709+
}

0 commit comments

Comments
 (0)