From 371305f58ac47a98d32f30a9edc6fafa72e842be Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Tue, 18 Sep 2018 13:14:53 -0700
Subject: [PATCH] Push an error on sigalg mismatch in X509_verify.

It was failing but not pushing an error. See
https://github.com/google/conscrypt/pull/537

Change-Id: Iafba1a5c0c7ef8e0535b335aa93df6f520c3803e
Reviewed-on: https://boringssl-review.googlesource.com/32044
Reviewed-by: Adam Langley <agl@google.com>
---
 crypto/err/x509.errordata |  1 +
 crypto/x509/x509_test.cc  | 45 +++++++++++++++++++++++++++++++++++++++
 crypto/x509/x_all.c       |  4 +++-
 include/openssl/x509.h    |  1 +
 4 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/crypto/err/x509.errordata b/crypto/err/x509.errordata
index 1a8c4f1b..6ed8fa35 100644
--- a/crypto/err/x509.errordata
+++ b/crypto/err/x509.errordata
@@ -28,6 +28,7 @@ X509,124,NO_CRL_NUMBER
 X509,125,PUBLIC_KEY_DECODE_ERROR
 X509,126,PUBLIC_KEY_ENCODE_ERROR
 X509,127,SHOULD_RETRY
+X509,137,SIGNATURE_ALGORITHM_MISMATCH
 X509,128,UNKNOWN_KEY_TYPE
 X509,129,UNKNOWN_NID
 X509,130,UNKNOWN_PURPOSE_ID
diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index 551bd8ca..9ed1b52a 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -627,6 +627,38 @@ static const char kNoBasicConstraintsNetscapeCALeaf[] =
     "XyPwWPbymSTpzKhB4xB7qQ==\n"
     "-----END CERTIFICATE-----\n";
 
+static const char kSelfSignedMismatchAlgorithms[] =
+    "-----BEGIN CERTIFICATE-----\n"
+    "MIIFMjCCAxqgAwIBAgIJAL0mG5fOeJ7xMA0GCSqGSIb3DQEBDQUAMC0xCzAJBgNV\n"
+    "BAYTAkdCMQ8wDQYDVQQHDAZMb25kb24xDTALBgNVBAoMBFRlc3QwIBcNMTgwOTE3\n"
+    "MTIxNzU3WhgPMjExODA4MjQxMjE3NTdaMC0xCzAJBgNVBAYTAkdCMQ8wDQYDVQQH\n"
+    "DAZMb25kb24xDTALBgNVBAoMBFRlc3QwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAw\n"
+    "ggIKAoICAQDCMhBrRAGGw+n2GdctBr/cEK4FZA6ajiHjihgpCHoSBdyL4R2jGKLS\n"
+    "g0WgaMXa1HpkKN7LcIySosEBPlmcRkr1RqbEvQStOSvoFCXYvtx3alM6HTbXMcDR\n"
+    "mqoKoABP6LXsPSoMWIgqMtP2X9EOppzHVIK1yFYFfbIlvYUV2Ka+MuMe0Vh5wvD1\n"
+    "4GanPb+cWSKgdRSVQovCCMY3yWtZKVEaxRpCsk/mYYIFWz0tcgMjIKwDx1XXgiAV\n"
+    "nU6NK43xbaw3XhtnaD/pv9lhTTbNrlcln9LjTD097BaK4R+1AEPHnpfxA9Ui3upn\n"
+    "kbsNUdGdOB0ksZi/vd7lh833YgquQUIAhYrbfvq/HFCpVV1gljzlS3sqULYpLE//\n"
+    "i3OsuL2mE+CYIJGpIi2GeJJWXciNMTJDOqTn+fRDtVb4RPp4Y70DJirp7XzaBi3q\n"
+    "H0edANCzPSRCDbZsOhzIXhXshldiXVRX666DDlbMQgLTEnNKrkwv6DmU8o15XQsb\n"
+    "8k1Os2YwXmkEOxUQ7AJZXVTZSf6UK9Znmdq1ZrHjybMfRUkHVxJcnKvrxfryralv\n"
+    "gzfvu+D6HuxrCo3Ojqa+nDgIbxKEBtdrcsMhq1jWPFhjwo1fSadAkKOfdCAuXJRD\n"
+    "THg3b4Sf+W7Cpc570YHrIpBf7WFl2XsPcEM0mJZ5+yATASCubNozQwIDAQABo1Mw\n"
+    "UTAdBgNVHQ4EFgQUES0hupZSqY21JOba10QyZuxm91EwHwYDVR0jBBgwFoAUES0h\n"
+    "upZSqY21JOba10QyZuxm91EwDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsF\n"
+    "AAOCAgEABTN5S30ng/RMpBweDm2N561PdpaCdiRXtAFCRVWR2mkDYC/Xj9Vqe6be\n"
+    "PyM7L/5OKYVjzF1yJu67z/dx+ja5o+41g17jdqla7hyPx+9B4uRyDh+1KJTa+duj\n"
+    "mw/aA1LCr6O6W4WizDOsChJ6FaB2Y1+GlFnKWb5nUdhVJqXQE1WOX9dZnw8Y4Npd\n"
+    "VmAsjWot0BZorJrt3fwfcv3QfA896twkbo7Llv/8qzg4sXZXZ4ZtgAOqnPngiSn+\n"
+    "JT/vYCXZ406VvAFpFqMcVz2dO/VGuL8lGIMHRKNyafrsV81EzH1W/XmRWOgvgj6r\n"
+    "yQI63ln/AMY72HQ97xLkE1xKunGz6bK5Ug5+O43Uftc4Mb6MUgzo+ZqEQ3Ob+cAV\n"
+    "cvjmtwDaPO/O39O5Xq0tLTlkn2/cKf4OQ6S++GDxzyRVHh5JXgP4j9+jfZY57Woy\n"
+    "R1bE7N50JjY4cDermBJKdlBIjL7UPhqmLyaG7V0hBitFlgGBUCcJtJOV0xYd5aF3\n"
+    "pxNkvMXhBmh95fjxJ0cJjpO7tN1RAwtMMNgsl7OUbuVRQCHOPW5DgP5qY21jDeRn\n"
+    "BY82382l+9QzykmJLI5MZnmj4BA9uIDCwMtoTTvP++SsvhUAbuvh7MOOUQL0EY4m\n"
+    "KStYq7X9PKseN+PvmfeoffIKc5R/Ha39oi7cGMVHCr8aiEhsf94=\n"
+    "-----END CERTIFICATE-----\n";
+
 // CertFromPEM parses the given, NUL-terminated pem block and returns an
 // |X509*|.
 static bssl::UniquePtr<X509> CertFromPEM(const char *pem) {
@@ -1504,3 +1536,16 @@ TEST(X509Test, NoBasicConstraintsNetscapeCA) {
   EXPECT_EQ(X509_V_ERR_INVALID_CA,
             Verify(leaf.get(), {root.get()}, {intermediate.get()}, {}, 0));
 }
+
+TEST(X509Test, MismatchAlgorithms) {
+  bssl::UniquePtr<X509> cert(CertFromPEM(kSelfSignedMismatchAlgorithms));
+  ASSERT_TRUE(cert);
+
+  bssl::UniquePtr<EVP_PKEY> pkey(X509_get_pubkey(cert.get()));
+  ASSERT_TRUE(pkey);
+
+  EXPECT_FALSE(X509_verify(cert.get(), pkey.get()));
+  uint32_t err = ERR_get_error();
+  EXPECT_EQ(ERR_LIB_X509, ERR_GET_LIB(err));
+  EXPECT_EQ(X509_R_SIGNATURE_ALGORITHM_MISMATCH, ERR_GET_REASON(err));
+}
diff --git a/crypto/x509/x_all.c b/crypto/x509/x_all.c
index 2a93b873..064c89c8 100644
--- a/crypto/x509/x_all.c
+++ b/crypto/x509/x_all.c
@@ -65,8 +65,10 @@
 
 int X509_verify(X509 *a, EVP_PKEY *r)
 {
-    if (X509_ALGOR_cmp(a->sig_alg, a->cert_info->signature))
+    if (X509_ALGOR_cmp(a->sig_alg, a->cert_info->signature)) {
+        OPENSSL_PUT_ERROR(X509, X509_R_SIGNATURE_ALGORITHM_MISMATCH);
         return 0;
+    }
     return (ASN1_item_verify(ASN1_ITEM_rptr(X509_CINF), a->sig_alg,
                              a->signature, a->cert_info, r));
 }
diff --git a/include/openssl/x509.h b/include/openssl/x509.h
index 72f7314b..ee3ecccc 100644
--- a/include/openssl/x509.h
+++ b/include/openssl/x509.h
@@ -1200,5 +1200,6 @@ BSSL_NAMESPACE_END
 #define X509_R_WRONG_TYPE 134
 #define X509_R_NAME_TOO_LONG 135
 #define X509_R_INVALID_PARAMETER 136
+#define X509_R_SIGNATURE_ALGORITHM_MISMATCH 137
 
 #endif