siemens / cmp-ra-component

A CMP Registration Authority (RA)
Apache License 2.0
3 stars 5 forks source link

unclear/misleading cert validation error in case `extraCerts` are incomplete #104

Closed DDvO closed 4 months ago

DDvO commented 5 months ago

When validating a message with signature-based protection where the protection cert is present in the extraCerts and the signature is correct but not all intermediate certs are present in the extraCerts and the CmpRaComponent cannot complete the chain (maybe from cached untrusted certs), it provides a rather unclear and actually misleading error message like:

PKIStatus: rejection
PKIFailureInfo: signerNotTrusted
StatusString: "downstream: signature check failed, protecting cert not trusted"

It should better indicate that path building/completion failed.

Akretsch commented 5 months ago

For certificate chain validation the BouncyCastle or Oracle implementation of java.security.cert.CertPathBuilder is used: final CertPathBuilder cpb = CertPathBuilder.getInstance("PKIX", PROVIDER); The java.security.cert.CertPathBuilder.build(CertPathParameters) method returns a valid chain or nothing. More detailed analysis would require the implementation of an own cert chain validation algorithm.

DDvO commented 5 months ago

Then please just make the response message sufficiently general such that it is at least not misleading, e.g. path building for validating the protection certificate failed. For the case that actually the signature check for the CMP message protection failed, there should be a different, to-the-point error.

Akretsch commented 5 months ago

fixed in 041927e6ea96bd09678aba9520cebeaf56580e48

DDvO commented 5 months ago

Thanks - yet I just noticed from the code that the error is thrown not on a failure of

final CertPathBuilder cpb = CertPathBuilder.getInstance("PKIX", PROVIDER);

like you wrote above, but on trustCredentialAdapter.validateCertAgainstTrust(protectingCert, extraCertsAsX509) == null

Therefore, please remove from the new error message the part "path building for" because the reason could also be more general, i.e., an error could happen not only during chain building but also during chain validation.

Akretsch commented 4 months ago

fixed in ded0d2b