diff --git a/talk/app/webrtc/peerconnection_unittest.cc b/talk/app/webrtc/peerconnection_unittest.cc index 00100ac567..7edd03926e 100644 --- a/talk/app/webrtc/peerconnection_unittest.cc +++ b/talk/app/webrtc/peerconnection_unittest.cc @@ -145,29 +145,16 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver, public SignalingMessageReceiver, public ObserverInterface { public: - static PeerConnectionTestClient* CreateClientWithDtlsIdentityStore( - const std::string& id, - const MediaConstraintsInterface* constraints, - const PeerConnectionFactory::Options* options, - rtc::scoped_ptr dtls_identity_store) { - PeerConnectionTestClient* client(new PeerConnectionTestClient(id)); - if (!client->Init(constraints, options, dtls_identity_store.Pass())) { - delete client; - return nullptr; - } - return client; - } - static PeerConnectionTestClient* CreateClient( const std::string& id, const MediaConstraintsInterface* constraints, const PeerConnectionFactory::Options* options) { - rtc::scoped_ptr dtls_identity_store( - rtc::SSLStreamAdapter::HaveDtlsSrtp() ? new FakeDtlsIdentityStore() - : nullptr); - - return CreateClientWithDtlsIdentityStore(id, constraints, options, - dtls_identity_store.Pass()); + PeerConnectionTestClient* client(new PeerConnectionTestClient(id)); + if (!client->Init(constraints, options)) { + delete client; + return nullptr; + } + return client; } ~PeerConnectionTestClient() { @@ -717,10 +704,8 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver, explicit PeerConnectionTestClient(const std::string& id) : id_(id) {} - bool Init( - const MediaConstraintsInterface* constraints, - const PeerConnectionFactory::Options* options, - rtc::scoped_ptr dtls_identity_store) { + bool Init(const MediaConstraintsInterface* constraints, + const PeerConnectionFactory::Options* options) { EXPECT_TRUE(!peer_connection_); EXPECT_TRUE(!peer_connection_factory_); allocator_factory_ = webrtc::FakePortAllocatorFactory::Create(); @@ -744,21 +729,23 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver, if (options) { peer_connection_factory_->SetOptions(*options); } - peer_connection_ = CreatePeerConnection( - allocator_factory_.get(), constraints, dtls_identity_store.Pass()); + peer_connection_ = CreatePeerConnection(allocator_factory_.get(), + constraints); return peer_connection_.get() != nullptr; } rtc::scoped_refptr CreatePeerConnection( webrtc::PortAllocatorFactoryInterface* factory, - const MediaConstraintsInterface* constraints, - rtc::scoped_ptr dtls_identity_store) { + const MediaConstraintsInterface* constraints) { // CreatePeerConnection with IceServers. webrtc::PeerConnectionInterface::IceServers ice_servers; webrtc::PeerConnectionInterface::IceServer ice_server; ice_server.uri = "stun:stun.l.google.com:19302"; ice_servers.push_back(ice_server); + rtc::scoped_ptr dtls_identity_store( + rtc::SSLStreamAdapter::HaveDtlsSrtp() ? new FakeDtlsIdentityStore() + : nullptr); return peer_connection_factory_->CreatePeerConnection( ice_servers, constraints, factory, dtls_identity_store.Pass(), this); } @@ -992,11 +979,6 @@ class MAYBE_JsepPeerConnectionP2PTestClient : public testing::Test { nullptr); } - void SetSignalingReceivers() { - initiating_client_->set_signaling_message_receiver(receiving_client_.get()); - receiving_client_->set_signaling_message_receiver(initiating_client_.get()); - } - bool CreateTestClients(MediaConstraintsInterface* init_constraints, PeerConnectionFactory::Options* init_options, MediaConstraintsInterface* recv_constraints, @@ -1008,7 +990,8 @@ class MAYBE_JsepPeerConnectionP2PTestClient : public testing::Test { if (!initiating_client_ || !receiving_client_) { return false; } - SetSignalingReceivers(); + initiating_client_->set_signaling_message_receiver(receiving_client_.get()); + receiving_client_->set_signaling_message_receiver(initiating_client_.get()); return true; } @@ -1085,31 +1068,6 @@ class MAYBE_JsepPeerConnectionP2PTestClient : public testing::Test { kMaxWaitForFramesMs); } - void SetupAndVerifyDtlsCall() { - MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); - FakeConstraints setup_constraints; - setup_constraints.AddMandatory(MediaConstraintsInterface::kEnableDtlsSrtp, - true); - ASSERT_TRUE(CreateTestClients(&setup_constraints, &setup_constraints)); - LocalP2PTest(); - VerifyRenderedSize(640, 480); - } - - PeerConnectionTestClient* CreateDtlsClientWithAlternateKey() { - FakeConstraints setup_constraints; - setup_constraints.AddMandatory(MediaConstraintsInterface::kEnableDtlsSrtp, - true); - - rtc::scoped_ptr dtls_identity_store( - rtc::SSLStreamAdapter::HaveDtlsSrtp() ? new FakeDtlsIdentityStore() - : nullptr); - dtls_identity_store->use_alternate_key(); - - // Make sure the new client is using a different certificate. - return PeerConnectionTestClient::CreateClientWithDtlsIdentityStore( - "New Peer: ", &setup_constraints, nullptr, dtls_identity_store.Pass()); - } - void SendRtpData(webrtc::DataChannelInterface* dc, const std::string& data) { // Messages may get lost on the unreliable DataChannel, so we send multiple // times to avoid test flakiness. @@ -1123,29 +1081,10 @@ class MAYBE_JsepPeerConnectionP2PTestClient : public testing::Test { PeerConnectionTestClient* initializing_client() { return initiating_client_.get(); } - - // Set the |initiating_client_| to the |client| passed in and return the - // original |initiating_client_|. - PeerConnectionTestClient* set_initializing_client( - PeerConnectionTestClient* client) { - PeerConnectionTestClient* old = initiating_client_.release(); - initiating_client_.reset(client); - return old; - } - PeerConnectionTestClient* receiving_client() { return receiving_client_.get(); } - // Set the |receiving_client_| to the |client| passed in and return the - // original |receiving_client_|. - PeerConnectionTestClient* set_receiving_client( - PeerConnectionTestClient* client) { - PeerConnectionTestClient* old = receiving_client_.release(); - receiving_client_.reset(client); - return old; - } - private: rtc::scoped_ptr pss_; rtc::scoped_ptr ss_; @@ -1207,7 +1146,13 @@ TEST_F(MAYBE_JsepPeerConnectionP2PTestClient, DISABLED_LocalP2PTest1280By720) { // This test sets up a call between two endpoints that are configured to use // DTLS key agreement. As a result, DTLS is negotiated and used for transport. TEST_F(MAYBE_JsepPeerConnectionP2PTestClient, LocalP2PTestDtls) { - SetupAndVerifyDtlsCall(); + MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); + FakeConstraints setup_constraints; + setup_constraints.AddMandatory(MediaConstraintsInterface::kEnableDtlsSrtp, + true); + ASSERT_TRUE(CreateTestClients(&setup_constraints, &setup_constraints)); + LocalP2PTest(); + VerifyRenderedSize(640, 480); } // This test sets up a audio call initially and then upgrades to audio/video, @@ -1224,40 +1169,6 @@ TEST_F(MAYBE_JsepPeerConnectionP2PTestClient, LocalP2PTestDtlsRenegotiate) { receiving_client()->Negotiate(); } -// This test sets up a call transfer to a new caller with a different DTLS -// fingerprint. -TEST_F(MAYBE_JsepPeerConnectionP2PTestClient, LocalP2PTestDtlsTransferCallee) { - MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); - SetupAndVerifyDtlsCall(); - - // Keeping the original peer around which will still send packets to the - // receiving client. These SRTP packets will be dropped. - rtc::scoped_ptr original_peer( - set_initializing_client(CreateDtlsClientWithAlternateKey())); - - SetSignalingReceivers(); - receiving_client()->SetExpectIceRestart(true); - LocalP2PTest(); - VerifyRenderedSize(640, 480); -} - -// This test sets up a call transfer to a new callee with a different DTLS -// fingerprint. -TEST_F(MAYBE_JsepPeerConnectionP2PTestClient, LocalP2PTestDtlsTransferCaller) { - MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); - SetupAndVerifyDtlsCall(); - - // Keeping the original peer around which will still send packets to the - // receiving client. These SRTP packets will be dropped. - rtc::scoped_ptr original_peer( - set_receiving_client(CreateDtlsClientWithAlternateKey())); - - SetSignalingReceivers(); - initializing_client()->IceRestart(); - LocalP2PTest(); - VerifyRenderedSize(640, 480); -} - // This test sets up a call between two endpoints that are configured to use // DTLS key agreement. The offerer don't support SDES. As a result, DTLS is // negotiated and used for transport. diff --git a/talk/app/webrtc/test/fakedtlsidentitystore.h b/talk/app/webrtc/test/fakedtlsidentitystore.h index 5e596ca91e..0f9bdb9e6c 100644 --- a/talk/app/webrtc/test/fakedtlsidentitystore.h +++ b/talk/app/webrtc/test/fakedtlsidentitystore.h @@ -34,67 +34,36 @@ #include "talk/app/webrtc/peerconnectioninterface.h" #include "webrtc/base/rtccertificate.h" -static const struct { - const char* rsa_private_key_pem; - const char* cert_pem; -} kKeysAndCerts[] = { - {"-----BEGIN RSA PRIVATE KEY-----\n" - "MIICdwIBADANBgkqhkiG9w0BAQEFAASCAmEwggJdAgEAAoGBAMYRkbhmI7kVA/rM\n" - "czsZ+6JDhDvnkF+vn6yCAGuRPV03zuRqZtDy4N4to7PZu9PjqrRl7nDMXrG3YG9y\n" - "rlIAZ72KjcKKFAJxQyAKLCIdawKRyp8RdK3LEySWEZb0AV58IadqPZDTNHHRX8dz\n" - "5aTSMsbbkZ+C/OzTnbiMqLL/vg6jAgMBAAECgYAvgOs4FJcgvp+TuREx7YtiYVsH\n" - "mwQPTum2z/8VzWGwR8BBHBvIpVe1MbD/Y4seyI2aco/7UaisatSgJhsU46/9Y4fq\n" - "2TwXH9QANf4at4d9n/R6rzwpAJOpgwZgKvdQjkfrKTtgLV+/dawvpxUYkRH4JZM1\n" - "CVGukMfKNrSVH4Ap4QJBAOJmGV1ASPnB4r4nc99at7JuIJmd7fmuVUwUgYi4XgaR\n" - "WhScBsgYwZ/JoywdyZJgnbcrTDuVcWG56B3vXbhdpMsCQQDf9zeJrjnPZ3Cqm79y\n" - "kdqANep0uwZciiNiWxsQrCHztywOvbFhdp8iYVFG9EK8DMY41Y5TxUwsHD+67zao\n" - "ZNqJAkEA1suLUP/GvL8IwuRneQd2tWDqqRQ/Td3qq03hP7e77XtF/buya3Ghclo5\n" - "54czUR89QyVfJEC6278nzA7n2h1uVQJAcG6mztNL6ja/dKZjYZye2CY44QjSlLo0\n" - "MTgTSjdfg/28fFn2Jjtqf9Pi/X+50LWI/RcYMC2no606wRk9kyOuIQJBAK6VSAim\n" - "1pOEjsYQn0X5KEIrz1G3bfCbB848Ime3U2/FWlCHMr6ch8kCZ5d1WUeJD3LbwMNG\n" - "UCXiYxSsu20QNVw=\n" - "-----END RSA PRIVATE KEY-----\n", - "-----BEGIN CERTIFICATE-----\n" - "MIIBmTCCAQKgAwIBAgIEbzBSAjANBgkqhkiG9w0BAQsFADARMQ8wDQYDVQQDEwZX\n" - "ZWJSVEMwHhcNMTQwMTAyMTgyNDQ3WhcNMTQwMjAxMTgyNDQ3WjARMQ8wDQYDVQQD\n" - "EwZXZWJSVEMwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAMYRkbhmI7kVA/rM\n" - "czsZ+6JDhDvnkF+vn6yCAGuRPV03zuRqZtDy4N4to7PZu9PjqrRl7nDMXrG3YG9y\n" - "rlIAZ72KjcKKFAJxQyAKLCIdawKRyp8RdK3LEySWEZb0AV58IadqPZDTNHHRX8dz\n" - "5aTSMsbbkZ+C/OzTnbiMqLL/vg6jAgMBAAEwDQYJKoZIhvcNAQELBQADgYEAUflI\n" - "VUe5Krqf5RVa5C3u/UTAOAUJBiDS3VANTCLBxjuMsvqOG0WvaYWP3HYPgrz0jXK2\n" - "LJE/mGw3MyFHEqi81jh95J+ypl6xKW6Rm8jKLR87gUvCaVYn/Z4/P3AqcQTB7wOv\n" - "UD0A8qfhfDM+LK6rPAnCsVN0NRDY3jvd6rzix9M=\n" - "-----END CERTIFICATE-----\n"}, - {"-----BEGIN RSA PRIVATE KEY-----\n" - "MIICXQIBAAKBgQDeYqlyJ1wuiMsi905e3X81/WA/G3ym50PIDZBVtSwZi7JVQPgj\n" - "Bl8CPZMvDh9EwB4Ji9ytA8dZZbQ4WbJWPr73zPpJSCvQqz6sOXSlenBRi72acNaQ\n" - "sOR/qPvviJx5I6Hqo4qemfnjZhAW85a5BpgrAwKgMLIQTHCTLWwVSyrDrwIDAQAB\n" - "AoGARni9eY8/hv+SX+I+05EdXt6MQXNUbQ+cSykBNCfVccLzIFEWUQMT2IHqwl6X\n" - "ShIXcq7/n1QzOAEiuzixauM3YHg4xZ1Um2Ha9a7ig5Xg4v6b43bmMkNE6LkoAtYs\n" - "qnQdfMh442b1liDud6IMb1Qk0amt3fSrgRMc547TZQVx4QECQQDxUeDm94r3p4ng\n" - "5rCLLC1K5/6HSTZsh7jatKPlz7GfP/IZlYV7iE5784/n0wRiCjZOS7hQRy/8m2Gp\n" - "pf4aZq+DAkEA6+np4d36FYikydvUrupLT3FkdRHGn/v83qOll/VmeNh+L1xMZlIP\n" - "tM26hAXCcQb7O5+J9y3cx2CAQsBS11ZXZQJAfGgTo76WG9p5UEJdXUInD2jOZPwv\n" - "XIATolxh6kXKcijLLLlSmT7KB0inNYIpzkkpee+7U1d/u6B3FriGaSHq9QJBAM/J\n" - "ICnDdLCgwNvWVraVQC3BpwSB2pswvCFwq7py94V60XFvbw80Ogc6qIv98qvQxVlX\n" - "hJIEgA/PjEi+0ng94Q0CQQDm8XSDby35gmjO+6eRmJtAjtB7nguLvrPXM6CPXRmD\n" - "sRoBocpHw6j9UdzZ6qYG0FkdXZghezXFY58ro2BYYRR3\n" - "-----END RSA PRIVATE KEY-----\n", - "-----BEGIN CERTIFICATE-----\n" - "MIICWDCCAcGgAwIBAgIJALgDjxMbBOhbMA0GCSqGSIb3DQEBCwUAMEUxCzAJBgNV\n" - "BAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBX\n" - "aWRnaXRzIFB0eSBMdGQwHhcNMTUxMTEzMjIzMjEzWhcNMTYxMTEyMjIzMjEzWjBF\n" - "MQswCQYDVQQGEwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UECgwYSW50\n" - "ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKB\n" - "gQDeYqlyJ1wuiMsi905e3X81/WA/G3ym50PIDZBVtSwZi7JVQPgjBl8CPZMvDh9E\n" - "wB4Ji9ytA8dZZbQ4WbJWPr73zPpJSCvQqz6sOXSlenBRi72acNaQsOR/qPvviJx5\n" - "I6Hqo4qemfnjZhAW85a5BpgrAwKgMLIQTHCTLWwVSyrDrwIDAQABo1AwTjAdBgNV\n" - "HQ4EFgQUx2tbJdlcSTCepn09UdYORXKuSTAwHwYDVR0jBBgwFoAUx2tbJdlcSTCe\n" - "pn09UdYORXKuSTAwDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQsFAAOBgQAmp9Id\n" - "E716gHMqeBG4S2FCgVFCr0a0ugkaneQAN/c2L9CbMemEN9W6jvucUIVOtYd90dDW\n" - "lXuowWmT/JctPe3D2qt4yvYW3puECHk2tVQmrJOZiZiTRtWm6HxkmoUYHYp/DtaS\n" - "1Xe29gSTnZtI5sQCrGMzk3SGRSSs7ejLKiVDBQ==\n" - "-----END CERTIFICATE-----\n"}}; +static const char kRSA_PRIVATE_KEY_PEM[] = + "-----BEGIN RSA PRIVATE KEY-----\n" + "MIICdwIBADANBgkqhkiG9w0BAQEFAASCAmEwggJdAgEAAoGBAMYRkbhmI7kVA/rM\n" + "czsZ+6JDhDvnkF+vn6yCAGuRPV03zuRqZtDy4N4to7PZu9PjqrRl7nDMXrG3YG9y\n" + "rlIAZ72KjcKKFAJxQyAKLCIdawKRyp8RdK3LEySWEZb0AV58IadqPZDTNHHRX8dz\n" + "5aTSMsbbkZ+C/OzTnbiMqLL/vg6jAgMBAAECgYAvgOs4FJcgvp+TuREx7YtiYVsH\n" + "mwQPTum2z/8VzWGwR8BBHBvIpVe1MbD/Y4seyI2aco/7UaisatSgJhsU46/9Y4fq\n" + "2TwXH9QANf4at4d9n/R6rzwpAJOpgwZgKvdQjkfrKTtgLV+/dawvpxUYkRH4JZM1\n" + "CVGukMfKNrSVH4Ap4QJBAOJmGV1ASPnB4r4nc99at7JuIJmd7fmuVUwUgYi4XgaR\n" + "WhScBsgYwZ/JoywdyZJgnbcrTDuVcWG56B3vXbhdpMsCQQDf9zeJrjnPZ3Cqm79y\n" + "kdqANep0uwZciiNiWxsQrCHztywOvbFhdp8iYVFG9EK8DMY41Y5TxUwsHD+67zao\n" + "ZNqJAkEA1suLUP/GvL8IwuRneQd2tWDqqRQ/Td3qq03hP7e77XtF/buya3Ghclo5\n" + "54czUR89QyVfJEC6278nzA7n2h1uVQJAcG6mztNL6ja/dKZjYZye2CY44QjSlLo0\n" + "MTgTSjdfg/28fFn2Jjtqf9Pi/X+50LWI/RcYMC2no606wRk9kyOuIQJBAK6VSAim\n" + "1pOEjsYQn0X5KEIrz1G3bfCbB848Ime3U2/FWlCHMr6ch8kCZ5d1WUeJD3LbwMNG\n" + "UCXiYxSsu20QNVw=\n" + "-----END RSA PRIVATE KEY-----\n"; + +static const char kCERT_PEM[] = + "-----BEGIN CERTIFICATE-----\n" + "MIIBmTCCAQKgAwIBAgIEbzBSAjANBgkqhkiG9w0BAQsFADARMQ8wDQYDVQQDEwZX\n" + "ZWJSVEMwHhcNMTQwMTAyMTgyNDQ3WhcNMTQwMjAxMTgyNDQ3WjARMQ8wDQYDVQQD\n" + "EwZXZWJSVEMwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAMYRkbhmI7kVA/rM\n" + "czsZ+6JDhDvnkF+vn6yCAGuRPV03zuRqZtDy4N4to7PZu9PjqrRl7nDMXrG3YG9y\n" + "rlIAZ72KjcKKFAJxQyAKLCIdawKRyp8RdK3LEySWEZb0AV58IadqPZDTNHHRX8dz\n" + "5aTSMsbbkZ+C/OzTnbiMqLL/vg6jAgMBAAEwDQYJKoZIhvcNAQELBQADgYEAUflI\n" + "VUe5Krqf5RVa5C3u/UTAOAUJBiDS3VANTCLBxjuMsvqOG0WvaYWP3HYPgrz0jXK2\n" + "LJE/mGw3MyFHEqi81jh95J+ypl6xKW6Rm8jKLR87gUvCaVYn/Z4/P3AqcQTB7wOv\n" + "UD0A8qfhfDM+LK6rPAnCsVN0NRDY3jvd6rzix9M=\n" + "-----END CERTIFICATE-----\n"; class FakeDtlsIdentityStore : public webrtc::DtlsIdentityStoreInterface, public rtc::MessageHandler { @@ -108,9 +77,6 @@ class FakeDtlsIdentityStore : public webrtc::DtlsIdentityStoreInterface, should_fail_ = should_fail; } - void use_original_key() { key_index_ = 0; } - void use_alternate_key() { key_index_ = 1; } - void RequestIdentity( rtc::KeyType key_type, const rtc::scoped_refptr& @@ -126,9 +92,8 @@ class FakeDtlsIdentityStore : public webrtc::DtlsIdentityStoreInterface, static rtc::scoped_refptr GenerateCertificate() { std::string cert; std::string key; - rtc::SSLIdentity::PemToDer("CERTIFICATE", kKeysAndCerts[0].cert_pem, &cert); - rtc::SSLIdentity::PemToDer("RSA PRIVATE KEY", - kKeysAndCerts[0].rsa_private_key_pem, &key); + rtc::SSLIdentity::PemToDer("CERTIFICATE", kCERT_PEM, &cert); + rtc::SSLIdentity::PemToDer("RSA PRIVATE KEY", kRSA_PRIVATE_KEY_PEM, &key); std::string pem_cert = rtc::SSLIdentity::DerToPem( rtc::kPemTypeCertificate, @@ -150,11 +115,6 @@ class FakeDtlsIdentityStore : public webrtc::DtlsIdentityStoreInterface, MSG_FAILURE, }; - const char* get_key() { - return kKeysAndCerts[key_index_].rsa_private_key_pem; - } - const char* get_cert() { return kKeysAndCerts[key_index_].cert_pem; } - // rtc::MessageHandler implementation. void OnMessage(rtc::Message* msg) { MessageData* message_data = static_cast(msg->pdata); @@ -164,8 +124,9 @@ class FakeDtlsIdentityStore : public webrtc::DtlsIdentityStoreInterface, case MSG_SUCCESS: { std::string cert; std::string key; - rtc::SSLIdentity::PemToDer("CERTIFICATE", get_cert(), &cert); - rtc::SSLIdentity::PemToDer("RSA PRIVATE KEY", get_key(), &key); + rtc::SSLIdentity::PemToDer("CERTIFICATE", kCERT_PEM, &cert); + rtc::SSLIdentity::PemToDer("RSA PRIVATE KEY", kRSA_PRIVATE_KEY_PEM, + &key); observer->OnSuccess(cert, key); break; } @@ -177,7 +138,6 @@ class FakeDtlsIdentityStore : public webrtc::DtlsIdentityStoreInterface, } bool should_fail_; - int key_index_ = 0; }; #endif // TALK_APP_WEBRTC_TEST_FAKEDTLSIDENTITYSERVICE_H_ diff --git a/talk/session/media/channel.cc b/talk/session/media/channel.cc index a25ab7d0df..f83afa1ea4 100644 --- a/talk/session/media/channel.cc +++ b/talk/session/media/channel.cc @@ -249,13 +249,6 @@ bool BaseChannel::SetTransport_w(const std::string& transport_name) { return true; } - // When using DTLS-SRTP, we must reset the SrtpFilter every time the transport - // changes and wait until the DTLS handshake is complete to set the newly - // negotiated parameters. - if (ShouldSetupDtlsSrtp()) { - srtp_filter_.ResetParams(); - } - set_transport_channel(transport_controller_->CreateTransportChannel_w( transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP)); if (!transport_channel()) { @@ -325,9 +318,6 @@ void BaseChannel::set_rtcp_transport_channel(TransportChannel* new_tc) { rtcp_transport_channel_ = new_tc; if (new_tc) { - RTC_CHECK(!(ShouldSetupDtlsSrtp() && srtp_filter_.IsActive())) - << "Setting RTCP for DTLS/SRTP after SrtpFilter is active " - << "should never happen."; ConnectToTransportChannel(new_tc); for (const auto& pair : rtcp_socket_options_) { new_tc->SetOption(pair.first, pair.second); @@ -346,7 +336,6 @@ void BaseChannel::ConnectToTransportChannel(TransportChannel* tc) { tc->SignalWritableState.connect(this, &BaseChannel::OnWritableState); tc->SignalReadPacket.connect(this, &BaseChannel::OnChannelRead); tc->SignalReadyToSend.connect(this, &BaseChannel::OnReadyToSend); - tc->SignalDtlsState.connect(this, &BaseChannel::OnDtlsState); } void BaseChannel::DisconnectFromTransportChannel(TransportChannel* tc) { @@ -355,7 +344,6 @@ void BaseChannel::DisconnectFromTransportChannel(TransportChannel* tc) { tc->SignalWritableState.disconnect(this); tc->SignalReadPacket.disconnect(this); tc->SignalReadyToSend.disconnect(this); - tc->SignalDtlsState.disconnect(this); } bool BaseChannel::Enable(bool enable) { @@ -428,10 +416,10 @@ bool BaseChannel::IsReadyToReceive() const { bool BaseChannel::IsReadyToSend() const { // Send outgoing data if we are enabled, have local and remote content, // and we have had some form of connectivity. - return enabled() && IsReceiveContentDirection(remote_content_direction_) && + return enabled() && + IsReceiveContentDirection(remote_content_direction_) && IsSendContentDirection(local_content_direction_) && - was_ever_writable() && - (srtp_filter_.IsActive() || !ShouldSetupDtlsSrtp()); + was_ever_writable(); } bool BaseChannel::SendPacket(rtc::Buffer* packet, @@ -486,22 +474,6 @@ void BaseChannel::OnReadyToSend(TransportChannel* channel) { SetReadyToSend(channel == rtcp_transport_channel_, true); } -void BaseChannel::OnDtlsState(TransportChannel* channel, - DtlsTransportState state) { - if (!ShouldSetupDtlsSrtp()) { - return; - } - - // Reset the srtp filter if it's not the CONNECTED state. For the CONNECTED - // state, setting up DTLS-SRTP context is deferred to ChannelWritable_w to - // cover other scenarios like the whole channel is writable (not just this - // TransportChannel) or when TransportChannel is attached after DTLS is - // negotiated. - if (state != DTLS_TRANSPORT_CONNECTED) { - srtp_filter_.ResetParams(); - } -} - void BaseChannel::SetReadyToSend(bool rtcp, bool ready) { if (rtcp) { rtcp_ready_to_send_ = ready; @@ -789,9 +761,8 @@ void BaseChannel::UpdateWritableState_w() { void BaseChannel::ChannelWritable_w() { ASSERT(worker_thread_ == rtc::Thread::Current()); - if (writable_) { + if (writable_) return; - } LOG(LS_INFO) << "Channel writable (" << content_name_ << ")" << (was_ever_writable_ ? "" : " for the first time"); @@ -807,8 +778,22 @@ void BaseChannel::ChannelWritable_w() { } } + // If we're doing DTLS-SRTP, now is the time. + if (!was_ever_writable_ && ShouldSetupDtlsSrtp()) { + if (!SetupDtlsSrtp(false)) { + SignalDtlsSetupFailure_w(false); + return; + } + + if (rtcp_transport_channel_) { + if (!SetupDtlsSrtp(true)) { + SignalDtlsSetupFailure_w(true); + return; + } + } + } + was_ever_writable_ = true; - MaybeSetupDtlsSrtp_w(); writable_ = true; ChangeState(); } @@ -837,8 +822,7 @@ bool BaseChannel::SetDtlsSrtpCryptoSuites(TransportChannel* tc, bool rtcp) { } bool BaseChannel::ShouldSetupDtlsSrtp() const { - // Since DTLS is applied to all channels, checking RTP should be enough. - return transport_channel_ && transport_channel_->IsDtlsActive(); + return true; } // This function returns true if either DTLS-SRTP is not in use @@ -849,7 +833,9 @@ bool BaseChannel::SetupDtlsSrtp(bool rtcp_channel) { TransportChannel* channel = rtcp_channel ? rtcp_transport_channel_ : transport_channel_; - RTC_DCHECK(channel->IsDtlsActive()); + // No DTLS + if (!channel->IsDtlsActive()) + return true; int selected_crypto_suite; @@ -929,28 +915,6 @@ bool BaseChannel::SetupDtlsSrtp(bool rtcp_channel) { return ret; } -void BaseChannel::MaybeSetupDtlsSrtp_w() { - if (srtp_filter_.IsActive()) { - return; - } - - if (!ShouldSetupDtlsSrtp()) { - return; - } - - if (!SetupDtlsSrtp(false)) { - SignalDtlsSetupFailure_w(false); - return; - } - - if (rtcp_transport_channel_) { - if (!SetupDtlsSrtp(true)) { - SignalDtlsSetupFailure_w(true); - return; - } - } -} - void BaseChannel::ChannelNotWritable_w() { ASSERT(worker_thread_ == rtc::Thread::Current()); if (!writable_) @@ -2299,7 +2263,7 @@ void DataChannel::GetSrtpCryptoSuites(std::vector* crypto_suites) const { } bool DataChannel::ShouldSetupDtlsSrtp() const { - return (data_channel_type_ == DCT_RTP) && BaseChannel::ShouldSetupDtlsSrtp(); + return (data_channel_type_ == DCT_RTP); } void DataChannel::OnStreamClosedRemotely(uint32_t sid) { diff --git a/talk/session/media/channel.h b/talk/session/media/channel.h index 11409548e3..ef0eb562f9 100644 --- a/talk/session/media/channel.h +++ b/talk/session/media/channel.h @@ -213,8 +213,6 @@ class BaseChannel int flags); void OnReadyToSend(TransportChannel* channel); - void OnDtlsState(TransportChannel* channel, DtlsTransportState state); - bool PacketIsRtcp(const TransportChannel* channel, const char* data, size_t len); bool SendPacket(bool rtcp, @@ -237,7 +235,6 @@ class BaseChannel // Do the DTLS key expansion and impose it on the SRTP/SRTCP filters. // |rtcp_channel| indicates whether to set up the RTP or RTCP filter. bool SetupDtlsSrtp(bool rtcp_channel); - void MaybeSetupDtlsSrtp_w(); // Set the DTLS-SRTP cipher policy on this channel as appropriate. bool SetDtlsSrtpCryptoSuites(TransportChannel* tc, bool rtcp); diff --git a/talk/session/media/srtpfilter.h b/talk/session/media/srtpfilter.h index 6b941f32fd..d30cee69c3 100644 --- a/talk/session/media/srtpfilter.h +++ b/talk/session/media/srtpfilter.h @@ -138,8 +138,6 @@ class SrtpFilter { // Update the silent threshold (in ms) for signaling errors. void set_signal_silent_time(uint32_t signal_silent_time_in_ms); - bool ResetParams(); - sigslot::repeater3 SignalSrtpError; protected: @@ -155,6 +153,7 @@ class SrtpFilter { CryptoParams* selected_params); bool ApplyParams(const CryptoParams& send_params, const CryptoParams& recv_params); + bool ResetParams(); static bool ParseKeyParams(const std::string& params, uint8_t* key, int len); private: diff --git a/webrtc/p2p/base/dtlstransportchannel.cc b/webrtc/p2p/base/dtlstransportchannel.cc index fce7469cf6..fff24f30b8 100644 --- a/webrtc/p2p/base/dtlstransportchannel.cc +++ b/webrtc/p2p/base/dtlstransportchannel.cc @@ -199,8 +199,6 @@ bool DtlsTransportChannelWrapper::SetRemoteFingerprint( size_t digest_len) { rtc::Buffer remote_fingerprint_value(digest, digest_len); - // Once we have the local certificate, the same remote fingerprint can be set - // multiple times. if (dtls_active_ && remote_fingerprint_value_ == remote_fingerprint_value && !digest_alg.empty()) { // This may happen during renegotiation. @@ -208,36 +206,28 @@ bool DtlsTransportChannelWrapper::SetRemoteFingerprint( return true; } - // If the other side doesn't support DTLS, turn off |dtls_active_|. + // Allow SetRemoteFingerprint with a NULL digest even if SetLocalCertificate + // hasn't been called. + if (dtls_ || (!dtls_active_ && !digest_alg.empty())) { + LOG_J(LS_ERROR, this) << "Can't set DTLS remote settings in this state."; + return false; + } + if (digest_alg.empty()) { - RTC_DCHECK(!digest_len); LOG_J(LS_INFO, this) << "Other side didn't support DTLS."; dtls_active_ = false; return true; } - // Otherwise, we must have a local certificate before setting remote - // fingerprint. - if (!dtls_active_) { - LOG_J(LS_ERROR, this) << "Can't set DTLS remote settings in this state."; - return false; - } - // At this point we know we are doing DTLS remote_fingerprint_value_ = remote_fingerprint_value.Pass(); remote_fingerprint_algorithm_ = digest_alg; - bool reconnect = dtls_; - if (!SetupDtls()) { set_dtls_state(DTLS_TRANSPORT_FAILED); return false; } - if (reconnect) { - Reconnect(); - } - return true; } @@ -540,13 +530,8 @@ void DtlsTransportChannelWrapper::OnDtlsEvent(rtc::StreamInterface* dtls, if (sig & rtc::SE_READ) { char buf[kMaxDtlsPacketLen]; size_t read; - rtc::StreamResult result = dtls_->Read(buf, sizeof(buf), &read, NULL); - if (result == rtc::SR_SUCCESS) { + if (dtls_->Read(buf, sizeof(buf), &read, NULL) == rtc::SR_SUCCESS) { SignalReadPacket(this, buf, read, rtc::CreatePacketTime(0), 0); - } else if (result == rtc::SR_EOS) { - // If the SSL stream has closed remotely, reset the |sig| to be SE_CLOSE - // so it could be handled below. - sig = rtc::SE_CLOSE; } } if (sig & rtc::SE_CLOSE) { @@ -631,12 +616,4 @@ void DtlsTransportChannelWrapper::OnConnectionRemoved( SignalConnectionRemoved(this); } -void DtlsTransportChannelWrapper::Reconnect() { - set_dtls_state(DTLS_TRANSPORT_NEW); - set_writable(false); - if (channel_->writable()) { - OnWritableState(channel_); - } -} - } // namespace cricket diff --git a/webrtc/p2p/base/dtlstransportchannel.h b/webrtc/p2p/base/dtlstransportchannel.h index 955b963a36..71f68877d4 100644 --- a/webrtc/p2p/base/dtlstransportchannel.h +++ b/webrtc/p2p/base/dtlstransportchannel.h @@ -216,7 +216,6 @@ class DtlsTransportChannelWrapper : public TransportChannelImpl { void OnRoleConflict(TransportChannelImpl* channel); void OnRouteChange(TransportChannel* channel, const Candidate& candidate); void OnConnectionRemoved(TransportChannelImpl* channel); - void Reconnect(); Transport* transport_; // The transport_ that created us. rtc::Thread* worker_thread_; // Everything should occur on this thread. diff --git a/webrtc/p2p/base/transportchannel.cc b/webrtc/p2p/base/transportchannel.cc index 6cbe2b7583..f497bce91b 100644 --- a/webrtc/p2p/base/transportchannel.cc +++ b/webrtc/p2p/base/transportchannel.cc @@ -51,7 +51,7 @@ void TransportChannel::set_dtls_state(DtlsTransportState state) { LOG_J(LS_VERBOSE, this) << "set_dtls_state from:" << dtls_state_ << " to " << state; dtls_state_ = state; - SignalDtlsState(this, state); + SignalDtlsState(this); } bool TransportChannel::SetSrtpCryptoSuites(const std::vector& ciphers) { diff --git a/webrtc/p2p/base/transportchannel.h b/webrtc/p2p/base/transportchannel.h index b91af139b7..de0bd45a25 100644 --- a/webrtc/p2p/base/transportchannel.h +++ b/webrtc/p2p/base/transportchannel.h @@ -79,9 +79,8 @@ class TransportChannel : public sigslot::has_slots<> { // Emitted when the TransportChannel's ability to send has changed. sigslot::signal1 SignalReadyToSend; sigslot::signal1 SignalReceivingState; - // Emitted whenever DTLS-SRTP is setup which will require setting up a new - // SRTP context. - sigslot::signal2 SignalDtlsState; + // Emitted when the DtlsTransportState has changed. + sigslot::signal1 SignalDtlsState; // Attempts to send the given packet. The return value is < 0 on failure. // TODO: Remove the default argument once channel code is updated.