From 41ba3db66c2b335a3cfa33745c3ba5c4a8fc6d4b Mon Sep 17 00:00:00 2001
From: samuel40791765 <sachiang@amazon.com>
Date: Thu, 15 Aug 2024 00:30:54 +0000
Subject: [PATCH] PR comments; add check and get rid of magic number

---
 crypto/ocsp/internal.h    | 5 +++++
 crypto/ocsp/ocsp_server.c | 3 ++-
 crypto/ocsp/ocsp_test.cc  | 5 +++--
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/crypto/ocsp/internal.h b/crypto/ocsp/internal.h
index f371e5b8df6..15a1ef27920 100644
--- 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
index acc6e4a9427..a2081872b61 100644
--- 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
index 5979d795a2f..e4258ea26f2 100644
--- 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,