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
caddytls: Make peer certificate verification pluggable #4389
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.
I like where this is going 👍
caddytls: Cleanups for ClientCertValidator changes
Regarding cla i have seen i accidently commited with wrong alias/git username :( So not sure how to sign cla. Or i need to rebase that branche changing author and create a new pull request? |
You can amend your commits to change the authorship of the commits to match the email address that's on your github account (google it, there's instructions on rewriting commits). Or, you can just add the email you committed with to your github account under https://github.com/settings/emails. |
Will circle back around to this soon, thanks for maintaining it! |
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
Hi Again, |
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.
Overall, I like the idea and the implementation. I would like to review this in more detail when I have time, but overall I'm not opposed to this approach.
We should see how #4518 plays out before committing to this, though, since I want to make sure we get client auth right.
Co-authored-by: Matt Holt <mholt@users.noreply.github.com>
Co-authored-by: Matt Holt <mholt@users.noreply.github.com>
…lidator to LeafCertClientAuth
Thanks for keeping this updated. Still interested in this change, but need to see what the outcome / resulting consensus on #4518 is... |
CI tests seem stuck on this so I'm going to try closing and reopening. |
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.
Finally getting back to this. I'll probably push a commit or three soon.
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.
Thanks so much for the enhancement, @Gr33nbl00d -- sorry it took me a few months to get around to it. Happy to merge this in!
FYI @Gr33nbl00d I spotted in the Go 1.19 release notes (draft notes, it's not out yet) that they're adding a new function |
I see but according to the method signature they use a byte array as parameter and return a complete list. |
Extension needed to fix #2341 by providing a plugin using this namespace to extend caddy with revocation checking
Needs review and will be discussed in #2341 before merge