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}
diff --git a/p2p/base/dtls_transport.cc b/p2p/base/dtls_transport.cc
index 8317bd2..99ee0f1 100644
--- a/p2p/base/dtls_transport.cc
+++ b/p2p/base/dtls_transport.cc
@@ -134,14 +134,13 @@
DtlsTransport::DtlsTransport(IceTransportInternal* ice_transport,
const webrtc::CryptoOptions& crypto_options,
- webrtc::RtcEventLog* event_log)
- : transport_name_(ice_transport->transport_name()),
- component_(ice_transport->component()),
+ webrtc::RtcEventLog* event_log,
+ rtc::SSLProtocolVersion max_version)
+ : component_(ice_transport->component()),
ice_transport_(ice_transport),
downward_(NULL),
srtp_ciphers_(crypto_options.GetSupportedDtlsSrtpCryptoSuites()),
- ssl_max_version_(rtc::SSL_PROTOCOL_DTLS_12),
- crypto_options_(crypto_options),
+ ssl_max_version_(max_version),
event_log_(event_log) {
RTC_DCHECK(ice_transport_);
ConnectToIceTransport();
@@ -149,16 +148,12 @@
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 @@
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_);
diff --git a/p2p/base/dtls_transport.h b/p2p/base/dtls_transport.h
index bd5061c..f37e468 100644
--- a/p2p/base/dtls_transport.h
+++ b/p2p/base/dtls_transport.h
@@ -101,13 +101,14 @@
//
// |event_log| is an optional RtcEventLog for logging state changes. It should
// outlive the DtlsTransport.
- explicit DtlsTransport(IceTransportInternal* ice_transport,
- const webrtc::CryptoOptions& crypto_options,
- webrtc::RtcEventLog* event_log);
+ DtlsTransport(
+ IceTransportInternal* ice_transport,
+ const webrtc::CryptoOptions& crypto_options,
+ 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 @@
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 @@
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 @@
webrtc::SequenceChecker thread_checker_;
- std::string transport_name_;
- int component_;
+ const 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_;
- webrtc::CryptoOptions crypto_options_;
+ const rtc::SSLProtocolVersion ssl_max_version_;
rtc::Buffer remote_fingerprint_value_;
std::string remote_fingerprint_algorithm_;
diff --git a/p2p/base/dtls_transport_factory.h b/p2p/base/dtls_transport_factory.h
index 9ad78a7..7c4a24a 100644
--- a/p2p/base/dtls_transport_factory.h
+++ b/p2p/base/dtls_transport_factory.h
@@ -31,7 +31,8 @@
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
diff --git a/p2p/base/dtls_transport_internal.h b/p2p/base/dtls_transport_internal.h
index 81aefa9..bd45790 100644
--- a/p2p/base/dtls_transport_internal.h
+++ b/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 @@
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 @@
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;
diff --git a/p2p/base/dtls_transport_unittest.cc b/p2p/base/dtls_transport_unittest.cc
index 6822e55..a2ad213 100644
--- a/p2p/base/dtls_transport_unittest.cc
+++ b/p2p/base/dtls_transport_unittest.cc
@@ -86,10 +86,9 @@
fake_ice_transport_->SignalReadPacket.connect(
this, &DtlsTestClient::OnFakeIceTransportReadPacket);
- dtls_transport_ = std::make_unique<DtlsTransport>(fake_ice_transport_.get(),
- webrtc::CryptoOptions(),
- /*event_log=*/nullptr);
- dtls_transport_->SetSslMaxProtocolVersion(ssl_max_version_);
+ dtls_transport_ = std::make_unique<DtlsTransport>(
+ fake_ice_transport_.get(), webrtc::CryptoOptions(),
+ /*event_log=*/nullptr, ssl_max_version_);
// Note: Certificate may be null here if testing passthrough.
dtls_transport_->SetLocalCertificate(certificate_);
dtls_transport_->SignalWritableState.connect(
diff --git a/p2p/base/fake_dtls_transport.h b/p2p/base/fake_dtls_transport.h
index daec158..0628c4c 100644
--- a/p2p/base/fake_dtls_transport.h
+++ b/p2p/base/fake_dtls_transport.h
@@ -146,9 +146,6 @@
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 @@
*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 @@
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;
diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc
index be93cd8..2784e80 100644
--- a/pc/jsep_transport_controller.cc
+++ b/pc/jsep_transport_controller.cc
@@ -414,14 +414,14 @@
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_);
diff --git a/pc/jsep_transport_controller_unittest.cc b/pc/jsep_transport_controller_unittest.cc
index 0424afe..674ac22 100644
--- a/pc/jsep_transport_controller_unittest.cc
+++ b/pc/jsep_transport_controller_unittest.cc
@@ -57,7 +57,8 @@
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));
}
