Bug 959026 - Add telemetry about cases where we don't do OCSP fetching when we expect to. r=keeler, a=bajaj
authorBrian Smith <brian@briansmith.org>
Sun, 12 Jan 2014 19:31:40 -0800
changeset 175757 dffdacbb272c95f3b666f5526866678ebf45be14
parent 175756 47b8a0b8c7043fc71257ca8806cad5264c6b288c
child 175758 e7d4d91519a09d89f31d7897c0621b183df1d0fa
push id445
push userffxbld
push dateMon, 10 Mar 2014 22:05:19 +0000
treeherdermozilla-release@dc38b741b04e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler, bajaj
bugs959026
milestone28.0a2
Bug 959026 - Add telemetry about cases where we don't do OCSP fetching when we expect to. r=keeler, a=bajaj
security/manager/ssl/src/SSLServerCertVerification.cpp
security/manager/ssl/src/SharedSSLState.cpp
security/manager/ssl/src/SharedSSLState.h
security/manager/ssl/src/nsNSSComponent.cpp
toolkit/components/telemetry/Histograms.json
--- a/security/manager/ssl/src/SSLServerCertVerification.cpp
+++ b/security/manager/ssl/src/SSLServerCertVerification.cpp
@@ -90,16 +90,19 @@
  * thread since they may do I/O, because many parts of nsNSSSocketInfo (the
  * subclass of TransportSecurityInfo used when validating certificates during
  * an SSL handshake) and the PSM NSS I/O layer are not thread-safe, and because
  * we need the event to interrupt the PR_Poll that may waiting for I/O on the
  * socket for which we are validating the cert.
  */
 
 #include "SSLServerCertVerification.h"
+
+#include <cstring>
+
 #include "CertVerifier.h"
 #include "nsIBadCertListener2.h"
 #include "nsICertOverrideService.h"
 #include "nsISiteSecurityService.h"
 #include "nsNSSComponent.h"
 #include "nsNSSCleaner.h"
 #include "nsRecentBadCerts.h"
 #include "nsNSSIOLayer.h"
@@ -628,39 +631,39 @@ private:
   RefPtr<CertErrorRunnable> mCertErrorRunnable;
 };
 
 class SSLServerCertVerificationJob : public nsRunnable
 {
 public:
   // Must be called only on the socket transport thread
   static SECStatus Dispatch(const void * fdForLogging,
-                            TransportSecurityInfo * infoObject,
+                            nsNSSSocketInfo * infoObject,
                             CERTCertificate * serverCert,
                             SECItem * stapledOCSPResponse,
                             uint32_t providerFlags);
 private:
   NS_DECL_NSIRUNNABLE
 
   // Must be called only on the socket transport thread
   SSLServerCertVerificationJob(const void * fdForLogging,
-                               TransportSecurityInfo * infoObject, 
+                               nsNSSSocketInfo * infoObject,
                                CERTCertificate * cert,
                                SECItem * stapledOCSPResponse,
                                uint32_t providerFlags);
   const void * const mFdForLogging;
-  const RefPtr<TransportSecurityInfo> mInfoObject;
+  const RefPtr<nsNSSSocketInfo> mInfoObject;
   const ScopedCERTCertificate mCert;
   const uint32_t mProviderFlags;
   const TimeStamp mJobStartTime;
   const ScopedSECItem mStapledOCSPResponse;
 };
 
 SSLServerCertVerificationJob::SSLServerCertVerificationJob(
-    const void * fdForLogging, TransportSecurityInfo * infoObject,
+    const void * fdForLogging, nsNSSSocketInfo * infoObject,
     CERTCertificate * cert, SECItem * stapledOCSPResponse,
     uint32_t providerFlags)
   : mFdForLogging(fdForLogging)
   , mInfoObject(infoObject)
   , mCert(CERT_DupCertificate(cert))
   , mProviderFlags(providerFlags)
   , mJobStartTime(TimeStamp::Now())
   , mStapledOCSPResponse(SECITEM_DupItem(stapledOCSPResponse))
@@ -850,17 +853,17 @@ BlockServerCertChangeForSpdy(nsNSSSocket
   // Report an error - changed cert is confirmed
   PR_LOG(gPIPNSSLog, PR_LOG_DEBUG,
          ("SPDY Refused to allow new cert during renegotiation\n"));
   PR_SetError(SSL_ERROR_RENEGOTIATION_NOT_ALLOWED, 0);
   return SECFailure;
 }
 
 SECStatus
-AuthCertificate(TransportSecurityInfo * infoObject, CERTCertificate * cert,
+AuthCertificate(nsNSSSocketInfo * infoObject, CERTCertificate * cert,
                 SECItem * stapledOCSPResponse, uint32_t providerFlags)
 {
   if (cert->serialNumber.data &&
       cert->issuerName &&
       !strcmp(cert->issuerName, 
         "CN=UTN-USERFirst-Hardware,OU=http://www.usertrust.com,O=The USERTRUST Network,L=Salt Lake City,ST=UT,C=US")) {
 
     unsigned char *server_cert_comparison_start = cert->serialNumber.data;
@@ -918,16 +921,35 @@ AuthCertificate(TransportSecurityInfo * 
       }
     } else {
       // stapled OCSP response present and good
       Telemetry::Accumulate(Telemetry::SSL_OCSP_STAPLING, 1);
     }
   } else {
     // no stapled OCSP response
     Telemetry::Accumulate(Telemetry::SSL_OCSP_STAPLING, 2);
+
+    uint32_t reasonsForNotFetching = 0;
+
+    char* ocspURI = CERT_GetOCSPAuthorityInfoAccessLocation(cert);
+    if (!ocspURI) {
+      reasonsForNotFetching |= 1; // invalid/missing OCSP URI
+    } else {
+      if (std::strncmp(ocspURI, "http://", 7)) { // approximation
+        reasonsForNotFetching |= 1; // invalid/missing OCSP URI
+      }
+      PORT_Free(ocspURI);
+    }
+
+    if (!infoObject->SharedState().IsOCSPFetchingEnabled()) {
+      reasonsForNotFetching |= 2;
+    }
+
+    Telemetry::Accumulate(Telemetry::SSL_OCSP_MAY_FETCH,
+                          reasonsForNotFetching);
   }
 
   CERTCertList *verifyCertChain = nullptr;
   SECOidTag evOidPolicy;
   rv = PSM_SSL_PKIX_AuthCertificate(cert, infoObject, infoObject->GetHostNameRaw(),
                                     &verifyCertChain, &evOidPolicy);
 
   // We want to remember the CA certs in the temp db, so that the application can find the
@@ -1039,17 +1061,17 @@ AuthCertificate(TransportSecurityInfo * 
     }
   }
 
   return rv;
 }
 
 /*static*/ SECStatus
 SSLServerCertVerificationJob::Dispatch(const void * fdForLogging,
-                                       TransportSecurityInfo * infoObject,
+                                       nsNSSSocketInfo * infoObject,
                                        CERTCertificate * serverCert,
                                        SECItem * stapledOCSPResponse,
                                        uint32_t providerFlags)
 {
   // Runs on the socket transport thread
   if (!infoObject || !serverCert) {
     NS_ERROR("Invalid parameters for SSL server cert validation");
     PR_SetError(PR_INVALID_ARGUMENT_ERROR, 0);
--- a/security/manager/ssl/src/SharedSSLState.cpp
+++ b/security/manager/ssl/src/SharedSSLState.cpp
@@ -131,16 +131,17 @@ PrivateBrowsingObserver::Observe(nsISupp
   return NS_OK;
 }
 
 SharedSSLState::SharedSSLState()
 : mClientAuthRemember(new nsClientAuthRememberService)
 , mMutex("SharedSSLState::mMutex")
 , mSocketCreated(false)
 , mOCSPStaplingEnabled(false)
+, mOCSPFetchingEnabled(false)
 {
   mIOLayerHelpers.Init();
   mClientAuthRemember->Init();
 }
 
 SharedSSLState::~SharedSSLState()
 {
 }
--- a/security/manager/ssl/src/SharedSSLState.h
+++ b/security/manager/ssl/src/SharedSSLState.h
@@ -31,38 +31,44 @@ public:
 
   nsSSLIOLayerHelpers& IOLayerHelpers() {
     return mIOLayerHelpers;
   }
 
   // Main-thread only
   void ResetStoredData();
   void NotePrivateBrowsingStatus();
-  void SetOCSPStaplingEnabled(bool enabled) { mOCSPStaplingEnabled = enabled; }
+  void SetOCSPOptions(bool fetchingEnabled, bool staplingEnabled)
+  {
+    mOCSPFetchingEnabled = fetchingEnabled;
+    mOCSPStaplingEnabled = staplingEnabled;
+  }
 
   // The following methods may be called from any thread
   bool SocketCreated();
   void NoteSocketCreated();
   static void NoteCertOverrideServiceInstantiated();
   static void NoteCertDBServiceInstantiated();
   bool IsOCSPStaplingEnabled() const { return mOCSPStaplingEnabled; }
+  bool IsOCSPFetchingEnabled() const { return mOCSPFetchingEnabled; }
 
 private:
   void Cleanup();
 
   nsCOMPtr<nsIObserver> mObserver;
   RefPtr<nsClientAuthRememberService> mClientAuthRemember;
   nsSSLIOLayerHelpers mIOLayerHelpers;
 
   // True if any sockets have been created that use this shared data.
   // Requires synchronization between the socket and main threads for
   // reading/writing.
   Mutex mMutex;
   bool mSocketCreated;
   bool mOCSPStaplingEnabled;
+  bool mOCSPFetchingEnabled;
 };
 
 SharedSSLState* PublicSSLState();
 SharedSSLState* PrivateSSLState();
 
 } // namespace psm
 } // namespace mozilla
 
