commit 7b832ad118c2fc9790c46664a5095a234722abce
author David Benjamin <davidben@google.com> Thu May 10 18:58:21 2018 -0400
committer Adam Langley <agl@google.com> Fri May 11 22:00:04 2018 +0000
tree 87ce3542c161f64e274c06c612b04632207e36b3
parent f05e3eafbcf6b161bbaa29b016c08c0522cc574a

Don't crash if asked to treat PBES2 as a PBES1 scheme.

Change-Id: I5d0570634a9ebf553f8c3d22e7cced9d2b972abf
Reviewed-on: https://boringssl-review.googlesource.com/28330
Reviewed-by: Adam Langley <agl@google.com>

crypto/pkcs8/pkcs8.c

diff --git a/crypto/pkcs8/pkcs8.c b/crypto/pkcs8/pkcs8.c
index 0206be2..3453113 100644
--- a/crypto/pkcs8/pkcs8.c
+++ b/crypto/pkcs8/pkcs8.c
@@ -317,9 +317,12 @@
     },
 };
 
-static const struct pbe_suite *get_pbe_suite(int pbe_nid) {
+static const struct pbe_suite *get_pkcs12_pbe_suite(int pbe_nid) {
   for (unsigned i = 0; i < OPENSSL_ARRAY_SIZE(kBuiltinPBE); i++) {
-    if (kBuiltinPBE[i].pbe_nid == pbe_nid) {
+    if (kBuiltinPBE[i].pbe_nid == pbe_nid &&
+        // If |cipher_func| or |md_func| are missing, this is a PBES2 scheme.
+        kBuiltinPBE[i].cipher_func != NULL &&
+        kBuiltinPBE[i].md_func != NULL) {
       return &kBuiltinPBE[i];
     }
   }
@@ -331,7 +334,7 @@
                             unsigned iterations, const char *pass,
                             size_t pass_len, const uint8_t *salt,
                             size_t salt_len) {
-  const struct pbe_suite *suite = get_pbe_suite(alg);
+  const struct pbe_suite *suite = get_pkcs12_pbe_suite(alg);
   if (suite == NULL) {
     OPENSSL_PUT_ERROR(PKCS8, PKCS8_R_UNKNOWN_ALGORITHM);
     return 0;

crypto/pkcs8/pkcs8_test.cc

diff --git a/crypto/pkcs8/pkcs8_test.cc b/crypto/pkcs8/pkcs8_test.cc
index 44388bb..df275fb 100644
--- a/crypto/pkcs8/pkcs8_test.cc
+++ b/crypto/pkcs8/pkcs8_test.cc
@@ -277,3 +277,38 @@
   TestRoundTrip(-1, EVP_aes_128_cbc(), "password", nullptr, 0, 1);
   TestRoundTrip(-1, EVP_rc2_cbc(), "password", nullptr, 0, 10);
 }
+
+TEST(PKCS8Test, InvalidPBES1NIDs) {
+  static const uint8_t kSampleKey[] = {
+      0x30, 0x81, 0x87, 0x02, 0x01, 0x00, 0x30, 0x13, 0x06, 0x07, 0x2a, 0x86,
+      0x48, 0xce, 0x3d, 0x02, 0x01, 0x06, 0x08, 0x2a, 0x86, 0x48, 0xce, 0x3d,
+      0x03, 0x01, 0x07, 0x04, 0x6d, 0x30, 0x6b, 0x02, 0x01, 0x01, 0x04, 0x20,
+      0x8a, 0x87, 0x2f, 0xb6, 0x28, 0x93, 0xc4, 0xd1, 0xff, 0xc5, 0xb9, 0xf0,
+      0xf9, 0x17, 0x58, 0x06, 0x9f, 0x83, 0x52, 0xe0, 0x8f, 0xa0, 0x5a, 0x49,
+      0xf8, 0xdb, 0x92, 0x6c, 0xb5, 0x72, 0x87, 0x25, 0xa1, 0x44, 0x03, 0x42,
+      0x00, 0x04, 0x2c, 0x15, 0x0f, 0x42, 0x9c, 0xe7, 0x0f, 0x21, 0x6c, 0x25,
+      0x2c, 0xf5, 0xe0, 0x62, 0xce, 0x1f, 0x63, 0x9c, 0xd5, 0xd1, 0x65, 0xc7,
+      0xf8, 0x94, 0x24, 0x07, 0x2c, 0x27, 0x19, 0x7d, 0x78, 0xb3, 0x3b, 0x92,
+      0x0e, 0x95, 0xcd, 0xb6, 0x64, 0xe9, 0x90, 0xdc, 0xf0, 0xcf, 0xea, 0x0d,
+      0x94, 0xe2, 0xa8, 0xe6, 0xaf, 0x9d, 0x0e, 0x58, 0x05, 0x6e, 0x65, 0x31,
+      0x04, 0x92, 0x5b, 0x9f, 0xe6, 0xc9,
+  };
+
+  const uint8_t *ptr = kSampleKey;
+  bssl::UniquePtr<PKCS8_PRIV_KEY_INFO> key(
+      d2i_PKCS8_PRIV_KEY_INFO(nullptr, &ptr, sizeof(kSampleKey)));
+  ASSERT_TRUE(key);
+  ASSERT_EQ(kSampleKey + sizeof(kSampleKey), ptr);
+
+  bssl::UniquePtr<X509_SIG> encrypted(PKCS8_encrypt(
+      NID_pbes2, nullptr, "password", -1, nullptr, 0, 0, key.get()));
+  EXPECT_FALSE(encrypted);
+
+  encrypted.reset(PKCS8_encrypt(NID_undef, nullptr, "password", -1, nullptr, 0,
+                                0, key.get()));
+  EXPECT_FALSE(encrypted);
+
+  encrypted.reset(PKCS8_encrypt(NID_rsaEncryption, nullptr, "password", -1,
+                                nullptr, 0, 0, key.get()));
+  EXPECT_FALSE(encrypted);
+}