Bug 1368107 - Remove TransportSecurityInfo::GetHostNameRaw(). r=keeler
authorCykesiopka <cykesiopka.bmo@gmail.com>
Sat, 03 Jun 2017 13:35:51 +0800
changeset 410343 c263f45e41cea8b6014db61d0584bd123604d695
parent 410342 d62f6dc68c5fadc179ff87c8dfb4200de8c456c7
child 410344 8d8bd769125e1eb326ed71c72b19406cfa74bc5c
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1368107
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1368107 - Remove TransportSecurityInfo::GetHostNameRaw(). r=keeler GetHostNameRaw() returns a char* string, which is less safe and ergonomic compared to the Mozilla string classes. GetHostName() can be used instead. MozReview-Commit-ID: GYvTnISNN35
security/certverifier/CertVerifier.cpp
security/certverifier/CertVerifier.h
security/manager/ssl/SSLServerCertVerification.cpp
security/manager/ssl/TransportSecurityInfo.h
security/manager/ssl/nsNSSCallbacks.cpp
security/manager/ssl/nsNSSCertificateDB.cpp
security/manager/ssl/nsNSSIOLayer.cpp
security/manager/ssl/nsSiteSecurityService.cpp
--- a/security/certverifier/CertVerifier.cpp
+++ b/security/certverifier/CertVerifier.cpp
@@ -15,16 +15,17 @@
 #include "MultiLogCTVerifier.h"
 #include "NSSCertDBTrustDomain.h"
 #include "NSSErrorsService.h"
 #include "cert.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/Casting.h"
 #include "mozilla/IntegerPrintfMacros.h"
 #include "nsNSSComponent.h"
+#include "nsPromiseFlatString.h"
 #include "nsServiceManagerUtils.h"
 #include "pk11pub.h"
 #include "pkix/pkix.h"
 #include "pkix/pkixnss.h"
 #include "secmod.h"
 
 using namespace mozilla::ct;
 using namespace mozilla::pkix;
