commit 62e0219679f5a120445a95def01c7679f5eb1790
author Emily Stark <estark@google.com> Sun Mar 06 23:41:16 2016 -0800
committer David Benjamin <davidben@google.com> Mon Mar 07 15:54:54 2016 +0000
tree 3235b96db90e2bd4c95a2a5f2b212c21da44cb2e
parent ad004af661759cdc4bf486e81f70a350a3a0a5a9

Handle empty keys in EVP_marshal_public_key()

Instead of crashing when an empty key is passed to
EVP_marshal_public_key(), return with an
EVP_R_UNSUPPORTED_ALGORITHM_ERROR. This brings e.g. X509_PUBKEY_set()
closer to how it behaved before 68772b31 (previously, it returned an
error on an empty public key rather than dereferencing pkey->ameth).

Change-Id: Ieac368725adb7f22329c035d9d0685b44b885888
Reviewed-on: https://boringssl-review.googlesource.com/7351
Reviewed-by: David Benjamin <davidben@google.com>

crypto/evp/evp_asn1.c

diff --git a/crypto/evp/evp_asn1.c b/crypto/evp/evp_asn1.c
index ecf304c..ffbb148 100644
--- a/crypto/evp/evp_asn1.c
+++ b/crypto/evp/evp_asn1.c
@@ -107,7 +107,7 @@
 }
 
 int EVP_marshal_public_key(CBB *cbb, const EVP_PKEY *key) {
-  if (key->ameth->pub_encode == NULL) {
+  if (key->ameth == NULL || key->ameth->pub_encode == NULL) {
     OPENSSL_PUT_ERROR(EVP, EVP_R_UNSUPPORTED_ALGORITHM);
     return 0;
   }
@@ -155,7 +155,7 @@
 }
 
 int EVP_marshal_private_key(CBB *cbb, const EVP_PKEY *key) {
-  if (key->ameth->priv_encode == NULL) {
+  if (key->ameth == NULL || key->ameth->priv_encode == NULL) {
     OPENSSL_PUT_ERROR(EVP, EVP_R_UNSUPPORTED_ALGORITHM);
     return 0;
   }

crypto/evp/evp_extra_test.cc

diff --git a/crypto/evp/evp_extra_test.cc b/crypto/evp/evp_extra_test.cc
index 3d08638..9bc36ad 100644
--- a/crypto/evp/evp_extra_test.cc
+++ b/crypto/evp/evp_extra_test.cc
@@ -586,6 +586,25 @@
   return true;
 }
 
+// TestEVPMarshalEmptyPublicKey tests |EVP_marshal_public_key| on an empty key.
+static bool TestEVPMarshalEmptyPublicKey(void) {
+  ScopedEVP_PKEY empty(EVP_PKEY_new());
+  if (!empty) {
+    return false;
+  }
+  ScopedCBB cbb;
+  if (EVP_marshal_public_key(cbb.get(), empty.get())) {
+    fprintf(stderr, "Marshalled empty public key.\n");
+    return false;
+  }
+  if (ERR_GET_REASON(ERR_peek_last_error()) != EVP_R_UNSUPPORTED_ALGORITHM) {
+    fprintf(stderr, "Marshalling an empty public key gave wrong error.\n");
+    return false;
+  }
+  ERR_clear_error();
+  return true;
+}
+
 // Testd2i_PrivateKey tests |d2i_PrivateKey|.
 static bool Testd2i_PrivateKey(void) {
   const uint8_t *derp = kExampleRSAKeyDER;
@@ -685,6 +704,12 @@
     return 1;
   }
 
+  if (!TestEVPMarshalEmptyPublicKey()) {
+    fprintf(stderr, "TestEVPMarshalEmptyPublicKey failed\n");
+    ERR_print_errors_fp(stderr);
+    return 1;
+  }
+
   if (!Testd2i_PrivateKey()) {
     fprintf(stderr, "Testd2i_PrivateKey failed\n");
     ERR_print_errors_fp(stderr);