From 7b61b84ab1035cd79ef9fdaacbc310e567edf2fe Mon Sep 17 00:00:00 2001 From: Philipp Hancke Date: Thu, 18 Jul 2024 10:34:19 -0700 Subject: [PATCH] Cleanup SSLStreamAdapter unit tests BUG=None Change-Id: I71fa442f6f9b95bad63a3d7d797433d95bf5c298 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/354780 Reviewed-by: Danil Chapovalov Reviewed-by: Harald Alvestrand Commit-Queue: Philipp Hancke Cr-Commit-Position: refs/heads/main@{#42663} --- rtc_base/openssl_stream_adapter.cc | 10 +-- rtc_base/ssl_stream_adapter_unittest.cc | 85 +++++++++++-------------- 2 files changed, 38 insertions(+), 57 deletions(-) diff --git a/rtc_base/openssl_stream_adapter.cc b/rtc_base/openssl_stream_adapter.cc index 9b7858c238..46ce8b894e 100644 --- a/rtc_base/openssl_stream_adapter.cc +++ b/rtc_base/openssl_stream_adapter.cc @@ -568,17 +568,14 @@ StreamResult OpenSSLStreamAdapter::Write(rtc::ArrayView data, case SSL_NONE: // pass-through in clear text return stream_->Write(data, written, error); - case SSL_WAIT: case SSL_CONNECTING: return SR_BLOCK; - case SSL_CONNECTED: if (WaitingToVerifyPeerCertificate()) { return SR_BLOCK; } break; - case SSL_ERROR: case SSL_CLOSED: default: @@ -610,7 +607,6 @@ StreamResult OpenSSLStreamAdapter::Write(rtc::ArrayView data, case SSL_ERROR_WANT_WRITE: RTC_DLOG(LS_VERBOSE) << " -- error want write"; return SR_BLOCK; - case SSL_ERROR_ZERO_RETURN: default: Error("SSL_write", (ssl_error ? ssl_error : -1), 0, false); @@ -913,7 +909,6 @@ int OpenSSLStreamAdapter::ContinueSSL() { FireEvent(SE_OPEN | SE_READ | SE_WRITE, 0); } break; - case SSL_ERROR_WANT_READ: { RTC_DLOG(LS_VERBOSE) << " -- error want read"; struct timeval timeout; @@ -922,13 +917,11 @@ int OpenSSLStreamAdapter::ContinueSSL() { SetTimeout(delay); } } break; - case SSL_ERROR_WANT_WRITE: RTC_DLOG(LS_VERBOSE) << " -- error want write"; break; - case SSL_ERROR_ZERO_RETURN: - default: + default: { SSLHandshakeError ssl_handshake_err = SSLHandshakeError::UNKNOWN; int err_code = ERR_peek_last_error(); if (err_code != 0 && ERR_GET_REASON(err_code) == SSL_R_NO_SHARED_CIPHER) { @@ -940,6 +933,7 @@ int OpenSSLStreamAdapter::ContinueSSL() { handshake_error_(ssl_handshake_err); } return (ssl_error != 0) ? ssl_error : -1; + } } return 0; diff --git a/rtc_base/ssl_stream_adapter_unittest.cc b/rtc_base/ssl_stream_adapter_unittest.cc index 12e311da08..ed96305537 100644 --- a/rtc_base/ssl_stream_adapter_unittest.cc +++ b/rtc_base/ssl_stream_adapter_unittest.cc @@ -1371,12 +1371,11 @@ TEST_P(SSLStreamAdapterTestDTLS, TestDTLSDelayedIdentityWithBogusDigest) { TestHandshakeWithDelayedIdentity(false); } -// Test DTLS-SRTP with all high ciphers -TEST_P(SSLStreamAdapterTestDTLS, TestDTLSSrtpHigh) { - std::vector high; - high.push_back(rtc::kSrtpAes128CmSha1_80); - SetDtlsSrtpCryptoSuites(high, true); - SetDtlsSrtpCryptoSuites(high, false); +// Test DTLS-SRTP with SrtpAes128CmSha1_80 +TEST_P(SSLStreamAdapterTestDTLS, TestDTLSSrtpAes128CmSha1_80) { + const std::vector crypto_suites = {rtc::kSrtpAes128CmSha1_80}; + SetDtlsSrtpCryptoSuites(crypto_suites, true); + SetDtlsSrtpCryptoSuites(crypto_suites, false); TestHandshake(); int client_cipher; @@ -1388,12 +1387,11 @@ TEST_P(SSLStreamAdapterTestDTLS, TestDTLSSrtpHigh) { ASSERT_EQ(client_cipher, rtc::kSrtpAes128CmSha1_80); } -// Test DTLS-SRTP with all low ciphers -TEST_P(SSLStreamAdapterTestDTLS, TestDTLSSrtpLow) { - std::vector low; - low.push_back(rtc::kSrtpAes128CmSha1_32); - SetDtlsSrtpCryptoSuites(low, true); - SetDtlsSrtpCryptoSuites(low, false); +// Test DTLS-SRTP with SrtpAes128CmSha1_32 +TEST_P(SSLStreamAdapterTestDTLS, TestDTLSSrtpAes128CmSha1_32) { + const std::vector crypto_suites = {rtc::kSrtpAes128CmSha1_32}; + SetDtlsSrtpCryptoSuites(crypto_suites, true); + SetDtlsSrtpCryptoSuites(crypto_suites, false); TestHandshake(); int client_cipher; @@ -1405,14 +1403,10 @@ TEST_P(SSLStreamAdapterTestDTLS, TestDTLSSrtpLow) { ASSERT_EQ(client_cipher, rtc::kSrtpAes128CmSha1_32); } -// Test DTLS-SRTP with a mismatch -- should not converge -TEST_P(SSLStreamAdapterTestDTLS, TestDTLSSrtpHighLow) { - std::vector high; - high.push_back(rtc::kSrtpAes128CmSha1_80); - std::vector low; - low.push_back(rtc::kSrtpAes128CmSha1_32); - SetDtlsSrtpCryptoSuites(high, true); - SetDtlsSrtpCryptoSuites(low, false); +// Test DTLS-SRTP with incompatible cipher suites -- should not converge. +TEST_P(SSLStreamAdapterTestDTLS, TestDTLSSrtpIncompatibleCipherSuites) { + SetDtlsSrtpCryptoSuites({rtc::kSrtpAes128CmSha1_80}, true); + SetDtlsSrtpCryptoSuites({rtc::kSrtpAes128CmSha1_32}, false); TestHandshake(); int client_cipher; @@ -1421,13 +1415,13 @@ TEST_P(SSLStreamAdapterTestDTLS, TestDTLSSrtpHighLow) { ASSERT_FALSE(GetDtlsSrtpCryptoSuite(false, &server_cipher)); } -// Test DTLS-SRTP with each side being mixed -- should select high +// Test DTLS-SRTP with each side being mixed -- should select the stronger +// cipher. TEST_P(SSLStreamAdapterTestDTLS, TestDTLSSrtpMixed) { - std::vector mixed; - mixed.push_back(rtc::kSrtpAes128CmSha1_80); - mixed.push_back(rtc::kSrtpAes128CmSha1_32); - SetDtlsSrtpCryptoSuites(mixed, true); - SetDtlsSrtpCryptoSuites(mixed, false); + const std::vector crypto_suites = {rtc::kSrtpAes128CmSha1_80, + rtc::kSrtpAes128CmSha1_32}; + SetDtlsSrtpCryptoSuites(crypto_suites, true); + SetDtlsSrtpCryptoSuites(crypto_suites, false); TestHandshake(); int client_cipher; @@ -1439,12 +1433,11 @@ TEST_P(SSLStreamAdapterTestDTLS, TestDTLSSrtpMixed) { ASSERT_EQ(client_cipher, rtc::kSrtpAes128CmSha1_80); } -// Test DTLS-SRTP with all GCM-128 ciphers. -TEST_P(SSLStreamAdapterTestDTLS, TestDTLSSrtpGCM128) { - std::vector gcm128; - gcm128.push_back(rtc::kSrtpAeadAes128Gcm); - SetDtlsSrtpCryptoSuites(gcm128, true); - SetDtlsSrtpCryptoSuites(gcm128, false); +// Test DTLS-SRTP with SrtpAeadAes128Gcm. +TEST_P(SSLStreamAdapterTestDTLS, TestDTLSSrtpAeadAes128Gcm) { + std::vector crypto_suites = {rtc::kSrtpAeadAes128Gcm}; + SetDtlsSrtpCryptoSuites(crypto_suites, true); + SetDtlsSrtpCryptoSuites(crypto_suites, false); TestHandshake(); int client_cipher; @@ -1458,10 +1451,9 @@ TEST_P(SSLStreamAdapterTestDTLS, TestDTLSSrtpGCM128) { // Test DTLS-SRTP with all GCM-256 ciphers. TEST_P(SSLStreamAdapterTestDTLS, TestDTLSSrtpGCM256) { - std::vector gcm256; - gcm256.push_back(rtc::kSrtpAeadAes256Gcm); - SetDtlsSrtpCryptoSuites(gcm256, true); - SetDtlsSrtpCryptoSuites(gcm256, false); + std::vector crypto_suites = {rtc::kSrtpAeadAes256Gcm}; + SetDtlsSrtpCryptoSuites(crypto_suites, true); + SetDtlsSrtpCryptoSuites(crypto_suites, false); TestHandshake(); int client_cipher; @@ -1473,14 +1465,10 @@ TEST_P(SSLStreamAdapterTestDTLS, TestDTLSSrtpGCM256) { ASSERT_EQ(client_cipher, rtc::kSrtpAeadAes256Gcm); } -// Test DTLS-SRTP with mixed GCM-128/-256 ciphers -- should not converge. -TEST_P(SSLStreamAdapterTestDTLS, TestDTLSSrtpGCMMismatch) { - std::vector gcm128; - gcm128.push_back(rtc::kSrtpAeadAes128Gcm); - std::vector gcm256; - gcm256.push_back(rtc::kSrtpAeadAes256Gcm); - SetDtlsSrtpCryptoSuites(gcm128, true); - SetDtlsSrtpCryptoSuites(gcm256, false); +// Test DTLS-SRTP with incompatbile GCM-128/-256 ciphers -- should not converge. +TEST_P(SSLStreamAdapterTestDTLS, TestDTLSSrtpIncompatibleGcmCipherSuites) { + SetDtlsSrtpCryptoSuites({rtc::kSrtpAeadAes128Gcm}, true); + SetDtlsSrtpCryptoSuites({rtc::kSrtpAeadAes256Gcm}, false); TestHandshake(); int client_cipher; @@ -1491,11 +1479,10 @@ TEST_P(SSLStreamAdapterTestDTLS, TestDTLSSrtpGCMMismatch) { // Test DTLS-SRTP with both GCM-128/-256 ciphers -- should select GCM-256. TEST_P(SSLStreamAdapterTestDTLS, TestDTLSSrtpGCMMixed) { - std::vector gcmBoth; - gcmBoth.push_back(rtc::kSrtpAeadAes256Gcm); - gcmBoth.push_back(rtc::kSrtpAeadAes128Gcm); - SetDtlsSrtpCryptoSuites(gcmBoth, true); - SetDtlsSrtpCryptoSuites(gcmBoth, false); + std::vector crypto_suites = {rtc::kSrtpAeadAes256Gcm, + rtc::kSrtpAeadAes128Gcm}; + SetDtlsSrtpCryptoSuites(crypto_suites, true); + SetDtlsSrtpCryptoSuites(crypto_suites, false); TestHandshake(); int client_cipher;