add support for OCSP_request_verify (#1778)

`OCSP_request_verify` is the last OCSP API needed for Ruby. OCSP request
signatures are optional according to the OCSP RFC, so this is less used
than `OCSP_basic_verify`. Much of the logic & flags are reused across
each other however. A significant portion of the logic was also reworked
without changing the behavior.
Documentation was also rewritten slightly so that it could be applied to
both OCSP verification APIs.
We don't support `OCSP_NOSIGS` for `OCSP_basic_verify` either, so I've
left that out of here for the time being.

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and the ISC license.

crypto/err/ocsp.errordata

@@ -12,6 +12,7 @@ OCSP,109,NO_REVOKED_TIME
 OCSP,130,NO_SIGNER_KEY
 OCSP,131,OCSP_REQUEST_DUPLICATE_SIGNATURE
 OCSP,110,PRIVATE_KEY_DOES_NOT_MATCH_CERTIFICATE
+OCSP,128,REQUEST_NOT_SIGNED
 OCSP,111,RESPONSE_CONTAINS_NO_REVOCATION_DATA
 OCSP,112,ROOT_CA_NOT_TRUSTED
 OCSP,115,SERVER_RESPONSE_PARSE_ERROR
@@ -23,3 +24,4 @@ OCSP,127,STATUS_TOO_OLD
 OCSP,132,UNKNOWN_FIELD_VALUE
 OCSP,119,UNKNOWN_MESSAGE_DIGEST
 OCSP,120,UNKNOWN_NID
+OCSP,129,UNSUPPORTED_REQUESTORNAME_TYPE

crypto/ocsp/ocsp_test.cc

@@ -526,38 +526,39 @@ TEST(OCSPTest, TestGoodOCSP_SHA256) {
   ASSERT_EQ(-1, X509_cmp_time(nextupd, &connection_time));
 }
 
-struct OCSPVerifyFlagTestVector {
+struct OCSPResponseVerifyFlagTestVector {
   const char *ocsp_response;
   unsigned long ocsp_verify_flag;
   bool include_expired_signer_cert;
   int expected_ocsp_verify_status;
 };
 
-static const OCSPVerifyFlagTestVector OCSPVerifyFlagTestVectors[] = {
-    {"ocsp_response", OCSP_NOINTERN, false, OCSP_VERIFYSTATUS_ERROR},
-    {"ocsp_response", OCSP_NOCHAIN, false, OCSP_VERIFYSTATUS_SUCCESS},
-    {"ocsp_response_expired_signer", OCSP_NOVERIFY, false,
-     OCSP_VERIFYSTATUS_SUCCESS},
-    {"ocsp_response_wrong_signer", OCSP_NOVERIFY, false,
-     OCSP_VERIFYSTATUS_SUCCESS},
-    {"ocsp_response", OCSP_NOEXPLICIT, false, OCSP_VERIFYSTATUS_SUCCESS},
-    {"ocsp_response", OCSP_TRUSTOTHER, false, OCSP_VERIFYSTATUS_SUCCESS},
-    {"ocsp_response_expired_signer", OCSP_TRUSTOTHER, false,
-     OCSP_VERIFYSTATUS_ERROR},
-    // |OCSP_TRUSTOTHER| sets |OCSP_NOVERIFY| if the signer cert is included
-    // within the parameters.
-    {"ocsp_response_expired_signer", OCSP_TRUSTOTHER, true,
-     OCSP_VERIFYSTATUS_SUCCESS},
+static const OCSPResponseVerifyFlagTestVector
+    OCSPResponseVerifyFlagTestVectors[] = {
+        {"ocsp_response", OCSP_NOINTERN, false, OCSP_VERIFYSTATUS_ERROR},
+        {"ocsp_response", OCSP_NOCHAIN, false, OCSP_VERIFYSTATUS_SUCCESS},
+        {"ocsp_response_expired_signer", OCSP_NOVERIFY, false,
+         OCSP_VERIFYSTATUS_SUCCESS},
+        {"ocsp_response_wrong_signer", OCSP_NOVERIFY, false,
+         OCSP_VERIFYSTATUS_SUCCESS},
+        {"ocsp_response", OCSP_NOEXPLICIT, false, OCSP_VERIFYSTATUS_SUCCESS},
+        {"ocsp_response", OCSP_TRUSTOTHER, false, OCSP_VERIFYSTATUS_SUCCESS},
+        {"ocsp_response_expired_signer", OCSP_TRUSTOTHER, false,
+         OCSP_VERIFYSTATUS_ERROR},
+        // |OCSP_TRUSTOTHER| sets |OCSP_NOVERIFY| if the signer cert is included
+        // within the parameters.
+        {"ocsp_response_expired_signer", OCSP_TRUSTOTHER, true,
+         OCSP_VERIFYSTATUS_SUCCESS},
 };
 
 class OCSPVerifyFlagTest
-    : public testing::TestWithParam<OCSPVerifyFlagTestVector> {};
+    : public testing::TestWithParam<OCSPResponseVerifyFlagTestVector> {};
 
 INSTANTIATE_TEST_SUITE_P(All, OCSPVerifyFlagTest,
-                         testing::ValuesIn(OCSPVerifyFlagTestVectors));
+                         testing::ValuesIn(OCSPResponseVerifyFlagTestVectors));
 
 TEST_P(OCSPVerifyFlagTest, OCSPVerifyFlagTest) {
-  const OCSPVerifyFlagTestVector &t = GetParam();
+  const OCSPResponseVerifyFlagTestVector &t = GetParam();
 
   std::string respData =
       GetTestData(std::string("crypto/ocsp/test/aws/" +
@@ -606,6 +607,98 @@ TEST_P(OCSPVerifyFlagTest, OCSPVerifyFlagTest) {
   ASSERT_EQ(t.expected_ocsp_verify_status, ocsp_verify_status);
 }
 
+struct OCSPRequestVerifyFlagTestVector {
+  const char *ocsp_request;
+  unsigned long ocsp_verify_flag;
+  bool include_expired_signer_cert;
+  int expected_ocsp_verify_status;
+};
+
+static const OCSPRequestVerifyFlagTestVector
+    OCSPRequestVerifyFlagTestVectors[] = {
+        // Although signatures for OCSP requests are optional according to the
+        // RFC, OCSP requests with absent signatures can't be verified.
+        {"ocsp_request", 0, false, OCSP_VERIFYSTATUS_ERROR},
+        // ocsp_request_signed's certificate is embedded in the request.
+        {"ocsp_request_signed", 0, false, OCSP_VERIFYSTATUS_SUCCESS},
+        {"ocsp_request_signed", OCSP_NOINTERN, false, OCSP_VERIFYSTATUS_ERROR},
+        {"ocsp_request_signed", OCSP_NOCHAIN, false, OCSP_VERIFYSTATUS_SUCCESS},
+        {"ocsp_request_wrong_signer", 0, false, OCSP_VERIFYSTATUS_ERROR},
+        {"ocsp_request_wrong_signer", OCSP_NOCHAIN, false,
+         OCSP_VERIFYSTATUS_ERROR},
+        // OCSP request verification will fail if the cert can't be found or
+        // when working with an expired cert.
+        {"ocsp_request_expired_signer", 0, true, OCSP_VERIFYSTATUS_ERROR},
+        {"ocsp_request_expired_signer", 0, false, OCSP_VERIFYSTATUS_ERROR},
+        {"ocsp_request_expired_signer_no_certs", 0, true,
+         OCSP_VERIFYSTATUS_ERROR},
+        {"ocsp_request_expired_signer_no_certs", 0, false,
+         OCSP_VERIFYSTATUS_ERROR},
+        // |OCSP_NOVERIFY| skips certificate verification, but can't succeed if
+        // the signer cert isn't found.
+        {"ocsp_request_expired_signer", OCSP_NOVERIFY, true,
+         OCSP_VERIFYSTATUS_SUCCESS},
+        {"ocsp_request_expired_signer", OCSP_NOVERIFY, false,
+         OCSP_VERIFYSTATUS_SUCCESS},
+        {"ocsp_request_expired_signer_no_certs", OCSP_NOVERIFY, true,
+         OCSP_VERIFYSTATUS_SUCCESS},
+        {"ocsp_request_expired_signer_no_certs", OCSP_NOVERIFY, false,
+         OCSP_VERIFYSTATUS_ERROR},
+        // |OCSP_TRUSTOTHER| sets |OCSP_NOVERIFY| if the signer cert is included
+        // within the parameters.
+        {"ocsp_request_expired_signer", OCSP_TRUSTOTHER, true,
+         OCSP_VERIFYSTATUS_ERROR},
+        {"ocsp_request_expired_signer", OCSP_TRUSTOTHER, false,
+         OCSP_VERIFYSTATUS_ERROR},
+        {"ocsp_request_expired_signer_no_certs", OCSP_TRUSTOTHER, true,
+         OCSP_VERIFYSTATUS_SUCCESS},
+        {"ocsp_request_expired_signer_no_certs", OCSP_TRUSTOTHER, false,
+         OCSP_VERIFYSTATUS_ERROR},
+};
+
+class OCSPRequestVerifyFlagTest
+    : public testing::TestWithParam<OCSPRequestVerifyFlagTestVector> {};
+
+INSTANTIATE_TEST_SUITE_P(All, OCSPRequestVerifyFlagTest,
+                         testing::ValuesIn(OCSPRequestVerifyFlagTestVectors));
+
+TEST_P(OCSPRequestVerifyFlagTest, OCSPVerifyFlagTest) {
+  const OCSPRequestVerifyFlagTestVector &t = GetParam();
+
+  std::string respData =
+      GetTestData(std::string("crypto/ocsp/test/aws/" +
+                              std::string(t.ocsp_request) + ".der")
+                      .c_str());
+  std::vector<uint8_t> ocsp_request_data(respData.begin(), respData.end());
+  bssl::UniquePtr<OCSP_REQUEST> ocsp_request =
+      LoadOCSP_REQUEST(ocsp_request_data);
+  ASSERT_TRUE(ocsp_request);
+
+  // Set up trust store and certificate chain.
+  bssl::UniquePtr<X509> ca_cert(CertFromPEM(
+      GetTestData(std::string("crypto/ocsp/test/aws/ca_cert.pem").c_str())
+          .c_str()));
+
+  bssl::UniquePtr<X509_STORE> trust_store(X509_STORE_new());
+  X509_STORE_add_cert(trust_store.get(), ca_cert.get());
+  bssl::UniquePtr<STACK_OF(X509)> server_cert_chain(sk_X509_new_null());
+  ASSERT_TRUE(server_cert_chain);
+  if (t.include_expired_signer_cert) {
+    bssl::UniquePtr<X509> signing_cert(CertFromPEM(
+        GetTestData(
+            std::string("crypto/ocsp/test/aws/ocsp_expired_cert.pem").c_str())
+            .c_str()));
+    ASSERT_TRUE(sk_X509_push(server_cert_chain.get(), signing_cert.get()));
+    X509_up_ref(signing_cert.get());
+  }
+
+  // Does basic verification on OCSP request.
+  const int ocsp_verify_status =
+      OCSP_request_verify(ocsp_request.get(), server_cert_chain.get(),
+                          trust_store.get(), t.ocsp_verify_flag);
+  ASSERT_EQ(t.expected_ocsp_verify_status, ocsp_verify_status);
+}
+
 TEST(OCSPTest, GetInfo) {
   // Create a sample |OCSP_CERTID| structure.
   bssl::UniquePtr<OCSP_CERTID> cert_id(LoadTestOCSP_CERTID());
@@ -1003,16 +1096,16 @@ TEST_P(OCSPRequestTest, OCSPRequestSign) {
   bssl::UniquePtr<OCSP_REQUEST> ocspRequest =
       LoadOCSP_REQUEST(ocsp_request_data);
 
-  bssl::UniquePtr<X509> server_cert(
+  bssl::UniquePtr<X509> signer_cert(
       CertFromPEM(GetTestData(std::string("crypto/ocsp/test/aws/" +
                                           std::string(t.signer_cert) + ".pem")
                                   .c_str())
                       .c_str()));
-  ASSERT_TRUE(server_cert);
-  bssl::UniquePtr<STACK_OF(X509)> additional_cert(CertChainFromPEM(
+  ASSERT_TRUE(signer_cert);
+  bssl::UniquePtr<X509> ca_cert(CertFromPEM(
       GetTestData(std::string("crypto/ocsp/test/aws/ca_cert.pem").c_str())
           .c_str()));
-  ASSERT_TRUE(additional_cert);
+  ASSERT_TRUE(ca_cert);
 
   if (t.expected_parse_status == OCSP_REQUEST_PARSE_SUCCESS) {
     bssl::UniquePtr<EVP_PKEY> pkey(EVP_PKEY_new());
@@ -1034,11 +1127,17 @@ TEST_P(OCSPRequestTest, OCSPRequestSign) {
       ASSERT_TRUE(EVP_PKEY_set1_EC_KEY(pkey.get(), ecdsa.get()));
     }
 
-    int ret = OCSP_request_sign(ocspRequest.get(), server_cert.get(),
-                                pkey.get(), t.dgst, additional_cert.get(), 0);
+    int ret =
+        OCSP_request_sign(ocspRequest.get(), signer_cert.get(), pkey.get(),
+                          t.dgst, CertsToStack({ca_cert.get()}).get(), 0);
     if (t.expected_sign_status == OCSP_SIGN_SUCCESS) {
       ASSERT_TRUE(ret);
       EXPECT_TRUE(OCSP_request_is_signed(ocspRequest.get()));
+      bssl::UniquePtr<X509_STORE> trust_store(X509_STORE_new());
+      ASSERT_TRUE(X509_STORE_add_cert(trust_store.get(), ca_cert.get()));
+      EXPECT_TRUE(OCSP_request_verify(ocspRequest.get(),
+                                      CertsToStack({signer_cert.get()}).get(),
+                                      trust_store.get(), 0));
     } else {
       ASSERT_FALSE(ret);
       EXPECT_FALSE(OCSP_request_is_signed(ocspRequest.get()));

crypto/ocsp/ocsp_verify.c

@@ -2,10 +2,11 @@
 // SPDX-License-Identifier: Apache-2.0 OR ISC
 
 #include <string.h>
+#include "../internal.h"
 #include "internal.h"
 
-#define SIGNER_IN_CERTSTACK 2
-#define SIGNER_IN_BASICRESP 1
+#define SIGNER_IN_PROVIDED_CERTS 2
+#define SIGNER_IN_OCSP_CERTS 1
 #define SIGNER_NOT_FOUND 0
 
 // Set up |X509_STORE_CTX| to verify signer and returns cert chain if verify is
@@ -57,15 +58,15 @@ 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_CERTSTACK;
+    return SIGNER_IN_PROVIDED_CERTS;
   }
 
   // look in certs stack the responder may have included in |OCSP_BASICRESP|,
   // unless the flags contain |OCSP_NOINTERN|.
   signer = ocsp_find_signer_sk(bs->certs, rid);
   if (signer != NULL && !IS_OCSP_FLAG_SET(flags, OCSP_NOINTERN)) {
     *psigner = signer;
-    return SIGNER_IN_BASICRESP;
+    return SIGNER_IN_OCSP_CERTS;
   }
   // Maybe lookup from store if by subject name.
 
@@ -340,8 +341,10 @@ 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_CERTSTACK) &&
+  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.
     flags |= OCSP_NOVERIFY;
   }
 
@@ -388,3 +391,109 @@ int OCSP_basic_verify(OCSP_BASICRESP *bs, STACK_OF(X509) *certs, X509_STORE *st,
   sk_X509_free(untrusted);
   return ret;
 }
+
+// ocsp_req_find_signer assigns |*psigner| to the signing certificate and
+// returns |SIGNER_IN_OCSP_CERTS| if it was found within |req| or returns
+// |SIGNER_IN_TRUSTED_CERTS| if it was found within |certs|. It returns
+// |SIGNER_NOT_FOUND| if the signing certificate is not found.
+static int ocsp_req_find_signer(X509 **psigner, OCSP_REQUEST *req,
+                                X509_NAME *nm, STACK_OF(X509) *certs,
+                                unsigned long flags) {
+  X509 *signer = NULL;
+  if (!IS_OCSP_FLAG_SET(flags, OCSP_NOINTERN)) {
+    signer = X509_find_by_subject(req->optionalSignature->certs, nm);
+    if (signer != NULL) {
+      *psigner = signer;
+      return SIGNER_IN_OCSP_CERTS;
+    }
+  }
+
+  signer = X509_find_by_subject(certs, nm);
+  if (signer != NULL) {
+    *psigner = signer;
+    return SIGNER_IN_PROVIDED_CERTS;
+  }
+  return SIGNER_NOT_FOUND;
+}
+
+int OCSP_request_verify(OCSP_REQUEST *req, STACK_OF(X509) *certs,
+                        X509_STORE *store, unsigned long flags) {
+  GUARD_PTR(req);
+  GUARD_PTR(req->tbsRequest);
+  GUARD_PTR(store);
+
+  // Check if |req| has signature to check against.
+  if (req->optionalSignature == NULL) {
+    OPENSSL_PUT_ERROR(OCSP, OCSP_R_REQUEST_NOT_SIGNED);
+    return 0;
+  }
+
+  GENERAL_NAME *gen = req->tbsRequest->requestorName;
+  if (gen == NULL || gen->type != GEN_DIRNAME) {
+    OPENSSL_PUT_ERROR(OCSP, OCSP_R_UNSUPPORTED_REQUESTORNAME_TYPE);
+    return 0;
+  }
+
+  // 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_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.
+    flags |= OCSP_NOVERIFY;
+  }
+
+  // Validate |req|'s signature.
+  EVP_PKEY *skey = X509_get0_pubkey(signer);
+  if (skey == NULL) {
+    OPENSSL_PUT_ERROR(OCSP, OCSP_R_NO_SIGNER_KEY);
+    return 0;
+  }
+  if (ASN1_item_verify(ASN1_ITEM_rptr(OCSP_REQINFO),
+                       req->optionalSignature->signatureAlgorithm,
+                       req->optionalSignature->signature, req->tbsRequest,
+                       skey) <= 0) {
+    OPENSSL_PUT_ERROR(OCSP, OCSP_R_SIGNATURE_FAILURE);
+    return 0;
+  }
+
+  // Set up |ctx| and start doing the actual verification.
+  X509_STORE_CTX *ctx = X509_STORE_CTX_new();
+  if (ctx == NULL) {
+    return 0;
+  }
+
+  // Validate the signing certificate.
+  int ret = 0;
+  if (!IS_OCSP_FLAG_SET(flags, OCSP_NOVERIFY)) {
+    // Initialize and set purpose of |ctx| for verification.
+    if (!X509_STORE_CTX_init(ctx, store, signer, NULL) &&
+        !X509_STORE_CTX_set_purpose(ctx, X509_PURPOSE_OCSP_HELPER)) {
+      OPENSSL_PUT_ERROR(OCSP, ERR_R_X509_LIB);
+      goto end;
+    }
+    if (!IS_OCSP_FLAG_SET(flags, OCSP_NOCHAIN)) {
+      X509_STORE_CTX_set_chain(ctx, req->optionalSignature->certs);
+    }
+
+    // Do the verification.
+    if (X509_verify_cert(ctx) <= 0) {
+      int err = X509_STORE_CTX_get_error(ctx);
+      OPENSSL_PUT_ERROR(OCSP, OCSP_R_CERTIFICATE_VERIFY_ERROR);
+      ERR_add_error_data(2,
+                         "Verify error:", X509_verify_cert_error_string(err));
+      goto end;
+    }
+  }
+  ret = 1;
+
+end:
+  X509_STORE_CTX_free(ctx);
+  return ret;
+}