From 25359e0cc28f964354a5bac4ad63eb5b22576aa8 Mon Sep 17 00:00:00 2001 From: hbos Date: Wed, 2 Mar 2016 07:55:53 -0800 Subject: [PATCH] DtlsIdentityStoreInterface::RequestIdentity gets optional expires param. This is a preparation CL. The expires param will be used in a follow-up CL. Initially it will only be used by the chromium implementation. Then we will either update the webrtc implementation (DtlsIdentityStoreImpl) to use it or we will remove that store completely as part of clean-up work. There are currently two versions of RequestIdentity, one that takes KeyType and one that takes KeyParams. The KeyType version is removed in favor of the new KeyParams + expires version. The KeyParams version without expires is kept as to not break chromium which currently implements that. This is the version that can be removed in a follow-up CL. BUG=webrtc:5092, chromium:544902 Review URL: https://codereview.webrtc.org/1749193002 Cr-Commit-Position: refs/heads/master@{#11846} --- webrtc/api/dtlsidentitystore.cc | 9 +++-- webrtc/api/dtlsidentitystore.h | 33 ++++++++++--------- webrtc/api/dtlsidentitystore_unittest.cc | 24 ++++++++++---- webrtc/api/peerconnectionfactory.cc | 5 +-- webrtc/api/test/fakedtlsidentitystore.h | 8 +++-- webrtc/api/webrtcsessiondescriptionfactory.cc | 8 +++-- 6 files changed, 56 insertions(+), 31 deletions(-) diff --git a/webrtc/api/dtlsidentitystore.cc b/webrtc/api/dtlsidentitystore.cc index 2f850305b3..31c9bc0543 100644 --- a/webrtc/api/dtlsidentitystore.cc +++ b/webrtc/api/dtlsidentitystore.cc @@ -122,12 +122,15 @@ DtlsIdentityStoreImpl::~DtlsIdentityStoreImpl() { } void DtlsIdentityStoreImpl::RequestIdentity( - rtc::KeyType key_type, - const rtc::scoped_refptr& observer) { + rtc::KeyParams key_params, + rtc::Optional expires, + const rtc::scoped_refptr& observer) { RTC_DCHECK(signaling_thread_->IsCurrent()); RTC_DCHECK(observer); - GenerateIdentity(key_type, observer); + // Dropping parameterization and |expires|. + // TODO(hbos,torbjorng): Use parameterizaton/expiration. webrtc:5092. + GenerateIdentity(key_params.type(), observer); } void DtlsIdentityStoreImpl::OnMessage(rtc::Message* msg) { diff --git a/webrtc/api/dtlsidentitystore.h b/webrtc/api/dtlsidentitystore.h index 1e78b9c3c3..8666b3cf3d 100644 --- a/webrtc/api/dtlsidentitystore.h +++ b/webrtc/api/dtlsidentitystore.h @@ -17,6 +17,7 @@ #include "webrtc/base/messagehandler.h" #include "webrtc/base/messagequeue.h" +#include "webrtc/base/optional.h" #include "webrtc/base/refcount.h" #include "webrtc/base/scoped_ptr.h" #include "webrtc/base/scoped_ref_ptr.h" @@ -56,23 +57,24 @@ class DtlsIdentityStoreInterface { // The |observer| will be called when the requested identity is ready, or when // identity generation fails. - // TODO(torbjorng,hbos): The following RequestIdentity is about to be removed, - // see below todo. - virtual void RequestIdentity( - rtc::KeyType key_type, - const rtc::scoped_refptr& observer) { - // Add default parameterization. - RequestIdentity(rtc::KeyParams(key_type), observer); - } - // TODO(torbjorng,hbos): Parameterized key types! The following - // RequestIdentity should replace the old one that takes rtc::KeyType. When - // the new one is implemented by Chromium and WebRTC the old one should be - // removed. crbug.com/544902, webrtc:5092. + // TODO(torbjorng,hbos): There are currently two versions of RequestIdentity, + // with default implementation to call the other version of itself (so that a + // call can be made regardless of which version has been overridden). The 1st + // version exists because it is currently implemented in chromium. The 2nd + // version will become the one and only RequestIdentity as soon as chromium + // implements the correct version. crbug.com/544902, webrtc:5092. virtual void RequestIdentity( rtc::KeyParams key_params, const rtc::scoped_refptr& observer) { - // Drop parameterization. - RequestIdentity(key_params.type(), observer); + // Add default ("null") expiration. + RequestIdentity(key_params, rtc::Optional(), observer); + } + virtual void RequestIdentity( + rtc::KeyParams key_params, + rtc::Optional expires, + const rtc::scoped_refptr& observer) { + // Drop |expires|. + RequestIdentity(key_params, observer); } }; @@ -89,7 +91,8 @@ class DtlsIdentityStoreImpl : public DtlsIdentityStoreInterface, // DtlsIdentityStoreInterface override; void RequestIdentity( - rtc::KeyType key_type, + rtc::KeyParams key_params, + rtc::Optional expires, const rtc::scoped_refptr& observer) override; // rtc::MessageHandler override; diff --git a/webrtc/api/dtlsidentitystore_unittest.cc b/webrtc/api/dtlsidentitystore_unittest.cc index 584ebb31fb..809e885216 100644 --- a/webrtc/api/dtlsidentitystore_unittest.cc +++ b/webrtc/api/dtlsidentitystore_unittest.cc @@ -85,7 +85,9 @@ class DtlsIdentityStoreTest : public testing::Test { TEST_F(DtlsIdentityStoreTest, RequestIdentitySuccessRSA) { EXPECT_TRUE_WAIT(store_->HasFreeIdentityForTesting(rtc::KT_RSA), kTimeoutMs); - store_->RequestIdentity(rtc::KT_RSA, observer_.get()); + store_->RequestIdentity(rtc::KeyParams(rtc::KT_RSA), + rtc::Optional(), + observer_.get()); EXPECT_TRUE_WAIT(observer_->LastRequestSucceeded(), kTimeoutMs); EXPECT_TRUE_WAIT(store_->HasFreeIdentityForTesting(rtc::KT_RSA), kTimeoutMs); @@ -93,7 +95,9 @@ TEST_F(DtlsIdentityStoreTest, RequestIdentitySuccessRSA) { observer_->Reset(); // Verifies that the callback is async when a free identity is ready. - store_->RequestIdentity(rtc::KT_RSA, observer_.get()); + store_->RequestIdentity(rtc::KeyParams(rtc::KT_RSA), + rtc::Optional(), + observer_.get()); EXPECT_FALSE(observer_->call_back_called()); EXPECT_TRUE_WAIT(observer_->LastRequestSucceeded(), kTimeoutMs); } @@ -102,13 +106,17 @@ TEST_F(DtlsIdentityStoreTest, RequestIdentitySuccessECDSA) { // Since store currently does not preemptively generate free ECDSA identities // we do not invoke HasFreeIdentityForTesting between requests. - store_->RequestIdentity(rtc::KT_ECDSA, observer_.get()); + store_->RequestIdentity(rtc::KeyParams(rtc::KT_ECDSA), + rtc::Optional(), + observer_.get()); EXPECT_TRUE_WAIT(observer_->LastRequestSucceeded(), kTimeoutMs); observer_->Reset(); // Verifies that the callback is async when a free identity is ready. - store_->RequestIdentity(rtc::KT_ECDSA, observer_.get()); + store_->RequestIdentity(rtc::KeyParams(rtc::KT_ECDSA), + rtc::Optional(), + observer_.get()); EXPECT_FALSE(observer_->call_back_called()); EXPECT_TRUE_WAIT(observer_->LastRequestSucceeded(), kTimeoutMs); } @@ -116,7 +124,9 @@ TEST_F(DtlsIdentityStoreTest, RequestIdentitySuccessECDSA) { TEST_F(DtlsIdentityStoreTest, DeleteStoreEarlyNoCrashRSA) { EXPECT_FALSE(store_->HasFreeIdentityForTesting(rtc::KT_RSA)); - store_->RequestIdentity(rtc::KT_RSA, observer_.get()); + store_->RequestIdentity(rtc::KeyParams(rtc::KT_RSA), + rtc::Optional(), + observer_.get()); store_.reset(); worker_thread_->Stop(); @@ -126,7 +136,9 @@ TEST_F(DtlsIdentityStoreTest, DeleteStoreEarlyNoCrashRSA) { TEST_F(DtlsIdentityStoreTest, DeleteStoreEarlyNoCrashECDSA) { EXPECT_FALSE(store_->HasFreeIdentityForTesting(rtc::KT_ECDSA)); - store_->RequestIdentity(rtc::KT_ECDSA, observer_.get()); + store_->RequestIdentity(rtc::KeyParams(rtc::KT_ECDSA), + rtc::Optional(), + observer_.get()); store_.reset(); worker_thread_->Stop(); diff --git a/webrtc/api/peerconnectionfactory.cc b/webrtc/api/peerconnectionfactory.cc index c05cd852f6..9d0e7014ac 100644 --- a/webrtc/api/peerconnectionfactory.cc +++ b/webrtc/api/peerconnectionfactory.cc @@ -45,10 +45,11 @@ class DtlsIdentityStoreWrapper : public DtlsIdentityStoreInterface { } void RequestIdentity( - rtc::KeyType key_type, + rtc::KeyParams key_params, + rtc::Optional expires, const rtc::scoped_refptr& observer) override { - store_->RequestIdentity(key_type, observer); + store_->RequestIdentity(key_params, expires, observer); } private: diff --git a/webrtc/api/test/fakedtlsidentitystore.h b/webrtc/api/test/fakedtlsidentitystore.h index 85acd3f66d..7c8a4fc71f 100644 --- a/webrtc/api/test/fakedtlsidentitystore.h +++ b/webrtc/api/test/fakedtlsidentitystore.h @@ -96,11 +96,15 @@ class FakeDtlsIdentityStore : public webrtc::DtlsIdentityStoreInterface, void use_alternate_key() { key_index_ = 1; } void RequestIdentity( - rtc::KeyType key_type, + rtc::KeyParams key_params, + rtc::Optional expires, const rtc::scoped_refptr& observer) override { // TODO(hbos): Should be able to generate KT_ECDSA too. - RTC_DCHECK(key_type == rtc::KT_RSA || should_fail_); + RTC_DCHECK((key_params.type() == rtc::KT_RSA && + key_params.rsa_params().mod_size == 1024 && + key_params.rsa_params().pub_exp == 0x10001) || + should_fail_); MessageData* msg = new MessageData( rtc::scoped_refptr(observer)); rtc::Thread::Current()->Post( diff --git a/webrtc/api/webrtcsessiondescriptionfactory.cc b/webrtc/api/webrtcsessiondescriptionfactory.cc index 8b1713549d..34cc438d29 100644 --- a/webrtc/api/webrtcsessiondescriptionfactory.cc +++ b/webrtc/api/webrtcsessiondescriptionfactory.cc @@ -188,13 +188,15 @@ WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory( identity_request_observer_->SignalCertificateReady.connect( this, &WebRtcSessionDescriptionFactory::SetCertificate); - rtc::KeyType key_type = rtc::KT_DEFAULT; + rtc::KeyParams key_params = rtc::KeyParams(); LOG(LS_VERBOSE) << "DTLS-SRTP enabled; sending DTLS identity request (key " - << "type: " << key_type << ")."; + << "type: " << key_params.type() << ")."; // Request identity. This happens asynchronously, so the caller will have a // chance to connect to SignalIdentityReady. - dtls_identity_store_->RequestIdentity(key_type, identity_request_observer_); + dtls_identity_store_->RequestIdentity(key_params, + rtc::Optional(), + identity_request_observer_); } WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory(