From 6932fb2e3405d8be66deac939718f27f32c8817f Mon Sep 17 00:00:00 2001 From: Mirko Bonadei Date: Mon, 15 Oct 2018 14:18:03 +0000 Subject: [PATCH] Revert "Reland: Use unique_ptr and ArrayView in SSLFingerprint" This reverts commit 47f3240a6621e90a17d92e620ae765c353c87e11. Reason for revert: Breaks WebRTC roll into Chromium. Original change's description: > Reland: Use unique_ptr and ArrayView in SSLFingerprint > > Bug: webrtc:9860 > Change-Id: I550528556aa27050015de29d9d7d99cd9df59ce5 > Reviewed-on: https://webrtc-review.googlesource.com/c/105520 > Reviewed-by: Benjamin Wright > Reviewed-by: Qingsi Wang > Commit-Queue: Steve Anton > Cr-Commit-Position: refs/heads/master@{#25149} TBR=steveanton@webrtc.org,qingsi@webrtc.org,benwright@webrtc.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: webrtc:9860 Change-Id: Ib1b5759abf6e79a569ca04b66eabc3021d4c16e4 Reviewed-on: https://webrtc-review.googlesource.com/c/106060 Reviewed-by: Mirko Bonadei Commit-Queue: Mirko Bonadei Cr-Commit-Position: refs/heads/master@{#25173} --- p2p/base/dtlstransport_unittest.cc | 5 ++- p2p/base/fakedtlstransport.h | 7 ++- p2p/base/transportdescriptionfactory.cc | 4 +- pc/jseptransport.cc | 8 ++-- pc/jseptransport_unittest.cc | 7 ++- pc/jseptransportcontroller_unittest.cc | 2 +- pc/peerconnection_crypto_unittest.cc | 4 +- pc/rtcstatscollector_unittest.cc | 2 +- pc/webrtcsdp.cc | 27 +++++------- pc/webrtcsdp_unittest.cc | 3 +- rtc_base/sslcertificate.cc | 4 +- rtc_base/sslfingerprint.cc | 58 ++++++++++--------------- rtc_base/sslfingerprint.h | 23 ++-------- rtc_base/sslidentity_unittest.cc | 2 +- 14 files changed, 62 insertions(+), 94 deletions(-) diff --git a/p2p/base/dtlstransport_unittest.cc b/p2p/base/dtlstransport_unittest.cc index 2ccb64cbaf..8dfbf6106c 100644 --- a/p2p/base/dtlstransport_unittest.cc +++ b/p2p/base/dtlstransport_unittest.cc @@ -47,8 +47,8 @@ void SetRemoteFingerprintFromCert( DtlsTransport* transport, const rtc::scoped_refptr& cert, bool modify_digest = false) { - std::unique_ptr fingerprint = - rtc::SSLFingerprint::CreateFromCertificate(*cert); + rtc::SSLFingerprint* fingerprint = + rtc::SSLFingerprint::CreateFromCertificate(cert); if (modify_digest) { ++fingerprint->digest[0]; } @@ -57,6 +57,7 @@ void SetRemoteFingerprintFromCert( fingerprint->algorithm, reinterpret_cast(fingerprint->digest.data()), fingerprint->digest.size())); + delete fingerprint; } class DtlsTestClient : public sigslot::has_slots<> { diff --git a/p2p/base/fakedtlstransport.h b/p2p/base/fakedtlstransport.h index 085d6e0ca1..44a12a597b 100644 --- a/p2p/base/fakedtlstransport.h +++ b/p2p/base/fakedtlstransport.h @@ -33,7 +33,7 @@ class FakeDtlsTransport : public DtlsTransportInternal { : ice_transport_(ice_transport), transport_name_(ice_transport->transport_name()), component_(ice_transport->component()), - dtls_fingerprint_("", nullptr) { + dtls_fingerprint_("", nullptr, 0) { RTC_DCHECK(ice_transport_); ice_transport_->SignalReadPacket.connect( this, &FakeDtlsTransport::OnIceTransportReadPacket); @@ -45,7 +45,7 @@ class FakeDtlsTransport : public DtlsTransportInternal { : owned_ice_transport_(std::move(ice)), transport_name_(owned_ice_transport_->transport_name()), component_(owned_ice_transport_->component()), - dtls_fingerprint_("", rtc::ArrayView()) { + dtls_fingerprint_("", nullptr, 0) { ice_transport_ = owned_ice_transport_.get(); ice_transport_->SignalReadPacket.connect( this, &FakeDtlsTransport::OnIceTransportReadPacket); @@ -133,8 +133,7 @@ class FakeDtlsTransport : public DtlsTransportInternal { bool SetRemoteFingerprint(const std::string& alg, const uint8_t* digest, size_t digest_len) override { - dtls_fingerprint_ = - rtc::SSLFingerprint(alg, rtc::MakeArrayView(digest, digest_len)); + dtls_fingerprint_ = rtc::SSLFingerprint(alg, digest, digest_len); return true; } bool SetSslMaxProtocolVersion(rtc::SSLProtocolVersion version) override { diff --git a/p2p/base/transportdescriptionfactory.cc b/p2p/base/transportdescriptionfactory.cc index 689cd4f7a2..670950d5ec 100644 --- a/p2p/base/transportdescriptionfactory.cc +++ b/p2p/base/transportdescriptionfactory.cc @@ -120,8 +120,8 @@ bool TransportDescriptionFactory::SetSecurityInfo(TransportDescription* desc, // This digest algorithm is used to produce the a=fingerprint lines in SDP. // RFC 4572 Section 5 requires that those lines use the same hash function as // the certificate's signature, which is what CreateFromCertificate does. - desc->identity_fingerprint = - rtc::SSLFingerprint::CreateFromCertificate(*certificate_); + desc->identity_fingerprint.reset( + rtc::SSLFingerprint::CreateFromCertificate(certificate_)); if (!desc->identity_fingerprint) { return false; } diff --git a/pc/jseptransport.cc b/pc/jseptransport.cc index 368b32f6f7..894dce122d 100644 --- a/pc/jseptransport.cc +++ b/pc/jseptransport.cc @@ -316,9 +316,8 @@ webrtc::RTCError JsepTransport::VerifyCertificateFingerprint( return webrtc::RTCError(webrtc::RTCErrorType::INVALID_PARAMETER, "Fingerprint provided but no identity available."); } - std::unique_ptr fp_tmp = - rtc::SSLFingerprint::CreateUnique(fingerprint->algorithm, - *certificate->identity()); + std::unique_ptr fp_tmp(rtc::SSLFingerprint::Create( + fingerprint->algorithm, certificate->identity())); RTC_DCHECK(fp_tmp.get() != NULL); if (*fp_tmp == *fingerprint) { return webrtc::RTCError::OK(); @@ -507,8 +506,7 @@ webrtc::RTCError JsepTransport::NegotiateAndSetDtlsParameters( "Local fingerprint supplied when caller didn't offer DTLS."); } else { // We are not doing DTLS - remote_fingerprint = absl::make_unique( - "", rtc::ArrayView()); + remote_fingerprint = absl::make_unique("", nullptr, 0); } // Now that we have negotiated everything, push it downward. // Note that we cache the result so that if we have race conditions diff --git a/pc/jseptransport_unittest.cc b/pc/jseptransport_unittest.cc index 748a4e078f..1b42578c73 100644 --- a/pc/jseptransport_unittest.cc +++ b/pc/jseptransport_unittest.cc @@ -126,7 +126,7 @@ class JsepTransport2Test : public testing::Test, public sigslot::has_slots<> { std::unique_ptr fingerprint; if (cert) { - fingerprint = rtc::SSLFingerprint::CreateFromCertificate(*cert); + fingerprint.reset(rtc::SSLFingerprint::CreateFromCertificate(cert)); } jsep_description.transport_desc = TransportDescription(std::vector(), ufrag, pwd, @@ -386,9 +386,8 @@ TEST_P(JsepTransport2WithRtcpMux, VerifyCertificateFingerprint) { ASSERT_TRUE(certificate->ssl_certificate().GetSignatureDigestAlgorithm( &digest_algorithm)); ASSERT_FALSE(digest_algorithm.empty()); - std::unique_ptr good_fingerprint = - rtc::SSLFingerprint::CreateUnique(digest_algorithm, - *certificate->identity()); + std::unique_ptr good_fingerprint( + rtc::SSLFingerprint::Create(digest_algorithm, certificate->identity())); ASSERT_NE(nullptr, good_fingerprint); EXPECT_TRUE(jsep_transport_ diff --git a/pc/jseptransportcontroller_unittest.cc b/pc/jseptransportcontroller_unittest.cc index d36b8f3ce1..26813e1c4f 100644 --- a/pc/jseptransportcontroller_unittest.cc +++ b/pc/jseptransportcontroller_unittest.cc @@ -170,7 +170,7 @@ class JsepTransportControllerTest : public JsepTransportController::Observer, rtc::scoped_refptr cert) { std::unique_ptr fingerprint; if (cert) { - fingerprint = rtc::SSLFingerprint::CreateFromCertificate(*cert); + fingerprint.reset(rtc::SSLFingerprint::CreateFromCertificate(cert)); } cricket::TransportDescription transport_desc(std::vector(), diff --git a/pc/peerconnection_crypto_unittest.cc b/pc/peerconnection_crypto_unittest.cc index 1a53d860bf..71e87636e2 100644 --- a/pc/peerconnection_crypto_unittest.cc +++ b/pc/peerconnection_crypto_unittest.cc @@ -703,8 +703,8 @@ TEST_P(PeerConnectionCryptoTest, SessionErrorIfFingerprintInvalid) { invalid_answer->description()->GetTransportInfoByName( audio_content->name); ASSERT_TRUE(audio_transport_info); - audio_transport_info->description.identity_fingerprint = - rtc::SSLFingerprint::CreateFromCertificate(*other_certificate); + audio_transport_info->description.identity_fingerprint.reset( + rtc::SSLFingerprint::CreateFromCertificate(other_certificate)); // Set the invalid answer and expect a fingerprint error. std::string error; diff --git a/pc/rtcstatscollector_unittest.cc b/pc/rtcstatscollector_unittest.cc index 1a03e9899b..e565e4610f 100644 --- a/pc/rtcstatscollector_unittest.cc +++ b/pc/rtcstatscollector_unittest.cc @@ -146,7 +146,7 @@ std::unique_ptr CreateFakeCertificateAndInfoFromDers( const rtc::SSLCertChain& chain = info->certificate->ssl_cert_chain(); std::unique_ptr fp; for (size_t i = 0; i < chain.GetSize(); i++) { - fp = rtc::SSLFingerprint::Create("sha-1", chain.Get(i)); + fp.reset(rtc::SSLFingerprint::Create("sha-1", &chain.Get(i))); EXPECT_TRUE(fp); info->fingerprints.push_back(fp->GetRfc4572Fingerprint()); } diff --git a/pc/webrtcsdp.cc b/pc/webrtcsdp.cc index cf6b10dcd7..9995d5bb94 100644 --- a/pc/webrtcsdp.cc +++ b/pc/webrtcsdp.cc @@ -20,7 +20,6 @@ #include #include #include -#include #include #include "api/mediatypes.h" @@ -336,10 +335,9 @@ static bool ParseIceOptions(const std::string& line, static bool ParseExtmap(const std::string& line, RtpExtension* extmap, SdpParseError* error); -static bool ParseFingerprintAttribute( - const std::string& line, - std::unique_ptr* fingerprint, - SdpParseError* error); +static bool ParseFingerprintAttribute(const std::string& line, + rtc::SSLFingerprint** fingerprint, + SdpParseError* error); static bool ParseDtlsSetup(const std::string& line, cricket::ConnectionRole* role, SdpParseError* error); @@ -2136,11 +2134,11 @@ bool ParseSessionDescription(const std::string& message, "Can't have multiple fingerprint attributes at the same level.", error); } - std::unique_ptr fingerprint; + rtc::SSLFingerprint* fingerprint = NULL; if (!ParseFingerprintAttribute(line, &fingerprint, error)) { return false; } - session_td->identity_fingerprint = std::move(fingerprint); + session_td->identity_fingerprint.reset(fingerprint); } else if (HasAttribute(line, kAttributeSetup)) { if (!ParseDtlsSetup(line, &(session_td->connection_role), error)) { return false; @@ -2187,10 +2185,9 @@ bool ParseGroupAttribute(const std::string& line, return true; } -static bool ParseFingerprintAttribute( - const std::string& line, - std::unique_ptr* fingerprint, - SdpParseError* error) { +static bool ParseFingerprintAttribute(const std::string& line, + rtc::SSLFingerprint** fingerprint, + SdpParseError* error) { if (!IsLineType(line, kLineTypeAttributes) || !HasAttribute(line, kAttributeFingerprint)) { return ParseFailedExpectLine(line, 0, kLineTypeAttributes, @@ -2216,8 +2213,7 @@ static bool ParseFingerprintAttribute( ::tolower); // The second field is the digest value. De-hexify it. - *fingerprint = - rtc::SSLFingerprint::CreateUniqueFromRfc4572(algorithm, fields[1]); + *fingerprint = rtc::SSLFingerprint::CreateFromRfc4572(algorithm, fields[1]); if (!*fingerprint) { return ParseFailed(line, "Failed to create fingerprint from the digest.", error); @@ -2857,11 +2853,12 @@ bool ParseContent(const std::string& message, return false; } } else if (HasAttribute(line, kAttributeFingerprint)) { - std::unique_ptr fingerprint; + rtc::SSLFingerprint* fingerprint = NULL; + if (!ParseFingerprintAttribute(line, &fingerprint, error)) { return false; } - transport->identity_fingerprint = std::move(fingerprint); + transport->identity_fingerprint.reset(fingerprint); } else if (HasAttribute(line, kAttributeSetup)) { if (!ParseDtlsSetup(line, &(transport->connection_role), error)) { return false; diff --git a/pc/webrtcsdp_unittest.cc b/pc/webrtcsdp_unittest.cc index 0ef15c8706..8eeeeabb77 100644 --- a/pc/webrtcsdp_unittest.cc +++ b/pc/webrtcsdp_unittest.cc @@ -1503,7 +1503,8 @@ class WebRtcSdpTest : public testing::Test { void AddFingerprint() { desc_.RemoveTransportInfoByName(kAudioContentName); desc_.RemoveTransportInfoByName(kVideoContentName); - rtc::SSLFingerprint fingerprint(rtc::DIGEST_SHA_1, kIdentityDigest); + rtc::SSLFingerprint fingerprint(rtc::DIGEST_SHA_1, kIdentityDigest, + sizeof(kIdentityDigest)); EXPECT_TRUE(desc_.AddTransportInfo(TransportInfo( kAudioContentName, TransportDescription(std::vector(), kUfragVoice, kPwdVoice, diff --git a/rtc_base/sslcertificate.cc b/rtc_base/sslcertificate.cc index d735ebf950..142543fe51 100644 --- a/rtc_base/sslcertificate.cc +++ b/rtc_base/sslcertificate.cc @@ -54,8 +54,8 @@ std::unique_ptr SSLCertificate::GetStats() const { // |SSLCertificate::GetSignatureDigestAlgorithm| is not supported by the // implementation of |SSLCertificate::ComputeDigest|. This currently happens // with MD5- and SHA-224-signed certificates when linked to libNSS. - std::unique_ptr ssl_fingerprint = - SSLFingerprint::Create(digest_algorithm, *this); + std::unique_ptr ssl_fingerprint( + SSLFingerprint::Create(digest_algorithm, this)); if (!ssl_fingerprint) return nullptr; std::string fingerprint = ssl_fingerprint->GetRfc4572Fingerprint(); diff --git a/rtc_base/sslfingerprint.cc b/rtc_base/sslfingerprint.cc index c62f70365f..4f1ae8f7a5 100644 --- a/rtc_base/sslfingerprint.cc +++ b/rtc_base/sslfingerprint.cc @@ -20,67 +20,57 @@ namespace rtc { SSLFingerprint* SSLFingerprint::Create(const std::string& algorithm, - const rtc::SSLIdentity& identity) { - return CreateUnique(algorithm, identity).release(); + const rtc::SSLIdentity* identity) { + if (!identity) { + return nullptr; + } + + return Create(algorithm, &(identity->certificate())); } -std::unique_ptr SSLFingerprint::CreateUnique( - const std::string& algorithm, - const rtc::SSLIdentity& identity) { - return Create(algorithm, identity.certificate()); -} - -std::unique_ptr SSLFingerprint::Create( - const std::string& algorithm, - const rtc::SSLCertificate& cert) { +SSLFingerprint* SSLFingerprint::Create(const std::string& algorithm, + const rtc::SSLCertificate* cert) { uint8_t digest_val[64]; size_t digest_len; - bool ret = cert.ComputeDigest(algorithm, digest_val, sizeof(digest_val), - &digest_len); + bool ret = cert->ComputeDigest(algorithm, digest_val, sizeof(digest_val), + &digest_len); if (!ret) { return nullptr; } - return absl::make_unique( - algorithm, ArrayView(digest_val, digest_len)); + + return new SSLFingerprint(algorithm, digest_val, digest_len); } SSLFingerprint* SSLFingerprint::CreateFromRfc4572( const std::string& algorithm, const std::string& fingerprint) { - return CreateUniqueFromRfc4572(algorithm, fingerprint).release(); -} - -std::unique_ptr SSLFingerprint::CreateUniqueFromRfc4572( - const std::string& algorithm, - const std::string& fingerprint) { if (algorithm.empty() || !rtc::IsFips180DigestAlgorithm(algorithm)) return nullptr; if (fingerprint.empty()) return nullptr; + size_t value_len; char value[rtc::MessageDigest::kMaxSize]; - size_t value_len = rtc::hex_decode_with_delimiter( + value_len = rtc::hex_decode_with_delimiter( value, sizeof(value), fingerprint.c_str(), fingerprint.length(), ':'); if (!value_len) return nullptr; - return absl::make_unique( - algorithm, - ArrayView(reinterpret_cast(value), value_len)); + return new SSLFingerprint(algorithm, reinterpret_cast(value), + value_len); } -std::unique_ptr SSLFingerprint::CreateFromCertificate( - const RTCCertificate& cert) { +SSLFingerprint* SSLFingerprint::CreateFromCertificate( + const RTCCertificate* cert) { std::string digest_alg; - if (!cert.ssl_certificate().GetSignatureDigestAlgorithm(&digest_alg)) { + if (!cert->ssl_certificate().GetSignatureDigestAlgorithm(&digest_alg)) { RTC_LOG(LS_ERROR) << "Failed to retrieve the certificate's digest algorithm"; return nullptr; } - std::unique_ptr fingerprint = - CreateUnique(digest_alg, *cert.identity()); + SSLFingerprint* fingerprint = Create(digest_alg, cert->identity()); if (!fingerprint) { RTC_LOG(LS_ERROR) << "Failed to create identity fingerprint, alg=" << digest_alg; @@ -88,14 +78,12 @@ std::unique_ptr SSLFingerprint::CreateFromCertificate( return fingerprint; } -SSLFingerprint::SSLFingerprint(const std::string& algorithm, - ArrayView digest_view) - : algorithm(algorithm), digest(digest_view.data(), digest_view.size()) {} - SSLFingerprint::SSLFingerprint(const std::string& algorithm, const uint8_t* digest_in, size_t digest_len) - : SSLFingerprint(algorithm, MakeArrayView(digest_in, digest_len)) {} + : algorithm(algorithm) { + digest.SetData(digest_in, digest_len); +} SSLFingerprint::SSLFingerprint(const SSLFingerprint& from) : algorithm(from.algorithm), digest(from.digest) {} diff --git a/rtc_base/sslfingerprint.h b/rtc_base/sslfingerprint.h index a2d324c187..b204bc77f3 100644 --- a/rtc_base/sslfingerprint.h +++ b/rtc_base/sslfingerprint.h @@ -22,34 +22,19 @@ namespace rtc { class SSLCertificate; struct SSLFingerprint { - // TODO(steveanton): Remove once downstream projects have moved off of this. static SSLFingerprint* Create(const std::string& algorithm, - const rtc::SSLIdentity& identity); - // TODO(steveanton): Rename to Create once projects have migrated. - static std::unique_ptr CreateUnique( - const std::string& algorithm, - const rtc::SSLIdentity& identity); + const rtc::SSLIdentity* identity); - static std::unique_ptr Create( - const std::string& algorithm, - const rtc::SSLCertificate& cert); + static SSLFingerprint* Create(const std::string& algorithm, + const rtc::SSLCertificate* cert); - // TODO(steveanton): Remove once downstream projects have moved off of this. static SSLFingerprint* CreateFromRfc4572(const std::string& algorithm, const std::string& fingerprint); - // TODO(steveanton): Rename to CreateFromRfc4572 once projects have migrated. - static std::unique_ptr CreateUniqueFromRfc4572( - const std::string& algorithm, - const std::string& fingerprint); // Creates a fingerprint from a certificate, using the same digest algorithm // as the certificate's signature. - static std::unique_ptr CreateFromCertificate( - const RTCCertificate& cert); + static SSLFingerprint* CreateFromCertificate(const RTCCertificate* cert); - SSLFingerprint(const std::string& algorithm, - ArrayView digest_view); - // TODO(steveanton): Remove once downstream projects have moved off of this. SSLFingerprint(const std::string& algorithm, const uint8_t* digest_in, size_t digest_len); diff --git a/rtc_base/sslidentity_unittest.cc b/rtc_base/sslidentity_unittest.cc index ba53d17da4..9560aaee4e 100644 --- a/rtc_base/sslidentity_unittest.cc +++ b/rtc_base/sslidentity_unittest.cc @@ -180,7 +180,7 @@ IdentityAndInfo CreateFakeIdentityAndInfoFromDers( const rtc::SSLCertChain& chain = info.identity->cert_chain(); std::unique_ptr fp; for (size_t i = 0; i < chain.GetSize(); i++) { - fp = rtc::SSLFingerprint::Create("sha-1", chain.Get(i)); + fp.reset(rtc::SSLFingerprint::Create("sha-1", &chain.Get(i))); EXPECT_TRUE(fp); info.fingerprints.push_back(fp->GetRfc4572Fingerprint()); }