RTCCertificate serialization.

webrtc/base/rtccertificate_unittest.cc

Index: webrtc/base/rtccertificate_unittest.cc
diff --git a/webrtc/base/rtccertificate_unittests.cc b/webrtc/base/rtccertificate_unittest.cc
similarity index 81%
rename from webrtc/base/rtccertificate_unittests.cc
rename to webrtc/base/rtccertificate_unittest.cc
index 84c854478b219773b44c9910cef3c400b4eca72d..3cca473368933c7536a4e818555e80565796f7fc 100644
--- a/webrtc/base/rtccertificate_unittests.cc
+++ b/webrtc/base/rtccertificate_unittest.cc
@@ -35,6 +35,12 @@ class RTCCertificateTest : public testing::Test {
   ~RTCCertificateTest() {}
 
  protected:
+  scoped_refptr<RTCCertificate> GenerateECDSA() {
+    scoped_ptr<SSLIdentity> identity(

[nisse-webrtc, 2016/04/22 11:38:25]

Do you really need the local variable?

[hbos, 2016/04/22 13:19:29]

I do now that I RTC_CHECK. (I think I did before or it would complain about
std::move but not sure.)

[nisse-webrtc, 2016/04/22 13:44:06]

Ok. Can Generate ever fail?

[hbos, 2016/04/25 14:23:23]

While it shouldn't happen under normal circumstances, hence the RTC_CHECK, it
*could* happen and the API says it returns null on failure. torbjorng might have
a better answer as to when it might fail.

+        SSLIdentity::Generate(kTestCertCommonName, KeyParams::ECDSA()));
+    return RTCCertificate::Create(std::move(identity));
+  }
+
   // Timestamp note:
   //   All timestamps in this unittest are expressed in number of seconds since
   // epoch, 1970-01-01T00:00:00Z (UTC). The RTCCertificate interface uses ms,
@@ -85,10 +91,7 @@ class RTCCertificateTest : public testing::Test {
 TEST_F(RTCCertificateTest, NewCertificateNotExpired) {
   // Generate a real certificate without specifying the expiration time.
   // Certificate type doesn't matter, using ECDSA because it's fast to generate.
-  scoped_ptr<SSLIdentity> identity(
-      SSLIdentity::Generate(kTestCertCommonName, KeyParams::ECDSA()));
-  scoped_refptr<RTCCertificate> certificate =
-      RTCCertificate::Create(std::move(identity));
+  scoped_refptr<RTCCertificate> certificate = GenerateECDSA();
 
   uint64_t now = NowSeconds();
   EXPECT_FALSE(HasExpiredSeconds(certificate, now));
@@ -115,4 +118,20 @@ TEST_F(RTCCertificateTest, ExpiresInOneSecond) {
   EXPECT_TRUE(HasExpiredSeconds(certificate, now + 2));
 }
 
+TEST_F(RTCCertificateTest, CloneWithPemSerialization) {
+  scoped_refptr<RTCCertificate> orig = GenerateECDSA();
+
+  // To PEM.
+  RTCCertificatePem orig_pem = orig->ToPem();
+  // Clone from PEM.
+  scoped_refptr<RTCCertificate> clone = RTCCertificate::FromPem(orig_pem);
+  EXPECT_TRUE(clone);
+  // Make sure the clone's PEM is identical to the original.

[torbjorng (webrtc), 2016/04/21 15:16:00]

I'm not sure this is a robust comparison, since presumably the underlying ASN1
data fields might appear in any order. Consider adding a comment about this
potential problem to facilitate future debugging.

Or better yet, use OpenSSL's X509_cmp.

[nisse-webrtc, 2016/04/22 11:38:25]

I have been trying to forget asn.1... So I take it PEM encoding uses BER rather
than DER? Is that true of openssl in general? What fields can be permuted (being
asn.1 sets rather than sequences)? 

[hbos, 2016/04/22 13:19:29]

X509_cmp only compares the certificate? Ah, there's an EVP_PKEY_cmp too.
Added comparison functions in SSLIdentity and RTCCertificate. Only using this
type of comparison in rtccertificate_unittest.cc.

As for the comparison of PEM strings in sslidentity_unittest.cc, considering the
same library call is used to produce both the original and clone's PEM I think
it's a pretty good bet that the order will be the same, the string identical,
and that the comparison will work and continue to work. But you're right, it is
an assumption with no gurantees.

I'm considering keeping the comparison in sslidentity_unittest.cc but have a
comment explaining this in case the assumption stops holding. What do you think?
(Of course I could remove the check now that I have a new comparison function,
but keeping it still boosts my confidence that everything is working as
intended.)

+  RTCCertificatePem clone_pem = clone->ToPem();
+  EXPECT_EQ(orig_pem.private_key(), clone_pem.private_key());
+  EXPECT_EQ(orig_pem.certificate(), clone_pem.certificate());
+  // Make sure the clone's expiration time is the same as the original.
+  EXPECT_EQ(orig->Expires(), clone->Expires());
+}
+
 }  // namespace rtc