[erlang-bugs] new_ssl [{verify,verify_peer}, {fail_if_no_peer_cert, true}] bug

[Essien Essien]

You're welcome :)

On Thu, Aug 20, 2009 at 9:23 AM, Dan Gudmundsson<dgud@REDACTED> wrote:
> That sounds correct to me.
>
> I will fix that R13B02.
>
> Thanks.
>
> /Dan
>
> Essien Essien wrote:
>>
>> Hi all,
>>
>> I'm either missing something very major or there is a bug in the way
>> the new_ssl implementation in R13B-2 handles a Client's
>> CertificateMessages after it has indeed issued a CertificateRequest to
>> the connecting client or rather the _absence_ of the
>> CerfificateMessage
>>
>> The rfc requires compliant clients to compulsorily return a
>> CertificateMessage even if its a zero length on, once the server
>> issues a CertificateRequest, but the rfc also says that it is up to
>> the server to decide what to do if the client refuses to send its
>> cert.
>>
>> I believe then that the fail_if_no_peer_cert option is designed to
>> control the server's behaviour when there is no client cert, but on
>> closer inspection and testing, the erlang stack seems to ONLY properly
>> fail if the clients sends it a CertificateMessage  of zero length.
>> Normally this would not be a problem, but Mono 2.4-2 infact does just
>> this. If Mono does not find a certificate to send back to the server
>> in response to a CertificateRequest during the handshake, it simply
>> does not send any CertificateMessage request at all, not even one with
>> a zero length, hence clearly breaking the rfc.
>>
>> This behaviour currently throws the erlang new_ssl implementation for
>> a loop and the gen_fsm used by new_ssl just silently moves to the
>> 'cipher' state, where it expects and processes a ClientKeyExchange
>> message, effectively allowing the erring client to make it through.
>>
>>
>> In summary, the only valid change from 'certify' to 'cipher' for a
>> server should be:
>>
>> 1. role = server, current_state = certify, verify = verify_peer,
>> fail_if_no_peer_cert = false, message = client_key_exchange:
>>          next_state = cipher
>>
>> 2. role = server, current_state = certify, verify = verify_peer,
>> fail_if_no_peer_cert = false, message = certificate
>>          next_state = cipher
>>
>>
>> 3. role = server, current_state = certify, verify = verify_peer,
>> fail_if_no_peer_cert = true, message = certificate{length > 0}
>>          next_state = cipher
>>
>> while the following states should produce an error:
>>
>> 4. role = server, current_state = certify, verify = verify_peer,
>> fail_if_no_peer_cert = true, message = client_key_exchange:
>>          next_state = ERROR
>>
>> 5. role = server, current_state = certify, verify = verify_peer,
>> fail_if_no_peer_cert = true, message = certificate{length = 0}
>>          next_state = ERROR
>>
>>
>> Of all the rules above, only rule 4 is not handled by new_ssl in
>> R13B-2 and the following patch (which is just a quick and dirty proof
>> of concept) properly kills the connection if the client in the event
>> that rule 4 occurs.
>>
>> <patch>
>>
>> --- /usr/lib/erlang/lib/ssl-3.10.1/src/ssl_connection.erl       2009-04-27
>> 07:05:47.000000000 +0100
>> +++ ssl-3.10.1/src/ssl_connection.erl   2009-08-07 13:09:02.142360183
>> +0100
>> @@ -477,6 +477,11 @@
>>            {stop, normal, State0}
>>     end;
>>
>> +certify(#client_key_exchange{}, State =
>> +        #state{role = server, ssl_options = #ssl_options{verify =
>> verify_peer,
>> +                fail_if_no_peer_cert = true}}) ->
>> +    {stop, normal, State};
>> +
>>  certify(#client_key_exchange{exchange_keys
>>                             = #encrypted_premaster_secret{premaster_secret
>>                                                           = EncPMS}},
>>
>>
>> </patch>
>>
>> ________________________________________________________________
>> erlang-bugs mailing list. See http://www.erlang.org/faq.html
>> erlang-bugs (at) erlang.org
>>
>