Reland: Modernize rtc::SSLCertificate
Bug: webrtc:9860
Change-Id: I2344e2333f68e5d58ca38dfc041a676692401312
Tbr: Benjamin Wright <benwright@webrtc.org>
Tbr: Qingsi Wang <qingsi@webrtc.org>
Reviewed-on: https://webrtc-review.googlesource.com/c/106604
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25225}diff --git a/p2p/base/fakedtlstransport.h b/p2p/base/fakedtlstransport.h
index 8b0619a..085d6e0 100644
--- a/p2p/base/fakedtlstransport.h
+++ b/p2p/base/fakedtlstransport.h
@@ -180,8 +180,10 @@
return local_cert_;
}
std::unique_ptr<rtc::SSLCertChain> GetRemoteSSLCertChain() const override {
- return remote_cert_ ? absl::make_unique<rtc::SSLCertChain>(remote_cert_)
- : nullptr;
+ if (!remote_cert_) {
+ return nullptr;
+ }
+ return absl::make_unique<rtc::SSLCertChain>(remote_cert_->Clone());
}
bool ExportKeyingMaterial(const std::string& label,
const uint8_t* context,
diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc
index 087a4ae..357e472 100644
--- a/pc/peerconnection.cc
+++ b/pc/peerconnection.cc
@@ -3159,7 +3159,7 @@
if (!chain || !chain->GetSize()) {
return nullptr;
}
- return chain->Get(0).GetUniqueReference();
+ return chain->Get(0).Clone();
}
std::unique_ptr<rtc::SSLCertChain>
diff --git a/pc/rtcstatscollector_unittest.cc b/pc/rtcstatscollector_unittest.cc
index 96ccfdf..1a03e98 100644
--- a/pc/rtcstatscollector_unittest.cc
+++ b/pc/rtcstatscollector_unittest.cc
@@ -703,8 +703,7 @@
CreateFakeCertificateAndInfoFromDers(
std::vector<std::string>({"(remote) single certificate"}));
pc_->SetRemoteCertChain(
- kTransportName,
- remote_certinfo->certificate->ssl_cert_chain().UniqueCopy());
+ kTransportName, remote_certinfo->certificate->ssl_cert_chain().Clone());
rtc::scoped_refptr<const RTCStatsReport> report = stats_->GetStatsReport();
@@ -818,7 +817,7 @@
std::vector<std::string>({"(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<CertificateInfo> video_local_certinfo =
@@ -830,7 +829,7 @@
std::vector<std::string>({"(remote) video"}));
pc_->SetRemoteCertChain(
kVideoTransport,
- video_remote_certinfo->certificate->ssl_cert_chain().UniqueCopy());
+ video_remote_certinfo->certificate->ssl_cert_chain().Clone());
rtc::scoped_refptr<const RTCStatsReport> report = stats_->GetStatsReport();
ExpectReportContainsCertificateInfo(report, *audio_local_certinfo);
@@ -854,8 +853,7 @@
"(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<const RTCStatsReport> report = stats_->GetStatsReport();
ExpectReportContainsCertificateInfo(report, *local_certinfo);
@@ -1956,8 +1954,7 @@
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 7c61b38..cbd7cc3 100644
--- a/pc/statscollector_unittest.cc
+++ b/pc/statscollector_unittest.cc
@@ -642,7 +642,7 @@
std::unique_ptr<rtc::SSLIdentity>(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 ae329e4..af86639 100644
--- a/pc/test/fakepeerconnectionforstats.h
+++ b/pc/test/fakepeerconnectionforstats.h
@@ -319,7 +319,7 @@
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 80a3e78..62ac9dd 100644
--- a/rtc_base/fakesslidentity.cc
+++ b/rtc_base/fakesslidentity.cc
@@ -29,8 +29,8 @@
FakeSSLCertificate::~FakeSSLCertificate() = default;
-FakeSSLCertificate* FakeSSLCertificate::GetReference() const {
- return new FakeSSLCertificate(*this);
+std::unique_ptr<SSLCertificate> FakeSSLCertificate::Clone() const {
+ return absl::make_unique<FakeSSLCertificate>(*this);
}
std::string FakeSSLCertificate::ToPEMString() const {
@@ -83,10 +83,10 @@
}
FakeSSLIdentity::FakeSSLIdentity(const FakeSSLCertificate& cert)
- : cert_chain_(absl::make_unique<SSLCertChain>(&cert)) {}
+ : cert_chain_(absl::make_unique<SSLCertChain>(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 4494a52..9d5770c 100644
--- a/rtc_base/fakesslidentity.h
+++ b/rtc_base/fakesslidentity.h
@@ -28,7 +28,7 @@
~FakeSSLCertificate() override;
// SSLCertificate implementation.
- FakeSSLCertificate* GetReference() const override;
+ std::unique_ptr<SSLCertificate> 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 ed67a89..92443a4 100644
--- a/rtc_base/opensslcertificate.cc
+++ b/rtc_base/opensslcertificate.cc
@@ -130,10 +130,11 @@
#endif
OpenSSLCertificate::OpenSSLCertificate(X509* x509) : x509_(x509) {
- AddReference();
+ RTC_DCHECK(x509_ != nullptr);
+ X509_up_ref(x509_);
}
-OpenSSLCertificate* OpenSSLCertificate::Generate(
+std::unique_ptr<OpenSSLCertificate> OpenSSLCertificate::Generate(
OpenSSLKeyPair* key_pair,
const SSLIdentityParams& params) {
SSLIdentityParams actual_params(params);
@@ -149,12 +150,12 @@
#if !defined(NDEBUG)
PrintCert(x509);
#endif
- OpenSSLCertificate* ret = new OpenSSLCertificate(x509);
+ auto ret = absl::make_unique<OpenSSLCertificate>(x509);
X509_free(x509);
return ret;
}
-OpenSSLCertificate* OpenSSLCertificate::FromPEMString(
+std::unique_ptr<OpenSSLCertificate> OpenSSLCertificate::FromPEMString(
const std::string& pem_string) {
BIO* bio = BIO_new_mem_buf(const_cast<char*>(pem_string.c_str()), -1);
if (!bio)
@@ -167,7 +168,7 @@
if (!x509)
return nullptr;
- OpenSSLCertificate* ret = new OpenSSLCertificate(x509);
+ auto ret = absl::make_unique<OpenSSLCertificate>(x509);
X509_free(x509);
return ret;
}
@@ -249,8 +250,8 @@
X509_free(x509_);
}
-OpenSSLCertificate* OpenSSLCertificate::GetReference() const {
- return new OpenSSLCertificate(x509_);
+std::unique_ptr<SSLCertificate> OpenSSLCertificate::Clone() const {
+ return absl::make_unique<OpenSSLCertificate>(x509_);
}
std::string OpenSSLCertificate::ToPEMString() const {
@@ -289,11 +290,6 @@
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 b7ecc3b..3b49f93 100644
--- a/rtc_base/opensslcertificate.h
+++ b/rtc_base/opensslcertificate.h
@@ -36,13 +36,15 @@
// 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<OpenSSLCertificate> Generate(
+ OpenSSLKeyPair* key_pair,
+ const SSLIdentityParams& params);
+ static std::unique_ptr<OpenSSLCertificate> FromPEMString(
+ const std::string& pem_string);
~OpenSSLCertificate() override;
- OpenSSLCertificate* GetReference() const override;
+ std::unique_ptr<SSLCertificate> Clone() const override;
X509* x509() const { return x509_; }
@@ -69,8 +71,6 @@
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 a8c6919..a5bbd5d 100644
--- a/rtc_base/opensslidentity.cc
+++ b/rtc_base/opensslidentity.cc
@@ -316,7 +316,7 @@
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 32a2752..3eb3b80 100644
--- a/rtc_base/opensslstreamadapter.cc
+++ b/rtc_base/opensslstreamadapter.cc
@@ -1114,7 +1114,7 @@
std::unique_ptr<SSLCertChain> 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 53af0f5..d735ebf 100644
--- a/rtc_base/sslcertificate.cc
+++ b/rtc_base/sslcertificate.cc
@@ -30,7 +30,7 @@
std::string&& fingerprint,
std::string&& fingerprint_algorithm,
std::string&& base64_certificate,
- std::unique_ptr<SSLCertificateStats>&& issuer)
+ std::unique_ptr<SSLCertificateStats> issuer)
: fingerprint(std::move(fingerprint)),
fingerprint_algorithm(std::move(fingerprint_algorithm)),
base64_certificate(std::move(base64_certificate)),
@@ -70,49 +70,30 @@
std::move(der_base64), nullptr);
}
-std::unique_ptr<SSLCertificate> SSLCertificate::GetUniqueReference() const {
- return absl::WrapUnique(GetReference());
-}
-
//////////////////////////////////////////////////////////////////////
// SSLCertChain
//////////////////////////////////////////////////////////////////////
+SSLCertChain::SSLCertChain(std::unique_ptr<SSLCertificate> single_cert) {
+ certs_.push_back(std::move(single_cert));
+}
+
SSLCertChain::SSLCertChain(std::vector<std::unique_ptr<SSLCertificate>> certs)
: certs_(std::move(certs)) {}
-SSLCertChain::SSLCertChain(const std::vector<SSLCertificate*>& certs) {
- RTC_DCHECK(!certs.empty());
- certs_.resize(certs.size());
- std::transform(
- certs.begin(), certs.end(), certs_.begin(),
- [](const SSLCertificate* cert) -> std::unique_ptr<SSLCertificate> {
- 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> SSLCertChain::Clone() const {
std::vector<std::unique_ptr<SSLCertificate>> new_certs(certs_.size());
- std::transform(certs_.begin(), certs_.end(), new_certs.begin(),
- [](const std::unique_ptr<SSLCertificate>& cert)
- -> std::unique_ptr<SSLCertificate> {
- return cert->GetUniqueReference();
- });
- return new SSLCertChain(std::move(new_certs));
-}
-
-std::unique_ptr<SSLCertChain> SSLCertChain::UniqueCopy() const {
- return absl::WrapUnique(Copy());
+ std::transform(
+ certs_.begin(), certs_.end(), new_certs.begin(),
+ [](const std::unique_ptr<SSLCertificate>& cert)
+ -> std::unique_ptr<SSLCertificate> { return cert->Clone(); });
+ return absl::make_unique<SSLCertChain>(std::move(new_certs));
}
std::unique_ptr<SSLCertificateStats> SSLCertChain::GetStats() const {
@@ -134,7 +115,8 @@
}
// static
-SSLCertificate* SSLCertificate::FromPEMString(const std::string& pem_string) {
+std::unique_ptr<SSLCertificate> 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 029404c..c04852f 100644
--- a/rtc_base/sslcertificate.h
+++ b/rtc_base/sslcertificate.h
@@ -28,7 +28,7 @@
SSLCertificateStats(std::string&& fingerprint,
std::string&& fingerprint_algorithm,
std::string&& base64_certificate,
- std::unique_ptr<SSLCertificateStats>&& issuer);
+ std::unique_ptr<SSLCertificateStats> issuer);
~SSLCertificateStats();
std::string fingerprint;
std::string fingerprint_algorithm;
@@ -51,17 +51,13 @@
// 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<SSLCertificate> 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<SSLCertificate> GetUniqueReference() const;
+ // underlying certificate, including its chain if present.
+ virtual std::unique_ptr<SSLCertificate> Clone() const = 0;
// Returns a PEM encoded string representation of the certificate.
virtual std::string ToPEMString() const = 0;
@@ -94,11 +90,8 @@
// SSLCertificate pointers.
class SSLCertChain {
public:
+ explicit SSLCertChain(std::unique_ptr<SSLCertificate> single_cert);
explicit SSLCertChain(std::vector<std::unique_ptr<SSLCertificate>> certs);
- // These constructors copy the provided SSLCertificate(s), so the caller
- // retains ownership.
- explicit SSLCertChain(const std::vector<SSLCertificate*>& certs);
- explicit SSLCertChain(const SSLCertificate* cert);
// Allow move semantics for the object.
SSLCertChain(SSLCertChain&&);
SSLCertChain& operator=(SSLCertChain&&);
@@ -112,10 +105,8 @@
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<SSLCertChain> UniqueCopy() const;
+ // certificate chain.
+ std::unique_ptr<SSLCertChain> 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 8183184..ba53d17 100644
--- a/rtc_base/sslidentity_unittest.cc
+++ b/rtc_base/sslidentity_unittest.cc
@@ -201,7 +201,7 @@
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 389b0ea..ff4c7a0 100644
--- a/rtc_base/sslstreamadapter_unittest.cc
+++ b/rtc_base/sslstreamadapter_unittest.cc
@@ -588,8 +588,7 @@
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) {