Support generation of EC keys using P256 curve and support ECDSA certs.

webrtc/base/sslidentity_unittest.cc

Index: webrtc/base/sslidentity_unittest.cc
diff --git a/webrtc/base/sslidentity_unittest.cc b/webrtc/base/sslidentity_unittest.cc
index fd754114288c1cd7a9b97bc429973df42a5ec474..7e30d62c12f06a89e3a9d0300da322e073e05b1a 100644
--- a/webrtc/base/sslidentity_unittest.cc
+++ b/webrtc/base/sslidentity_unittest.cc
@@ -38,33 +38,46 @@ const unsigned char kTestCertSha1[] = {0xA6, 0xC8, 0x59, 0xEA,
 
 class SSLIdentityTest : public testing::Test {
  public:
-  SSLIdentityTest() :
-      identity1_(), identity2_() {
-  }
+  SSLIdentityTest()
+      : identity_rsa1_(),

[juberti1, 2015/07/21 22:54:13]

These don't need to be initialized.

[torbjorng (webrtc), 2015/08/17 12:12:45]

Done.

+        identity_rsa2_(),
+        identity_ecdsa1_(),
+        identity_ecdsa2_() {}
 
   ~SSLIdentityTest() {
   }
 
   virtual void SetUp() {
-    identity1_.reset(SSLIdentity::Generate("test1"));
-    identity2_.reset(SSLIdentity::Generate("test2"));
+    identity_rsa1_.reset(SSLIdentity::Generate("test1", rtc::KT_RSA));
+    identity_rsa2_.reset(SSLIdentity::Generate("test2", rtc::KT_RSA));
+    identity_ecdsa1_.reset(SSLIdentity::Generate("test3", rtc::KT_ECDSA));
+    identity_ecdsa2_.reset(SSLIdentity::Generate("test4", rtc::KT_ECDSA));
 
-    ASSERT_TRUE(identity1_);
-    ASSERT_TRUE(identity2_);
+    ASSERT_TRUE(identity_rsa1_);
+    ASSERT_TRUE(identity_rsa2_);
+    ASSERT_TRUE(identity_ecdsa1_);
+    ASSERT_TRUE(identity_ecdsa2_);
 
-    test_cert_.reset(
-        rtc::SSLCertificate::FromPEMString(kTestCertificate));
+    test_cert_.reset(rtc::SSLCertificate::FromPEMString(kTestCertificate));
     ASSERT_TRUE(test_cert_);
   }
 
   void TestGetSignatureDigestAlgorithm() {
     std::string digest_algorithm;
-    // Both NSSIdentity::Generate and OpenSSLIdentity::Generate are
-    // hard-coded to generate RSA-SHA256 certificates.
-    ASSERT_TRUE(identity1_->certificate().GetSignatureDigestAlgorithm(
+
+    ASSERT_TRUE(identity_rsa1_->certificate().GetSignatureDigestAlgorithm(
+        &digest_algorithm));
+    ASSERT_EQ(rtc::DIGEST_SHA_256, digest_algorithm);
+
+    ASSERT_TRUE(identity_rsa2_->certificate().GetSignatureDigestAlgorithm(
         &digest_algorithm));
     ASSERT_EQ(rtc::DIGEST_SHA_256, digest_algorithm);
-    ASSERT_TRUE(identity2_->certificate().GetSignatureDigestAlgorithm(
+
+    ASSERT_TRUE(identity_ecdsa1_->certificate().GetSignatureDigestAlgorithm(
+        &digest_algorithm));
+    ASSERT_EQ(rtc::DIGEST_SHA_256, digest_algorithm);
+
+    ASSERT_TRUE(identity_ecdsa2_->certificate().GetSignatureDigestAlgorithm(
         &digest_algorithm));
     ASSERT_EQ(rtc::DIGEST_SHA_256, digest_algorithm);
 
@@ -73,58 +86,80 @@ class SSLIdentityTest : public testing::Test {
     ASSERT_EQ(rtc::DIGEST_MD5, digest_algorithm);
   }
 
-  void TestDigest(const std::string &algorithm, size_t expected_len,
-                  const unsigned char *expected_digest = NULL) {
-    unsigned char digest1[64];
-    unsigned char digest1b[64];
-    unsigned char digest2[64];
-    size_t digest1_len;
-    size_t digest1b_len;
-    size_t digest2_len;
+  typedef unsigned char DigestType[64];  // 64 is the largest hash used here.

[juberti1, 2015/07/21 22:54:13]

You can use MessageDigest::kMaxSize instead of 64 here

[torbjorng (webrtc), 2015/08/17 12:12:45]

Done.

+
+  void TestDigestHelper(DigestType digest,
+                        const SSLIdentity* identity,
+                        const std::string& algorithm,
+                        size_t expected_len) {
+    DigestType digest1;
+    size_t digest_len;
     bool rv;
 
-    rv = identity1_->certificate().ComputeDigest(algorithm,
-                                                 digest1, sizeof(digest1),
-                                                 &digest1_len);
+    memset(digest, 0, expected_len);
+    rv = identity->certificate().ComputeDigest(algorithm, digest,
+                                               sizeof(DigestType), &digest_len);
     EXPECT_TRUE(rv);
-    EXPECT_EQ(expected_len, digest1_len);
+    EXPECT_EQ(expected_len, digest_len);
 
-    rv = identity1_->certificate().ComputeDigest(algorithm,
-                                                 digest1b, sizeof(digest1b),
-                                                 &digest1b_len);
+    // Repeat digest computation for the identity as a sanity check.
+    memset(digest1, 0xff, expected_len);
+    rv = identity->certificate().ComputeDigest(algorithm, digest1,
+                                               sizeof(DigestType), &digest_len);
     EXPECT_TRUE(rv);
-    EXPECT_EQ(expected_len, digest1b_len);
-    EXPECT_EQ(0, memcmp(digest1, digest1b, expected_len));
+    EXPECT_EQ(expected_len, digest_len);
+
+    EXPECT_EQ(0, memcmp(digest, digest1, expected_len));
+  }
 
+  void TestDigest(const std::string& algorithm, size_t expected_len) {

[juberti1, 2015/07/21 22:54:13]

I would call this TestDigestForGeneratedCerts, or something like that

[torbjorng (webrtc), 2015/08/17 12:12:45]

Done.

+    DigestType digest[4];
 
-    rv = identity2_->certificate().ComputeDigest(algorithm,
-                                                 digest2, sizeof(digest2),
-                                                 &digest2_len);
-    EXPECT_TRUE(rv);
-    EXPECT_EQ(expected_len, digest2_len);
-    EXPECT_NE(0, memcmp(digest1, digest2, expected_len));
-
-    // If we have an expected hash for the test cert, check it.
-    if (expected_digest) {
-      unsigned char digest3[64];
-      size_t digest3_len;
-
-      rv = test_cert_->ComputeDigest(algorithm, digest3, sizeof(digest3),
-                                    &digest3_len);
-      EXPECT_TRUE(rv);
-      EXPECT_EQ(expected_len, digest3_len);
-      EXPECT_EQ(0, memcmp(digest3, expected_digest, expected_len));
+    ASSERT_TRUE(expected_len <= 64);

[juberti1, 2015/07/21 22:54:13]

Suggest using <= sizeof(digest[0]), to avoid accidental mistakes

[torbjorng (webrtc), 2015/08/17 12:12:45]

Done, except using sizeof(DigestType).

+
+    TestDigestHelper(digest[0], identity_rsa1_.get(), algorithm, expected_len);
+    TestDigestHelper(digest[1], identity_rsa2_.get(), algorithm, expected_len);
+    TestDigestHelper(digest[2], identity_ecdsa1_.get(), algorithm,
+                     expected_len);
+    TestDigestHelper(digest[3], identity_ecdsa2_.get(), algorithm,
+                     expected_len);
+
+    // Sanity check that all four digests are unique.  This could theoretically
+    // fail, since SHA256 collisions have a non-zero probability.
+    for (int i = 0; i < 4; i++) {
+      for (int j = 0; j < 4; j++) {
+        if (i != j)
+          EXPECT_NE(0, memcmp(digest[i], digest[j], expected_len));
+      }
     }
   }
 
+  void TestDigestExample(const std::string& algorithm,

[juberti1, 2015/07/21 22:54:13]

I would call this TestDigestForFixedCert

[torbjorng (webrtc), 2015/08/17 12:12:45]

Done.

+                         size_t expected_len,
+                         const unsigned char* expected_digest) {
+    bool rv;
+    DigestType digest;
+    size_t digest_len;
+
+    ASSERT_TRUE(expected_len <= 64);

[juberti1, 2015/07/21 22:54:13]

<= sizeof(digest)

[torbjorng (webrtc), 2015/08/17 12:12:45]

Done.

+
+    rv = test_cert_->ComputeDigest(algorithm, digest, sizeof(digest),
+                                   &digest_len);
+    EXPECT_TRUE(rv);
+    EXPECT_EQ(expected_len, digest_len);
+    EXPECT_EQ(0, memcmp(digest, expected_digest, expected_len));
+  }
+
  private:
-  rtc::scoped_ptr<SSLIdentity> identity1_;
-  rtc::scoped_ptr<SSLIdentity> identity2_;
+  rtc::scoped_ptr<SSLIdentity> identity_rsa1_;
+  rtc::scoped_ptr<SSLIdentity> identity_rsa2_;
+  rtc::scoped_ptr<SSLIdentity> identity_ecdsa1_;
+  rtc::scoped_ptr<SSLIdentity> identity_ecdsa2_;
   rtc::scoped_ptr<rtc::SSLCertificate> test_cert_;
 };
 
 TEST_F(SSLIdentityTest, DigestSHA1) {
-  TestDigest(rtc::DIGEST_SHA_1, 20, kTestCertSha1);
+  TestDigestExample(rtc::DIGEST_SHA_1, 20, kTestCertSha1);

[juberti1, 2015/07/21 22:54:13]

I would have a call to both TestDigestForGeneratedCert and
TestDigestForFixedCert here.
You could also do the same for the other hash functions; here are the hashes for
kTestCert:
SHA224(a.bin)= d4cec6cf28cbe9773836cfb13b4ad7bdae242108cf6a440d3f942a5b
SHA256(a.bin)= 416bb49347797724770b8b2ea62be0f90aed1f31a6f75ca15ac4b0a2a478b976
SHA384(a.bin)=
42319a791dd608bf3bba36d8374a9a75d3256e2892be06b7c5a083e386b103fc6447d6d8aad9366004ccbe7d6ae83449
SHA512(a.bin)=
511dec023d5145d3d81da49d43c9ee326f4f37eeab3f25df72fc611ad592ff6b287158b3e18a18cf61330e14c304aa07f6a5dadc42422235ce26584a336dbcb6

[torbjorng (webrtc), 2015/08/17 12:12:45]

Using more hashes checking fixed data makes sense. I've completed the set using
your hashes (thanks!).

I'm afraid don't understand what TestDigestForGeneratedCert would do here.
Adding test_cert_ checking to TestDigestForGeneratedCert could be done.
Comparing hashes with identities with hashes for a cert is somewhat odd, though.

The (pre-existing) code of this test could perhaps take some cleanups. Adding
fixed RSA/EC identities (as opposed to fixed certs) in
TestDigestForGeneratedCert, makes sense.  Using the identities and certs from
the end of this file for that purpose and let them replace kTestCertificate also
makes some sense. IMHO, that's another CL.

 }
 
 // HASH_AlgSHA224 is not supported in the chromium linux build.
@@ -148,7 +183,7 @@ TEST_F(SSLIdentityTest, DigestSHA512) {
   TestDigest(rtc::DIGEST_SHA_512, 64);
 }
 
-TEST_F(SSLIdentityTest, FromPEMStrings) {
+TEST_F(SSLIdentityTest, FromPEMStringsRSA) {
   static const char kRSA_PRIVATE_KEY_PEM[] =
       "-----BEGIN RSA PRIVATE KEY-----\n"
       "MIICdwIBADANBgkqhkiG9w0BAQEFAASCAmEwggJdAgEAAoGBAMYRkbhmI7kVA/rM\n"
@@ -186,6 +221,39 @@ TEST_F(SSLIdentityTest, FromPEMStrings) {
   EXPECT_EQ(kCERT_PEM, identity->certificate().ToPEMString());
 }
 
+#if SSL_USE_OPENSSL
+// This will not work on NSS as PK11_ImportDERPrivateKeyInfoAndReturnKey is not
+// ready for EC keys.  Furthermore, NSSIdentity::FromPEMStrings is currently
+// hardwired for RSA (the header matching via kPemTypeRsaPrivateKey needs
+// trivial generalization).
+TEST_F(SSLIdentityTest, FromPEMStringsEC) {
+  static const char kRSA_PRIVATE_KEY_PEM[] =
+      "-----BEGIN EC PRIVATE KEY-----\n"
+      "MHcCAQEEIKkIztWLPbs4Y2zWv7VW2Ov4is2ifleCuPgRB8fRv3IkoAoGCCqGSM49\n"
+      "AwEHoUQDQgAEDPV33NrhSdhg9cBRkUWUXnVMXc3h17i9ARbSmNgminKcBXb8/y8L\n"
+      "A76cMWQPPM0ybHO8OS7ZVg2U/m+TwE1M2g==\n"
+      "-----END EC PRIVATE KEY-----\n";
+  static const char kCERT_PEM[] =
+      "-----BEGIN CERTIFICATE-----\n"
+      "MIIB0jCCAXmgAwIBAgIJAMCjpFt9t6LMMAoGCCqGSM49BAMCMEUxCzAJBgNVBAYT\n"
+      "AkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBXaWRn\n"
+      "aXRzIFB0eSBMdGQwIBcNMTUwNjMwMTMwMTIyWhgPMjI4OTA0MTMxMzAxMjJaMEUx\n"
+      "CzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRl\n"
+      "cm5ldCBXaWRnaXRzIFB0eSBMdGQwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAQM\n"
+      "9Xfc2uFJ2GD1wFGRRZRedUxdzeHXuL0BFtKY2CaKcpwFdvz/LwsDvpwxZA88zTJs\n"
+      "c7w5LtlWDZT+b5PATUzao1AwTjAdBgNVHQ4EFgQUYHq6nxNNIE832ZmaHc/noODO\n"
+      "rtAwHwYDVR0jBBgwFoAUYHq6nxNNIE832ZmaHc/noODOrtAwDAYDVR0TBAUwAwEB\n"
+      "/zAKBggqhkjOPQQDAgNHADBEAiAQRojsTyZG0BlKoU7gOt5h+yAMLl2cxmDtOIQr\n"
+      "GWP/PwIgJynB4AUDsPT0DWmethOXYijB5sY5UPd9DvgmiS/Mr6s=\n"
+      "-----END CERTIFICATE-----\n";
+
+  rtc::scoped_ptr<SSLIdentity> identity(
+      SSLIdentity::FromPEMStrings(kRSA_PRIVATE_KEY_PEM, kCERT_PEM));
+  EXPECT_TRUE(identity);
+  EXPECT_EQ(kCERT_PEM, identity->certificate().ToPEMString());
+}
+#endif
+
 TEST_F(SSLIdentityTest, PemDerConversion) {
   std::string der;
   EXPECT_TRUE(SSLIdentity::PemToDer("CERTIFICATE", kTestCertificate, &der));