Don't allow trailing data when creating an RSAPrivateKey.

crypto/rsa_private_key_nss.cc

Index: crypto/rsa_private_key_nss.cc
diff --git a/crypto/rsa_private_key_nss.cc b/crypto/rsa_private_key_nss.cc
index 0065875005d8d87699cdc5270a1e457ee10f2b60..e934d6b4925e6b781b2217cb54e96343dcf4fc76 100644
--- a/crypto/rsa_private_key_nss.cc
+++ b/crypto/rsa_private_key_nss.cc
@@ -278,29 +278,36 @@ RSAPrivateKey* RSAPrivateKey::CreateFromPrivateKeyInfoWithParams(
 
   scoped_ptr<RSAPrivateKey> result(new RSAPrivateKey);
 
-  SECItem der_private_key_info;
-  der_private_key_info.data = const_cast<unsigned char*>(&input.front());
-  der_private_key_info.len = input.size();
+  ScopedPLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
+  if (!arena) {
+    NOTREACHED();
+    return NULL;
+  }
+
+  // Excess data is illegal, but NSS silently accepts it, so first ensure that
+  // |input| consists of a single ASN.1 element.
+  SECItem input_item, der_private_key_info;

[Ryan Sleevi, 2014/11/22 01:13:57]

nit: I tend to prefer we don't use this style in //net, as it can make it
visually difficult to find the type of a given variable.

[davidben, 2014/11/24 20:23:59]

Done.

+  input_item.data = const_cast<unsigned char*>(&input.front());
+  input_item.len = input.size();
+  SECStatus rv = SEC_QuickDERDecodeItem(arena.get(), &der_private_key_info,
+                                        SEC_ASN1_GET(SEC_AnyTemplate),
+                                        &input_item);
+  if (rv != SECSuccess)
+    return NULL;
+
   // Allow the private key to be used for key unwrapping, data decryption,
   // and signature generation.
   const unsigned int key_usage = KU_KEY_ENCIPHERMENT | KU_DATA_ENCIPHERMENT |
                                  KU_DIGITAL_SIGNATURE;
-  // TODO(davidben): PK11_ImportDERPrivateKeyInfoAndReturnKey calls NSS's
-  // SEC_ASN1DecodeItem which does not enforce that there is no trailing
-  // data.
-  SECStatus rv =  PK11_ImportDERPrivateKeyInfoAndReturnKey(
+  rv = PK11_ImportDERPrivateKeyInfoAndReturnKey(
       slot, &der_private_key_info, NULL, NULL, permanent, sensitive,
       key_usage, &result->key_, NULL);
-  if (rv != SECSuccess) {
-    NOTREACHED();
+  if (rv != SECSuccess)
     return NULL;
-  }
 
   result->public_key_ = SECKEY_ConvertToPublicKey(result->key_);
-  if (!result->public_key_) {
-    NOTREACHED();
+  if (!result->public_key_)
     return NULL;
-  }
 
   return result.release();
 }