diff --git a/api/BUILD.gn b/api/BUILD.gn index 61c000d3f0..e8d03f0e3e 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -274,7 +274,6 @@ rtc_library("libjingle_peerconnection_api") { visibility = [ "*" ] cflags = [] sources = [ - "crypto_params.h", "data_channel_interface.cc", "data_channel_interface.h", "jsep.cc", diff --git a/api/crypto_params.h b/api/crypto_params.h deleted file mode 100644 index 34906ea0ef..0000000000 --- a/api/crypto_params.h +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright (c) 2004 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#ifndef API_CRYPTO_PARAMS_H_ -#define API_CRYPTO_PARAMS_H_ - -#include - -#include "absl/strings/string_view.h" - -namespace cricket { - -// Parameters for SRTP negotiation, as described in RFC 4568. -// TODO(benwright) - Rename to SrtpCryptoParams as these only apply to SRTP and -// not generic crypto parameters for WebRTC. -struct CryptoParams { - CryptoParams() : tag(0) {} - CryptoParams(int t, - absl::string_view cs, - absl::string_view kp, - absl::string_view sp) - : tag(t), crypto_suite(cs), key_params(kp), session_params(sp) {} - - bool Matches(const CryptoParams& params) const { - return (tag == params.tag && crypto_suite == params.crypto_suite); - } - - int tag; - std::string crypto_suite; - std::string key_params; - std::string session_params; -}; - -} // namespace cricket - -#endif // API_CRYPTO_PARAMS_H_ diff --git a/p2p/base/transport_description.h b/p2p/base/transport_description.h index 7d28ad52e9..53a1804933 100644 --- a/p2p/base/transport_description.h +++ b/p2p/base/transport_description.h @@ -25,17 +25,6 @@ namespace cricket { -// SEC_ENABLED and SEC_REQUIRED should only be used if the session -// was negotiated over TLS, to protect the inline crypto material -// exchange. -// SEC_DISABLED: No crypto in outgoing offer, ignore any supplied crypto. -// SEC_ENABLED: Crypto in outgoing offer and answer (if supplied in offer). -// SEC_REQUIRED: Crypto in outgoing offer and answer. Fail any offer with absent -// or unsupported crypto. -// TODO(deadbeef): Remove this or rename it to something more appropriate, like -// SdesPolicy. -enum SecurePolicy { SEC_DISABLED, SEC_ENABLED, SEC_REQUIRED }; - // Whether our side of the call is driving the negotiation, or the other side. enum IceRole { ICEROLE_CONTROLLING = 0, ICEROLE_CONTROLLED, ICEROLE_UNKNOWN }; diff --git a/p2p/base/transport_description_factory.cc b/p2p/base/transport_description_factory.cc index 7eb21da166..6e3af94384 100644 --- a/p2p/base/transport_description_factory.cc +++ b/p2p/base/transport_description_factory.cc @@ -23,7 +23,7 @@ namespace cricket { TransportDescriptionFactory::TransportDescriptionFactory( const webrtc::FieldTrialsView& field_trials) - : secure_(SEC_DISABLED), field_trials_(field_trials) {} + : field_trials_(field_trials) {} TransportDescriptionFactory::~TransportDescriptionFactory() = default; @@ -47,13 +47,15 @@ std::unique_ptr TransportDescriptionFactory::CreateOffer( desc->AddOption(ICE_OPTION_RENOMINATION); } - // If we are trying to establish a secure transport, add a fingerprint. - if (secure_ == SEC_ENABLED || secure_ == SEC_REQUIRED) { - // Fail if we can't create the fingerprint. - // If we are the initiator set role to "actpass". - if (!SetSecurityInfo(desc.get(), CONNECTIONROLE_ACTPASS)) { - return NULL; - } + // If we are not trying to establish a secure transport, don't add a + // fingerprint. + if (insecure_ && !certificate_) { + return desc; + } + // Fail if we can't create the fingerprint. + // If we are the initiator set role to "actpass". + if (!SetSecurityInfo(desc.get(), CONNECTIONROLE_ACTPASS)) { + return NULL; } return desc; @@ -87,43 +89,49 @@ std::unique_ptr TransportDescriptionFactory::CreateAnswer( if (options.enable_ice_renomination) { desc->AddOption(ICE_OPTION_RENOMINATION); } - - // Negotiate security params. - if (offer && offer->identity_fingerprint.get()) { - // The offer supports DTLS, so answer with DTLS, as long as we support it. - if (secure_ == SEC_ENABLED || secure_ == SEC_REQUIRED) { - ConnectionRole role = CONNECTIONROLE_NONE; - // If the offer does not constrain the role, go with preference. - if (offer->connection_role == CONNECTIONROLE_ACTPASS) { - role = (options.prefer_passive_role) ? CONNECTIONROLE_PASSIVE - : CONNECTIONROLE_ACTIVE; - } else if (offer->connection_role == CONNECTIONROLE_ACTIVE) { - role = CONNECTIONROLE_PASSIVE; - } else if (offer->connection_role == CONNECTIONROLE_PASSIVE) { - role = CONNECTIONROLE_ACTIVE; - } else if (offer->connection_role == CONNECTIONROLE_NONE) { - // This case may be reached if a=setup is not present in the SDP. - RTC_LOG(LS_WARNING) << "Remote offer connection role is NONE, which is " - "a protocol violation"; - role = (options.prefer_passive_role) ? CONNECTIONROLE_PASSIVE - : CONNECTIONROLE_ACTIVE; - } else { - RTC_LOG(LS_ERROR) << "Remote offer connection role is " << role - << " which is a protocol violation"; - RTC_DCHECK_NOTREACHED(); - } - - if (!SetSecurityInfo(desc.get(), role)) { - return NULL; - } + // Special affordance for testing: Answer without DTLS params + // if we are insecure without a certificate, or if we are + // insecure with a non-DTLS offer. + if ((!certificate_ || !offer->identity_fingerprint.get()) && insecure()) { + return desc; + } + if (!offer->identity_fingerprint.get()) { + if (require_transport_attributes) { + // We require DTLS, but the other side didn't offer it. Fail. + RTC_LOG(LS_WARNING) << "Failed to create TransportDescription answer " + "because of incompatible security settings"; + return NULL; } - } else if (require_transport_attributes && secure_ == SEC_REQUIRED) { - // We require DTLS, but the other side didn't offer it. Fail. - RTC_LOG(LS_WARNING) << "Failed to create TransportDescription answer " - "because of incompatible security settings"; + // This may be a bundled section, fingerprint may legitimately be missing. + return desc; + } + // Negotiate security params. + // The offer supports DTLS, so answer with DTLS. + RTC_CHECK(certificate_); + ConnectionRole role = CONNECTIONROLE_NONE; + // If the offer does not constrain the role, go with preference. + if (offer->connection_role == CONNECTIONROLE_ACTPASS) { + role = (options.prefer_passive_role) ? CONNECTIONROLE_PASSIVE + : CONNECTIONROLE_ACTIVE; + } else if (offer->connection_role == CONNECTIONROLE_ACTIVE) { + role = CONNECTIONROLE_PASSIVE; + } else if (offer->connection_role == CONNECTIONROLE_PASSIVE) { + role = CONNECTIONROLE_ACTIVE; + } else if (offer->connection_role == CONNECTIONROLE_NONE) { + // This case may be reached if a=setup is not present in the SDP. + RTC_LOG(LS_WARNING) << "Remote offer connection role is NONE, which is " + "a protocol violation"; + role = (options.prefer_passive_role) ? CONNECTIONROLE_PASSIVE + : CONNECTIONROLE_ACTIVE; + } else { + RTC_LOG(LS_ERROR) << "Remote offer connection role is " << role + << " which is a protocol violation"; + RTC_DCHECK_NOTREACHED(); + return NULL; + } + if (!SetSecurityInfo(desc.get(), role)) { return NULL; } - return desc; } diff --git a/p2p/base/transport_description_factory.h b/p2p/base/transport_description_factory.h index 11352f88b4..bd0cb9078e 100644 --- a/p2p/base/transport_description_factory.h +++ b/p2p/base/transport_description_factory.h @@ -43,15 +43,12 @@ class TransportDescriptionFactory { const webrtc::FieldTrialsView& field_trials); ~TransportDescriptionFactory(); - SecurePolicy secure() const { return secure_; } // The certificate to use when setting up DTLS. const rtc::scoped_refptr& certificate() const { return certificate_; } - // Specifies the transport security policy to use. - void set_secure(SecurePolicy s) { secure_ = s; } - // Specifies the certificate to use (only used when secure != SEC_DISABLED). + // Specifies the certificate to use void set_certificate(rtc::scoped_refptr certificate) { certificate_ = std::move(certificate); } @@ -76,12 +73,18 @@ class TransportDescriptionFactory { IceCredentialsIterator* ice_credentials) const; const webrtc::FieldTrialsView& trials() const { return field_trials_; } + // Functions for disabling encryption - test only! + // In insecure mode, the connection will accept a description without + // fingerprint, and will generate SDP even if certificate is not set. + // If certificate is set, it will accept a description both with and + // without fingerprint, but will generate a description with fingerprint. + bool insecure() const { return insecure_; } + void SetInsecureForTesting() { insecure_ = true; } private: bool SetSecurityInfo(TransportDescription* description, ConnectionRole role) const; - - SecurePolicy secure_; + bool insecure_ = false; rtc::scoped_refptr certificate_; const webrtc::FieldTrialsView& field_trials_; }; diff --git a/p2p/base/transport_description_factory_unittest.cc b/p2p/base/transport_description_factory_unittest.cc index 0da5b7c294..0d38ac6d7b 100644 --- a/p2p/base/transport_description_factory_unittest.cc +++ b/p2p/base/transport_description_factory_unittest.cc @@ -33,6 +33,7 @@ using cricket::TransportDescriptionFactory; using cricket::TransportOptions; using ::testing::Contains; using ::testing::Not; +using ::testing::NotNull; class TransportDescriptionFactoryTest : public ::testing::Test { public: @@ -43,7 +44,11 @@ class TransportDescriptionFactoryTest : public ::testing::Test { cert1_(rtc::RTCCertificate::Create(std::unique_ptr( new rtc::FakeSSLIdentity("User1")))), cert2_(rtc::RTCCertificate::Create(std::unique_ptr( - new rtc::FakeSSLIdentity("User2")))) {} + new rtc::FakeSSLIdentity("User2")))) { + // By default, certificates are supplied. + f1_.set_certificate(cert1_); + f2_.set_certificate(cert2_); + } void CheckDesc(const TransportDescription* desc, absl::string_view opt, @@ -75,7 +80,12 @@ class TransportDescriptionFactoryTest : public ::testing::Test { // in the offer and answer is changed. // If `dtls` is true, the test verifies that the finger print is not changed. void TestIceRestart(bool dtls) { - SetDtls(dtls); + if (dtls) { + f1_.set_certificate(cert1_); + f2_.set_certificate(cert2_); + } else { + SetInsecure(); + } cricket::TransportOptions options; // The initial offer / answer exchange. std::unique_ptr offer = @@ -102,6 +112,8 @@ class TransportDescriptionFactoryTest : public ::testing::Test { void VerifyUfragAndPasswordChanged(bool dtls, const TransportDescription* org_desc, const TransportDescription* restart_desc) { + ASSERT_THAT(org_desc, NotNull()); + ASSERT_THAT(restart_desc, NotNull()); EXPECT_NE(org_desc->ice_pwd, restart_desc->ice_pwd); EXPECT_NE(org_desc->ice_ufrag, restart_desc->ice_ufrag); EXPECT_EQ(static_cast(cricket::ICE_UFRAG_LENGTH), @@ -118,7 +130,9 @@ class TransportDescriptionFactoryTest : public ::testing::Test { } void TestIceRenomination(bool dtls) { - SetDtls(dtls); + if (!dtls) { + SetInsecureNoDtls(); + } cricket::TransportOptions options; // The initial offer / answer exchange. @@ -148,16 +162,16 @@ class TransportDescriptionFactoryTest : public ::testing::Test { } protected: - void SetDtls(bool dtls) { - if (dtls) { - f1_.set_secure(cricket::SEC_ENABLED); - f2_.set_secure(cricket::SEC_ENABLED); - f1_.set_certificate(cert1_); - f2_.set_certificate(cert2_); - } else { - f1_.set_secure(cricket::SEC_DISABLED); - f2_.set_secure(cricket::SEC_DISABLED); - } + // This will enable responding to non-DTLS requests. + void SetInsecure() { + f1_.SetInsecureForTesting(); + f2_.SetInsecureForTesting(); + } + // This will disable the ability to respond to DTLS requests. + void SetInsecureNoDtls() { + SetInsecure(); + f1_.set_certificate(nullptr); + f2_.set_certificate(nullptr); } webrtc::test::ScopedKeyValueConfig field_trials_; @@ -169,30 +183,18 @@ class TransportDescriptionFactoryTest : public ::testing::Test { rtc::scoped_refptr cert2_; }; -TEST_F(TransportDescriptionFactoryTest, TestOfferDefault) { - std::unique_ptr desc = - f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_); - CheckDesc(desc.get(), "", "", "", ""); -} - TEST_F(TransportDescriptionFactoryTest, TestOfferDtls) { - f1_.set_secure(cricket::SEC_ENABLED); - f1_.set_certificate(cert1_); std::string digest_alg; ASSERT_TRUE( cert1_->GetSSLCertificate().GetSignatureDigestAlgorithm(&digest_alg)); std::unique_ptr desc = f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_); CheckDesc(desc.get(), "", "", "", digest_alg); - // Ensure it also works with SEC_REQUIRED. - f1_.set_secure(cricket::SEC_REQUIRED); - desc = f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_); - CheckDesc(desc.get(), "", "", "", digest_alg); } // Test generating an offer with DTLS fails with no identity. TEST_F(TransportDescriptionFactoryTest, TestOfferDtlsWithNoIdentity) { - f1_.set_secure(cricket::SEC_ENABLED); + f1_.set_certificate(nullptr); std::unique_ptr desc = f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_); ASSERT_TRUE(desc.get() == NULL); @@ -201,8 +203,6 @@ TEST_F(TransportDescriptionFactoryTest, TestOfferDtlsWithNoIdentity) { // Test updating an offer with DTLS to pick ICE. // The ICE credentials should stay the same in the new offer. TEST_F(TransportDescriptionFactoryTest, TestOfferDtlsReofferDtls) { - f1_.set_secure(cricket::SEC_ENABLED); - f1_.set_certificate(cert1_); std::string digest_alg; ASSERT_TRUE( cert1_->GetSSLCertificate().GetSignatureDigestAlgorithm(&digest_alg)); @@ -215,19 +215,25 @@ TEST_F(TransportDescriptionFactoryTest, TestOfferDtlsReofferDtls) { } TEST_F(TransportDescriptionFactoryTest, TestAnswerDefault) { + std::string digest_alg; + ASSERT_TRUE( + cert1_->GetSSLCertificate().GetSignatureDigestAlgorithm(&digest_alg)); std::unique_ptr offer = f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_); ASSERT_TRUE(offer.get() != NULL); std::unique_ptr desc = f2_.CreateAnswer( offer.get(), TransportOptions(), true, NULL, &ice_credentials_); - CheckDesc(desc.get(), "", "", "", ""); + CheckDesc(desc.get(), "", "", "", digest_alg); desc = f2_.CreateAnswer(offer.get(), TransportOptions(), true, NULL, &ice_credentials_); - CheckDesc(desc.get(), "", "", "", ""); + CheckDesc(desc.get(), "", "", "", digest_alg); } // Test that we can update an answer properly; ICE credentials shouldn't change. TEST_F(TransportDescriptionFactoryTest, TestReanswer) { + std::string digest_alg; + ASSERT_TRUE( + cert1_->GetSSLCertificate().GetSignatureDigestAlgorithm(&digest_alg)); std::unique_ptr offer = f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_); ASSERT_TRUE(offer.get() != NULL); @@ -237,13 +243,13 @@ TEST_F(TransportDescriptionFactoryTest, TestReanswer) { std::unique_ptr desc = f2_.CreateAnswer( offer.get(), TransportOptions(), true, old_desc.get(), &ice_credentials_); ASSERT_TRUE(desc.get() != NULL); - CheckDesc(desc.get(), "", old_desc->ice_ufrag, old_desc->ice_pwd, ""); + CheckDesc(desc.get(), "", old_desc->ice_ufrag, old_desc->ice_pwd, digest_alg); } // Test that we handle answering an offer with DTLS with no DTLS. TEST_F(TransportDescriptionFactoryTest, TestAnswerDtlsToNoDtls) { - f1_.set_secure(cricket::SEC_ENABLED); - f1_.set_certificate(cert1_); + f2_.SetInsecureForTesting(); + f2_.set_certificate(nullptr); std::unique_ptr offer = f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_); ASSERT_TRUE(offer.get() != NULL); @@ -255,28 +261,25 @@ TEST_F(TransportDescriptionFactoryTest, TestAnswerDtlsToNoDtls) { // Test that we handle answering an offer without DTLS if we have DTLS enabled, // but fail if we require DTLS. TEST_F(TransportDescriptionFactoryTest, TestAnswerNoDtlsToDtls) { - f2_.set_secure(cricket::SEC_ENABLED); - f2_.set_certificate(cert2_); + f1_.SetInsecureForTesting(); + f1_.set_certificate(nullptr); std::unique_ptr offer = f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_); ASSERT_TRUE(offer.get() != NULL); + // Normal case. std::unique_ptr desc = f2_.CreateAnswer( offer.get(), TransportOptions(), true, NULL, &ice_credentials_); - CheckDesc(desc.get(), "", "", "", ""); - f2_.set_secure(cricket::SEC_REQUIRED); + ASSERT_TRUE(desc.get() == NULL); + // Insecure case. + f2_.SetInsecureForTesting(); desc = f2_.CreateAnswer(offer.get(), TransportOptions(), true, NULL, &ice_credentials_); - ASSERT_TRUE(desc.get() == NULL); + CheckDesc(desc.get(), "", "", "", ""); } -// Test that we handle answering an DTLS offer with DTLS, both if we have -// DTLS enabled and required. +// Test that we handle answering an DTLS offer with DTLS, +// even if we don't require DTLS. TEST_F(TransportDescriptionFactoryTest, TestAnswerDtlsToDtls) { - f1_.set_secure(cricket::SEC_ENABLED); - f1_.set_certificate(cert1_); - - f2_.set_secure(cricket::SEC_ENABLED); - f2_.set_certificate(cert2_); // f2_ produces the answer that is being checked in this test, so the // answer must contain fingerprint lines with cert2_'s digest algorithm. std::string digest_alg2; @@ -289,7 +292,8 @@ TEST_F(TransportDescriptionFactoryTest, TestAnswerDtlsToDtls) { std::unique_ptr desc = f2_.CreateAnswer( offer.get(), TransportOptions(), true, NULL, &ice_credentials_); CheckDesc(desc.get(), "", "", "", digest_alg2); - f2_.set_secure(cricket::SEC_REQUIRED); + + f2_.SetInsecureForTesting(); desc = f2_.CreateAnswer(offer.get(), TransportOptions(), true, NULL, &ice_credentials_); CheckDesc(desc.get(), "", "", "", digest_alg2); @@ -325,6 +329,7 @@ TEST_F(TransportDescriptionFactoryTest, AddsTrickleIceOption) { cricket::TransportOptions options; std::unique_ptr offer = f1_.CreateOffer(options, nullptr, &ice_credentials_); + ASSERT_THAT(offer, NotNull()); EXPECT_TRUE(offer->HasOption("trickle")); std::unique_ptr answer = f2_.CreateAnswer(offer.get(), options, true, nullptr, &ice_credentials_); @@ -359,11 +364,6 @@ TEST_F(TransportDescriptionFactoryTest, CreateAnswerIceCredentialsIterator) { } TEST_F(TransportDescriptionFactoryTest, CreateAnswerToDtlsActpassOffer) { - f1_.set_secure(cricket::SEC_ENABLED); - f1_.set_certificate(cert1_); - - f2_.set_secure(cricket::SEC_ENABLED); - f2_.set_certificate(cert2_); cricket::TransportOptions options; std::unique_ptr offer = f1_.CreateOffer(options, nullptr, &ice_credentials_); @@ -374,11 +374,6 @@ TEST_F(TransportDescriptionFactoryTest, CreateAnswerToDtlsActpassOffer) { } TEST_F(TransportDescriptionFactoryTest, CreateAnswerToDtlsActiveOffer) { - f1_.set_secure(cricket::SEC_ENABLED); - f1_.set_certificate(cert1_); - - f2_.set_secure(cricket::SEC_ENABLED); - f2_.set_certificate(cert2_); cricket::TransportOptions options; std::unique_ptr offer = f1_.CreateOffer(options, nullptr, &ice_credentials_); @@ -390,11 +385,6 @@ TEST_F(TransportDescriptionFactoryTest, CreateAnswerToDtlsActiveOffer) { } TEST_F(TransportDescriptionFactoryTest, CreateAnswerToDtlsPassiveOffer) { - f1_.set_secure(cricket::SEC_ENABLED); - f1_.set_certificate(cert1_); - - f2_.set_secure(cricket::SEC_ENABLED); - f2_.set_certificate(cert2_); cricket::TransportOptions options; std::unique_ptr offer = f1_.CreateOffer(options, nullptr, &ice_credentials_); diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 14971b8af1..6f21d00bc9 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -218,7 +218,6 @@ rtc_source_set("jsep_transport") { ":rtp_transport_internal", ":sctp_transport", ":session_description", - ":srtp_filter", ":srtp_transport", ":transport_stats", "../api:array_view", @@ -573,29 +572,6 @@ rtc_source_set("sctp_utils") { ] absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ] } -rtc_source_set("srtp_filter") { - visibility = [ ":*" ] - sources = [ - "srtp_filter.cc", - "srtp_filter.h", - ] - deps = [ - ":session_description", - "../api:array_view", - "../api:libjingle_peerconnection_api", - "../api:sequence_checker", - "../rtc_base:buffer", - "../rtc_base:logging", - "../rtc_base:ssl", - "../rtc_base:zero_memory", - "../rtc_base/third_party/base64", - ] - absl_deps = [ - "//third_party/abseil-cpp/absl/strings", - "//third_party/abseil-cpp/absl/types:optional", - ] -} - rtc_source_set("srtp_session") { visibility = [ ":*" ] sources = [ @@ -2048,7 +2024,6 @@ if (rtc_include_tests && !build_with_chromium) { "rtp_transport_unittest.cc", "sctp_transport_unittest.cc", "session_description_unittest.cc", - "srtp_filter_unittest.cc", "srtp_session_unittest.cc", "srtp_transport_unittest.cc", "test/rtp_transport_test_util.h", @@ -2083,7 +2058,6 @@ if (rtc_include_tests && !build_with_chromium) { ":rtp_transport_internal", ":sctp_transport", ":session_description", - ":srtp_filter", ":srtp_session", ":srtp_transport", ":used_ids", diff --git a/pc/dtls_srtp_transport.h b/pc/dtls_srtp_transport.h index 995809ed4b..654eb2f628 100644 --- a/pc/dtls_srtp_transport.h +++ b/pc/dtls_srtp_transport.h @@ -16,7 +16,6 @@ #include #include "absl/types/optional.h" -#include "api/crypto_params.h" #include "api/dtls_transport_interface.h" #include "api/rtc_error.h" #include "p2p/base/dtls_transport_internal.h" diff --git a/pc/jsep_transport.cc b/pc/jsep_transport.cc index 2398a0ad2d..faff4e8cf4 100644 --- a/pc/jsep_transport.cc +++ b/pc/jsep_transport.cc @@ -36,12 +36,10 @@ JsepTransportDescription::JsepTransportDescription() {} JsepTransportDescription::JsepTransportDescription( bool rtcp_mux_enabled, - const std::vector& cryptos, const std::vector& encrypted_header_extension_ids, int rtp_abs_sendtime_extn_id, const TransportDescription& transport_desc) : rtcp_mux_enabled(rtcp_mux_enabled), - cryptos(cryptos), encrypted_header_extension_ids(encrypted_header_extension_ids), rtp_abs_sendtime_extn_id(rtp_abs_sendtime_extn_id), transport_desc(transport_desc) {} @@ -49,7 +47,6 @@ JsepTransportDescription::JsepTransportDescription( JsepTransportDescription::JsepTransportDescription( const JsepTransportDescription& from) : rtcp_mux_enabled(from.rtcp_mux_enabled), - cryptos(from.cryptos), encrypted_header_extension_ids(from.encrypted_header_extension_ids), rtp_abs_sendtime_extn_id(from.rtp_abs_sendtime_extn_id), transport_desc(from.transport_desc) {} @@ -62,7 +59,6 @@ JsepTransportDescription& JsepTransportDescription::operator=( return *this; } rtcp_mux_enabled = from.rtcp_mux_enabled; - cryptos = from.cryptos; encrypted_header_extension_ids = from.encrypted_header_extension_ids; rtp_abs_sendtime_extn_id = from.rtp_abs_sendtime_extn_id; transport_desc = from.transport_desc; @@ -167,17 +163,7 @@ webrtc::RTCError JsepTransport::SetLocalJsepTransportDescription( "Failed to setup RTCP mux."); } - // If doing SDES, setup the SDES crypto parameters. - if (sdes_transport_) { - RTC_DCHECK(!unencrypted_rtp_transport_); - RTC_DCHECK(!dtls_srtp_transport_); - if (!SetSdes(jsep_description.cryptos, - jsep_description.encrypted_header_extension_ids, type, - ContentSource::CS_LOCAL)) { - return webrtc::RTCError(webrtc::RTCErrorType::INVALID_PARAMETER, - "Failed to setup SDES crypto parameters."); - } - } else if (dtls_srtp_transport_) { + if (dtls_srtp_transport_) { RTC_DCHECK(!unencrypted_rtp_transport_); RTC_DCHECK(!sdes_transport_); dtls_srtp_transport_->UpdateRecvEncryptedHeaderExtensionIds( @@ -254,19 +240,7 @@ webrtc::RTCError JsepTransport::SetRemoteJsepTransportDescription( "Failed to setup RTCP mux."); } - // If doing SDES, setup the SDES crypto parameters. - if (sdes_transport_) { - RTC_DCHECK(!unencrypted_rtp_transport_); - RTC_DCHECK(!dtls_srtp_transport_); - if (!SetSdes(jsep_description.cryptos, - jsep_description.encrypted_header_extension_ids, type, - ContentSource::CS_REMOTE)) { - return webrtc::RTCError(webrtc::RTCErrorType::INVALID_PARAMETER, - "Failed to setup SDES crypto parameters."); - } - sdes_transport_->CacheRtpAbsSendTimeHeaderExtension( - jsep_description.rtp_abs_sendtime_extn_id); - } else if (dtls_srtp_transport_) { + if (dtls_srtp_transport_) { RTC_DCHECK(!unencrypted_rtp_transport_); RTC_DCHECK(!sdes_transport_); dtls_srtp_transport_->UpdateSendEncryptedHeaderExtensionIds( @@ -474,51 +448,6 @@ void JsepTransport::ActivateRtcpMux() { rtcp_mux_active_callback_(); } -bool JsepTransport::SetSdes(const std::vector& cryptos, - const std::vector& encrypted_extension_ids, - webrtc::SdpType type, - ContentSource source) { - RTC_DCHECK_RUN_ON(network_thread_); - bool ret = false; - ret = sdes_negotiator_.Process(cryptos, type, source); - if (!ret) { - return ret; - } - - if (source == ContentSource::CS_LOCAL) { - recv_extension_ids_ = encrypted_extension_ids; - } else { - send_extension_ids_ = encrypted_extension_ids; - } - - // If setting an SDES answer succeeded, apply the negotiated parameters - // to the SRTP transport. - if ((type == SdpType::kPrAnswer || type == SdpType::kAnswer) && ret) { - if (sdes_negotiator_.send_crypto_suite() && - sdes_negotiator_.recv_crypto_suite()) { - RTC_DCHECK(send_extension_ids_); - RTC_DCHECK(recv_extension_ids_); - ret = sdes_transport_->SetRtpParams( - *(sdes_negotiator_.send_crypto_suite()), - sdes_negotiator_.send_key().data(), - static_cast(sdes_negotiator_.send_key().size()), - *(send_extension_ids_), *(sdes_negotiator_.recv_crypto_suite()), - sdes_negotiator_.recv_key().data(), - static_cast(sdes_negotiator_.recv_key().size()), - *(recv_extension_ids_)); - } else { - RTC_LOG(LS_INFO) << "No crypto keys are provided for SDES."; - if (type == SdpType::kAnswer) { - // Explicitly reset the `sdes_transport_` if no crypto param is - // provided in the answer. No need to call `ResetParams()` for - // `sdes_negotiator_` because it resets the params inside `SetAnswer`. - sdes_transport_->ResetParams(); - } - } - } - return ret; -} - webrtc::RTCError JsepTransport::NegotiateAndSetDtlsParameters( SdpType local_description_type) { RTC_DCHECK_RUN_ON(network_thread_); diff --git a/pc/jsep_transport.h b/pc/jsep_transport.h index f2643070a1..af0c797fc8 100644 --- a/pc/jsep_transport.h +++ b/pc/jsep_transport.h @@ -19,7 +19,6 @@ #include "absl/types/optional.h" #include "api/candidate.h" -#include "api/crypto_params.h" #include "api/ice_transport_interface.h" #include "api/jsep.h" #include "api/rtc_error.h" @@ -40,7 +39,6 @@ #include "pc/rtp_transport_internal.h" #include "pc/sctp_transport.h" #include "pc/session_description.h" -#include "pc/srtp_filter.h" #include "pc/srtp_transport.h" #include "pc/transport_stats.h" #include "rtc_base/checks.h" @@ -59,7 +57,6 @@ struct JsepTransportDescription { JsepTransportDescription(); JsepTransportDescription( bool rtcp_mux_enabled, - const std::vector& cryptos, const std::vector& encrypted_header_extension_ids, int rtp_abs_sendtime_extn_id, const TransportDescription& transport_description); @@ -69,7 +66,6 @@ struct JsepTransportDescription { JsepTransportDescription& operator=(const JsepTransportDescription& from); bool rtcp_mux_enabled = true; - std::vector cryptos; std::vector encrypted_header_extension_ids; int rtp_abs_sendtime_extn_id = -1; // TODO(zhihuang): Add the ICE and DTLS related variables and methods from @@ -243,11 +239,6 @@ class JsepTransport { void ActivateRtcpMux() RTC_RUN_ON(network_thread_); - bool SetSdes(const std::vector& cryptos, - const std::vector& encrypted_extension_ids, - webrtc::SdpType type, - ContentSource source); - // Negotiates and sets the DTLS parameters based on the current local and // remote transport description, such as the DTLS role to use, and whether // DTLS should be activated. @@ -310,7 +301,6 @@ class JsepTransport { const rtc::scoped_refptr sctp_transport_; - SrtpFilter sdes_negotiator_ RTC_GUARDED_BY(network_thread_); RtcpMuxFilter rtcp_mux_negotiator_ RTC_GUARDED_BY(network_thread_); // Cache the encrypted header extension IDs for SDES negoitation. diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index ff0abc0c57..9473d962ef 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -951,8 +951,8 @@ JsepTransportController::CreateJsepTransportDescription( : content_desc->rtcp_mux(); return cricket::JsepTransportDescription( - rtcp_mux_enabled, content_desc->cryptos(), encrypted_extension_ids, - rtp_abs_sendtime_extn_id, transport_info.description); + rtcp_mux_enabled, encrypted_extension_ids, rtp_abs_sendtime_extn_id, + transport_info.description); } std::vector JsepTransportController::GetEncryptedHeaderExtensionIds( @@ -1058,12 +1058,6 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( if (transport) { return RTCError::OK(); } - const cricket::MediaContentDescription* content_desc = - content_info.media_description(); - if (certificate_ && !content_desc->cryptos().empty()) { - return RTCError(RTCErrorType::INVALID_PARAMETER, - "SDES and DTLS-SRTP cannot be enabled at the same time."); - } rtc::scoped_refptr ice = CreateIceTransport(content_info.name, /*rtcp=*/false); @@ -1090,10 +1084,6 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( << "Creating UnencryptedRtpTransport, becayse encryption is disabled."; unencrypted_rtp_transport = CreateUnencryptedRtpTransport( content_info.name, rtp_dtls_transport.get(), rtcp_dtls_transport.get()); - } else if (!content_desc->cryptos().empty()) { - sdes_transport = CreateSdesTransport( - content_info.name, rtp_dtls_transport.get(), rtcp_dtls_transport.get()); - RTC_LOG(LS_INFO) << "Creating SdesTransport."; } else { RTC_LOG(LS_INFO) << "Creating DtlsSrtpTransport."; dtls_srtp_transport = CreateDtlsSrtpTransport( diff --git a/pc/jsep_transport_unittest.cc b/pc/jsep_transport_unittest.cc index f057d37a0d..18dd2d8896 100644 --- a/pc/jsep_transport_unittest.cc +++ b/pc/jsep_transport_unittest.cc @@ -48,11 +48,6 @@ static const char kIceUfrag2[] = "U002"; static const char kIcePwd2[] = "TESTIEPWD00000000000002"; static const char kTransportName[] = "Test Transport"; -enum class SrtpMode { - kSdes, - kDtlsSrtp, -}; - struct NegotiateRoleParams { ConnectionRole local_role; ConnectionRole remote_role; @@ -110,8 +105,7 @@ class JsepTransport2Test : public ::testing::Test, public sigslot::has_slots<> { // Create a new JsepTransport with a FakeDtlsTransport and a // FakeIceTransport. - std::unique_ptr CreateJsepTransport2(bool rtcp_mux_enabled, - SrtpMode srtp_mode) { + std::unique_ptr CreateJsepTransport2(bool rtcp_mux_enabled) { auto ice_internal = std::make_unique( kTransportName, ICE_CANDIDATE_COMPONENT_RTP); auto rtp_dtls_transport = @@ -131,19 +125,8 @@ class JsepTransport2Test : public ::testing::Test, public sigslot::has_slots<> { std::unique_ptr unencrypted_rtp_transport; std::unique_ptr sdes_transport; std::unique_ptr dtls_srtp_transport; - switch (srtp_mode) { - case SrtpMode::kSdes: - sdes_transport = CreateSdesTransport(rtp_dtls_transport.get(), - rtcp_dtls_transport.get()); - sdes_transport_ = sdes_transport.get(); - break; - case SrtpMode::kDtlsSrtp: dtls_srtp_transport = CreateDtlsSrtpTransport( rtp_dtls_transport.get(), rtcp_dtls_transport.get()); - break; - default: - RTC_DCHECK_NOTREACHED(); - } auto jsep_transport = std::make_unique( kTransportName, /*local_certificate=*/nullptr, std::move(ice), @@ -205,7 +188,7 @@ class JsepTransport2WithRtcpMux : public JsepTransport2Test, // This test verifies the ICE parameters are properly applied to the transports. TEST_P(JsepTransport2WithRtcpMux, SetIceParameters) { bool rtcp_mux_enabled = GetParam(); - jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled, SrtpMode::kDtlsSrtp); + jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled); JsepTransportDescription jsep_description; jsep_description.transport_desc = TransportDescription(kIceUfrag1, kIcePwd1); @@ -251,7 +234,7 @@ TEST_P(JsepTransport2WithRtcpMux, SetIceParameters) { // Similarly, test DTLS parameters are properly applied to the transports. TEST_P(JsepTransport2WithRtcpMux, SetDtlsParameters) { bool rtcp_mux_enabled = GetParam(); - jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled, SrtpMode::kDtlsSrtp); + jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled); // Create certificates. rtc::scoped_refptr local_cert = @@ -302,7 +285,7 @@ TEST_P(JsepTransport2WithRtcpMux, SetDtlsParameters) { // CONNECTIONROLE_PASSIVE, expecting SSL_CLIENT role. TEST_P(JsepTransport2WithRtcpMux, SetDtlsParametersWithPassiveAnswer) { bool rtcp_mux_enabled = GetParam(); - jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled, SrtpMode::kDtlsSrtp); + jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled); // Create certificates. rtc::scoped_refptr local_cert = @@ -354,7 +337,7 @@ TEST_P(JsepTransport2WithRtcpMux, SetDtlsParametersWithPassiveAnswer) { // only starts returning "false" once an ICE restart has been initiated. TEST_P(JsepTransport2WithRtcpMux, NeedsIceRestart) { bool rtcp_mux_enabled = GetParam(); - jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled, SrtpMode::kDtlsSrtp); + jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled); // Use the same JsepTransportDescription for both offer and answer. JsepTransportDescription description; @@ -399,7 +382,7 @@ TEST_P(JsepTransport2WithRtcpMux, NeedsIceRestart) { TEST_P(JsepTransport2WithRtcpMux, GetStats) { bool rtcp_mux_enabled = GetParam(); - jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled, SrtpMode::kDtlsSrtp); + jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled); size_t expected_stats_size = rtcp_mux_enabled ? 1u : 2u; TransportStats stats; @@ -415,7 +398,7 @@ TEST_P(JsepTransport2WithRtcpMux, GetStats) { // certificate matches the fingerprint. TEST_P(JsepTransport2WithRtcpMux, VerifyCertificateFingerprint) { bool rtcp_mux_enabled = GetParam(); - jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled, SrtpMode::kDtlsSrtp); + jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled); EXPECT_FALSE( jsep_transport_->VerifyCertificateFingerprint(nullptr, nullptr).ok()); @@ -489,8 +472,7 @@ TEST_P(JsepTransport2WithRtcpMux, ValidDtlsRoleNegotiation) { }; for (auto& param : valid_client_params) { - jsep_transport_ = - CreateJsepTransport2(rtcp_mux_enabled, SrtpMode::kDtlsSrtp); + jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled); jsep_transport_->SetLocalCertificate(certificate); local_description.transport_desc.connection_role = param.local_role; @@ -535,8 +517,7 @@ TEST_P(JsepTransport2WithRtcpMux, ValidDtlsRoleNegotiation) { }; for (auto& param : valid_server_params) { - jsep_transport_ = - CreateJsepTransport2(rtcp_mux_enabled, SrtpMode::kDtlsSrtp); + jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled); jsep_transport_->SetLocalCertificate(certificate); local_description.transport_desc.connection_role = param.local_role; @@ -607,8 +588,7 @@ TEST_P(JsepTransport2WithRtcpMux, InvalidDtlsRoleNegotiation) { SdpType::kPrAnswer}}; for (auto& param : duplicate_params) { - jsep_transport_ = - CreateJsepTransport2(rtcp_mux_enabled, SrtpMode::kDtlsSrtp); + jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled); jsep_transport_->SetLocalCertificate(certificate); local_description.transport_desc.connection_role = param.local_role; @@ -658,8 +638,7 @@ TEST_P(JsepTransport2WithRtcpMux, InvalidDtlsRoleNegotiation) { SdpType::kPrAnswer}}; for (auto& param : offerer_without_actpass_params) { - jsep_transport_ = - CreateJsepTransport2(rtcp_mux_enabled, SrtpMode::kDtlsSrtp); + jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled); jsep_transport_->SetLocalCertificate(certificate); local_description.transport_desc.connection_role = param.local_role; @@ -705,7 +684,7 @@ TEST_F(JsepTransport2Test, ValidDtlsReofferFromAnswerer) { rtc::RTCCertificate::Create( rtc::SSLIdentity::Create("testing", rtc::KT_ECDSA)); bool rtcp_mux_enabled = true; - jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled, SrtpMode::kDtlsSrtp); + jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled); jsep_transport_->SetLocalCertificate(certificate); JsepTransportDescription local_offer = @@ -752,7 +731,7 @@ TEST_F(JsepTransport2Test, InvalidDtlsReofferFromAnswerer) { rtc::RTCCertificate::Create( rtc::SSLIdentity::Create("testing", rtc::KT_ECDSA)); bool rtcp_mux_enabled = true; - jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled, SrtpMode::kDtlsSrtp); + jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled); jsep_transport_->SetLocalCertificate(certificate); JsepTransportDescription local_offer = @@ -798,7 +777,7 @@ TEST_F(JsepTransport2Test, RemoteOfferWithCurrentNegotiatedDtlsRole) { rtc::RTCCertificate::Create( rtc::SSLIdentity::Create("testing", rtc::KT_ECDSA)); bool rtcp_mux_enabled = true; - jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled, SrtpMode::kDtlsSrtp); + jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled); jsep_transport_->SetLocalCertificate(certificate); JsepTransportDescription remote_desc = @@ -843,7 +822,7 @@ TEST_F(JsepTransport2Test, RemoteOfferThatChangesNegotiatedDtlsRole) { rtc::RTCCertificate::Create( rtc::SSLIdentity::Create("testing", rtc::KT_ECDSA)); bool rtcp_mux_enabled = true; - jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled, SrtpMode::kDtlsSrtp); + jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled); jsep_transport_->SetLocalCertificate(certificate); JsepTransportDescription remote_desc = @@ -890,7 +869,7 @@ TEST_F(JsepTransport2Test, RemoteOfferThatChangesFingerprintAndDtlsRole) { rtc::RTCCertificate::Create( rtc::SSLIdentity::Create("testing2", rtc::KT_ECDSA)); bool rtcp_mux_enabled = true; - jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled, SrtpMode::kDtlsSrtp); + jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled); jsep_transport_->SetLocalCertificate(certificate); JsepTransportDescription remote_desc = @@ -943,7 +922,7 @@ TEST_F(JsepTransport2Test, DtlsSetupWithLegacyAsAnswerer) { rtc::RTCCertificate::Create( rtc::SSLIdentity::Create("testing", rtc::KT_ECDSA)); bool rtcp_mux_enabled = true; - jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled, SrtpMode::kDtlsSrtp); + jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled); jsep_transport_->SetLocalCertificate(certificate); JsepTransportDescription remote_desc = @@ -975,8 +954,7 @@ TEST_F(JsepTransport2Test, DtlsSetupWithLegacyAsAnswerer) { // Tests that when the RTCP mux is successfully negotiated, the RTCP transport // will be destroyed and the SignalRtpMuxActive will be fired. TEST_F(JsepTransport2Test, RtcpMuxNegotiation) { - jsep_transport_ = - CreateJsepTransport2(/*rtcp_mux_enabled=*/false, SrtpMode::kDtlsSrtp); + jsep_transport_ = CreateJsepTransport2(/*rtcp_mux_enabled=*/false); JsepTransportDescription local_desc; local_desc.rtcp_mux_enabled = true; ASSERT_NE(nullptr, jsep_transport_->rtcp_dtls_transport()); @@ -998,8 +976,7 @@ TEST_F(JsepTransport2Test, RtcpMuxNegotiation) { EXPECT_TRUE(signal_rtcp_mux_active_received_); // The remote side doesn't support RTCP-mux. - jsep_transport_ = - CreateJsepTransport2(/*rtcp_mux_enabled=*/false, SrtpMode::kDtlsSrtp); + jsep_transport_ = CreateJsepTransport2(/*rtcp_mux_enabled=*/false); signal_rtcp_mux_active_received_ = false; remote_desc.rtcp_mux_enabled = false; ASSERT_TRUE( @@ -1015,87 +992,10 @@ TEST_F(JsepTransport2Test, RtcpMuxNegotiation) { EXPECT_FALSE(signal_rtcp_mux_active_received_); } -TEST_F(JsepTransport2Test, SdesNegotiation) { - jsep_transport_ = - CreateJsepTransport2(/*rtcp_mux_enabled=*/true, SrtpMode::kSdes); - ASSERT_TRUE(sdes_transport_); - EXPECT_FALSE(sdes_transport_->IsSrtpActive()); - - JsepTransportDescription offer_desc; - offer_desc.cryptos.push_back(cricket::CryptoParams( - 1, rtc::kCsAesCm128HmacSha1_32, "inline:" + rtc::CreateRandomString(40), - std::string())); - ASSERT_TRUE( - jsep_transport_ - ->SetLocalJsepTransportDescription(offer_desc, SdpType::kOffer) - .ok()); - - JsepTransportDescription answer_desc; - answer_desc.cryptos.push_back(cricket::CryptoParams( - 1, rtc::kCsAesCm128HmacSha1_32, "inline:" + rtc::CreateRandomString(40), - std::string())); - ASSERT_TRUE( - jsep_transport_ - ->SetRemoteJsepTransportDescription(answer_desc, SdpType::kAnswer) - .ok()); - EXPECT_TRUE(sdes_transport_->IsSrtpActive()); -} - -TEST_F(JsepTransport2Test, SdesNegotiationWithEmptyCryptosInAnswer) { - jsep_transport_ = - CreateJsepTransport2(/*rtcp_mux_enabled=*/true, SrtpMode::kSdes); - ASSERT_TRUE(sdes_transport_); - EXPECT_FALSE(sdes_transport_->IsSrtpActive()); - - JsepTransportDescription offer_desc; - offer_desc.cryptos.push_back(cricket::CryptoParams( - 1, rtc::kCsAesCm128HmacSha1_32, "inline:" + rtc::CreateRandomString(40), - std::string())); - ASSERT_TRUE( - jsep_transport_ - ->SetLocalJsepTransportDescription(offer_desc, SdpType::kOffer) - .ok()); - - JsepTransportDescription answer_desc; - ASSERT_TRUE( - jsep_transport_ - ->SetRemoteJsepTransportDescription(answer_desc, SdpType::kAnswer) - .ok()); - // SRTP is not active because the crypto parameter is answer is empty. - EXPECT_FALSE(sdes_transport_->IsSrtpActive()); -} - -TEST_F(JsepTransport2Test, SdesNegotiationWithMismatchedCryptos) { - jsep_transport_ = - CreateJsepTransport2(/*rtcp_mux_enabled=*/true, SrtpMode::kSdes); - ASSERT_TRUE(sdes_transport_); - EXPECT_FALSE(sdes_transport_->IsSrtpActive()); - - JsepTransportDescription offer_desc; - offer_desc.cryptos.push_back(cricket::CryptoParams( - 1, rtc::kCsAesCm128HmacSha1_32, "inline:" + rtc::CreateRandomString(40), - std::string())); - ASSERT_TRUE( - jsep_transport_ - ->SetLocalJsepTransportDescription(offer_desc, SdpType::kOffer) - .ok()); - - JsepTransportDescription answer_desc; - answer_desc.cryptos.push_back(cricket::CryptoParams( - 1, rtc::kCsAesCm128HmacSha1_80, "inline:" + rtc::CreateRandomString(40), - std::string())); - // Expected to fail because the crypto parameters don't match. - ASSERT_FALSE( - jsep_transport_ - ->SetRemoteJsepTransportDescription(answer_desc, SdpType::kAnswer) - .ok()); -} - // Tests that the remote candidates can be added to the transports after both // local and remote descriptions are set. TEST_F(JsepTransport2Test, AddRemoteCandidates) { - jsep_transport_ = - CreateJsepTransport2(/*rtcp_mux_enabled=*/true, SrtpMode::kDtlsSrtp); + jsep_transport_ = CreateJsepTransport2(/*rtcp_mux_enabled=*/true); auto fake_ice_transport = static_cast( jsep_transport_->rtp_dtls_transport()->ice_transport()); @@ -1119,7 +1019,6 @@ TEST_F(JsepTransport2Test, AddRemoteCandidates) { } enum class Scenario { - kSdes, kDtlsBeforeCallerSendOffer, kDtlsBeforeCallerSetAnswer, kDtlsAfterCallerSetAnswer, @@ -1131,9 +1030,9 @@ class JsepTransport2HeaderExtensionTest protected: JsepTransport2HeaderExtensionTest() {} - void CreateJsepTransportPair(SrtpMode mode) { - jsep_transport1_ = CreateJsepTransport2(/*rtcp_mux_enabled=*/true, mode); - jsep_transport2_ = CreateJsepTransport2(/*rtcp_mux_enabled=*/true, mode); + void CreateJsepTransportPair() { + jsep_transport1_ = CreateJsepTransport2(/*rtcp_mux_enabled=*/true); + jsep_transport2_ = CreateJsepTransport2(/*rtcp_mux_enabled=*/true); auto fake_dtls1 = static_cast(jsep_transport1_->rtp_dtls_transport()); @@ -1145,14 +1044,12 @@ class JsepTransport2HeaderExtensionTest fake_dtls2->fake_ice_transport()->SignalReadPacket.connect( this, &JsepTransport2HeaderExtensionTest::OnReadPacket2); - if (mode == SrtpMode::kDtlsSrtp) { auto cert1 = rtc::RTCCertificate::Create( rtc::SSLIdentity::Create("session1", rtc::KT_DEFAULT)); jsep_transport1_->rtp_dtls_transport()->SetLocalCertificate(cert1); auto cert2 = rtc::RTCCertificate::Create( rtc::SSLIdentity::Create("session1", rtc::KT_DEFAULT)); jsep_transport2_->rtp_dtls_transport()->SetLocalCertificate(cert2); - } } void OnReadPacket1(rtc::PacketTransportInternal* transport, @@ -1239,17 +1136,10 @@ class JsepTransport2HeaderExtensionTest TEST_P(JsepTransport2HeaderExtensionTest, EncryptedHeaderExtensionNegotiation) { Scenario scenario = std::get<0>(GetParam()); bool use_gcm = std::get<1>(GetParam()); - SrtpMode mode = SrtpMode ::kDtlsSrtp; - if (scenario == Scenario::kSdes) { - mode = SrtpMode::kSdes; - } - CreateJsepTransportPair(mode); + CreateJsepTransportPair(); recv_encrypted_headers1_.push_back(kHeaderExtensionIDs[0]); recv_encrypted_headers2_.push_back(kHeaderExtensionIDs[1]); - cricket::CryptoParams sdes_param(1, rtc::kCsAesCm128HmacSha1_80, - "inline:" + rtc::CreateRandomString(40), - std::string()); if (use_gcm) { auto fake_dtls1 = static_cast(jsep_transport1_->rtp_dtls_transport()); @@ -1266,9 +1156,6 @@ TEST_P(JsepTransport2HeaderExtensionTest, EncryptedHeaderExtensionNegotiation) { JsepTransportDescription offer_desc; offer_desc.encrypted_header_extension_ids = recv_encrypted_headers1_; - if (scenario == Scenario::kSdes) { - offer_desc.cryptos.push_back(sdes_param); - } ASSERT_TRUE( jsep_transport1_ ->SetLocalJsepTransportDescription(offer_desc, SdpType::kOffer) @@ -1280,9 +1167,6 @@ TEST_P(JsepTransport2HeaderExtensionTest, EncryptedHeaderExtensionNegotiation) { JsepTransportDescription answer_desc; answer_desc.encrypted_header_extension_ids = recv_encrypted_headers2_; - if (scenario == Scenario::kSdes) { - answer_desc.cryptos.push_back(sdes_param); - } ASSERT_TRUE( jsep_transport2_ ->SetLocalJsepTransportDescription(answer_desc, SdpType::kAnswer) @@ -1301,8 +1185,7 @@ TEST_P(JsepTransport2HeaderExtensionTest, EncryptedHeaderExtensionNegotiation) { ->SetRemoteJsepTransportDescription(answer_desc, SdpType::kAnswer) .ok()); - if (scenario == Scenario::kDtlsAfterCallerSetAnswer || - scenario == Scenario::kSdes) { + if (scenario == Scenario::kDtlsAfterCallerSetAnswer) { ConnectTransport(); } EXPECT_TRUE(jsep_transport1_->rtp_transport()->IsSrtpActive()); @@ -1341,7 +1224,6 @@ INSTANTIATE_TEST_SUITE_P( JsepTransport2Test, JsepTransport2HeaderExtensionTest, ::testing::Values( - std::make_tuple(Scenario::kSdes, false), std::make_tuple(Scenario::kDtlsBeforeCallerSendOffer, true), std::make_tuple(Scenario::kDtlsBeforeCallerSetAnswer, true), std::make_tuple(Scenario::kDtlsAfterCallerSetAnswer, true), @@ -1351,8 +1233,7 @@ INSTANTIATE_TEST_SUITE_P( // This test verifies the ICE parameters are properly applied to the transports. TEST_F(JsepTransport2Test, SetIceParametersWithRenomination) { - jsep_transport_ = - CreateJsepTransport2(/* rtcp_mux_enabled= */ true, SrtpMode::kDtlsSrtp); + jsep_transport_ = CreateJsepTransport2(/* rtcp_mux_enabled= */ true); JsepTransportDescription jsep_description; jsep_description.transport_desc = TransportDescription(kIceUfrag1, kIcePwd1); diff --git a/pc/media_session.cc b/pc/media_session.cc index f146562b37..4f63f3088f 100644 --- a/pc/media_session.cc +++ b/pc/media_session.cc @@ -22,7 +22,6 @@ #include "absl/strings/match.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" -#include "api/crypto_params.h" #include "media/base/codec.h" #include "media/base/media_constants.h" #include "media/base/media_engine.h" @@ -47,19 +46,6 @@ using webrtc::RTCError; using webrtc::RTCErrorType; using webrtc::RtpTransceiverDirection; -const char kInline[] = "inline:"; - -void GetSupportedSdesCryptoSuiteNames( - void (*func)(const webrtc::CryptoOptions&, std::vector*), - const webrtc::CryptoOptions& crypto_options, - std::vector* names) { - std::vector crypto_suites; - func(crypto_options, &crypto_suites); - for (const auto crypto : crypto_suites) { - names->push_back(rtc::SrtpCryptoSuiteToName(crypto)); - } -} - webrtc::RtpExtension RtpExtensionFromCapability( const webrtc::RtpHeaderExtensionCapability& capability) { return webrtc::RtpExtension(capability.uri, @@ -167,140 +153,6 @@ bool IsMediaContentOfType(const ContentInfo* content, MediaType media_type) { return content->media_description()->type() == media_type; } -bool CreateCryptoParams(int tag, - const std::string& cipher, - CryptoParams* crypto_out) { - int key_len; - int salt_len; - if (!rtc::GetSrtpKeyAndSaltLengths(rtc::SrtpCryptoSuiteFromName(cipher), - &key_len, &salt_len)) { - return false; - } - - int master_key_len = key_len + salt_len; - std::string master_key; - if (!rtc::CreateRandomData(master_key_len, &master_key)) { - return false; - } - - RTC_CHECK_EQ(master_key_len, master_key.size()); - std::string key = rtc::Base64::Encode(master_key); - - crypto_out->tag = tag; - crypto_out->crypto_suite = cipher; - crypto_out->key_params = kInline; - crypto_out->key_params += key; - return true; -} - -bool AddCryptoParams(const std::string& crypto_suite, - CryptoParamsVec* cryptos_out) { - int size = static_cast(cryptos_out->size()); - - cryptos_out->resize(size + 1); - return CreateCryptoParams(size, crypto_suite, &cryptos_out->at(size)); -} - -void AddMediaCryptos(const CryptoParamsVec& cryptos, - MediaContentDescription* media) { - for (const CryptoParams& crypto : cryptos) { - media->AddCrypto(crypto); - } -} - -bool CreateMediaCryptos(const std::vector& crypto_suites, - MediaContentDescription* media) { - CryptoParamsVec cryptos; - for (const std::string& crypto_suite : crypto_suites) { - if (!AddCryptoParams(crypto_suite, &cryptos)) { - return false; - } - } - AddMediaCryptos(cryptos, media); - return true; -} - -const CryptoParamsVec* GetCryptos(const ContentInfo* content) { - if (!content || !content->media_description()) { - return nullptr; - } - return &content->media_description()->cryptos(); -} - -bool FindMatchingCrypto(const CryptoParamsVec& cryptos, - const CryptoParams& crypto, - CryptoParams* crypto_out) { - auto it = absl::c_find_if( - cryptos, [&crypto](const CryptoParams& c) { return crypto.Matches(c); }); - if (it == cryptos.end()) { - return false; - } - *crypto_out = *it; - return true; -} - -// For audio, HMAC 32 (if enabled) is prefered over HMAC 80 because of the -// low overhead. -void GetSupportedAudioSdesCryptoSuites( - const webrtc::CryptoOptions& crypto_options, - std::vector* crypto_suites) { - if (crypto_options.srtp.enable_aes128_sha1_32_crypto_cipher) { - crypto_suites->push_back(rtc::kSrtpAes128CmSha1_32); - } - crypto_suites->push_back(rtc::kSrtpAes128CmSha1_80); - if (crypto_options.srtp.enable_gcm_crypto_suites) { - crypto_suites->push_back(rtc::kSrtpAeadAes256Gcm); - crypto_suites->push_back(rtc::kSrtpAeadAes128Gcm); - } -} - -void GetSupportedAudioSdesCryptoSuiteNames( - const webrtc::CryptoOptions& crypto_options, - std::vector* crypto_suite_names) { - GetSupportedSdesCryptoSuiteNames(GetSupportedAudioSdesCryptoSuites, - crypto_options, crypto_suite_names); -} - -void GetSupportedVideoSdesCryptoSuites( - const webrtc::CryptoOptions& crypto_options, - std::vector* crypto_suites) { - crypto_suites->push_back(rtc::kSrtpAes128CmSha1_80); - if (crypto_options.srtp.enable_gcm_crypto_suites) { - crypto_suites->push_back(rtc::kSrtpAeadAes256Gcm); - crypto_suites->push_back(rtc::kSrtpAeadAes128Gcm); - } -} - -void GetSupportedVideoSdesCryptoSuiteNames( - const webrtc::CryptoOptions& crypto_options, - std::vector* crypto_suite_names) { - GetSupportedSdesCryptoSuiteNames(GetSupportedVideoSdesCryptoSuites, - crypto_options, crypto_suite_names); -} - -// Support any GCM cipher (if enabled through options). For video support only -// 80-bit SHA1 HMAC. For audio 32-bit HMAC is tolerated (if enabled) unless -// bundle is enabled because it is low overhead. -// Pick the crypto in the list that is supported. -bool SelectCrypto(const MediaContentDescription* offer, - bool bundle, - const webrtc::CryptoOptions& crypto_options, - CryptoParams* crypto_out) { - bool audio = offer->type() == MEDIA_TYPE_AUDIO; - const CryptoParamsVec& cryptos = offer->cryptos(); - - for (const CryptoParams& crypto : cryptos) { - if ((crypto_options.srtp.enable_gcm_crypto_suites && - rtc::IsGcmCryptoSuiteName(crypto.crypto_suite)) || - rtc::kCsAesCm128HmacSha1_80 == crypto.crypto_suite || - (rtc::kCsAesCm128HmacSha1_32 == crypto.crypto_suite && audio && - !bundle && crypto_options.srtp.enable_aes128_sha1_32_crypto_cipher)) { - return CreateCryptoParams(crypto.tag, crypto.crypto_suite, crypto_out); - } - } - return false; -} - // Finds all StreamParams of all media types and attach them to stream_params. StreamParamsVec GetCurrentStreamParams( const std::vector& active_local_contents) { @@ -496,118 +348,6 @@ bool UpdateTransportInfoForBundle(const ContentGroup& bundle_group, return true; } -// Gets the CryptoParamsVec of the given `content_name` from `sdesc`, and -// sets it to `cryptos`. -bool GetCryptosByName(const SessionDescription* sdesc, - const std::string& content_name, - CryptoParamsVec* cryptos) { - if (!sdesc || !cryptos) { - return false; - } - const ContentInfo* content = sdesc->GetContentByName(content_name); - if (!content || !content->media_description()) { - return false; - } - *cryptos = content->media_description()->cryptos(); - return true; -} - -// Prunes the `target_cryptos` by removing the crypto params (crypto_suite) -// which are not available in `filter`. -void PruneCryptos(const CryptoParamsVec& filter, - CryptoParamsVec* target_cryptos) { - if (!target_cryptos) { - return; - } - - target_cryptos->erase( - std::remove_if(target_cryptos->begin(), target_cryptos->end(), - // Returns true if the `crypto`'s crypto_suite is not - // found in `filter`. - [&filter](const CryptoParams& crypto) { - for (const CryptoParams& entry : filter) { - if (entry.crypto_suite == crypto.crypto_suite) - return false; - } - return true; - }), - target_cryptos->end()); -} - -bool IsRtpContent(SessionDescription* sdesc, const std::string& content_name) { - bool is_rtp = false; - ContentInfo* content = sdesc->GetContentByName(content_name); - if (content && content->media_description()) { - is_rtp = IsRtpProtocol(content->media_description()->protocol()); - } - return is_rtp; -} - -// Updates the crypto parameters of the `sdesc` according to the given -// `bundle_group`. The crypto parameters of all the contents within the -// `bundle_group` should be updated to use the common subset of the -// available cryptos. -bool UpdateCryptoParamsForBundle(const ContentGroup& bundle_group, - SessionDescription* sdesc) { - // The bundle should not be empty. - if (!sdesc || !bundle_group.FirstContentName()) { - return false; - } - - bool common_cryptos_needed = false; - // Get the common cryptos. - const ContentNames& content_names = bundle_group.content_names(); - CryptoParamsVec common_cryptos; - bool first = true; - for (const std::string& content_name : content_names) { - if (!IsRtpContent(sdesc, content_name)) { - continue; - } - // The common cryptos are needed if any of the content does not have DTLS - // enabled. - if (!sdesc->GetTransportInfoByName(content_name)->description.secure()) { - common_cryptos_needed = true; - } - if (first) { - first = false; - // Initial the common_cryptos with the first content in the bundle group. - if (!GetCryptosByName(sdesc, content_name, &common_cryptos)) { - return false; - } - if (common_cryptos.empty()) { - // If there's no crypto params, we should just return. - return true; - } - } else { - CryptoParamsVec cryptos; - if (!GetCryptosByName(sdesc, content_name, &cryptos)) { - return false; - } - PruneCryptos(cryptos, &common_cryptos); - } - } - - if (common_cryptos.empty() && common_cryptos_needed) { - return false; - } - - // Update to use the common cryptos. - for (const std::string& content_name : content_names) { - if (!IsRtpContent(sdesc, content_name)) { - continue; - } - ContentInfo* content = sdesc->GetContentByName(content_name); - if (IsMediaContent(content)) { - MediaContentDescription* media_desc = content->media_description(); - if (!media_desc) { - return false; - } - media_desc->set_cryptos(common_cryptos); - } - } - return true; -} - std::vector GetActiveContents( const SessionDescription& description, const MediaSessionOptions& session_options) { @@ -634,9 +374,6 @@ std::vector GetActiveContents( RTCError CreateContentOffer( const MediaDescriptionOptions& media_description_options, const MediaSessionOptions& session_options, - const SecurePolicy& secure_policy, - const CryptoParamsVec* current_cryptos, - const std::vector& crypto_suites, const RtpHeaderExtensions& rtp_extensions, UniqueRandomIdGenerator* ssrc_generator, StreamParamsVec* current_streams, @@ -665,22 +402,6 @@ RTCError CreateContentOffer( AddSimulcastToMediaDescription(media_description_options, offer); - if (secure_policy != SEC_DISABLED) { - if (current_cryptos) { - AddMediaCryptos(*current_cryptos, offer); - } - if (offer->cryptos().empty()) { - if (!CreateMediaCryptos(crypto_suites, offer)) { - LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, - "Failed to create crypto parameters"); - } - } - } - - if (secure_policy == SEC_REQUIRED && offer->cryptos().empty()) { - LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, - "Failed to create crypto parameters"); - } return RTCError::OK(); } @@ -688,9 +409,6 @@ RTCError CreateMediaContentOffer( const MediaDescriptionOptions& media_description_options, const MediaSessionOptions& session_options, const std::vector& codecs, - const SecurePolicy& secure_policy, - const CryptoParamsVec* current_cryptos, - const std::vector& crypto_suites, const RtpHeaderExtensions& rtp_extensions, UniqueRandomIdGenerator* ssrc_generator, StreamParamsVec* current_streams, @@ -705,7 +423,6 @@ RTCError CreateMediaContentOffer( } return CreateContentOffer(media_description_options, session_options, - secure_policy, current_cryptos, crypto_suites, rtp_extensions, ssrc_generator, current_streams, offer); } @@ -1343,8 +1060,6 @@ bool CreateMediaContentAnswer( const MediaContentDescription* offer, const MediaDescriptionOptions& media_description_options, const MediaSessionOptions& session_options, - const SecurePolicy& sdes_policy, - const CryptoParamsVec* current_cryptos, const RtpHeaderExtensions& local_rtp_extensions, UniqueRandomIdGenerator* ssrc_generator, bool enable_encrypted_rtp_header_extensions, @@ -1385,21 +1100,6 @@ bool CreateMediaContentAnswer( answer->set_remote_estimate(offer->remote_estimate()); - if (sdes_policy != SEC_DISABLED) { - CryptoParams crypto; - if (SelectCrypto(offer, bundle_enabled, session_options.crypto_options, - &crypto)) { - if (current_cryptos) { - FindMatchingCrypto(*current_cryptos, crypto, &crypto); - } - answer->AddCrypto(crypto); - } - } - - if (answer->cryptos().empty() && sdes_policy == SEC_REQUIRED) { - return false; - } - AddSimulcastToMediaDescription(media_description_options, answer); answer->set_direction(NegotiateRtpTransceiverDirection( @@ -1438,9 +1138,7 @@ bool IsMediaProtocolSupported(MediaType type, } void SetMediaProtocol(bool secure_transport, MediaContentDescription* desc) { - if (!desc->cryptos().empty()) - desc->set_protocol(kMediaProtocolSavpf); - else if (secure_transport) + if (secure_transport) desc->set_protocol(kMediaProtocolDtlsSavpf); else desc->set_protocol(kMediaProtocolAvpf); @@ -1462,23 +1160,6 @@ const TransportDescription* GetTransportDescription( return desc; } -// Gets the current DTLS state from the transport description. -bool IsDtlsActive(const ContentInfo* content, - const SessionDescription* current_description) { - if (!content) { - return false; - } - - size_t msection_index = content - ¤t_description->contents()[0]; - - if (current_description->transport_infos().size() <= msection_index) { - return false; - } - - return current_description->transport_infos()[msection_index] - .description.secure(); -} - webrtc::RTCErrorOr GetNegotiatedCodecsForOffer( const MediaDescriptionOptions& media_description_options, const MediaSessionOptions& session_options, @@ -1843,11 +1524,6 @@ MediaSessionDescriptionFactory::CreateOfferOrError( RTCErrorType::INTERNAL_ERROR, "CreateOffer failed to UpdateTransportInfoForBundle"); } - if (!UpdateCryptoParamsForBundle(offer_bundle, offer.get())) { - LOG_AND_RETURN_ERROR( - RTCErrorType::INTERNAL_ERROR, - "CreateOffer failed to UpdateCryptoParamsForBundle."); - } } } @@ -2021,12 +1697,6 @@ MediaSessionDescriptionFactory::CreateAnswerOrError( RTCErrorType::INTERNAL_ERROR, "CreateAnswer failed to UpdateTransportInfoForBundle."); } - - if (!UpdateCryptoParamsForBundle(answer_bundle, answer.get())) { - LOG_AND_RETURN_ERROR( - RTCErrorType::INTERNAL_ERROR, - "CreateAnswer failed to UpdateCryptoParamsForBundle."); - } } } } @@ -2378,19 +2048,6 @@ RTCError MediaSessionDescriptionFactory::AddRtpContentForOffer( return error_or_filtered_codecs.MoveError(); } - cricket::SecurePolicy sdes_policy = - IsDtlsActive(current_content, current_description) ? cricket::SEC_DISABLED - : secure(); - - std::vector crypto_suites; - if (media_description_options.type == MEDIA_TYPE_AUDIO) { - GetSupportedAudioSdesCryptoSuiteNames(session_options.crypto_options, - &crypto_suites); - } else { - GetSupportedVideoSdesCryptoSuiteNames(session_options.crypto_options, - &crypto_suites); - } - std::unique_ptr content_description; if (media_description_options.type == MEDIA_TYPE_AUDIO) { content_description = std::make_unique(); @@ -2400,15 +2057,15 @@ RTCError MediaSessionDescriptionFactory::AddRtpContentForOffer( auto error = CreateMediaContentOffer( media_description_options, session_options, - error_or_filtered_codecs.MoveValue(), sdes_policy, - GetCryptos(current_content), crypto_suites, header_extensions, - ssrc_generator(), current_streams, content_description.get(), + error_or_filtered_codecs.MoveValue(), header_extensions, ssrc_generator(), + current_streams, content_description.get(), transport_desc_factory_->trials()); if (!error.ok()) { return error; } - bool secure_transport = transport_desc_factory_->secure() != SEC_DISABLED; + // Insecure transport should only occur in testing. + bool secure_transport = !(transport_desc_factory_->insecure()); SetMediaProtocol(secure_transport, content_description.get()); content_description->set_direction(media_description_options.direction); @@ -2432,15 +2089,9 @@ RTCError MediaSessionDescriptionFactory::AddDataContentForOffer( IceCredentialsIterator* ice_credentials) const { auto data = std::make_unique(); - bool secure_transport = (transport_desc_factory_->secure() != SEC_DISABLED); + bool secure_transport = true; - cricket::SecurePolicy sdes_policy = - IsDtlsActive(current_content, current_description) ? cricket::SEC_DISABLED - : secure(); std::vector crypto_suites; - // SDES doesn't make sense for SCTP, so we disable it, and we only - // get SDES crypto suites for RTP-based data channels. - sdes_policy = cricket::SEC_DISABLED; // Unlike SetMediaProtocol below, we need to set the protocol // before we call CreateMediaContentOffer. Otherwise, // CreateMediaContentOffer won't know this is SCTP and will @@ -2450,10 +2101,9 @@ RTCError MediaSessionDescriptionFactory::AddDataContentForOffer( data->set_use_sctpmap(session_options.use_obsolete_sctp_sdp); data->set_max_message_size(kSctpSendBufferSize); - auto error = CreateContentOffer( - media_description_options, session_options, sdes_policy, - GetCryptos(current_content), crypto_suites, RtpHeaderExtensions(), - ssrc_generator(), current_streams, data.get()); + auto error = CreateContentOffer(media_description_options, session_options, + RtpHeaderExtensions(), ssrc_generator(), + current_streams, data.get()); if (!error.ok()) { return error; } @@ -2522,10 +2172,14 @@ RTCError MediaSessionDescriptionFactory::AddRtpContentForAnswer( } else { offer_content_description = offer_content->media_description()->as_video(); } + // If this section is part of a bundle, bundle_transport is non-null. + // Then require_transport_attributes is false - we can handle sections + // without the DTLS parameters. Otherwise, transport attributes MUST + // be present. std::unique_ptr transport = CreateTransportAnswer( media_description_options.mid, offer_description, media_description_options.transport_options, current_description, - bundle_transport != nullptr, ice_credentials); + bundle_transport == nullptr, ice_credentials); if (!transport) { LOG_AND_RETURN_ERROR( RTCErrorType::INTERNAL_ERROR, @@ -2566,9 +2220,6 @@ RTCError MediaSessionDescriptionFactory::AddRtpContentForAnswer( } else { answer_content = std::make_unique(); } - // Do not require or create SDES cryptos if DTLS is used. - cricket::SecurePolicy sdes_policy = - transport->secure() ? cricket::SEC_DISABLED : secure(); if (!SetCodecsInAnswer( offer_content_description, filtered_codecs, media_description_options, session_options, ssrc_generator(), current_streams, @@ -2578,7 +2229,6 @@ RTCError MediaSessionDescriptionFactory::AddRtpContentForAnswer( } if (!CreateMediaContentAnswer( offer_content_description, media_description_options, session_options, - sdes_policy, GetCryptos(current_content), filtered_rtp_header_extensions(header_extensions), ssrc_generator(), enable_encrypted_rtp_header_extensions_, current_streams, bundle_enabled, answer_content.get())) { @@ -2629,9 +2279,6 @@ RTCError MediaSessionDescriptionFactory::AddDataContentForAnswer( "Failed to create transport answer, data transport is missing"); } - // Do not require or create SDES cryptos if DTLS is used. - cricket::SecurePolicy sdes_policy = - data_transport->secure() ? cricket::SEC_DISABLED : secure(); bool bundle_enabled = offer_description->HasGroup(GROUP_TYPE_BUNDLE) && session_options.bundle_enabled; RTC_CHECK(IsMediaContentOfType(offer_content, MEDIA_TYPE_DATA)); @@ -2656,9 +2303,9 @@ RTCError MediaSessionDescriptionFactory::AddDataContentForAnswer( } if (!CreateMediaContentAnswer( offer_data_description, media_description_options, session_options, - sdes_policy, GetCryptos(current_content), RtpHeaderExtensions(), - ssrc_generator(), enable_encrypted_rtp_header_extensions_, - current_streams, bundle_enabled, data_answer.get())) { + RtpHeaderExtensions(), ssrc_generator(), + enable_encrypted_rtp_header_extensions_, current_streams, + bundle_enabled, data_answer.get())) { LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, "Failed to create answer"); } diff --git a/pc/media_session.h b/pc/media_session.h index 0b3cb67c35..c9fbc4ba63 100644 --- a/pc/media_session.h +++ b/pc/media_session.h @@ -160,8 +160,6 @@ class MediaSessionDescriptionFactory { const VideoCodecs& recv_codecs); RtpHeaderExtensions filtered_rtp_header_extensions( RtpHeaderExtensions extensions) const; - SecurePolicy secure() const { return secure_; } - void set_secure(SecurePolicy s) { secure_ = s; } void set_enable_encrypted_rtp_header_extensions(bool enable) { enable_encrypted_rtp_header_extensions_ = enable; @@ -320,9 +318,6 @@ class MediaSessionDescriptionFactory { webrtc::AlwaysValidPointer const ssrc_generator_; bool enable_encrypted_rtp_header_extensions_ = false; - // TODO(zhihuang): Rename secure_ to sdec_policy_; rename the related getter - // and setter. - SecurePolicy secure_ = SEC_DISABLED; const TransportDescriptionFactory* transport_desc_factory_; }; diff --git a/pc/media_session_unittest.cc b/pc/media_session_unittest.cc index ecfb4397bb..d00ad1c90e 100644 --- a/pc/media_session_unittest.cc +++ b/pc/media_session_unittest.cc @@ -25,7 +25,6 @@ #include "absl/strings/string_view.h" #include "absl/types/optional.h" #include "api/candidate.h" -#include "api/crypto_params.h" #include "api/rtp_parameters.h" #include "media/base/codec.h" #include "media/base/media_constants.h" @@ -50,10 +49,6 @@ #include "test/gtest.h" #include "test/scoped_key_value_config.h" -#define ASSERT_CRYPTO(cd, s, cs) \ - ASSERT_EQ(s, cd->cryptos().size()); \ - ASSERT_EQ(cs, cd->cryptos()[0].crypto_suite) - namespace cricket { namespace { @@ -254,12 +249,6 @@ const char* kMediaProtocols[] = {"RTP/AVP", "RTP/SAVP", "RTP/AVPF", const char* kMediaProtocolsDtls[] = {"TCP/TLS/RTP/SAVPF", "TCP/TLS/RTP/SAVP", "UDP/TLS/RTP/SAVPF", "UDP/TLS/RTP/SAVP"}; -// SRTP cipher name negotiated by the tests. This must be updated if the -// default changes. -const char* kDefaultSrtpCryptoSuite = kCsAesCm128HmacSha1_80; -const char* kDefaultSrtpCryptoSuiteGcm = kCsAeadAes256Gcm; -const uint8_t kDefaultCryptoSuiteSize = 3U; - // These constants are used to make the code using "AddMediaDescriptionOptions" // more readable. constexpr bool kStopped = true; @@ -388,17 +377,6 @@ MediaSessionOptions CreateAudioMediaSession() { return session_options; } -// prefers GCM SDES crypto suites by removing non-GCM defaults. -void PreferGcmCryptoParameters(CryptoParamsVec* cryptos) { - cryptos->erase( - std::remove_if(cryptos->begin(), cryptos->end(), - [](const CryptoParams& crypto) { - return crypto.crypto_suite != kCsAeadAes256Gcm && - crypto.crypto_suite != kCsAeadAes128Gcm; - }), - cryptos->end()); -} - // TODO(zhihuang): Most of these tests were written while MediaSessionOptions // was designed for Plan B SDP, where only one audio "m=" section and one video // "m=" section could be generated, and ordering couldn't be controlled. Many of @@ -451,18 +429,6 @@ class MediaSessionDescriptionFactoryTest : public testing::Test { return video_streams; } - bool CompareCryptoParams(const CryptoParamsVec& c1, - const CryptoParamsVec& c2) { - if (c1.size() != c2.size()) - return false; - for (size_t i = 0; i < c1.size(); ++i) - if (c1[i].tag != c2[i].tag || c1[i].crypto_suite != c2[i].crypto_suite || - c1[i].key_params != c2[i].key_params || - c1[i].session_params != c2[i].session_params) - return false; - return true; - } - // Returns true if the transport info contains "renomination" as an // ICE option. bool GetIceRenomination(const TransportInfo* transport_info) { @@ -566,50 +532,6 @@ class MediaSessionDescriptionFactoryTest : public testing::Test { } } - void TestCryptoWithBundle(bool offer) { - f1_.set_secure(SEC_ENABLED); - MediaSessionOptions options; - AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &options); - std::unique_ptr ref_desc; - std::unique_ptr desc; - if (offer) { - options.bundle_enabled = false; - ref_desc = f1_.CreateOfferOrError(options, nullptr).MoveValue(); - options.bundle_enabled = true; - desc = f1_.CreateOfferOrError(options, ref_desc.get()).MoveValue(); - } else { - options.bundle_enabled = true; - ref_desc = f1_.CreateOfferOrError(options, nullptr).MoveValue(); - desc = - f1_.CreateAnswerOrError(ref_desc.get(), options, nullptr).MoveValue(); - } - ASSERT_TRUE(desc); - const MediaContentDescription* audio_media_desc = - desc->GetContentDescriptionByName("audio"); - ASSERT_TRUE(audio_media_desc); - const MediaContentDescription* video_media_desc = - desc->GetContentDescriptionByName("video"); - ASSERT_TRUE(video_media_desc); - EXPECT_TRUE(CompareCryptoParams(audio_media_desc->cryptos(), - video_media_desc->cryptos())); - ASSERT_CRYPTO(audio_media_desc, offer ? kDefaultCryptoSuiteSize : 1U, - kDefaultSrtpCryptoSuite); - - // Verify the selected crypto is one from the reference audio - // media content. - const MediaContentDescription* ref_audio_media_desc = - ref_desc->GetContentDescriptionByName("audio"); - bool found = false; - for (size_t i = 0; i < ref_audio_media_desc->cryptos().size(); ++i) { - if (ref_audio_media_desc->cryptos()[i].Matches( - audio_media_desc->cryptos()[0])) { - found = true; - break; - } - } - EXPECT_TRUE(found); - } - // This test that the audio and video media direction is set to // `expected_direction_in_answer` in an answer if the offer direction is set // to `direction_in_offer` and the answer is willing to both send and receive. @@ -650,59 +572,6 @@ class MediaSessionDescriptionFactoryTest : public testing::Test { return true; } - void TestVideoGcmCipher(bool gcm_offer, bool gcm_answer) { - MediaSessionOptions offer_opts; - AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &offer_opts); - offer_opts.crypto_options.srtp.enable_gcm_crypto_suites = gcm_offer; - - MediaSessionOptions answer_opts; - AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &answer_opts); - answer_opts.crypto_options.srtp.enable_gcm_crypto_suites = gcm_answer; - - f1_.set_secure(SEC_ENABLED); - f2_.set_secure(SEC_ENABLED); - std::unique_ptr offer = - f1_.CreateOfferOrError(offer_opts, nullptr).MoveValue(); - ASSERT_TRUE(offer.get()); - if (gcm_offer && gcm_answer) { - for (ContentInfo& content : offer->contents()) { - auto cryptos = content.media_description()->cryptos(); - PreferGcmCryptoParameters(&cryptos); - content.media_description()->set_cryptos(cryptos); - } - } - std::unique_ptr answer = - f2_.CreateAnswerOrError(offer.get(), answer_opts, nullptr).MoveValue(); - const ContentInfo* ac = answer->GetContentByName("audio"); - const ContentInfo* vc = answer->GetContentByName("video"); - ASSERT_TRUE(ac); - ASSERT_TRUE(vc); - EXPECT_EQ(MediaProtocolType::kRtp, ac->type); - EXPECT_EQ(MediaProtocolType::kRtp, vc->type); - const MediaContentDescription* acd = ac->media_description(); - const MediaContentDescription* vcd = vc->media_description(); - EXPECT_EQ(MEDIA_TYPE_AUDIO, acd->type()); - EXPECT_THAT(acd->codecs(), ElementsAreArray(kAudioCodecsAnswer)); - EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // negotiated auto bw - EXPECT_EQ(0U, acd->first_ssrc()); // no sender is attached - EXPECT_TRUE(acd->rtcp_mux()); // negotiated rtcp-mux - if (gcm_offer && gcm_answer) { - ASSERT_CRYPTO(acd, 1U, kDefaultSrtpCryptoSuiteGcm); - } else { - ASSERT_CRYPTO(acd, 1U, kDefaultSrtpCryptoSuite); - } - EXPECT_EQ(MEDIA_TYPE_VIDEO, vcd->type()); - EXPECT_THAT(vcd->codecs(), ElementsAreArray(kVideoCodecsAnswer)); - EXPECT_EQ(0U, vcd->first_ssrc()); // no sender is attached - EXPECT_TRUE(vcd->rtcp_mux()); // negotiated rtcp-mux - if (gcm_offer && gcm_answer) { - ASSERT_CRYPTO(vcd, 1U, kDefaultSrtpCryptoSuiteGcm); - } else { - ASSERT_CRYPTO(vcd, 1U, kDefaultSrtpCryptoSuite); - } - EXPECT_EQ(kMediaProtocolSavpf, vcd->protocol()); - } - void TestTransportSequenceNumberNegotiation( const RtpHeaderExtensions& local, const RtpHeaderExtensions& offered, @@ -768,7 +637,6 @@ class MediaSessionDescriptionFactoryTest : public testing::Test { // Create a typical audio offer, and ensure it matches what we expect. TEST_F(MediaSessionDescriptionFactoryTest, TestCreateAudioOffer) { - f1_.set_secure(SEC_ENABLED); std::unique_ptr offer = f1_.CreateOfferOrError(CreateAudioMediaSession(), nullptr).MoveValue(); ASSERT_TRUE(offer.get()); @@ -783,14 +651,12 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateAudioOffer) { EXPECT_EQ(0U, acd->first_ssrc()); // no sender is attached. EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // default bandwidth (auto) EXPECT_TRUE(acd->rtcp_mux()); // rtcp-mux defaults on - ASSERT_CRYPTO(acd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite); - EXPECT_EQ(kMediaProtocolSavpf, acd->protocol()); + EXPECT_EQ(kMediaProtocolDtlsSavpf, acd->protocol()); } // Create an offer with just Opus and RED. TEST_F(MediaSessionDescriptionFactoryTest, TestCreateAudioOfferWithJustOpusAndRed) { - f1_.set_secure(SEC_ENABLED); // First, prefer to only use opus and red. std::vector preferences; preferences.push_back( @@ -819,7 +685,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, // Create an offer with RED before Opus, which enables RED with Opus encoding. TEST_F(MediaSessionDescriptionFactoryTest, TestCreateAudioOfferWithRedForOpus) { - f1_.set_secure(SEC_ENABLED); // First, prefer to only use opus and red. std::vector preferences; preferences.push_back( @@ -850,7 +715,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateAudioOfferWithRedForOpus) { TEST_F(MediaSessionDescriptionFactoryTest, TestCreateVideoOffer) { MediaSessionOptions opts; AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &opts); - f1_.set_secure(SEC_ENABLED); std::unique_ptr offer = f1_.CreateOfferOrError(opts, nullptr).MoveValue(); ASSERT_TRUE(offer.get()); @@ -867,15 +731,13 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateVideoOffer) { EXPECT_EQ(0U, acd->first_ssrc()); // no sender is attached EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // default bandwidth (auto) EXPECT_TRUE(acd->rtcp_mux()); // rtcp-mux defaults on - ASSERT_CRYPTO(acd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite); - EXPECT_EQ(kMediaProtocolSavpf, acd->protocol()); + EXPECT_EQ(kMediaProtocolDtlsSavpf, acd->protocol()); EXPECT_EQ(MEDIA_TYPE_VIDEO, vcd->type()); EXPECT_EQ(f1_.video_sendrecv_codecs(), vcd->codecs()); EXPECT_EQ(0U, vcd->first_ssrc()); // no sender is attached EXPECT_EQ(kAutoBandwidth, vcd->bandwidth()); // default bandwidth (auto) EXPECT_TRUE(vcd->rtcp_mux()); // rtcp-mux defaults on - ASSERT_CRYPTO(vcd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite); - EXPECT_EQ(kMediaProtocolSavpf, vcd->protocol()); + EXPECT_EQ(kMediaProtocolDtlsSavpf, vcd->protocol()); } // Test creating an offer with bundle where the Codecs have the same dynamic @@ -906,8 +768,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestBundleOfferWithSameCodecPlType) { // after an audio only session has been negotiated. TEST_F(MediaSessionDescriptionFactoryTest, TestCreateUpdatedVideoOfferWithBundle) { - f1_.set_secure(SEC_ENABLED); - f2_.set_secure(SEC_ENABLED); MediaSessionOptions opts; AddMediaDescriptionOptions(MEDIA_TYPE_AUDIO, "audio", RtpTransceiverDirection::kRecvOnly, kActive, @@ -934,10 +794,8 @@ TEST_F(MediaSessionDescriptionFactoryTest, EXPECT_TRUE(vcd); EXPECT_TRUE(acd); - ASSERT_CRYPTO(acd, 1U, kDefaultSrtpCryptoSuite); - EXPECT_EQ(kMediaProtocolSavpf, acd->protocol()); - ASSERT_CRYPTO(vcd, 1U, kDefaultSrtpCryptoSuite); - EXPECT_EQ(kMediaProtocolSavpf, vcd->protocol()); + EXPECT_EQ(kMediaProtocolDtlsSavpf, acd->protocol()); + EXPECT_EQ(kMediaProtocolDtlsSavpf, vcd->protocol()); } // Create an SCTP data offer with bundle without error. @@ -945,7 +803,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateSctpDataOffer) { MediaSessionOptions opts; opts.bundle_enabled = true; AddDataSection(RtpTransceiverDirection::kSendRecv, &opts); - f1_.set_secure(SEC_ENABLED); std::unique_ptr offer = f1_.CreateOfferOrError(opts, nullptr).MoveValue(); EXPECT_TRUE(offer.get()); @@ -953,7 +810,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateSctpDataOffer) { auto dcd = GetFirstSctpDataContentDescription(offer.get()); ASSERT_TRUE(dcd); // Since this transport is insecure, the protocol should be "SCTP". - EXPECT_EQ(kMediaProtocolSctp, dcd->protocol()); + EXPECT_EQ(kMediaProtocolUdpDtlsSctp, dcd->protocol()); } // Create an SCTP data offer with bundle without error. @@ -961,8 +818,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateSecureSctpDataOffer) { MediaSessionOptions opts; opts.bundle_enabled = true; AddDataSection(RtpTransceiverDirection::kSendRecv, &opts); - f1_.set_secure(SEC_ENABLED); - tdf1_.set_secure(SEC_ENABLED); std::unique_ptr offer = f1_.CreateOfferOrError(opts, nullptr).MoveValue(); EXPECT_TRUE(offer.get()); @@ -978,19 +833,18 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateImplicitSctpDataOffer) { MediaSessionOptions opts; opts.bundle_enabled = true; AddDataSection(RtpTransceiverDirection::kSendRecv, &opts); - f1_.set_secure(SEC_ENABLED); std::unique_ptr offer1( f1_.CreateOfferOrError(opts, nullptr).MoveValue()); ASSERT_TRUE(offer1.get()); const ContentInfo* data = offer1->GetContentByName("data"); ASSERT_TRUE(data); - ASSERT_EQ(kMediaProtocolSctp, data->media_description()->protocol()); + ASSERT_EQ(kMediaProtocolUdpDtlsSctp, data->media_description()->protocol()); std::unique_ptr offer2( f1_.CreateOfferOrError(opts, offer1.get()).MoveValue()); data = offer2->GetContentByName("data"); ASSERT_TRUE(data); - EXPECT_EQ(kMediaProtocolSctp, data->media_description()->protocol()); + EXPECT_EQ(kMediaProtocolUdpDtlsSctp, data->media_description()->protocol()); } // Test that if BUNDLE is enabled and all media sections are rejected then the @@ -1289,8 +1143,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateOfferContentOrder) { // Create a typical audio answer, and ensure it matches what we expect. TEST_F(MediaSessionDescriptionFactoryTest, TestCreateAudioAnswer) { - f1_.set_secure(SEC_ENABLED); - f2_.set_secure(SEC_ENABLED); std::unique_ptr offer = f1_.CreateOfferOrError(CreateAudioMediaSession(), nullptr).MoveValue(); ASSERT_TRUE(offer.get()); @@ -1308,24 +1160,16 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateAudioAnswer) { EXPECT_EQ(0U, acd->first_ssrc()); // no sender is attached EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // negotiated auto bw EXPECT_TRUE(acd->rtcp_mux()); // negotiated rtcp-mux - ASSERT_CRYPTO(acd, 1U, kDefaultSrtpCryptoSuite); - EXPECT_EQ(kMediaProtocolSavpf, acd->protocol()); + EXPECT_EQ(kMediaProtocolDtlsSavpf, acd->protocol()); } // Create a typical audio answer with GCM ciphers enabled, and ensure it // matches what we expect. TEST_F(MediaSessionDescriptionFactoryTest, TestCreateAudioAnswerGcm) { - f1_.set_secure(SEC_ENABLED); - f2_.set_secure(SEC_ENABLED); MediaSessionOptions opts = CreateAudioMediaSession(); std::unique_ptr offer = f1_.CreateOfferOrError(opts, nullptr).MoveValue(); ASSERT_TRUE(offer.get()); - for (ContentInfo& content : offer->contents()) { - auto cryptos = content.media_description()->cryptos(); - PreferGcmCryptoParameters(&cryptos); - content.media_description()->set_cryptos(cryptos); - } std::unique_ptr answer = f2_.CreateAnswerOrError(offer.get(), opts, nullptr).MoveValue(); const ContentInfo* ac = answer->GetContentByName("audio"); @@ -1339,8 +1183,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateAudioAnswerGcm) { EXPECT_EQ(0U, acd->first_ssrc()); // no sender is attached EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // negotiated auto bw EXPECT_TRUE(acd->rtcp_mux()); // negotiated rtcp-mux - ASSERT_CRYPTO(acd, 1U, kDefaultSrtpCryptoSuiteGcm); - EXPECT_EQ(kMediaProtocolSavpf, acd->protocol()); + EXPECT_EQ(kMediaProtocolDtlsSavpf, acd->protocol()); } // Create an audio answer with no common codecs, and ensure it is rejected. @@ -1369,8 +1212,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TEST_F(MediaSessionDescriptionFactoryTest, TestCreateVideoAnswer) { MediaSessionOptions opts; AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &opts); - f1_.set_secure(SEC_ENABLED); - f2_.set_secure(SEC_ENABLED); std::unique_ptr offer = f1_.CreateOfferOrError(opts, nullptr).MoveValue(); ASSERT_TRUE(offer.get()); @@ -1389,31 +1230,11 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateVideoAnswer) { EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // negotiated auto bw EXPECT_EQ(0U, acd->first_ssrc()); // no sender is attached EXPECT_TRUE(acd->rtcp_mux()); // negotiated rtcp-mux - ASSERT_CRYPTO(acd, 1U, kDefaultSrtpCryptoSuite); EXPECT_EQ(MEDIA_TYPE_VIDEO, vcd->type()); EXPECT_THAT(vcd->codecs(), ElementsAreArray(kVideoCodecsAnswer)); EXPECT_EQ(0U, vcd->first_ssrc()); // no sender is attached EXPECT_TRUE(vcd->rtcp_mux()); // negotiated rtcp-mux - ASSERT_CRYPTO(vcd, 1U, kDefaultSrtpCryptoSuite); - EXPECT_EQ(kMediaProtocolSavpf, vcd->protocol()); -} - -// Create a typical video answer with GCM ciphers enabled, and ensure it -// matches what we expect. -TEST_F(MediaSessionDescriptionFactoryTest, TestCreateVideoAnswerGcm) { - TestVideoGcmCipher(true, true); -} - -// Create a typical video answer with GCM ciphers enabled for the offer only, -// and ensure it matches what we expect. -TEST_F(MediaSessionDescriptionFactoryTest, TestCreateVideoAnswerGcmOffer) { - TestVideoGcmCipher(true, false); -} - -// Create a typical video answer with GCM ciphers enabled for the answer only, -// and ensure it matches what we expect. -TEST_F(MediaSessionDescriptionFactoryTest, TestCreateVideoAnswerGcmAnswer) { - TestVideoGcmCipher(false, true); + EXPECT_EQ(kMediaProtocolDtlsSavpf, vcd->protocol()); } // Create a video answer with no common codecs, and ensure it is rejected. @@ -1512,13 +1333,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateDataAnswerWithoutSctpmap) { // and "TCP/DTLS/SCTP" offers. TEST_F(MediaSessionDescriptionFactoryTest, TestCreateDataAnswerToDifferentOfferedProtos) { - // Need to enable DTLS offer/answer generation (disabled by default in this - // test). - f1_.set_secure(SEC_ENABLED); - f2_.set_secure(SEC_ENABLED); - tdf1_.set_secure(SEC_ENABLED); - tdf2_.set_secure(SEC_ENABLED); - MediaSessionOptions opts; AddDataSection(RtpTransceiverDirection::kSendRecv, &opts); std::unique_ptr offer = @@ -1547,13 +1361,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TEST_F(MediaSessionDescriptionFactoryTest, TestCreateDataAnswerToOfferWithDefinedMessageSize) { - // Need to enable DTLS offer/answer generation (disabled by default in this - // test). - f1_.set_secure(SEC_ENABLED); - f2_.set_secure(SEC_ENABLED); - tdf1_.set_secure(SEC_ENABLED); - tdf2_.set_secure(SEC_ENABLED); - MediaSessionOptions opts; AddDataSection(RtpTransceiverDirection::kSendRecv, &opts); std::unique_ptr offer = @@ -1577,13 +1384,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TEST_F(MediaSessionDescriptionFactoryTest, TestCreateDataAnswerToOfferWithZeroMessageSize) { - // Need to enable DTLS offer/answer generation (disabled by default in this - // test). - f1_.set_secure(SEC_ENABLED); - f2_.set_secure(SEC_ENABLED); - tdf1_.set_secure(SEC_ENABLED); - tdf2_.set_secure(SEC_ENABLED); - MediaSessionOptions opts; AddDataSection(RtpTransceiverDirection::kSendRecv, &opts); std::unique_ptr offer = @@ -1672,13 +1472,13 @@ TEST_F(MediaSessionDescriptionFactoryTest, CreateAnswerToInactiveOffer) { RtpTransceiverDirection::kInactive); } -// Test that the media protocol is RTP/AVPF if DTLS and SDES are disabled. +// Test that the media protocol is RTP/AVPF if DTLS is disabled. TEST_F(MediaSessionDescriptionFactoryTest, AudioOfferAnswerWithCryptoDisabled) { MediaSessionOptions opts = CreateAudioMediaSession(); - f1_.set_secure(SEC_DISABLED); - f2_.set_secure(SEC_DISABLED); - tdf1_.set_secure(SEC_DISABLED); - tdf2_.set_secure(SEC_DISABLED); + tdf1_.SetInsecureForTesting(); + tdf1_.set_certificate(nullptr); + tdf2_.SetInsecureForTesting(); + tdf2_.set_certificate(nullptr); std::unique_ptr offer = f1_.CreateOfferOrError(opts, nullptr).MoveValue(); @@ -2510,7 +2310,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateMultiStreamVideoOffer) { AttachSenderToMediaDescriptionOptions("audio", MEDIA_TYPE_AUDIO, kAudioTrack2, {kMediaStream1}, 1, &opts); - f1_.set_secure(SEC_ENABLED); std::unique_ptr offer = f1_.CreateOfferOrError(opts, nullptr).MoveValue(); @@ -2536,11 +2335,9 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateMultiStreamVideoOffer) { EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // default bandwidth (auto) EXPECT_TRUE(acd->rtcp_mux()); // rtcp-mux defaults on - ASSERT_CRYPTO(acd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite); EXPECT_EQ(MEDIA_TYPE_VIDEO, vcd->type()); EXPECT_EQ(f1_.video_sendrecv_codecs(), vcd->codecs()); - ASSERT_CRYPTO(vcd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite); const StreamParamsVec& video_streams = vcd->streams(); ASSERT_EQ(1U, video_streams.size()); @@ -2571,10 +2368,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateMultiStreamVideoOffer) { EXPECT_EQ(acd->codecs(), updated_acd->codecs()); EXPECT_EQ(vcd->type(), updated_vcd->type()); EXPECT_EQ(vcd->codecs(), updated_vcd->codecs()); - ASSERT_CRYPTO(updated_acd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite); - EXPECT_TRUE(CompareCryptoParams(acd->cryptos(), updated_acd->cryptos())); - ASSERT_CRYPTO(updated_vcd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite); - EXPECT_TRUE(CompareCryptoParams(vcd->cryptos(), updated_vcd->cryptos())); const StreamParamsVec& updated_audio_streams = updated_acd->streams(); ASSERT_EQ(2U, updated_audio_streams.size()); @@ -2795,8 +2588,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateMultiStreamVideoAnswer) { AddMediaDescriptionOptions(MEDIA_TYPE_VIDEO, "video", RtpTransceiverDirection::kRecvOnly, kActive, &offer_opts); - f1_.set_secure(SEC_ENABLED); - f2_.set_secure(SEC_ENABLED); std::unique_ptr offer = f1_.CreateOfferOrError(offer_opts, nullptr).MoveValue(); @@ -2824,8 +2615,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateMultiStreamVideoAnswer) { ASSERT_TRUE(vc); const MediaContentDescription* acd = ac->media_description(); const MediaContentDescription* vcd = vc->media_description(); - ASSERT_CRYPTO(acd, 1U, kDefaultSrtpCryptoSuite); - ASSERT_CRYPTO(vcd, 1U, kDefaultSrtpCryptoSuite); EXPECT_EQ(MEDIA_TYPE_AUDIO, acd->type()); EXPECT_THAT(acd->codecs(), ElementsAreArray(kAudioCodecsAnswer)); @@ -2870,11 +2659,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateMultiStreamVideoAnswer) { const MediaContentDescription* updated_acd = ac->media_description(); const MediaContentDescription* updated_vcd = vc->media_description(); - ASSERT_CRYPTO(updated_acd, 1U, kDefaultSrtpCryptoSuite); - EXPECT_TRUE(CompareCryptoParams(acd->cryptos(), updated_acd->cryptos())); - ASSERT_CRYPTO(updated_vcd, 1U, kDefaultSrtpCryptoSuite); - EXPECT_TRUE(CompareCryptoParams(vcd->cryptos(), updated_vcd->cryptos())); - EXPECT_EQ(acd->type(), updated_acd->type()); EXPECT_EQ(acd->codecs(), updated_acd->codecs()); EXPECT_EQ(vcd->type(), updated_vcd->type()); @@ -3816,54 +3600,9 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestTransportInfo(false, options, true); } -// Create an offer with bundle enabled and verify the crypto parameters are -// the common set of the available cryptos. -TEST_F(MediaSessionDescriptionFactoryTest, TestCryptoWithOfferBundle) { - TestCryptoWithBundle(true); -} - -// Create an answer with bundle enabled and verify the crypto parameters are -// the common set of the available cryptos. -TEST_F(MediaSessionDescriptionFactoryTest, TestCryptoWithAnswerBundle) { - TestCryptoWithBundle(false); -} - -// Verifies that creating answer fails if the offer has UDP/TLS/RTP/SAVPF but -// DTLS is not enabled locally. -TEST_F(MediaSessionDescriptionFactoryTest, - TestOfferDtlsSavpfWithoutDtlsFailed) { - f1_.set_secure(SEC_ENABLED); - f2_.set_secure(SEC_ENABLED); - tdf1_.set_secure(SEC_DISABLED); - tdf2_.set_secure(SEC_DISABLED); - - std::unique_ptr offer = - f1_.CreateOfferOrError(CreateAudioMediaSession(), nullptr).MoveValue(); - ASSERT_TRUE(offer.get()); - ContentInfo* offer_content = offer->GetContentByName("audio"); - ASSERT_TRUE(offer_content); - MediaContentDescription* offer_audio_desc = - offer_content->media_description(); - offer_audio_desc->set_protocol(kMediaProtocolDtlsSavpf); - - std::unique_ptr answer = - f2_.CreateAnswerOrError(offer.get(), CreateAudioMediaSession(), nullptr) - .MoveValue(); - ASSERT_TRUE(answer); - ContentInfo* answer_content = answer->GetContentByName("audio"); - ASSERT_TRUE(answer_content); - - ASSERT_TRUE(answer_content->rejected); -} - // Offers UDP/TLS/RTP/SAVPF and verifies the answer can be created and contains // UDP/TLS/RTP/SAVPF. TEST_F(MediaSessionDescriptionFactoryTest, TestOfferDtlsSavpfCreateAnswer) { - f1_.set_secure(SEC_ENABLED); - f2_.set_secure(SEC_ENABLED); - tdf1_.set_secure(SEC_ENABLED); - tdf2_.set_secure(SEC_ENABLED); - std::unique_ptr offer = f1_.CreateOfferOrError(CreateAudioMediaSession(), nullptr).MoveValue(); ASSERT_TRUE(offer.get()); @@ -3887,136 +3626,24 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestOfferDtlsSavpfCreateAnswer) { EXPECT_EQ(kMediaProtocolDtlsSavpf, answer_audio_desc->protocol()); } -// Test that we include both SDES and DTLS in the offer, but only include SDES -// in the answer if DTLS isn't negotiated. -TEST_F(MediaSessionDescriptionFactoryTest, TestCryptoDtls) { - f1_.set_secure(SEC_ENABLED); - f2_.set_secure(SEC_ENABLED); - tdf1_.set_secure(SEC_ENABLED); - tdf2_.set_secure(SEC_DISABLED); - MediaSessionOptions options; - AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &options); - std::unique_ptr offer, answer; - const MediaContentDescription* audio_media_desc; - const MediaContentDescription* video_media_desc; - const TransportDescription* audio_trans_desc; - const TransportDescription* video_trans_desc; - - // Generate an offer with SDES and DTLS support. - offer = f1_.CreateOfferOrError(options, nullptr).MoveValue(); - ASSERT_TRUE(offer.get()); - - audio_media_desc = offer->GetContentDescriptionByName("audio"); - ASSERT_TRUE(audio_media_desc); - video_media_desc = offer->GetContentDescriptionByName("video"); - ASSERT_TRUE(video_media_desc); - EXPECT_EQ(kDefaultCryptoSuiteSize, audio_media_desc->cryptos().size()); - EXPECT_EQ(kDefaultCryptoSuiteSize, video_media_desc->cryptos().size()); - - audio_trans_desc = offer->GetTransportDescriptionByName("audio"); - ASSERT_TRUE(audio_trans_desc); - video_trans_desc = offer->GetTransportDescriptionByName("video"); - ASSERT_TRUE(video_trans_desc); - ASSERT_TRUE(audio_trans_desc->identity_fingerprint.get()); - ASSERT_TRUE(video_trans_desc->identity_fingerprint.get()); - - // Generate an answer with only SDES support, since tdf2 has crypto disabled. - answer = f2_.CreateAnswerOrError(offer.get(), options, nullptr).MoveValue(); - ASSERT_TRUE(answer.get()); - - audio_media_desc = answer->GetContentDescriptionByName("audio"); - ASSERT_TRUE(audio_media_desc); - video_media_desc = answer->GetContentDescriptionByName("video"); - ASSERT_TRUE(video_media_desc); - EXPECT_EQ(1u, audio_media_desc->cryptos().size()); - EXPECT_EQ(1u, video_media_desc->cryptos().size()); - - audio_trans_desc = answer->GetTransportDescriptionByName("audio"); - ASSERT_TRUE(audio_trans_desc); - video_trans_desc = answer->GetTransportDescriptionByName("video"); - ASSERT_TRUE(video_trans_desc); - ASSERT_FALSE(audio_trans_desc->identity_fingerprint.get()); - ASSERT_FALSE(video_trans_desc->identity_fingerprint.get()); - - // Enable DTLS; the answer should now only have DTLS support. - tdf2_.set_secure(SEC_ENABLED); - answer = f2_.CreateAnswerOrError(offer.get(), options, nullptr).MoveValue(); - ASSERT_TRUE(answer.get()); - - audio_media_desc = answer->GetContentDescriptionByName("audio"); - ASSERT_TRUE(audio_media_desc); - video_media_desc = answer->GetContentDescriptionByName("video"); - ASSERT_TRUE(video_media_desc); - EXPECT_TRUE(audio_media_desc->cryptos().empty()); - EXPECT_TRUE(video_media_desc->cryptos().empty()); - EXPECT_EQ(kMediaProtocolSavpf, audio_media_desc->protocol()); - EXPECT_EQ(kMediaProtocolSavpf, video_media_desc->protocol()); - - audio_trans_desc = answer->GetTransportDescriptionByName("audio"); - ASSERT_TRUE(audio_trans_desc); - video_trans_desc = answer->GetTransportDescriptionByName("video"); - ASSERT_TRUE(video_trans_desc); - ASSERT_TRUE(audio_trans_desc->identity_fingerprint.get()); - ASSERT_TRUE(video_trans_desc->identity_fingerprint.get()); - - // Try creating offer again. DTLS enabled now, crypto's should be empty - // in new offer. - offer = f1_.CreateOfferOrError(options, offer.get()).MoveValue(); - ASSERT_TRUE(offer.get()); - audio_media_desc = offer->GetContentDescriptionByName("audio"); - ASSERT_TRUE(audio_media_desc); - video_media_desc = offer->GetContentDescriptionByName("video"); - ASSERT_TRUE(video_media_desc); - EXPECT_TRUE(audio_media_desc->cryptos().empty()); - EXPECT_TRUE(video_media_desc->cryptos().empty()); - - audio_trans_desc = offer->GetTransportDescriptionByName("audio"); - ASSERT_TRUE(audio_trans_desc); - video_trans_desc = offer->GetTransportDescriptionByName("video"); - ASSERT_TRUE(video_trans_desc); - ASSERT_TRUE(audio_trans_desc->identity_fingerprint.get()); - ASSERT_TRUE(video_trans_desc->identity_fingerprint.get()); -} - -// Test that an answer can't be created if cryptos are required but the offer is -// unsecure. -TEST_F(MediaSessionDescriptionFactoryTest, TestSecureAnswerToUnsecureOffer) { - MediaSessionOptions options = CreateAudioMediaSession(); - f1_.set_secure(SEC_DISABLED); - tdf1_.set_secure(SEC_DISABLED); - f2_.set_secure(SEC_REQUIRED); - tdf1_.set_secure(SEC_ENABLED); - - std::unique_ptr offer = - f1_.CreateOfferOrError(options, nullptr).MoveValue(); - ASSERT_TRUE(offer.get()); - - auto error = f2_.CreateAnswerOrError(offer.get(), options, nullptr); - EXPECT_FALSE(error.ok()); -} // Test that we accept a DTLS offer without SDES and create an appropriate // answer. TEST_F(MediaSessionDescriptionFactoryTest, TestCryptoOfferDtlsButNotSdes) { + /* TODO(hta): Figure this one out. f1_.set_secure(SEC_DISABLED); f2_.set_secure(SEC_ENABLED); tdf1_.set_secure(SEC_ENABLED); tdf2_.set_secure(SEC_ENABLED); + */ MediaSessionOptions options; AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &options); - // Generate an offer with DTLS but without SDES. + // Generate an offer with DTLS std::unique_ptr offer = f1_.CreateOfferOrError(options, nullptr).MoveValue(); ASSERT_TRUE(offer.get()); - const AudioContentDescription* audio_offer = - GetFirstAudioContentDescription(offer.get()); - ASSERT_TRUE(audio_offer->cryptos().empty()); - const VideoContentDescription* video_offer = - GetFirstVideoContentDescription(offer.get()); - ASSERT_TRUE(video_offer->cryptos().empty()); - const TransportDescription* audio_offer_trans_desc = offer->GetTransportDescriptionByName("audio"); ASSERT_TRUE(audio_offer_trans_desc->identity_fingerprint.get()); @@ -4698,14 +4325,10 @@ class MediaProtocolTest : public testing::TestWithParam { MAKE_VECTOR(kAudioCodecs2)); f2_.set_video_codecs(MAKE_VECTOR(kVideoCodecs2), MAKE_VECTOR(kVideoCodecs2)); - f1_.set_secure(SEC_ENABLED); - f2_.set_secure(SEC_ENABLED); tdf1_.set_certificate(rtc::RTCCertificate::Create( std::unique_ptr(new rtc::FakeSSLIdentity("id1")))); tdf2_.set_certificate(rtc::RTCCertificate::Create( std::unique_ptr(new rtc::FakeSSLIdentity("id2")))); - tdf1_.set_secure(SEC_ENABLED); - tdf2_.set_secure(SEC_ENABLED); } protected: @@ -4752,6 +4375,9 @@ INSTANTIATE_TEST_SUITE_P(MediaProtocolDtlsPatternTest, TEST_F(MediaSessionDescriptionFactoryTest, TestSetAudioCodecs) { webrtc::test::ScopedKeyValueConfig field_trials; TransportDescriptionFactory tdf(field_trials); + tdf.set_certificate(rtc::RTCCertificate::Create( + std::unique_ptr(new rtc::FakeSSLIdentity("id")))); + UniqueRandomIdGenerator ssrc_generator; MediaSessionDescriptionFactory sf(nullptr, false, &ssrc_generator, &tdf); std::vector send_codecs = MAKE_VECTOR(kAudioCodecs1); @@ -4821,6 +4447,9 @@ bool CodecsMatch(const std::vector& codecs1, void TestAudioCodecsOffer(RtpTransceiverDirection direction) { webrtc::test::ScopedKeyValueConfig field_trials; TransportDescriptionFactory tdf(field_trials); + tdf.set_certificate(rtc::RTCCertificate::Create( + std::unique_ptr(new rtc::FakeSSLIdentity("id")))); + UniqueRandomIdGenerator ssrc_generator; MediaSessionDescriptionFactory sf(nullptr, false, &ssrc_generator, &tdf); const std::vector send_codecs = MAKE_VECTOR(kAudioCodecs1); @@ -4921,6 +4550,11 @@ void TestAudioCodecsAnswer(RtpTransceiverDirection offer_direction, webrtc::test::ScopedKeyValueConfig field_trials; TransportDescriptionFactory offer_tdf(field_trials); TransportDescriptionFactory answer_tdf(field_trials); + offer_tdf.set_certificate(rtc::RTCCertificate::Create( + std::unique_ptr(new rtc::FakeSSLIdentity("offer_id")))); + answer_tdf.set_certificate( + rtc::RTCCertificate::Create(std::unique_ptr( + new rtc::FakeSSLIdentity("answer_id")))); UniqueRandomIdGenerator ssrc_generator1, ssrc_generator2; MediaSessionDescriptionFactory offer_factory(nullptr, false, &ssrc_generator1, &offer_tdf); diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index e90d1e83f9..76cf13aa18 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -2702,9 +2702,7 @@ void PeerConnection::ReportRemoteIceCandidateAdded( bool PeerConnection::SrtpRequired() const { RTC_DCHECK_RUN_ON(signaling_thread()); - return (dtls_enabled_ || - sdp_handler_->webrtc_session_desc_factory()->SdesPolicy() == - cricket::SEC_REQUIRED); + return dtls_enabled_; } void PeerConnection::OnTransportControllerGatheringState( diff --git a/pc/peer_connection_crypto_unittest.cc b/pc/peer_connection_crypto_unittest.cc index 4274e88b07..1d90e04714 100644 --- a/pc/peer_connection_crypto_unittest.cc +++ b/pc/peer_connection_crypto_unittest.cc @@ -24,7 +24,6 @@ #include "api/audio_codecs/builtin_audio_encoder_factory.h" #include "api/create_peerconnection_factory.h" #include "api/crypto/crypto_options.h" -#include "api/crypto_params.h" #include "api/jsep.h" #include "api/peer_connection_interface.h" #include "api/scoped_refptr.h" @@ -178,13 +177,6 @@ SdpContentPredicate HaveDtlsFingerprint() { }; } -SdpContentPredicate HaveSdesCryptos() { - return [](const cricket::ContentInfo* content, - const cricket::TransportInfo* transport) { - return !content->media_description()->cryptos().empty(); - }; -} - SdpContentPredicate HaveProtocol(const std::string& protocol) { return [protocol](const cricket::ContentInfo* content, const cricket::TransportInfo* transport) { @@ -192,22 +184,6 @@ SdpContentPredicate HaveProtocol(const std::string& protocol) { }; } -SdpContentPredicate HaveSdesGcmCryptos(size_t num_crypto_suites) { - return [num_crypto_suites](const cricket::ContentInfo* content, - const cricket::TransportInfo* transport) { - const auto& cryptos = content->media_description()->cryptos(); - if (cryptos.size() != num_crypto_suites) { - return false; - } - for (size_t i = 0; i < cryptos.size(); ++i) { - if (cryptos[i].key_params.size() == 67U && - cryptos[i].crypto_suite == "AEAD_AES_256_GCM") - return true; - } - return false; - }; -} - class PeerConnectionCryptoTest : public PeerConnectionCryptoBaseTest, public ::testing::WithParamInterface { @@ -215,20 +191,13 @@ class PeerConnectionCryptoTest PeerConnectionCryptoTest() : PeerConnectionCryptoBaseTest(GetParam()) {} }; -SdpContentMutator RemoveSdesCryptos() { - return [](cricket::ContentInfo* content, cricket::TransportInfo* transport) { - content->media_description()->set_cryptos({}); - }; -} - SdpContentMutator RemoveDtlsFingerprint() { return [](cricket::ContentInfo* content, cricket::TransportInfo* transport) { transport->description.identity_fingerprint.reset(); }; } -// When DTLS is enabled, the SDP offer/answer should have a DTLS fingerprint and -// no SDES cryptos. +// When DTLS is enabled, the SDP offer/answer should have a DTLS fingerprint TEST_P(PeerConnectionCryptoTest, CorrectCryptoInOfferWhenDtlsEnabled) { RTCConfiguration config; auto caller = CreatePeerConnectionWithAudioVideo(config); @@ -238,7 +207,6 @@ TEST_P(PeerConnectionCryptoTest, CorrectCryptoInOfferWhenDtlsEnabled) { ASSERT_FALSE(offer->description()->contents().empty()); EXPECT_TRUE(SdpContentsAll(HaveDtlsFingerprint(), offer->description())); - EXPECT_TRUE(SdpContentsNone(HaveSdesCryptos(), offer->description())); EXPECT_TRUE(SdpContentsAll(HaveProtocol(cricket::kMediaProtocolDtlsSavpf), offer->description())); } @@ -253,7 +221,6 @@ TEST_P(PeerConnectionCryptoTest, CorrectCryptoInAnswerWhenDtlsEnabled) { ASSERT_FALSE(answer->description()->contents().empty()); EXPECT_TRUE(SdpContentsAll(HaveDtlsFingerprint(), answer->description())); - EXPECT_TRUE(SdpContentsNone(HaveSdesCryptos(), answer->description())); EXPECT_TRUE(SdpContentsAll(HaveProtocol(cricket::kMediaProtocolDtlsSavpf), answer->description())); } diff --git a/pc/peer_connection_interface_unittest.cc b/pc/peer_connection_interface_unittest.cc index 61794bb0f9..08fb1632d6 100644 --- a/pc/peer_connection_interface_unittest.cc +++ b/pc/peer_connection_interface_unittest.cc @@ -420,29 +420,6 @@ static const char kSdpStringMs1Video1[] = "a=ssrc:4 cname:stream1\r\n" "a=ssrc:4 msid:stream1 videotrack1\r\n"; -static const char kDtlsSdesFallbackSdp[] = - "v=0\r\n" - "o=xxxxxx 7 2 IN IP4 0.0.0.0\r\n" - "s=-\r\n" - "c=IN IP4 0.0.0.0\r\n" - "t=0 0\r\n" - "a=group:BUNDLE audio\r\n" - "a=msid-semantic: WMS\r\n" - "m=audio 1 RTP/SAVPF 0\r\n" - "a=sendrecv\r\n" - "a=rtcp-mux\r\n" - "a=mid:audio\r\n" - "a=ssrc:1 cname:stream1\r\n" - "a=ice-ufrag:e5785931\r\n" - "a=ice-pwd:36fb7878390db89481c1d46daa4278d8\r\n" - "a=rtpmap:0 pcmu/8000\r\n" - "a=fingerprint:sha-1 " - "4A:AD:B9:B1:3F:82:18:3B:54:02:12:DF:3E:5D:49:6B:19:E5:7C:AB\r\n" - "a=setup:actpass\r\n" - "a=crypto:0 AES_CM_128_HMAC_SHA1_80 " - "inline:NzB4d1BINUAvLEw6UzF3WSJ+PSdFcGdUJShpX1Zj|2^20|1:32 " - "dummy_session_params\r\n"; - class RtcEventLogOutputNull final : public RtcEventLogOutput { public: bool IsActive() const override { return true; } @@ -1662,7 +1639,7 @@ TEST_P(PeerConnectionInterfaceTest, AddTrackWithoutStream) { // Test that we can call GetStats() after AddTrack but before connecting // the PeerConnection to a peer. TEST_P(PeerConnectionInterfaceTest, AddTrackBeforeConnecting) { - CreatePeerConnectionWithoutDtls(); + CreatePeerConnection(); rtc::scoped_refptr audio_track( CreateAudioTrack("audio_track")); rtc::scoped_refptr video_track( @@ -1673,7 +1650,7 @@ TEST_P(PeerConnectionInterfaceTest, AddTrackBeforeConnecting) { } TEST_P(PeerConnectionInterfaceTest, AttachmentIdIsSetOnAddTrack) { - CreatePeerConnectionWithoutDtls(); + CreatePeerConnection(); rtc::scoped_refptr audio_track( CreateAudioTrack("audio_track")); rtc::scoped_refptr video_track( @@ -1695,7 +1672,7 @@ TEST_P(PeerConnectionInterfaceTest, AttachmentIdIsSetOnAddTrack) { // Don't run under Unified Plan since the stream API is not available. TEST_F(PeerConnectionInterfaceTestPlanB, AttachmentIdIsSetOnAddStream) { - CreatePeerConnectionWithoutDtls(); + CreatePeerConnection(); AddVideoStream(kStreamId1); auto senders = pc_->GetSenders(); ASSERT_EQ(1u, senders.size()); @@ -2130,24 +2107,6 @@ TEST_P(PeerConnectionInterfaceTest, ReceiveFireFoxOffer) { #endif } -// Test that fallback from DTLS to SDES is not supported. -// The fallback was previously supported but was removed to simplify the code -// and because it's non-standard. -TEST_P(PeerConnectionInterfaceTest, DtlsSdesFallbackNotSupported) { - RTCConfiguration rtc_config; - CreatePeerConnection(rtc_config); - // Wait for fake certificate to be generated. Previously, this is what caused - // the "a=crypto" lines to be rejected. - AddAudioTrack("audio_label"); - AddVideoTrack("video_label"); - ASSERT_NE(nullptr, fake_certificate_generator_); - EXPECT_EQ_WAIT(1, fake_certificate_generator_->generated_certificates(), - kTimeout); - std::unique_ptr desc( - CreateSessionDescription(SdpType::kOffer, kDtlsSdesFallbackSdp, nullptr)); - EXPECT_FALSE(DoSetSessionDescription(std::move(desc), /*local=*/false)); -} - // Test that we can create an audio only offer and receive an answer with a // limited set of audio codecs and receive an updated offer with more audio // codecs, where the added codecs are not supported. diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index 19cd9ba45c..d75606ab4e 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -99,7 +99,7 @@ const char kSdpWithoutIceUfragPwd[] = "Called with SDP without ice-ufrag and ice-pwd."; const char kSdpWithoutDtlsFingerprint[] = "Called with SDP without DTLS fingerprint."; -const char kSdpWithoutSdesCrypto[] = "Called with SDP without SDES crypto."; +const char kSdpWithoutCrypto[] = "Called with SDP without crypto setup."; const char kSessionError[] = "Session error code: "; const char kSessionErrorDesc[] = "Session error description: "; @@ -271,7 +271,7 @@ bool MediaSectionsHaveSameCount(const SessionDescription& desc1, const SessionDescription& desc2) { return desc1.contents().size() == desc2.contents().size(); } -// Checks that each non-rejected content has SDES crypto keys or a DTLS +// Checks that each non-rejected content has a DTLS // fingerprint, unless it's in a BUNDLE group, in which case only the // BUNDLE-tag section (first media section/description in the BUNDLE group) // needs a ufrag and pwd. Mismatches, such as replying with a DTLS fingerprint @@ -310,10 +310,7 @@ RTCError VerifyCrypto(const SessionDescription* desc, kSdpWithoutDtlsFingerprint); } } else { - if (media->cryptos().empty()) { - LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, - kSdpWithoutSdesCrypto); - } + LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, kSdpWithoutCrypto); } } return RTCError::OK(); @@ -1393,7 +1390,9 @@ void SdpOfferAnswerHandler::Initialize( pc_->trials()); if (pc_->options()->disable_encryption) { - webrtc_session_desc_factory_->SetSdesPolicy(cricket::SEC_DISABLED); + RTC_LOG(LS_INFO) + << "Disabling encryption. This should only be done in tests."; + webrtc_session_desc_factory_->SetInsecureForTesting(); } webrtc_session_desc_factory_->set_enable_encrypted_rtp_header_extensions( @@ -3551,8 +3550,7 @@ RTCError SdpOfferAnswerHandler::ValidateSessionDescription( // Verify crypto settings. std::string crypto_error; - if (webrtc_session_desc_factory_->SdesPolicy() == cricket::SEC_REQUIRED || - pc_->dtls_enabled()) { + if (pc_->dtls_enabled()) { RTCError crypto_error = VerifyCrypto( sdesc->description(), pc_->dtls_enabled(), bundle_groups_by_mid); if (!crypto_error.ok()) { diff --git a/pc/session_description.h b/pc/session_description.h index 6ef9c316e1..fe037a5786 100644 --- a/pc/session_description.h +++ b/pc/session_description.h @@ -23,7 +23,6 @@ #include "absl/memory/memory.h" #include "absl/strings/string_view.h" -#include "api/crypto_params.h" #include "api/media_types.h" #include "api/rtp_parameters.h" #include "api/rtp_transceiver_direction.h" @@ -43,7 +42,6 @@ namespace cricket { -using CryptoParamsVec = std::vector; using RtpHeaderExtensions = std::vector; // Options to control how session descriptions are generated. @@ -123,12 +121,6 @@ class MediaContentDescription { bandwidth_type_ = bandwidth_type; } - const std::vector& cryptos() const { return cryptos_; } - void AddCrypto(const CryptoParams& params) { cryptos_.push_back(params); } - void set_cryptos(const std::vector& cryptos) { - cryptos_ = cryptos; - } - // List of RTP header extensions. URIs are **NOT** guaranteed to be unique // as they can appear twice when both encrypted and non-encrypted extensions // are present. @@ -268,7 +260,6 @@ class MediaContentDescription { int bandwidth_ = kAutoBandwidth; std::string bandwidth_type_ = kApplicationSpecificBandwidth; - std::vector cryptos_; std::vector rtp_header_extensions_; bool rtp_header_extensions_set_ = false; StreamParamsVec send_streams_; diff --git a/pc/srtp_filter.cc b/pc/srtp_filter.cc deleted file mode 100644 index b8be63cd22..0000000000 --- a/pc/srtp_filter.cc +++ /dev/null @@ -1,280 +0,0 @@ -/* - * Copyright 2009 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "pc/srtp_filter.h" - -#include - -#include - -#include "absl/strings/match.h" -#include "rtc_base/logging.h" -#include "rtc_base/ssl_stream_adapter.h" -#include "rtc_base/third_party/base64/base64.h" -#include "rtc_base/zero_memory.h" - -namespace cricket { - -SrtpFilter::SrtpFilter() {} - -SrtpFilter::~SrtpFilter() {} - -bool SrtpFilter::IsActive() const { - return state_ >= ST_ACTIVE; -} - -bool SrtpFilter::Process(const std::vector& cryptos, - webrtc::SdpType type, - ContentSource source) { - bool ret = false; - switch (type) { - case webrtc::SdpType::kOffer: - ret = SetOffer(cryptos, source); - break; - case webrtc::SdpType::kPrAnswer: - ret = SetProvisionalAnswer(cryptos, source); - break; - case webrtc::SdpType::kAnswer: - ret = SetAnswer(cryptos, source); - break; - default: - break; - } - - if (!ret) { - return false; - } - - return true; -} - -bool SrtpFilter::SetOffer(const std::vector& offer_params, - ContentSource source) { - if (!ExpectOffer(source)) { - RTC_LOG(LS_ERROR) << "Wrong state to update SRTP offer"; - return false; - } - return StoreParams(offer_params, source); -} - -bool SrtpFilter::SetAnswer(const std::vector& answer_params, - ContentSource source) { - return DoSetAnswer(answer_params, source, true); -} - -bool SrtpFilter::SetProvisionalAnswer( - const std::vector& answer_params, - ContentSource source) { - return DoSetAnswer(answer_params, source, false); -} - -bool SrtpFilter::ExpectOffer(ContentSource source) { - return ((state_ == ST_INIT) || (state_ == ST_ACTIVE) || - (state_ == ST_SENTOFFER && source == CS_LOCAL) || - (state_ == ST_SENTUPDATEDOFFER && source == CS_LOCAL) || - (state_ == ST_RECEIVEDOFFER && source == CS_REMOTE) || - (state_ == ST_RECEIVEDUPDATEDOFFER && source == CS_REMOTE)); -} - -bool SrtpFilter::StoreParams(const std::vector& params, - ContentSource source) { - offer_params_ = params; - if (state_ == ST_INIT) { - state_ = (source == CS_LOCAL) ? ST_SENTOFFER : ST_RECEIVEDOFFER; - } else if (state_ == ST_ACTIVE) { - state_ = - (source == CS_LOCAL) ? ST_SENTUPDATEDOFFER : ST_RECEIVEDUPDATEDOFFER; - } - return true; -} - -bool SrtpFilter::ExpectAnswer(ContentSource source) { - return ((state_ == ST_SENTOFFER && source == CS_REMOTE) || - (state_ == ST_RECEIVEDOFFER && source == CS_LOCAL) || - (state_ == ST_SENTUPDATEDOFFER && source == CS_REMOTE) || - (state_ == ST_RECEIVEDUPDATEDOFFER && source == CS_LOCAL) || - (state_ == ST_SENTPRANSWER_NO_CRYPTO && source == CS_LOCAL) || - (state_ == ST_SENTPRANSWER && source == CS_LOCAL) || - (state_ == ST_RECEIVEDPRANSWER_NO_CRYPTO && source == CS_REMOTE) || - (state_ == ST_RECEIVEDPRANSWER && source == CS_REMOTE)); -} - -bool SrtpFilter::DoSetAnswer(const std::vector& answer_params, - ContentSource source, - bool final) { - if (!ExpectAnswer(source)) { - RTC_LOG(LS_ERROR) << "Invalid state for SRTP answer"; - return false; - } - - // If the answer doesn't requests crypto complete the negotiation of an - // unencrypted session. - // Otherwise, finalize the parameters and apply them. - if (answer_params.empty()) { - if (final) { - return ResetParams(); - } else { - // Need to wait for the final answer to decide if - // we should go to Active state. - state_ = (source == CS_LOCAL) ? ST_SENTPRANSWER_NO_CRYPTO - : ST_RECEIVEDPRANSWER_NO_CRYPTO; - return true; - } - } - CryptoParams selected_params; - if (!NegotiateParams(answer_params, &selected_params)) - return false; - - const CryptoParams& new_send_params = - (source == CS_REMOTE) ? selected_params : answer_params[0]; - const CryptoParams& new_recv_params = - (source == CS_REMOTE) ? answer_params[0] : selected_params; - if (!ApplySendParams(new_send_params) || !ApplyRecvParams(new_recv_params)) { - return false; - } - applied_send_params_ = new_send_params; - applied_recv_params_ = new_recv_params; - - if (final) { - offer_params_.clear(); - state_ = ST_ACTIVE; - } else { - state_ = (source == CS_LOCAL) ? ST_SENTPRANSWER : ST_RECEIVEDPRANSWER; - } - return true; -} - -bool SrtpFilter::NegotiateParams(const std::vector& answer_params, - CryptoParams* selected_params) { - // We're processing an accept. We should have exactly one set of params, - // unless the offer didn't mention crypto, in which case we shouldn't be here. - bool ret = (answer_params.size() == 1U && !offer_params_.empty()); - if (ret) { - // We should find a match between the answer params and the offered params. - std::vector::const_iterator it; - for (it = offer_params_.begin(); it != offer_params_.end(); ++it) { - if (answer_params[0].Matches(*it)) { - break; - } - } - - if (it != offer_params_.end()) { - *selected_params = *it; - } else { - ret = false; - } - } - - if (!ret) { - RTC_LOG(LS_WARNING) << "Invalid parameters in SRTP answer"; - } - return ret; -} - -bool SrtpFilter::ResetParams() { - offer_params_.clear(); - applied_send_params_ = CryptoParams(); - applied_recv_params_ = CryptoParams(); - send_crypto_suite_ = absl::nullopt; - recv_crypto_suite_ = absl::nullopt; - send_key_.Clear(); - recv_key_.Clear(); - state_ = ST_INIT; - return true; -} - -bool SrtpFilter::ApplySendParams(const CryptoParams& send_params) { - if (applied_send_params_.crypto_suite == send_params.crypto_suite && - applied_send_params_.key_params == send_params.key_params) { - RTC_LOG(LS_INFO) << "Applying the same SRTP send parameters again. No-op."; - - // We do not want to reset the ROC if the keys are the same. So just return. - return true; - } - - send_crypto_suite_ = rtc::SrtpCryptoSuiteFromName(send_params.crypto_suite); - if (send_crypto_suite_ == rtc::kSrtpInvalidCryptoSuite) { - RTC_LOG(LS_WARNING) << "Unknown crypto suite(s) received:" - " send crypto_suite " - << send_params.crypto_suite; - return false; - } - - int send_key_len, send_salt_len; - if (!rtc::GetSrtpKeyAndSaltLengths(*send_crypto_suite_, &send_key_len, - &send_salt_len)) { - RTC_LOG(LS_ERROR) << "Could not get lengths for crypto suite(s):" - " send crypto_suite " - << send_params.crypto_suite; - return false; - } - - send_key_ = rtc::ZeroOnFreeBuffer(send_key_len + send_salt_len); - return ParseKeyParams(send_params.key_params, send_key_.data(), - send_key_.size()); -} - -bool SrtpFilter::ApplyRecvParams(const CryptoParams& recv_params) { - if (applied_recv_params_.crypto_suite == recv_params.crypto_suite && - applied_recv_params_.key_params == recv_params.key_params) { - RTC_LOG(LS_INFO) << "Applying the same SRTP recv parameters again. No-op."; - - // We do not want to reset the ROC if the keys are the same. So just return. - return true; - } - - recv_crypto_suite_ = rtc::SrtpCryptoSuiteFromName(recv_params.crypto_suite); - if (recv_crypto_suite_ == rtc::kSrtpInvalidCryptoSuite) { - RTC_LOG(LS_WARNING) << "Unknown crypto suite(s) received:" - " recv crypto_suite " - << recv_params.crypto_suite; - return false; - } - - int recv_key_len, recv_salt_len; - if (!rtc::GetSrtpKeyAndSaltLengths(*recv_crypto_suite_, &recv_key_len, - &recv_salt_len)) { - RTC_LOG(LS_ERROR) << "Could not get lengths for crypto suite(s):" - " recv crypto_suite " - << recv_params.crypto_suite; - return false; - } - - recv_key_ = rtc::ZeroOnFreeBuffer(recv_key_len + recv_salt_len); - return ParseKeyParams(recv_params.key_params, recv_key_.data(), - recv_key_.size()); -} - -bool SrtpFilter::ParseKeyParams(const std::string& key_params, - uint8_t* key, - size_t len) { - // example key_params: "inline:YUJDZGVmZ2hpSktMbW9QUXJzVHVWd3l6MTIzNDU2" - - // Fail if key-method is wrong. - if (!absl::StartsWith(key_params, "inline:")) { - return false; - } - - // Fail if base64 decode fails, or the key is the wrong size. - std::string key_b64(key_params.substr(7)), key_str; - if (!rtc::Base64::Decode(key_b64, rtc::Base64::DO_STRICT, &key_str, - nullptr) || - key_str.size() != len) { - return false; - } - - memcpy(key, key_str.c_str(), len); - // TODO(bugs.webrtc.org/8905): Switch to ZeroOnFreeBuffer for storing - // sensitive data. - rtc::ExplicitZeroMemory(&key_str[0], key_str.size()); - return true; -} - -} // namespace cricket diff --git a/pc/srtp_filter.h b/pc/srtp_filter.h deleted file mode 100644 index 59c43f624b..0000000000 --- a/pc/srtp_filter.h +++ /dev/null @@ -1,147 +0,0 @@ -/* - * Copyright 2009 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#ifndef PC_SRTP_FILTER_H_ -#define PC_SRTP_FILTER_H_ - -#include -#include - -#include -#include -#include -#include -#include - -#include "absl/types/optional.h" -#include "api/array_view.h" -#include "api/crypto_params.h" -#include "api/jsep.h" -#include "api/sequence_checker.h" -#include "pc/session_description.h" -#include "rtc_base/buffer.h" -#include "rtc_base/ssl_stream_adapter.h" - -// Forward declaration to avoid pulling in libsrtp headers here -struct srtp_event_data_t; -struct srtp_ctx_t_; - -namespace cricket { - -// A helper class used to negotiate SDES crypto params. -// TODO(zhihuang): Find a better name for this class, like "SdesNegotiator". -class SrtpFilter { - public: - enum Mode { PROTECT, UNPROTECT }; - enum Error { - ERROR_NONE, - ERROR_FAIL, - ERROR_AUTH, - ERROR_REPLAY, - }; - - SrtpFilter(); - ~SrtpFilter(); - - // Whether the filter is active (i.e. crypto has been properly negotiated). - bool IsActive() const; - - // Handle the offer/answer negotiation of the crypto parameters internally. - // TODO(zhihuang): Make SetOffer/ProvisionalAnswer/Answer private as helper - // methods once start using Process. - bool Process(const std::vector& cryptos, - webrtc::SdpType type, - ContentSource source); - - // Indicates which crypto algorithms and keys were contained in the offer. - // offer_params should contain a list of available parameters to use, or none, - // if crypto is not desired. This must be called before SetAnswer. - bool SetOffer(const std::vector& offer_params, - ContentSource source); - // Same as SetAnwer. But multiple calls are allowed to SetProvisionalAnswer - // after a call to SetOffer. - bool SetProvisionalAnswer(const std::vector& answer_params, - ContentSource source); - // Indicates which crypto algorithms and keys were contained in the answer. - // answer_params should contain the negotiated parameters, which may be none, - // if crypto was not desired or could not be negotiated (and not required). - // This must be called after SetOffer. If crypto negotiation completes - // successfully, this will advance the filter to the active state. - bool SetAnswer(const std::vector& answer_params, - ContentSource source); - - bool ResetParams(); - - static bool ParseKeyParams(const std::string& params, - uint8_t* key, - size_t len); - - absl::optional send_crypto_suite() { return send_crypto_suite_; } - absl::optional recv_crypto_suite() { return recv_crypto_suite_; } - - rtc::ArrayView send_key() { return send_key_; } - rtc::ArrayView recv_key() { return recv_key_; } - - protected: - bool ExpectOffer(ContentSource source); - - bool StoreParams(const std::vector& params, - ContentSource source); - - bool ExpectAnswer(ContentSource source); - - bool DoSetAnswer(const std::vector& answer_params, - ContentSource source, - bool final); - - bool NegotiateParams(const std::vector& answer_params, - CryptoParams* selected_params); - - private: - bool ApplySendParams(const CryptoParams& send_params); - - bool ApplyRecvParams(const CryptoParams& recv_params); - - enum State { - ST_INIT, // SRTP filter unused. - ST_SENTOFFER, // Offer with SRTP parameters sent. - ST_RECEIVEDOFFER, // Offer with SRTP parameters received. - ST_SENTPRANSWER_NO_CRYPTO, // Sent provisional answer without crypto. - // Received provisional answer without crypto. - ST_RECEIVEDPRANSWER_NO_CRYPTO, - ST_ACTIVE, // Offer and answer set. - // SRTP filter is active but new parameters are offered. - // When the answer is set, the state transitions to ST_ACTIVE or ST_INIT. - ST_SENTUPDATEDOFFER, - // SRTP filter is active but new parameters are received. - // When the answer is set, the state transitions back to ST_ACTIVE. - ST_RECEIVEDUPDATEDOFFER, - // SRTP filter is active but the sent answer is only provisional. - // When the final answer is set, the state transitions to ST_ACTIVE or - // ST_INIT. - ST_SENTPRANSWER, - // SRTP filter is active but the received answer is only provisional. - // When the final answer is set, the state transitions to ST_ACTIVE or - // ST_INIT. - ST_RECEIVEDPRANSWER - }; - State state_ = ST_INIT; - std::vector offer_params_; - CryptoParams applied_send_params_; - CryptoParams applied_recv_params_; - absl::optional send_crypto_suite_; - absl::optional recv_crypto_suite_; - rtc::ZeroOnFreeBuffer send_key_; - rtc::ZeroOnFreeBuffer recv_key_; -}; - -} // namespace cricket - -#endif // PC_SRTP_FILTER_H_ diff --git a/pc/srtp_filter_unittest.cc b/pc/srtp_filter_unittest.cc deleted file mode 100644 index fed023199f..0000000000 --- a/pc/srtp_filter_unittest.cc +++ /dev/null @@ -1,472 +0,0 @@ -/* - * Copyright 2004 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "pc/srtp_filter.h" - -#include - -#include "api/crypto_params.h" -#include "rtc_base/ssl_stream_adapter.h" -#include "test/gtest.h" - -using cricket::CryptoParams; -using cricket::CS_LOCAL; -using cricket::CS_REMOTE; - -namespace rtc { - -static const char kTestKeyParams1[] = - "inline:WVNfX19zZW1jdGwgKCkgewkyMjA7fQp9CnVubGVz"; -static const char kTestKeyParams2[] = - "inline:PS1uQCVeeCFCanVmcjkpPywjNWhcYD0mXXtxaVBR"; -static const char kTestKeyParams3[] = - "inline:1234X19zZW1jdGwgKCkgewkyMjA7fQp9CnVubGVz"; -static const char kTestKeyParams4[] = - "inline:4567QCVeeCFCanVmcjkpPywjNWhcYD0mXXtxaVBR"; -static const char kTestKeyParamsGcm1[] = - "inline:e166KFlKzJsGW0d5apX+rrI05vxbrvMJEzFI14aTDCa63IRTlLK4iH66uOI="; -static const char kTestKeyParamsGcm2[] = - "inline:6X0oCd55zfz4VgtOwsuqcFq61275PDYN5uwuu3p7ZUHbfUY2FMpdP4m2PEo="; -static const char kTestKeyParamsGcm3[] = - "inline:YKlABGZWMgX32xuMotrG0v0T7G83veegaVzubQ=="; -static const char kTestKeyParamsGcm4[] = - "inline:gJ6tWoUym2v+/F6xjr7xaxiS3QbJJozl3ZD/0A=="; -static const cricket::CryptoParams kTestCryptoParams1(1, - "AES_CM_128_HMAC_SHA1_80", - kTestKeyParams1, - ""); -static const cricket::CryptoParams kTestCryptoParams2(1, - "AES_CM_128_HMAC_SHA1_80", - kTestKeyParams2, - ""); -static const cricket::CryptoParams kTestCryptoParamsGcm1(1, - "AEAD_AES_256_GCM", - kTestKeyParamsGcm1, - ""); -static const cricket::CryptoParams kTestCryptoParamsGcm2(1, - "AEAD_AES_256_GCM", - kTestKeyParamsGcm2, - ""); -static const cricket::CryptoParams kTestCryptoParamsGcm3(1, - "AEAD_AES_128_GCM", - kTestKeyParamsGcm3, - ""); -static const cricket::CryptoParams kTestCryptoParamsGcm4(1, - "AEAD_AES_128_GCM", - kTestKeyParamsGcm4, - ""); - -class SrtpFilterTest : public ::testing::Test { - protected: - SrtpFilterTest() {} - static std::vector MakeVector(const CryptoParams& params) { - std::vector vec; - vec.push_back(params); - return vec; - } - - void TestSetParams(const std::vector& params1, - const std::vector& params2) { - EXPECT_TRUE(f1_.SetOffer(params1, CS_LOCAL)); - EXPECT_TRUE(f2_.SetOffer(params1, CS_REMOTE)); - EXPECT_FALSE(f1_.IsActive()); - EXPECT_FALSE(f2_.IsActive()); - EXPECT_TRUE(f2_.SetAnswer(params2, CS_LOCAL)); - EXPECT_TRUE(f1_.SetAnswer(params2, CS_REMOTE)); - EXPECT_TRUE(f1_.IsActive()); - EXPECT_TRUE(f2_.IsActive()); - } - - void VerifyKeysAreEqual(ArrayView key1, - ArrayView key2) { - EXPECT_EQ(key1.size(), key2.size()); - EXPECT_EQ(0, memcmp(key1.data(), key2.data(), key1.size())); - } - - void VerifyCryptoParamsMatch(const std::string& cs1, const std::string& cs2) { - EXPECT_EQ(rtc::SrtpCryptoSuiteFromName(cs1), f1_.send_crypto_suite()); - EXPECT_EQ(rtc::SrtpCryptoSuiteFromName(cs2), f2_.send_crypto_suite()); - VerifyKeysAreEqual(f1_.send_key(), f2_.recv_key()); - VerifyKeysAreEqual(f2_.send_key(), f1_.recv_key()); - } - - cricket::SrtpFilter f1_; - cricket::SrtpFilter f2_; -}; - -// Test that we can set up the session and keys properly. -TEST_F(SrtpFilterTest, TestGoodSetupOneCryptoSuite) { - EXPECT_TRUE(f1_.SetOffer(MakeVector(kTestCryptoParams1), CS_LOCAL)); - EXPECT_FALSE(f1_.IsActive()); - EXPECT_TRUE(f1_.SetAnswer(MakeVector(kTestCryptoParams2), CS_REMOTE)); - EXPECT_TRUE(f1_.IsActive()); -} - -TEST_F(SrtpFilterTest, TestGoodSetupOneCryptoSuiteGcm) { - EXPECT_TRUE(f1_.SetOffer(MakeVector(kTestCryptoParamsGcm1), CS_LOCAL)); - EXPECT_FALSE(f1_.IsActive()); - EXPECT_TRUE(f1_.SetAnswer(MakeVector(kTestCryptoParamsGcm2), CS_REMOTE)); - EXPECT_TRUE(f1_.IsActive()); -} - -// Test that we can set up things with multiple params. -TEST_F(SrtpFilterTest, TestGoodSetupMultipleCryptoSuites) { - std::vector offer(MakeVector(kTestCryptoParams1)); - std::vector answer(MakeVector(kTestCryptoParams2)); - offer.push_back(kTestCryptoParams1); - offer[1].tag = 2; - offer[1].crypto_suite = kCsAesCm128HmacSha1_32; - answer[0].tag = 2; - answer[0].crypto_suite = kCsAesCm128HmacSha1_32; - EXPECT_TRUE(f1_.SetOffer(offer, CS_LOCAL)); - EXPECT_FALSE(f1_.IsActive()); - EXPECT_TRUE(f1_.SetAnswer(answer, CS_REMOTE)); - EXPECT_TRUE(f1_.IsActive()); -} - -TEST_F(SrtpFilterTest, TestGoodSetupMultipleCryptoSuitesGcm) { - std::vector offer(MakeVector(kTestCryptoParamsGcm1)); - std::vector answer(MakeVector(kTestCryptoParamsGcm3)); - offer.push_back(kTestCryptoParamsGcm4); - offer[1].tag = 2; - answer[0].tag = 2; - EXPECT_TRUE(f1_.SetOffer(offer, CS_LOCAL)); - EXPECT_FALSE(f1_.IsActive()); - EXPECT_TRUE(f1_.SetAnswer(answer, CS_REMOTE)); - EXPECT_TRUE(f1_.IsActive()); -} - -// Test that we handle the cases where crypto is not desired. -TEST_F(SrtpFilterTest, TestGoodSetupNoCryptoSuites) { - std::vector offer, answer; - EXPECT_TRUE(f1_.SetOffer(offer, CS_LOCAL)); - EXPECT_TRUE(f1_.SetAnswer(answer, CS_REMOTE)); - EXPECT_FALSE(f1_.IsActive()); -} - -// Test that we handle the cases where crypto is not desired by the remote side. -TEST_F(SrtpFilterTest, TestGoodSetupNoAnswerCryptoSuites) { - std::vector answer; - EXPECT_TRUE(f1_.SetOffer(MakeVector(kTestCryptoParams1), CS_LOCAL)); - EXPECT_TRUE(f1_.SetAnswer(answer, CS_REMOTE)); - EXPECT_FALSE(f1_.IsActive()); -} - -// Test that we fail if we call the functions the wrong way. -TEST_F(SrtpFilterTest, TestBadSetup) { - std::vector offer(MakeVector(kTestCryptoParams1)); - std::vector answer(MakeVector(kTestCryptoParams2)); - EXPECT_FALSE(f1_.SetAnswer(answer, CS_LOCAL)); - EXPECT_FALSE(f1_.SetAnswer(answer, CS_REMOTE)); - EXPECT_TRUE(f1_.SetOffer(offer, CS_LOCAL)); - EXPECT_FALSE(f1_.SetAnswer(answer, CS_LOCAL)); - EXPECT_FALSE(f1_.IsActive()); -} - -// Test that we can set offer multiple times from the same source. -TEST_F(SrtpFilterTest, TestGoodSetupMultipleOffers) { - EXPECT_TRUE(f1_.SetOffer(MakeVector(kTestCryptoParams1), CS_LOCAL)); - EXPECT_TRUE(f1_.SetOffer(MakeVector(kTestCryptoParams2), CS_LOCAL)); - EXPECT_FALSE(f1_.IsActive()); - EXPECT_TRUE(f1_.SetAnswer(MakeVector(kTestCryptoParams2), CS_REMOTE)); - EXPECT_TRUE(f1_.IsActive()); - EXPECT_TRUE(f1_.SetOffer(MakeVector(kTestCryptoParams1), CS_LOCAL)); - EXPECT_TRUE(f1_.SetOffer(MakeVector(kTestCryptoParams2), CS_LOCAL)); - EXPECT_TRUE(f1_.SetAnswer(MakeVector(kTestCryptoParams2), CS_REMOTE)); - - EXPECT_TRUE(f2_.SetOffer(MakeVector(kTestCryptoParams1), CS_REMOTE)); - EXPECT_TRUE(f2_.SetOffer(MakeVector(kTestCryptoParams2), CS_REMOTE)); - EXPECT_FALSE(f2_.IsActive()); - EXPECT_TRUE(f2_.SetAnswer(MakeVector(kTestCryptoParams2), CS_LOCAL)); - EXPECT_TRUE(f2_.IsActive()); - EXPECT_TRUE(f2_.SetOffer(MakeVector(kTestCryptoParams1), CS_REMOTE)); - EXPECT_TRUE(f2_.SetOffer(MakeVector(kTestCryptoParams2), CS_REMOTE)); - EXPECT_TRUE(f2_.SetAnswer(MakeVector(kTestCryptoParams2), CS_LOCAL)); -} -// Test that we can't set offer multiple times from different sources. -TEST_F(SrtpFilterTest, TestBadSetupMultipleOffers) { - EXPECT_TRUE(f1_.SetOffer(MakeVector(kTestCryptoParams1), CS_LOCAL)); - EXPECT_FALSE(f1_.SetOffer(MakeVector(kTestCryptoParams2), CS_REMOTE)); - EXPECT_FALSE(f1_.IsActive()); - EXPECT_TRUE(f1_.SetAnswer(MakeVector(kTestCryptoParams1), CS_REMOTE)); - EXPECT_TRUE(f1_.IsActive()); - EXPECT_TRUE(f1_.SetOffer(MakeVector(kTestCryptoParams2), CS_LOCAL)); - EXPECT_FALSE(f1_.SetOffer(MakeVector(kTestCryptoParams1), CS_REMOTE)); - EXPECT_TRUE(f1_.SetAnswer(MakeVector(kTestCryptoParams2), CS_REMOTE)); - - EXPECT_TRUE(f2_.SetOffer(MakeVector(kTestCryptoParams2), CS_REMOTE)); - EXPECT_FALSE(f2_.SetOffer(MakeVector(kTestCryptoParams1), CS_LOCAL)); - EXPECT_FALSE(f2_.IsActive()); - EXPECT_TRUE(f2_.SetAnswer(MakeVector(kTestCryptoParams2), CS_LOCAL)); - EXPECT_TRUE(f2_.IsActive()); - EXPECT_TRUE(f2_.SetOffer(MakeVector(kTestCryptoParams2), CS_REMOTE)); - EXPECT_FALSE(f2_.SetOffer(MakeVector(kTestCryptoParams1), CS_LOCAL)); - EXPECT_TRUE(f2_.SetAnswer(MakeVector(kTestCryptoParams2), CS_LOCAL)); -} - -// Test that we fail if we have params in the answer when none were offered. -TEST_F(SrtpFilterTest, TestNoAnswerCryptoSuites) { - std::vector offer; - EXPECT_TRUE(f1_.SetOffer(offer, CS_LOCAL)); - EXPECT_FALSE(f1_.SetAnswer(MakeVector(kTestCryptoParams2), CS_REMOTE)); - EXPECT_FALSE(f1_.IsActive()); -} - -// Test that we fail if we have too many params in our answer. -TEST_F(SrtpFilterTest, TestMultipleAnswerCryptoSuites) { - std::vector answer(MakeVector(kTestCryptoParams2)); - answer.push_back(kTestCryptoParams2); - answer[1].tag = 2; - answer[1].crypto_suite = kCsAesCm128HmacSha1_32; - EXPECT_TRUE(f1_.SetOffer(MakeVector(kTestCryptoParams1), CS_LOCAL)); - EXPECT_FALSE(f1_.SetAnswer(answer, CS_REMOTE)); - EXPECT_FALSE(f1_.IsActive()); -} - -// Test that we fail if we don't support the crypto suite. -TEST_F(SrtpFilterTest, TestInvalidCryptoSuite) { - std::vector offer(MakeVector(kTestCryptoParams1)); - std::vector answer(MakeVector(kTestCryptoParams2)); - offer[0].crypto_suite = answer[0].crypto_suite = "FOO"; - EXPECT_TRUE(f1_.SetOffer(offer, CS_LOCAL)); - EXPECT_FALSE(f1_.SetAnswer(answer, CS_REMOTE)); - EXPECT_FALSE(f1_.IsActive()); -} - -// Test that we fail if we can't agree on a tag. -TEST_F(SrtpFilterTest, TestNoMatchingTag) { - std::vector offer(MakeVector(kTestCryptoParams1)); - std::vector answer(MakeVector(kTestCryptoParams2)); - answer[0].tag = 99; - EXPECT_TRUE(f1_.SetOffer(offer, CS_LOCAL)); - EXPECT_FALSE(f1_.SetAnswer(answer, CS_REMOTE)); - EXPECT_FALSE(f1_.IsActive()); -} - -// Test that we fail if we can't agree on a crypto suite. -TEST_F(SrtpFilterTest, TestNoMatchingCryptoSuite) { - std::vector offer(MakeVector(kTestCryptoParams1)); - std::vector answer(MakeVector(kTestCryptoParams2)); - answer[0].tag = 2; - answer[0].crypto_suite = "FOO"; - EXPECT_TRUE(f1_.SetOffer(offer, CS_LOCAL)); - EXPECT_FALSE(f1_.SetAnswer(answer, CS_REMOTE)); - EXPECT_FALSE(f1_.IsActive()); -} - -// Test that we fail keys with bad base64 content. -TEST_F(SrtpFilterTest, TestInvalidKeyData) { - std::vector offer(MakeVector(kTestCryptoParams1)); - std::vector answer(MakeVector(kTestCryptoParams2)); - answer[0].key_params = "inline:!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"; - EXPECT_TRUE(f1_.SetOffer(offer, CS_LOCAL)); - EXPECT_FALSE(f1_.SetAnswer(answer, CS_REMOTE)); - EXPECT_FALSE(f1_.IsActive()); -} - -// Test that we fail keys with the wrong key-method. -TEST_F(SrtpFilterTest, TestWrongKeyMethod) { - std::vector offer(MakeVector(kTestCryptoParams1)); - std::vector answer(MakeVector(kTestCryptoParams2)); - answer[0].key_params = "outline:PS1uQCVeeCFCanVmcjkpPywjNWhcYD0mXXtxaVBR"; - EXPECT_TRUE(f1_.SetOffer(offer, CS_LOCAL)); - EXPECT_FALSE(f1_.SetAnswer(answer, CS_REMOTE)); - EXPECT_FALSE(f1_.IsActive()); -} - -// Test that we fail keys of the wrong length. -TEST_F(SrtpFilterTest, TestKeyTooShort) { - std::vector offer(MakeVector(kTestCryptoParams1)); - std::vector answer(MakeVector(kTestCryptoParams2)); - answer[0].key_params = "inline:PS1uQCVeeCFCanVmcjkpPywjNWhcYD0mXXtx"; - EXPECT_TRUE(f1_.SetOffer(offer, CS_LOCAL)); - EXPECT_FALSE(f1_.SetAnswer(answer, CS_REMOTE)); - EXPECT_FALSE(f1_.IsActive()); -} - -// Test that we fail keys of the wrong length. -TEST_F(SrtpFilterTest, TestKeyTooLong) { - std::vector offer(MakeVector(kTestCryptoParams1)); - std::vector answer(MakeVector(kTestCryptoParams2)); - answer[0].key_params = "inline:PS1uQCVeeCFCanVmcjkpPywjNWhcYD0mXXtxaVBRABCD"; - EXPECT_TRUE(f1_.SetOffer(offer, CS_LOCAL)); - EXPECT_FALSE(f1_.SetAnswer(answer, CS_REMOTE)); - EXPECT_FALSE(f1_.IsActive()); -} - -// Test that we fail keys with lifetime or MKI set (since we don't support) -TEST_F(SrtpFilterTest, TestUnsupportedOptions) { - std::vector offer(MakeVector(kTestCryptoParams1)); - std::vector answer(MakeVector(kTestCryptoParams2)); - answer[0].key_params = - "inline:PS1uQCVeeCFCanVmcjkpPywjNWhcYD0mXXtxaVBR|2^20|1:4"; - EXPECT_TRUE(f1_.SetOffer(offer, CS_LOCAL)); - EXPECT_FALSE(f1_.SetAnswer(answer, CS_REMOTE)); - EXPECT_FALSE(f1_.IsActive()); -} - -// Test that we can encrypt/decrypt after negotiating AES_CM_128_HMAC_SHA1_80. -TEST_F(SrtpFilterTest, TestProtect_AES_CM_128_HMAC_SHA1_80) { - std::vector offer(MakeVector(kTestCryptoParams1)); - std::vector answer(MakeVector(kTestCryptoParams2)); - offer.push_back(kTestCryptoParams1); - offer[1].tag = 2; - offer[1].crypto_suite = kCsAesCm128HmacSha1_32; - TestSetParams(offer, answer); - VerifyCryptoParamsMatch(kCsAesCm128HmacSha1_80, kCsAesCm128HmacSha1_80); -} - -// Test that we can encrypt/decrypt after negotiating AES_CM_128_HMAC_SHA1_32. -TEST_F(SrtpFilterTest, TestProtect_AES_CM_128_HMAC_SHA1_32) { - std::vector offer(MakeVector(kTestCryptoParams1)); - std::vector answer(MakeVector(kTestCryptoParams2)); - offer.push_back(kTestCryptoParams1); - offer[1].tag = 2; - offer[1].crypto_suite = kCsAesCm128HmacSha1_32; - answer[0].tag = 2; - answer[0].crypto_suite = kCsAesCm128HmacSha1_32; - TestSetParams(offer, answer); - VerifyCryptoParamsMatch(kCsAesCm128HmacSha1_32, kCsAesCm128HmacSha1_32); -} - -// Test that we can change encryption parameters. -TEST_F(SrtpFilterTest, TestChangeParameters) { - std::vector offer(MakeVector(kTestCryptoParams1)); - std::vector answer(MakeVector(kTestCryptoParams2)); - - TestSetParams(offer, answer); - VerifyCryptoParamsMatch(kCsAesCm128HmacSha1_80, kCsAesCm128HmacSha1_80); - - // Change the key parameters and crypto_suite. - offer[0].key_params = kTestKeyParams3; - offer[0].crypto_suite = kCsAesCm128HmacSha1_32; - answer[0].key_params = kTestKeyParams4; - answer[0].crypto_suite = kCsAesCm128HmacSha1_32; - - EXPECT_TRUE(f1_.SetOffer(offer, CS_LOCAL)); - EXPECT_TRUE(f2_.SetOffer(offer, CS_REMOTE)); - EXPECT_TRUE(f1_.IsActive()); - EXPECT_TRUE(f1_.IsActive()); - - // Test that the old keys are valid until the negotiation is complete. - VerifyCryptoParamsMatch(kCsAesCm128HmacSha1_80, kCsAesCm128HmacSha1_80); - - // Complete the negotiation and test that we can still understand each other. - EXPECT_TRUE(f2_.SetAnswer(answer, CS_LOCAL)); - EXPECT_TRUE(f1_.SetAnswer(answer, CS_REMOTE)); - - VerifyCryptoParamsMatch(kCsAesCm128HmacSha1_32, kCsAesCm128HmacSha1_32); -} - -// Test that we can send and receive provisional answers with crypto enabled. -// Also test that we can change the crypto. -TEST_F(SrtpFilterTest, TestProvisionalAnswer) { - std::vector offer(MakeVector(kTestCryptoParams1)); - offer.push_back(kTestCryptoParams1); - offer[1].tag = 2; - offer[1].crypto_suite = kCsAesCm128HmacSha1_32; - std::vector answer(MakeVector(kTestCryptoParams2)); - - EXPECT_TRUE(f1_.SetOffer(offer, CS_LOCAL)); - EXPECT_TRUE(f2_.SetOffer(offer, CS_REMOTE)); - EXPECT_FALSE(f1_.IsActive()); - EXPECT_FALSE(f2_.IsActive()); - EXPECT_TRUE(f2_.SetProvisionalAnswer(answer, CS_LOCAL)); - EXPECT_TRUE(f1_.SetProvisionalAnswer(answer, CS_REMOTE)); - EXPECT_TRUE(f1_.IsActive()); - EXPECT_TRUE(f2_.IsActive()); - VerifyCryptoParamsMatch(kCsAesCm128HmacSha1_80, kCsAesCm128HmacSha1_80); - - answer[0].key_params = kTestKeyParams4; - answer[0].tag = 2; - answer[0].crypto_suite = kCsAesCm128HmacSha1_32; - EXPECT_TRUE(f2_.SetAnswer(answer, CS_LOCAL)); - EXPECT_TRUE(f1_.SetAnswer(answer, CS_REMOTE)); - EXPECT_TRUE(f1_.IsActive()); - EXPECT_TRUE(f2_.IsActive()); - VerifyCryptoParamsMatch(kCsAesCm128HmacSha1_32, kCsAesCm128HmacSha1_32); -} - -// Test that a provisional answer doesn't need to contain a crypto. -TEST_F(SrtpFilterTest, TestProvisionalAnswerWithoutCrypto) { - std::vector offer(MakeVector(kTestCryptoParams1)); - std::vector answer; - - EXPECT_TRUE(f1_.SetOffer(offer, CS_LOCAL)); - EXPECT_TRUE(f2_.SetOffer(offer, CS_REMOTE)); - EXPECT_FALSE(f1_.IsActive()); - EXPECT_FALSE(f2_.IsActive()); - EXPECT_TRUE(f2_.SetProvisionalAnswer(answer, CS_LOCAL)); - EXPECT_TRUE(f1_.SetProvisionalAnswer(answer, CS_REMOTE)); - EXPECT_FALSE(f1_.IsActive()); - EXPECT_FALSE(f2_.IsActive()); - - answer.push_back(kTestCryptoParams2); - EXPECT_TRUE(f2_.SetAnswer(answer, CS_LOCAL)); - EXPECT_TRUE(f1_.SetAnswer(answer, CS_REMOTE)); - EXPECT_TRUE(f1_.IsActive()); - EXPECT_TRUE(f2_.IsActive()); - VerifyCryptoParamsMatch(kCsAesCm128HmacSha1_80, kCsAesCm128HmacSha1_80); -} - -// Test that if we get a new local offer after a provisional answer -// with no crypto, that we are in an inactive state. -TEST_F(SrtpFilterTest, TestLocalOfferAfterProvisionalAnswerWithoutCrypto) { - std::vector offer(MakeVector(kTestCryptoParams1)); - std::vector answer; - - EXPECT_TRUE(f1_.SetOffer(offer, CS_LOCAL)); - EXPECT_TRUE(f2_.SetOffer(offer, CS_REMOTE)); - EXPECT_TRUE(f1_.SetProvisionalAnswer(answer, CS_REMOTE)); - EXPECT_TRUE(f2_.SetProvisionalAnswer(answer, CS_LOCAL)); - EXPECT_FALSE(f1_.IsActive()); - EXPECT_FALSE(f2_.IsActive()); - // The calls to set an offer after a provisional answer fail, so the - // state doesn't change. - EXPECT_FALSE(f1_.SetOffer(offer, CS_LOCAL)); - EXPECT_FALSE(f2_.SetOffer(offer, CS_REMOTE)); - EXPECT_FALSE(f1_.IsActive()); - EXPECT_FALSE(f2_.IsActive()); - - answer.push_back(kTestCryptoParams2); - EXPECT_TRUE(f2_.SetAnswer(answer, CS_LOCAL)); - EXPECT_TRUE(f1_.SetAnswer(answer, CS_REMOTE)); - EXPECT_TRUE(f1_.IsActive()); - EXPECT_TRUE(f2_.IsActive()); - VerifyCryptoParamsMatch(kCsAesCm128HmacSha1_80, kCsAesCm128HmacSha1_80); -} - -// Test that we can disable encryption. -TEST_F(SrtpFilterTest, TestDisableEncryption) { - std::vector offer(MakeVector(kTestCryptoParams1)); - std::vector answer(MakeVector(kTestCryptoParams2)); - - TestSetParams(offer, answer); - VerifyCryptoParamsMatch(kCsAesCm128HmacSha1_80, kCsAesCm128HmacSha1_80); - - offer.clear(); - answer.clear(); - EXPECT_TRUE(f1_.SetOffer(offer, CS_LOCAL)); - EXPECT_TRUE(f2_.SetOffer(offer, CS_REMOTE)); - EXPECT_TRUE(f1_.IsActive()); - EXPECT_TRUE(f2_.IsActive()); - - // Test that the old keys are valid until the negotiation is complete. - VerifyCryptoParamsMatch(kCsAesCm128HmacSha1_80, kCsAesCm128HmacSha1_80); - - // Complete the negotiation. - EXPECT_TRUE(f2_.SetAnswer(answer, CS_LOCAL)); - EXPECT_TRUE(f1_.SetAnswer(answer, CS_REMOTE)); - - EXPECT_FALSE(f1_.IsActive()); - EXPECT_FALSE(f2_.IsActive()); -} - -} // namespace rtc diff --git a/pc/srtp_transport.h b/pc/srtp_transport.h index 29721f3b68..bad4adc135 100644 --- a/pc/srtp_transport.h +++ b/pc/srtp_transport.h @@ -19,7 +19,6 @@ #include #include "absl/types/optional.h" -#include "api/crypto_params.h" #include "api/field_trials_view.h" #include "api/rtc_error.h" #include "p2p/base/packet_transport_internal.h" @@ -154,8 +153,6 @@ class SrtpTransport : public RtpTransport { std::unique_ptr send_rtcp_session_; std::unique_ptr recv_rtcp_session_; - absl::optional send_params_; - absl::optional recv_params_; absl::optional send_crypto_suite_; absl::optional recv_crypto_suite_; rtc::ZeroOnFreeBuffer send_key_; diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc index a255233ccc..6b63eb8a6e 100644 --- a/pc/webrtc_sdp.cc +++ b/pc/webrtc_sdp.cc @@ -29,7 +29,6 @@ #include "absl/strings/ascii.h" #include "absl/strings/match.h" #include "api/candidate.h" -#include "api/crypto_params.h" #include "api/jsep_ice_candidate.h" #include "api/jsep_session_description.h" #include "api/media_types.h" @@ -74,7 +73,6 @@ using cricket::AudioContentDescription; using cricket::Candidate; using cricket::Candidates; using cricket::ContentInfo; -using cricket::CryptoParams; using cricket::ICE_CANDIDATE_COMPONENT_RTCP; using cricket::ICE_CANDIDATE_COMPONENT_RTP; using cricket::kApplicationSpecificBandwidth; @@ -157,7 +155,6 @@ static const char kSsrcAttributeMsid[] = "msid"; static const char kDefaultMsid[] = "default"; static const char kNoStreamMsid[] = "-"; static const char kAttributeSsrcGroup[] = "ssrc-group"; -static const char kAttributeCrypto[] = "crypto"; static const char kAttributeCandidate[] = "candidate"; static const char kAttributeCandidateTyp[] = "typ"; static const char kAttributeCandidateRaddr[] = "raddr"; @@ -340,9 +337,6 @@ static bool ParseSsrcAttribute(absl::string_view line, static bool ParseSsrcGroupAttribute(absl::string_view line, SsrcGroupVec* ssrc_groups, SdpParseError* error); -static bool ParseCryptoAttribute(absl::string_view line, - MediaContentDescription* media_desc, - SdpParseError* error); static bool ParseRtpmapAttribute(absl::string_view line, const cricket::MediaType media_type, const std::vector& payload_types, @@ -1668,18 +1662,6 @@ void BuildRtpContentAttributes(const MediaContentDescription* media_desc, AddLine(os.str(), message); } - // RFC 4568 - // a=crypto: [] - for (const CryptoParams& crypto_params : media_desc->cryptos()) { - InitAttrLine(kAttributeCrypto, &os); - os << kSdpDelimiterColon << crypto_params.tag << " " - << crypto_params.crypto_suite << " " << crypto_params.key_params; - if (!crypto_params.session_params.empty()) { - os << " " << crypto_params.session_params; - } - AddLine(os.str(), message); - } - // RFC 4566 // a=rtpmap: / // [/] @@ -3217,10 +3199,6 @@ bool ParseContent(absl::string_view message, if (!ParseSsrcAttribute(*line, &ssrc_infos, msid_signaling, error)) { return false; } - } else if (HasAttribute(*line, kAttributeCrypto)) { - if (!ParseCryptoAttribute(*line, media_desc, error)) { - return false; - } } else if (HasAttribute(*line, kAttributeRtpmap)) { if (!ParseRtpmapAttribute(*line, media_type, payload_types, media_desc, error)) { @@ -3548,37 +3526,6 @@ bool ParseSsrcGroupAttribute(absl::string_view line, return true; } -bool ParseCryptoAttribute(absl::string_view line, - MediaContentDescription* media_desc, - SdpParseError* error) { - std::vector fields = - rtc::split(line.substr(kLinePrefixLength), kSdpDelimiterSpaceChar); - // RFC 4568 - // a=crypto: [] - const size_t expected_min_fields = 3; - if (fields.size() < expected_min_fields) { - return ParseFailedExpectMinFieldNum(line, expected_min_fields, error); - } - std::string tag_value; - if (!GetValue(fields[0], kAttributeCrypto, &tag_value, error)) { - return false; - } - int tag = 0; - if (!GetValueFromString(line, tag_value, &tag, error)) { - return false; - } - const absl::string_view crypto_suite = fields[1]; - const absl::string_view key_params = fields[2]; - absl::string_view session_params; - if (fields.size() > 3) { - session_params = fields[3]; - } - - media_desc->AddCrypto( - CryptoParams(tag, crypto_suite, key_params, session_params)); - return true; -} - // Updates or creates a new codec entry in the audio description with according // to `name`, `clockrate`, `bitrate`, and `channels`. void UpdateCodec(int payload_type, diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc index 382a4967d5..6811381bb7 100644 --- a/pc/webrtc_sdp_unittest.cc +++ b/pc/webrtc_sdp_unittest.cc @@ -25,7 +25,6 @@ #include "absl/strings/string_view.h" #include "absl/types/optional.h" #include "api/array_view.h" -#include "api/crypto_params.h" #include "api/jsep_session_description.h" #include "api/media_types.h" #include "api/rtp_parameters.h" @@ -59,7 +58,6 @@ using cricket::AudioContentDescription; using cricket::Candidate; using cricket::ContentGroup; using cricket::ContentInfo; -using cricket::CryptoParams; using cricket::ICE_CANDIDATE_COMPONENT_RTCP; using cricket::ICE_CANDIDATE_COMPONENT_RTP; using cricket::kFecSsrcGroupSemantics; @@ -1343,20 +1341,6 @@ class WebRtcSdpTest : public ::testing::Test { // rtcp_reduced_size EXPECT_EQ(cd1->rtcp_reduced_size(), cd2->rtcp_reduced_size()); - // cryptos - EXPECT_EQ(cd1->cryptos().size(), cd2->cryptos().size()); - if (cd1->cryptos().size() != cd2->cryptos().size()) { - ADD_FAILURE(); - return; - } - for (size_t i = 0; i < cd1->cryptos().size(); ++i) { - const CryptoParams c1 = cd1->cryptos().at(i); - const CryptoParams c2 = cd2->cryptos().at(i); - EXPECT_TRUE(c1.Matches(c2)); - EXPECT_EQ(c1.key_params, c2.key_params); - EXPECT_EQ(c1.session_params, c2.session_params); - } - // protocol // Use an equivalence class here, for old and new versions of the // protocol description. @@ -1631,11 +1615,6 @@ class WebRtcSdpTest : public ::testing::Test { absl::WrapUnique(video_desc_)); } - void RemoveCryptos() { - audio_desc_->set_cryptos(std::vector()); - video_desc_->set_cryptos(std::vector()); - } - // Removes everything in StreamParams from the session description that is // used for a=ssrc lines. void RemoveSsrcSignalingFromStreamParams() { diff --git a/pc/webrtc_session_description_factory.cc b/pc/webrtc_session_description_factory.cc index 42a8da3e70..9919260aa3 100644 --- a/pc/webrtc_session_description_factory.cc +++ b/pc/webrtc_session_description_factory.cc @@ -128,13 +128,10 @@ WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory( RTC_DCHECK(signaling_thread_); if (!dtls_enabled) { - SetSdesPolicy(cricket::SEC_REQUIRED); - RTC_LOG(LS_VERBOSE) << "DTLS-SRTP disabled."; + RTC_LOG(LS_INFO) << "DTLS-SRTP disabled"; + transport_desc_factory_.SetInsecureForTesting(); return; } - - // SRTP-SDES is disabled if DTLS is on. - SetSdesPolicy(cricket::SEC_DISABLED); if (certificate) { // Use `certificate`. certificate_request_state_ = CERTIFICATE_WAITING; @@ -142,31 +139,32 @@ WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory( RTC_LOG(LS_VERBOSE) << "DTLS-SRTP enabled; has certificate parameter."; RTC_LOG(LS_INFO) << "Using certificate supplied to the constructor."; SetCertificate(certificate); - } else { - // Generate certificate. - certificate_request_state_ = CERTIFICATE_WAITING; - - auto callback = [weak_ptr = weak_factory_.GetWeakPtr()]( - rtc::scoped_refptr certificate) { - if (!weak_ptr) { - return; - } - if (certificate) { - weak_ptr->SetCertificate(std::move(certificate)); - } else { - weak_ptr->OnCertificateRequestFailed(); - } - }; - - rtc::KeyParams key_params = rtc::KeyParams(); - RTC_LOG(LS_VERBOSE) - << "DTLS-SRTP enabled; sending DTLS identity request (key type: " - << key_params.type() << ")."; - - // Request certificate. This happens asynchronously on a different thread. - cert_generator_->GenerateCertificateAsync(key_params, absl::nullopt, - std::move(callback)); + return; } + // Generate certificate. + RTC_DCHECK(cert_generator_); + certificate_request_state_ = CERTIFICATE_WAITING; + + auto callback = [weak_ptr = weak_factory_.GetWeakPtr()]( + rtc::scoped_refptr certificate) { + if (!weak_ptr) { + return; + } + if (certificate) { + weak_ptr->SetCertificate(std::move(certificate)); + } else { + weak_ptr->OnCertificateRequestFailed(); + } + }; + + rtc::KeyParams key_params = rtc::KeyParams(); + RTC_LOG(LS_VERBOSE) + << "DTLS-SRTP enabled; sending DTLS identity request (key type: " + << key_params.type() << ")."; + + // Request certificate. This happens asynchronously on a different thread. + cert_generator_->GenerateCertificateAsync(key_params, absl::nullopt, + std::move(callback)); } WebRtcSessionDescriptionFactory::~WebRtcSessionDescriptionFactory() { @@ -258,15 +256,6 @@ void WebRtcSessionDescriptionFactory::CreateAnswer( } } -void WebRtcSessionDescriptionFactory::SetSdesPolicy( - cricket::SecurePolicy secure_policy) { - session_desc_factory_.set_secure(secure_policy); -} - -cricket::SecurePolicy WebRtcSessionDescriptionFactory::SdesPolicy() const { - return session_desc_factory_.secure(); -} - void WebRtcSessionDescriptionFactory::InternalCreateOffer( CreateSessionDescriptionRequest request) { if (sdp_info_->local_description()) { @@ -450,7 +439,6 @@ void WebRtcSessionDescriptionFactory::SetCertificate( on_certificate_ready_(certificate); transport_desc_factory_.set_certificate(std::move(certificate)); - transport_desc_factory_.set_secure(cricket::SEC_ENABLED); while (!create_session_description_requests_.empty()) { if (create_session_description_requests_.front().type == @@ -462,4 +450,5 @@ void WebRtcSessionDescriptionFactory::SetCertificate( create_session_description_requests_.pop(); } } + } // namespace webrtc diff --git a/pc/webrtc_session_description_factory.h b/pc/webrtc_session_description_factory.h index 22ead41d9b..eed922eda7 100644 --- a/pc/webrtc_session_description_factory.h +++ b/pc/webrtc_session_description_factory.h @@ -73,9 +73,6 @@ class WebRtcSessionDescriptionFactory { void CreateAnswer(CreateSessionDescriptionObserver* observer, const cricket::MediaSessionOptions& session_options); - void SetSdesPolicy(cricket::SecurePolicy secure_policy); - cricket::SecurePolicy SdesPolicy() const; - void set_enable_encrypted_rtp_header_extensions(bool enable) { session_desc_factory_.set_enable_encrypted_rtp_header_extensions(enable); } @@ -88,6 +85,9 @@ class WebRtcSessionDescriptionFactory { bool waiting_for_certificate_for_testing() const { return certificate_request_state_ == CERTIFICATE_WAITING; } + void SetInsecureForTesting() { + transport_desc_factory_.SetInsecureForTesting(); + } private: enum CertificateRequestState { @@ -144,6 +144,7 @@ class WebRtcSessionDescriptionFactory { std::function&)> on_certificate_ready_; + rtc::WeakPtrFactory weak_factory_{this}; }; } // namespace webrtc diff --git a/test/peer_scenario/tests/unsignaled_stream_test.cc b/test/peer_scenario/tests/unsignaled_stream_test.cc index 4f478b4b2a..dced274e68 100644 --- a/test/peer_scenario/tests/unsignaled_stream_test.cc +++ b/test/peer_scenario/tests/unsignaled_stream_test.cc @@ -98,7 +98,6 @@ TEST_P(UnsignaledStreamTest, ReplacesUnsignaledStreamOnCompletedSignaling) { PeerScenarioClient::Config config = PeerScenarioClient::Config(); // Disable encryption so that we can inject a fake early media packet without // triggering srtp failures. - config.disable_encryption = true; auto* caller = s.CreateClient(config); auto* callee = s.CreateClient(config);