-
Notifications
You must be signed in to change notification settings - Fork 45
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 CRL checking for client certificate #39
Conversation
If the client provides a certificate, make sure it is actually valid AND not revoked.
Oh, for this to actually work, you'll need to patch public_key: |
fetch(Rest); | ||
fetch([{uniformResourceIdentifier, "http"++_=URL}|Rest]) -> | ||
lager:debug("getting CRL from ~p~n", [URL]), | ||
inets:start(), |
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.
Do we have to worry about an error being returned here because it's already been started?
I don't mean to be overly pedantic here, but adding some docstrings to the functions would go a long way to being able to revisit this code later and know why it does certain things. |
%% XXX the 'Issuer' we get passed here is actually a lie, public key treats the Authority Key Identifier as the 'issuer' | ||
%% Read the CA certs out of the file | ||
%{ok, Bin} = file:read_file(CACerts), | ||
%CACerts = load_certs("/home/andrew/lavabit"), |
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.
These commented lines probably shouldn't be here.
Add comments; verify clean Dialyzer, test against Riak Test, and document SSL functions.
👍 once #40 is merged. Should address most of my comments in this PR. |
Add CRL checking for client certificate
If the client provides a certificate, make sure it is actually valid AND
not revoked.
See also basho/riak_test#416