@@ -937,46 +938,46 @@ CertVerifier::VerifyCert(CERTCertificate
 }
 
 Result
 CertVerifier::VerifySSLServerCert(const UniqueCERTCertificate& peerCert,
                      /*optional*/ const SECItem* stapledOCSPResponse,
                      /*optional*/ const SECItem* sctsFromTLS,
                                   Time time,
                      /*optional*/ void* pinarg,
-                                  const char* hostname,
+                                  const nsACString& hostname,
                           /*out*/ UniqueCERTCertList& builtChain,
                      /*optional*/ UniqueCERTCertList* peerCertChain,
                      /*optional*/ bool saveIntermediatesInPermanentDatabase,
                      /*optional*/ Flags flags,
                      /*optional*/ const OriginAttributes& originAttributes,
                  /*optional out*/ SECOidTag* evOidPolicy,
                  /*optional out*/ OCSPStaplingStatus* ocspStaplingStatus,
                  /*optional out*/ KeySizeStatus* keySizeStatus,
                  /*optional out*/ SHA1ModeResult* sha1ModeResult,
                  /*optional out*/ PinningTelemetryInfo* pinningTelemetryInfo,
                  /*optional out*/ CertificateTransparencyInfo* ctInfo)
 {
   MOZ_ASSERT(peerCert);
   // XXX: MOZ_ASSERT(pinarg);
-  MOZ_ASSERT(hostname);
-  MOZ_ASSERT(hostname[0]);
+  MOZ_ASSERT(!hostname.IsEmpty());
 
   if (evOidPolicy) {
     *evOidPolicy = SEC_OID_UNKNOWN;
   }
 
-  if (!hostname || !hostname[0]) {
+  if (hostname.IsEmpty()) {
     return Result::ERROR_BAD_CERT_DOMAIN;
   }
 
   // CreateCertErrorRunnable assumes that CheckCertHostname is only called
   // if VerifyCert succeeded.
   Result rv = VerifyCert(peerCert.get(), certificateUsageSSLServer, time,
-                         pinarg, hostname, builtChain, peerCertChain, flags,
+                         pinarg, PromiseFlatCString(hostname).get(), builtChain,
+                         peerCertChain, flags,
                          stapledOCSPResponse, sctsFromTLS, originAttributes,
                          evOidPolicy, ocspStaplingStatus, keySizeStatus,
                          sha1ModeResult, pinningTelemetryInfo, ctInfo);
   if (rv != Success) {
     return rv;
   }
 
   Input peerCertInput;
@@ -1000,18 +1001,19 @@ CertVerifier::VerifySSLServerCert(const 
   if (!(flags & FLAG_TLS_IGNORE_STATUS_REQUEST)) {
     rv = CheckTLSFeaturesAreSatisfied(peerCertInput, responseInputPtr);
     if (rv != Success) {
       return rv;
     }
   }
 
   Input hostnameInput;
-  rv = hostnameInput.Init(BitwiseCast<const uint8_t*, const char*>(hostname),
-                          strlen(hostname));
+  rv = hostnameInput.Init(
+    BitwiseCast<const uint8_t*, const char*>(hostname.BeginReading()),
+    hostname.Length());
   if (rv != Success) {
     return Result::FATAL_ERROR_INVALID_ARGS;
   }
   bool isBuiltInRoot;
   rv = IsCertChainRootBuiltInRoot(builtChain, isBuiltInRoot);
   if (rv != Success) {
     return rv;
   }
--- a/security/certverifier/CertVerifier.h
+++ b/security/certverifier/CertVerifier.h
@@ -10,16 +10,17 @@
 #include "BRNameMatchingPolicy.h"
 #include "CTPolicyEnforcer.h"
 #include "CTVerifyResult.h"
 #include "OCSPCache.h"
 #include "ScopedNSSTypes.h"
 #include "mozilla/Telemetry.h"
 #include "mozilla/TimeStamp.h"
 #include "mozilla/UniquePtr.h"
+#include "nsString.h"
 #include "pkix/pkixtypes.h"
 
 #if defined(_MSC_VER)
 #pragma warning(push)
 // Silence "RootingAPI.h(718): warning C4324: 'js::DispatchWrapper<T>':
 // structure was padded due to alignment specifier with [ T=void * ]"
 #pragma warning(disable:4324)
 #endif /* defined(_MSC_VER) */
@@ -143,17 +144,17 @@ public:
    /*optional out*/ CertificateTransparencyInfo* ctInfo = nullptr);
 
   mozilla::pkix::Result VerifySSLServerCert(
                     const UniqueCERTCertificate& peerCert,
        /*optional*/ const SECItem* stapledOCSPResponse,
        /*optional*/ const SECItem* sctsFromTLS,
                     mozilla::pkix::Time time,
        /*optional*/ void* pinarg,
-                    const char* hostname,
+                    const nsACString& hostname,
             /*out*/ UniqueCERTCertList& builtChain,
        /*optional*/ UniqueCERTCertList* peerCertChain = nullptr,
        /*optional*/ bool saveIntermediatesInPermanentDatabase = false,
        /*optional*/ Flags flags = 0,
        /*optional*/ const OriginAttributes& originAttributes =
                       OriginAttributes(),
    /*optional out*/ SECOidTag* evOidPolicy = nullptr,
    /*optional out*/ OCSPStaplingStatus* ocspStaplingStatus = nullptr,
--- a/security/manager/ssl/SSLServerCertVerification.cpp
+++ b/security/manager/ssl/SSLServerCertVerification.cpp
@@ -123,16 +123,17 @@
 #include "nsISocketProvider.h"
 #include "nsIThreadPool.h"
 #include "nsNSSCertificate.h"
 #include "nsNSSComponent.h"
 #include "nsNSSIOLayer.h"
 #include "nsNSSShutDown.h"
 #include "nsSSLStatus.h"
 #include "nsServiceManagerUtils.h"
+#include "nsString.h"
 #include "nsURLHelper.h"
 #include "nsXPCOMCIDInternal.h"
 #include "pkix/pkix.h"
 #include "pkix/pkixnss.h"
 #include "secerr.h"
 #include "secoidt.h"
 #include "secport.h"
 #include "ssl.h"
@@ -333,25 +334,24 @@ MapCertErrorToProbeValue(PRErrorCode err
     probeValue ^= FATAL_ERROR_FLAG;
     probeValue += 90;
   }
   return probeValue;
 }
 
 SECStatus
 DetermineCertOverrideErrors(const UniqueCERTCertificate& cert,
-                            const char* hostName,
+                            const nsACString& hostName,
                             PRTime now, PRErrorCode defaultErrorCodeToReport,
                             /*out*/ uint32_t& collectedErrors,
                             /*out*/ PRErrorCode& errorCodeTrust,
                             /*out*/ PRErrorCode& errorCodeMismatch,
                             /*out*/ PRErrorCode& errorCodeTime)
 {
   MOZ_ASSERT(cert);
-  MOZ_ASSERT(hostName);
   MOZ_ASSERT(collectedErrors == 0);
   MOZ_ASSERT(errorCodeTrust == 0);
   MOZ_ASSERT(errorCodeMismatch == 0);
   MOZ_ASSERT(errorCodeTime == 0);
 
   // Assumes the error prioritization described in mozilla::pkix's
   // BuildForward function. Also assumes that CheckCertHostname was only
   // called if CertVerifier::VerifyCert succeeded.
@@ -414,18 +414,18 @@ DetermineCertOverrideErrors(const Unique
   if (defaultErrorCodeToReport != SSL_ERROR_BAD_CERT_DOMAIN) {
     Input certInput;
     if (certInput.Init(cert->derCert.data, cert->derCert.len) != Success) {
       PR_SetError(SEC_ERROR_BAD_DER, 0);
       return SECFailure;
     }
     Input hostnameInput;
     Result result = hostnameInput.Init(
-      BitwiseCast<const uint8_t*, const char*>(hostName),
-      strlen(hostName));
+      BitwiseCast<const uint8_t*, const char*>(hostName.BeginReading()),
+      hostName.Length());
     if (result != Success) {
       PR_SetError(SEC_ERROR_INVALID_ARGS, 0);
       return SECFailure;
     }
     // Use a lax policy so as to not generate potentially spurious name
     // mismatch "hints".
     BRNameMatchingPolicy nameMatchingPolicy(
       BRNameMatchingPolicy::Mode::DoNotEnforce);
@@ -677,17 +677,17 @@ CreateCertErrorRunnable(CertVerifier& ce
 
   uint32_t probeValue = MapCertErrorToProbeValue(defaultErrorCodeToReport);
   Telemetry::Accumulate(Telemetry::SSL_CERT_VERIFICATION_ERRORS, probeValue);
 
   uint32_t collected_errors = 0;
   PRErrorCode errorCodeTrust = 0;
   PRErrorCode errorCodeMismatch = 0;
   PRErrorCode errorCodeTime = 0;
-  if (DetermineCertOverrideErrors(cert, infoObject->GetHostNameRaw(), now,
+  if (DetermineCertOverrideErrors(cert, infoObject->GetHostName(), now,
                                   defaultErrorCodeToReport, collected_errors,
                                   errorCodeTrust, errorCodeMismatch,
                                   errorCodeTime) != SECSuccess) {
     // Attempt to enforce that if DetermineCertOverrideErrors failed,
     // PR_SetError was set with a non-overridable error. This is because if we
     // return from CreateCertErrorRunnable without calling
     // infoObject->SetStatusErrorBits, we won't have the required information
     // to actually add a certificate error override. This results in a broken
@@ -1398,17 +1398,17 @@ AuthCertificate(CertVerifier& certVerifi
   if (!infoObject->SharedState().IsOCSPStaplingEnabled() ||
       !infoObject->SharedState().IsOCSPMustStapleEnabled()) {
     flags |= CertVerifier::FLAG_TLS_IGNORE_STATUS_REQUEST;
   }
 
   Result rv = certVerifier.VerifySSLServerCert(cert, stapledOCSPResponse,
                                                sctsFromTLSExtension, time,
                                                infoObject,
-                                               infoObject->GetHostNameRaw(),
+                                               infoObject->GetHostName(),
                                                certList, &peerCertChain,
                                                saveIntermediates, flags,
                                                infoObject->
                                                       GetOriginAttributes(),
                                                &evOidPolicy,
                                                &ocspStaplingStatus,
                                                &keySizeStatus, &sha1ModeResult,
                                                &pinningTelemetryInfo,
--- a/security/manager/ssl/TransportSecurityInfo.h
+++ b/security/manager/ssl/TransportSecurityInfo.h
@@ -38,47 +38,46 @@ class TransportSecurityInfo : public nsI
                               public nsIClassInfo,
                               public nsNSSShutDownObject,
                               public nsOnPK11LogoutCancelObject
 {
 protected:
   virtual ~TransportSecurityInfo();
 public:
   TransportSecurityInfo();
-  
+
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSITRANSPORTSECURITYINFO
   NS_DECL_NSIINTERFACEREQUESTOR
   NS_DECL_NSISSLSTATUSPROVIDER
   NS_DECL_NSIASSOCIATEDCONTENTSECURITY
   NS_DECL_NSISERIALIZABLE
   NS_DECL_NSICLASSINFO
 
   nsresult SetSecurityState(uint32_t aState);
 
   const nsACString & GetHostName() const { return mHostName; }
-  const char * GetHostNameRaw() const { return mHostName.get(); }
 
   void SetHostName(const char* host);
 
   int32_t GetPort() const { return mPort; }
   nsresult GetPort(int32_t *aPort);
   nsresult SetPort(int32_t aPort);
 
   const OriginAttributes& GetOriginAttributes() const {
     return mOriginAttributes;
   }
   nsresult SetOriginAttributes(const OriginAttributes& aOriginAttributes);
 
   PRErrorCode GetErrorCode() const;
-  
+
   void GetErrorLogMessage(PRErrorCode errorCode,
                           ::mozilla::psm::SSLErrorMessageType errorMessageType,
                           nsString &result);
-  
+
   void SetCanceled(PRErrorCode errorCode,
                    ::mozilla::psm::SSLErrorMessageType errorMessageType);
 
   /* Set SSL Status values */
   nsresult SetSSLStatus(nsSSLStatus *aSSLStatus);
   nsSSLStatus* SSLStatus() { return mSSLStatus; }
   void SetStatusErrorBits(nsNSSCertificate* cert, uint32_t collected_errors);
 
--- a/security/manager/ssl/nsNSSCallbacks.cpp
+++ b/security/manager/ssl/nsNSSCallbacks.cpp
@@ -1127,17 +1127,17 @@ DetermineEVAndCTStatusAndSetNewCert(RefP
   UniqueCERTCertList unusedBuiltChain;
   const bool saveIntermediates = false;
   mozilla::pkix::Result rv = certVerifier->VerifySSLServerCert(
     cert,
     stapledOCSPResponse,
     sctsFromTLSExtension,
     mozilla::pkix::Now(),
     infoObject,
-    infoObject->GetHostNameRaw(),
+    infoObject->GetHostName(),
     unusedBuiltChain,
     &peerCertChain,
     saveIntermediates,
     flags,
     infoObject->GetOriginAttributes(),
     &evOidPolicy,
     nullptr, // OCSP stapling telemetry
     nullptr, // key size telemetry
--- a/security/manager/ssl/nsNSSCertificateDB.cpp
+++ b/security/manager/ssl/nsNSSCertificateDB.cpp
@@ -1395,31 +1395,31 @@ VerifyCertAtTime(nsIX509Cert* aCert,
 
   RefPtr<SharedCertVerifier> certVerifier(GetDefaultCertVerifier());
   NS_ENSURE_TRUE(certVerifier, NS_ERROR_FAILURE);
 
   UniqueCERTCertList resultChain;
   SECOidTag evOidPolicy;
   mozilla::pkix::Result result;
 
-  const nsCString& flatHostname = PromiseFlatCString(aHostname);
   if (!aHostname.IsVoid() && aUsage == certificateUsageSSLServer) {
     result = certVerifier->VerifySSLServerCert(nssCert,
                                                nullptr, // stapledOCSPResponse
                                                nullptr, // sctsFromTLSExtension
                                                aTime,
                                                nullptr, // Assume no context
-                                               flatHostname.get(),
+                                               aHostname,
                                                resultChain,
                                                nullptr, // no peerCertChain
                                                false, // don't save intermediates
                                                aFlags,
                                                OriginAttributes(),
                                                &evOidPolicy);
   } else {
+    const nsCString& flatHostname = PromiseFlatCString(aHostname);
     result = certVerifier->VerifyCert(nssCert.get(), aUsage, aTime,
                                       nullptr, // Assume no context
                                       aHostname.IsVoid() ? nullptr
                                                          : flatHostname.get(),
                                       resultChain,
                                       nullptr, // no peerCertChain
                                       aFlags,
                                       nullptr, // stapledOCSPResponse
--- a/security/manager/ssl/nsNSSIOLayer.cpp
+++ b/security/manager/ssl/nsNSSIOLayer.cpp
@@ -445,26 +445,25 @@ nsNSSSocketInfo::IsAcceptableForHost(con
   // actually be the wrong chain. We should consider having JoinConnection
   // return the certificate chain built here, so that the calling Necko code
   // can associate the correct certificate chain with the HTTP transactions it
   // is trying to join onto this connection.
   RefPtr<SharedCertVerifier> certVerifier(GetDefaultCertVerifier());
   if (!certVerifier) {
     return NS_OK;
   }
-  nsAutoCString hostnameFlat(PromiseFlatCString(hostname));
   CertVerifier::Flags flags = CertVerifier::FLAG_LOCAL_ONLY;
   UniqueCERTCertList unusedBuiltChain;
   mozilla::pkix::Result result =
     certVerifier->VerifySSLServerCert(nssCert,
                                       nullptr, // stapledOCSPResponse
                                       nullptr, // sctsFromTLSExtension
                                       mozilla::pkix::Now(),
                                       nullptr, // pinarg
-                                      hostnameFlat.get(),
+                                      hostname,
                                       unusedBuiltChain,
                                       nullptr, // no peerCertChain
                                       false, // save intermediates
                                       flags);
   if (result != mozilla::pkix::Success) {
     return NS_OK;
   }
 
--- a/security/manager/ssl/nsSiteSecurityService.cpp
+++ b/security/manager/ssl/nsSiteSecurityService.cpp
@@ -1071,17 +1071,17 @@ nsSiteSecurityService::ProcessPKPHeader(
   // to the site (or it's disabled, in which case we wouldn't want to enforce it
   // anyway).
   CertVerifier::Flags flags = CertVerifier::FLAG_LOCAL_ONLY |
                               CertVerifier::FLAG_TLS_IGNORE_STATUS_REQUEST;
   if (certVerifier->VerifySSLServerCert(nssCert,
                                         nullptr, // stapledOCSPResponse
                                         nullptr, // sctsFromTLSExtension
                                         now, nullptr, // pinarg
-                                        host.get(), // hostname
+                                        host, // hostname
                                         certList,
                                         nullptr, // no peerCertChain
                                         false, // don't store intermediates
                                         flags,
                                         aOriginAttributes)
         != mozilla::pkix::Success) {
     return NS_ERROR_FAILURE;
   }