Bug 338986 - Unauthorized OCSP response error from user's default OCSP responder. r=kengert, sr=rrelyea.

5 files changed, 404 insertions, 298 deletions

diff --git a/security/nss/cmd/lib/SECerrs.h b/security/nss/cmd/lib/SECerrs.h
index 8d2908ab1..aa5e28772 100644
--- a/security/nss/cmd/lib/SECerrs.h
+++ b/security/nss/cmd/lib/SECerrs.h
@@ -514,3 +514,8 @@ ER3(SEC_ERROR_NOT_INITIALIZED,      		(SEC_ERROR_BASE + 154),
 ER3(SEC_ERROR_TOKEN_NOT_LOGGED_IN,  		(SEC_ERROR_BASE + 155),
 "The operation failed because the PKCS#11 token is not logged in.")
 
+ER3(SEC_ERROR_OCSP_RESPONDER_CERT_INVALID,  	(SEC_ERROR_BASE + 156),
+"OCSP Trusted Responder Cert is invalid.")
+
+ER3(SEC_ERROR_OCSP_BAD_SIGNATURE,      		(SEC_ERROR_BASE + 157),
+"OCSP response has an invalid signature.")
diff --git a/security/nss/lib/certdb/cert.h b/security/nss/lib/certdb/cert.h
index 9dfe8d775..d237cd555 100644
--- a/security/nss/lib/certdb/cert.h
+++ b/security/nss/lib/certdb/cert.h
@@ -1510,8 +1510,8 @@ CERT_UnlockCertTrust(CERTCertificate *cert);
  * results in a NULL being returned (and an appropriate error set).
  */ 
 extern SECItem *
-CERT_SPKDigestValueForCert(PRArenaPool *arena, CERTCertificate *cert,
-			   SECOidTag digestAlg, SECItem *fill);
+cert_GetSPKIDigest(PRArenaPool *arena, const CERTCertificate *cert,
+                   SECOidTag digestAlg, SECItem *fill);
 
 /*
  * fill in nsCertType field of the cert based on the cert extension
diff --git a/security/nss/lib/certdb/certt.h b/security/nss/lib/certdb/certt.h
index b50ad6e5c..5d4eb6f42 100644
--- a/security/nss/lib/certdb/certt.h
+++ b/security/nss/lib/certdb/certt.h
@@ -493,6 +493,7 @@ typedef enum SECCertUsageEnum {
 
 typedef PRInt64 SECCertificateUsage;
 
+#define certificateUsageCheckAllUsages         (0x0000)
 #define certificateUsageSSLClient              (0x0001)
 #define certificateUsageSSLServer              (0x0002)
 #define certificateUsageSSLServerWithStepUp    (0x0004)
diff --git a/security/nss/lib/certhigh/ocsp.c b/security/nss/lib/certhigh/ocsp.c
index 481b9523a..5fab28928 100644
--- a/security/nss/lib/certhigh/ocsp.c
+++ b/security/nss/lib/certhigh/ocsp.c
@@ -433,6 +433,12 @@ static const SEC_ASN1Template ocsp_ResponderIDOtherTemplate[] = {
 	offsetof(ocspResponderID, responderIDValue.other) }
 };
 
+/* Decode choice container, but leave x509 name object encoded */
+static const SEC_ASN1Template ocsp_ResponderIDDerNameTemplate[] = {
+    { SEC_ASN1_EXPLICIT | SEC_ASN1_CONSTRUCTED | SEC_ASN1_CONTEXT_SPECIFIC | 1,
+	0, SEC_AnyTemplate }
+};
+
 /*
  * SingleResponse	::=	SEQUENCE {
  *	certID			CertID,
@@ -671,6 +677,112 @@ CERT_DestroyOCSPCertID(CERTOCSPCertID* certID)
     return SECFailure;
 }
 
+/*
+ * 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 *
+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);
+    }
+
+    digestObject = HASH_GetHashObjectByOidTag(digestAlg);
+    if ( digestObject == NULL ) {
+        goto loser;
+    }
+
+    if (fill == NULL || fill->data == NULL) {
+	result = SECITEM_AllocItem(arena, fill, digestObject->length);
+	if ( result == NULL ) {
+	   goto loser;
+	}
+	digestBuff = result->data;
+    } else {
+	if (fill->len < digestObject->length) {
+	    PORT_SetError(SEC_ERROR_INVALID_ARGS);
+	    goto loser;
+	}
+	digestBuff = fill->data;
+    }
+
+    if (PK11_HashBuf(digestAlg, digestBuff,
+                     src->data, src->len) != SECSuccess) {
+        goto loser;
+    }
+
+    if ( arena != NULL ) {
+        PORT_ArenaUnmark(arena, mark);
+    }
+
+    if (result == NULL) {
+        result = fill;
+    }
+    return result;
+
+loser:
+    if (arena != NULL) {
+        PORT_ArenaRelease(arena, mark);
+    } else {
+        if (result != NULL) {
+            SECITEM_FreeItem(result, (fill == NULL) ? PR_TRUE : PR_FALSE);
+        }
+    }
+    return(NULL);
+}
+
+/*
+ * Digest the cert's subject public key 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).
+ */
+SECItem *
+cert_GetSPKIDigest(PRArenaPool *arena, const CERTCertificate *cert,
+                           SECOidTag digestAlg, SECItem *fill)
+{
+    SECItem spk;
+
+    /*
+     * Copy just the length and data pointer (nothing needs to be freed)
+     * of the subject public key so we can convert the length from bits
+     * to bytes, which is what the digest function expects.
+     */
+    spk = cert->subjectPublicKeyInfo.subjectPublicKey;
+    DER_ConvertBitString(&spk);
+
+    return ocsp_DigestValue(arena, digestAlg, fill, &spk);
+}
+
+/*
+ * Digest the cert's subject name using the specified algorithm.
+ */
+static SECItem *
+cert_GetSubjectNameDigest(PRArenaPool *arena, const CERTCertificate *cert,
+                           SECOidTag digestAlg, SECItem *fill)
+{
+    SECItem name;
+
+    /*
+     * Copy just the length and data pointer (nothing needs to be freed)
+     * of the subject name
+     */
+    name = cert->derSubject;
+
+    return ocsp_DigestValue(arena, digestAlg, fill, &name);
+}
 
 /*
  * Create and fill-in a CertID.  This function fills in the hash values
@@ -709,58 +821,35 @@ ocsp_CreateCertID(PRArenaPool *arena, CERTCertificate *cert, int64 time)
 	goto loser;
     }
 
-    tempItem = SEC_ASN1EncodeItem(NULL, NULL, &issuerCert->subject,
-				  CERT_NameTemplate);
-    if (tempItem == NULL) {
-	goto loser;
-    }
-
-    if (SECITEM_AllocItem(arena, &(certID->issuerNameHash),
-			  SHA1_LENGTH) == NULL) {
-	goto loser;
-    }
-    rv = PK11_HashBuf(SEC_OID_SHA1, certID->issuerNameHash.data,
-		      tempItem->data, tempItem->len);
-    if (rv != SECSuccess) {
-	goto loser; 
+    if (cert_GetSubjectNameDigest(arena, issuerCert, SEC_OID_SHA1,
+                                  &(certID->issuerNameHash)) == NULL) {
+        goto loser;
     }
     certID->issuerSHA1NameHash.data = certID->issuerNameHash.data;
     certID->issuerSHA1NameHash.len = certID->issuerNameHash.len;
-    /* cache the other two hash algorithms as well */
-    if (SECITEM_AllocItem(arena, &(certID->issuerMD5NameHash),
-			  MD5_LENGTH) == NULL) {
-	goto loser;
-    }
-    rv = PK11_HashBuf(SEC_OID_MD5, certID->issuerMD5NameHash.data,
-		      tempItem->data, tempItem->len);
-    if (rv != SECSuccess) {
-	goto loser; 
-    }
-    if (SECITEM_AllocItem(arena, &(certID->issuerMD2NameHash),
-			  MD2_LENGTH) == NULL) {
-	goto loser;
-    }
-    rv = PK11_HashBuf(SEC_OID_MD2, certID->issuerMD2NameHash.data,
-		      tempItem->data, tempItem->len);
-    if (rv != SECSuccess) {
-	goto loser; 
+
+    if (cert_GetSubjectNameDigest(arena, issuerCert, SEC_OID_MD5,
+                                  &(certID->issuerMD5NameHash)) == NULL) {
+        goto loser;
     }
 
-    SECITEM_FreeItem(tempItem, PR_TRUE);
-    tempItem = NULL;
+    if (cert_GetSubjectNameDigest(arena, issuerCert, SEC_OID_MD2,
+                                  &(certID->issuerMD2NameHash)) == NULL) {
+        goto loser;
+    }
 
-    if (CERT_SPKDigestValueForCert(arena, issuerCert, SEC_OID_SHA1,
+    if (cert_GetSPKIDigest(arena, issuerCert, SEC_OID_SHA1,
 				   &(certID->issuerKeyHash)) == NULL) {
 	goto loser;
     }
     certID->issuerSHA1KeyHash.data = certID->issuerKeyHash.data;
     certID->issuerSHA1KeyHash.len = certID->issuerKeyHash.len;
     /* cache the other two hash algorithms as well */
-    if (CERT_SPKDigestValueForCert(arena, issuerCert, SEC_OID_MD5,
+    if (cert_GetSPKIDigest(arena, issuerCert, SEC_OID_MD5,
 				   &(certID->issuerMD5KeyHash)) == NULL) {
 	goto loser;
     }
-    if (CERT_SPKDigestValueForCert(arena, issuerCert, SEC_OID_MD2,
+    if (cert_GetSPKIDigest(arena, issuerCert, SEC_OID_MD2,
 				   &(certID->issuerMD2KeyHash)) == NULL) {
 	goto loser;
     }
@@ -2425,7 +2514,7 @@ loser:
 
 /* Checks a certificate for the key usage extension of OCSP signer. */
 static PRBool
-ocsp_CertIsOCSPSigner(CERTCertificate *cert)
+ocsp_CertIsOCSPDesignatedResponder(CERTCertificate *cert)
 {
     SECStatus rv;
     SECItem extItem;
@@ -2510,19 +2599,19 @@ ocsp_matchcert(SECItem *certIndex,CERTCertificate *testCert)
     item.data = buf;
     item.len = SHA1_LENGTH;
 
-    if (CERT_SPKDigestValueForCert(NULL,testCert,SEC_OID_SHA1, &item) == NULL) {
+    if (cert_GetSPKIDigest(NULL,testCert,SEC_OID_SHA1, &item) == NULL) {
 	return PR_FALSE;
     }
     if  (SECITEM_ItemsAreEqual(certIndex,&item)) {
 	return PR_TRUE;
     }
-    if (CERT_SPKDigestValueForCert(NULL,testCert,SEC_OID_MD5, &item) == NULL) {
+    if (cert_GetSPKIDigest(NULL,testCert,SEC_OID_MD5, &item) == NULL) {
 	return PR_FALSE;
     }
     if  (SECITEM_ItemsAreEqual(certIndex,&item)) {
 	return PR_TRUE;
     }
-    if (CERT_SPKDigestValueForCert(NULL,testCert,SEC_OID_MD2, &item) == NULL) {
+    if (cert_GetSPKIDigest(NULL,testCert,SEC_OID_MD2, &item) == NULL) {
 	return PR_FALSE;
     }
     if  (SECITEM_ItemsAreEqual(certIndex,&item)) {
@@ -2532,48 +2621,71 @@ ocsp_matchcert(SECItem *certIndex,CERTCertificate *testCert)
     return PR_FALSE;
 }
 
+static PRBool
+ocsp_CertIsOCSPDefaultResponder(CERTCertDBHandle *handle, CERTCertificate *cert);
+
 static CERTCertificate *
 ocsp_CertGetDefaultResponder(CERTCertDBHandle *handle,CERTOCSPCertID *certID);
 
 /*
- * Check the signature on some OCSP data.  This is a helper function that
- * can be used to check either a request or a response.  The result is
- * saved in the signature structure itself for future reference (to avoid
- * repeating the expensive verification operation), as well as returned.
- * In addition to checking the signature, the certificate (and its chain)
- * are also checked for validity (at the specified time) and usage.
- *
- * The type of cert lookup to be performed is specified by "lookupByName":
- * if true, then "certIndex" is actually a CERTName; otherwise it is a
- * SECItem which contains a key hash.
- *
- * If the signature verifies okay, and the argument "pSignerCert" is not
- * null, that parameter will be filled-in with a pointer to the signer's
- * certificate.  The caller is then responsible for destroying the cert.
- *
- * A return of SECSuccess means the verification succeeded.  If not,
- * an error will be set with the reason.  Most likely are:
+ * FUNCTION: CERT_VerifyOCSPResponseSignature
+ *   Check the signature on an OCSP Response.  Will also perform a
+ *   verification of the signer's certificate.  Note, however, that a
+ *   successful verification does not make any statement about the
+ *   signer's *authority* to provide status for the certificate(s),
+ *   that must be checked individually for each certificate.
+ * INPUTS:
+ *   CERTOCSPResponse *response
+ *     Pointer to response structure with signature to be checked.
+ *   CERTCertDBHandle *handle
+ *     Pointer to CERTCertDBHandle for certificate DB to use for verification.
+ *   void *pwArg
+ *     Pointer to argument for password prompting, if needed.
+ * OUTPUTS:
+ *   CERTCertificate **pSignerCert
+ *     Pointer in which to store signer's certificate; only filled-in if
+ *     non-null.
+ * RETURN:
+ *   Returns SECSuccess when signature is valid, anything else means invalid.
+ *   Possible errors set:
+ *	SEC_ERROR_OCSP_MALFORMED_RESPONSE - unknown type of ResponderID
+ *	SEC_ERROR_INVALID_TIME - bad format of "ProducedAt" time
  *	SEC_ERROR_UNKNOWN_SIGNER - signer's cert could not be found
  *	SEC_ERROR_BAD_SIGNATURE - the signature did not verify
- * Other errors are any of the many possible failures in cert verification
- * (e.g. SEC_ERROR_REVOKED_CERTIFICATE, SEC_ERROR_UNTRUSTED_ISSUER) when
- * verifying the signer's cert, or low-level problems (no memory, etc.)
+ *   Other errors are any of the many possible failures in cert verification
+ *   (e.g. SEC_ERROR_REVOKED_CERTIFICATE, SEC_ERROR_UNTRUSTED_ISSUER) when
+ *   verifying the signer's cert, or low-level problems (no memory, etc.)
  */
-static SECStatus
-ocsp_CheckSignature(ocspSignature *signature, SECItem *encodedTBS,
-		    CERTCertDBHandle *handle, SECCertUsage certUsage,
-		    int64 checkTime, PRBool lookupByName, void *certIndex,
-		    void *pwArg, CERTCertificate **pSignerCert,
-		    CERTCertificate *issuer)
+SECStatus
+CERT_VerifyOCSPResponseSignature(CERTOCSPResponse *response,	
+				 CERTCertDBHandle *handle, void *pwArg,
+				 CERTCertificate **pSignerCert,
+				 CERTCertificate *issuer)
 {
     SECItem rawSignature;
+    SECItem *tbsResponseDataDER;
     CERTCertificate *responder = NULL;
     CERTCertificate *signerCert = NULL;
     SECKEYPublicKey *signerKey = NULL;
     CERTCertificate **certs = NULL;
     SECStatus rv = SECFailure;
-    int certCount;
-    int i;
+    int certCount = 0;
+    PRBool lookupByName;
+    void *certIndex;
+    int64 producedAt;
+
+    /* ocsp_DecodeBasicOCSPResponse will fail if asn1 decoder is unable
+     * to properly decode tbsData (see the function and
+     * ocsp_BasicOCSPResponseTemplate). Thus, tbsData can not be
+     * equal to null */
+    ocspResponseData *tbsData = ocsp_GetResponseData(response,
+                                                     &tbsResponseDataDER);
+    ocspSignature *signature = ocsp_GetResponseSignature(response);
+
+    if (!signature) {
+        PORT_SetError(SEC_ERROR_OCSP_BAD_SIGNATURE);
+        return SECFailure;
+    }
 
     /*
      * If this signature has already gone through verification, just
@@ -2589,6 +2701,23 @@ ocsp_CheckSignature(ocspSignature *signature, SECItem *encodedTBS,
 	return signature->status;
     }
 
+    PORT_Assert(tbsData->responderID != NULL);
+    switch (tbsData->responderID->responderIDType) {
+    case ocspResponderID_byName:
+	lookupByName = PR_TRUE;
+	certIndex = &tbsData->derResponderID;
+	break;
+    case ocspResponderID_byKey:
+	lookupByName = PR_FALSE;
+	certIndex = &tbsData->responderID->responderIDValue.keyHash;
+	break;
+    case ocspResponderID_other:
+    default:
+	PORT_Assert(0);
+	PORT_SetError(SEC_ERROR_OCSP_MALFORMED_RESPONSE);
+	return SECFailure;
+    }
+
     /*
      * If the signature contains some certificates as well, temporarily
      * import them in case they are needed for verification.
@@ -2596,39 +2725,47 @@ ocsp_CheckSignature(ocspSignature *signature, SECItem *encodedTBS,
      * Note that the result of this is that each cert in "certs" needs
      * to be destroyed.
      */
-    certCount = 0;
     if (signature->derCerts != NULL) {
 	for (; signature->derCerts[certCount] != NULL; certCount++) {
 	    /* just counting */
-	    /*IMPORT CERT TO SPKI TABLE */
 	}
+	rv = CERT_ImportCerts(handle, certUsageStatusResponder, certCount,
+	                      signature->derCerts, &certs,
+	                      PR_FALSE, PR_FALSE, NULL);
+	if (rv != SECSuccess)
+	     goto finish;
     }
-    rv = CERT_ImportCerts(handle, certUsage, certCount,
-			  signature->derCerts, &certs,
-			  PR_FALSE, PR_FALSE, NULL);
-    if (rv != SECSuccess)
-	goto finish;
 
     /*
      * Now look up the certificate that did the signing.
      * The signer can be specified either by name or by key hash.
      */
     if (lookupByName) {
-	SECItem *encodedName;
-
-	encodedName = SEC_ASN1EncodeItem(NULL, NULL, certIndex,
-					 CERT_NameTemplate);
-	if (encodedName == NULL)
-	    goto finish;
-
-	signerCert = CERT_FindCertByName(handle, encodedName);
-	SECITEM_FreeItem(encodedName, PR_TRUE);
+	SECItem *crIndex = (SECItem*)certIndex;
+	SECItem encodedName;
+	PLArenaPool *arena;
+
+	arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
+	if (arena != NULL) {
+
+	    rv = SEC_QuickDERDecodeItem(arena, &encodedName,
+	                                ocsp_ResponderIDDerNameTemplate,
+	                                crIndex);
+	    if (rv != SECSuccess) {
+	        if (PORT_GetError() == SEC_ERROR_BAD_DER)
+	            PORT_SetError(SEC_ERROR_OCSP_MALFORMED_RESPONSE);
+	    } else {
+	            signerCert = CERT_FindCertByName(handle, &encodedName);
+	    }
+	    PORT_FreeArena(arena, PR_FALSE);
+	}
     } else {
 	/*
 	 * The signer is either 1) a known issuer CA we passed in,
 	 * 2) the default OCSP responder, or 3) an intermediate CA
 	 * passed in the cert list to use. Figure out which it is.
 	 */
+	int i;
 	responder = ocsp_CertGetDefaultResponder(handle,NULL);
 	if (responder && ocsp_matchcert(certIndex,responder)) {
 	    signerCert = CERT_DupCertificate(responder);
@@ -2660,14 +2797,35 @@ ocsp_CheckSignature(ocspSignature *signature, SECItem *encodedTBS,
     signature->wasChecked = PR_TRUE;
 
     /*
+     * The function will also verify the signer certificate; we
+     * need to tell it *when* that certificate must be valid -- for our
+     * purposes we expect it to be valid when the response was signed.
+     * The value of "producedAt" is the signing time.
+     */
+    rv = DER_GeneralizedTimeToTime(&producedAt, &tbsData->producedAt);
+    if (rv != SECSuccess)
+        goto finish;
+
+    /*
      * Just because we have a cert does not mean it is any good; check
      * it for validity, trust and usage.
      */
-    rv = CERT_VerifyCert(handle, signerCert, PR_TRUE, certUsage, checkTime,
-			 pwArg, NULL);
-    if (rv != SECSuccess) {
-        PORT_SetError(SEC_ERROR_OCSP_INVALID_SIGNING_CERT);
-	goto finish;
+    if (ocsp_CertIsOCSPDefaultResponder(handle, signerCert)) {
+        rv = SECSuccess;
+    } else {
+        if (CERT_IsCACert(signerCert, NULL)) {
+            rv = CERT_VerifyCACertForUsage(handle, signerCert, PR_TRUE,
+                                           certUsageStatusResponder,
+                                           producedAt, pwArg, NULL);
+        } else {
+            rv = CERT_VerifyCert(handle, signerCert, PR_TRUE,
+                                 certUsageStatusResponder,
+                                 producedAt, pwArg, NULL);
+        }
+        if (rv != SECSuccess) {
+            PORT_SetError(SEC_ERROR_OCSP_INVALID_SIGNING_CERT);
+            goto finish;
+        }
     }
 
     /*
@@ -2677,7 +2835,6 @@ ocsp_CheckSignature(ocspSignature *signature, SECItem *encodedTBS,
     signerKey = CERT_ExtractPublicKey(signerCert);
     if (signerKey == NULL)
 	goto finish;
-
     /*
      * We copy the signature data *pointer* and length, so that we can
      * modify the length without damaging the original copy.  This is a
@@ -2690,9 +2847,14 @@ ocsp_CheckSignature(ocspSignature *signature, SECItem *encodedTBS,
      */
     DER_ConvertBitString(&rawSignature);
 
-    rv = VFY_VerifyDataWithAlgorithmID(encodedTBS->data, encodedTBS->len, 
-			signerKey, &rawSignature,
-			&signature->signatureAlgorithm, NULL, pwArg);
+    rv = VFY_VerifyDataWithAlgorithmID(tbsResponseDataDER->data,
+                                       tbsResponseDataDER->len,
+                                       signerKey, &rawSignature,
+                                       &signature->signatureAlgorithm,
+                                       NULL, pwArg);
+    if (rv != SECSuccess && PORT_GetError() == SEC_ERROR_BAD_SIGNATURE) {
+        PORT_SetError(SEC_ERROR_OCSP_BAD_SIGNATURE);
+    }
 
 finish:
     if (signature->wasChecked)
@@ -2726,84 +2888,6 @@ finish:
     return rv;
 }
 
-
-/*
- * FUNCTION: CERT_VerifyOCSPResponseSignature
- *   Check the signature on an OCSP Response.  Will also perform a
- *   verification of the signer's certificate.  Note, however, that a
- *   successful verification does not make any statement about the
- *   signer's *authority* to provide status for the certificate(s),
- *   that must be checked individually for each certificate.
- * INPUTS:
- *   CERTOCSPResponse *response
- *     Pointer to response structure with signature to be checked.
- *   CERTCertDBHandle *handle
- *     Pointer to CERTCertDBHandle for certificate DB to use for verification.
- *   void *pwArg
- *     Pointer to argument for password prompting, if needed.
- * OUTPUTS:
- *   CERTCertificate **pSignerCert
- *     Pointer in which to store signer's certificate; only filled-in if
- *     non-null.
- * RETURN:
- *   Returns SECSuccess when signature is valid, anything else means invalid.
- *   Possible errors set:
- *	SEC_ERROR_OCSP_MALFORMED_RESPONSE - unknown type of ResponderID
- *	SEC_ERROR_INVALID_TIME - bad format of "ProducedAt" time
- *	SEC_ERROR_UNKNOWN_SIGNER - signer's cert could not be found
- *	SEC_ERROR_BAD_SIGNATURE - the signature did not verify
- *   Other errors are any of the many possible failures in cert verification
- *   (e.g. SEC_ERROR_REVOKED_CERTIFICATE, SEC_ERROR_UNTRUSTED_ISSUER) when
- *   verifying the signer's cert, or low-level problems (no memory, etc.)
- */
-SECStatus
-CERT_VerifyOCSPResponseSignature(CERTOCSPResponse *response,	
-				 CERTCertDBHandle *handle, void *pwArg,
-				 CERTCertificate **pSignerCert,
-				 CERTCertificate *issuer)
-{
-    ocspResponseData *tbsData;		/* this is what is signed */
-    SECItem *tbsResponseDataDER;
-    PRBool byName;
-    void *certIndex;
-    int64 producedAt;
-    SECStatus rv;
-
-    tbsData = ocsp_GetResponseData(response, &tbsResponseDataDER);
-
-    PORT_Assert(tbsData->responderID != NULL);
-    switch (tbsData->responderID->responderIDType) {
-      case ocspResponderID_byName:
-	byName = PR_TRUE;
-	certIndex = &tbsData->responderID->responderIDValue.name;
-	break;
-      case ocspResponderID_byKey:
-	byName = PR_FALSE;
-	certIndex = &tbsData->responderID->responderIDValue.keyHash;
-	break;
-      case ocspResponderID_other:
-      default:
-	PORT_Assert(0);
-	PORT_SetError(SEC_ERROR_OCSP_MALFORMED_RESPONSE);
-	return SECFailure;
-    }
-
-    /*
-     * ocsp_CheckSignature will also verify the signer certificate; we
-     * need to tell it *when* that certificate must be valid -- for our
-     * purposes we expect it to be valid when the response was signed.
-     * The value of "producedAt" is the signing time.
-     */
-    rv = DER_GeneralizedTimeToTime(&producedAt, &tbsData->producedAt);
-    if (rv != SECSuccess)
-	return rv;
-
-    return ocsp_CheckSignature(ocsp_GetResponseSignature(response),
-			       tbsResponseDataDER,
-			       handle, certUsageStatusResponder, producedAt,
-			       byName, certIndex, pwArg, pSignerCert, issuer);
-}
-
 /*
  * See if the request's certID and the single response's certID match.
  * This can be easy or difficult, depending on whether the same hash
@@ -2866,6 +2950,9 @@ ocsp_CertIDsMatch(CERTCertDBHandle *handle,
 	keyHash = &requestCertID->issuerMD2KeyHash;
 	nameHash = &requestCertID->issuerMD2NameHash;
 	break;
+    default:
+	PORT_SetError(SEC_ERROR_INVALID_ALGORITHM);
+ 	return SECFailure;
     }
 
     if ((keyHash != NULL)
@@ -2899,7 +2986,7 @@ ocsp_GetSingleResponseForCertID(CERTOCSPSingleResponse **responses,
 
     for (i = 0; responses[i] != NULL; i++) {
 	single = responses[i];
-	if (ocsp_CertIDsMatch(handle, certID, single->certID) == PR_TRUE) {
+	if (ocsp_CertIDsMatch(handle, certID, single->certID)) {
 	    return single;
 	}
     }
@@ -2940,12 +3027,13 @@ ocsp_GetCheckingContext(CERTCertDBHandle *handle)
 
     return ocspcx;
 }
+
 /*
- * Return true if the given signerCert is the default responder for
- * the given certID.  If not, or if any error, return false.
+ * Return cert reference if the given signerCert is the default responder for
+ * the given certID.  If not, or if any error, return NULL.
  */
 static CERTCertificate *
-ocsp_CertGetDefaultResponder(CERTCertDBHandle *handle,CERTOCSPCertID *certID)
+ocsp_CertGetDefaultResponder(CERTCertDBHandle *handle, CERTOCSPCertID *certID)
 {
     ocspCheckingContext *ocspcx;
 
@@ -2971,18 +3059,32 @@ loser:
 }
 
 /*
- * Return true if the given signerCert is the default responder for
- * the given certID.  If not, or if any error, return false.
+ * Return true if the cert is one of the default responders configured for
+ * ocsp context. If not, or if any error, return false.
  */
 static PRBool
-ocsp_CertIsDefaultResponderForCertID(CERTCertDBHandle *handle,
-				     CERTCertificate *signerCert,
-				     CERTOCSPCertID *certID)
+ocsp_CertIsOCSPDefaultResponder(CERTCertDBHandle *handle, CERTCertificate *cert)
 {
-    CERTCertificate *defaultResponderCert;
+    ocspCheckingContext *ocspcx;
+
+    ocspcx = ocsp_GetCheckingContext(handle);
+    if (ocspcx == NULL)
+	return PR_FALSE;
+
+   /*
+    * Right now we have only one default responder.  It applies to
+    * all certs when it is used, so the check is simple and certID
+    * has no bearing on the answer.  Someday in the future we may
+    * allow configuration of different responders for different
+    * issuers, and then we would have to use the issuer specified
+    * in certID to determine if signerCert is the right one.
+    */
+    if (ocspcx->useDefaultResponder &&
+        CERT_CompareCerts(ocspcx->defaultResponderCert, cert)) {
+	return PR_TRUE;
+    }
 
-    defaultResponderCert = ocsp_CertGetDefaultResponder(handle, certID);
-    return (PRBool) (defaultResponderCert == signerCert);
+    return PR_FALSE;
 }
 
 /*
@@ -3008,67 +3110,109 @@ ocsp_AuthorizedResponderForCertID(CERTCertDBHandle *handle,
 				  CERTOCSPCertID *certID,
 				  int64 thisUpdate)
 {
-    CERTCertificate *issuerCert = NULL;
-    SECItem *issuerKeyHash = NULL;
+    CERTCertificate *issuerCert = NULL, *defRespCert;
+    SECItem *keyHash = NULL;
+    SECItem *nameHash = NULL;
     SECOidTag hashAlg;
-    PRBool okay = PR_FALSE;
+    PRBool keyHashEQ = PR_FALSE, nameHashEQ = PR_FALSE;
 
     /*
      * Check first for a trusted responder, which overrides everything else.
      */
-    if (ocsp_CertIsDefaultResponderForCertID(handle, signerCert, certID))
-	return PR_TRUE;
+    if ((defRespCert = ocsp_CertGetDefaultResponder(handle, certID)) &&
+        CERT_CompareCerts(defRespCert, signerCert)) {
+        return PR_TRUE;
+    }
 
     /*
      * In the other two cases, we need to do an issuer comparison.
      * How we do it depends on whether the signer certificate has the
      * special extension (for a designated responder) or not.
+     *
+     * First, lets check if signer of the response is the actual issuer
+     * of the cert. For that we will use signer cert key hash and cert subj
+     * name hash and will compare them with already calculated issuer key
+     * hash and issuer name hash. The hash algorithm is picked from response
+     * certID hash to avoid second hash calculation.
      */
 
-    if (ocsp_CertIsOCSPSigner(signerCert)) {
-	/*
-	 * The signer is a designated responder.  Its issuer must match
-	 * the issuer of the cert being checked.
-	 */
-	issuerCert = CERT_FindCertIssuer(signerCert, thisUpdate,
-					 certUsageAnyCA);
-	if (issuerCert == NULL) {
-	    /*
-	     * We could leave the SEC_ERROR_UNKNOWN_ISSUER error alone,
-	     * but the following will give slightly more information.
-	     * Once we have an error stack, things will be much better.
-	     */
-	    PORT_SetError(SEC_ERROR_OCSP_UNAUTHORIZED_RESPONSE);
-	    goto loser;
-	}
-    } else {
-	/*
-	 * The signer must *be* the issuer of the cert being checked.
-	 */
-	issuerCert = signerCert;
+    hashAlg = SECOID_FindOIDTag(&certID->hashAlgorithm.algorithm);
+
+    keyHash = cert_GetSPKIDigest(NULL, signerCert, hashAlg, NULL);
+    if (keyHash != NULL) {
+
+        keyHashEQ =
+            (SECITEM_CompareItem(keyHash,
+                                 &certID->issuerKeyHash) == SECEqual);
+        SECITEM_FreeItem(keyHash, PR_TRUE);
+    }
+    if (keyHashEQ &&
+        (nameHash = cert_GetSubjectNameDigest(NULL, signerCert,
+                                              hashAlg, NULL))) {
+        nameHashEQ =
+            (SECITEM_CompareItem(nameHash,
+                                 &certID->issuerNameHash) == SECEqual);
+            
+        SECITEM_FreeItem(nameHash, PR_TRUE);
+        if (nameHashEQ) {
+            /* The issuer of the cert is the the signer of the response */
+            return PR_TRUE;
+        }
     }
 
-    hashAlg = SECOID_FindOIDTag(&certID->hashAlgorithm.algorithm);
-    issuerKeyHash = CERT_SPKDigestValueForCert(NULL, issuerCert, hashAlg, NULL);
-    if (issuerKeyHash == NULL)
-	goto loser;
 
-    if (SECITEM_CompareItem(issuerKeyHash,
-			    &certID->issuerKeyHash) != SECEqual) {
-	PORT_SetError(SEC_ERROR_OCSP_UNAUTHORIZED_RESPONSE);
-	goto loser;
+    keyHashEQ = PR_FALSE;
+    nameHashEQ = PR_FALSE;
+
+    if (!ocsp_CertIsOCSPDesignatedResponder(signerCert)) {
+        PORT_SetError(SEC_ERROR_OCSP_UNAUTHORIZED_RESPONSE);
+        return PR_FALSE;
     }
 
-    okay = PR_TRUE;
+    /*
+     * The signer is a designated responder.  Its issuer must match
+     * the issuer of the cert being checked.
+     */
+    issuerCert = CERT_FindCertIssuer(signerCert, thisUpdate,
+                                     certUsageAnyCA);
+    if (issuerCert == NULL) {
+        /*
+         * We could leave the SEC_ERROR_UNKNOWN_ISSUER error alone,
+         * but the following will give slightly more information.
+         * Once we have an error stack, things will be much better.
+         */
+        PORT_SetError(SEC_ERROR_OCSP_UNAUTHORIZED_RESPONSE);
+        return PR_FALSE;
+    }
 
-loser:
-    if (issuerKeyHash != NULL)
-	SECITEM_FreeItem(issuerKeyHash, PR_TRUE);
+    keyHash = cert_GetSPKIDigest(NULL, issuerCert, hashAlg, NULL);
+    nameHash = cert_GetSubjectNameDigest(NULL, issuerCert, hashAlg, NULL);
 
-    if (issuerCert != NULL && issuerCert != signerCert)
-	CERT_DestroyCertificate(issuerCert);
+    CERT_DestroyCertificate(issuerCert);
+
+    if (keyHash != NULL && nameHash != NULL) {
+        keyHashEQ = 
+            (SECITEM_CompareItem(keyHash,
+                                 &certID->issuerKeyHash) == SECEqual);
+
+        nameHashEQ =
+            (SECITEM_CompareItem(nameHash,
+                                 &certID->issuerNameHash) == SECEqual);
+    }
+
+    if (keyHash) {
+        SECITEM_FreeItem(keyHash, PR_TRUE);
+    }
+    if (nameHash) {
+        SECITEM_FreeItem(nameHash, PR_TRUE);
+    }
+
+    if (keyHashEQ && nameHashEQ) {
+        return PR_TRUE;
+    }
 
-    return okay;
+    PORT_SetError(SEC_ERROR_OCSP_UNAUTHORIZED_RESPONSE);
+    return PR_FALSE;
 }
 
 /*
@@ -3982,6 +4126,8 @@ CERT_EnableOCSPDefaultResponder(CERTCertDBHandle *handle)
 {
     ocspCheckingContext *statusContext;
     CERTCertificate *cert;
+    SECStatus rv;
+    SECCertificateUsage usage;
 
     if (handle == NULL) {
 	PORT_SetError(SEC_ERROR_INVALID_ARGS);
@@ -4026,6 +4172,25 @@ CERT_EnableOCSPDefaultResponder(CERTCertDBHandle *handle)
     if (cert == NULL)
 	return SECFailure;
 
+   /*
+    * Supplied cert should at least have  a signing capability in order for us
+    * to use it as a trusted responder cert. Ability to sign is guarantied  if
+    * cert is validated to have any set of the usages below.
+    */
+    rv = CERT_VerifyCertificateNow(handle, cert, PR_TRUE,
+                                   certificateUsageCheckAllUsages,
+                                   NULL, &usage);
+    if (rv != SECSuccess || (usage & (certificateUsageSSLClient |
+                                      certificateUsageSSLServer |
+                                      certificateUsageSSLServerWithStepUp |
+                                      certificateUsageEmailSigner |
+                                      certificateUsageObjectSigner |
+                                      certificateUsageStatusResponder |
+                                      certificateUsageSSLCA)) == 0) {
+	PORT_SetError(SEC_ERROR_OCSP_RESPONDER_CERT_INVALID);
+	return SECFailure;
+    }
+
     /*
      * And hang onto it.
      */
@@ -4056,6 +4221,7 @@ CERT_DisableOCSPDefaultResponder(CERTCertDBHandle *handle)
 {
     CERTStatusConfig *statusConfig;
     ocspCheckingContext *statusContext;
+    CERTCertificate *tmpCert;
 
     if (handle == NULL) {
 	PORT_SetError(SEC_ERROR_INVALID_ARGS);
@@ -4071,9 +4237,10 @@ CERT_DisableOCSPDefaultResponder(CERTCertDBHandle *handle)
     if (statusContext == NULL)
 	return SECFailure;
 
-    if (statusContext->defaultResponderCert != NULL) {
-	CERT_DestroyCertificate(statusContext->defaultResponderCert);
+    tmpCert = statusContext->defaultResponderCert;
+    if (tmpCert) {
 	statusContext->defaultResponderCert = NULL;
+	CERT_DestroyCertificate(tmpCert);
     }
 
     /*
@@ -4083,75 +4250,6 @@ CERT_DisableOCSPDefaultResponder(CERTCertDBHandle *handle)
     return SECSuccess;
 }
 
-/*
- * Digest the cert's subject public key 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).
- */
-SECItem *
-CERT_SPKDigestValueForCert(PRArenaPool *arena, CERTCertificate *cert,
-                           SECOidTag digestAlg, SECItem *fill)
-{
-    const SECHashObject *digestObject;
-    void *digestContext;
-    SECItem *result = NULL;
-    void *mark = NULL;
-    SECItem spk;
-
-    if ( arena != NULL ) {
-        mark = PORT_ArenaMark(arena);
-    }
-
-    digestObject = HASH_GetHashObjectByOidTag(digestAlg);
-    if ( digestObject == NULL ) {
-        goto loser;
-    }
-
-    if ((fill == NULL) || (fill->data == NULL)) {
-	result = SECITEM_AllocItem(arena, fill, digestObject->length);
-	if ( result == NULL ) {
-	   goto loser;
-	}
-	fill = result;
-    }
-
-    /*
-     * Copy just the length and data pointer (nothing needs to be freed)
-     * of the subject public key so we can convert the length from bits
-     * to bytes, which is what the digest function expects.
-     */
-    spk = cert->subjectPublicKeyInfo.subjectPublicKey;
-    DER_ConvertBitString(&spk);
-
-    /*
-     * Now digest the value, using the specified algorithm.
-     */
-    digestContext = digestObject->create();
-    if ( digestContext == NULL ) {
-        goto loser;
-    }
-    digestObject->begin(digestContext);
-    digestObject->update(digestContext, spk.data, spk.len);
-    digestObject->end(digestContext, fill->data, &(fill->len), fill->len);
-    digestObject->destroy(digestContext, PR_TRUE);
-
-    if ( arena != NULL ) {
-        PORT_ArenaUnmark(arena, mark);
-    }
-    return(fill);
-
-loser:
-    if ( arena != NULL ) {
-        PORT_ArenaRelease(arena, mark);
-    } else {
-        if ( result != NULL ) {
-            SECITEM_FreeItem(result, (fill == NULL) ? PR_TRUE : PR_FALSE);
-        }
-    }
-    return(NULL);
-}
 
 SECStatus
 CERT_GetOCSPResponseStatus(CERTOCSPResponse *response)
diff --git a/security/nss/lib/util/secerr.h b/security/nss/lib/util/secerr.h
index d47734fe1..eca0cb352 100644
--- a/security/nss/lib/util/secerr.h
+++ b/security/nss/lib/util/secerr.h
@@ -206,6 +206,8 @@ SEC_ERROR_NO_EVENT                          = 	(SEC_ERROR_BASE + 152),
 SEC_ERROR_CRL_ALREADY_EXISTS                = 	(SEC_ERROR_BASE + 153),
 SEC_ERROR_NOT_INITIALIZED                   = 	(SEC_ERROR_BASE + 154),
 SEC_ERROR_TOKEN_NOT_LOGGED_IN               = 	(SEC_ERROR_BASE + 155),
+SEC_ERROR_OCSP_RESPONDER_CERT_INVALID      =	(SEC_ERROR_BASE + 156),
+SEC_ERROR_OCSP_BAD_SIGNATURE               =	(SEC_ERROR_BASE + 157),
 
 /* Add new error codes above here. */
 SEC_ERROR_END_OF_LIST