Check some ASN1_STRING types in crypto/x509

This adds runtime checks that types which are aliases of ASN1_STRING
are in fact the expected ASN.1 type. Not comprehensive -- I got the
obvious ones from x509.h. These checks are not generally covered by
unit tests, except for one which was easy to test as a sanity-check.

Bug: 445
Change-Id: I8cd689b6b1e6121fce62c7f0ab25fee7e2a0b2ff
Update-Note: Various X.509 functions will now fail given the wrong ASN1_STRING subtype.
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54286
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

crypto/x509/a_sign.c

@@ -67,6 +67,10 @@
 int ASN1_item_sign(const ASN1_ITEM *it, X509_ALGOR *algor1, X509_ALGOR *algor2,
                    ASN1_BIT_STRING *signature, void *asn, EVP_PKEY *pkey,
                    const EVP_MD *type) {
+  if (signature->type != V_ASN1_BIT_STRING) {
+    OPENSSL_PUT_ERROR(ASN1, ASN1_R_WRONG_TYPE);
+    return 0;
+  }
   EVP_MD_CTX ctx;
   EVP_MD_CTX_init(&ctx);
   if (!EVP_DigestSignInit(&ctx, NULL, type, NULL, pkey)) {
@@ -83,6 +87,11 @@ int ASN1_item_sign_ctx(const ASN1_ITEM *it, X509_ALGOR *algor1,
   unsigned char *buf_in = NULL, *buf_out = NULL;
   size_t inl = 0, outl = 0;
 
+  if (signature->type != V_ASN1_BIT_STRING) {
+    OPENSSL_PUT_ERROR(ASN1, ASN1_R_WRONG_TYPE);
+    goto err;
+  }
+
   pkey = EVP_PKEY_CTX_get0_pkey(ctx->pctx);
 
   // Write out the requested copies of the AlgorithmIdentifier.

crypto/x509/x509_cmp.c

@@ -232,6 +232,10 @@ X509 *X509_find_by_issuer_and_serial(STACK_OF(X509) *sk, X509_NAME *name,
     return NULL;
   }
 
+  if (serial->type != V_ASN1_INTEGER) {
+    return NULL;
+  }
+
   x.cert_info = &cinf;
   cinf.serialNumber = serial;
   cinf.issuer = name;

crypto/x509/x509_lu.c

@@ -146,6 +146,9 @@ int X509_LOOKUP_by_issuer_serial(X509_LOOKUP *ctx, int type, X509_NAME *name,
   if ((ctx->method == NULL) || (ctx->method->get_by_issuer_serial == NULL)) {
     return 0;
   }
+  if (serial->type != V_ASN1_INTEGER) {
+    return 0;
+  }
   return ctx->method->get_by_issuer_serial(ctx, type, name, serial, ret) > 0;
 }
 

crypto/x509/x509_set.c

@@ -98,8 +98,12 @@ int X509_set_version(X509 *x, long version) {
 }
 
 int X509_set_serialNumber(X509 *x, const ASN1_INTEGER *serial) {
-  ASN1_INTEGER *in;
+  if (serial->type != V_ASN1_INTEGER) {
+    OPENSSL_PUT_ERROR(ASN1, ASN1_R_WRONG_TYPE);
+    return 0;
+  }
 
+  ASN1_INTEGER *in;
   if (x == NULL) {
     return 0;
   }

crypto/x509/x509_test.cc

@@ -4806,3 +4806,16 @@ TEST(X509Test, NameEntry) {
   X509_NAME_ENTRY_free(X509_NAME_delete_entry(name.get(), 1));
   check_name("CN=Name,O=Org");
 }
+
+// Tests that non-integer types are rejected when passed as an argument to
+// X509_set_serialNumber().
+TEST(X509Test, SetSerialNumberChecksASN1StringType) {
+  bssl::UniquePtr<X509> root = CertFromPEM(kRootCAPEM);
+  ASSERT_TRUE(root);
+
+  // Passing an IA5String to X509_set_serialNumber() should fail.
+  bssl::UniquePtr<ASN1_IA5STRING> str(ASN1_IA5STRING_new());
+  ASSERT_TRUE(str);
+  int r = X509_set_serialNumber(root.get(), str.get());
+  ASSERT_EQ(r, 0);
+}

crypto/x509/x509cset.c

@@ -216,6 +216,11 @@ int X509_REVOKED_set_serialNumber(X509_REVOKED *revoked,
                                   const ASN1_INTEGER *serial) {
   ASN1_INTEGER *in;
 
+  if (serial->type != V_ASN1_INTEGER) {
+    OPENSSL_PUT_ERROR(ASN1, ASN1_R_WRONG_TYPE);
+    return 0;
+  }
+
   if (revoked == NULL) {
     return 0;
   }

crypto/x509/x_crl.c

@@ -430,6 +430,9 @@ static struct CRYPTO_STATIC_MUTEX g_crl_sort_lock = CRYPTO_STATIC_MUTEX_INIT;
 
 static int crl_lookup(X509_CRL *crl, X509_REVOKED **ret, ASN1_INTEGER *serial,
                       X509_NAME *issuer) {
+  if (serial->type != V_ASN1_INTEGER) {
+    return 0;
+  }
   X509_REVOKED rtmp, *rev;
   size_t idx;
   rtmp.serialNumber = serial;