Modernize rtc::SSLCertificate

Bug: webrtc:9860
Change-Id: Idfce546ded500d957397c5bd873200565d3e6b64
Reviewed-on: https://webrtc-review.googlesource.com/c/105280
Reviewed-by: Benjamin Wright <benwright@webrtc.org>
Reviewed-by: Qingsi Wang <qingsi@webrtc.org>
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25150}

p2p/base/fakedtlstransport.h

@@ -180,8 +180,10 @@ class FakeDtlsTransport : public DtlsTransportInternal {
     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,

pc/peerconnection.cc

@@ -3144,7 +3144,7 @@ PeerConnection::GetRemoteAudioSSLCertificate() {
   if (!chain || !chain->GetSize()) {
     return nullptr;
   }
-  return chain->Get(0).GetUniqueReference();
+  return chain->Get(0).Clone();
 }
 
 std::unique_ptr<rtc::SSLCertChain>

pc/rtcstatscollector_unittest.cc

@@ -703,8 +703,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCertificateStatsSingle) {
       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 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCertificateStatsMultiple) {
           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 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCertificateStatsMultiple) {
           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 @@ 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<const RTCStatsReport> 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();
 

pc/statscollector_unittest.cc

@@ -642,7 +642,7 @@ class StatsCollectorTest : public testing::Test {
             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);
 

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;
     }

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<SSLCertificate> FakeSSLCertificate::Clone() const {
+  return absl::make_unique<FakeSSLCertificate>(*this);
 }
 
 std::string FakeSSLCertificate::ToPEMString() const {
@@ -83,10 +83,10 @@ FakeSSLIdentity::FakeSSLIdentity(const std::vector<std::string>& pem_strings) {
 }
 
 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;
 

rtc_base/fakesslidentity.h

@@ -28,7 +28,7 @@ class FakeSSLCertificate : public SSLCertificate {
   ~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;

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> 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<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 @@ OpenSSLCertificate* OpenSSLCertificate::FromPEMString(
   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 @@ OpenSSLCertificate::~OpenSSLCertificate() {
   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 @@ 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;
 }

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<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 @@ class OpenSSLCertificate : public SSLCertificate {
   int64_t CertificateExpirationTime() const override;
 
  private:
-  void AddReference() const;
-
   X509* x509_;  // NOT OWNED
   RTC_DISALLOW_COPY_AND_ASSIGN(OpenSSLCertificate);
 };

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) {

rtc_base/opensslstreamadapter.cc

@@ -1091,7 +1091,7 @@ bool OpenSSLStreamAdapter::VerifyPeerCertificate() {
 
 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) {

rtc_base/sslcertificate.cc

@@ -30,7 +30,7 @@ SSLCertificateStats::SSLCertificateStats(
     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::unique_ptr<SSLCertificateStats> SSLCertificate::GetStats() const {
                                                 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 @@ std::unique_ptr<SSLCertificateStats> SSLCertChain::GetStats() const {
 }
 
 // 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);
 }
 

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<SSLCertificateStats>&& issuer);
+                      std::unique_ptr<SSLCertificateStats> 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<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 @@ class SSLCertificate {
 // 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 @@ 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<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

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_);
   }
 

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) {