Implement certificate lifetime parameter as required by WebRTC RFC.

webrtc/base/sslidentity.h

 /*
  *  Copyright 2004 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 18 matching lines @@
 // Abstract interface overridden by SSL library specific
 // implementations.
 
 // A somewhat opaque type used to encapsulate a certificate.
 // Wraps the SSL library's notion of a certificate, with reference counting.
 // The SSLCertificate object is pretty much immutable once created.
 // (The OpenSSL implementation only does reference counting and
 // possibly caching of intermediate results.)
 class SSLCertificate {
  public:
-  // Parses and build a certificate from a PEM encoded string.
+  // Parses and builds a certificate from a PEM encoded string.
   // Returns NULL on failure.
   // The length of the string representation of the certificate is
   // stored in *pem_length if it is non-NULL, and only if
   // parsing was successful.
   // Caller is responsible for freeing the returned object.
   static SSLCertificate* FromPEMString(const std::string& pem_string);
   virtual ~SSLCertificate() {}
 
   // Returns a new SSLCertificate object instance wrapping the same
   // underlying certificate, including its chain if present.
@@ skipping 68 matching lines @@
 // TODO(hbos,torbjorng): Don't change KT_DEFAULT without first updating
 // PeerConnectionFactory_nativeCreatePeerConnection's certificate generation
 // code.
 enum KeyType { KT_RSA, KT_ECDSA, KT_LAST, KT_DEFAULT = KT_RSA };
 
 static const int kRsaDefaultModSize = 1024;
 static const int kRsaDefaultExponent = 0x10001;  // = 2^16+1 = 65537
 static const int kRsaMinModSize = 1024;
 static const int kRsaMaxModSize = 8192;
 
+// Certificate default validity lifetime.
+static const int kDefaultCertificateLifetime = 60 * 60 * 24 * 30;  // 30 days
+// Certificate validity window.
+// This is to compensate for slightly incorrect system clocks.
+static const int kCertificateWindow = -60 * 60 * 24;

[Ryan Sleevi, 2016/03/08 17:04:43]

Explain the units this is in (seconds?)
Explain why it's negative in the constant (unclear)
Explain how the value was derived (empirically or just gut? Because our user
metrics indicate this is a bad assumption that they're only off a day)

[torbjorng (webrtc), 2016/03/30 14:00:29]

Like all things in x509, this is seconds.

I agree the comments could be improved.

I cannot tell you how the value of one day was derived; this code was just moved
in this CL and not semantically changed. My guess is that localtime/UTC
confusion on some systems and the fact that times on the planet are up to 24 h
from UTC might be the root of this value.

[Ryan Sleevi, 2016/03/31 02:07:53]

I'm not sure what you meant, but X.509 is based on fractional microseconds.
It's... hairy.

However, including the units in constants like this substantially improves
readability. 
That seems like it would be a bug in the WebRTC code, then, perhaps setting the
certificate date (which is always in UTC/Z with respect to RFC 5280
certificates) based on local time, rather than the adjustment.

[torbjorng (webrtc), 2016/03/31 13:18:34]

Really? The ANS1_TIME type used therein explicitly forbids fractional seconds.
Fixing this is a separate CL.
I am not aware if that this would happen in WebRTC except if people have
incorrectly set system clocks. We never mess with localtime anywhere near any
certificate code...

+
 struct RSAParams {
   unsigned int mod_size;
   unsigned int pub_exp;
 };
 
 enum ECCurve { EC_NIST_P256, /* EC_FANCY, */ EC_LAST };
 
 class KeyParams {
  public:
   // Generate a KeyParams object from a simple KeyType, using default params.
@@ skipping 39 matching lines @@
   time_t not_after;   // Absolute time since epoch in seconds.
   KeyParams key_params;
 };
 
 // Our identity in an SSL negotiation: a keypair and certificate (both
 // with the same public key).
 // This too is pretty much immutable once created.
 class SSLIdentity {
  public:
   // Generates an identity (keypair and self-signed certificate). If
-  // common_name is non-empty, it will be used for the certificate's
-  // subject and issuer name, otherwise a random string will be used.
+  // |common_name| is non-empty, it will be used for the certificate's subject
+  // and issuer name, otherwise a random string will be used. The key type and
+  // parameters are defined in |key_param|. The certificate's lifetime in
+  // seconds from the current time is defined in |certificate_lifetime|; it
+  // should be a non-negative number.
   // Returns NULL on failure.
   // Caller is responsible for freeing the returned object.
   static SSLIdentity* Generate(const std::string& common_name,

[Ryan Sleevi, 2016/03/08 17:04:43]

Per Google style guide on overloading, this would be better called
GenerateWithExpiration

[torbjorng (webrtc), 2016/03/30 14:00:29]

I'm fixing this in a follow-up CL.

-                               const KeyParams& key_param);
+                               const KeyParams& key_param,
+                               time_t certificate_lifetime);
+  static SSLIdentity* Generate(const std::string& common_name,
+                               const KeyParams& key_param) {
+    return Generate(common_name, key_param, kDefaultCertificateLifetime);
+  }
   static SSLIdentity* Generate(const std::string& common_name,
                                KeyType key_type) {
     return Generate(common_name, KeyParams(key_type));
   }
 
   // Generates an identity with the specified validity period.
+  // TODO(torbjorng): Now that Generate() accepts relevant params, make tests
+  // use that instead of this function.
   static SSLIdentity* GenerateForTest(const SSLIdentityParams& params);
 
   // Construct an identity from a private key and a certificate.
   static SSLIdentity* FromPEMStrings(const std::string& private_key,
                                      const std::string& certificate);
 
   virtual ~SSLIdentity() {}
 
   // Returns a new SSLIdentity object instance wrapping the same
   // identity information.
@@ skipping 18 matching lines @@
 // |s| is not 0-terminated; its char count is defined by |length|.
 int64_t ASN1TimeToSec(const unsigned char* s, size_t length, bool long_format);
 
 extern const char kPemTypeCertificate[];
 extern const char kPemTypeRsaPrivateKey[];
 extern const char kPemTypeEcPrivateKey[];
 
 }  // namespace rtc
 
 #endif  // WEBRTC_BASE_SSLIDENTITY_H_