Update NSS to NSS_3_15_BETA2.

nss/lib/certhigh/ocsp.c

 /* 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/. */
 
 /*
  * Implementation of OCSP services, for both client and server.
  * (XXX, really, mostly just for client right now, but intended to do both.)
  *
  * $Id$
  */
@@ skipping 106 matching lines @@
                               void *pwArg,
                               PRBool *certIDWasConsumed,
                               SECStatus *rv_ocsp);
 
 static SECStatus
 ocsp_CacheEncodedOCSPResponse(CERTCertDBHandle *handle,
                               CERTOCSPCertID *certID,
                               CERTCertificate *cert,
                               int64 time,
                               void *pwArg,
-                              SECItem *encodedResponse,
+                              const SECItem *encodedResponse,
+                              PRBool cacheInvalid,
                               PRBool *certIDWasConsumed,
-                              PRBool cacheNegative,
                               SECStatus *rv_ocsp);
 
 static SECStatus
 ocsp_GetVerifiedSingleResponseForCertID(CERTCertDBHandle *handle, 
                                         CERTOCSPResponse *response, 
                                         CERTOCSPCertID   *certID,
                                         CERTCertificate  *signerCert,
                                         int64             time,
                                         CERTOCSPSingleResponse **pSingleResponse);
 
 static SECStatus
 ocsp_CertRevokedAfter(ocspRevokedInfo *revokedInfo, int64 time);
 
+static CERTOCSPCertID *
+cert_DupOCSPCertID(CERTOCSPCertID *src);

[wtc, 2013/04/24 22:49:45]

The argument should be 'const'.

+
 #ifndef DEBUG
 #define OCSP_TRACE(msg)
 #define OCSP_TRACE_TIME(msg, time)
 #define OCSP_TRACE_CERT(cert)
 #define OCSP_TRACE_CERTID(certid)
 #else
 #define OCSP_TRACE(msg) ocsp_Trace msg
 #define OCSP_TRACE_TIME(msg, time) ocsp_dumpStringWithTime(msg, time)
 #define OCSP_TRACE_CERT(cert) dumpCertificate(cert)
 #define OCSP_TRACE_CERTID(certid) dumpCertID(certid)
