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..dc033924cdfb1cd0a0ee859906856df04dbe5c82 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()
+      : identityrsa1_(),
+        identityrsa2_(),
+        identityecdsa1_(),
+        identityecdsa2_() {}
 
   ~SSLIdentityTest() {
   }
 
   virtual void SetUp() {
-    identity1_.reset(SSLIdentity::Generate("test1"));
-    identity2_.reset(SSLIdentity::Generate("test2"));
+    identityrsa1_.reset(SSLIdentity::Generate("test1", rtc::KT_RSA));
+    identityrsa2_.reset(SSLIdentity::Generate("test2", rtc::KT_RSA));
+    identityecdsa1_.reset(SSLIdentity::Generate("test3", rtc::KT_ECDSA));
+    identityecdsa2_.reset(SSLIdentity::Generate("test4", rtc::KT_ECDSA));
 
-    ASSERT_TRUE(identity1_);
-    ASSERT_TRUE(identity2_);
+    ASSERT_TRUE(identityrsa1_);
+    ASSERT_TRUE(identityrsa2_);
+    ASSERT_TRUE(identityecdsa1_);
+    ASSERT_TRUE(identityecdsa2_);
 
-    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(identityrsa1_->certificate().GetSignatureDigestAlgorithm(
+        &digest_algorithm));
+    ASSERT_EQ(rtc::DIGEST_SHA_256, digest_algorithm);
+
+    ASSERT_TRUE(identityrsa2_->certificate().GetSignatureDigestAlgorithm(
+        &digest_algorithm));
+    ASSERT_EQ(rtc::DIGEST_SHA_256, digest_algorithm);
+
+    ASSERT_TRUE(identityecdsa1_->certificate().GetSignatureDigestAlgorithm(
         &digest_algorithm));
     ASSERT_EQ(rtc::DIGEST_SHA_256, digest_algorithm);
-    ASSERT_TRUE(identity2_->certificate().GetSignatureDigestAlgorithm(
+
+    ASSERT_TRUE(identityecdsa2_->certificate().GetSignatureDigestAlgorithm(
         &digest_algorithm));
     ASSERT_EQ(rtc::DIGEST_SHA_256, digest_algorithm);
 
@@ -75,38 +88,59 @@ class SSLIdentityTest : public testing::Test {
 
   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 digest_type[64];  // 64 is the largest hash used here.

[juberti1, 2015/07/03 03:15:18]

proper style for this would be DigestType

[torbjorng (webrtc), 2015/07/06 10:11:55]

Done.

+    digest_type digest1;
+    digest_type digest[4];
+    size_t digest_len;
     bool rv;
 
-    rv = identity1_->certificate().ComputeDigest(algorithm,
-                                                 digest1, sizeof(digest1),
-                                                 &digest1_len);
+    ASSERT_TRUE(expected_len <= 64);
+
+    // Record digests of our 4 identities.
+    memset(digest[0], 0, expected_len);

[juberti1, 2015/07/03 03:15:18]

This seems like a lot of redundant code. Suggest extracting a helper routine
that calls ComputeDigest twice on a supplied cert and verifies it gets the right
length and same result. Then you can call that routine 5x from this function.

[torbjorng (webrtc), 2015/07/06 10:11:55]

Done.

+    rv = identityrsa1_->certificate().ComputeDigest(
+        algorithm, digest[0], sizeof(digest_type), &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);
+    memset(digest[1], 0, expected_len);
+    rv = identityrsa2_->certificate().ComputeDigest(
+        algorithm, digest[1], sizeof(digest_type), &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);
 
+    memset(digest[2], 0, expected_len);
+    rv = identityecdsa1_->certificate().ComputeDigest(
+        algorithm, digest[2], sizeof(digest_type), &digest_len);
+    EXPECT_TRUE(rv);
+    EXPECT_EQ(expected_len, digest_len);
 
-    rv = identity2_->certificate().ComputeDigest(algorithm,
-                                                 digest2, sizeof(digest2),
-                                                 &digest2_len);
+    memset(digest[3], 0, expected_len);
+    rv = identityecdsa2_->certificate().ComputeDigest(
+        algorithm, digest[3], sizeof(digest_type), &digest_len);
     EXPECT_TRUE(rv);
-    EXPECT_EQ(expected_len, digest2_len);
-    EXPECT_NE(0, memcmp(digest1, digest2, expected_len));
+    EXPECT_EQ(expected_len, digest_len);
+
+    // Repeat digest computation for one identity as a sanity check.
+    memset(digest1, 0xff, expected_len);
+    rv = identityrsa1_->certificate().ComputeDigest(
+        algorithm, digest1, sizeof(digest_type), &digest_len);
+    EXPECT_TRUE(rv);
+    EXPECT_EQ(expected_len, digest_len);
+    EXPECT_EQ(0, memcmp(digest[0], digest1, expected_len));
+
+    // Sanity check that all four digests are unique.  This could theoretically

[juberti1, 2015/07/03 03:15:18]

This seems like it might not be all that valuable (although the chance of a
failure is remarkably low).

[torbjorng (webrtc), 2015/07/06 10:11:55]

I inherited this check from the old code, I only generalised it.

It only catches seriously misbehaved functions. A real collision will
only happen if random generation is badly broken so that two keys
actually come out the same.

+    // 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));
+      }
+    }
 
     // If we have an expected hash for the test cert, check it.
     if (expected_digest) {

[juberti1, 2015/07/03 03:15:18]

This is only used in one case, so suggest factoring this out into its own
function.

[torbjorng (webrtc), 2015/07/06 10:11:55]

Done.

-      unsigned char digest3[64];
+      digest_type digest3;
       size_t digest3_len;
 
       rv = test_cert_->ComputeDigest(algorithm, digest3, sizeof(digest3),
@@ -118,8 +152,10 @@ class SSLIdentityTest : public testing::Test {
   }
 
  private:
-  rtc::scoped_ptr<SSLIdentity> identity1_;
-  rtc::scoped_ptr<SSLIdentity> identity2_;
+  rtc::scoped_ptr<SSLIdentity> identityrsa1_;

[juberti1, 2015/07/03 03:15:18]

identiy_rsa1_, etc would be more readable

[torbjorng (webrtc), 2015/07/06 10:11:55]

Done.

+  rtc::scoped_ptr<SSLIdentity> identityrsa2_;
+  rtc::scoped_ptr<SSLIdentity> identityecdsa1_;
+  rtc::scoped_ptr<SSLIdentity> identityecdsa2_;
   rtc::scoped_ptr<rtc::SSLCertificate> test_cert_;
 };
 
@@ -148,7 +184,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 +222,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));