<div dir="ltr"><div>Thank you for the patch. I'll check it. (Style question: is <span style="font-family:arial,sans-serif;font-size:13px">lists:filtermap better than list comprehension?)</span></div><div><br></div>> <span style="font-family:arial,sans-serif;font-size:13px">#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.   </span><span style="font-family:arial,sans-serif;font-size:13px"> </span><div style="font-family:arial,sans-serif;font-size:13px">

<br></div><div style="font-family:arial,sans-serif;font-size:13px">Yes, I understand that. I wanted to start broken client to see how server would work with it.</div><div style="font-family:arial,sans-serif;font-size:13px">

The option to do that could be using sys:replace_state, but as far as I understand the code, client_hello is sent immediately after starting a process, so it seems there is no way to send crafted hello and continue handshake with couple of lines instead of ssl:connect.</div>

<div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">Given that possibility it could be much easier to reproduce server crashes when working with buggy clients.</div>

<div><font face="arial, sans-serif">For example, How do I write a test for badmatch in </font><span style="font-family:arial,sans-serif;font-size:13px;color:rgb(80,0,80)">tls_record:decode_cipher_text? The function takes </span><font color="#500050" face="arial, sans-serif">#connection_states{}, and creating that record relies on </font><span style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:13px">#ssl_options{} which cannot be created without opening a socket and calling gen_fsm:enter_loop.</span></div>

<div style="font-family:arial,sans-serif;font-size:13px"><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">2014-04-16 17:07 GMT+04:00 Ingela Andin <span dir="ltr"><<a href="mailto:ingela.andin@gmail.com" target="_blank">ingela.andin@gmail.com</a>></span>:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi!<br><div><div class="gmail_extra"><br><div class="gmail_quote">2014-04-16 0:07 GMT+02:00 Danil Zagoskin <span dir="ltr"><<a href="mailto:z@gosk.in" target="_blank">z@gosk.in</a>></span>:<div class="">

<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr">Hi, Ingela.<div><br></div><div>I finally had some time for investigating this problem.</div><div><br></div><div>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 <a href="http://www.erlang.org/doc/apps/ssl/using_ssl.html" target="_blank">http://www.erlang.org/doc/apps/ssl/using_ssl.html</a> (</div>


</div></blockquote><div><br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>the difference was transport_accept and ssl_accept were run in separate process)</div>




<div></div></div></blockquote><div><br></div></div><div>That  should be ok, you are suppose to be able to do that. <br></div><div class=""><div><br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">


<div dir="ltr"><div>Today I met some new crash:</div><div><div>** {{badmatch,{alert,2,20,{"ssl_cipher.erl",215}}},</div><div>    [{tls_record,decode_cipher_text,2,[{file,"tls_record.erl"},{line,157}]},</div>




<div>     {tls_connection,next_record,1,[{file,"tls_connection.erl"},{line,503}]},</div><div>     {tls_connection,next_state,4,[{file,"tls_connection.erl"},{line,475}]},</div><div>     {gen_fsm,handle_msg,7,[{file,"gen_fsm.erl"},{line,503}]},</div>




<div>     {proc_lib,init_p_do_apply,3,[{file,"proc_lib.erl"},{line,239}]}]}</div></div><div><br></div><div>That's because ssl_record:decipher/3 may return #alert{}, but tls_record:decode_cipher_text/2 isn't expecting that.</div>




<div><br></div></div></blockquote><div><br></div></div><div>Hum, looks like a bug. We will look into that. <br></div><div class=""><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">


<div dir="ltr"><div></div><div><br></div><div>As for {case_clause,{4}} in ssl_v3:mac_hash/3 I found that precondition for crash is easily reproducible: <a href="https://gist.github.com/stolen/10780653" target="_blank">https://gist.github.com/stolen/10780653</a></div>




<div>Ordinary servers negotiate with sha hash or close socket while ssl in 17.0 negotiates with sha256 hash.</div><div>I don't know the protocol good enough to perform further states, but I hope this helps.</div><div>




