-
Notifications
You must be signed in to change notification settings - Fork 1.7k
verification: add skeleton for revocation checking #14501
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
Open
tnytown
wants to merge
16
commits into
pyca:main
Choose a base branch
from
trail-of-forks:tnytown/crl-skeleton
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
a521276
verification: add skeleton for revocation checking
tnytown 280da61
test CRLRevocationChecker with limbo
tnytown 533f582
reintroduce trait object, Python ABC
tnytown 98bd2db
fix circular import
tnytown dfe51b6
correctly export PyRevocationChecker
tnytown e5e4ba9
hook revocation up to verification
tnytown c9863a5
more limbo harness hax
tnytown 0b64bb5
rework error states for Python revocation checkers
tnytown 9d1e5a9
get rid of limbo CRL tests for now
tnytown c8fd39d
oops
tnytown 5342e5c
accept args on subclasses of PyRevocationChecker
tnytown 530c3fd
fixup tests
tnytown aed6887
fixup tests
tnytown 33412e0
fixup tests
tnytown a9ff050
exercise CRL checker in verification tests
tnytown f2d21ef
rearrange tests for coverage
tnytown File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| // This file is dual licensed under the terms of the Apache License, Version | ||
| // 2.0, and the BSD License. See the LICENSE file in the root of this repository | ||
| // for complete details. | ||
|
|
||
| use cryptography_x509::crl::CertificateRevocationList; | ||
|
|
||
| use crate::{ | ||
| ops::{CryptoOps, VerificationCertificate}, | ||
| policy::Policy, | ||
| ValidationResult, | ||
| }; | ||
|
|
||
| trait CheckRevocation<B: CryptoOps> { | ||
| fn is_revoked( | ||
| &self, | ||
| cert: &VerificationCertificate<'_, B>, | ||
| issuer: &VerificationCertificate<'_, B>, | ||
| policy: &Policy<'_, B>, | ||
| ) -> ValidationResult<'_, bool, B>; | ||
| } | ||
|
|
||
| pub struct CrlRevocationChecker<'a> { | ||
| crls: Vec<&'a CertificateRevocationList<'a>>, | ||
| } | ||
|
|
||
| impl<'a, B: CryptoOps> CheckRevocation<B> for CrlRevocationChecker<'a> { | ||
| fn is_revoked( | ||
| &self, | ||
| cert: &VerificationCertificate<'_, B>, | ||
| issuer: &VerificationCertificate<'_, B>, | ||
| policy: &Policy<'_, B>, | ||
| ) -> ValidationResult<'_, bool, B> { | ||
| let _crls = &self.crls; | ||
| let _cert = cert; | ||
| let _issuer = issuer; | ||
| let _policy = policy; | ||
|
|
||
| Ok(false) | ||
| } | ||
| } | ||
|
|
||
| impl<'a> CrlRevocationChecker<'a> { | ||
| pub fn new(crls: impl IntoIterator<Item = &'a CertificateRevocationList<'a>>) -> Self { | ||
| Self { | ||
| crls: crls.into_iter().collect(), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Wrapper for revocation checkers that dispatches `is_revoked` calls to the inner implementation. | ||
| /// This allows us to avoid trait object-based polymorphism in the verifier. | ||
| pub enum RevocationChecker<'a> { | ||
| Crl(&'a CrlRevocationChecker<'a>), | ||
| } | ||
|
|
||
| impl RevocationChecker<'_> { | ||
| /// Checks the revocation status of the leaf of the provided chain. | ||
| pub fn is_revoked<B: CryptoOps>( | ||
| &self, | ||
| cert: &VerificationCertificate<'_, B>, | ||
| issuer: &VerificationCertificate<'_, B>, | ||
| policy: &Policy<'_, B>, | ||
| ) -> ValidationResult<'_, bool, B> { | ||
| match self { | ||
| RevocationChecker::Crl(c) => c.is_revoked(cert, issuer, policy), | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So right now the idea is that we won't actually have a Python API you can implement for revocation checking, it'll be handled internally and these are more markers, is that the idea?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, sorry that this isn't more clear—I experimented with having an extensible Python API but it was a bit complicated to shim over considering that the implementation for CRL is going to be in Rust. I can sketch out a shim if you'd like?see latest diff which supports Python-extensible revocation checkers