admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Refactor: Renaming ssl_cert_chain to GetSSLCertificateChain()

Browse Source 
Underscore methods in the middle of classes is against the chromium style guide
this change is part of a long series of changes to refactor crypto code in
WebRTC to conform to the chromium standard better.

1. ssl_cert() -> GetSSLCertificate()
2. ssl_cert_chain() -> GetSSLCertificateChain()
3. Small tidying up in rtccertificategenerator.cc

Bug: webrtc:9860
Change-Id: I670f76e31d6d4f873034edb72d958b3c227379cb
Reviewed-on: https://webrtc-review.googlesource.com/c/107802
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Commit-Queue: Benjamin Wright <benwright@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25371}
This commit is contained in:
Benjamin Wright 2018-10-25 01:16:26 -07:00 committed by Commit Bot
parent 359d60a594
commit 6c6c9df99d
11 changed files with 48 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
p2p/base/dtlstransport_unittest.cc
View File

@ -513,8 +513,8 @@ TEST_F(DtlsTransportTest, TestCertificatesBeforeConnect) {
// remote certificate, because connection has not yet occurred.
auto certificate1 = client1_.dtls_transport()->GetLocalCertificate();
auto certificate2 = client2_.dtls_transport()->GetLocalCertificate();
ASSERT_NE(certificate1->ssl_certificate().ToPEMString(),
certificate2->ssl_certificate().ToPEMString());
ASSERT_NE(certificate1->GetSSLCertificate().ToPEMString(),
certificate2->GetSSLCertificate().ToPEMString());
ASSERT_FALSE(client1_.dtls_transport()->GetRemoteSSLCertChain());
ASSERT_FALSE(client2_.dtls_transport()->GetRemoteSSLCertChain());
}
@ -527,8 +527,8 @@ TEST_F(DtlsTransportTest, TestCertificatesAfterConnect) {
// After connection, each side has a distinct local certificate.
auto certificate1 = client1_.dtls_transport()->GetLocalCertificate();
auto certificate2 = client2_.dtls_transport()->GetLocalCertificate();
ASSERT_NE(certificate1->ssl_certificate().ToPEMString(),
certificate2->ssl_certificate().ToPEMString());
ASSERT_NE(certificate1->GetSSLCertificate().ToPEMString(),
certificate2->GetSSLCertificate().ToPEMString());
// Each side's remote certificate is the other side's local certificate.
std::unique_ptr<rtc::SSLCertChain> remote_cert1 =
@ -536,13 +536,13 @@ TEST_F(DtlsTransportTest, TestCertificatesAfterConnect) {
ASSERT_TRUE(remote_cert1);
ASSERT_EQ(1u, remote_cert1->GetSize());
ASSERT_EQ(remote_cert1->Get(0).ToPEMString(),
certificate2->ssl_certificate().ToPEMString());
certificate2->GetSSLCertificate().ToPEMString());
std::unique_ptr<rtc::SSLCertChain> remote_cert2 =
client2_.dtls_transport()->GetRemoteSSLCertChain();
ASSERT_TRUE(remote_cert2);
ASSERT_EQ(1u, remote_cert2->GetSize());
ASSERT_EQ(remote_cert2->Get(0).ToPEMString(),
certificate1->ssl_certificate().ToPEMString());
certificate1->GetSSLCertificate().ToPEMString());
}
// Test that packets are retransmitted according to the expected schedule.

6
p2p/base/transportdescriptionfactory_unittest.cc
View File

@ -167,7 +167,7 @@ TEST_F(TransportDescriptionFactoryTest, TestOfferDtls) {
f1_.set_certificate(cert1_);
std::string digest_alg;
ASSERT_TRUE(
cert1_->ssl_certificate().GetSignatureDigestAlgorithm(&digest_alg));
cert1_->GetSSLCertificate().GetSignatureDigestAlgorithm(&digest_alg));
std::unique_ptr<TransportDescription> desc(
f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_));
CheckDesc(desc.get(), "", "", "", digest_alg);
@ -192,7 +192,7 @@ TEST_F(TransportDescriptionFactoryTest, TestOfferDtlsReofferDtls) {
f1_.set_certificate(cert1_);
std::string digest_alg;
ASSERT_TRUE(
cert1_->ssl_certificate().GetSignatureDigestAlgorithm(&digest_alg));
cert1_->GetSSLCertificate().GetSignatureDigestAlgorithm(&digest_alg));
std::unique_ptr<TransportDescription> old_desc(
f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_));
ASSERT_TRUE(old_desc.get() != NULL);
@ -269,7 +269,7 @@ TEST_F(TransportDescriptionFactoryTest, TestAnswerDtlsToDtls) {
// answer must contain fingerprint lines with cert2_'s digest algorithm.
std::string digest_alg2;
ASSERT_TRUE(
cert2_->ssl_certificate().GetSignatureDigestAlgorithm(&digest_alg2));
cert2_->GetSSLCertificate().GetSignatureDigestAlgorithm(&digest_alg2));
std::unique_ptr<TransportDescription> offer(
f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_));

