diff --git a/p2p/base/dtls_transport.cc b/p2p/base/dtls_transport.cc index 27d1849d7a..a0a3a35100 100644 --- a/p2p/base/dtls_transport.cc +++ b/p2p/base/dtls_transport.cc @@ -233,6 +233,13 @@ bool DtlsTransport::GetSslCipherSuite(int* cipher) { return dtls_->GetSslCipherSuite(cipher); } +std::optional DtlsTransport::GetTlsCipherSuiteName() const { + if (dtls_state() != webrtc::DtlsTransportState::kConnected) { + return std::nullopt; + } + return dtls_->GetTlsCipherSuiteName(); +} + webrtc::RTCError DtlsTransport::SetRemoteParameters( absl::string_view digest_alg, const uint8_t* digest, diff --git a/p2p/base/dtls_transport.h b/p2p/base/dtls_transport.h index 02249c234b..03a29fa75e 100644 --- a/p2p/base/dtls_transport.h +++ b/p2p/base/dtls_transport.h @@ -169,6 +169,7 @@ class DtlsTransport : public DtlsTransportInternal { // Find out which DTLS cipher was negotiated bool GetSslCipherSuite(int* cipher) override; + std::optional GetTlsCipherSuiteName() const override; // Once DTLS has been established, this method retrieves the certificate // chain in use by the remote peer, for use in external identity diff --git a/p2p/base/dtls_transport_internal.h b/p2p/base/dtls_transport_internal.h index 548f59dd02..2fc3ebded8 100644 --- a/p2p/base/dtls_transport_internal.h +++ b/p2p/base/dtls_transport_internal.h @@ -70,6 +70,7 @@ class DtlsTransportInternal : public rtc::PacketTransportInternal { // Finds out which DTLS cipher was negotiated. // TODO(zhihuang): Remove this once all dependencies implement this. virtual bool GetSslCipherSuite(int* cipher) = 0; + virtual std::optional GetTlsCipherSuiteName() const = 0; // Find out which signature algorithm was used by the peer. Returns values // from diff --git a/p2p/base/fake_dtls_transport.h b/p2p/base/fake_dtls_transport.h index bbcbe43e3f..f0dba0a9f9 100644 --- a/p2p/base/fake_dtls_transport.h +++ b/p2p/base/fake_dtls_transport.h @@ -213,6 +213,10 @@ class FakeDtlsTransport : public DtlsTransportInternal { void SetSslCipherSuite(std::optional cipher_suite) { ssl_cipher_suite_ = cipher_suite; } + + std::optional GetTlsCipherSuiteName() const override { + return "FakeTlsCipherSuite"; + } uint16_t GetSslPeerSignatureAlgorithm() const override { return 0; } rtc::scoped_refptr GetLocalCertificate() const override { return local_cert_; diff --git a/pc/jsep_transport.cc b/pc/jsep_transport.cc index 95f7aed7bc..7645360c78 100644 --- a/pc/jsep_transport.cc +++ b/pc/jsep_transport.cc @@ -642,6 +642,7 @@ bool JsepTransport::GetTransportStats(DtlsTransportInternal* dtls_transport, dtls_transport->GetSslVersionBytes(&substats.ssl_version_bytes); dtls_transport->GetSrtpCryptoSuite(&substats.srtp_crypto_suite); dtls_transport->GetSslCipherSuite(&substats.ssl_cipher_suite); + substats.tls_cipher_suite_name = dtls_transport->GetTlsCipherSuiteName(); substats.dtls_state = dtls_transport->dtls_state(); rtc::SSLRole dtls_role; if (dtls_transport->GetDtlsRole(&dtls_role)) { diff --git a/pc/legacy_stats_collector.cc b/pc/legacy_stats_collector.cc index 5665fb70e3..6c5e52f527 100644 --- a/pc/legacy_stats_collector.cc +++ b/pc/legacy_stats_collector.cc @@ -1012,13 +1012,10 @@ void LegacyStatsCollector::ExtractSessionInfo_s(SessionStats& session_stats) { StatsReport::kStatsValueNameSrtpCipher, rtc::SrtpCryptoSuiteToName(srtp_crypto_suite)); } - int ssl_cipher_suite = channel_iter.ssl_cipher_suite; - if (ssl_cipher_suite != rtc::kTlsNullWithNullNull && - rtc::SSLStreamAdapter::SslCipherSuiteToName(ssl_cipher_suite) - .length()) { + if (channel_iter.tls_cipher_suite_name) { channel_report->AddString( StatsReport::kStatsValueNameDtlsCipher, - rtc::SSLStreamAdapter::SslCipherSuiteToName(ssl_cipher_suite)); + std::string(*channel_iter.tls_cipher_suite_name)); } // Collect stats for non-pooled candidates. Note that the reports diff --git a/pc/legacy_stats_collector_unittest.cc b/pc/legacy_stats_collector_unittest.cc index a792f7afa9..2264baab67 100644 --- a/pc/legacy_stats_collector_unittest.cc +++ b/pc/legacy_stats_collector_unittest.cc @@ -67,11 +67,6 @@ using ::testing::UnorderedElementsAre; namespace webrtc { -namespace internal { -// This value comes from openssl/tls1.h -static const int TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA = 0xC014; -} // namespace internal - // Error return values const char kNotFound[] = "NOT FOUND"; @@ -674,8 +669,7 @@ class LegacyStatsCollectorTest : public ::testing::Test { TransportChannelStats channel_stats; channel_stats.component = 1; channel_stats.srtp_crypto_suite = rtc::kSrtpAes128CmSha1_80; - channel_stats.ssl_cipher_suite = - internal::TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA; + channel_stats.tls_cipher_suite_name = "cipher_suite_for_test"; pc->SetTransportStats(kTransportName, channel_stats); // Fake certificate to report. @@ -722,9 +716,7 @@ class LegacyStatsCollectorTest : public ::testing::Test { std::string dtls_cipher_suite = ExtractStatsValue(StatsReport::kStatsReportTypeComponent, reports, StatsReport::kStatsValueNameDtlsCipher); - EXPECT_EQ(rtc::SSLStreamAdapter::SslCipherSuiteToName( - internal::TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA), - dtls_cipher_suite); + EXPECT_EQ(dtls_cipher_suite, "cipher_suite_for_test"); std::string srtp_crypto_suite = ExtractStatsValue(StatsReport::kStatsReportTypeComponent, reports, StatsReport::kStatsValueNameSrtpCipher); diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc index cd9beb2d6d..2375dadddc 100644 --- a/pc/rtc_stats_collector.cc +++ b/pc/rtc_stats_collector.cc @@ -1961,14 +1961,7 @@ void RTCStatsCollector::ProduceTransportStats_n( transport_stats->dtls_role = "unknown"; } - if (channel_stats.ssl_cipher_suite != rtc::kTlsNullWithNullNull && - rtc::SSLStreamAdapter::SslCipherSuiteToName( - channel_stats.ssl_cipher_suite) - .length()) { - transport_stats->dtls_cipher = - rtc::SSLStreamAdapter::SslCipherSuiteToName( - channel_stats.ssl_cipher_suite); - } + transport_stats->dtls_cipher = channel_stats.tls_cipher_suite_name; if (channel_stats.srtp_crypto_suite != rtc::kSrtpInvalidCryptoSuite && rtc::SrtpCryptoSuiteToName(channel_stats.srtp_crypto_suite) .length()) { diff --git a/pc/rtc_stats_collector_unittest.cc b/pc/rtc_stats_collector_unittest.cc index b0fec8140c..23c5e8f759 100644 --- a/pc/rtc_stats_collector_unittest.cc +++ b/pc/rtc_stats_collector_unittest.cc @@ -2942,8 +2942,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCTransportStatsWithCrypto) { "thelocalufrag"; rtp_transport_channel_stats.ice_transport_stats.ice_state = IceTransportState::kConnected; - // 0x2F is TLS_RSA_WITH_AES_128_CBC_SHA according to IANA - rtp_transport_channel_stats.ssl_cipher_suite = 0x2F; + rtp_transport_channel_stats.tls_cipher_suite_name = + "TLS_RSA_WITH_AES_128_CBC_SHA"; rtp_transport_channel_stats.srtp_crypto_suite = rtc::kSrtpAes128CmSha1_80; pc_->SetTransportStats(kTransportName, {rtp_transport_channel_stats}); diff --git a/pc/transport_stats.h b/pc/transport_stats.h index db98813345..ab40f2a28a 100644 --- a/pc/transport_stats.h +++ b/pc/transport_stats.h @@ -31,6 +31,7 @@ struct TransportChannelStats { int ssl_version_bytes = 0; int srtp_crypto_suite = rtc::kSrtpInvalidCryptoSuite; int ssl_cipher_suite = rtc::kTlsNullWithNullNull; + std::optional tls_cipher_suite_name; std::optional dtls_role; webrtc::DtlsTransportState dtls_state = webrtc::DtlsTransportState::kNew; IceTransportStats ice_transport_stats; diff --git a/rtc_base/openssl_stream_adapter.cc b/rtc_base/openssl_stream_adapter.cc index d9f873b586..df9632ca78 100644 --- a/rtc_base/openssl_stream_adapter.cc +++ b/rtc_base/openssl_stream_adapter.cc @@ -63,12 +63,6 @@ #error "webrtc requires at least OpenSSL version 1.1.0, to support DTLS-SRTP" #endif -// Defines for the TLS Cipher Suite Map. -#define DEFINE_CIPHER_ENTRY_SSL3(name) \ - { SSL3_CK_##name, "TLS_" #name } -#define DEFINE_CIPHER_ENTRY_TLS1(name) \ - { TLS1_CK_##name, "TLS_" #name } - namespace rtc { namespace { using ::webrtc::SafeTask; @@ -93,68 +87,6 @@ constexpr SrtpCipherMapEntry kSrtpCipherMap[] = { {"SRTP_AEAD_AES_128_GCM", kSrtpAeadAes128Gcm}, {"SRTP_AEAD_AES_256_GCM", kSrtpAeadAes256Gcm}}; -#ifndef OPENSSL_IS_BORINGSSL -// The "SSL_CIPHER_standard_name" function is only available in OpenSSL when -// compiled with tracing, so we need to define the mapping manually here. -constexpr SslCipherMapEntry kSslCipherMap[] = { - // TLS v1.0 ciphersuites from RFC2246. - DEFINE_CIPHER_ENTRY_SSL3(RSA_RC4_128_SHA), - {SSL3_CK_RSA_DES_192_CBC3_SHA, "TLS_RSA_WITH_3DES_EDE_CBC_SHA"}, - - // AES ciphersuites from RFC3268. - {TLS1_CK_RSA_WITH_AES_128_SHA, "TLS_RSA_WITH_AES_128_CBC_SHA"}, - {TLS1_CK_DHE_RSA_WITH_AES_128_SHA, "TLS_DHE_RSA_WITH_AES_128_CBC_SHA"}, - {TLS1_CK_RSA_WITH_AES_256_SHA, "TLS_RSA_WITH_AES_256_CBC_SHA"}, - {TLS1_CK_DHE_RSA_WITH_AES_256_SHA, "TLS_DHE_RSA_WITH_AES_256_CBC_SHA"}, - - // ECC ciphersuites from RFC4492. - DEFINE_CIPHER_ENTRY_TLS1(ECDHE_ECDSA_WITH_RC4_128_SHA), - {TLS1_CK_ECDHE_ECDSA_WITH_DES_192_CBC3_SHA, - "TLS_ECDHE_ECDSA_WITH_3DES_EDE_CBC_SHA"}, - DEFINE_CIPHER_ENTRY_TLS1(ECDHE_ECDSA_WITH_AES_128_CBC_SHA), - DEFINE_CIPHER_ENTRY_TLS1(ECDHE_ECDSA_WITH_AES_256_CBC_SHA), - - DEFINE_CIPHER_ENTRY_TLS1(ECDHE_RSA_WITH_RC4_128_SHA), - {TLS1_CK_ECDHE_RSA_WITH_DES_192_CBC3_SHA, - "TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA"}, - DEFINE_CIPHER_ENTRY_TLS1(ECDHE_RSA_WITH_AES_128_CBC_SHA), - DEFINE_CIPHER_ENTRY_TLS1(ECDHE_RSA_WITH_AES_256_CBC_SHA), - - // TLS v1.2 ciphersuites. - {TLS1_CK_RSA_WITH_AES_128_SHA256, "TLS_RSA_WITH_AES_128_CBC_SHA256"}, - {TLS1_CK_RSA_WITH_AES_256_SHA256, "TLS_RSA_WITH_AES_256_CBC_SHA256"}, - {TLS1_CK_DHE_RSA_WITH_AES_128_SHA256, - "TLS_DHE_RSA_WITH_AES_128_CBC_SHA256"}, - {TLS1_CK_DHE_RSA_WITH_AES_256_SHA256, - "TLS_DHE_RSA_WITH_AES_256_CBC_SHA256"}, - - // TLS v1.2 GCM ciphersuites from RFC5288. - DEFINE_CIPHER_ENTRY_TLS1(RSA_WITH_AES_128_GCM_SHA256), - DEFINE_CIPHER_ENTRY_TLS1(RSA_WITH_AES_256_GCM_SHA384), - DEFINE_CIPHER_ENTRY_TLS1(DHE_RSA_WITH_AES_128_GCM_SHA256), - DEFINE_CIPHER_ENTRY_TLS1(DHE_RSA_WITH_AES_256_GCM_SHA384), - DEFINE_CIPHER_ENTRY_TLS1(DH_RSA_WITH_AES_128_GCM_SHA256), - DEFINE_CIPHER_ENTRY_TLS1(DH_RSA_WITH_AES_256_GCM_SHA384), - - // ECDH HMAC based ciphersuites from RFC5289. - {TLS1_CK_ECDHE_ECDSA_WITH_AES_128_SHA256, - "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256"}, - {TLS1_CK_ECDHE_ECDSA_WITH_AES_256_SHA384, - "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384"}, - {TLS1_CK_ECDHE_RSA_WITH_AES_128_SHA256, - "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256"}, - {TLS1_CK_ECDHE_RSA_WITH_AES_256_SHA384, - "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384"}, - - // ECDH GCM based ciphersuites from RFC5289. - DEFINE_CIPHER_ENTRY_TLS1(ECDHE_ECDSA_WITH_AES_128_GCM_SHA256), - DEFINE_CIPHER_ENTRY_TLS1(ECDHE_ECDSA_WITH_AES_256_GCM_SHA384), - DEFINE_CIPHER_ENTRY_TLS1(ECDHE_RSA_WITH_AES_128_GCM_SHA256), - DEFINE_CIPHER_ENTRY_TLS1(ECDHE_RSA_WITH_AES_256_GCM_SHA384), - - {0, nullptr}}; -#endif // #ifndef OPENSSL_IS_BORINGSSL - #ifdef OPENSSL_IS_BORINGSSL // Enabled by EnableTimeCallbackForTesting. Should never be set in production // code. @@ -388,23 +320,14 @@ bool OpenSSLStreamAdapter::SetPeerCertificateDigest( return true; } -std::string OpenSSLStreamAdapter::SslCipherSuiteToName(int cipher_suite) { -#ifdef OPENSSL_IS_BORINGSSL - const SSL_CIPHER* ssl_cipher = SSL_get_cipher_by_value(cipher_suite); - if (!ssl_cipher) { - return std::string(); +std::optional OpenSSLStreamAdapter::GetTlsCipherSuiteName() + const { + if (state_ != SSL_CONNECTED) { + return std::nullopt; } - return SSL_CIPHER_standard_name(ssl_cipher); -#else - const int openssl_cipher_id = 0x03000000L | cipher_suite; - for (const SslCipherMapEntry* entry = kSslCipherMap; entry->rfc_name; - ++entry) { - if (openssl_cipher_id == static_cast(entry->openssl_id)) { - return entry->rfc_name; - } - } - return std::string(); -#endif + + const SSL_CIPHER* current_cipher = SSL_get_current_cipher(ssl_); + return SSL_CIPHER_standard_name(current_cipher); } bool OpenSSLStreamAdapter::GetSslCipherSuite(int* cipher_suite) { diff --git a/rtc_base/openssl_stream_adapter.h b/rtc_base/openssl_stream_adapter.h index e67992be45..f9eee0f5ac 100644 --- a/rtc_base/openssl_stream_adapter.h +++ b/rtc_base/openssl_stream_adapter.h @@ -103,8 +103,7 @@ class OpenSSLStreamAdapter final : public SSLStreamAdapter, void Close() override; StreamState GetState() const override; - // TODO(guoweis): Move this away from a static class method. - static std::string SslCipherSuiteToName(int crypto_suite); + std::optional GetTlsCipherSuiteName() const override; bool GetSslCipherSuite(int* cipher) override; [[deprecated("Use GetSslVersionBytes")]] SSLProtocolVersion GetSslVersion() diff --git a/rtc_base/ssl_stream_adapter.cc b/rtc_base/ssl_stream_adapter.cc index 5023363e48..7e09bf01ca 100644 --- a/rtc_base/ssl_stream_adapter.cc +++ b/rtc_base/ssl_stream_adapter.cc @@ -119,9 +119,6 @@ bool SSLStreamAdapter::IsAcceptableCipher(absl::string_view cipher, KeyType key_type) { return OpenSSLStreamAdapter::IsAcceptableCipher(cipher, key_type); } -std::string SSLStreamAdapter::SslCipherSuiteToName(int cipher_suite) { - return OpenSSLStreamAdapter::SslCipherSuiteToName(cipher_suite); -} /////////////////////////////////////////////////////////////////////////////// // Test only settings diff --git a/rtc_base/ssl_stream_adapter.h b/rtc_base/ssl_stream_adapter.h index 758d0e2013..e16db4ad48 100644 --- a/rtc_base/ssl_stream_adapter.h +++ b/rtc_base/ssl_stream_adapter.h @@ -185,6 +185,9 @@ class SSLStreamAdapter : public StreamInterface { // Retrieves the IANA registration id of the cipher suite used for the // connection (e.g. 0x2F for "TLS_RSA_WITH_AES_128_CBC_SHA"). virtual bool GetSslCipherSuite(int* cipher_suite); + // Returns the name of the cipher suite used for the DTLS transport, + // as defined in the "Description" column of the IANA cipher suite registry. + virtual std::optional GetTlsCipherSuiteName() const = 0; // Retrieves the enum value for SSL version. // Will return -1 until the version has been negotiated. @@ -236,11 +239,6 @@ class SSLStreamAdapter : public StreamInterface { static bool IsAcceptableCipher(int cipher, KeyType key_type); static bool IsAcceptableCipher(absl::string_view cipher, KeyType key_type); - // TODO(guoweis): Move this away from a static class method. Currently this is - // introduced such that any caller could depend on sslstreamadapter.h without - // depending on specific SSL implementation. - static std::string SslCipherSuiteToName(int cipher_suite); - //////////////////////////////////////////////////////////////////////////// // Testing only member functions ////////////////////////////////////////////////////////////////////////////