diff --git a/p2p/base/fakedtlstransport.h b/p2p/base/fakedtlstransport.h index 8b0619aa08..085d6e0ca1 100644 --- a/p2p/base/fakedtlstransport.h +++ b/p2p/base/fakedtlstransport.h @@ -180,8 +180,10 @@ class FakeDtlsTransport : public DtlsTransportInternal { return local_cert_; } std::unique_ptr GetRemoteSSLCertChain() const override { - return remote_cert_ ? absl::make_unique(remote_cert_) - : nullptr; + if (!remote_cert_) { + return nullptr; + } + return absl::make_unique(remote_cert_->Clone()); } bool ExportKeyingMaterial(const std::string& label, const uint8_t* context, diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index 087a4aee18..357e4726df 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -3159,7 +3159,7 @@ PeerConnection::GetRemoteAudioSSLCertificate() { if (!chain || !chain->GetSize()) { return nullptr; } - return chain->Get(0).GetUniqueReference(); + return chain->Get(0).Clone(); } std::unique_ptr diff --git a/pc/rtcstatscollector_unittest.cc b/pc/rtcstatscollector_unittest.cc index 96ccfdf3ad..1a03e9899b 100644 --- a/pc/rtcstatscollector_unittest.cc +++ b/pc/rtcstatscollector_unittest.cc @@ -703,8 +703,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCertificateStatsSingle) { CreateFakeCertificateAndInfoFromDers( std::vector({"(remote) single certificate"})); pc_->SetRemoteCertChain( - kTransportName, - remote_certinfo->certificate->ssl_cert_chain().UniqueCopy()); + kTransportName, remote_certinfo->certificate->ssl_cert_chain().Clone()); rtc::scoped_refptr report = stats_->GetStatsReport(); @@ -818,7 +817,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCertificateStatsMultiple) { std::vector({"(remote) audio"})); pc_->SetRemoteCertChain( kAudioTransport, - audio_remote_certinfo->certificate->ssl_cert_chain().UniqueCopy()); + audio_remote_certinfo->certificate->ssl_cert_chain().Clone()); pc_->AddVideoChannel("video", kVideoTransport); std::unique_ptr video_local_certinfo = @@ -830,7 +829,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCertificateStatsMultiple) { std::vector({"(remote) video"})); pc_->SetRemoteCertChain( kVideoTransport, - video_remote_certinfo->certificate->ssl_cert_chain().UniqueCopy()); + video_remote_certinfo->certificate->ssl_cert_chain().Clone()); rtc::scoped_refptr report = stats_->GetStatsReport(); ExpectReportContainsCertificateInfo(report, *audio_local_certinfo); @@ -854,8 +853,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCertificateStatsChain) { "(remote) another", "(remote) chain"}); pc_->SetRemoteCertChain( - kTransportName, - remote_certinfo->certificate->ssl_cert_chain().UniqueCopy()); + kTransportName, remote_certinfo->certificate->ssl_cert_chain().Clone()); rtc::scoped_refptr report = stats_->GetStatsReport(); ExpectReportContainsCertificateInfo(report, *local_certinfo); @@ -1956,8 +1954,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCTransportStats) { CreateFakeCertificateAndInfoFromDers( {"(remote) local", "(remote) chain"}); pc_->SetRemoteCertChain( - kTransportName, - remote_certinfo->certificate->ssl_cert_chain().UniqueCopy()); + kTransportName, remote_certinfo->certificate->ssl_cert_chain().Clone()); report = stats_->GetFreshStatsReport(); diff --git a/pc/statscollector_unittest.cc b/pc/statscollector_unittest.cc index 7c61b385a9..cbd7cc37dd 100644 --- a/pc/statscollector_unittest.cc +++ b/pc/statscollector_unittest.cc @@ -642,7 +642,7 @@ class StatsCollectorTest : public testing::Test { std::unique_ptr(local_identity.GetReference()))); pc->SetLocalCertificate(kTransportName, local_certificate); pc->SetRemoteCertChain(kTransportName, - remote_identity.cert_chain().UniqueCopy()); + remote_identity.cert_chain().Clone()); stats->UpdateStats(PeerConnectionInterface::kStatsOutputLevelStandard); diff --git a/pc/test/fakepeerconnectionforstats.h b/pc/test/fakepeerconnectionforstats.h index ae329e4450..af86639eca 100644 --- a/pc/test/fakepeerconnectionforstats.h +++ b/pc/test/fakepeerconnectionforstats.h @@ -319,7 +319,7 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase { const std::string& transport_name) override { auto it = remote_cert_chains_by_transport_.find(transport_name); if (it != remote_cert_chains_by_transport_.end()) { - return it->second->UniqueCopy(); + return it->second->Clone(); } else { return nullptr; } diff --git a/rtc_base/fakesslidentity.cc b/rtc_base/fakesslidentity.cc index 80a3e78887..62ac9dd020 100644 --- a/rtc_base/fakesslidentity.cc +++ b/rtc_base/fakesslidentity.cc @@ -29,8 +29,8 @@ FakeSSLCertificate::FakeSSLCertificate(const FakeSSLCertificate&) = default; FakeSSLCertificate::~FakeSSLCertificate() = default; -FakeSSLCertificate* FakeSSLCertificate::GetReference() const { - return new FakeSSLCertificate(*this); +std::unique_ptr FakeSSLCertificate::Clone() const { + return absl::make_unique(*this); } std::string FakeSSLCertificate::ToPEMString() const { @@ -83,10 +83,10 @@ FakeSSLIdentity::FakeSSLIdentity(const std::vector& pem_strings) { } FakeSSLIdentity::FakeSSLIdentity(const FakeSSLCertificate& cert) - : cert_chain_(absl::make_unique(&cert)) {} + : cert_chain_(absl::make_unique(cert.Clone())) {} FakeSSLIdentity::FakeSSLIdentity(const FakeSSLIdentity& o) - : cert_chain_(o.cert_chain_->UniqueCopy()) {} + : cert_chain_(o.cert_chain_->Clone()) {} FakeSSLIdentity::~FakeSSLIdentity() = default; diff --git a/rtc_base/fakesslidentity.h b/rtc_base/fakesslidentity.h index 4494a524ef..9d5770ce28 100644 --- a/rtc_base/fakesslidentity.h +++ b/rtc_base/fakesslidentity.h @@ -28,7 +28,7 @@ class FakeSSLCertificate : public SSLCertificate { ~FakeSSLCertificate() override; // SSLCertificate implementation. - FakeSSLCertificate* GetReference() const override; + std::unique_ptr Clone() const override; std::string ToPEMString() const override; void ToDER(Buffer* der_buffer) const override; int64_t CertificateExpirationTime() const override; diff --git a/rtc_base/opensslcertificate.cc b/rtc_base/opensslcertificate.cc index ed67a8938e..92443a4458 100644 --- a/rtc_base/opensslcertificate.cc +++ b/rtc_base/opensslcertificate.cc @@ -130,10 +130,11 @@ static void PrintCert(X509* x509) { #endif OpenSSLCertificate::OpenSSLCertificate(X509* x509) : x509_(x509) { - AddReference(); + RTC_DCHECK(x509_ != nullptr); + X509_up_ref(x509_); } -OpenSSLCertificate* OpenSSLCertificate::Generate( +std::unique_ptr OpenSSLCertificate::Generate( OpenSSLKeyPair* key_pair, const SSLIdentityParams& params) { SSLIdentityParams actual_params(params); @@ -149,12 +150,12 @@ OpenSSLCertificate* OpenSSLCertificate::Generate( #if !defined(NDEBUG) PrintCert(x509); #endif - OpenSSLCertificate* ret = new OpenSSLCertificate(x509); + auto ret = absl::make_unique(x509); X509_free(x509); return ret; } -OpenSSLCertificate* OpenSSLCertificate::FromPEMString( +std::unique_ptr OpenSSLCertificate::FromPEMString( const std::string& pem_string) { BIO* bio = BIO_new_mem_buf(const_cast(pem_string.c_str()), -1); if (!bio) @@ -167,7 +168,7 @@ OpenSSLCertificate* OpenSSLCertificate::FromPEMString( if (!x509) return nullptr; - OpenSSLCertificate* ret = new OpenSSLCertificate(x509); + auto ret = absl::make_unique(x509); X509_free(x509); return ret; } @@ -249,8 +250,8 @@ OpenSSLCertificate::~OpenSSLCertificate() { X509_free(x509_); } -OpenSSLCertificate* OpenSSLCertificate::GetReference() const { - return new OpenSSLCertificate(x509_); +std::unique_ptr OpenSSLCertificate::Clone() const { + return absl::make_unique(x509_); } std::string OpenSSLCertificate::ToPEMString() const { @@ -289,11 +290,6 @@ void OpenSSLCertificate::ToDER(Buffer* der_buffer) const { BIO_free(bio); } -void OpenSSLCertificate::AddReference() const { - RTC_DCHECK(x509_ != nullptr); - X509_up_ref(x509_); -} - bool OpenSSLCertificate::operator==(const OpenSSLCertificate& other) const { return X509_cmp(x509_, other.x509_) == 0; } diff --git a/rtc_base/opensslcertificate.h b/rtc_base/opensslcertificate.h index b7ecc3b78d..3b49f93ef5 100644 --- a/rtc_base/opensslcertificate.h +++ b/rtc_base/opensslcertificate.h @@ -36,13 +36,15 @@ class OpenSSLCertificate : public SSLCertificate { // OpenSSLCertificate share ownership. explicit OpenSSLCertificate(X509* x509); - static OpenSSLCertificate* Generate(OpenSSLKeyPair* key_pair, - const SSLIdentityParams& params); - static OpenSSLCertificate* FromPEMString(const std::string& pem_string); + static std::unique_ptr Generate( + OpenSSLKeyPair* key_pair, + const SSLIdentityParams& params); + static std::unique_ptr FromPEMString( + const std::string& pem_string); ~OpenSSLCertificate() override; - OpenSSLCertificate* GetReference() const override; + std::unique_ptr Clone() const override; X509* x509() const { return x509_; } @@ -69,8 +71,6 @@ class OpenSSLCertificate : public SSLCertificate { int64_t CertificateExpirationTime() const override; private: - void AddReference() const; - X509* x509_; // NOT OWNED RTC_DISALLOW_COPY_AND_ASSIGN(OpenSSLCertificate); }; diff --git a/rtc_base/opensslidentity.cc b/rtc_base/opensslidentity.cc index a8c6919779..a5bbd5d72d 100644 --- a/rtc_base/opensslidentity.cc +++ b/rtc_base/opensslidentity.cc @@ -316,7 +316,7 @@ const SSLCertChain& OpenSSLIdentity::cert_chain() const { OpenSSLIdentity* OpenSSLIdentity::GetReference() const { return new OpenSSLIdentity(absl::WrapUnique(key_pair_->GetReference()), - absl::WrapUnique(cert_chain_->Copy())); + cert_chain_->Clone()); } bool OpenSSLIdentity::ConfigureIdentity(SSL_CTX* ctx) { diff --git a/rtc_base/opensslstreamadapter.cc b/rtc_base/opensslstreamadapter.cc index 32a2752b68..3eb3b80c7b 100644 --- a/rtc_base/opensslstreamadapter.cc +++ b/rtc_base/opensslstreamadapter.cc @@ -1114,7 +1114,7 @@ bool OpenSSLStreamAdapter::VerifyPeerCertificate() { std::unique_ptr OpenSSLStreamAdapter::GetPeerSSLCertChain() const { - return peer_cert_chain_ ? peer_cert_chain_->UniqueCopy() : nullptr; + return peer_cert_chain_ ? peer_cert_chain_->Clone() : nullptr; } int OpenSSLStreamAdapter::SSLVerifyCallback(X509_STORE_CTX* store, void* arg) { diff --git a/rtc_base/sslcertificate.cc b/rtc_base/sslcertificate.cc index 53af0f5854..d735ebf950 100644 --- a/rtc_base/sslcertificate.cc +++ b/rtc_base/sslcertificate.cc @@ -30,7 +30,7 @@ SSLCertificateStats::SSLCertificateStats( std::string&& fingerprint, std::string&& fingerprint_algorithm, std::string&& base64_certificate, - std::unique_ptr&& issuer) + std::unique_ptr issuer) : fingerprint(std::move(fingerprint)), fingerprint_algorithm(std::move(fingerprint_algorithm)), base64_certificate(std::move(base64_certificate)), @@ -70,49 +70,30 @@ std::unique_ptr SSLCertificate::GetStats() const { std::move(der_base64), nullptr); } -std::unique_ptr SSLCertificate::GetUniqueReference() const { - return absl::WrapUnique(GetReference()); -} - ////////////////////////////////////////////////////////////////////// // SSLCertChain ////////////////////////////////////////////////////////////////////// +SSLCertChain::SSLCertChain(std::unique_ptr single_cert) { + certs_.push_back(std::move(single_cert)); +} + SSLCertChain::SSLCertChain(std::vector> certs) : certs_(std::move(certs)) {} -SSLCertChain::SSLCertChain(const std::vector& certs) { - RTC_DCHECK(!certs.empty()); - certs_.resize(certs.size()); - std::transform( - certs.begin(), certs.end(), certs_.begin(), - [](const SSLCertificate* cert) -> std::unique_ptr { - return cert->GetUniqueReference(); - }); -} - -SSLCertChain::SSLCertChain(const SSLCertificate* cert) { - certs_.push_back(cert->GetUniqueReference()); -} - SSLCertChain::SSLCertChain(SSLCertChain&& rhs) = default; SSLCertChain& SSLCertChain::operator=(SSLCertChain&&) = default; -SSLCertChain::~SSLCertChain() {} +SSLCertChain::~SSLCertChain() = default; -SSLCertChain* SSLCertChain::Copy() const { +std::unique_ptr SSLCertChain::Clone() const { std::vector> new_certs(certs_.size()); - std::transform(certs_.begin(), certs_.end(), new_certs.begin(), - [](const std::unique_ptr& cert) - -> std::unique_ptr { - return cert->GetUniqueReference(); - }); - return new SSLCertChain(std::move(new_certs)); -} - -std::unique_ptr SSLCertChain::UniqueCopy() const { - return absl::WrapUnique(Copy()); + std::transform( + certs_.begin(), certs_.end(), new_certs.begin(), + [](const std::unique_ptr& cert) + -> std::unique_ptr { return cert->Clone(); }); + return absl::make_unique(std::move(new_certs)); } std::unique_ptr SSLCertChain::GetStats() const { @@ -134,7 +115,8 @@ std::unique_ptr SSLCertChain::GetStats() const { } // static -SSLCertificate* SSLCertificate::FromPEMString(const std::string& pem_string) { +std::unique_ptr SSLCertificate::FromPEMString( + const std::string& pem_string) { return OpenSSLCertificate::FromPEMString(pem_string); } diff --git a/rtc_base/sslcertificate.h b/rtc_base/sslcertificate.h index 029404cf3e..c04852f2f2 100644 --- a/rtc_base/sslcertificate.h +++ b/rtc_base/sslcertificate.h @@ -28,7 +28,7 @@ struct SSLCertificateStats { SSLCertificateStats(std::string&& fingerprint, std::string&& fingerprint_algorithm, std::string&& base64_certificate, - std::unique_ptr&& issuer); + std::unique_ptr issuer); ~SSLCertificateStats(); std::string fingerprint; std::string fingerprint_algorithm; @@ -51,17 +51,13 @@ class SSLCertificate { // The length of the string representation of the certificate is // stored in *pem_length if it is non-null, and only if // parsing was successful. - // Caller is responsible for freeing the returned object. - static SSLCertificate* FromPEMString(const std::string& pem_string); - virtual ~SSLCertificate() {} + static std::unique_ptr FromPEMString( + const std::string& pem_string); + virtual ~SSLCertificate() = default; // Returns a new SSLCertificate object instance wrapping the same - // underlying certificate, including its chain if present. Caller is - // responsible for freeing the returned object. Use GetUniqueReference - // instead. - virtual SSLCertificate* GetReference() const = 0; - - std::unique_ptr GetUniqueReference() const; + // underlying certificate, including its chain if present. + virtual std::unique_ptr Clone() const = 0; // Returns a PEM encoded string representation of the certificate. virtual std::string ToPEMString() const = 0; @@ -94,11 +90,8 @@ class SSLCertificate { // SSLCertificate pointers. class SSLCertChain { public: + explicit SSLCertChain(std::unique_ptr single_cert); explicit SSLCertChain(std::vector> certs); - // These constructors copy the provided SSLCertificate(s), so the caller - // retains ownership. - explicit SSLCertChain(const std::vector& certs); - explicit SSLCertChain(const SSLCertificate* cert); // Allow move semantics for the object. SSLCertChain(SSLCertChain&&); SSLCertChain& operator=(SSLCertChain&&); @@ -112,10 +105,8 @@ class SSLCertChain { const SSLCertificate& Get(size_t pos) const { return *(certs_[pos]); } // Returns a new SSLCertChain object instance wrapping the same underlying - // certificate chain. Caller is responsible for freeing the returned object. - SSLCertChain* Copy() const; - // Same as above, but returning a unique_ptr for convenience. - std::unique_ptr UniqueCopy() const; + // certificate chain. + std::unique_ptr Clone() const; // Gets information (fingerprint, etc.) about this certificate chain. This is // used for certificate stats, see diff --git a/rtc_base/sslidentity_unittest.cc b/rtc_base/sslidentity_unittest.cc index 81831848a9..ba53d17da4 100644 --- a/rtc_base/sslidentity_unittest.cc +++ b/rtc_base/sslidentity_unittest.cc @@ -201,7 +201,7 @@ class SSLIdentityTest : public testing::Test { ASSERT_TRUE(identity_ecdsa1_); ASSERT_TRUE(identity_ecdsa2_); - test_cert_.reset(rtc::SSLCertificate::FromPEMString(kTestCertificate)); + test_cert_ = rtc::SSLCertificate::FromPEMString(kTestCertificate); ASSERT_TRUE(test_cert_); } diff --git a/rtc_base/sslstreamadapter_unittest.cc b/rtc_base/sslstreamadapter_unittest.cc index 389b0eaaf1..ff4c7a0d92 100644 --- a/rtc_base/sslstreamadapter_unittest.cc +++ b/rtc_base/sslstreamadapter_unittest.cc @@ -588,8 +588,7 @@ class SSLStreamAdapterTestBase : public testing::Test, chain = client_ssl_->GetPeerSSLCertChain(); else chain = server_ssl_->GetPeerSSLCertChain(); - return (chain && chain->GetSize()) ? chain->Get(0).GetUniqueReference() - : nullptr; + return (chain && chain->GetSize()) ? chain->Get(0).Clone() : nullptr; } bool GetSslCipherSuite(bool client, int* retval) {