admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Enable SRTP GCM ciphers by default

Browse Source 
Bug: webrtc:15178
Change-Id: I0216ce8f194fffc820723d82b9c04a76573c2f4f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/305381
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40828}
This commit is contained in:
Saúl Ibarra Corretgé 2023-09-26 16:15:01 +02:00 committed by WebRTC LUCI CQ
parent 98e71f57ea
commit d8633868b3
4 changed files with 14 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
api/crypto/crypto_options.cc
View File

@ -23,13 +23,6 @@ CryptoOptions::CryptoOptions(const CryptoOptions& other) {
CryptoOptions::~CryptoOptions() {} CryptoOptions::~CryptoOptions() {}
// static
CryptoOptions CryptoOptions::NoGcm() {
CryptoOptions options;
options.srtp.enable_gcm_crypto_suites = false;
return options;
}
std::vector<int> CryptoOptions::GetSupportedDtlsSrtpCryptoSuites() const { std::vector<int> CryptoOptions::GetSupportedDtlsSrtpCryptoSuites() const {
std::vector<int> crypto_suites; std::vector<int> crypto_suites;
// Note: kSrtpAes128CmSha1_80 is what is required to be supported (by // Note: kSrtpAes128CmSha1_80 is what is required to be supported (by

7
api/crypto/crypto_options.h
View File

@ -25,11 +25,6 @@ struct RTC_EXPORT CryptoOptions {
CryptoOptions(const CryptoOptions& other); CryptoOptions(const CryptoOptions& other);
~CryptoOptions(); ~CryptoOptions();
// Helper method to return an instance of the CryptoOptions with GCM crypto
// suites disabled. This method should be used instead of depending on current
// default values set by the constructor.
static CryptoOptions NoGcm();
// Returns a list of the supported DTLS-SRTP Crypto suites based on this set // Returns a list of the supported DTLS-SRTP Crypto suites based on this set
// of crypto options. // of crypto options.
std::vector<int> GetSupportedDtlsSrtpCryptoSuites() const; std::vector<int> GetSupportedDtlsSrtpCryptoSuites() const;
@ -41,7 +36,7 @@ struct RTC_EXPORT CryptoOptions {
struct Srtp { struct Srtp {
// Enable GCM crypto suites from RFC 7714 for SRTP. GCM will only be used // Enable GCM crypto suites from RFC 7714 for SRTP. GCM will only be used
// if both sides enable it. // if both sides enable it.
bool enable_gcm_crypto_suites = false; bool enable_gcm_crypto_suites = true;
// If set to true, the (potentially insecure) crypto cipher // If set to true, the (potentially insecure) crypto cipher
// kSrtpAes128CmSha1_32 will be included in the list of supported ciphers // kSrtpAes128CmSha1_32 will be included in the list of supported ciphers

2
api/peer_connection_interface.h
View File

@ -1493,7 +1493,7 @@ class RTC_EXPORT PeerConnectionFactoryInterface
rtc::SSLProtocolVersion ssl_max_version = rtc::SSL_PROTOCOL_DTLS_12; rtc::SSLProtocolVersion ssl_max_version = rtc::SSL_PROTOCOL_DTLS_12;
// Sets crypto related options, e.g. enabled cipher suites. // Sets crypto related options, e.g. enabled cipher suites.
CryptoOptions crypto_options = CryptoOptions::NoGcm(); CryptoOptions crypto_options = {};
}; };
// Set the options to be used for subsequently created PeerConnections. // Set the options to be used for subsequently created PeerConnections.

25
pc/media_session_unittest.cc
View File

@ -284,6 +284,7 @@ static const char* kMediaProtocolsDtls[] = {
// default changes. // default changes.
static const char* kDefaultSrtpCryptoSuite = kCsAesCm128HmacSha1_80; static const char* kDefaultSrtpCryptoSuite = kCsAesCm128HmacSha1_80;
static const char* kDefaultSrtpCryptoSuiteGcm = kCsAeadAes256Gcm; static const char* kDefaultSrtpCryptoSuiteGcm = kCsAeadAes256Gcm;
static const uint8_t kDefaultCryptoSuiteSize = 3U;
// These constants are used to make the code using "AddMediaDescriptionOptions" // These constants are used to make the code using "AddMediaDescriptionOptions"
// more readable. // more readable.
@ -622,9 +623,8 @@ class MediaSessionDescriptionFactoryTest : public ::testing::Test {
ASSERT_TRUE(video_media_desc); ASSERT_TRUE(video_media_desc);
EXPECT_TRUE(CompareCryptoParams(audio_media_desc->cryptos(), EXPECT_TRUE(CompareCryptoParams(audio_media_desc->cryptos(),
video_media_desc->cryptos())); video_media_desc->cryptos()));
EXPECT_EQ(1u, audio_media_desc->cryptos().size()); ASSERT_CRYPTO(audio_media_desc, offer ? kDefaultCryptoSuiteSize : 1U,
EXPECT_EQ(kDefaultSrtpCryptoSuite, kDefaultSrtpCryptoSuite);
audio_media_desc->cryptos()[0].crypto_suite);
// Verify the selected crypto is one from the reference audio // Verify the selected crypto is one from the reference audio
// media content. // media content.
@ -819,7 +819,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateAudioOffer) {
EXPECT_EQ(0U, acd->first_ssrc()); // no sender is attached. EXPECT_EQ(0U, acd->first_ssrc()); // no sender is attached.
EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // default bandwidth (auto) EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // default bandwidth (auto)
EXPECT_TRUE(acd->rtcp_mux()); // rtcp-mux defaults on EXPECT_TRUE(acd->rtcp_mux()); // rtcp-mux defaults on
ASSERT_CRYPTO(acd, 1U, kDefaultSrtpCryptoSuite); ASSERT_CRYPTO(acd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite);
EXPECT_EQ(cricket::kMediaProtocolSavpf, acd->protocol()); EXPECT_EQ(cricket::kMediaProtocolSavpf, acd->protocol());
} }
@ -844,14 +844,14 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateVideoOffer) {
EXPECT_EQ(0U, acd->first_ssrc()); // no sender is attached EXPECT_EQ(0U, acd->first_ssrc()); // no sender is attached
EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // default bandwidth (auto) EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // default bandwidth (auto)
EXPECT_TRUE(acd->rtcp_mux()); // rtcp-mux defaults on EXPECT_TRUE(acd->rtcp_mux()); // rtcp-mux defaults on
ASSERT_CRYPTO(acd, 1U, kDefaultSrtpCryptoSuite); ASSERT_CRYPTO(acd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite);
EXPECT_EQ(cricket::kMediaProtocolSavpf, acd->protocol()); EXPECT_EQ(cricket::kMediaProtocolSavpf, acd->protocol());
EXPECT_EQ(MEDIA_TYPE_VIDEO, vcd->type()); EXPECT_EQ(MEDIA_TYPE_VIDEO, vcd->type());
EXPECT_EQ(f1_.video_sendrecv_codecs(), vcd->codecs()); EXPECT_EQ(f1_.video_sendrecv_codecs(), vcd->codecs());
EXPECT_EQ(0U, vcd->first_ssrc()); // no sender is attached EXPECT_EQ(0U, vcd->first_ssrc()); // no sender is attached
EXPECT_EQ(kAutoBandwidth, vcd->bandwidth()); // default bandwidth (auto) EXPECT_EQ(kAutoBandwidth, vcd->bandwidth()); // default bandwidth (auto)
EXPECT_TRUE(vcd->rtcp_mux()); // rtcp-mux defaults on EXPECT_TRUE(vcd->rtcp_mux()); // rtcp-mux defaults on
ASSERT_CRYPTO(vcd, 1U, kDefaultSrtpCryptoSuite); ASSERT_CRYPTO(vcd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite);
EXPECT_EQ(cricket::kMediaProtocolSavpf, vcd->protocol()); EXPECT_EQ(cricket::kMediaProtocolSavpf, vcd->protocol());
} }
@ -1298,7 +1298,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateAudioAnswerGcm) {
f1_.set_secure(SEC_ENABLED); f1_.set_secure(SEC_ENABLED);
f2_.set_secure(SEC_ENABLED); f2_.set_secure(SEC_ENABLED);
MediaSessionOptions opts = CreatePlanBMediaSessionOptions(); MediaSessionOptions opts = CreatePlanBMediaSessionOptions();
opts.crypto_options.srtp.enable_gcm_crypto_suites = true;
std::unique_ptr<SessionDescription> offer = std::unique_ptr<SessionDescription> offer =
f1_.CreateOfferOrError(opts, nullptr).MoveValue(); f1_.CreateOfferOrError(opts, nullptr).MoveValue();
ASSERT_TRUE(offer.get()); ASSERT_TRUE(offer.get());
@ -2475,11 +2474,11 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateMultiStreamVideoOffer) {
EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // default bandwidth (auto) EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // default bandwidth (auto)
EXPECT_TRUE(acd->rtcp_mux()); // rtcp-mux defaults on EXPECT_TRUE(acd->rtcp_mux()); // rtcp-mux defaults on
ASSERT_CRYPTO(acd, 1U, kDefaultSrtpCryptoSuite); ASSERT_CRYPTO(acd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite);
EXPECT_EQ(MEDIA_TYPE_VIDEO, vcd->type()); EXPECT_EQ(MEDIA_TYPE_VIDEO, vcd->type());
EXPECT_EQ(f1_.video_sendrecv_codecs(), vcd->codecs()); EXPECT_EQ(f1_.video_sendrecv_codecs(), vcd->codecs());
ASSERT_CRYPTO(vcd, 1U, kDefaultSrtpCryptoSuite); ASSERT_CRYPTO(vcd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite);
const StreamParamsVec& video_streams = vcd->streams(); const StreamParamsVec& video_streams = vcd->streams();
ASSERT_EQ(1U, video_streams.size()); ASSERT_EQ(1U, video_streams.size());
@ -2512,9 +2511,9 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateMultiStreamVideoOffer) {
EXPECT_EQ(acd->codecs(), updated_acd->codecs()); EXPECT_EQ(acd->codecs(), updated_acd->codecs());
EXPECT_EQ(vcd->type(), updated_vcd->type()); EXPECT_EQ(vcd->type(), updated_vcd->type());
EXPECT_EQ(vcd->codecs(), updated_vcd->codecs()); EXPECT_EQ(vcd->codecs(), updated_vcd->codecs());
ASSERT_CRYPTO(updated_acd, 1U, kDefaultSrtpCryptoSuite); ASSERT_CRYPTO(updated_acd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite);
EXPECT_TRUE(CompareCryptoParams(acd->cryptos(), updated_acd->cryptos())); EXPECT_TRUE(CompareCryptoParams(acd->cryptos(), updated_acd->cryptos()));
ASSERT_CRYPTO(updated_vcd, 1U, kDefaultSrtpCryptoSuite); ASSERT_CRYPTO(updated_vcd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite);
EXPECT_TRUE(CompareCryptoParams(vcd->cryptos(), updated_vcd->cryptos())); EXPECT_TRUE(CompareCryptoParams(vcd->cryptos(), updated_vcd->cryptos()));
const StreamParamsVec& updated_audio_streams = updated_acd->streams(); const StreamParamsVec& updated_audio_streams = updated_acd->streams();
@ -3881,8 +3880,8 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCryptoDtls) {
ASSERT_TRUE(audio_media_desc); ASSERT_TRUE(audio_media_desc);
video_media_desc = offer->GetContentDescriptionByName("video"); video_media_desc = offer->GetContentDescriptionByName("video");
ASSERT_TRUE(video_media_desc); ASSERT_TRUE(video_media_desc);
EXPECT_EQ(1u, audio_media_desc->cryptos().size()); EXPECT_EQ(kDefaultCryptoSuiteSize, audio_media_desc->cryptos().size());
EXPECT_EQ(1u, video_media_desc->cryptos().size()); EXPECT_EQ(kDefaultCryptoSuiteSize, video_media_desc->cryptos().size());
audio_trans_desc = offer->GetTransportDescriptionByName("audio"); audio_trans_desc = offer->GetTransportDescriptionByName("audio");
ASSERT_TRUE(audio_trans_desc); ASSERT_TRUE(audio_trans_desc);