CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 247
A69: Certificate Revocation List Enhancements #382
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
Conversation
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.
looks good, suggested a bunch of changes related to wording / moving statements around
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.
Few minor things
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.
A few more comments on top of @erm-g's. But looks good.
@markdroth, @dfawley, can y'all take a look?
The basic APIs for the CRL Reloading features. This adds external types to represent CRL Providers, CRLs, and CertificateInfo. Internally we will use `CrlImpl` - this layer is needed to hide OpenSSL details from the user. GRFC - grpc/proposal#382 Things Done * Add external API for `CrlProvider`, `Crl`, `CertInfo` (`CertInfo` is used during CRL lookup rather than passing the entire certificate). * Add code paths in `ssl_transport_security` to utilize CRL providers * Add `StaticCrlProvider` * Refactor `crl_ssl_transport_security_test.cc` so it is more extensible and can be used with providers
This adds the directory reloader implementation of the CrlProvider. This will periodically reload CRL files in a directory per [gRFC A69](grpc/proposal#382) Included in this is the following: * A public API to create the `DirectoryReloaderCrlProvider` * A basic directory interface in gprpp and platform specific impls for getting the list of files in a directory (unfortunately prior C++17, there is no std::filesystem, so we have to have platform specific impls) * The implementation of `DirectoryReloaderCrlProvider` takes an event_engine and a directory interface. This allows us to test using the fuzzing event engine for time mocking, and to implement a test directory interface so we avoid having to make temporary directories and files in the tests. This is notably not in `include`, and the `CreateDirectoryReloaderCrlProvider` is the only way to construct one from the public API, so we don't expose the event engine and directory details to the user. --------- Co-authored-by: gtcooke94 <gtcooke94@users.noreply.github.com>
) This adds the directory reloader implementation of the CrlProvider. This will periodically reload CRL files in a directory per [gRFC A69](grpc/proposal#382) Included in this is the following: * A public API to create the `DirectoryReloaderCrlProvider` * A basic directory interface in gprpp and platform specific impls for getting the list of files in a directory (unfortunately prior C++17, there is no std::filesystem, so we have to have platform specific impls) * The implementation of `DirectoryReloaderCrlProvider` takes an event_engine and a directory interface. This allows us to test using the fuzzing event engine for time mocking, and to implement a test directory interface so we avoid having to make temporary directories and files in the tests. This is notably not in `include`, and the `CreateDirectoryReloaderCrlProvider` is the only way to construct one from the public API, so we don't expose the event engine and directory details to the user. --------- Co-authored-by: gtcooke94 <gtcooke94@users.noreply.github.com>
) This adds the directory reloader implementation of the CrlProvider. This will periodically reload CRL files in a directory per [gRFC A69](grpc/proposal#382) Included in this is the following: * A public API to create the `DirectoryReloaderCrlProvider` * A basic directory interface in gprpp and platform specific impls for getting the list of files in a directory (unfortunately prior C++17, there is no std::filesystem, so we have to have platform specific impls) * The implementation of `DirectoryReloaderCrlProvider` takes an event_engine and a directory interface. This allows us to test using the fuzzing event engine for time mocking, and to implement a test directory interface so we avoid having to make temporary directories and files in the tests. This is notably not in `include`, and the `CreateDirectoryReloaderCrlProvider` is the only way to construct one from the public API, so we don't expose the event engine and directory details to the user. --------- Co-authored-by: gtcooke94 <gtcooke94@users.noreply.github.com>
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.
The overall design looks good! My comments here are mostly just details that need to be spelled out a little more clearly.
Please let me know if you have any questions. Thanks!
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.
This looks good! Remaining comments are just minor wording suggestions.
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.
Some formatting things otherwise LGTM!
Co-authored-by: Doug Fawley <dfawley@google.com>
Co-authored-by: Doug Fawley <dfawley@google.com>
Co-authored-by: Doug Fawley <dfawley@google.com>
No description provided.