PR comments; documentation and better var names

crypto/ocsp/ocsp_verify.c

@@ -5,7 +5,7 @@
 #include "../internal.h"
 #include "internal.h"
 
-#define SIGNER_IN_TRUSTED_CERTS 2
+#define SIGNER_IN_PROVIDED_CERTS 2
 #define SIGNER_IN_OCSP_CERTS 1
 #define SIGNER_NOT_FOUND 0
 
@@ -58,7 +58,7 @@ static int ocsp_find_signer(X509 **psigner, OCSP_BASICRESP *bs,
   signer = ocsp_find_signer_sk(certs, rid);
   if (signer != NULL) {
     *psigner = signer;
-    return SIGNER_IN_TRUSTED_CERTS;
+    return SIGNER_IN_PROVIDED_CERTS;
   }
 
   // look in certs stack the responder may have included in |OCSP_BASICRESP|,
@@ -341,7 +341,7 @@ int OCSP_basic_verify(OCSP_BASICRESP *bs, STACK_OF(X509) *certs, X509_STORE *st,
     OPENSSL_PUT_ERROR(OCSP, OCSP_R_SIGNER_CERTIFICATE_NOT_FOUND);
     goto end;
   }
-  if ((ret == SIGNER_IN_TRUSTED_CERTS) &&
+  if ((ret == SIGNER_IN_PROVIDED_CERTS) &&
       IS_OCSP_FLAG_SET(flags, OCSP_TRUSTOTHER)) {
     // We skip verification if the flag to trust |certs| is set and the signer
     // is found within that stack.
@@ -411,7 +411,7 @@ static int ocsp_req_find_signer(X509 **psigner, OCSP_REQUEST *req,
   signer = X509_find_by_subject(certs, nm);
   if (signer != NULL) {
     *psigner = signer;
-    return SIGNER_IN_TRUSTED_CERTS;
+    return SIGNER_IN_PROVIDED_CERTS;
   }
   return SIGNER_NOT_FOUND;
 }
@@ -434,15 +434,15 @@ int OCSP_request_verify(OCSP_REQUEST *req, STACK_OF(X509) *certs,
     return 0;
   }
 
-  // Find |signer| from |certs| against criteria.
+  // Find |signer| from |certs| or |req->optionalSignature->certs| against criteria.
   X509 *signer = NULL;
   int signer_status =
       ocsp_req_find_signer(&signer, req, gen->d.directoryName, certs, flags);
   if (signer_status <= SIGNER_NOT_FOUND || signer == NULL) {
     OPENSSL_PUT_ERROR(OCSP, OCSP_R_SIGNER_CERTIFICATE_NOT_FOUND);
     return 0;
   }
-  if (signer_status == SIGNER_IN_TRUSTED_CERTS &&
+  if (signer_status == SIGNER_IN_PROVIDED_CERTS &&
       IS_OCSP_FLAG_SET(flags, OCSP_TRUSTOTHER)) {
     // We skip certificate verification if the flag to trust |certs| is set and
     // the signer is found within that stack.

include/openssl/ocsp.h

@@ -58,8 +58,11 @@ extern "C" {
 // Certificates included within |bs| or |req| will be included in the
 // search for the signing certificate by default, unless |OCSP_NOINTERN| is set.
 #define OCSP_NOINTERN 0x2
-// OCSP_NOCHAIN is for |OCSP_basic_verify| and |OCSP_request_verify|. All
-// certificates in both |certs| and in |bs|/|req| are considered as untrusted
+// OCSP_NOCHAIN is for |OCSP_basic_verify| and |OCSP_request_verify|.
+// For |OCSP_basic_verify|, certificates in both |certs| and in |bs| are
+// considered as certificates for the construction of the validation path for
+// the signer certificate by default, unless |OCSP_NOCHAIN| is set.
+// For |OCSP_request_verify|, certificates in |req| are considered as
 // certificates for the construction of the validation path for the signer
 // certificate by default, unless |OCSP_NOCHAIN| is set.
 #define OCSP_NOCHAIN 0x8
@@ -352,15 +355,20 @@ OPENSSL_EXPORT int OCSP_check_validity(ASN1_GENERALIZEDTIME *thisUpdate,
 // Returns 1 if the response is valid, 0 if the signature cannot be verified,
 // or -1 on fatal errors such as malloc failure.
 //
-// Note: 1. Checks that OCSP response CAN be verified, not that it has been
-//          verified.
+// Note: 1. Checks that OCSP response CAN be verified, but does not imply
+//          anything about the corresponding certificate's revocation status.
 //       2. |OCSP_resp_find_status| should be used to check if the OCSP
 //          response's cert status is |V_OCSP_CERTSTATUS_GOOD|.
 //          |OCSP_check_validity| should also be used to validate that the OCSP
 //          response's timestamps are correct.
 OPENSSL_EXPORT int OCSP_basic_verify(OCSP_BASICRESP *bs, STACK_OF(X509) *certs,
                                      X509_STORE *st, unsigned long flags);
 
+// OCSP_request_verify verifies the OCSP request message, |req|, with |st|.
+// OCSP request signatures are optional according to RFC6960, but one can check
+// that |req| is correctly signed and that the signer certificate can be
+// validated if a signature exists. This returns 1 if |req| is valid or returns
+// 0 if |req|'s signature is non-existent or cannot be verified.
 OPENSSL_EXPORT int OCSP_request_verify(OCSP_REQUEST *req, STACK_OF(X509) *certs,
                                        X509_STORE *st, unsigned long flags);