Bug 1330365 - Use mozilla::TimeStamp instead of NSPR's PRIntervalTime for OCSP timeout code. r?keeler draft
authorCykesiopka <cykesiopka.bmo@gmail.com>
Sat, 14 Jan 2017 13:12:43 +0800
changeset 460970 ef2ab4072e9fe46d15bc523a330e5c22735affad
parent 460968 6b716be52be01188cd7c5f82538c55f83d9dad27
child 542177 b372ddeca94d790bdee47ffee2717180cfd8effa
push id41532
push usercykesiopka.bmo@gmail.com
push dateSat, 14 Jan 2017 05:13:20 +0000
reviewerskeeler
bugs1330365
milestone53.0a1
Bug 1330365 - Use mozilla::TimeStamp instead of NSPR's PRIntervalTime for OCSP timeout code. r?keeler mozilla::TimeStamp is generally superior to PRIntervalTime, and switching lets us get rid of yet another NSPR dependency. This patch also: 1. Gets rid of code in nsNSSHttpRequestSession::createFcn() that limits the max OCSP timeout. This is a relic from when NSS was used for OCSP requests, and is no longer necessary. 2. Converts all uses of PR_NOT_REACHED() to MFBT asserts while we're nearby. MozReview-Commit-ID: KvgOWWhP8Km
security/certverifier/NSSCertDBTrustDomain.cpp
security/certverifier/OCSPRequestor.cpp
security/certverifier/OCSPRequestor.h
security/manager/ssl/nsNSSCallbacks.cpp
security/manager/ssl/nsNSSCallbacks.h
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -14,16 +14,17 @@
 #include "OCSPVerificationTrustDomain.h"
 #include "PublicKeyPinningService.h"
 #include "cert.h"
 #include "certdb.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/Casting.h"
 #include "mozilla/Move.h"
 #include "mozilla/PodOperations.h"
+#include "mozilla/TimeStamp.h"
 #include "mozilla/Unused.h"
 #include "nsNSSCertificate.h"
 #include "nsServiceManagerUtils.h"
 #include "nss.h"
 #include "pk11pub.h"
 #include "pkix/Result.h"
 #include "pkix/pkix.h"
 #include "pkix/pkixnss.h"
@@ -261,35 +262,35 @@ NSSCertDBTrustDomain::GetCertTrust(EndEn
 
 Result
 NSSCertDBTrustDomain::DigestBuf(Input item, DigestAlgorithm digestAlg,
                                 /*out*/ uint8_t* digestBuf, size_t digestBufLen)
 {
   return DigestBufNSS(item, digestAlg, digestBuf, digestBufLen);
 }
 
-static PRIntervalTime
+static TimeDuration
 OCSPFetchingTypeToTimeoutTime(NSSCertDBTrustDomain::OCSPFetching ocspFetching)
 {
   switch (ocspFetching) {
     case NSSCertDBTrustDomain::FetchOCSPForDVSoftFail:
-      return PR_SecondsToInterval(2);
+      return TimeDuration::FromSeconds(2);
     case NSSCertDBTrustDomain::FetchOCSPForEV:
     case NSSCertDBTrustDomain::FetchOCSPForDVHardFail:
-      return PR_SecondsToInterval(10);
+      return TimeDuration::FromSeconds(10);
     // The rest of these are error cases. Assert in debug builds, but return
     // the default value corresponding to 2 seconds in release builds.
     case NSSCertDBTrustDomain::NeverFetchOCSP:
     case NSSCertDBTrustDomain::LocalOnlyOCSPForEV:
-      PR_NOT_REACHED("we should never see this OCSPFetching type here");
+      MOZ_ASSERT_UNREACHABLE("we should never see this OCSPFetching type here");
       break;
   }
 
-  PR_NOT_REACHED("we're not handling every OCSPFetching type");
-  return PR_SecondsToInterval(2);
+  MOZ_ASSERT_UNREACHABLE("we're not handling every OCSPFetching type");
+  return TimeDuration::FromSeconds(2);
 }
 
 // Copied and modified from CERT_GetOCSPAuthorityInfoAccessLocation and
 // CERT_GetGeneralNameByType. Returns a non-Result::Success result on error,
 // Success with url == nullptr when an OCSP URI was not found, and Success with
 // url != nullptr when an OCSP URI was found. The output url will be owned
 // by the arena.
 static Result
@@ -981,17 +982,17 @@ NSSCertDBTrustDomain::CheckValidityIsAcc
     case ValidityCheckingMode::CheckingOff:
       return Success;
     case ValidityCheckingMode::CheckForEV:
       // The EV Guidelines say the maximum is 27 months, but we use a slightly
       // higher limit here to (hopefully) minimize compatibility breakage.
       maxValidityDuration = DURATION_27_MONTHS_PLUS_SLOP;
       break;
     default:
-      PR_NOT_REACHED("We're not handling every ValidityCheckingMode type");
+      MOZ_ASSERT_UNREACHABLE("We're not handling every ValidityCheckingMode type");
   }
 
   if (validityDuration > maxValidityDuration) {
     return Result::ERROR_VALIDITY_TOO_LONG;
   }
 
   return Success;
 }
--- a/security/certverifier/OCSPRequestor.cpp
+++ b/security/certverifier/OCSPRequestor.cpp
@@ -70,17 +70,17 @@ AppendEscapedBase64Item(const SECItem* e
   base64Request.ReplaceSubstring("=", "%3D");
   path.Append(base64Request);
   return NS_OK;
 }
 
 Result
 DoOCSPRequest(const UniquePLArenaPool& arena, const char* url,
               const OriginAttributes& originAttributes,
-              const SECItem* encodedRequest, PRIntervalTime timeout,
+              const SECItem* encodedRequest, TimeDuration timeout,
               bool useGET,
       /*out*/ SECItem*& encodedResponse)
 {
   MOZ_ASSERT(arena.get());
   MOZ_ASSERT(url);
   MOZ_ASSERT(encodedRequest);
   MOZ_ASSERT(encodedRequest->data);
   if (!arena.get() || !url || !encodedRequest || !encodedRequest->data) {
--- a/security/certverifier/OCSPRequestor.h
+++ b/security/certverifier/OCSPRequestor.h
@@ -3,26 +3,27 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef OCSPRequestor_h
 #define OCSPRequestor_h
 
 #include "CertVerifier.h"
+#include "mozilla/TimeStamp.h"
 #include "secmodt.h"
 
 namespace mozilla {
 class OriginAttributes;
 }
 
 namespace mozilla { namespace psm {
 
 // The memory returned via |encodedResponse| is owned by the given arena.
 Result DoOCSPRequest(const UniquePLArenaPool& arena, const char* url,
                      const OriginAttributes& originAttributes,
-                     const SECItem* encodedRequest, PRIntervalTime timeout,
+                     const SECItem* encodedRequest, TimeDuration timeout,
                      bool useGET,
              /*out*/ SECItem*& encodedResponse);
 
 } } // namespace mozilla::psm
 
 #endif // OCSPRequestor_h
--- a/security/manager/ssl/nsNSSCallbacks.cpp
+++ b/security/manager/ssl/nsNSSCallbacks.cpp
@@ -10,17 +10,16 @@
 #include "ScopedNSSTypes.h"
 #include "SharedCertVerifier.h"
 #include "SharedSSLState.h"
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/Casting.h"
 #include "mozilla/RefPtr.h"
 #include "mozilla/Telemetry.h"
-#include "mozilla/TimeStamp.h"
 #include "mozilla/Unused.h"
 #include "nsContentUtils.h"
 #include "nsICertOverrideService.h"
 #include "nsIHttpChannelInternal.h"
 #include "nsIPrompt.h"
 #include "nsISupportsPriority.h"
 #include "nsITokenDialogs.h"
 #include "nsIUploadChannel.h"
@@ -226,37 +225,30 @@ nsNSSHttpServerSession::createSessionFcn
 }
 
 mozilla::pkix::Result
 nsNSSHttpRequestSession::createFcn(const nsNSSHttpServerSession* session,
                                    const char* http_protocol_variant,
                                    const char* path_and_query_string,
                                    const char* http_request_method,
                                    const OriginAttributes& origin_attributes,
-                                   const PRIntervalTime timeout,
+                                   const TimeDuration timeout,
                            /*out*/ nsNSSHttpRequestSession** pRequest)
 {
   if (!session || !http_protocol_variant || !path_and_query_string ||
       !http_request_method || !pRequest) {
     return Result::FATAL_ERROR_INVALID_ARGS;
   }
 
   nsNSSHttpRequestSession* rs = new nsNSSHttpRequestSession;
   if (!rs) {
     return Result::FATAL_ERROR_NO_MEMORY;
   }
 
-  rs->mTimeoutInterval = timeout;
-
-  // Use a maximum timeout value of 10 seconds because of bug 404059.
-  // FIXME: Use a better approach once 406120 is ready.
-  uint32_t maxBug404059Timeout = PR_TicksPerSecond() * 10;
-  if (timeout > maxBug404059Timeout) {
-    rs->mTimeoutInterval = maxBug404059Timeout;
-  }
+  rs->mTimeout = timeout;
 
   rs->mURL.Assign(http_protocol_variant);
   rs->mURL.AppendLiteral("://");
   rs->mURL.Append(session->mHost);
   rs->mURL.Append(':');
   rs->mURL.AppendInt(session->mPort);
   rs->mURL.Append(path_and_query_string);
 
@@ -417,17 +409,17 @@ nsNSSHttpRequestSession::internal_send_r
     return Result::FATAL_ERROR_LIBRARY_FAILURE;
   }
 
   bool request_canceled = false;
 
   {
     MutexAutoLock locker(waitLock);
 
-    const PRIntervalTime start_time = PR_IntervalNow();
+    const TimeStamp startTime = TimeStamp::NowLoRes();
     PRIntervalTime wait_interval;
 
     bool running_on_main_thread = NS_IsMainThread();
     if (running_on_main_thread)
     {
       // The result of running this on the main thread
       // is a series of small timeouts mixed with spinning the
       // event loop - this is always dangerous as there is so much main
@@ -455,25 +447,23 @@ nsNSSHttpRequestSession::internal_send_r
         // thread manager. Thanks a lot to Christian Biesinger who
         // made me aware of this possibility. (kaie)
 
         MutexAutoUnlock unlock(waitLock);
         NS_ProcessNextEvent(nullptr);
       }
 
       waitCondition.Wait(wait_interval);
-      
+
       if (!waitFlag)
         break;
 
       if (!request_canceled)
       {
-        bool timeout = 
-          (PRIntervalTime)(PR_IntervalNow() - start_time) > mTimeoutInterval;
- 
+        bool timeout = (TimeStamp::NowLoRes() - startTime) > mTimeout;
         if (timeout)
         {
           request_canceled = true;
 
           RefPtr<nsCancelHTTPDownloadEvent> cancelevent(
             new nsCancelHTTPDownloadEvent);
           cancelevent->mListener = mListener;
           rv = NS_DispatchToMainThread(cancelevent);
@@ -546,20 +536,20 @@ nsNSSHttpRequestSession::internal_send_r
       *http_response_content_type = mListener->mHttpResponseContentType.get();
     }
   }
 
   return Success;
 }
 
 nsNSSHttpRequestSession::nsNSSHttpRequestSession()
-: mRefCount(1),
-  mHasPostData(false),
-  mTimeoutInterval(0),
-  mListener(new nsHTTPListener)
+  : mRefCount(1)
+  , mHasPostData(false)
+  , mTimeout(0)
+  , mListener(new nsHTTPListener)
 {
 }
 
 nsNSSHttpRequestSession::~nsNSSHttpRequestSession()
 {
 }
 
 nsHTTPListener::nsHTTPListener()
--- a/security/manager/ssl/nsNSSCallbacks.h
+++ b/security/manager/ssl/nsNSSCallbacks.h
@@ -6,16 +6,17 @@
 
 #ifndef nsNSSCallbacks_h
 #define nsNSSCallbacks_h
 
 #include "mozilla/Attributes.h"
 #include "mozilla/BasePrincipal.h"
 #include "mozilla/CondVar.h"
 #include "mozilla/Mutex.h"
+#include "mozilla/TimeStamp.h"
 #include "nsAutoPtr.h"
 #include "nsCOMPtr.h"
 #include "nsIStreamLoader.h"
 #include "nspr.h"
 #include "nsString.h"
 #include "pk11func.h"
 #include "pkix/pkixtypes.h"
 
@@ -98,17 +99,17 @@ protected:
 public:
   typedef mozilla::pkix::Result Result;
 
   static Result createFcn(const nsNSSHttpServerSession* session,
                           const char* httpProtocolVariant,
                           const char* pathAndQueryString,
                           const char* httpRequestMethod,
                           const OriginAttributes& originAttributes,
-                          const PRIntervalTime timeout,
+                          const mozilla::TimeDuration timeout,
                   /*out*/ nsNSSHttpRequestSession** pRequest);
 
   Result setPostDataFcn(const char* httpData,
                         const uint32_t httpDataLen,
                         const char* httpContentType);
 
   Result trySendAndReceiveFcn(PRPollDesc** pPollDesc,
                               uint16_t* httpResponseCode,
@@ -124,17 +125,17 @@ public:
   nsCString mRequestMethod;
 
   bool mHasPostData;
   nsCString mPostData;
   nsCString mPostContentType;
 
   OriginAttributes mOriginAttributes;
 
-  PRIntervalTime mTimeoutInterval;
+  mozilla::TimeDuration mTimeout;
 
   RefPtr<nsHTTPListener> mListener;
 
 protected:
   nsNSSHttpRequestSession();
   ~nsNSSHttpRequestSession();
 
   Result internal_send_receive_attempt(bool& retryableError,
@@ -158,17 +159,17 @@ public:
     return nsNSSHttpServerSession::createSessionFcn(host, portnum, pSession);
   }
 
   static Result createFcn(const nsNSSHttpServerSession* session,
                           const char* httpProtocolVariant,
                           const char* pathAndQueryString,
                           const char* httpRequestMethod,
                           const OriginAttributes& originAttributes,
-                          const PRIntervalTime timeout,
+                          const mozilla::TimeDuration timeout,
                   /*out*/ nsNSSHttpRequestSession** pRequest)
   {
     return nsNSSHttpRequestSession::createFcn(session, httpProtocolVariant,
                                               pathAndQueryString,
                                               httpRequestMethod, originAttributes,
                                               timeout, pRequest);
   }