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

CRL support tasks #57

Closed
7 tasks done
cpu opened this issue Apr 27, 2023 · 6 comments
Closed
7 tasks done

CRL support tasks #57

cpu opened this issue Apr 27, 2023 · 6 comments

Comments

@cpu
Copy link
Member

cpu commented Apr 27, 2023

A meta-issue to collect up some of the pieces I think will be needed to complete CRL support in Webpki.

For now I'm trying to limit the scope to verifying end-entity certificate revocation state (based on discussion in rustls/rustls#1164 and similar functionality in other TLS libraries), and not considering CRLs during path-building, however where possible we should leave the door open for supporting that in the future.

Auxiliary work (nice to have, not required for initial support):

@cpu
Copy link
Member Author

cpu commented Apr 27, 2023

@ctz @djc @jbr @jsha I would be interested in your input on each of these issues when you have time, and on the initial parsing support I have prepared in #44

I have a very crude prototype validating an end entity client certificate against a CRL but I'm not satisfied with the implementation. I've pulled out these issues based on my experience implementing the prototype as a way to try and make progress on some of the trickier parts.

One question I have in mind above and beyond the individual tasks is how important it is to folks to keep webpki alloc-free given Rustls only uses it with the alloc feature enabled. It makes the implementation of some details less performant and harder to wrangle (though admittedly part of that is probably my own unfamiliarity with this programming style coming mostly from garbage collected languages) .

@djc
Copy link
Member

djc commented May 16, 2023

I guess it's fine to only enable CRL support with alloc enabled. Do you expect you'll also need to "regress" existing parts of the code base to allocate, where they currently don't?

@cpu
Copy link
Member Author

cpu commented May 16, 2023

I think I've managed to push the functionality where I'd want to consider using alloc out to traits that the consumer can implement, or that we can provide behind a alloc feature flag in-crate. I haven't had to regress any existing parts after all. I think since up-front discussion of the design space hasn't been producing much engagement I'll try to get my code up for review as-is and we can iterate on the various trade offs there.

@djc
Copy link
Member

djc commented May 16, 2023

I think I've managed to push the functionality where I'd want to consider using alloc out to traits that the consumer can implement, or that we can provide behind a alloc feature flag in-crate.

Nice!

I haven't had to regress any existing parts after all. I think since up-front discussion of the design space hasn't been producing much engagement I'll try to get my code up for review as-is and we can iterate on the various trade offs there.

Right -- sorry for the lack of feedback. I did look at all the issues you filed a few weeks ago to see if there was something there to respond to, but I don't have enough context yet on either the webpki code base or the subject matter of CRL to have much pre-existing input. Hope to follow-up with reviews once you start submitting stuff, though.

@cpu
Copy link
Member Author

cpu commented May 16, 2023

Right -- sorry for the lack of feedback.

no worries, not criticism just acknowledging that I think a different strategy will work better. I appreciate your reviews :-) Once the webpki test refactoring lands I can push my next block of work that builds on #44 and #26. I could share it before then if folks want but the combined diff is pretty enormous and IMO distracting.

@cpu
Copy link
Member Author

cpu commented Jun 19, 2023

Closing this out 🎉 There are a couple of optimizations left but I think the base functionality is here.

I'm going to switch to working on getting this exposed in Rustls. Separately we should consider cutting a release when we're confident that the pieces are arranged correctly.

@cpu cpu closed this as completed Jun 19, 2023
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

No branches or pull requests

2 participants