RTCCertificate serialization.

webrtc/base/rtccertificate_unittest.cc

 /*
  *  Copyright 2015 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 17 matching lines @@
 static const char* kTestCertCommonName = "RTCCertificateTest's certificate";
 
 }  // namespace
 
 class RTCCertificateTest : public testing::Test {
  public:
   RTCCertificateTest() {}
   ~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,
   // but only seconds-precision is supported by SSLCertificate. To make the
   // tests clearer we convert everything to seconds since the precision matters
   // when generating certificates or comparing timestamps.
   //   As a result, ExpiresSeconds and HasExpiredSeconds are used instead of
   // RTCCertificate::Expires and ::HasExpired for ms -> s conversion.
 
   uint64_t NowSeconds() const {
@@ skipping 30 matching lines @@
     params.key_params = KeyParams::ECDSA();
 
     scoped_ptr<SSLIdentity> identity(SSLIdentity::GenerateForTest(params));
     return RTCCertificate::Create(std::move(identity));
   }
 };
 
 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));
   // Even without specifying the expiration time we would expect it to be valid
   // for at least half an hour.
   EXPECT_FALSE(HasExpiredSeconds(certificate, now + 30*60));
 }
 
 TEST_F(RTCCertificateTest, UsesExpiresAskedFor) {
   uint64_t now = NowSeconds();
   scoped_refptr<RTCCertificate> certificate =
       GenerateCertificateWithExpires(now);
   EXPECT_EQ(now, ExpiresSeconds(certificate));
 }
 
 TEST_F(RTCCertificateTest, ExpiresInOneSecond) {
   // Generate a certificate that expires in 1s.
   uint64_t now = NowSeconds();
   scoped_refptr<RTCCertificate> certificate =
       GenerateCertificateWithExpires(now + 1);
   // Now it should not have expired.
   EXPECT_FALSE(HasExpiredSeconds(certificate, now));
   // In 2s it should have expired.
   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