Improve exception handling for OCSP validation

dogtagpki · Aug 16, 2023 · f3f6e51 · f3f6e51
1 parent 62ca334
commit f3f6e51
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 9 deletions.
diff --git a/.github/workflows/ocsp-crl-ldap-test.yml b/.github/workflows/ocsp-crl-ldap-test.yml
@@ -332,7 +332,7 @@ jobs:
               -c ca_signing \
               --serial $CERT_ID | tee output
 
-          # the responder does not provide valid answer
+          # the responder should return "Unknown"
           sed -n "s/^CertStatus=\(.*\)$/\1/p" output > actual
           echo "Unknown" > expected
           diff expected actual
@@ -347,7 +347,7 @@ jobs:
           # remove file names and line numbers so it can be compared
           sed -n "s/^$CERT_ID:\s*\(\S*\)$/\1/p" output > actual
 
-          # the responder does not provide valid answer
+          # the responder should return "unknown"
           echo "unknown" > expected
 
           diff expected actual

diff --git a/base/ca/src/main/java/com/netscape/ca/CertificateAuthority.java b/base/ca/src/main/java/com/netscape/ca/CertificateAuthority.java
@@ -1389,7 +1389,7 @@ public OCSPResponse validate(OCSPRequest request)
                     MessageDigest md = MessageDigest.getInstance(digestName);
                     nameHash = md.digest(ocspCA.getSubjectObj().getX500Name().getEncoded());
                 } catch (NoSuchAlgorithmException | IOException e) {
-                    logger.info("CertificateAuthority: OCSP request hash algorithm " + digestName + " not recognised - ");
+                    logger.warn("CertificateAuthority: OCSP request hash algorithm " + digestName + " not recognised: " + e.getMessage(), e);
                 }
             }
             if(Arrays.equals(nameHash, cid.getIssuerNameHash().toByteArray())) {
@@ -1583,6 +1583,7 @@ public SingleResponse processRequest(Request req) {
                 nameHash = md.digest(mName.getEncoded());
                 keyHash = md.digest(key.getKey());
             } catch (NoSuchAlgorithmException | IOException e) {
+                logger.warn("CertificateAuthority: OCSP request hash algorithm " + digestName + " not recognised: " + e.getMessage(), e);
             }
         }
         if (!Arrays.equals(cid.getIssuerNameHash().toByteArray(), nameHash) ||
@@ -1666,11 +1667,11 @@ public SingleResponse processRequest(Request req) {
                 certStatus = new UnknownInfo();
             }
         } catch (EDBRecordNotFoundException e) {
-            logger.debug(name + "cert record not found");
+            logger.debug("{} cert record not found", name);
             certStatus = new UnknownInfo(); // not issued by this CA
         } catch (Exception e) {
             // internal error
-            logger.debug(name + "failed on certificateRepository.readCertificateRecord " + e.toString());
+            logger.error(name + " Unable to retrieve certificate record: " + e.getMessage(), e);
             certStatus = new UnknownInfo();
         }
 

diff --git a/base/ocsp/src/main/java/com/netscape/cms/ocsp/DefStore.java b/base/ocsp/src/main/java/com/netscape/cms/ocsp/DefStore.java
@@ -346,7 +346,7 @@ public SingleResponse processRequest(Request req) throws Exception {
                 X509Key key = (X509Key) cert.getPublicKey();
 
                 byte[] digest = md.digest(key.getKey());
-                logger.info("DefStore:   Digest: " + new String(Hex.encodeHex(digest)));
+                logger.info("DefStore:   Digest: {}", new String(Hex.encodeHex(digest)));
                 byte[] name = md.digest(cert.getSubjectObj().getX500Name().getEncoded());
 
                 if (!Arrays.equals(digest, keyhsh) && Arrays.equals(name, namehash)) {
@@ -379,7 +379,7 @@ public SingleResponse processRequest(Request req) throws Exception {
                     logger.debug("DefStore: using crl cache");
                 }
 
-                logger.info("DefStore: Adding CRL issuing point container for " + new String(Hex.encodeHex(digest)));
+                logger.info("DefStore: Adding CRL issuing point container for {}", new String(Hex.encodeHex(digest)));
                 mCacheCRLIssuingPoints.put(new String(digest), new CRLIPContainer(theRec, theCert, theCRL));
                 break;
             }
@@ -394,7 +394,7 @@ public SingleResponse processRequest(Request req) throws Exception {
         logger.info("DefStore: Issuer: " + theCert);
 
         if (theCert == null) {
-            logger.info("Missing issuer certificate");
+            logger.warn("Missing issuer certificate");
             // Unknown cert so respond with unknown state
             return new SingleResponse(cid, new UnknownInfo(), new GeneralizedTime(new Date()), null);
         }

diff --git a/base/ocsp/src/main/java/com/netscape/cms/ocsp/LDAPStore.java b/base/ocsp/src/main/java/com/netscape/cms/ocsp/LDAPStore.java
@@ -443,7 +443,7 @@ public SingleResponse processRequest(Request req) throws Exception {
         }
 
         if (theCert == null) {
-            logger.info("Missing issuer certificate");
+            logger.warn("Missing issuer certificate");
             // Unknown cert so respond with unknown state
             return new SingleResponse(cid, new UnknownInfo(), new GeneralizedTime(new Date()), null);
         }