provide RSA2048 as per RFC
BUG=webrtc:4972
Review URL: https://codereview.webrtc.org/1329493005
Cr-Commit-Position: refs/heads/master@{#10209}
diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc
index 2853ca4..d7db041 100644
--- a/talk/app/webrtc/webrtcsession_unittest.cc
+++ b/talk/app/webrtc/webrtcsession_unittest.cc
@@ -596,8 +596,9 @@
rtc::ToString(rtc::CreateRandomId());
// Confirmed to work with KT_RSA and KT_ECDSA.
tdesc_factory_->set_certificate(rtc::RTCCertificate::Create(
- rtc::scoped_ptr<rtc::SSLIdentity>(rtc::SSLIdentity::Generate(
- identity_name, rtc::KT_DEFAULT)).Pass()));
+ rtc::scoped_ptr<rtc::SSLIdentity>(
+ rtc::SSLIdentity::Generate(identity_name, rtc::KT_DEFAULT))
+ .Pass()));
tdesc_factory_->set_secure(cricket::SEC_REQUIRED);
}
diff --git a/webrtc/base/opensslidentity.cc b/webrtc/base/opensslidentity.cc
index de4e6a7..feda674 100644
--- a/webrtc/base/opensslidentity.cc
+++ b/webrtc/base/opensslidentity.cc
@@ -33,9 +33,6 @@
// We could have exposed a myriad of parameters for the crypto stuff,
// but keeping it simple seems best.
-// Strength of generated keys. Those are RSA.
-static const int KEY_LENGTH = 1024;
-
// Random bits for certificate serial number
static const int SERIAL_RAND_BITS = 64;
@@ -46,15 +43,16 @@
static const int CERTIFICATE_WINDOW = -60*60*24;
// Generate a key pair. Caller is responsible for freeing the returned object.
-static EVP_PKEY* MakeKey(KeyType key_type) {
+static EVP_PKEY* MakeKey(const KeyParams& key_params) {
LOG(LS_INFO) << "Making key pair";
EVP_PKEY* pkey = EVP_PKEY_new();
- if (key_type == KT_RSA) {
+ if (key_params.type() == KT_RSA) {
+ int key_length = key_params.rsa_params().mod_size;
BIGNUM* exponent = BN_new();
RSA* rsa = RSA_new();
if (!pkey || !exponent || !rsa ||
- !BN_set_word(exponent, 0x10001) || // 65537 RSA exponent
- !RSA_generate_key_ex(rsa, KEY_LENGTH, exponent, NULL) ||
+ !BN_set_word(exponent, key_params.rsa_params().pub_exp) ||
+ !RSA_generate_key_ex(rsa, key_length, exponent, NULL) ||
!EVP_PKEY_assign_RSA(pkey, rsa)) {
EVP_PKEY_free(pkey);
BN_free(exponent);
@@ -64,16 +62,23 @@
}
// ownership of rsa struct was assigned, don't free it.
BN_free(exponent);
- } else if (key_type == KT_ECDSA) {
- EC_KEY* ec_key = EC_KEY_new_by_curve_name(NID_X9_62_prime256v1);
- if (!pkey || !ec_key || !EC_KEY_generate_key(ec_key) ||
- !EVP_PKEY_assign_EC_KEY(pkey, ec_key)) {
+ } else if (key_params.type() == KT_ECDSA) {
+ if (key_params.ec_curve() == EC_NIST_P256) {
+ EC_KEY* ec_key = EC_KEY_new_by_curve_name(NID_X9_62_prime256v1);
+ if (!pkey || !ec_key || !EC_KEY_generate_key(ec_key) ||
+ !EVP_PKEY_assign_EC_KEY(pkey, ec_key)) {
+ EVP_PKEY_free(pkey);
+ EC_KEY_free(ec_key);
+ LOG(LS_ERROR) << "Failed to make EC key pair";
+ return NULL;
+ }
+ // ownership of ec_key struct was assigned, don't free it.
+ } else {
+ // Add generation of any other curves here.
EVP_PKEY_free(pkey);
- EC_KEY_free(ec_key);
- LOG(LS_ERROR) << "Failed to make EC key pair";
+ LOG(LS_ERROR) << "ECDSA key requested for unknown curve";
return NULL;
}
- // ownership of ec_key struct was assigned, don't free it.
} else {
EVP_PKEY_free(pkey);
LOG(LS_ERROR) << "Key type requested not understood";
@@ -155,8 +160,8 @@
}
}
-OpenSSLKeyPair* OpenSSLKeyPair::Generate(KeyType key_type) {
- EVP_PKEY* pkey = MakeKey(key_type);
+OpenSSLKeyPair* OpenSSLKeyPair::Generate(const KeyParams& key_params) {
+ EVP_PKEY* pkey = MakeKey(key_params);
if (!pkey) {
LogSSLErrors("Generating key pair");
return NULL;
@@ -379,7 +384,7 @@
OpenSSLIdentity* OpenSSLIdentity::GenerateInternal(
const SSLIdentityParams& params) {
- OpenSSLKeyPair* key_pair = OpenSSLKeyPair::Generate(params.key_type);
+ OpenSSLKeyPair* key_pair = OpenSSLKeyPair::Generate(params.key_params);
if (key_pair) {
OpenSSLCertificate* certificate =
OpenSSLCertificate::Generate(key_pair, params);
@@ -392,12 +397,12 @@
}
OpenSSLIdentity* OpenSSLIdentity::Generate(const std::string& common_name,
- KeyType key_type) {
+ const KeyParams& key_params) {
SSLIdentityParams params;
+ params.key_params = key_params;
params.common_name = common_name;
params.not_before = CERTIFICATE_WINDOW;
params.not_after = CERTIFICATE_LIFETIME;
- params.key_type = key_type;
return GenerateInternal(params);
}
diff --git a/webrtc/base/opensslidentity.h b/webrtc/base/opensslidentity.h
index d8ba138..f957ef2 100644
--- a/webrtc/base/opensslidentity.h
+++ b/webrtc/base/opensslidentity.h
@@ -32,7 +32,7 @@
ASSERT(pkey_ != NULL);
}
- static OpenSSLKeyPair* Generate(KeyType key_type);
+ static OpenSSLKeyPair* Generate(const KeyParams& key_params);
virtual ~OpenSSLKeyPair();
@@ -100,7 +100,7 @@
class OpenSSLIdentity : public SSLIdentity {
public:
static OpenSSLIdentity* Generate(const std::string& common_name,
- KeyType key_type);
+ const KeyParams& key_params);
static OpenSSLIdentity* GenerateForTest(const SSLIdentityParams& params);
static SSLIdentity* FromPEMStrings(const std::string& private_key,
const std::string& certificate);
diff --git a/webrtc/base/ssladapter_unittest.cc b/webrtc/base/ssladapter_unittest.cc
index 9b69451..7869b6e 100644
--- a/webrtc/base/ssladapter_unittest.cc
+++ b/webrtc/base/ssladapter_unittest.cc
@@ -131,10 +131,10 @@
class SSLAdapterTestDummyServer : public sigslot::has_slots<> {
public:
explicit SSLAdapterTestDummyServer(const rtc::SSLMode& ssl_mode,
- const rtc::KeyType key_type)
+ const rtc::KeyParams& key_params)
: ssl_mode_(ssl_mode) {
// Generate a key pair and a certificate for this host.
- ssl_identity_.reset(rtc::SSLIdentity::Generate(GetHostname(), key_type));
+ ssl_identity_.reset(rtc::SSLIdentity::Generate(GetHostname(), key_params));
server_socket_.reset(CreateSocket(ssl_mode_));
@@ -271,10 +271,10 @@
public sigslot::has_slots<> {
public:
explicit SSLAdapterTestBase(const rtc::SSLMode& ssl_mode,
- const rtc::KeyType key_type)
+ const rtc::KeyParams& key_params)
: ssl_mode_(ssl_mode),
ss_scope_(new rtc::VirtualSocketServer(NULL)),
- server_(new SSLAdapterTestDummyServer(ssl_mode_, key_type)),
+ server_(new SSLAdapterTestDummyServer(ssl_mode_, key_params)),
client_(new SSLAdapterTestDummyClient(ssl_mode_)),
handshake_wait_(kTimeout) {}
@@ -348,25 +348,25 @@
class SSLAdapterTestTLS_RSA : public SSLAdapterTestBase {
public:
SSLAdapterTestTLS_RSA()
- : SSLAdapterTestBase(rtc::SSL_MODE_TLS, rtc::KT_RSA) {}
+ : SSLAdapterTestBase(rtc::SSL_MODE_TLS, rtc::KeyParams::RSA()) {}
};
class SSLAdapterTestTLS_ECDSA : public SSLAdapterTestBase {
public:
SSLAdapterTestTLS_ECDSA()
- : SSLAdapterTestBase(rtc::SSL_MODE_TLS, rtc::KT_ECDSA) {}
+ : SSLAdapterTestBase(rtc::SSL_MODE_TLS, rtc::KeyParams::ECDSA()) {}
};
class SSLAdapterTestDTLS_RSA : public SSLAdapterTestBase {
public:
SSLAdapterTestDTLS_RSA()
- : SSLAdapterTestBase(rtc::SSL_MODE_DTLS, rtc::KT_RSA) {}
+ : SSLAdapterTestBase(rtc::SSL_MODE_DTLS, rtc::KeyParams::RSA()) {}
};
class SSLAdapterTestDTLS_ECDSA : public SSLAdapterTestBase {
public:
SSLAdapterTestDTLS_ECDSA()
- : SSLAdapterTestBase(rtc::SSL_MODE_DTLS, rtc::KT_ECDSA) {}
+ : SSLAdapterTestBase(rtc::SSL_MODE_DTLS, rtc::KeyParams::ECDSA()) {}
};
#if SSL_USE_OPENSSL
diff --git a/webrtc/base/sslidentity.cc b/webrtc/base/sslidentity.cc
index ce209dd..b3336ea 100644
--- a/webrtc/base/sslidentity.cc
+++ b/webrtc/base/sslidentity.cc
@@ -108,8 +108,8 @@
}
SSLIdentity* SSLIdentity::Generate(const std::string& common_name,
- KeyType key_type) {
- return OpenSSLIdentity::Generate(common_name, key_type);
+ const KeyParams& key_params) {
+ return OpenSSLIdentity::Generate(common_name, key_params);
}
SSLIdentity* SSLIdentity::GenerateForTest(const SSLIdentityParams& params) {
diff --git a/webrtc/base/sslidentity.h b/webrtc/base/sslidentity.h
index 3a1bbd0..99cbac8 100644
--- a/webrtc/base/sslidentity.h
+++ b/webrtc/base/sslidentity.h
@@ -18,6 +18,7 @@
#include <vector>
#include "webrtc/base/buffer.h"
+#include "webrtc/base/checks.h"
#include "webrtc/base/messagedigest.h"
namespace rtc {
@@ -107,25 +108,105 @@
RTC_DISALLOW_COPY_AND_ASSIGN(SSLCertChain);
};
+// KT_DEFAULT is currently an alias for KT_RSA. This is likely to change.
+// KT_LAST is intended for vector declarations and loops over all key types;
+// it does not represent any key type in itself.
// 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;
+
+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.
+ explicit KeyParams(KeyType key_type = KT_DEFAULT) {
+ if (key_type == KT_ECDSA) {
+ type_ = KT_ECDSA;
+ params_.curve = EC_NIST_P256;
+ } else if (key_type == KT_RSA) {
+ type_ = KT_RSA;
+ params_.rsa.mod_size = kRsaDefaultModSize;
+ params_.rsa.pub_exp = kRsaDefaultExponent;
+ } else {
+ RTC_NOTREACHED();
+ }
+ }
+
+ // Generate a a KeyParams for RSA with explicit parameters.
+ static KeyParams RSA(int mod_size = kRsaDefaultModSize,
+ int pub_exp = kRsaDefaultExponent) {
+ KeyParams kt(KT_RSA);
+ kt.params_.rsa.mod_size = mod_size;
+ kt.params_.rsa.pub_exp = pub_exp;
+ return kt;
+ }
+
+ // Generate a a KeyParams for ECDSA specifying the curve.
+ static KeyParams ECDSA(ECCurve curve = EC_NIST_P256) {
+ KeyParams kt(KT_ECDSA);
+ kt.params_.curve = curve;
+ return kt;
+ }
+
+ // Check validity of a KeyParams object. Since the factory functions have
+ // no way of returning errors, this function can be called after creation
+ // to make sure the parameters are OK.
+ bool IsValid() {
+ if (type_ == KT_RSA) {
+ return (params_.rsa.mod_size >= kRsaMinModSize &&
+ params_.rsa.mod_size <= kRsaMaxModSize &&
+ params_.rsa.pub_exp > params_.rsa.mod_size);
+ } else if (type_ == KT_ECDSA) {
+ return (params_.curve == EC_NIST_P256);
+ }
+ return false;
+ }
+
+ RSAParams rsa_params() const {
+ RTC_DCHECK(type_ == KT_RSA);
+ return params_.rsa;
+ }
+
+ ECCurve ec_curve() const {
+ RTC_DCHECK(type_ == KT_ECDSA);
+ return params_.curve;
+ }
+
+ KeyType type() const { return type_; }
+
+ private:
+ KeyType type_;
+ union {
+ RSAParams rsa;
+ ECCurve curve;
+ } params_;
+};
+
// TODO(hbos): Remove once rtc::KeyType (to be modified) and
// blink::WebRTCKeyType (to be landed) match. By using this function in Chromium
// appropriately we can change KeyType enum -> class without breaking Chromium.
KeyType IntKeyTypeFamilyToKeyType(int key_type_family);
-// Parameters for generating an identity for testing. 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. |not_before| and |not_after| are
-// offsets to the current time in number of seconds.
+// Parameters for generating a 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.
struct SSLIdentityParams {
std::string common_name;
- int not_before; // in seconds.
- int not_after; // in seconds.
- KeyType key_type;
+ int not_before; // offset from current time in seconds.
+ int not_after; // offset from current time in seconds.
+ KeyParams key_params;
};
// Our identity in an SSL negotiation: a keypair and certificate (both
@@ -139,7 +220,11 @@
// Returns NULL on failure.
// Caller is responsible for freeing the returned object.
static SSLIdentity* Generate(const std::string& common_name,
- KeyType key_type);
+ const KeyParams& key_param);
+ 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.
static SSLIdentity* GenerateForTest(const SSLIdentityParams& params);
diff --git a/webrtc/base/sslstreamadapter_unittest.cc b/webrtc/base/sslstreamadapter_unittest.cc
index c65bb63..a3e8d9c 100644
--- a/webrtc/base/sslstreamadapter_unittest.cc
+++ b/webrtc/base/sslstreamadapter_unittest.cc
@@ -161,11 +161,12 @@
class SSLStreamAdapterTestBase : public testing::Test,
public sigslot::has_slots<> {
public:
- SSLStreamAdapterTestBase(const std::string& client_cert_pem,
- const std::string& client_private_key_pem,
- bool dtls,
- rtc::KeyType client_key_type = rtc::KT_DEFAULT,
- rtc::KeyType server_key_type = rtc::KT_DEFAULT)
+ SSLStreamAdapterTestBase(
+ const std::string& client_cert_pem,
+ const std::string& client_private_key_pem,
+ bool dtls,
+ rtc::KeyParams client_key_type = rtc::KeyParams(rtc::KT_DEFAULT),
+ rtc::KeyParams server_key_type = rtc::KeyParams(rtc::KT_DEFAULT))
: client_buffer_(kFifoBufferSize),
server_buffer_(kFifoBufferSize),
client_stream_(
@@ -224,17 +225,17 @@
server_ssl_->SignalEvent.connect(this, &SSLStreamAdapterTestBase::OnEvent);
rtc::SSLIdentityParams client_params;
+ client_params.key_params = rtc::KeyParams(rtc::KT_DEFAULT);
client_params.common_name = "client";
client_params.not_before = not_before;
client_params.not_after = not_after;
- client_params.key_type = rtc::KT_DEFAULT;
client_identity_ = rtc::SSLIdentity::GenerateForTest(client_params);
rtc::SSLIdentityParams server_params;
+ server_params.key_params = rtc::KeyParams(rtc::KT_DEFAULT);
server_params.common_name = "server";
server_params.not_before = not_before;
server_params.not_after = not_after;
- server_params.key_type = rtc::KT_DEFAULT;
server_identity_ = rtc::SSLIdentity::GenerateForTest(server_params);
client_ssl_->SetIdentity(client_identity_);
@@ -462,7 +463,7 @@
class SSLStreamAdapterTestTLS
: public SSLStreamAdapterTestBase,
- public WithParamInterface<tuple<rtc::KeyType, rtc::KeyType>> {
+ public WithParamInterface<tuple<rtc::KeyParams, rtc::KeyParams>> {
public:
SSLStreamAdapterTestTLS()
: SSLStreamAdapterTestBase("",
@@ -570,7 +571,7 @@
class SSLStreamAdapterTestDTLS
: public SSLStreamAdapterTestBase,
- public WithParamInterface<tuple<rtc::KeyType, rtc::KeyType>> {
+ public WithParamInterface<tuple<rtc::KeyParams, rtc::KeyParams>> {
public:
SSLStreamAdapterTestDTLS()
: SSLStreamAdapterTestBase("",
@@ -978,9 +979,10 @@
ASSERT_TRUE(GetSslCipherSuite(false, &server_cipher));
ASSERT_EQ(client_cipher, server_cipher);
- ASSERT_EQ(rtc::SSLStreamAdapter::GetDefaultSslCipherForTest(
- rtc::SSL_PROTOCOL_DTLS_10, ::testing::get<1>(GetParam())),
- server_cipher);
+ ASSERT_EQ(
+ rtc::SSLStreamAdapter::GetDefaultSslCipherForTest(
+ rtc::SSL_PROTOCOL_DTLS_10, ::testing::get<1>(GetParam()).type()),
+ server_cipher);
}
// Test getting the used DTLS 1.2 ciphers.
@@ -996,9 +998,10 @@
ASSERT_TRUE(GetSslCipherSuite(false, &server_cipher));
ASSERT_EQ(client_cipher, server_cipher);
- ASSERT_EQ(rtc::SSLStreamAdapter::GetDefaultSslCipherForTest(
- rtc::SSL_PROTOCOL_DTLS_12, ::testing::get<1>(GetParam())),
- server_cipher);
+ ASSERT_EQ(
+ rtc::SSLStreamAdapter::GetDefaultSslCipherForTest(
+ rtc::SSL_PROTOCOL_DTLS_12, ::testing::get<1>(GetParam()).type()),
+ server_cipher);
}
// DTLS 1.2 enabled for client only -> DTLS 1.0 will be used.
@@ -1013,9 +1016,10 @@
ASSERT_TRUE(GetSslCipherSuite(false, &server_cipher));
ASSERT_EQ(client_cipher, server_cipher);
- ASSERT_EQ(rtc::SSLStreamAdapter::GetDefaultSslCipherForTest(
- rtc::SSL_PROTOCOL_DTLS_10, ::testing::get<1>(GetParam())),
- server_cipher);
+ ASSERT_EQ(
+ rtc::SSLStreamAdapter::GetDefaultSslCipherForTest(
+ rtc::SSL_PROTOCOL_DTLS_10, ::testing::get<1>(GetParam()).type()),
+ server_cipher);
}
// DTLS 1.2 enabled for server only -> DTLS 1.0 will be used.
@@ -1030,16 +1034,30 @@
ASSERT_TRUE(GetSslCipherSuite(false, &server_cipher));
ASSERT_EQ(client_cipher, server_cipher);
- ASSERT_EQ(rtc::SSLStreamAdapter::GetDefaultSslCipherForTest(
- rtc::SSL_PROTOCOL_DTLS_10, ::testing::get<1>(GetParam())),
- server_cipher);
+ ASSERT_EQ(
+ rtc::SSLStreamAdapter::GetDefaultSslCipherForTest(
+ rtc::SSL_PROTOCOL_DTLS_10, ::testing::get<1>(GetParam()).type()),
+ server_cipher);
}
-INSTANTIATE_TEST_CASE_P(SSLStreamAdapterTestsTLS,
- SSLStreamAdapterTestTLS,
- Combine(Values(rtc::KT_RSA, rtc::KT_ECDSA),
- Values(rtc::KT_RSA, rtc::KT_ECDSA)));
-INSTANTIATE_TEST_CASE_P(SSLStreamAdapterTestsDTLS,
- SSLStreamAdapterTestDTLS,
- Combine(Values(rtc::KT_RSA, rtc::KT_ECDSA),
- Values(rtc::KT_RSA, rtc::KT_ECDSA)));
+// The RSA keysizes here might look strange, why not include the RFC's size
+// 2048?. The reason is test case slowness; testing two sizes to exercise
+// parametrization is sufficient.
+INSTANTIATE_TEST_CASE_P(
+ SSLStreamAdapterTestsTLS,
+ SSLStreamAdapterTestTLS,
+ Combine(Values(rtc::KeyParams::RSA(1024, 65537),
+ rtc::KeyParams::RSA(1152, 65537),
+ rtc::KeyParams::ECDSA(rtc::EC_NIST_P256)),
+ Values(rtc::KeyParams::RSA(1024, 65537),
+ rtc::KeyParams::RSA(1152, 65537),
+ rtc::KeyParams::ECDSA(rtc::EC_NIST_P256))));
+INSTANTIATE_TEST_CASE_P(
+ SSLStreamAdapterTestsDTLS,
+ SSLStreamAdapterTestDTLS,
+ Combine(Values(rtc::KeyParams::RSA(1024, 65537),
+ rtc::KeyParams::RSA(1152, 65537),
+ rtc::KeyParams::ECDSA(rtc::EC_NIST_P256)),
+ Values(rtc::KeyParams::RSA(1024, 65537),
+ rtc::KeyParams::RSA(1152, 65537),
+ rtc::KeyParams::ECDSA(rtc::EC_NIST_P256))));
