diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc index d7db04154f..2853ca43a7 100644 --- a/talk/app/webrtc/webrtcsession_unittest.cc +++ b/talk/app/webrtc/webrtcsession_unittest.cc @@ -596,9 +596,8 @@ class WebRtcSessionTest 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::Generate(identity_name, rtc::KT_DEFAULT)) - .Pass())); + rtc::scoped_ptr(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 feda6744f0..de4e6a771e 100644 --- a/webrtc/base/opensslidentity.cc +++ b/webrtc/base/opensslidentity.cc @@ -33,6 +33,9 @@ namespace rtc { // 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; @@ -43,16 +46,15 @@ static const int CERTIFICATE_LIFETIME = 60*60*24*30; // 30 days, arbitrarily static const int CERTIFICATE_WINDOW = -60*60*24; // Generate a key pair. Caller is responsible for freeing the returned object. -static EVP_PKEY* MakeKey(const KeyParams& key_params) { +static EVP_PKEY* MakeKey(KeyType key_type) { LOG(LS_INFO) << "Making key pair"; EVP_PKEY* pkey = EVP_PKEY_new(); - if (key_params.type() == KT_RSA) { - int key_length = key_params.rsa_params().mod_size; + if (key_type == KT_RSA) { BIGNUM* exponent = BN_new(); RSA* rsa = RSA_new(); if (!pkey || !exponent || !rsa || - !BN_set_word(exponent, key_params.rsa_params().pub_exp) || - !RSA_generate_key_ex(rsa, key_length, exponent, NULL) || + !BN_set_word(exponent, 0x10001) || // 65537 RSA exponent + !RSA_generate_key_ex(rsa, KEY_LENGTH, exponent, NULL) || !EVP_PKEY_assign_RSA(pkey, rsa)) { EVP_PKEY_free(pkey); BN_free(exponent); @@ -62,23 +64,16 @@ static EVP_PKEY* MakeKey(const KeyParams& key_params) { } // ownership of rsa struct was assigned, don't free it. BN_free(exponent); - } 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. + } 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)) { EVP_PKEY_free(pkey); - LOG(LS_ERROR) << "ECDSA key requested for unknown curve"; + 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 { EVP_PKEY_free(pkey); LOG(LS_ERROR) << "Key type requested not understood"; @@ -160,8 +155,8 @@ static void LogSSLErrors(const std::string& prefix) { } } -OpenSSLKeyPair* OpenSSLKeyPair::Generate(const KeyParams& key_params) { - EVP_PKEY* pkey = MakeKey(key_params); +OpenSSLKeyPair* OpenSSLKeyPair::Generate(KeyType key_type) { + EVP_PKEY* pkey = MakeKey(key_type); if (!pkey) { LogSSLErrors("Generating key pair"); return NULL; @@ -384,7 +379,7 @@ OpenSSLIdentity::~OpenSSLIdentity() = default; OpenSSLIdentity* OpenSSLIdentity::GenerateInternal( const SSLIdentityParams& params) { - OpenSSLKeyPair* key_pair = OpenSSLKeyPair::Generate(params.key_params); + OpenSSLKeyPair* key_pair = OpenSSLKeyPair::Generate(params.key_type); if (key_pair) { OpenSSLCertificate* certificate = OpenSSLCertificate::Generate(key_pair, params); @@ -397,12 +392,12 @@ OpenSSLIdentity* OpenSSLIdentity::GenerateInternal( } OpenSSLIdentity* OpenSSLIdentity::Generate(const std::string& common_name, - const KeyParams& key_params) { + KeyType key_type) { 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 f957ef2288..d8ba138f62 100644 --- a/webrtc/base/opensslidentity.h +++ b/webrtc/base/opensslidentity.h @@ -32,7 +32,7 @@ class OpenSSLKeyPair { ASSERT(pkey_ != NULL); } - static OpenSSLKeyPair* Generate(const KeyParams& key_params); + static OpenSSLKeyPair* Generate(KeyType key_type); virtual ~OpenSSLKeyPair(); @@ -100,7 +100,7 @@ class OpenSSLCertificate : public SSLCertificate { class OpenSSLIdentity : public SSLIdentity { public: static OpenSSLIdentity* Generate(const std::string& common_name, - const KeyParams& key_params); + KeyType key_type); 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 7869b6eb63..9b6945132a 100644 --- a/webrtc/base/ssladapter_unittest.cc +++ b/webrtc/base/ssladapter_unittest.cc @@ -131,10 +131,10 @@ class SSLAdapterTestDummyClient : public sigslot::has_slots<> { class SSLAdapterTestDummyServer : public sigslot::has_slots<> { public: explicit SSLAdapterTestDummyServer(const rtc::SSLMode& ssl_mode, - const rtc::KeyParams& key_params) + const rtc::KeyType key_type) : ssl_mode_(ssl_mode) { // Generate a key pair and a certificate for this host. - ssl_identity_.reset(rtc::SSLIdentity::Generate(GetHostname(), key_params)); + ssl_identity_.reset(rtc::SSLIdentity::Generate(GetHostname(), key_type)); server_socket_.reset(CreateSocket(ssl_mode_)); @@ -271,10 +271,10 @@ class SSLAdapterTestBase : public testing::Test, public sigslot::has_slots<> { public: explicit SSLAdapterTestBase(const rtc::SSLMode& ssl_mode, - const rtc::KeyParams& key_params) + const rtc::KeyType key_type) : ssl_mode_(ssl_mode), ss_scope_(new rtc::VirtualSocketServer(NULL)), - server_(new SSLAdapterTestDummyServer(ssl_mode_, key_params)), + server_(new SSLAdapterTestDummyServer(ssl_mode_, key_type)), client_(new SSLAdapterTestDummyClient(ssl_mode_)), handshake_wait_(kTimeout) {} @@ -348,25 +348,25 @@ class SSLAdapterTestBase : public testing::Test, class SSLAdapterTestTLS_RSA : public SSLAdapterTestBase { public: SSLAdapterTestTLS_RSA() - : SSLAdapterTestBase(rtc::SSL_MODE_TLS, rtc::KeyParams::RSA()) {} + : SSLAdapterTestBase(rtc::SSL_MODE_TLS, rtc::KT_RSA) {} }; class SSLAdapterTestTLS_ECDSA : public SSLAdapterTestBase { public: SSLAdapterTestTLS_ECDSA() - : SSLAdapterTestBase(rtc::SSL_MODE_TLS, rtc::KeyParams::ECDSA()) {} + : SSLAdapterTestBase(rtc::SSL_MODE_TLS, rtc::KT_ECDSA) {} }; class SSLAdapterTestDTLS_RSA : public SSLAdapterTestBase { public: SSLAdapterTestDTLS_RSA() - : SSLAdapterTestBase(rtc::SSL_MODE_DTLS, rtc::KeyParams::RSA()) {} + : SSLAdapterTestBase(rtc::SSL_MODE_DTLS, rtc::KT_RSA) {} }; class SSLAdapterTestDTLS_ECDSA : public SSLAdapterTestBase { public: SSLAdapterTestDTLS_ECDSA() - : SSLAdapterTestBase(rtc::SSL_MODE_DTLS, rtc::KeyParams::ECDSA()) {} + : SSLAdapterTestBase(rtc::SSL_MODE_DTLS, rtc::KT_ECDSA) {} }; #if SSL_USE_OPENSSL diff --git a/webrtc/base/sslidentity.cc b/webrtc/base/sslidentity.cc index b3336ea9ee..ce209dd416 100644 --- a/webrtc/base/sslidentity.cc +++ b/webrtc/base/sslidentity.cc @@ -108,8 +108,8 @@ SSLCertificate* SSLCertificate::FromPEMString(const std::string& pem_string) { } SSLIdentity* SSLIdentity::Generate(const std::string& common_name, - const KeyParams& key_params) { - return OpenSSLIdentity::Generate(common_name, key_params); + KeyType key_type) { + return OpenSSLIdentity::Generate(common_name, key_type); } SSLIdentity* SSLIdentity::GenerateForTest(const SSLIdentityParams& params) { diff --git a/webrtc/base/sslidentity.h b/webrtc/base/sslidentity.h index 99cbac8c3d..3a1bbd0856 100644 --- a/webrtc/base/sslidentity.h +++ b/webrtc/base/sslidentity.h @@ -18,7 +18,6 @@ #include #include "webrtc/base/buffer.h" -#include "webrtc/base/checks.h" #include "webrtc/base/messagedigest.h" namespace rtc { @@ -108,105 +107,25 @@ class SSLCertChain { 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 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. +// 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. struct SSLIdentityParams { std::string common_name; - int not_before; // offset from current time in seconds. - int not_after; // offset from current time in seconds. - KeyParams key_params; + int not_before; // in seconds. + int not_after; // in seconds. + KeyType key_type; }; // Our identity in an SSL negotiation: a keypair and certificate (both @@ -220,11 +139,7 @@ class SSLIdentity { // Returns NULL on failure. // Caller is responsible for freeing the returned object. static SSLIdentity* Generate(const std::string& common_name, - const KeyParams& key_param); - static SSLIdentity* Generate(const std::string& common_name, - KeyType key_type) { - return Generate(common_name, KeyParams(key_type)); - } + KeyType 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 a3e8d9c637..c65bb63ec0 100644 --- a/webrtc/base/sslstreamadapter_unittest.cc +++ b/webrtc/base/sslstreamadapter_unittest.cc @@ -161,12 +161,11 @@ static const int kFifoBufferSize = 4096; 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::KeyParams client_key_type = rtc::KeyParams(rtc::KT_DEFAULT), - rtc::KeyParams server_key_type = rtc::KeyParams(rtc::KT_DEFAULT)) + 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) : client_buffer_(kFifoBufferSize), server_buffer_(kFifoBufferSize), client_stream_( @@ -225,17 +224,17 @@ class SSLStreamAdapterTestBase : public testing::Test, 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_); @@ -463,7 +462,7 @@ class SSLStreamAdapterTestBase : public testing::Test, class SSLStreamAdapterTestTLS : public SSLStreamAdapterTestBase, - public WithParamInterface> { + public WithParamInterface> { public: SSLStreamAdapterTestTLS() : SSLStreamAdapterTestBase("", @@ -571,7 +570,7 @@ class SSLStreamAdapterTestTLS class SSLStreamAdapterTestDTLS : public SSLStreamAdapterTestBase, - public WithParamInterface> { + public WithParamInterface> { public: SSLStreamAdapterTestDTLS() : SSLStreamAdapterTestBase("", @@ -979,10 +978,9 @@ TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuite) { 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()).type()), - server_cipher); + ASSERT_EQ(rtc::SSLStreamAdapter::GetDefaultSslCipherForTest( + rtc::SSL_PROTOCOL_DTLS_10, ::testing::get<1>(GetParam())), + server_cipher); } // Test getting the used DTLS 1.2 ciphers. @@ -998,10 +996,9 @@ TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuiteDtls12Both) { 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()).type()), - server_cipher); + ASSERT_EQ(rtc::SSLStreamAdapter::GetDefaultSslCipherForTest( + rtc::SSL_PROTOCOL_DTLS_12, ::testing::get<1>(GetParam())), + server_cipher); } // DTLS 1.2 enabled for client only -> DTLS 1.0 will be used. @@ -1016,10 +1013,9 @@ TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuiteDtls12Client) { 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()).type()), - server_cipher); + ASSERT_EQ(rtc::SSLStreamAdapter::GetDefaultSslCipherForTest( + rtc::SSL_PROTOCOL_DTLS_10, ::testing::get<1>(GetParam())), + server_cipher); } // DTLS 1.2 enabled for server only -> DTLS 1.0 will be used. @@ -1034,30 +1030,16 @@ TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuiteDtls12Server) { 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()).type()), - server_cipher); + ASSERT_EQ(rtc::SSLStreamAdapter::GetDefaultSslCipherForTest( + rtc::SSL_PROTOCOL_DTLS_10, ::testing::get<1>(GetParam())), + server_cipher); } -// 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)))); +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)));