PR comments; add check and get rid of magic number

aws · Aug 15, 2024 · 41ba3db · 41ba3db
1 parent b709724
commit 41ba3db
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 3 deletions.
diff --git a/crypto/ocsp/internal.h b/crypto/ocsp/internal.h
@@ -14,6 +14,11 @@
 extern "C" {
 #endif
 
+// CRLReason does not have a status assigned to the value 7.
+//
+// See Reason Code RFC: https://www.rfc-editor.org/rfc/rfc5280#section-5.3.1.
+#define OCSP_UNASSIGNED_REVOKED_STATUS 7
+
 // OCSP Request ASN.1 specification:
 // https://datatracker.ietf.org/doc/html/rfc6960#section-4.1.1
 //

diff --git a/crypto/ocsp/ocsp_server.c b/crypto/ocsp/ocsp_server.c
@@ -77,6 +77,7 @@ OCSP_SINGLERESP *OCSP_basic_add1_status(OCSP_BASICRESP *resp, OCSP_CERTID *cid,
                                         ASN1_TIME *this_update,
                                         ASN1_TIME *next_update) {
   GUARD_PTR(resp);
+  GUARD_PTR(resp->tbsResponseData);
   GUARD_PTR(cid);
   GUARD_PTR(this_update);
   // Ambiguous status values are not allowed.
@@ -138,7 +139,7 @@ OCSP_SINGLERESP *OCSP_basic_add1_status(OCSP_BASICRESP *resp, OCSP_CERTID *cid,
       // valid reason codes are 0-10. Value 7 is not used.
       if (revoked_reason < OCSP_REVOKED_STATUS_UNSPECIFIED ||
           revoked_reason > OCSP_REVOKED_STATUS_AACOMPROMISE ||
-          revoked_reason == 7) {
+          revoked_reason == OCSP_UNASSIGNED_REVOKED_STATUS) {
         OPENSSL_PUT_ERROR(OCSP, OCSP_R_UNKNOWN_FIELD_VALUE);
         goto err;
       }

diff --git a/crypto/ocsp/ocsp_test.cc b/crypto/ocsp/ocsp_test.cc
@@ -675,8 +675,9 @@ TEST(OCSPTest, BasicAddStatus) {
 
   // Try setting a revoked response with an invalid revoked reason number.
   EXPECT_FALSE(OCSP_basic_add1_status(
-      basicResponse.get(), certId.get(), V_OCSP_CERTSTATUS_REVOKED, 7,
-      revoked_time.get(), this_update.get(), nullptr));
+      basicResponse.get(), certId.get(), V_OCSP_CERTSTATUS_REVOKED,
+      OCSP_UNASSIGNED_REVOKED_STATUS, revoked_time.get(), this_update.get(),
+      nullptr));
 
   EXPECT_TRUE(OCSP_basic_add1_status(
       basicResponse.get(), certId.get(), V_OCSP_CERTSTATUS_REVOKED,