diff --git a/x509/pkix/pkix.go b/x509/pkix/pkix.go index 843fa1f2cd..1716f908ab 100644 --- a/x509/pkix/pkix.go +++ b/x509/pkix/pkix.go @@ -18,6 +18,7 @@ import ( // AlgorithmIdentifier represents the ASN.1 structure of the same name. See RFC // 5280, section 4.1.1.2. type AlgorithmIdentifier struct { + Raw asn1.RawContent Algorithm asn1.ObjectIdentifier Parameters asn1.RawValue `asn1:"optional"` } diff --git a/x509/revoked_test.go b/x509/revoked_test.go index b5158d1c60..9dfd8142f5 100644 --- a/x509/revoked_test.go +++ b/x509/revoked_test.go @@ -94,6 +94,7 @@ func TestParseCertificateList(t *testing.T) { want: TBSCertList{ Version: 1, Signature: pkix.AlgorithmIdentifier{ + Raw: fromHex("300d06092a864886f70d01010b0500"), Algorithm: oidSignatureSHA256WithRSA, Parameters: asn1.RawValue{Class: 0, Tag: 5, Bytes: []byte{}, FullBytes: []byte{5, 0}}, }, @@ -348,6 +349,7 @@ func TestParseCertificateList(t *testing.T) { want: TBSCertList{ Version: 1, Signature: pkix.AlgorithmIdentifier{ + Raw: fromHex("300d06092a864886f70d01010b0500"), Algorithm: oidSignatureSHA256WithRSA, Parameters: asn1.RawValue{Class: 0, Tag: 5, Bytes: []byte{}, FullBytes: []byte{5, 0}}, }, @@ -416,6 +418,7 @@ func TestParseCertificateList(t *testing.T) { want: TBSCertList{ Version: 1, Signature: pkix.AlgorithmIdentifier{ + Raw: fromHex("300d06092a864886f70d01010b0500"), Algorithm: oidSignatureSHA256WithRSA, Parameters: asn1.RawValue{Class: 0, Tag: 5, Bytes: []byte{}, FullBytes: []byte{5, 0}}, }, @@ -915,6 +918,7 @@ func TestParseCertificateList(t *testing.T) { want: TBSCertList{ Version: 1, Signature: pkix.AlgorithmIdentifier{ + Raw: fromHex("300d06092a864886f70d01010b0500"), Algorithm: oidSignatureSHA256WithRSA, Parameters: asn1.RawValue{Class: 0, Tag: 5, Bytes: []byte{}, FullBytes: []byte{5, 0}}, }, @@ -987,6 +991,7 @@ func TestParseCertificateList(t *testing.T) { want: TBSCertList{ Version: 1, Signature: pkix.AlgorithmIdentifier{ + Raw: fromHex("300d06092a864886f70d01010b0500"), Algorithm: oidSignatureSHA256WithRSA, Parameters: asn1.RawValue{Class: 0, Tag: 5, Bytes: []byte{}, FullBytes: []byte{5, 0}}, }, @@ -1059,6 +1064,7 @@ func TestParseCertificateList(t *testing.T) { want: TBSCertList{ Version: 1, Signature: pkix.AlgorithmIdentifier{ + Raw: fromHex("300d06092a864886f70d01010b0500"), Algorithm: oidSignatureSHA256WithRSA, Parameters: asn1.RawValue{Class: 0, Tag: 5, Bytes: []byte{}, FullBytes: []byte{5, 0}}, }, @@ -1224,6 +1230,7 @@ func TestParseCertificateList(t *testing.T) { want: TBSCertList{ Version: 1, Signature: pkix.AlgorithmIdentifier{ + Raw: fromHex("300d06092a864886f70d01010b0500"), Algorithm: oidSignatureSHA256WithRSA, Parameters: asn1.RawValue{Class: 0, Tag: 5, Bytes: []byte{}, FullBytes: []byte{5, 0}}, }, @@ -1343,6 +1350,7 @@ func TestParseCertificateList(t *testing.T) { want: TBSCertList{ Version: 1, Signature: pkix.AlgorithmIdentifier{ + Raw: fromHex("300d06092a864886f70d01010b0500"), Algorithm: oidSignatureSHA256WithRSA, Parameters: asn1.RawValue{Class: 0, Tag: 5, Bytes: []byte{}, FullBytes: []byte{5, 0}}, }, @@ -1463,6 +1471,7 @@ func TestParseCertificateList(t *testing.T) { want: TBSCertList{ Version: 1, Signature: pkix.AlgorithmIdentifier{ + Raw: fromHex("300d06092a864886f70d01010b0500"), Algorithm: oidSignatureSHA256WithRSA, Parameters: asn1.RawValue{Class: 0, Tag: 5, Bytes: []byte{}, FullBytes: []byte{5, 0}}, }, diff --git a/x509/testdata/invalid/mismatching-sig-alg.pem b/x509/testdata/invalid/mismatching-sig-alg.pem new file mode 100644 index 0000000000..049a344af6 --- /dev/null +++ b/x509/testdata/invalid/mismatching-sig-alg.pem @@ -0,0 +1,8 @@ +-----BEGIN CERTIFICATE----- +MIIBAjCBqqADAgECAgEAMAoGCCqGSM49BAMCMAAwHhcNMDkxMTEwMjMwMDAwWhcN +MTkwODEwMjMwMDAwWjAAMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEJwxgz8ox +ipHbj2fHlQRt3xgxmSyXinGNe8K5eT0ChiGpG1oMVQPMzBgsG5ZSgdxwYn/J9GfN +0/RDMbSldQsZO6MVMBMwEQYDVR0RAQH/BAcwBYIDYXNkMAoGCCqGSM49BAMDA0cA +MEQCIDu3tl2kE6Hy2EJeNrnlQXqQHOpFPxFut7B9sPe8Ios1AiAleoh3TIr9neiT +M5Nt98OAuqEcUT4Etn/B/6I6xIes+Q== +-----END CERTIFICATE----- diff --git a/x509/x509.go b/x509/x509.go index 140e7244bc..917d78779f 100644 --- a/x509/x509.go +++ b/x509/x509.go @@ -1813,9 +1813,25 @@ func parseNameConstraintsExtension(out *Certificate, e pkix.Extension, nfe *NonF return unhandled, nil } -func parseCertificate(in *certificate) (*Certificate, error) { +func parseCertificate(in *certificate, tbsOnly bool) (*Certificate, error) { var nfe NonFatalErrors + // Certificates contain two signature algorithm identifier fields, + // one in the inner signed tbsCertificate structure and one in the + // outer unsigned certificate structure. RFC 5280 requires these + // fields match, but golang doesn't impose this restriction. Because + // the outer structure is not covered by the signature the algorithm + // field is entirely malleable. This allows a user to bypass the + // leaf data uniqueness check that happens in trillian by altering + // the unbounded OID or parameter fields of the algorithmIdentifier + // structure and submit an infinite number of duplicate but slightly + // different looking certificates to a log. To avoid this directly + // compare the bytes of the two algorithmIdentifier structures + // and reject the certificate if they do not match. + if !tbsOnly && !bytes.Equal(in.SignatureAlgorithm.Raw, in.TBSCertificate.SignatureAlgorithm.Raw) { + return nil, errors.New("x509: mismatching signature algorithm identifiers") + } + out := new(Certificate) out.Raw = in.Raw out.RawTBSCertificate = in.TBSCertificate.Raw @@ -2095,7 +2111,7 @@ func ParseTBSCertificate(asn1Data []byte) (*Certificate, error) { } ret, err := parseCertificate(&certificate{ Raw: tbsCert.Raw, - TBSCertificate: tbsCert}) + TBSCertificate: tbsCert}, true) if err != nil { errs, ok := err.(NonFatalErrors) if !ok { @@ -2127,7 +2143,7 @@ func ParseCertificate(asn1Data []byte) (*Certificate, error) { if len(rest) > 0 { return nil, asn1.SyntaxError{Msg: "trailing data"} } - ret, err := parseCertificate(&cert) + ret, err := parseCertificate(&cert, false) if err != nil { errs, ok := err.(NonFatalErrors) if !ok { @@ -2166,7 +2182,7 @@ func ParseCertificates(asn1Data []byte) ([]*Certificate, error) { ret := make([]*Certificate, len(v)) for i, ci := range v { - cert, err := parseCertificate(ci) + cert, err := parseCertificate(ci, false) if err != nil { errs, ok := err.(NonFatalErrors) if !ok { diff --git a/x509/x509_test.go b/x509/x509_test.go index 03377bb247..fa3fc2c760 100644 --- a/x509/x509_test.go +++ b/x509/x509_test.go @@ -2861,6 +2861,7 @@ func TestParseCertificateFail(t *testing.T) { {desc: "RSAIntegerNotMinimal", in: "testdata/invalid/xf-der-pubkey-rsa-nonminimal-int.pem", wantErr: "integer not minimally-encoded"}, {desc: "SubjectNonPrintable", in: "testdata/invalid/xf-subject-nonprintable.pem", wantErr: "PrintableString contains invalid character"}, {desc: "NegativeRSAModulus", in: "testdata/invalid/xf-pubkey-rsa-modulus-negative.pem", wantErr: "RSA modulus is not a positive number"}, + {desc: "MismatchingSigAlg", in: "testdata/invalid/mismatching-sig-alg.pem", wantErr: "mismatching signature algorithm identifiers", wantFatal: true}, } for _, test := range tests { t.Run(test.desc, func(t *testing.T) {