@@ skipping 606 matching lines @@
     now = PR_Now();
     retval = (cacheItem->nextFetchAttemptTime > now);
     OCSP_TRACE(("OCSP ocsp_IsCacheItemFresh: %d\n", retval));
     PR_ExitMonitor(OCSP_Global.monitor);
     return retval;
 }
 
 /*
  * Status in *certIDWasConsumed will always be correct, regardless of 
  * return value.
+ * If the caller is unable to transfer ownership of certID,
+ * then the caller must set certIDWasConsumed to NULL,
+ * and this function will potentially duplicate the certID object.
  */
 static SECStatus
 ocsp_CreateOrUpdateCacheEntry(OCSPCacheData *cache, 
                               CERTOCSPCertID *certID,
                               CERTOCSPSingleResponse *single,
                               PRBool *certIDWasConsumed)
 {
     SECStatus rv;
     OCSPCacheItem *cacheItem;
     OCSP_TRACE(("OCSP ocsp_CreateOrUpdateCacheEntry\n"));
   
-    if (!certIDWasConsumed) {
-        PORT_SetError(SEC_ERROR_INVALID_ARGS);
-        return SECFailure;
-    }
+    if (certIDWasConsumed)
     *certIDWasConsumed = PR_FALSE;

[wtc, 2013/04/24 22:49:45]

We should fix the indentation.

   
     PR_EnterMonitor(OCSP_Global.monitor);
     PORT_Assert(OCSP_Global.maxCacheEntries >= 0);
   
     cacheItem = ocsp_FindCacheEntry(cache, certID);
     if (!cacheItem) {
-        rv = ocsp_CreateCacheItemAndConsumeCertID(cache, certID, 
+        CERTOCSPCertID *myCertID;
+        if (certIDWasConsumed) {
+            myCertID = certID;
+            *certIDWasConsumed = PR_TRUE;
+        } else {
+            myCertID = cert_DupOCSPCertID(certID);
+            if (!myCertID) {
+                PR_ExitMonitor(OCSP_Global.monitor);
+                PORT_SetError(PR_OUT_OF_MEMORY_ERROR);
+                return SECFailure;
+            }
+        }
+
+        rv = ocsp_CreateCacheItemAndConsumeCertID(cache, myCertID,
                                                   &cacheItem);
         if (rv != SECSuccess) {
             PR_ExitMonitor(OCSP_Global.monitor);
             return rv;
         }
-        *certIDWasConsumed = PR_TRUE;
     }
     if (single) {
-        rv = ocsp_SetCacheItemResponse(cacheItem, single);
-        if (rv != SECSuccess) {
-            ocsp_RemoveCacheItem(cache, cacheItem);
-            PR_ExitMonitor(OCSP_Global.monitor);
-            return rv;
+        PRTime thisUpdate;
+        rv = DER_GeneralizedTimeToTime(&thisUpdate, &single->thisUpdate);
+
+        if (!cacheItem->haveThisUpdate ||
+            (rv == SECSuccess && cacheItem->thisUpdate < thisUpdate)) {
+            rv = ocsp_SetCacheItemResponse(cacheItem, single);
+            if (rv != SECSuccess) {
+                ocsp_RemoveCacheItem(cache, cacheItem);
+                PR_ExitMonitor(OCSP_Global.monitor);
+                return rv;
+            }
+        } else {
+            OCSP_TRACE(("Not caching response because the response is not newer than the cache"));

[wtc, 2013/04/24 22:49:45]

Fold this long line.

         }
     } else {
         cacheItem->missingResponseError = PORT_GetError();
+        if (cacheItem->certStatusArena) {
+            PORT_FreeArena(cacheItem->certStatusArena, PR_FALSE);
+            cacheItem->certStatusArena = NULL;
+        }
     }
     ocsp_FreshenCacheItemNextFetchAttemptTime(cacheItem);
     ocsp_CheckCacheSize(cache);
 
     PR_ExitMonitor(OCSP_Global.monitor);
     return SECSuccess;
 }
 
 extern SECStatus
 CERT_SetOCSPFailureMode(SEC_OcspFailureMode ocspFailureMode)
@@ skipping 720 matching lines @@
 }
 
 /*
  * Digest data using the specified algorithm.
  * The necessary storage for the digest data is allocated.  If "fill" is
  * non-null, the data is put there, otherwise a SECItem is allocated.
  * Allocation from "arena" if it is non-null, heap otherwise.  Any problem
  * results in a NULL being returned (and an appropriate error set).
  */
 
-static SECItem *
+SECItem *
 ocsp_DigestValue(PRArenaPool *arena, SECOidTag digestAlg, 
                  SECItem *fill, const SECItem *src)
 {
     const SECHashObject *digestObject;
     SECItem *result = NULL;
     void *mark = NULL;
     void *digestBuff = NULL;
 
     if ( arena != NULL ) {
         mark = PORT_ArenaMark(arena);
@@ skipping 186 matching lines @@
     
     certID = ocsp_CreateCertID(arena, cert, time);
     if (!certID) {
         PORT_FreeArena(arena, PR_FALSE);
         return NULL;
     }
     certID->poolp = arena;
     return certID;
 }
 
+static CERTOCSPCertID *
+cert_DupOCSPCertID(CERTOCSPCertID *src)
+{
+    CERTOCSPCertID *dest;
+    PRArenaPool *arena = NULL;
+
+    if (!src) {
+        PORT_SetError(SEC_ERROR_INVALID_ARGS);
+        return NULL;
+    }
+
+    arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
+    if (!arena)
+        goto loser;
+
+    dest = PORT_ArenaZNew(arena, CERTOCSPCertID);
+      if (!dest)

[wtc, 2013/04/24 22:49:45]

Fix indentation.

+        goto loser;
+
+#define DUPHELP(element) \
+    if (src->element.data) {  \
+        if (SECITEM_CopyItem(arena, &dest->element, &src->element) \
+            != SECSuccess) \
+            goto loser;     \

[wtc, 2013/04/24 22:49:45]

Add curly braces. Combine nested ifs using &&.

+    }
+
+    DUPHELP(hashAlgorithm.algorithm)
+    DUPHELP(hashAlgorithm.parameters)
+    DUPHELP(issuerNameHash)
+    DUPHELP(issuerKeyHash)
+    DUPHELP(serialNumber)
+    DUPHELP(issuerSHA1NameHash)
+    DUPHELP(issuerMD5NameHash)
+    DUPHELP(issuerMD2NameHash)
+    DUPHELP(issuerSHA1KeyHash)
+    DUPHELP(issuerMD5KeyHash)
+    DUPHELP(issuerMD2KeyHash)
+
+    dest->poolp = arena;
+    return dest;
+
+loser:
+    if (arena)
+        PORT_FreeArena(arena, PR_FALSE);
+    PORT_SetError(PR_OUT_OF_MEMORY_ERROR);
+    return NULL;
+}
+
 /*
  * Callback to set Extensions in request object
  */
 void SetSingleReqExts(void *object, CERTCertExtension **exts)
 {
   ocspSingleRequest *singleRequest =
     (ocspSingleRequest *)object;
 
   singleRequest->singleRequestExtensions = exts;
 }
@@ skipping 763 matching lines @@
  *   SECItem *src
  *     Pointer to a SECItem holding DER encoded OCSP Response.
  * RETURN:
  *   Returns a pointer to a CERTOCSPResponse (the decoded OCSP Response);
  *   the caller is responsible for destroying it.  Or NULL if error (either
  *   response could not be decoded (SEC_ERROR_OCSP_MALFORMED_RESPONSE),
  *   it was of an unexpected type (SEC_ERROR_OCSP_UNKNOWN_RESPONSE_TYPE),
  *   or a low-level or internal error occurred).
  */
 CERTOCSPResponse *
-CERT_DecodeOCSPResponse(SECItem *src)
+CERT_DecodeOCSPResponse(const SECItem *src)
 {
     PRArenaPool *arena = NULL;
     CERTOCSPResponse *response = NULL;
     SECStatus rv = SECFailure;
     ocspResponseStatus sv;
     SECItem newSrc;
 
     arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
     if (arena == NULL) {
         goto loser;
@@ skipping 2261 matching lines @@
  *   void *pwArg
  *     argument for password prompting, if needed
  * RETURN:
  *   SECSuccess if the cert was found in the cache, or if the OCSP response was
  *   found to be valid and inserted into the cache. SECFailure otherwise.
  */
 SECStatus
 CERT_CacheOCSPResponseFromSideChannel(CERTCertDBHandle *handle,
                                       CERTCertificate *cert,
                                       int64 time,
-                                      SECItem *encodedResponse,
+                                      const SECItem *encodedResponse,
                                       void *pwArg)
 {
-    CERTOCSPCertID *certID;
+    CERTOCSPCertID *certID = NULL;
     PRBool certIDWasConsumed = PR_FALSE;
     SECStatus rv = SECFailure;
     SECStatus rvOcsp;
     SECErrorCodes dummy_error_code; /* we ignore this */
 
+    /* The OCSP cache can be in three states regarding this certificate:
+     *    + Good (cached, timely, 'good' response, or revoked in the future)
+     *    + Revoked (cached, timely, but doesn't fit in the last category)
+     *    + Miss (no knowledge)
+     *
+     * Likewise, the side-channel information can be
+     *    + Good (timely, 'good' response, or revoked in the future)
+     *    + Revoked (timely, but doesn't fit in the last category)
+     *    + Invalid (bad syntax, bad signature, not timely etc)
+     *
+     * The common case is that the cache result is Good and so is the
+     * side-channel information. We want to save processing time in this case
+     * so we say that any time we see a Good result from the cache we return
+     * early.
+     *
+     *                       Cache result
+     *      | Good             Revoked               Miss
+     *   ---+--------------------------------------------
+     *    G |  noop           Cache more           Cache it
+     * S    |                 recent result
+     * i    |
+     * d    |
+     * e    |
+     *    R |  noop           Cache more           Cache it
+     * C    |                 recent result
+     * h    |
+     * a    |
+     * n    |
+     * n  I |  noop           Noop                  Noop
+     * e    |
+     * l    |
+     *
+     * When we fetch from the network we might choose to cache a negative
+     * result when the response is invalid. This saves us hammering, uselessly,
+     * at a broken responder. However, side channels are commonly attacker
+     * controlled and so we must not cache a negative result for an Invalid
+     * side channel.
+     */
+
+    if (!cert) {
+        PORT_SetError(SEC_ERROR_INVALID_ARGS);
+        return SECFailure;
+    }
     certID = CERT_CreateOCSPCertID(cert, time);
     if (!certID)
         return SECFailure;
     rv = ocsp_GetCachedOCSPResponseStatusIfFresh(
         certID, time, PR_FALSE, /* ignoreGlobalOcspFailureSetting */
         &rvOcsp, &dummy_error_code);
     if (rv == SECSuccess && rvOcsp == SECSuccess) {
-        /* The cached value is good. We don't want to waste time validating
-         * this OCSP response. */
+        /* The cached value is good. We don't want to waste time validating
+         * this OCSP response. This is the first column in the table above. */
         CERT_DestroyOCSPCertID(certID);
         return rv;
     }
 
-    /* Since the OCSP response came from a side channel it is attacker
-     * controlled. The attacker can have chosen any valid OCSP response,
-     * including responses from the past. In this case,
-     * ocsp_GetVerifiedSingleResponseForCertID will fail. If we recorded a
-     * negative cache entry in this case, then the attacker would have
-     * 'poisoned' our cache (denial of service), so we don't record negative
-     * results. */
-    rv = ocsp_CacheEncodedOCSPResponse(handle, certID, cert, time, pwArg,
-                                       encodedResponse, &certIDWasConsumed,
-                                       PR_FALSE /* don't cache failures */,
+    /* The logic for caching the more recent response is handled in
+     * ocsp_CreateOrUpdateCacheEntry, which is called by this function. */
+    rv = ocsp_CacheEncodedOCSPResponse(handle, certID, cert, time,
+                                       pwArg, encodedResponse,
+                                       PR_FALSE /* don't cache if invalid */,
+                                       &certIDWasConsumed,
                                        &rvOcsp);
     if (!certIDWasConsumed) {
         CERT_DestroyOCSPCertID(certID);
     }
     return rv == SECSuccess ? rvOcsp : rv;
 }
 
 /*
  * Status in *certIDWasConsumed will always be correct, regardless of 
  * return value.
@@ skipping 67 matching lines @@
      */
     encodedResponse = 
         ocsp_GetEncodedOCSPResponseForSingleCert(NULL, certID, cert, location,
                                                  time, locationIsDefault,
                                                  pwArg, &request);
     if (encodedResponse == NULL) {
         goto loser;
     }
 
     rv = ocsp_CacheEncodedOCSPResponse(handle, certID, cert, time, pwArg,
-                                       encodedResponse, certIDWasConsumed,
-                                       PR_TRUE /* cache failures */, rv_ocsp);
+                                       encodedResponse,
+                                       PR_TRUE /* cache if invalid */,
+                                       certIDWasConsumed, rv_ocsp);
 
 loser:
     if (request != NULL)
         CERT_DestroyOCSPRequest(request);
     if (encodedResponse != NULL)
         SECITEM_FreeItem(encodedResponse, PR_TRUE);
     if (location != NULL)
         PORT_Free(location);
 
     return rv;
@@ skipping 17 matching lines @@
  *   CERTOCSPCertID *certID
  *     the cert ID corresponding to |cert|
  *   CERTCertificate *cert
  *     the certificate being checked
  *   int64 time
  *     time for which status is to be determined
  *   void *pwArg
  *     the opaque argument to the password prompting function.
  *   SECItem *encodedResponse
  *     the DER encoded bytes of the OCSP response
+ *   PRBool cacheInvalid
+ *     If true then invalid responses will cause a negative cache entry to be
+ *     created. (Invalid means bad syntax, bad signature etc)
  *   PRBool *certIDWasConsumed
  *     (output) on return, this is true iff |certID| was consumed by this
  *     function.
  *   SECStatus *rv_ocsp
  *     (output) on return, this is SECSuccess iff the response is good (see
  *     definition of 'good' above).
  * RETURN:
  *   SECSuccess iff the response is valid.
  */
 static SECStatus
 ocsp_CacheEncodedOCSPResponse(CERTCertDBHandle *handle,
                               CERTOCSPCertID *certID,
                               CERTCertificate *cert,
                               int64 time,
                               void *pwArg,
-                              SECItem *encodedResponse,
+                              const SECItem *encodedResponse,
+                              PRBool cacheInvalid,
                               PRBool *certIDWasConsumed,
-                              PRBool cacheNegative,
                               SECStatus *rv_ocsp)
 {
     CERTOCSPResponse *response = NULL;
     CERTCertificate *signerCert = NULL;
     CERTCertificate *issuerCert = NULL;
     CERTOCSPSingleResponse *single = NULL;
     SECStatus rv = SECFailure;
 
     *certIDWasConsumed = PR_FALSE;
     *rv_ocsp = SECFailure;
@@ skipping 38 matching lines @@
      */
 
     rv = ocsp_GetVerifiedSingleResponseForCertID(handle, response, certID, 
                                                  signerCert, time, &single);
     if (rv != SECSuccess)
         goto loser;
 
     *rv_ocsp = ocsp_SingleResponseCertHasGoodStatus(single, time);
 
 loser:
-    if (cacheNegative || *rv_ocsp == SECSuccess) {
+    /* If single == NULL here then the response was invalid. */
+    if (single != NULL || cacheInvalid) {
         PR_EnterMonitor(OCSP_Global.monitor);
         if (OCSP_Global.maxCacheEntries >= 0) {
             /* single == NULL means: remember response failure */
             ocsp_CreateOrUpdateCacheEntry(&OCSP_Global.cache, certID, single,
                                           certIDWasConsumed);
             /* ignore cache update failures */
         }
         PR_ExitMonitor(OCSP_Global.monitor);
     }
 
@@ skipping 626 matching lines @@
       case ocspResponse_unauthorized:
         PORT_SetError(SEC_ERROR_OCSP_UNAUTHORIZED_REQUEST);
         break;
       case ocspResponse_unused:
       default:
         PORT_SetError(SEC_ERROR_OCSP_UNKNOWN_RESPONSE_STATUS);
         break;
     }
     return SECFailure;
 }