new_ssl [{verify,verify_peer}, {fail_if_no_peer_cert, true}] bug

[Essien Essien]

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>