Remove unnecessary SPKI parameter to CreateFromEncryptedPrivateKeyInfo.

This hasn't been used for a while. We should remove this function
altogether, but components/gcm_driver hasn't switched to the more
efficient format yet. Meanwhile, we can still simplify some stuff.

This removes all uses of public_key_x509 in components/gcm_crypto, but
it does not stop computing it. That will be done in a follow-up CL.

Bug: 667060
Change-Id: I6d5f952c4fd822152f0aa142bafe59be933e393f
Reviewed-on: https://chromium-review.googlesource.com/667060
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Nick Harper <nharper@chromium.org>
Commit-Queue: David Benjamin <davidben@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503473}

components/gcm_driver/crypto/gcm_crypto_test_helpers.cc

@@ -33,8 +33,7 @@ bool CreateEncryptedPayloadForTesting(const base::StringPiece& payload,
   std::string shared_secret;
 
   // Calculate the shared secret between the sender and its peer.
-  if (!ComputeSharedP256Secret(private_key, public_key_x509, peer_public_key,
-                               &shared_secret)) {
+  if (!ComputeSharedP256Secret(private_key, peer_public_key, &shared_secret)) {
     return false;
   }
 

components/gcm_driver/crypto/gcm_encryption_provider.cc

@@ -265,8 +265,8 @@ void GCMEncryptionProvider::DecryptMessageWithKey(
   DCHECK_EQ(KeyPair::ECDH_P256, pair.type());
 
   std::string shared_secret;
-  if (!ComputeSharedP256Secret(pair.private_key(), pair.public_key_x509(),
-                               public_key, &shared_secret)) {
+  if (!ComputeSharedP256Secret(pair.private_key(), public_key,
+                               &shared_secret)) {
     DLOG(ERROR) << "Unable to calculate the shared secret.";
     callback.Run(GCMDecryptionResult::INVALID_SHARED_SECRET, IncomingMessage());
     return;

components/gcm_driver/crypto/gcm_encryption_provider_unittest.cc

@@ -489,8 +489,7 @@ void GCMEncryptionProviderTest::TestEncryptionRoundTrip(
 
   std::string shared_secret;
   ASSERT_TRUE(ComputeSharedP256Secret(
-      pair.private_key(), pair.public_key_x509(), server_pair.public_key(),
-      &shared_secret));
+      pair.private_key(), server_pair.public_key(), &shared_secret));
 
   IncomingMessage message;
   size_t record_size;

components/gcm_driver/crypto/gcm_message_cryptographer_unittest.cc

@@ -49,16 +49,6 @@ const unsigned char kCommonRecipientPublicKey[] = {
 static_assert(arraysize(kCommonRecipientPublicKey) == 65,
               "Raw P-256 public keys must be 65 bytes in size.");
 
-const unsigned char kCommonRecipientPublicKeyX509[] = {
-    0x30, 0x59, 0x30, 0x13, 0x06, 0x07, 0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x02,
-    0x01, 0x06, 0x08, 0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x03, 0x01, 0x07, 0x03,
-    0x42, 0x00, 0x04, 0x35, 0x02, 0x67, 0xB9, 0x10, 0x8F, 0x9B, 0xF1, 0x85,
-    0xF5, 0x1B, 0xD7, 0xA4, 0xEF, 0xBD, 0x28, 0xB3, 0x11, 0x40, 0xBA, 0xD0,
-    0xEE, 0xB2, 0x97, 0xDA, 0x6A, 0x93, 0x2D, 0x26, 0x45, 0xBD, 0xB2, 0x9A,
-    0x9F, 0xB8, 0x19, 0xD8, 0x21, 0x6F, 0x66, 0xE3, 0xF6, 0x0B, 0x74, 0xB2,
-    0x28, 0x38, 0xDC, 0xA7, 0x8A, 0x58, 0x0D, 0x56, 0x47, 0x3E, 0xD0, 0x5B,
-    0x5C, 0x93, 0x4E, 0xB3, 0x89, 0x87, 0x64};
-
 const unsigned char kCommonRecipientPrivateKey[] = {
     0x30, 0x81, 0xB0, 0x30, 0x1B, 0x06, 0x0A, 0x2A, 0x86, 0x48, 0x86, 0xF7,
     0x0D, 0x01, 0x0C, 0x01, 0x03, 0x30, 0x0D, 0x04, 0x08, 0xF3, 0xD6, 0x39,
@@ -260,16 +250,8 @@ class GCMMessageCryptographerTestBase : public ::testing::Test {
         kCommonRecipientPrivateKey,
         kCommonRecipientPrivateKey + arraysize(kCommonRecipientPrivateKey));
 
-    // TODO(peter): Remove the dependency on the x509 keying material when
-    // crbug.com/618025 has been resolved.
-    std::string recipient_public_key_x509(
-        kCommonRecipientPublicKeyX509,
-        kCommonRecipientPublicKeyX509 +
-            arraysize(kCommonRecipientPublicKeyX509));
-
     ASSERT_TRUE(ComputeSharedP256Secret(
-        recipient_private_key, recipient_public_key_x509, sender_public_key_,
-        &ecdh_shared_secret_));
+        recipient_private_key, sender_public_key_, &ecdh_shared_secret_));
 
     auth_secret_.assign(kCommonAuthSecret,
                         kCommonAuthSecret + arraysize(kCommonAuthSecret));
@@ -707,28 +689,22 @@ TEST_F(GCMMessageCryptographerTestVectorTest, DecryptionVectorsDraft08) {
 class GCMMessageCryptographerReferenceTest : public ::testing::Test {
  protected:
   // Computes the shared secret between the sender and the receiver. The sender
-  // must have a key-pair containing a X.509 SubjectPublicKeyInfo block and a
-  // ASN.1-encoded PKCS #8 EncryptedPrivateKeyInfo block, whereas the receiver
-  // must have a public key in uncompressed EC point format.
+  // must have a ASN.1-encoded PKCS #8 EncryptedPrivateKeyInfo block, whereas
+  // the receiver must have a public key in uncompressed EC point format.
   void ComputeSharedSecret(
       const base::StringPiece& encoded_sender_private_key,
-      const base::StringPiece& encoded_sender_public_key_x509,
       const base::StringPiece& encoded_receiver_public_key,
       std::string* shared_secret) const {
-    std::string sender_private_key, sender_public_key_x509, receiver_public_key;
+    std::string sender_private_key, receiver_public_key;
     ASSERT_TRUE(base::Base64UrlDecode(
         encoded_sender_private_key,
         base::Base64UrlDecodePolicy::IGNORE_PADDING, &sender_private_key));
-    ASSERT_TRUE(base::Base64UrlDecode(
-        encoded_sender_public_key_x509,
-        base::Base64UrlDecodePolicy::IGNORE_PADDING, &sender_public_key_x509));
     ASSERT_TRUE(base::Base64UrlDecode(
         encoded_receiver_public_key,
         base::Base64UrlDecodePolicy::IGNORE_PADDING, &receiver_public_key));
 
-    ASSERT_TRUE(ComputeSharedP256Secret(
-        sender_private_key, sender_public_key_x509, receiver_public_key,
-        shared_secret));
+    ASSERT_TRUE(ComputeSharedP256Secret(sender_private_key, receiver_public_key,
+                                        shared_secret));
   }
 };
 
@@ -751,9 +727,6 @@ TEST_F(GCMMessageCryptographerReferenceTest, ReferenceDraft03) {
   const char kSenderPublicKeyUncompressed[] =
       "BNoRDbb84JGm8g5Z5CFxurSqsXWJ11ItfXEWYVLE85Y7CYkDjXsIEc4aqxYaQ1G8BqkXCJ6D"
       "PpDrWtdWj_mugHU";
-  const char kSenderPublicX509[] =
-      "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE2hENtvzgkabyDlnkIXG6tKqxdYnXUi19cRZh"
-      "UsTzljsJiQONewgRzhqrFhpDUbwGqRcInoM-kOta11aP-a6AdQ";
 
   // The keying material used by the recipient to decrypt the |kCiphertext|.
   const char kRecipientPrivate[] =
@@ -764,9 +737,6 @@ TEST_F(GCMMessageCryptographerReferenceTest, ReferenceDraft03) {
   const char kRecipientPublicKeyUncompressed[] =
       "BCEkBjzL8Z3C-oi2Q7oE5t2Np-p7osjGLg93qUP0wvqRT21EEWyf0cQDQcakQMqz4hQKYOQ3"
       "il2nNZct4HgAUQU";
-  const char kRecipientPublicX509[] =
-      "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEISQGPMvxncL6iLZDugTm3Y2n6nuiyMYuD3ep"
-      "Q_TC-pFPbUQRbJ_RxANBxqRAyrPiFApg5DeKXac1ly3geABRBQ";
 
   // The ciphertext and associated plaintext of the message.
   const char kCiphertext[] = "6nqAQUME8hNqw5J3kl8cpVVJylXKYqZOeseZG8UueKpA";
@@ -775,12 +745,11 @@ TEST_F(GCMMessageCryptographerReferenceTest, ReferenceDraft03) {
   std::string sender_shared_secret, receiver_shared_secret;
 
   // Compute the shared secrets between the sender and receiver's keys.
-  ASSERT_NO_FATAL_FAILURE(ComputeSharedSecret(kSenderPrivate, kSenderPublicX509,
-                                              kRecipientPublicKeyUncompressed,
-                                              &sender_shared_secret));
   ASSERT_NO_FATAL_FAILURE(ComputeSharedSecret(
-      kRecipientPrivate, kRecipientPublicX509, kSenderPublicKeyUncompressed,
-      &receiver_shared_secret));
+      kSenderPrivate, kRecipientPublicKeyUncompressed, &sender_shared_secret));
+  ASSERT_NO_FATAL_FAILURE(ComputeSharedSecret(kRecipientPrivate,
+                                              kSenderPublicKeyUncompressed,
+                                              &receiver_shared_secret));
 
   ASSERT_GT(sender_shared_secret.size(), 0u);
   ASSERT_EQ(sender_shared_secret, receiver_shared_secret);
@@ -835,9 +804,6 @@ TEST_F(GCMMessageCryptographerReferenceTest, ReferenceDraft08) {
       "AyQzjG7jdSXooDx_yPXgwMskoNzKhBXIG0AZF7MBimdXFTg_wlv38empWRr_-x4fnQiIBKya"
       "pt6uBOfYVuE_nnhqudqvSxiiv6hikBvxS2zabgph8-vFPQG10uUv8xkAO6vPdtTABCUzCXQU"
       "p1QmuP471ZdaNAMuCPmxry-C";
-  const char kSenderPublicX509[] =
-      "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE_jP0qw3qcZFNtVgj9ztUlI9BMG2SBzLbuaWa"
-      "UyhkgiAOWXp7e8JguhwieZhYCZLpOXMALzASooro8Gu7eOXsDw";
 
   // The keying material used by the recipient to decrypt the |kCiphertext|.
   const char kRecipientPrivate[] =
@@ -848,9 +814,6 @@ TEST_F(GCMMessageCryptographerReferenceTest, ReferenceDraft08) {
   const char kRecipientPublicKeyUncompressed[] =
       "BCVxsr7N_eNgVRqvHtD0zTZsEc6-VV-JvLexhqUzORcxaOzi6-AYWXvTBHm4bjyPjs7Vd8pZ"
       "GH6SRpkNtoIAiw4";
-  const char kRecipientPublicX509[] =
-      "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEJXGyvs3942BVGq8e0PTNNmwRzr5VX4m8t7GG"
-      "pTM5FzFo7OLr4BhZe9MEebhuPI-OztV3ylkYfpJGmQ22ggCLDg";
 
   // The plain text of the message, as well as the encrypted reference message.
   const char kPlaintext[] = "When I grow up, I want to be a watermelon";
@@ -876,22 +839,17 @@ TEST_F(GCMMessageCryptographerReferenceTest, ReferenceDraft08) {
   std::string sender_shared_secret, receiver_shared_secret;
 
   // Compute the shared secrets between the sender and receiver's keys.
-  ASSERT_NO_FATAL_FAILURE(ComputeSharedSecret(kSenderPrivate, kSenderPublicX509,
-                                              kRecipientPublicKeyUncompressed,
-                                              &sender_shared_secret));
+  ASSERT_NO_FATAL_FAILURE(ComputeSharedSecret(
+      kSenderPrivate, kRecipientPublicKeyUncompressed, &sender_shared_secret));
 
   // Compute the shared secret based on the sender's public key, which isn't a
   // constant but instead is included in the message's binary header.
-  std::string sender_private_key, sender_public_key_x509;
+  std::string sender_private_key;
   ASSERT_TRUE(base::Base64UrlDecode(kRecipientPrivate,
                                     base::Base64UrlDecodePolicy::IGNORE_PADDING,
                                     &sender_private_key));
-  ASSERT_TRUE(base::Base64UrlDecode(kRecipientPublicX509,
-                                    base::Base64UrlDecodePolicy::IGNORE_PADDING,
-                                    &sender_public_key_x509));
 
-  ASSERT_TRUE(ComputeSharedP256Secret(sender_private_key,
-                                      sender_public_key_x509, sender_public_key,
+  ASSERT_TRUE(ComputeSharedP256Secret(sender_private_key, sender_public_key,
                                       &receiver_shared_secret));
 
   ASSERT_GT(sender_shared_secret.size(), 0u);

components/gcm_driver/crypto/p256_key_util.cc

@@ -92,18 +92,14 @@ bool CreateP256KeyPair(std::string* out_private_key,
 }
 
 bool ComputeSharedP256Secret(const base::StringPiece& private_key,
-                             const base::StringPiece& public_key_x509,
                              const base::StringPiece& peer_public_key,
                              std::string* out_shared_secret) {
   DCHECK(out_shared_secret);
 
   std::unique_ptr<crypto::ECPrivateKey> local_key_pair(
       crypto::ECPrivateKey::CreateFromEncryptedPrivateKeyInfo(
           std::vector<uint8_t>(private_key.data(),
-                               private_key.data() + private_key.size()),
-          std::vector<uint8_t>(
-              public_key_x509.data(),
-              public_key_x509.data() + public_key_x509.size())));
+                               private_key.data() + private_key.size())));
 
   if (!local_key_pair) {
     DLOG(ERROR) << "Unable to create the local key pair.";

components/gcm_driver/crypto/p256_key_util.h

@@ -28,13 +28,11 @@ bool CreateP256KeyPair(std::string* out_private_key,
 // whether the secret could be computed, and was written to the out argument.
 //
 // The |private_key| must be an ASN.1-encoded PKCS#8 EncryptedPrivateKeyInfo
-// block together, where |public_key_x509| must be an X.509 SubjectPublicKeyInfo
-// block. This is necessary for NSS to be able to import the |private_key|.
+// block. This is legacy from the NSS implementation.
 //
 // The |peer_public_key| must be an octet string in uncompressed form per
 // SEC1 2.3.3.
 bool ComputeSharedP256Secret(const base::StringPiece& private_key,
-                             const base::StringPiece& public_key_x509,
                              const base::StringPiece& peer_public_key,
                              std::string* out_shared_secret) WARN_UNUSED_RESULT;
 

components/gcm_driver/crypto/p256_key_util_unittest.cc

@@ -26,9 +26,6 @@ const char kBobPrivateKey[] =
     "1RglXKiPdy+Ue5eubZ3Xe/pz/5yQWvysXLaTT+/LeuAoq2gwKK2AINTWglIEMCmcveERa0gZDW"
     "avhYEYtEynNLVop4bHlii5DcUY1/e9DY5Dxh7b2dYdWI2Ev+529KughVKK8qFu12wn3cSSlfXh"
     "9Hb8Nh/4yP0jg6k5k=";
-const char kBobPublicKeyX509[] =
-    "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEVVJqpW6OqkeXNhDBZjweZb+he+5Iyca7vwIYU3"
-    "IdDHup4xG3A1Ih03GQE6jBz+0g9x/Rf/J2tgEg2DWl2TxD/Q==";
 const char kBobPublicKey[] =
     "BFVSaqVujqpHlzYQwWY8HmW/oXvuSMnGu78CGFNyHQx7qeMRtwNSIdNxkBOowc/tIPcf0X/ydr"
     "YBINg1pdk8Q/0=";
@@ -38,9 +35,6 @@ const char kCarolPrivateKey[] =
     "J4KT/C32F6K3edQnZ2J750g6nMVtkoK9TF23UcEIVB0Lo7FG4T5WF03wjC4A+5FC/1mYzsWFHO"
     "6AugLhum5psqX3fq6UmgYoir9dJsI7Rmmn1JH8Gtw6KJHMncPi1lGriLZqzcrw+oULVf6dcnH1"
     "z9F39GuYob5+sY7ak=";
-const char kCarolPublicKeyX509[] =
-    "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAETqNDkapziFRUvEXfP9zEpwJi+kYh/+JbV5BZDD"
-    "QKS9kmXvKz+ahuKVmpYNkDI0ZdYM1KkNMUxd4/hprQ1L9sEA==";
 const char kCarolPublicKey[] =
     "BE6jQ5Gqc4hUVLxF3z/cxKcCYvpGIf/iW1eQWQw0CkvZJl7ys/mobilZqWDZAyNGXWDNSpDTFM"
     "XeP4aa0NS/bBA=";
@@ -87,60 +81,56 @@ TEST(P256KeyUtilTest, SharedSecretCalculation) {
   ASSERT_NE(bob_private_key, alice_private_key);
 
   std::string bob_shared_secret, alice_shared_secret;
-  ASSERT_TRUE(ComputeSharedP256Secret(bob_private_key, bob_public_key_x509,
-                                      alice_public_key, &bob_shared_secret));
-  ASSERT_TRUE(ComputeSharedP256Secret(alice_private_key, alice_public_key_x509,
-                                      bob_public_key, &alice_shared_secret));
+  ASSERT_TRUE(ComputeSharedP256Secret(bob_private_key, alice_public_key,
+                                      &bob_shared_secret));
+  ASSERT_TRUE(ComputeSharedP256Secret(alice_private_key, bob_public_key,
+                                      &alice_shared_secret));
 
   EXPECT_GT(bob_shared_secret.size(), 0u);
   EXPECT_EQ(bob_shared_secret, alice_shared_secret);
 
   std::string unused_shared_secret;
 
   // Empty and too short peer public values should be considered invalid.
-  ASSERT_FALSE(ComputeSharedP256Secret(bob_private_key, bob_public_key_x509,
-                                       "", &unused_shared_secret));
-  ASSERT_FALSE(ComputeSharedP256Secret(bob_private_key, bob_public_key_x509,
-                                       bob_public_key.substr(1),
-                                       &unused_shared_secret));
-
+  ASSERT_FALSE(
+      ComputeSharedP256Secret(bob_private_key, "", &unused_shared_secret));
+  ASSERT_FALSE(ComputeSharedP256Secret(
+      bob_private_key, bob_public_key.substr(1), &unused_shared_secret));
 }
 
 TEST(P256KeyUtilTest, SharedSecretWithPreExistingKey) {
-  std::string bob_private_key, bob_public_key_x509, bob_public_key;
+  std::string bob_private_key, bob_public_key;
   std::string alice_private_key, alice_public_key_x509, alice_public_key;
 
   ASSERT_TRUE(base::Base64Decode(kBobPrivateKey, &bob_private_key));
-  ASSERT_TRUE(base::Base64Decode(kBobPublicKeyX509, &bob_public_key_x509));
   ASSERT_TRUE(base::Base64Decode(kBobPublicKey, &bob_public_key));
 
   // First verify against a newly created, ephemeral key-pair.
   ASSERT_TRUE(CreateP256KeyPair(
       &alice_private_key, &alice_public_key_x509, &alice_public_key));
 
   std::string bob_shared_secret, alice_shared_secret;
-  ASSERT_TRUE(ComputeSharedP256Secret(bob_private_key, bob_public_key_x509,
-                                      alice_public_key, &bob_shared_secret));
-  ASSERT_TRUE(ComputeSharedP256Secret(alice_private_key, alice_public_key_x509,
-                                      bob_public_key, &alice_shared_secret));
+  ASSERT_TRUE(ComputeSharedP256Secret(bob_private_key, alice_public_key,
+                                      &bob_shared_secret));
+  ASSERT_TRUE(ComputeSharedP256Secret(alice_private_key, bob_public_key,
+                                      &alice_shared_secret));
 
   EXPECT_GT(bob_shared_secret.size(), 0u);
   EXPECT_EQ(bob_shared_secret, alice_shared_secret);
 
-  std::string carol_private_key, carol_public_key_x509, carol_public_key;
+  std::string carol_private_key, carol_public_key;
 
   ASSERT_TRUE(base::Base64Decode(kCarolPrivateKey, &carol_private_key));
-  ASSERT_TRUE(base::Base64Decode(kCarolPublicKeyX509, &carol_public_key_x509));
   ASSERT_TRUE(base::Base64Decode(kCarolPublicKey, &carol_public_key));
 
   bob_shared_secret.clear();
   std::string carol_shared_secret;
 
   // Then verify against another stored key-pair and shared secret.
-  ASSERT_TRUE(ComputeSharedP256Secret(bob_private_key, bob_public_key_x509,
-                                      carol_public_key, &bob_shared_secret));
-  ASSERT_TRUE(ComputeSharedP256Secret(carol_private_key, carol_public_key_x509,
-                                      bob_public_key, &carol_shared_secret));
+  ASSERT_TRUE(ComputeSharedP256Secret(bob_private_key, carol_public_key,
+                                      &bob_shared_secret));
+  ASSERT_TRUE(ComputeSharedP256Secret(carol_private_key, bob_public_key,
+                                      &carol_shared_secret));
 
   EXPECT_GT(carol_shared_secret.size(), 0u);
   EXPECT_EQ(carol_shared_secret, bob_shared_secret);

crypto/ec_private_key.cc

@@ -59,10 +59,7 @@ std::unique_ptr<ECPrivateKey> ECPrivateKey::CreateFromPrivateKeyInfo(
 
 // static
 std::unique_ptr<ECPrivateKey> ECPrivateKey::CreateFromEncryptedPrivateKeyInfo(
-    const std::vector<uint8_t>& encrypted_private_key_info,
-    const std::vector<uint8_t>& subject_public_key_info) {
-  // TODO(davidben): The |subject_public_key_info| parameter is a remnant of
-  // the NSS implementation. Remove it.
+    const std::vector<uint8_t>& encrypted_private_key_info) {
   OpenSSLErrStackTracer err_tracer(FROM_HERE);
 
   CBS cbs;

crypto/ec_private_key.h

@@ -47,8 +47,7 @@ class CRYPTO_EXPORT ECPrivateKey {
   // This function is deprecated. Use CreateFromPrivateKeyInfo for new code.
   // See https://crbug.com/603319.
   static std::unique_ptr<ECPrivateKey> CreateFromEncryptedPrivateKeyInfo(
-      const std::vector<uint8_t>& encrypted_private_key_info,
-      const std::vector<uint8_t>& subject_public_key_info);
+      const std::vector<uint8_t>& encrypted_private_key_info);
 
   // Returns a copy of the object.
   std::unique_ptr<ECPrivateKey> Copy() const;