2
pc/jseptransport_unittest.cc
View File

@ -383,7 +383,7 @@ TEST_P(JsepTransport2WithRtcpMux, VerifyCertificateFingerprint) {
ASSERT_NE(nullptr, certificate);
std::string digest_algorithm;
ASSERT_TRUE(certificate->ssl_certificate().GetSignatureDigestAlgorithm(
ASSERT_TRUE(certificate->GetSSLCertificate().GetSignatureDigestAlgorithm(
&digest_algorithm));
ASSERT_FALSE(digest_algorithm.empty());
std::unique_ptr<rtc::SSLFingerprint> good_fingerprint =

8
pc/peerconnection_integrationtest.cc
View File

@ -1821,26 +1821,26 @@ TEST_P(PeerConnectionIntegrationTest,
auto caller_remote_cert = GetRemoteAudioSSLCertificate(caller());
ASSERT_TRUE(caller_remote_cert);
EXPECT_EQ(callee_cert->ssl_certificate().ToPEMString(),
EXPECT_EQ(callee_cert->GetSSLCertificate().ToPEMString(),
caller_remote_cert->ToPEMString());
auto callee_remote_cert = GetRemoteAudioSSLCertificate(callee());
ASSERT_TRUE(callee_remote_cert);
EXPECT_EQ(caller_cert->ssl_certificate().ToPEMString(),
EXPECT_EQ(caller_cert->GetSSLCertificate().ToPEMString(),
callee_remote_cert->ToPEMString());
auto caller_remote_cert_chain = GetRemoteAudioSSLCertChain(caller());
ASSERT_TRUE(caller_remote_cert_chain);
ASSERT_EQ(1U, caller_remote_cert_chain->GetSize());
auto remote_cert = &caller_remote_cert_chain->Get(0);
EXPECT_EQ(callee_cert->ssl_certificate().ToPEMString(),
EXPECT_EQ(callee_cert->GetSSLCertificate().ToPEMString(),
remote_cert->ToPEMString());
auto callee_remote_cert_chain = GetRemoteAudioSSLCertChain(callee());
ASSERT_TRUE(callee_remote_cert_chain);
ASSERT_EQ(1U, callee_remote_cert_chain->GetSize());
remote_cert = &callee_remote_cert_chain->Get(0);
EXPECT_EQ(caller_cert->ssl_certificate().ToPEMString(),
EXPECT_EQ(caller_cert->GetSSLCertificate().ToPEMString(),
remote_cert->ToPEMString());
}

2
pc/rtcstatscollector.cc
View File

@ -1388,7 +1388,7 @@ RTCStatsCollector::PrepareTransportCertificateStats_n(
rtc::scoped_refptr<rtc::RTCCertificate> local_certificate;
if (pc_->GetLocalCertificate(transport_name, &local_certificate)) {
certificate_stats_pair.local =
local_certificate->ssl_cert_chain().GetStats();
local_certificate->GetSSLCertificateChain().GetStats();
}
std::unique_ptr<rtc::SSLCertChain> remote_cert_chain =

15
pc/rtcstatscollector_unittest.cc
View File

@ -143,7 +143,7 @@ std::unique_ptr<CertificateInfo> CreateFakeCertificateAndInfoFromDers(
}
// Fingerprints for the whole certificate chain, starting with leaf
// certificate.
const rtc::SSLCertChain& chain = info->certificate->ssl_cert_chain();
const rtc::SSLCertChain& chain = info->certificate->GetSSLCertificateChain();
std::unique_ptr<rtc::SSLFingerprint> fp;
for (size_t i = 0; i < chain.GetSize(); i++) {
fp = rtc::SSLFingerprint::Create("sha-1", chain.Get(i));
@ -703,7 +703,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCertificateStatsSingle) {
CreateFakeCertificateAndInfoFromDers(
std::vector<std::string>({"(remote) single certificate"}));
pc_->SetRemoteCertChain(
kTransportName, remote_certinfo->certificate->ssl_cert_chain().Clone());
kTransportName,
remote_certinfo->certificate->GetSSLCertificateChain().Clone());
rtc::scoped_refptr<const RTCStatsReport> report = stats_->GetStatsReport();
@ -817,7 +818,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCertificateStatsMultiple) {
std::vector<std::string>({"(remote) audio"}));
pc_->SetRemoteCertChain(
kAudioTransport,
audio_remote_certinfo->certificate->ssl_cert_chain().Clone());
audio_remote_certinfo->certificate->GetSSLCertificateChain().Clone());
pc_->AddVideoChannel("video", kVideoTransport);
std::unique_ptr<CertificateInfo> video_local_certinfo =
@ -829,7 +830,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCertificateStatsMultiple) {
std::vector<std::string>({"(remote) video"}));
pc_->SetRemoteCertChain(
kVideoTransport,
video_remote_certinfo->certificate->ssl_cert_chain().Clone());
video_remote_certinfo->certificate->GetSSLCertificateChain().Clone());
rtc::scoped_refptr<const RTCStatsReport> report = stats_->GetStatsReport();
ExpectReportContainsCertificateInfo(report, *audio_local_certinfo);
@ -853,7 +854,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCertificateStatsChain) {
"(remote) another",
"(remote) chain"});
pc_->SetRemoteCertChain(
kTransportName, remote_certinfo->certificate->ssl_cert_chain().Clone());
kTransportName,
remote_certinfo->certificate->GetSSLCertificateChain().Clone());
rtc::scoped_refptr<const RTCStatsReport> report = stats_->GetStatsReport();
ExpectReportContainsCertificateInfo(report, *local_certinfo);
@ -1954,7 +1956,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCTransportStats) {
CreateFakeCertificateAndInfoFromDers(
{"(remote) local", "(remote) chain"});
pc_->SetRemoteCertChain(
kTransportName, remote_certinfo->certificate->ssl_cert_chain().Clone());
kTransportName,
remote_certinfo->certificate->GetSSLCertificateChain().Clone());
report = stats_->GetFreshStatsReport();

