x509: disallow mismatching signature algorithm identifiers

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"`
 }

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

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

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 {

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