--- a/security/manager/ssl/src/nsNSSComponent.cpp
+++ b/security/manager/ssl/src/nsNSSComponent.cpp
@@ -1019,18 +1019,18 @@ void nsNSSComponent::setValidationOption
   bool aiaDownloadEnabled = Preferences::GetBool("security.missing_cert_download.enabled",
                                                  false);
 
   bool ocspStaplingEnabled = Preferences::GetBool("security.ssl.enable_ocsp_stapling",
                                                   true);
   if (!ocspEnabled) {
     ocspStaplingEnabled = false;
   }
-  PublicSSLState()->SetOCSPStaplingEnabled(ocspStaplingEnabled);
-  PrivateSSLState()->SetOCSPStaplingEnabled(ocspStaplingEnabled);
+  PublicSSLState()->SetOCSPOptions(ocspEnabled, ocspStaplingEnabled);
+  PrivateSSLState()->SetOCSPOptions(ocspEnabled, ocspStaplingEnabled);
 
   setNonPkixOcspEnabled(ocspEnabled);
 
   CERT_SetOCSPFailureMode( ocspRequired ?
                            ocspMode_FailureIsVerificationFailure
                            : ocspMode_FailureIsNotAVerificationFailure);
 
   int OCSPTimeoutSeconds = 3;
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -4696,16 +4696,21 @@
     "n_values": 8,
     "description": "Type of handshake (1=resumption, 2=false started, 3=chose not to false start, 4=not allowed to false start)"
   },
   "SSL_OCSP_STAPLING": {
     "kind": "enumerated",
     "n_values": 8,
     "description": "Status of OCSP stapling on this handshake (1=present, good; 2=none; 3=present, expired; 4=present, other error)"
   },
+  "SSL_OCSP_MAY_FETCH": {
+    "kind": "enumerated",
+    "n_values": 8,
+    "description": "For non-stapling cases, is OCSP fetching a possibility? (0=yes, 1=no because missing/invalid OCSP URI, 2=no because fetching disabled, 3=no because both)"
+  },
   "CERT_OCSP_ENABLED": {
     "kind": "boolean",
     "description": "Is OCSP fetching enabled? (pref security.OCSP.enabled)"
   },
   "CERT_OCSP_REQUIRED": {
     "kind": "boolean",
     "description": "Is OCSP required when the cert has an OCSP URI? (pref security.OCSP.require)"
   }