Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring CmsValidator (internal CRL resolution) #2635

Merged
merged 2 commits into from
Aug 16, 2021

Conversation

lucassaldanha
Copy link
Member

PR description

Loading CRL list inside CmsValidator (instead of externally)

Changelog

Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com>
@vmichalik vmichalik added the dev experience The build system, things that enable easier development etc. label Aug 15, 2021
@@ -178,4 +178,18 @@ private boolean isCertificateTrusted(
throw new RuntimeException("Error validating certificate chain", e);
}
}

private Optional<CertStore> loadCRLs(final KeyStoreWrapper truststore) {
Copy link
Contributor

@jframe jframe Aug 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't really loading CRL as such, maybe createCertStore?

@@ -178,4 +178,18 @@ private boolean isCertificateTrusted(
throw new RuntimeException("Error validating certificate chain", e);
}
}

private Optional<CertStore> loadCRLs(final KeyStoreWrapper truststore) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed there is CRLUtil that does something similar to this function, should that be used instead or this replaces that one?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRLUtil is used when loading the CRL pem file. Not for the truststore. They are similar but not the same.

Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com>
Copy link
Contributor

@jframe jframe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lucassaldanha lucassaldanha merged commit d844a37 into hyperledger:main Aug 16, 2021
@lucassaldanha lucassaldanha deleted the CmsValidatorRefactor branch August 16, 2021 08:02
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
* Refactoring CmsValidator (internal CRL resolution)

Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev experience The build system, things that enable easier development etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants