add support for OCSP_request_verify

aws · Aug 21, 2024 · ff19094 · ff19094
1 parent 3c69611
commit ff19094
Show file tree

Hide file tree

Showing 11 changed files with 1,427 additions and 1,166 deletions.
diff --git a/crypto/err/ocsp.errordata b/crypto/err/ocsp.errordata
@@ -11,6 +11,7 @@ OCSP,108,NO_RESPONSE_DATA
 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
@@ -21,3 +22,4 @@ OCSP,126,STATUS_NOT_YET_VALID
 OCSP,127,STATUS_TOO_OLD
 OCSP,119,UNKNOWN_MESSAGE_DIGEST
 OCSP,120,UNKNOWN_NID
+OCSP,129,UNSUPPORTED_REQUESTORNAME_TYPE
diff --git a/crypto/ocsp/ocsp_test.cc b/crypto/ocsp/ocsp_test.cc
@@ -517,38 +517,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/" +
@@ -597,6 +598,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) {
   bssl::UniquePtr<X509> issuer(CertFromPEM(
       GetTestData(std::string("crypto/ocsp/test/aws/ca_cert.pem").c_str())
@@ -909,16 +1002,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());
@@ -940,11 +1033,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()));

diff --git a/crypto/ocsp/ocsp_verify.c b/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_TRUSTED_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_TRUSTED_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_TRUSTED_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_TRUSTED_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| 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 &&
+      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;
+}
diff --git a/crypto/ocsp/test/aws/OCSP-TEST.md b/crypto/ocsp/test/aws/OCSP-TEST.md
@@ -55,6 +55,18 @@ openssl ocsp -sha256 -issuer ca_cert.pem -cert rsa_cert.pem -signer ocsp_cert.pe
 ```
 openssl ocsp -issuer ca_cert.pem -cert rsa_cert.pem -signer ocsp_cert.pem -signkey ocsp_key.pem -sign_other ca_cert.pem -reqout ocsp_request_attached_cert.der
 ```
+## Generating a OCSP request signed by the wrong signer
+```
+openssl ocsp -issuer ca_cert.pem -cert server_cert.pem -signer server_ecdsa_cert.pem -signkey server_ecdsa_key.pem -no_certs -reqout ocsp_request_wrong_signer.der
+```
+
+## Generating a OCSP request signed by an expired signer
+```
+openssl ocsp -issuer ca_cert.pem -cert server_cert.pem -signer ocsp_expired_cert.pem -signkey ocsp_key.pem -reqout ocsp_request_expired_signer.der
+```
+```
+openssl ocsp -issuer ca_cert.pem -cert server_cert.pem -signer ocsp_expired_cert.pem -signkey ocsp_key.pem -no_certs -reqout ocsp_request_expired_signer_no_certs.der
+```
 
 ## Generating a new OCSP response for the leaf cert
 The current OCSP responses expire in 10 years. Our tests using these files only check if the timefield value has been 

diff --git a/crypto/ocsp/test/aws/ocsp_request_expired_signer.der b/crypto/ocsp/test/aws/ocsp_request_expired_signer.der
diff --git a/crypto/ocsp/test/aws/ocsp_request_expired_signer_no_certs.der b/crypto/ocsp/test/aws/ocsp_request_expired_signer_no_certs.der
diff --git a/crypto/ocsp/test/aws/ocsp_request_wrong_signer.der b/crypto/ocsp/test/aws/ocsp_request_wrong_signer.der