RTCPeerConnection.generateCertificate: Optionally specify expiration.

content/renderer/media/peer_connection_identity_store.cc

Index: content/renderer/media/peer_connection_identity_store.cc
diff --git a/content/renderer/media/peer_connection_identity_store.cc b/content/renderer/media/peer_connection_identity_store.cc
index c6f432dc6c465c0553ca3fd97c04beab9657e4fa..90f3a364ca5d2ed812a50d8e9dcdbcd4830f8ce7 100644
--- a/content/renderer/media/peer_connection_identity_store.cc
+++ b/content/renderer/media/peer_connection_identity_store.cc
@@ -17,6 +17,7 @@ namespace {
 const char kIdentityName[] = "WebRTC";
 static unsigned int kRSAChromiumKeyLength = 1024;
 static unsigned int kRSAChromiumPubExp = 0x10001;
+static uint64_t kYearInSeconds = 365 * 24 * 60 * 60;
 
 // Bridges identity requests between the main render thread and libjingle's
 // signaling thread.
@@ -115,7 +116,8 @@ PeerConnectionIdentityStore::~PeerConnectionIdentityStore() {
 }
 
 void PeerConnectionIdentityStore::RequestIdentity(
-    rtc::KeyParams key_params,
+    const rtc::KeyParams& key_params,
+    const rtc::Optional<uint64_t>& expires_ms,
     const rtc::scoped_refptr<webrtc::DtlsIdentityRequestObserver>& observer) {
   DCHECK(signaling_thread_->BelongsToCurrentThread());
   DCHECK(observer);
@@ -127,7 +129,8 @@ void PeerConnectionIdentityStore::RequestIdentity(
   // header file(s).
   if (key_params.type() == rtc::KT_RSA &&
       key_params.rsa_params().mod_size == kRSAChromiumKeyLength &&
-      key_params.rsa_params().pub_exp == kRSAChromiumPubExp) {
+      key_params.rsa_params().pub_exp == kRSAChromiumPubExp &&
+      !expires_ms) {
     // Use Chromium identity generation code for its hardwired parameters (RSA,
     // 1024, 0x10001). This generation code is preferred over WebRTC generation
     // code due to the performance benefits of caching.
@@ -140,8 +143,18 @@ void PeerConnectionIdentityStore::RequestIdentity(
   } else {
     // Fall back on WebRTC identity generation code for everything else, e.g.
     // RSA with any other parameters or ECDSA. These will not be cached.
-    scoped_ptr<rtc::SSLIdentity> identity(rtc::SSLIdentity::Generate(
-        kIdentityName, key_params));
+    scoped_ptr<rtc::SSLIdentity> identity;
+    if (!expires_ms) {
+      identity.reset(rtc::SSLIdentity::Generate(kIdentityName, key_params));
+    } else {
+      uint64_t expires_s = *expires_ms / 1000;
+      // Limit the expiration time to something reasonable (a year). This also
+      // ensures that the value is not too large for time_t.
+      if (expires_s > kYearInSeconds)
+        expires_s = kYearInSeconds;
+      identity.reset(rtc::SSLIdentity::Generate(
+          kIdentityName, key_params, static_cast<time_t>(expires_s)));

[Ryan Sleevi, 2016/03/08 17:06:10]

BUG: You cannot do this sort of cast. There's no guarantee that time_t is
expressed like this.

Explicitly convert (e.g. using base::Time)

[hbos_chromium, 2016/04/13 17:03:08]

From JavaScript we get a uint64_t timestamp. We want to use this all the way
down but lower layer stuff uses time_t, we have no choice but to convert in at
least one of the layers, preferrably we only convert once, thus uint64_t ->
time_t. Introducing more conversion steps, say by using base::Time, would only
increase the risk of errors and not add anything since the parameter is just
passed around.

You correctly pointed out that in a different CL
(https://codereview.webrtc.org/1683193003/) that OpenSSL has other API which
means we ultimately don't have to use time_t. TODOs and a bug
(https://bugs.chromium.org/p/webrtc/issues/detail?id=5720) has been created to
update SSLIdentity::Generate not to use time_t, in the meantime it is safe to
assume that time_t can hold up to a year's worth of seconds (and the
if-statement before the static_cast ensures that |expres_s| is not larger than
this).

I agree that we should stop using time_t and I have added a TODO with the bug in
the comment in this CL, but I don't think it should block this CL.

[hbos_chromium, 2016/04/13 17:09:39]

PS I have a CL in the works that will use uint64_t for its API to be called from
here instead of SSLIdentity::Generate, so that the |expires_ms| parameter can be
passed around all the way down to the webrtc repo code. (Though until
webrtc:5720 is fixed a static_cast is inevitable, but it then occurs at a lower
layer and in the webrtc repo.)

[hbos_chromium, 2016/04/18 10:04:27]

Hang on, I'll patch to use this now before asking to PTAL again.

[hbos_chromium, 2016/04/18 13:23:59]

Nevermind, using the new API should be done separately, starting to use it
before future cleanup work is done is like jumping through an extra hoop since
another roll would be necessary. Please review as-is.

+    }
 
     // Invoke |observer| callbacks asynchronously. The callbacks of
     // DtlsIdentityStoreInterface implementations have to be async.