-
Notifications
You must be signed in to change notification settings - Fork 289
Add OCSP and CRL support to certificate verify #1161
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
Add OCSP and CRL support to certificate verify #1161
Conversation
48474d3 to
7c8ddc9
Compare
|
Getting a certificate, before revoking: Revoking: Verifying with step certificate verify: |
|
And a non-revoked cert: |
7c8ddc9 to
0642c10
Compare
|
Existing CRL functionality works still.. And JSON: |
72e08bc to
2cbe6d8
Compare
|
Try verifying a cert against a user provided CRL Try verifying a cert against a user provided CRL for a different CA Try verifying a cert against a user provided OCSP server Try verifying a cert against an OCSP for a different CA |
5ff3603 to
2049858
Compare
|
Linting fixed @maraino |
2049858 to
9b45f50
Compare
maraino
left a comment
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.
In most cases, if the certificate passed as a parameter contains an intermediate certificate, we can assume this is the issuer if the --ca flag is not provided.
80b1b7c to
2a074ff
Compare
|
Updated to
For the CRL & OCSP endpoints, as long as we get a valid response from a CRL or OCSP we break the loop; whether the response indicates the certificate is good or revoked or unknown makes no difference to breaking the loop. |
|
Running against a public website Running against internal website with private CA Running against internal website with private CA after revoking |
2a074ff to
c369d92
Compare
maraino
left a comment
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.
Hi @redrac, I've added suggestion that I think solves all the linter errors, and renames the verbose flag to --verbose, I can apply those and merge, unless you see something wrong with my changes.
Add args and functionality to certificate verify to check a CRL and OCSP for a certificate based on the extensions. Users can pass flags to enable verification of each (CRL, OCSP). The command will try and get the CRL and OCSP server from the certifiacate and verify the certificate against each. I also moved functions from the crl command into internal/crlutil package so they can be re-used with the certificate verify command. Implements smallstep#845
c369d92 to
d64d33a
Compare
|
@maraino looks, good I made one small tweak to the break logic |
maraino
left a comment
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.
lgtm
|
Late to the game, but is it an option to rename the flags to shorter versions, e.g. |
Name of feature:
Add OCSP and CRL support to certificate verify
Pain or issue this feature alleviates:
Add args and functionality to certificate verify to check a CRL and OCSP for a certificate based on the extensions. Users can pass flags to enable verification of each (CRL, OCSP). The command will try and get the CRL and OCSP server from the certificate extensions if the endpoints are not provided.
Supporting links/other PRs/issues:
Implements #845
Notes
internal/crlutilso it can be reused