[erlang-questions] Errors in SSL handshake (weird client)

[Ingela Andin]

Hi!

2014-04-16 0:07 GMT+02:00 Danil Zagoskin <z@REDACTED>:

> Hi, Ingela.
>
> I finally had some time for investigating this problem.
>
> I didn't see badarg in erlang:size again, but it definitely appeared in
> 17.0 with simple accept method described as minimal example at
> http://www.erlang.org/doc/apps/ssl/using_ssl.html (
>



> the difference was transport_accept and ssl_accept were run in separate
> process)
>

That  should be ok, you are suppose to be able to do that.



> Today I met some new crash:
> ** {{badmatch,{alert,2,20,{"ssl_cipher.erl",215}}},
>
> [{tls_record,decode_cipher_text,2,[{file,"tls_record.erl"},{line,157}]},
>
>  {tls_connection,next_record,1,[{file,"tls_connection.erl"},{line,503}]},
>
>  {tls_connection,next_state,4,[{file,"tls_connection.erl"},{line,475}]},
>      {gen_fsm,handle_msg,7,[{file,"gen_fsm.erl"},{line,503}]},
>      {proc_lib,init_p_do_apply,3,[{file,"proc_lib.erl"},{line,239}]}]}
>
> That's because ssl_record:decipher/3 may return #alert{}, but
> tls_record:decode_cipher_text/2 isn't expecting that.
>
>
Hum, looks like a bug. We will look into that.


>
> As for {case_clause,{4}} in ssl_v3:mac_hash/3 I found that precondition
> for crash is easily reproducible: https://gist.github.com/stolen/10780653
> Ordinary servers negotiate with sha hash or close socket while ssl in 17.0
> negotiates with sha256 hash.
> I don't know the protocol good enough to perform further states, but I
> hope this helps.
>
>
Okej, the following patch should take care of that problem.

diff --git a/lib/ssl/src/ssl.erl b/lib/ssl/src/ssl.erl
index 743753b..093d4af 100644
--- a/lib/ssl/src/ssl.erl
+++ b/lib/ssl/src/ssl.erl
@@ -953,8 +953,8 @@ handle_cipher_option(Value, Version)  when
is_list(Value) ->
     error:_->
         throw({error, {options, {ciphers, Value}}})
     end.
-binary_cipher_suites(Version, []) -> %% Defaults to all supported suits
-    ssl_cipher:suites(Version);
+binary_cipher_suites(Version, []) ->
+    [];
 binary_cipher_suites(Version, [{_,_,_,_}| _] = Ciphers0) -> %% Backwards
compatibility
     Ciphers = [{KeyExchange, Cipher, Hash} || {KeyExchange, Cipher, Hash,
_} <- Ciphers0],
     binary_cipher_suites(Version, Ciphers);
diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl
index 1108edc..93d4efd 100644
--- a/lib/ssl/src/ssl_handshake.erl
+++ b/lib/ssl/src/ssl_handshake.erl
@@ -1017,11 +1017,14 @@ decode_suites('3_bytes', Dec) ->
 %%-------------Cipeher suite handling --------------------------------

 available_suites(UserSuites, Version) ->
+    VersionSupportedSuites = ssl_cipher:suites(Version),
     case UserSuites of
     [] ->
-        ssl_cipher:suites(Version);
+        VersionSupportedSuites;
     _ ->
-        UserSuites
+        lists:filtermap(fun(Suite) ->
+                    lists:member(Suite, VersionSupportedSuites)
+                end, UserSuites)
     end.

 available_suites(ServerCert, UserSuites, Version, Curve) ->




> Unfortunately, I don't understand how this code should be tested — most of
> functions require #state{} or #connection_states{} which require
> #ssl_options{} and other stuff, and at I have found no exported function
> creating #ssl_options{} without side-effects.
>

#ssl_options{] is an internal representation of the proplists input list
and should not be a visible in the API, records are generally not a good
API choice as they enforce compile time dependencies.


Regards Ingela - Erlang/OTP team - Ericsson AB



>
> 2014-04-15 16:52 GMT+04:00 Ingela Andin <ingela.andin@REDACTED>:
>
> Hello again!
>>
>>
>>>  sometimes badarg in erlang:size([22,3,1,0,158,1,0,0,154,3,1,83,74|...])
>>> at tls_record.erl:122.
>>>
>>
>> Same symptom was recently reported on erlang-bugs and it turned out to be
>> due to upgrading a gen_tcp socket in active mode. When upgrading a gen_tcp
>> socket to an ssl socket it must be put in passive mode ({active, false})
>> before the client
>> is allowed to start the handshake.  Normaly if the upgrade is negotiated
>> this is not a problem for the server to set the option
>> before signaling to the client to go ahead with  the handshake. If the
>> upgrade is only performed on the server side instead of calling the ssl API
>> ( some people may do that due to the previously lack of possibility to
>> specify ssl options when calling ssl:ssl_accept with an "sslsocket", this
>> is no longer the case in 17.0) the listen socket needs to be put in passive
>> mode (listen options are inherited by the acceptsocket) to make sure that
>> the it will work, otherwhise it will work sometimes and sometime result in
>> the error above.
>>
>> Regards Ingela Erlang/OTP team - Ericsson AB
>>
>
>
>
> --
> Danil Zagoskin | z@REDACTED
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://erlang.org/pipermail/erlang-questions/attachments/20140416/971e916d/attachment.htm>