Fix ocsp unknown ca #4542
Fix ocsp unknown ca #4542
Conversation
|
@edewata the majority of this PR has been reviewed, so you can focus only in the few additional changes I have added to port this to master. |
| } catch (NoSuchAlgorithmException | IOException e) { | ||
| logger.info("CertificateAuthority: OCSP request hash algorithm " + digestName + " not recognised - "); | ||
| } |
There was a problem hiding this comment.
This is an unexpected exception, so I'd suggest to log the error message and the stack trace for troubleshooting, e.g.:
logger.error("CertificateAuthority: Unable to generate message digest: " + e.getMessage(), e);
Suppose we have a request with an invalid digestName, what would be the eventual OCSP response? I just want to make sure OCSP won't return a valid response (i.e. good, revoked, unknown) that can be misinterpreted by the OCSP client.
If there's such a risk, it might be better to just let the exception bubble up (i.e. rethrow the exception) and let the OCSP client handle the failure. But if it's OK to ignore, then maybe we should just generate a warning without the stack trace:
logger.warn("CertificateAuthority: Unable to generate message digest: " + e.getMessage());
There was a problem hiding this comment.
The current behaviour is to return "unknown" if the digest does not work. This is done in processRequest() method of the same class. This is acceptable for the protocol because the client should handle this case as this responder cannot validate the certificate and other responders should be invoked.
The protocol provide additional error states which were not used. As an improvement we could implement them to be more accurate in the answer but we can do in a separate PR if it is needed.
However, I think it is better to convert to warning and print the stack trace so these conditions are better identified.
base/ca/src/main/java/com/netscape/ca/CertificateAuthority.java
Outdated
Show resolved
Hide resolved
base/ca/src/main/java/com/netscape/ca/CertificateAuthority.java
Outdated
Show resolved
Hide resolved
The CA's internal OCSP fails to handle certs issued by an unknown CA. There is code in the CA's validation to handle that scenario but that validation is never triggered as the request handling code that wraps it considers not knowing the origin CA to be an error condition. The code is changed to allow the validating CA to proceed even if the origin CA is unknown, reporting Unknown for the CertStatus, while delegating to the origin CA if it is found.
In case of multiple CAs the correct one was selected using the first certificate. This could provide inconsistent. Now the selection is based on the request issuer name. Additionally, the output has been made consistent with the external OCSP for all the possibilities of subject and issuers.
a couple things: - respond with Unknown when CA can't find cert in its db - added safety net Exception to return Unknown - minor debug message when CA signs an object that's not a cert fixes https://bugzilla.redhat.com/show_bug.cgi?id=2221818
minor adjustment for Exception and debug messages.
OCSP requests have the certificate serial number, the hash of CA DN and the hash of the CA public key. According to the specification, in order to recognise a request both hashes have to match but the current implementation was verifying only the public key hash. This commit add a check on the other hash of the request.
The OCSP responder was failing if CA or CRL were not found. This has been modified so the responder will provide an `unknown` result in these cases.
fmarco76
force-pushed
the
FixOCSPUnknownCA
branch
from
f3f6e51
to
061b780
Compare
|
Kudos, SonarCloud Quality Gate passed! |
|
@edewata Thanks! |
|
Thanks for the update! |
This PR is for porting the change of #4532 and #4534 to master branch.
Additionally, the DefStore is updated with the same code of LDAPStore in order to make both stores consistent.
Finally, the new behaviour requires to modify the action workflow because of different output in some cases.