From ca98bd6f5c9c56ecf15029d32ad2f59f339ff3ff Mon Sep 17 00:00:00 2001 From: Emman Date: Tue, 19 Apr 2022 16:29:41 +0800 Subject: [PATCH] crypto/x509: reject duplicate extensions #48 --- smx509/parser.go | 6 ++++++ smx509/x509.go | 29 ++++++++++++++----------- smx509/x509_test.go | 52 ++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 72 insertions(+), 15 deletions(-) diff --git a/smx509/parser.go b/smx509/parser.go index 73e5c36..179d998 100644 --- a/smx509/parser.go +++ b/smx509/parser.go @@ -952,6 +952,7 @@ func parseCertificate(der []byte) (*Certificate, error) { return nil, errors.New("x509: malformed extensions") } if present { + seenExts := make(map[string]bool) if !extensions.ReadASN1(&extensions, cryptobyte_asn1.SEQUENCE) { return nil, errors.New("x509: malformed extensions") } @@ -964,6 +965,11 @@ func parseCertificate(der []byte) (*Certificate, error) { if err != nil { return nil, err } + oidStr := ext.Id.String() + if seenExts[oidStr] { + return nil, errors.New("x509: certificate contains duplicate extensions") + } + seenExts[oidStr] = true cert.Extensions = append(cert.Extensions, ext) } err = processExtensions(cert) diff --git a/smx509/x509.go b/smx509/x509.go index 49427cf..effbe9d 100644 --- a/smx509/x509.go +++ b/smx509/x509.go @@ -1580,13 +1580,18 @@ func parseCSRExtensions(rawAttributes []asn1.RawValue) ([]pkix.Extension, error) } var ret []pkix.Extension + seenExts := make(map[string]bool) for _, rawAttr := range rawAttributes { var attr pkcs10Attribute if rest, err := asn1.Unmarshal(rawAttr.FullBytes, &attr); err != nil || len(rest) != 0 || len(attr.Values) == 0 { // Ignore attributes that don't parse. continue } - + oidStr := attr.Id.String() + if seenExts[oidStr] { + return nil, errors.New("x509: certificate request contains duplicate extensions") + } + seenExts[oidStr] = true if !attr.Id.Equal(oidExtensionRequest) { continue } @@ -1595,6 +1600,14 @@ func parseCSRExtensions(rawAttributes []asn1.RawValue) ([]pkix.Extension, error) if _, err := asn1.Unmarshal(attr.Values[0].FullBytes, &extensions); err != nil { return nil, err } + requestedExts := make(map[string]bool) + for _, ext := range extensions { + oidStr := ext.Id.String() + if requestedExts[oidStr] { + return nil, errors.New("x509: certificate request contains duplicate requested extensions") + } + requestedExts[oidStr] = true + } ret = append(ret, extensions...) } @@ -1791,13 +1804,7 @@ func ParseCertificateRequest(asn1Data []byte) (*CertificateRequest, error) { } else if len(rest) != 0 { return nil, asn1.SyntaxError{Msg: "trailing data"} } - if !csr.SignatureAlgorithm.Algorithm.Equal(oidSignatureSM2WithSM3) { - csrR, err := x509.ParseCertificateRequest(asn1Data) - if err != nil { - return nil, err - } - return (*CertificateRequest)(csrR), nil - } + return parseCertificateRequest(&csr) } @@ -1812,9 +1819,6 @@ func ParseCertificateRequestPEM(data []byte) (*CertificateRequest, error) { } func parseCertificateRequest(in *certificateRequest) (*CertificateRequest, error) { - if !oidSignatureSM2WithSM3.Equal(in.SignatureAlgorithm.Algorithm) { - return nil, errors.New("unsupport signature algorithm") - } out := &CertificateRequest{ Raw: in.Raw, RawTBSCertificateRequest: in.TBSCSR.Raw, @@ -1850,7 +1854,8 @@ func parseCertificateRequest(in *certificateRequest) (*CertificateRequest, error } for _, extension := range out.Extensions { - if extension.Id.Equal(oidExtensionSubjectAltName) { + switch { + case extension.Id.Equal(oidExtensionSubjectAltName): out.DNSNames, out.EmailAddresses, out.IPAddresses, out.URIs, err = parseSANExtension(extension.Value) if err != nil { return nil, err diff --git a/smx509/x509_test.go b/smx509/x509_test.go index 699e2e9..a86b363 100644 --- a/smx509/x509_test.go +++ b/smx509/x509_test.go @@ -244,8 +244,8 @@ func Test_ParseCertificate(t *testing.T) { func TestParseAliCertificateRequest(t *testing.T) { err := parseAndCheckCsr([]byte(csrFromAli)) - if err != nil { - t.Fatal(err) + if err == nil { + t.Fatal("ParseCertificate should fail when parsing certificate with duplicate extensions") } } @@ -412,7 +412,7 @@ func Test_CreateCertificateRequest(t *testing.T) { out, err := ParseCertificateRequest(derBytes) if err != nil { - t.Errorf("%s: failed to create certificate request: %s", test.name, err) + t.Errorf("%s: failed to parse certificate request: %s", test.name, err) continue } @@ -2522,3 +2522,49 @@ func TestCreateNegativeSerial(t *testing.T) { t.Errorf("CreateCertificate returned unexpected error: want %q, got %q", expectedErr, err) } } + +const dupExtCert = `-----BEGIN CERTIFICATE----- +MIIBrjCCARegAwIBAgIBATANBgkqhkiG9w0BAQsFADAPMQ0wCwYDVQQDEwR0ZXN0 +MCIYDzAwMDEwMTAxMDAwMDAwWhgPMDAwMTAxMDEwMDAwMDBaMA8xDTALBgNVBAMT +BHRlc3QwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAMiFchnHms9l9NninAIz +SkY9acwl9Bk2AtmJrNCenFpiA17AcOO5q8DJYwdXi6WPKlVgcyH+ysW8XMWkq+CP +yhtF/+LMzl9odaUF2iUy3vgTC5gxGLWH5URVssx21Und2Pm2f4xyou5IVxbS9dxy +jLvV9PEY9BIb0H+zFthjhihDAgMBAAGjFjAUMAgGAioDBAIFADAIBgIqAwQCBQAw +DQYJKoZIhvcNAQELBQADgYEAlhQ4TQQKIQ8GUyzGiN/75TCtQtjhMGemxc0cNgre +d9rmm4DjydH0t7/sMCB56lQrfhJNplguzsbjFW4l245KbNKHfLiqwEGUgZjBNKur +ot6qX/skahLtt0CNOaFIge75HVKe/69OrWQGdp18dkay/KS4Glu8YMKIjOhfrUi1 +NZA= +-----END CERTIFICATE-----` + +func TestDuplicateExtensionsCert(t *testing.T) { + b, _ := pem.Decode([]byte(dupExtCert)) + if b == nil { + t.Fatalf("couldn't decode test certificate") + } + _, err := ParseCertificate(b.Bytes) + if err == nil { + t.Fatal("ParseCertificate should fail when parsing certificate with duplicate extensions") + } +} + +const dupExtCSR = `-----BEGIN CERTIFICATE REQUEST----- +MIIBczCB3QIBADAPMQ0wCwYDVQQDEwR0ZXN0MIGfMA0GCSqGSIb3DQEBAQUAA4GN +ADCBiQKBgQC5PbxMGVJ8aLF9lq/EvGObXTRMB7ieiZL9N+DJZg1n/ECCnZLIvYrr +ZmmDV7YZsClgxKGfjJB0RQFFyZElFM9EfHEs8NJdidDKCRdIhDXQWRyhXKevHvdm +CQNKzUeoxvdHpU/uscSkw6BgUzPyLyTx9A6ye2ix94z8Y9hGOBO2DQIDAQABoCUw +IwYJKoZIhvcNAQkOMRYwFDAIBgIqAwQCBQAwCAYCKgMEAgUAMA0GCSqGSIb3DQEB +CwUAA4GBAHROEsE7URk1knXmBnQtIHwoq663vlMcX3Hes58pUy020rWP8QkocA+X +VF18/phg3p5ILlS4fcbbP2bEeV0pePo2k00FDPsJEKCBAX2LKxbU7Vp2OuV2HM2+ +VLOVx0i+/Q7fikp3hbN1JwuMTU0v2KL/IKoUcZc02+5xiYrnOIt5 +-----END CERTIFICATE REQUEST-----` + +func TestDuplicateExtensionsCSR(t *testing.T) { + b, _ := pem.Decode([]byte(dupExtCSR)) + if b == nil { + t.Fatalf("couldn't decode test certificate") + } + _, err := ParseCertificateRequest(b.Bytes) + if err == nil { + t.Fatal("ParseCertificate should fail when parsing certificate with duplicate extensions") + } +}