Client SSL Certificates #113
Conversation
Added an explicit option for SSL's `verify_mode`. In the event that a CA file or CA path are passed, default to expecting clients to have signed certs. If nothing is provided, use the preexisting behavior of not verifying clients.
|
On second thought, I'm adding a test to verify that unsigned certificates fail, which is even more important than making sure signed certs succeed. Don't yet merge. |
|
👍 |
|
Alright, good to go. |
|
|
||
| # if verify_mode isn't explicitly set, verify peers if we've | ||
| # been provided CA information that would enable us to do so | ||
| ssl_context.verify_mode = case |
There was a problem hiding this comment.
Ditto on the style here. I'd probably go with an if elsif else and assign ssl_context.verify_mode in each e.g.:
if options.include?(:verify_mode)
ssl_context.verify_mode = options[:verify_mode]
elsif options.include?(:ca_file) || options.include?(:ca_path)
...There was a problem hiding this comment.
I may have misunderstood @sconover, but I believe he was expressing appreciation for the style and not concern. :)
Personally, I feel like the tabular layout of case statements like this greatly increases readability and reduces noise. But if you prefer it with a series of elsifs, I'm happy to go with your preferred style.
There was a problem hiding this comment.
I think idiomatic Ruby style is to not indent whens after case statements, which is why I think this is kind of awkward. Although I may have misunderstood @sconover here ;)
|
lgtm with suggestion |
|
Will merge this as-is. Seems fine ;) |
Added support for verifying client SSL certs.