4
pc/statscollector.cc
View File

@ -783,8 +783,8 @@ void StatsCollector::ExtractSessionInfo() {
StatsReport::Id local_cert_report_id, remote_cert_report_id;
rtc::scoped_refptr<rtc::RTCCertificate> certificate;
if (pc_->GetLocalCertificate(transport_name, &certificate)) {
StatsReport* r =
AddCertificateReports(certificate->ssl_cert_chain().GetStats());
StatsReport* r = AddCertificateReports(
certificate->GetSSLCertificateChain().GetStats());
if (r)
local_cert_report_id = r->id();
}

11
rtc_base/rtccertificate.cc
View File

@ -32,7 +32,7 @@ RTCCertificate::RTCCertificate(SSLIdentity* identity) : identity_(identity) {
RTCCertificate::~RTCCertificate() {}
uint64_t RTCCertificate::Expires() const {
int64_t expires = ssl_certificate().CertificateExpirationTime();
int64_t expires = GetSSLCertificate().CertificateExpirationTime();
if (expires != -1)
return static_cast<uint64_t>(expires) * kNumMillisecsPerSec;
// If the expiration time could not be retrieved return an expired timestamp.
@ -43,17 +43,22 @@ bool RTCCertificate::HasExpired(uint64_t now) const {
return Expires() <= now;
}
const SSLCertificate& RTCCertificate::GetSSLCertificate() const {
return identity_->certificate();
}
// Deprecated: TODO(benwright) - Remove once chromium is updated.
const SSLCertificate& RTCCertificate::ssl_certificate() const {
return identity_->certificate();
}
const SSLCertChain& RTCCertificate::ssl_cert_chain() const {
const SSLCertChain& RTCCertificate::GetSSLCertificateChain() const {
return identity_->cert_chain();
}
RTCCertificatePEM RTCCertificate::ToPEM() const {
return RTCCertificatePEM(identity_->PrivateKeyToPEMString(),
ssl_certificate().ToPEMString());
GetSSLCertificate().ToPEMString());
}
scoped_refptr<RTCCertificate> RTCCertificate::FromPEM(

10
rtc_base/rtccertificate.h
View File

@ -58,11 +58,15 @@ class RTCCertificate : public RefCountInterface {
// Checks if the certificate has expired, where |now| is expressed in ms
// relative to epoch, 1970-01-01T00:00:00Z.
bool HasExpired(uint64_t now) const;
const SSLCertificate& GetSSLCertificate() const;
const SSLCertChain& GetSSLCertificateChain() const;
// Deprecated: TODO(benwright) - Remove once chromium is updated.
const SSLCertificate& ssl_certificate() const;
const SSLCertChain& ssl_cert_chain() const;
// TODO(hbos): If possible, remove once RTCCertificate and its
// ssl_certificate() is used in all relevant places. Should not pass around
// GetSSLCertificate() is used in all relevant places. Should not pass around
// raw SSLIdentity* for the sake of accessing SSLIdentity::certificate().
// However, some places might need SSLIdentity* for its public/private key...
SSLIdentity* identity() const { return identity_.get(); }
@ -80,7 +84,7 @@ class RTCCertificate : public RefCountInterface {
private:
// The SSLIdentity is the owner of the SSLCertificate. To protect our
// ssl_certificate() we take ownership of |identity_|.
// GetSSLCertificate() we take ownership of |identity_|.
std::unique_ptr<SSLIdentity> identity_;
};

14
rtc_base/rtccertificategenerator.cc
View File

@ -28,7 +28,6 @@ namespace {
// A certificates' subject and issuer name.
const char kIdentityName[] = "WebRTC";
const uint64_t kYearInSeconds = 365 * 24 * 60 * 60;
enum {
@ -65,11 +64,9 @@ class RTCCertificateGenerationTask : public RefCountInterface,
switch (msg->message_id) {
case MSG_GENERATE:
RTC_DCHECK(worker_thread_->IsCurrent());
// Perform the certificate generation work here on the worker thread.
certificate_ = RTCCertificateGenerator::GenerateCertificate(
key_params_, expires_ms_);
// Handle callbacks on signaling thread. Pass on the |msg->pdata|
// (which references |this| with ref counting) to that thread.
signaling_thread_->Post(RTC_FROM_HERE, this, MSG_GENERATE_DONE,
@ -77,14 +74,12 @@ class RTCCertificateGenerationTask : public RefCountInterface,
break;
case MSG_GENERATE_DONE:
RTC_DCHECK(signaling_thread_->IsCurrent());
// Perform callback with result here on the signaling thread.
if (certificate_) {
callback_->OnSuccess(certificate_);
} else {
callback_->OnFailure();
}
// Destroy |msg->pdata| which references |this| with ref counting. This
// may result in |this| being deleted - do not touch member variables
// after this line.
@ -110,9 +105,11 @@ class RTCCertificateGenerationTask : public RefCountInterface,
scoped_refptr<RTCCertificate> RTCCertificateGenerator::GenerateCertificate(
const KeyParams& key_params,
const absl::optional<uint64_t>& expires_ms) {
if (!key_params.IsValid())
if (!key_params.IsValid()) {
return nullptr;
SSLIdentity* identity;
}
SSLIdentity* identity = nullptr;
if (!expires_ms) {
identity = SSLIdentity::Generate(kIdentityName, key_params);
} else {
@ -129,8 +126,9 @@ scoped_refptr<RTCCertificate> RTCCertificateGenerator::GenerateCertificate(
identity = SSLIdentity::GenerateWithExpiration(kIdentityName, key_params,
cert_lifetime_s);
}
if (!identity)
if (!identity) {
return nullptr;
}
std::unique_ptr<SSLIdentity> identity_sptr(identity);
return RTCCertificate::Create(std::move(identity_sptr));
}

2
rtc_base/sslfingerprint.cc
View File

@ -79,7 +79,7 @@ std::unique_ptr<SSLFingerprint> SSLFingerprint::CreateUniqueFromRfc4572(
std::unique_ptr<SSLFingerprint> SSLFingerprint::CreateFromCertificate(
const RTCCertificate& cert) {
std::string digest_alg;
if (!cert.ssl_certificate().GetSignatureDigestAlgorithm(&digest_alg)) {
if (!cert.GetSSLCertificate().GetSignatureDigestAlgorithm(&digest_alg)) {
RTC_LOG(LS_ERROR)
<< "Failed to retrieve the certificate's digest algorithm";
return nullptr;