<br></div></div></blockquote><div><br></div></div><div>Okej, the following patch should take care of that problem. <br></div><div><br>diff --git a/lib/ssl/src/ssl.erl b/lib/ssl/src/ssl.erl<br>index 743753b..093d4af 100644<br>

--- a/lib/ssl/src/ssl.erl<br>
+++ b/lib/ssl/src/ssl.erl<br>@@ -953,8 +953,8 @@ handle_cipher_option(Value, Version)  when is_list(Value) -><br>     error:_-><br>         throw({error, {options, {ciphers, Value}}})<br>     end.<br>-binary_cipher_suites(Version, []) -> %% Defaults to all supported suits<br>


-    ssl_cipher:suites(Version);<br>+binary_cipher_suites(Version, []) -> <br>+    [];<br> binary_cipher_suites(Version, [{_,_,_,_}| _] = Ciphers0) -> %% Backwards compatibility<br>     Ciphers = [{KeyExchange, Cipher, Hash} || {KeyExchange, Cipher, Hash, _} <- Ciphers0],<br>


     binary_cipher_suites(Version, Ciphers);<br>diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl<br>index 1108edc..93d4efd 100644<br>--- a/lib/ssl/src/ssl_handshake.erl<br>+++ b/lib/ssl/src/ssl_handshake.erl<br>


@@ -1017,11 +1017,14 @@ decode_suites('3_bytes', Dec) -><br> %%-------------Cipeher suite handling --------------------------------<br> <br> available_suites(UserSuites, Version) -><br>+    VersionSupportedSuites = ssl_cipher:suites(Version),<br>


     case UserSuites of<br>     [] -><br>-        ssl_cipher:suites(Version);<br>+        VersionSupportedSuites;<br>     _ -><br>-        UserSuites<br>+        lists:filtermap(fun(Suite) -><br>+                    lists:member(Suite, VersionSupportedSuites)<br>


+                end, UserSuites) <br>     end.<br> <br> available_suites(ServerCert, UserSuites, Version, Curve) -><br><br><br> </div><div class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">


<div dir="ltr"><div></div><div>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.</div>


</div></blockquote><div><br></div></div><div>#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.    <br>


</div><div><br><br></div><div>Regards Ingela - Erlang/OTP team - Ericsson AB <br></div><div class=""><div><br><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">


<div class="gmail_extra"><br><br><div class="gmail_quote">2014-04-15 16:52 GMT+04:00 Ingela Andin <span dir="ltr"><<a href="mailto:ingela.andin@gmail.com" target="_blank">ingela.andin@gmail.com</a>></span>:<div><div>


<br>

<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hello again!<br><div><div><div class="gmail_extra"> <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">




<div dir="ltr">
sometimes badarg in erlang:size([22,3,1,0,158,1,0,0,154,3,1,83,74|...]) at tls_record.erl:122.<br></div></blockquote></div><br></div></div><div class="gmail_extra">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<br>





</div><div class="gmail_extra">is allowed to start the handshake.  Normaly if the upgrade is negotiated this is not a problem for the server to set the option<br></div><div class="gmail_extra">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. 
          </div><div><div class="gmail_extra"><br></div><div class="gmail_extra">Regards Ingela Erlang/OTP team - Ericsson AB<br></div></div></div></div>
</blockquote></div></div></div><span><font color="#888888"><br><br clear="all"><div><br></div>-- <br><div dir="ltr"><div><font face="'courier new', monospace">Danil Zagoskin | <a href="mailto:z@gosk.in" target="_blank">z@gosk.in</a></font></div>


</div>
</font></span></div>
</blockquote></div></div><br></div></div></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr"><div><font face="'courier new', monospace">Danil Zagoskin | <a href="mailto:z@gosk.in" target="_blank">z@gosk.in</a></font></div></div>
</div>