Skip to content

[WIP] Fixes Issue #1508 providing more information about certificate parsing issues#1511

Closed
pjsg wants to merge 5 commits into
bcgit:mainfrom
CBitLabs:parsing
Closed

[WIP] Fixes Issue #1508 providing more information about certificate parsing issues#1511
pjsg wants to merge 5 commits into
bcgit:mainfrom
CBitLabs:parsing

Conversation

@pjsg
Copy link
Copy Markdown

@pjsg pjsg commented Oct 19, 2023

This PR adds an IllegalArgumentWarningException that is a subclass of IllegalArgumentException which is used to convey more information about certificate parsing errors. In particular, the Exception includes a list of all the errors that were encountered during the parsing, and includes (if possible) the parsed certificate object.

This change is backwards compatible.

I haven't (yet) added tests or updated the documentation -- I'll do that if this approach seems worthwhile.

@dghgit
Copy link
Copy Markdown
Contributor

dghgit commented Oct 20, 2023

For RSAKeyParameter you can set:

"org.bouncycastle.rsa.allow_unsafe_mod" to true to avoid the validations

these are things that by rights would usually result in a CVE if they were accepted. If (for other reasons) it's necessary to accept such a key, you might as well be transparent about it.

If anything on the list fails though the key should be rejected outright, it's not actually safe to use (or put another way, it's probably not worth putting together a list of what's wrong with it, it's not going to help, the key is invalid and should be rejected, so defining special handling for the exception isn't going to help...).

I'm wondering looking at this if what you're really looking for is a reporting class, something like the CertPathReviewer, which can work through a certificate and explain what's wrong with it?

@pjsg
Copy link
Copy Markdown
Author

pjsg commented Oct 26, 2023

Yes -- I'm looking for a detailed reporting class and I didn't want to duplicate all the parsing code and thereby cause maintenance problems (as they would, inevitably) get out of sync.

Do you have a suggestion on how to achieve this goal?

@dghgit
Copy link
Copy Markdown
Contributor

dghgit commented May 27, 2026

Thanks for the submission. We've taken a slightly different angle on this, have added an X509CertificateReviewer to the org.bouncycastle.cert package which will give a verbose (hopefully not too verbose) report on a certificate it is given with it's own checks a list based check on extensions and the TBS certificate. This should give space for further additions to pick up other things as well as avoiding any possible issues with classes relying on fail first/fast. We hope it will be regarded as merged with revision. Now up on https://www.bouncycastle.org/betas

@dghgit dghgit closed this May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants