spanify SSLStreamAdapter::SetPeerCertificateDigest

BUG=webrtc:357776213

Change-Id: Ie6189ac21b9f76f7ce5ddb3e4208c08793df73ff
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/368220
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@meta.com>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43462}

p2p/BUILD.gn

@@ -398,15 +398,20 @@ rtc_library("dtls_transport") {
     ":packet_transport_internal",
     "../api:array_view",
     "../api:dtls_transport_interface",
+    "../api:rtc_error",
+    "../api:scoped_refptr",
     "../api:sequence_checker",
     "../api/crypto:options",
     "../api/rtc_event_log",
+    "../api/units:timestamp",
     "../logging:ice_log",
     "../rtc_base:buffer",
     "../rtc_base:buffer_queue",
     "../rtc_base:checks",
     "../rtc_base:dscp",
     "../rtc_base:logging",
+    "../rtc_base:network_route",
+    "../rtc_base:socket",
     "../rtc_base:socket_address",
     "../rtc_base:ssl",
     "../rtc_base:ssl_adapter",
@@ -414,6 +419,7 @@ rtc_library("dtls_transport") {
     "../rtc_base:stringutils",
     "../rtc_base:threading",
     "../rtc_base:timeutils",
+    "../rtc_base/network:ecn_marking",
     "../rtc_base/network:received_packet",
     "../rtc_base/system:no_unique_address",
     "//third_party/abseil-cpp/absl/memory",

p2p/base/dtls_transport.cc

@@ -11,28 +11,38 @@
 #include "p2p/base/dtls_transport.h"
 
 #include <algorithm>
+#include <cstddef>
 #include <cstdint>
 #include <memory>
+#include <optional>
+#include <string>
 #include <utility>
 
-#include "absl/memory/memory.h"
 #include "absl/strings/string_view.h"
 #include "api/array_view.h"
+#include "api/crypto/crypto_options.h"
 #include "api/dtls_transport_interface.h"
+#include "api/rtc_error.h"
 #include "api/rtc_event_log/rtc_event_log.h"
+#include "api/scoped_refptr.h"
+#include "api/sequence_checker.h"
+#include "api/units/timestamp.h"
 #include "logging/rtc_event_log/events/rtc_event_dtls_transport_state.h"
 #include "logging/rtc_event_log/events/rtc_event_dtls_writable_state.h"
+#include "p2p/base/dtls_transport_internal.h"
+#include "p2p/base/ice_transport_internal.h"
 #include "p2p/base/packet_transport_internal.h"
 #include "rtc_base/buffer.h"
 #include "rtc_base/checks.h"
-#include "rtc_base/dscp.h"
 #include "rtc_base/logging.h"
+#include "rtc_base/network/ecn_marking.h"
 #include "rtc_base/network/received_packet.h"
+#include "rtc_base/network_route.h"
 #include "rtc_base/rtc_certificate.h"
+#include "rtc_base/socket.h"
 #include "rtc_base/socket_address.h"
 #include "rtc_base/ssl_stream_adapter.h"
 #include "rtc_base/stream.h"
-#include "rtc_base/thread.h"
 #include "rtc_base/time_utils.h"
 
 namespace cricket {
@@ -312,11 +322,9 @@ bool DtlsTransport::SetRemoteFingerprint(absl::string_view digest_alg,
     // This can occur if DTLS is set up before a remote fingerprint is
     // received. For instance, if we set up DTLS due to receiving an early
     // ClientHello.
-    rtc::SSLPeerCertificateDigestError err;
+    rtc::SSLPeerCertificateDigestError err = dtls_->SetPeerCertificateDigest(
-    if (!dtls_->SetPeerCertificateDigest(
+        remote_fingerprint_algorithm_, remote_fingerprint_value_);
-            remote_fingerprint_algorithm_,
+    if (err != rtc::SSLPeerCertificateDigestError::NONE) {
-            reinterpret_cast<unsigned char*>(remote_fingerprint_value_.data()),
-            remote_fingerprint_value_.size(), &err)) {
       RTC_LOG(LS_ERROR) << ToString()
                         << ": Couldn't set DTLS certificate digest.";
       set_dtls_state(webrtc::DtlsTransportState::kFailed);
@@ -381,10 +389,9 @@ bool DtlsTransport::SetupDtls() {
   dtls_->SetEventCallback(
       [this](int events, int err) { OnDtlsEvent(events, err); });
   if (remote_fingerprint_value_.size() &&
-      !dtls_->SetPeerCertificateDigest(
+      dtls_->SetPeerCertificateDigest(remote_fingerprint_algorithm_,
-          remote_fingerprint_algorithm_,
+                                      remote_fingerprint_value_) !=
-          reinterpret_cast<unsigned char*>(remote_fingerprint_value_.data()),
+          rtc::SSLPeerCertificateDigestError::NONE) {
-          remote_fingerprint_value_.size())) {
     RTC_LOG(LS_ERROR) << ToString()
                       << ": Couldn't set DTLS certificate digest.";
     return false;

rtc_base/openssl_stream_adapter.cc

@@ -20,6 +20,7 @@
 #include <cstdint>
 #include <cstring>
 #include <memory>
+#include <optional>
 #include <string>
 #include <utility>
 #include <vector>
@@ -269,47 +270,33 @@ void OpenSSLStreamAdapter::SetServerRole(SSLRole role) {
   role_ = role;
 }
 
-bool OpenSSLStreamAdapter::SetPeerCertificateDigest(
+SSLPeerCertificateDigestError OpenSSLStreamAdapter::SetPeerCertificateDigest(
     absl::string_view digest_alg,
-    const unsigned char* digest_val,
+    rtc::ArrayView<uint8_t> digest_val) {
-    size_t digest_len,
-    SSLPeerCertificateDigestError* error) {
   RTC_DCHECK(!peer_certificate_verified_);
   RTC_DCHECK(!HasPeerCertificateDigest());
   size_t expected_len;
-  if (error) {
-    *error = SSLPeerCertificateDigestError::NONE;
-  }
 
   if (!OpenSSLDigest::GetDigestSize(digest_alg, &expected_len)) {
     RTC_LOG(LS_WARNING) << "Unknown digest algorithm: " << digest_alg;
-    if (error) {
+    return SSLPeerCertificateDigestError::UNKNOWN_ALGORITHM;
-      *error = SSLPeerCertificateDigestError::UNKNOWN_ALGORITHM;
-    }
-    return false;
   }
-  if (expected_len != digest_len) {
+  if (expected_len != digest_val.size()) {
-    if (error) {
+    return SSLPeerCertificateDigestError::INVALID_LENGTH;
-      *error = SSLPeerCertificateDigestError::INVALID_LENGTH;
-    }
-    return false;
   }
 
-  peer_certificate_digest_value_.SetData(digest_val, digest_len);
+  peer_certificate_digest_value_.SetData(digest_val);
   peer_certificate_digest_algorithm_ = std::string(digest_alg);
 
   if (!peer_cert_chain_) {
     // Normal case, where the digest is set before we obtain the certificate
     // from the handshake.
-    return true;
+    return SSLPeerCertificateDigestError::NONE;
   }
 
   if (!VerifyPeerCertificate()) {
     Error("SetPeerCertificateDigest", -1, SSL_AD_BAD_CERTIFICATE, false);
-    if (error) {
+    return SSLPeerCertificateDigestError::VERIFICATION_FAILED;
-      *error = SSLPeerCertificateDigestError::VERIFICATION_FAILED;
-    }
-    return false;
   }
 
   if (state_ == SSL_CONNECTED) {
@@ -318,8 +305,7 @@ bool OpenSSLStreamAdapter::SetPeerCertificateDigest(
     // events and may not be prepared for reentrancy.
     PostEvent(SE_OPEN | SE_READ | SE_WRITE, 0);
   }
-
+  return SSLPeerCertificateDigestError::NONE;
-  return true;
 }
 
 std::optional<absl::string_view> OpenSSLStreamAdapter::GetTlsCipherSuiteName()

rtc_base/openssl_stream_adapter.h

@@ -78,11 +78,9 @@ class OpenSSLStreamAdapter final : public SSLStreamAdapter {
 
   // Default argument is for compatibility
   void SetServerRole(SSLRole role = SSL_SERVER) override;
-  bool SetPeerCertificateDigest(
+  SSLPeerCertificateDigestError SetPeerCertificateDigest(
       absl::string_view digest_alg,
-      const unsigned char* digest_val,
+      rtc::ArrayView<uint8_t> digest_val) override;
-      size_t digest_len,
-      SSLPeerCertificateDigestError* error = nullptr) override;
 
   std::unique_ptr<SSLCertChain> GetPeerSSLCertChain() const override;
 

rtc_base/ssl_stream_adapter.cc

@@ -15,10 +15,10 @@
 #include <memory>
 #include <string>
 #include <utility>
-#include <vector>
 
 #include "absl/functional/any_invocable.h"
 #include "absl/strings/string_view.h"
+#include "api/array_view.h"
 #include "rtc_base/openssl_stream_adapter.h"
 #include "rtc_base/ssl_identity.h"
 #include "rtc_base/stream.h"
@@ -98,6 +98,20 @@ bool SSLStreamAdapter::IsAcceptableCipher(absl::string_view cipher,
   return OpenSSLStreamAdapter::IsAcceptableCipher(cipher, key_type);
 }
 
+// Default shim for backward compat.
+bool SSLStreamAdapter::SetPeerCertificateDigest(
+    absl::string_view digest_alg,
+    const unsigned char* digest_val,
+    size_t digest_len,
+    SSLPeerCertificateDigestError* error) {
+  unsigned char* nonconst_val = const_cast<unsigned char*>(digest_val);
+  SSLPeerCertificateDigestError ret = SetPeerCertificateDigest(
+      digest_alg, rtc::ArrayView<uint8_t>(nonconst_val, digest_len));
+  if (error)
+    *error = ret;
+  return ret == SSLPeerCertificateDigestError::NONE;
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // Test only settings
 ///////////////////////////////////////////////////////////////////////////////

rtc_base/ssl_stream_adapter.h

@@ -15,12 +15,14 @@
 #include <stdint.h>
 
 #include <memory>
+#include <optional>
 #include <string>
 #include <vector>
 
 #include "absl/functional/any_invocable.h"
-#include "absl/memory/memory.h"
 #include "absl/strings/string_view.h"
+#include "api/array_view.h"
+#include "rtc_base/buffer.h"
 #include "rtc_base/ssl_certificate.h"
 #include "rtc_base/ssl_identity.h"
 #include "rtc_base/stream.h"
@@ -170,13 +172,16 @@ class SSLStreamAdapter : public StreamInterface {
   // channel (such as the signaling channel). This must specify the terminal
   // certificate, not just a CA. SSLStream makes a copy of the digest value.
   //
-  // Returns true if successful.
+  // Returns SSLPeerCertificateDigestError::NONE if successful.
-  // `error` is optional and provides more information about the failure.
+  virtual SSLPeerCertificateDigestError SetPeerCertificateDigest(
-  virtual bool SetPeerCertificateDigest(
       absl::string_view digest_alg,
-      const unsigned char* digest_val,
+      rtc::ArrayView<uint8_t> digest_val) = 0;
-      size_t digest_len,
+  [[deprecated(
-      SSLPeerCertificateDigestError* error = nullptr) = 0;
+      "Use SetPeerCertificateDigest with ArrayView instead")]] virtual bool
+  SetPeerCertificateDigest(absl::string_view digest_alg,
+                           const unsigned char* digest_val,
+                           size_t digest_len,
+                           SSLPeerCertificateDigestError* error = nullptr);
 
   // Retrieves the peer's certificate chain including leaf certificate, if a
   // connection has been established.

rtc_base/ssl_stream_adapter_unittest.cc

@@ -10,7 +10,11 @@
 
 #include "rtc_base/ssl_stream_adapter.h"
 
+#ifdef OPENSSL_IS_BORINGSSL
+#include <openssl/digest.h>
+#else
 #include <openssl/evp.h>
+#endif
 #include <openssl/sha.h>
 
 #include <cstddef>
@@ -20,6 +24,7 @@
 #include <memory>
 #include <set>
 #include <string>
+#include <tuple>
 #include <utility>
 #include <vector>
 
@@ -29,6 +34,7 @@
 #include "api/sequence_checker.h"
 #include "api/task_queue/pending_task_safety_flag.h"
 #include "api/units/time_delta.h"
+#include "rtc_base/buffer.h"
 #include "rtc_base/buffer_queue.h"
 #include "rtc_base/callback_list.h"
 #include "rtc_base/checks.h"
@@ -36,10 +42,7 @@
 #include "rtc_base/fake_clock.h"
 #include "rtc_base/gunit.h"
 #include "rtc_base/logging.h"
-#include "rtc_base/memory/fifo_buffer.h"
-#include "rtc_base/memory_stream.h"
 #include "rtc_base/message_digest.h"
-#include "rtc_base/openssl_stream_adapter.h"
 #include "rtc_base/ssl_identity.h"
 #include "rtc_base/stream.h"
 #include "rtc_base/third_party/sigslot/sigslot.h"
@@ -508,9 +511,9 @@ class SSLStreamAdapterTestBase : public ::testing::Test,
   }
 
   void SetPeerIdentitiesByDigest(bool correct, bool expect_success) {
-    unsigned char server_digest[EVP_MAX_MD_SIZE];
+    rtc::Buffer server_digest(0, EVP_MAX_MD_SIZE);
     size_t server_digest_len;
-    unsigned char client_digest[EVP_MAX_MD_SIZE];
+    rtc::Buffer client_digest(0, EVP_MAX_MD_SIZE);
     size_t client_digest_len;
     bool rv;
     rtc::SSLPeerCertificateDigestError err;
@@ -524,29 +527,31 @@ class SSLStreamAdapterTestBase : public ::testing::Test,
     RTC_DCHECK(client_identity());
 
     rv = server_identity()->certificate().ComputeDigest(
-        digest_algorithm_, server_digest, digest_length_, &server_digest_len);
+        digest_algorithm_, server_digest.data(), digest_length_,
+        &server_digest_len);
     ASSERT_TRUE(rv);
+    server_digest.SetSize(server_digest_len);
     rv = client_identity()->certificate().ComputeDigest(
-        digest_algorithm_, client_digest, digest_length_, &client_digest_len);
+        digest_algorithm_, client_digest.data(), digest_length_,
+        &client_digest_len);
     ASSERT_TRUE(rv);
+    client_digest.SetSize(client_digest_len);
 
     if (!correct) {
       RTC_LOG(LS_INFO) << "Setting bogus digest for server cert";
       server_digest[0]++;
     }
-    rv = client_ssl_->SetPeerCertificateDigest(digest_algorithm_, server_digest,
+    err =
-                                               server_digest_len, &err);
+        client_ssl_->SetPeerCertificateDigest(digest_algorithm_, server_digest);
     EXPECT_EQ(expected_err, err);
-    EXPECT_EQ(expect_success, rv);
 
     if (!correct) {
       RTC_LOG(LS_INFO) << "Setting bogus digest for client cert";
       client_digest[0]++;
     }
-    rv = server_ssl_->SetPeerCertificateDigest(digest_algorithm_, client_digest,
+    err =
-                                               client_digest_len, &err);
+        server_ssl_->SetPeerCertificateDigest(digest_algorithm_, client_digest);
     EXPECT_EQ(expected_err, err);
-    EXPECT_EQ(expect_success, rv);
 
     identities_set_ = true;
   }
@@ -668,21 +673,25 @@ class SSLStreamAdapterTestBase : public ::testing::Test,
 
     // Collect both of the certificate digests; needs to be done before calling
     // SetPeerCertificateDigest as that may reset the identity.
-    unsigned char server_digest[EVP_MAX_MD_SIZE];
+    rtc::Buffer server_digest(0, EVP_MAX_MD_SIZE);
     size_t server_digest_len;
-    unsigned char client_digest[EVP_MAX_MD_SIZE];
+    rtc::Buffer client_digest(0, EVP_MAX_MD_SIZE);
     size_t client_digest_len;
     bool rv;
 
     ASSERT_THAT(server_identity(), NotNull());
     rv = server_identity()->certificate().ComputeDigest(
-        digest_algorithm_, server_digest, digest_length_, &server_digest_len);
+        digest_algorithm_, server_digest.data(), digest_length_,
+        &server_digest_len);
     ASSERT_TRUE(rv);
+    server_digest.SetSize(server_digest_len);
 
     ASSERT_THAT(client_identity(), NotNull());
     rv = client_identity()->certificate().ComputeDigest(
-        digest_algorithm_, client_digest, digest_length_, &client_digest_len);
+        digest_algorithm_, client_digest.data(), digest_length_,
+        &client_digest_len);
     ASSERT_TRUE(rv);
+    client_digest.SetSize(client_digest_len);
 
     if (!valid_identity) {
       RTC_LOG(LS_INFO) << "Setting bogus digest for client/server certs";
@@ -696,10 +705,9 @@ class SSLStreamAdapterTestBase : public ::testing::Test,
         valid_identity
             ? rtc::SSLPeerCertificateDigestError::NONE
             : rtc::SSLPeerCertificateDigestError::VERIFICATION_FAILED;
-    rv = client_ssl_->SetPeerCertificateDigest(digest_algorithm_, server_digest,
+    err =
-                                               server_digest_len, &err);
+        client_ssl_->SetPeerCertificateDigest(digest_algorithm_, server_digest);
     EXPECT_EQ(expected_err, err);
-    EXPECT_EQ(valid_identity, rv);
     // State should then transition to SS_OPEN or SS_CLOSED based on validation
     // of the identity.
     if (valid_identity) {
@@ -715,10 +723,9 @@ class SSLStreamAdapterTestBase : public ::testing::Test,
     }
 
     // Set the peer certificate digest for the server.
-    rv = server_ssl_->SetPeerCertificateDigest(digest_algorithm_, client_digest,
+    err =
-                                               client_digest_len, &err);
+        server_ssl_->SetPeerCertificateDigest(digest_algorithm_, client_digest);
     EXPECT_EQ(expected_err, err);
-    EXPECT_EQ(valid_identity, rv);
     if (valid_identity) {
       EXPECT_EQ(rtc::SS_OPEN, server_ssl_->GetState());
     } else {
@@ -1466,6 +1473,19 @@ TEST_F(SSLStreamAdapterTestDTLSFromPEMStrings, TestDTLSGetPeerCertificate) {
   ASSERT_EQ(kCERT_PEM, server_peer_cert->ToPEMString());
 }
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+TEST_F(SSLStreamAdapterTestDTLSFromPEMStrings,
+       DeprecatedSetPeerCertificateDigest) {
+  rtc::SSLPeerCertificateDigestError error;
+  // Pass in a wrong length to trigger an error.
+  bool ret = client_ssl_->SetPeerCertificateDigest(rtc::DIGEST_SHA_256, {},
+                                                   /*length=*/0, &error);
+  EXPECT_FALSE(ret);
+  EXPECT_EQ(error, rtc::SSLPeerCertificateDigestError::INVALID_LENGTH);
+}
+#pragma clang diagnostic pop
+
 // Test getting the DTLS 1.2 version.
 TEST_F(SSLStreamAdapterTestDTLS, TestGetSslVersionBytes) {
   // https://datatracker.ietf.org/doc/html/rfc9147#section-5.3