Simplify DtlsTransport state.

Make a few more members const, remove members that aren't used,
set max ssl version number on construction and remove setter.

Bug: none
Change-Id: I6c1a7cabf1e795e027f1bc53b994517e9aef0e93
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/213780
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33622}

p2p/base/dtls_transport.cc

@@ -134,14 +134,13 @@ void StreamInterfaceChannel::Close() {
 
 DtlsTransport::DtlsTransport(IceTransportInternal* ice_transport,
                              const webrtc::CryptoOptions& crypto_options,
-                             webrtc::RtcEventLog* event_log)
+                             webrtc::RtcEventLog* event_log,
-    : transport_name_(ice_transport->transport_name()),
+                             rtc::SSLProtocolVersion max_version)
-      component_(ice_transport->component()),
+    : component_(ice_transport->component()),
       ice_transport_(ice_transport),
       downward_(NULL),
       srtp_ciphers_(crypto_options.GetSupportedDtlsSrtpCryptoSuites()),
-      ssl_max_version_(rtc::SSL_PROTOCOL_DTLS_12),
+      ssl_max_version_(max_version),
-      crypto_options_(crypto_options),
       event_log_(event_log) {
   RTC_DCHECK(ice_transport_);
   ConnectToIceTransport();
@@ -149,16 +148,12 @@ DtlsTransport::DtlsTransport(IceTransportInternal* ice_transport,
 
 DtlsTransport::~DtlsTransport() = default;
 
-const webrtc::CryptoOptions& DtlsTransport::crypto_options() const {
-  return crypto_options_;
-}
-
 DtlsTransportState DtlsTransport::dtls_state() const {
   return dtls_state_;
 }
 
 const std::string& DtlsTransport::transport_name() const {
-  return transport_name_;
+  return ice_transport_->transport_name();
 }
 
 int DtlsTransport::component() const {
@@ -199,17 +194,6 @@ rtc::scoped_refptr<rtc::RTCCertificate> DtlsTransport::GetLocalCertificate()
   return local_certificate_;
 }
 
-bool DtlsTransport::SetSslMaxProtocolVersion(rtc::SSLProtocolVersion version) {
-  if (dtls_active_) {
-    RTC_LOG(LS_ERROR) << "Not changing max. protocol version "
-                         "while DTLS is negotiating";
-    return false;
-  }
-
-  ssl_max_version_ = version;
-  return true;
-}
-
 bool DtlsTransport::SetDtlsRole(rtc::SSLRole role) {
   if (dtls_) {
     RTC_DCHECK(dtls_role_);

p2p/base/dtls_transport.h

@@ -101,13 +101,14 @@ class DtlsTransport : public DtlsTransportInternal {
   //
   // |event_log| is an optional RtcEventLog for logging state changes. It should
   // outlive the DtlsTransport.
-  explicit DtlsTransport(IceTransportInternal* ice_transport,
+  DtlsTransport(
+      IceTransportInternal* ice_transport,
       const webrtc::CryptoOptions& crypto_options,
-                         webrtc::RtcEventLog* event_log);
+      webrtc::RtcEventLog* event_log,
+      rtc::SSLProtocolVersion max_version = rtc::SSL_PROTOCOL_DTLS_12);
 
   ~DtlsTransport() override;
 
-  const webrtc::CryptoOptions& crypto_options() const override;
   DtlsTransportState dtls_state() const override;
   const std::string& transport_name() const override;
   int component() const override;
@@ -142,8 +143,6 @@ class DtlsTransport : public DtlsTransportInternal {
 
   bool GetOption(rtc::Socket::Option opt, int* value) override;
 
-  bool SetSslMaxProtocolVersion(rtc::SSLProtocolVersion version) override;
-
   // Find out which TLS version was negotiated
   bool GetSslVersionBytes(int* version) const override;
   // Find out which DTLS-SRTP cipher was negotiated
@@ -191,7 +190,7 @@ class DtlsTransport : public DtlsTransportInternal {
     const absl::string_view RECEIVING_ABBREV[2] = {"_", "R"};
     const absl::string_view WRITABLE_ABBREV[2] = {"_", "W"};
     rtc::StringBuilder sb;
-    sb << "DtlsTransport[" << transport_name_ << "|" << component_ << "|"
+    sb << "DtlsTransport[" << transport_name() << "|" << component_ << "|"
        << RECEIVING_ABBREV[receiving()] << WRITABLE_ABBREV[writable()] << "]";
     return sb.Release();
   }
@@ -224,20 +223,18 @@ class DtlsTransport : public DtlsTransportInternal {
 
   webrtc::SequenceChecker thread_checker_;
 
-  std::string transport_name_;
+  const int component_;
-  int component_;
   DtlsTransportState dtls_state_ = DTLS_TRANSPORT_NEW;
   // Underlying ice_transport, not owned by this class.
-  IceTransportInternal* ice_transport_;
+  IceTransportInternal* const ice_transport_;
   std::unique_ptr<rtc::SSLStreamAdapter> dtls_;  // The DTLS stream
   StreamInterfaceChannel*
       downward_;  // Wrapper for ice_transport_, owned by dtls_.
-  std::vector<int> srtp_ciphers_;  // SRTP ciphers to use with DTLS.
+  const std::vector<int> srtp_ciphers_;  // SRTP ciphers to use with DTLS.
   bool dtls_active_ = false;
   rtc::scoped_refptr<rtc::RTCCertificate> local_certificate_;
   absl::optional<rtc::SSLRole> dtls_role_;
-  rtc::SSLProtocolVersion ssl_max_version_;
+  const rtc::SSLProtocolVersion ssl_max_version_;
-  webrtc::CryptoOptions crypto_options_;
   rtc::Buffer remote_fingerprint_value_;
   std::string remote_fingerprint_algorithm_;
 

p2p/base/dtls_transport_factory.h

@@ -31,7 +31,8 @@ class DtlsTransportFactory {
 
   virtual std::unique_ptr<DtlsTransportInternal> CreateDtlsTransport(
       IceTransportInternal* ice,
-      const webrtc::CryptoOptions& crypto_options) = 0;
+      const webrtc::CryptoOptions& crypto_options,
+      rtc::SSLProtocolVersion max_version) = 0;
 };
 
 }  // namespace cricket

p2p/base/dtls_transport_internal.h

@@ -18,6 +18,7 @@
 #include <string>
 #include <utility>
 
+#include "absl/base/attributes.h"
 #include "api/crypto/crypto_options.h"
 #include "api/dtls_transport_interface.h"
 #include "api/scoped_refptr.h"
@@ -28,7 +29,6 @@
 #include "rtc_base/ssl_certificate.h"
 #include "rtc_base/ssl_fingerprint.h"
 #include "rtc_base/ssl_stream_adapter.h"
-#include "rtc_base/third_party/sigslot/sigslot.h"
 
 namespace cricket {
 
@@ -64,8 +64,6 @@ class DtlsTransportInternal : public rtc::PacketTransportInternal {
  public:
   ~DtlsTransportInternal() override;
 
-  virtual const webrtc::CryptoOptions& crypto_options() const = 0;
-
   virtual DtlsTransportState dtls_state() const = 0;
 
   virtual int component() const = 0;
@@ -109,7 +107,10 @@ class DtlsTransportInternal : public rtc::PacketTransportInternal {
                                     const uint8_t* digest,
                                     size_t digest_len) = 0;
 
-  virtual bool SetSslMaxProtocolVersion(rtc::SSLProtocolVersion version) = 0;
+  ABSL_DEPRECATED("Set the max version via construction.")
+  bool SetSslMaxProtocolVersion(rtc::SSLProtocolVersion version) {
+    return true;
+  }
 
   // Expose the underneath IceTransport.
   virtual IceTransportInternal* ice_transport() = 0;

p2p/base/dtls_transport_unittest.cc

@@ -86,10 +86,9 @@ class DtlsTestClient : public sigslot::has_slots<> {
     fake_ice_transport_->SignalReadPacket.connect(
         this, &DtlsTestClient::OnFakeIceTransportReadPacket);
 
-    dtls_transport_ = std::make_unique<DtlsTransport>(fake_ice_transport_.get(),
+    dtls_transport_ = std::make_unique<DtlsTransport>(
-                                                      webrtc::CryptoOptions(),
+        fake_ice_transport_.get(), webrtc::CryptoOptions(),
-                                                      /*event_log=*/nullptr);
+        /*event_log=*/nullptr, ssl_max_version_);
-    dtls_transport_->SetSslMaxProtocolVersion(ssl_max_version_);
     // Note: Certificate may be null here if testing passthrough.
     dtls_transport_->SetLocalCertificate(certificate_);
     dtls_transport_->SignalWritableState.connect(

p2p/base/fake_dtls_transport.h

@@ -146,9 +146,6 @@ class FakeDtlsTransport : public DtlsTransportInternal {
         rtc::SSLFingerprint(alg, rtc::MakeArrayView(digest, digest_len));
     return true;
   }
-  bool SetSslMaxProtocolVersion(rtc::SSLProtocolVersion version) override {
-    return true;
-  }
   bool SetDtlsRole(rtc::SSLRole role) override {
     dtls_role_ = std::move(role);
     return true;
@@ -160,12 +157,6 @@ class FakeDtlsTransport : public DtlsTransportInternal {
     *role = *dtls_role_;
     return true;
   }
-  const webrtc::CryptoOptions& crypto_options() const override {
-    return crypto_options_;
-  }
-  void SetCryptoOptions(const webrtc::CryptoOptions& crypto_options) {
-    crypto_options_ = crypto_options;
-  }
   bool SetLocalCertificate(
       const rtc::scoped_refptr<rtc::RTCCertificate>& certificate) override {
     do_dtls_ = true;
@@ -303,7 +294,6 @@ class FakeDtlsTransport : public DtlsTransportInternal {
   absl::optional<rtc::SSLRole> dtls_role_;
   int crypto_suite_ = rtc::SRTP_AES128_CM_SHA1_80;
   absl::optional<int> ssl_cipher_suite_;
-  webrtc::CryptoOptions crypto_options_;
 
   DtlsTransportState dtls_state_ = DTLS_TRANSPORT_NEW;
 

pc/jsep_transport_controller.cc

@@ -414,14 +414,14 @@ JsepTransportController::CreateDtlsTransport(
 
   if (config_.dtls_transport_factory) {
     dtls = config_.dtls_transport_factory->CreateDtlsTransport(
-        ice, config_.crypto_options);
+        ice, config_.crypto_options, config_.ssl_max_version);
   } else {
     dtls = std::make_unique<cricket::DtlsTransport>(ice, config_.crypto_options,
-                                                    config_.event_log);
+                                                    config_.event_log,
+                                                    config_.ssl_max_version);
   }
 
   RTC_DCHECK(dtls);
-  dtls->SetSslMaxProtocolVersion(config_.ssl_max_version);
   dtls->ice_transport()->SetIceRole(ice_role_);
   dtls->ice_transport()->SetIceTiebreaker(ice_tiebreaker_);
   dtls->ice_transport()->SetIceConfig(ice_config_);

pc/jsep_transport_controller_unittest.cc

@@ -57,7 +57,8 @@ class FakeDtlsTransportFactory : public cricket::DtlsTransportFactory {
  public:
   std::unique_ptr<cricket::DtlsTransportInternal> CreateDtlsTransport(
       cricket::IceTransportInternal* ice,
-      const webrtc::CryptoOptions& crypto_options) override {
+      const webrtc::CryptoOptions& crypto_options,
+      rtc::SSLProtocolVersion max_version) override {
     return std::make_unique<FakeDtlsTransport>(
         static_cast<cricket::FakeIceTransport*>(ice));
   }