[erlang-patches] ssl: Bugfix: Use cypher suite's PRF in prf/5.

[Kenneth Lakin]

The PR is at:
https://github.com/erlang/otp/pull/1042

You should be able to fetch with:
git fetch git://github.com/kennethlakin/otp.git tls-use-negotiated-prf

And compare with:
https://github.com/erlang/otp/compare/maint-18...kennethlakin:tls-use-negotiated-prf
https://github.com/erlang/otp/compare/maint-18...kennethlakin:tls-use-negotiated-prf.patch

The remainder of this message is the same as the text in the PR:

TLS 1.2 allows the negotiated cipher to specify its own PRF algorithm.
ssl:prf/5 currently follows a code path that uses hard-coded PRF
algorithms that are always correct for TLS 1.0, always wrong for TLS
1.1, and sometimes correct for TLS 1.2. The TLS handshaking code calls
tls_v1:prf/5 through another path, so it does not run into this bug.
(Given that ssl:prf/5 was added to support EAP-TLS and friends, I'm
probably one of a tiny handful of people who make use of this function.
(EAP-TLS and friends use the output of the TLS PRF as keying material.
If the authenticating server's keys don't match what the client thinks
the keys should be, then the client bails out.))

The patch modifies ssl_handshake:prf to accept a PRF algorithm (rather
than using the hard-coded one), and
ssl_connection:handle_sync_event({prf ...) to pass the connection's
negotiated PRF algorithm along to ssl_handshake:prf. It also adds code
to test the output of ssl:prf/5.

To demonstrate the bug and the correctness of the fix, run the following
in an Erlang shell:

dbg:start().
dbg:tracer().
dbg:tpl(tls_v1, prf, []).
dbg:p(all, c).
ssl:start().
Ciphers={ciphers, [{ecdhe_rsa,aes_256_gcm,null,sha384},
{ecdhe_rsa,aes_256_cbc,sha384,sha384}]}.
{ok, S}=ssl:connect("www.google.com", 443, [Ciphers]).
ssl:prf(S, <<"">>, <<"">>, [<<"">>], 16).
ssl:close(S).
f(S).
f(Ciphers).

(Notice that we're only using ciphers that have a SHA384 PRF.) The buggy
code produces the following output. (Random binary data has been elided.
The PRF to be used is the first argument to tls_v1:prf. 5 is the SHA384
PRF, 4 is the SHA256 PRF, and both values are defined in ssl_record.hrl.)

7> {ok, S}=ssl:connect("www.google.com", 443, [Ciphers]).
(<0.59.0>) call tls_v1:prf(5,...,<<"master secret">>,[...,...],48)
(<0.59.0>) call tls_v1:prf(5,...,"key expansion",[...,...],72)
(<0.59.0>) call tls_v1:prf(5,...,"key expansion",[...,...],72)
(<0.59.0>) call tls_v1:prf(5,...,<<"client finished">>,...,12)
(<0.59.0>) call tls_v1:prf(5,...,<<"server finished">>,...,12)
{ok,{sslsocket,{gen_tcp,#Port<0.892>,tls_connection,
                        undefined},
               <0.59.0>}}
8> ssl:prf(S, <<"">>, <<"">>, [<<"">>], 16).
(<0.59.0>) call tls_v1:prf(4,<<>>,<<>>,[<<>>],16)
{ok,<<166,249,145,171,43,95,158,232,6,60,17,90,183,180,0,
      155>>}

Notice how the first argument passed to tls_v1:prf/5 by way of the call
to ssl:prf/5 is *not* the same as the first argument passed in during
the handshaking process.

The fixed code produces the following output:

7> {ok, S}=ssl:connect("www.google.com", 443, [Ciphers]).
(<0.59.0>) call tls_v1:prf(5,...,<<"master secret">>,[...,...],48)
(<0.59.0>) call tls_v1:prf(5,...,"key expansion",[...,...],72)
(<0.59.0>) call tls_v1:prf(5,...,"key expansion",[...,...],72)
(<0.59.0>) call tls_v1:prf(5,...,<<"client finished">>,...,12)
(<0.59.0>) call tls_v1:prf(5,...,<<"server finished">>,...,12)
{ok,{sslsocket,{gen_tcp,#Port<0.892>,tls_connection,
                        undefined},
               <0.59.0>}}
8> ssl:prf(S, <<"">>, <<"">>, [<<"">>], 16).
(<0.59.0>) call tls_v1:prf(5,<<>>,<<>>,[<<>>],16)
{ok,<<153,182,217,96,186,130,105,85,65,103,123,247,146,
      91,47,106>>}

Notice how the first argument passed to tls_v1:prf/5 by way of the call
to ssl:prf/5 is the now same as the first argument passed in during the
handshaking process.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://erlang.org/pipermail/erlang-patches/attachments/20160503/3813e4bc/attachment.bin>