Do not treat errors as success #66
Conversation
| validate_function(_,{bad_cert, _} = Reason, _) -> | ||
| {fail, Reason}; | ||
| validate_function(_,{extension, _}, UserState) -> | ||
| {unknown, UserState}. |
There was a problem hiding this comment.
Unless we have a style guide that says to omit them, I'd rather have a _Cert variable than _.
Also, should we still have a catch-all function that returns valid/fail/unknown/something?
There was a problem hiding this comment.
Good point about the anonymous bindings, will tweak that.
The public_key OTP library handles the real smarts here; our function just allows us to do some extra checks. By passing back what came in if we don't need to do anything, we're letting it make the real decisions. So if a failure or unknown status comes in, we'll let it decide what to do. It's the same as if we didn't supply the function at all.
See pubkey_cert:verify_fun if you'd like more context.
|
Huzzah! I have been able to now see this error in the Python client: |
|
@bkerley has successfully tested the new branch with new certificates, using c-ruby. The only other test I'd like to see is verification that the old buggy certificates are not authenticated successfully using this branch. @javajolt has, however, already tested that using Python, so at this point, Bryce, even that test is optional. I think this is good, but leaving is to Bryce to make the call. |
|
+1 4d10c83 I'm satisfied with how it works against the Ruby client. |
Do not treat errors as success Reviewed-by: bkerley
|
@borshop merge |
As discussed in #65 (at least two problems tackled there, so this does not yet suffice to close the ticket) the function sent to the OTP SSL libraries for supplemental certificate validation should fail by default, not succeed.
I have not been able to generate certificates that demonstrate the undesirable behavior for automated testing, but certificates originally created by @bkerley revealed the problem and were used to test this fix.