From 1d846b2acb1042048b4fae27701c2f7e65eb90e3 Mon Sep 17 00:00:00 2001 From: Torbjorn Granlund Date: Thu, 31 Mar 2016 16:21:04 +0200 Subject: [PATCH] This CL addresses late feedback on https://codereview.webrtc.org/1683193003/ BUG= R=hbos@webrtc.org, perkj@webrtc.org Review URL: https://codereview.webrtc.org/1844313002 . Cr-Commit-Position: refs/heads/master@{#12179} --- webrtc/base/opensslidentity.cc | 12 +++++++----- webrtc/base/opensslidentity.h | 6 +++--- webrtc/base/sslidentity.cc | 18 +++++++++--------- webrtc/base/sslidentity.h | 11 ++++++----- webrtc/base/sslidentity_unittest.cc | 2 +- 5 files changed, 26 insertions(+), 23 deletions(-) diff --git a/webrtc/base/opensslidentity.cc b/webrtc/base/opensslidentity.cc index 24b97b136e..9c2112e157 100644 --- a/webrtc/base/opensslidentity.cc +++ b/webrtc/base/opensslidentity.cc @@ -407,16 +407,18 @@ OpenSSLIdentity* OpenSSLIdentity::GenerateInternal( return NULL; } -OpenSSLIdentity* OpenSSLIdentity::Generate(const std::string& common_name, - const KeyParams& key_params, - time_t certificate_lifetime) { +OpenSSLIdentity* OpenSSLIdentity::GenerateWithExpiration( + const std::string& common_name, + const KeyParams& key_params, + time_t certificate_lifetime) { SSLIdentityParams params; params.key_params = key_params; params.common_name = common_name; time_t now = time(NULL); - params.not_before = now + kCertificateWindow; + params.not_before = now + kCertificateWindowInSeconds; params.not_after = now + certificate_lifetime; - RTC_DCHECK(params.not_before < params.not_after); + if (params.not_before > params.not_after) + return nullptr; return GenerateInternal(params); } diff --git a/webrtc/base/opensslidentity.h b/webrtc/base/opensslidentity.h index 8b30e6092a..df495087e3 100644 --- a/webrtc/base/opensslidentity.h +++ b/webrtc/base/opensslidentity.h @@ -101,9 +101,9 @@ class OpenSSLCertificate : public SSLCertificate { // them consistently. class OpenSSLIdentity : public SSLIdentity { public: - static OpenSSLIdentity* Generate(const std::string& common_name, - const KeyParams& key_params, - time_t certificate_lifetime); + static OpenSSLIdentity* GenerateWithExpiration(const std::string& common_name, + const KeyParams& key_params, + time_t certificate_lifetime); static OpenSSLIdentity* GenerateForTest(const SSLIdentityParams& params); static SSLIdentity* FromPEMStrings(const std::string& private_key, const std::string& certificate); diff --git a/webrtc/base/sslidentity.cc b/webrtc/base/sslidentity.cc index 14dfeb7914..5fa8bbf6b4 100644 --- a/webrtc/base/sslidentity.cc +++ b/webrtc/base/sslidentity.cc @@ -156,25 +156,25 @@ SSLCertificate* SSLCertificate::FromPEMString(const std::string& pem_string) { } // static -SSLIdentity* SSLIdentity::Generate(const std::string& common_name, - const KeyParams& key_params, - time_t certificate_lifetime) { - return OpenSSLIdentity::Generate(common_name, key_params, - certificate_lifetime); +SSLIdentity* SSLIdentity::GenerateWithExpiration(const std::string& common_name, + const KeyParams& key_params, + time_t certificate_lifetime) { + return OpenSSLIdentity::GenerateWithExpiration(common_name, key_params, + certificate_lifetime); } // static SSLIdentity* SSLIdentity::Generate(const std::string& common_name, const KeyParams& key_params) { - return OpenSSLIdentity::Generate(common_name, key_params, - kDefaultCertificateLifetime); + return OpenSSLIdentity::GenerateWithExpiration( + common_name, key_params, kDefaultCertificateLifetimeInSeconds); } // static SSLIdentity* SSLIdentity::Generate(const std::string& common_name, KeyType key_type) { - return OpenSSLIdentity::Generate(common_name, KeyParams(key_type), - kDefaultCertificateLifetime); + return OpenSSLIdentity::GenerateWithExpiration( + common_name, KeyParams(key_type), kDefaultCertificateLifetimeInSeconds); } SSLIdentity* SSLIdentity::GenerateForTest(const SSLIdentityParams& params) { diff --git a/webrtc/base/sslidentity.h b/webrtc/base/sslidentity.h index be0f3aa107..77c9e186c1 100644 --- a/webrtc/base/sslidentity.h +++ b/webrtc/base/sslidentity.h @@ -127,10 +127,11 @@ static const int kRsaMinModSize = 1024; static const int kRsaMaxModSize = 8192; // Certificate default validity lifetime. -static const int kDefaultCertificateLifetime = 60 * 60 * 24 * 30; // 30 days +static const int kDefaultCertificateLifetimeInSeconds = + 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; +static const int kCertificateWindowInSeconds = -60 * 60 * 24; struct RSAParams { unsigned int mod_size; @@ -198,9 +199,9 @@ class SSLIdentity { // 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, - const KeyParams& key_param, - time_t certificate_lifetime); + static SSLIdentity* GenerateWithExpiration(const std::string& common_name, + const KeyParams& key_param, + time_t certificate_lifetime); static SSLIdentity* Generate(const std::string& common_name, const KeyParams& key_param); static SSLIdentity* Generate(const std::string& common_name, diff --git a/webrtc/base/sslidentity_unittest.cc b/webrtc/base/sslidentity_unittest.cc index e9e9f98711..f110f76861 100644 --- a/webrtc/base/sslidentity_unittest.cc +++ b/webrtc/base/sslidentity_unittest.cc @@ -397,7 +397,7 @@ class SSLIdentityExpirationTest : public testing::Test { rtc::CreateRandomId() % (0x80000000 - time_before_generation); rtc::KeyParams key_params = rtc::KeyParams::ECDSA(rtc::EC_NIST_P256); SSLIdentity* identity = - rtc::SSLIdentity::Generate("", key_params, lifetime); + rtc::SSLIdentity::GenerateWithExpiration("", key_params, lifetime); time_t time_after_generation = time(nullptr); EXPECT_LE(time_before_generation + lifetime, identity->certificate().CertificateExpirationTime());