Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix ssl ecc handshake crash on unknown curve #286

Closed
wants to merge 2 commits into from

2 participants

@stolen

original message at http://erlang.org/pipermail/erlang-questions/2014-March/078083.html

When buggy client or security scanner opens a connection to OTP ssl server and sends Supported Elliptic Curves Client Hello Extension with '0' or any other curve id not defined in tls_v1:enum_to_oid/1, a server crashes.

This pull requests fixes this problem by ignoring unknown curve ids.

stolen added some commits
@stolen stolen Add test for unknown elliptic curve supported by client
When TLS client sends a Supported Elliptic Curves Client Hello Extension
containing an unknown curve enum value, a server crashes with a
function_clause instead of just ignoring specified unknown curve.
29a89a6
@stolen stolen Fix ssl server crash on unknown elliptic curve enum value
When TLS client sends Supported Elliptic Curves Client Hello Extension
server should select curves supported by both of them or refuse to
negotiate the use of an ECC cipher suite. So it should be OK to ignore
unknown curves specified by a client.
e10f831
@proxyles
Owner

We have decided to include this but with a slightly different implementation. Thank you for your contribution!

@proxyles proxyles closed this
@stolen

Are there chances for it to be included in R16B04?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 9, 2014
  1. @stolen

    Add test for unknown elliptic curve supported by client

    stolen authored
    When TLS client sends a Supported Elliptic Curves Client Hello Extension
    containing an unknown curve enum value, a server crashes with a
    function_clause instead of just ignoring specified unknown curve.
  2. @stolen

    Fix ssl server crash on unknown elliptic curve enum value

    stolen authored
    When TLS client sends Supported Elliptic Curves Client Hello Extension
    server should select curves supported by both of them or refuse to
    negotiate the use of an ECC cipher suite. So it should be OK to ignore
    unknown curves specified by a client.
This page is out of date. Refresh to see the latest.
View
15 lib/ssl/src/ssl_handshake.erl
@@ -1654,7 +1654,7 @@ dec_hello_extensions(<<?UINT16(?SIGNATURE_ALGORITHMS_EXT), ?UINT16(Len),
dec_hello_extensions(<<?UINT16(?ELLIPTIC_CURVES_EXT), ?UINT16(Len),
ExtData:Len/binary, Rest/binary>>, Acc) ->
<<?UINT16(_), EllipticCurveList/binary>> = ExtData,
- EllipticCurves = [tls_v1:enum_to_oid(X) || <<X:16>> <= EllipticCurveList],
+ EllipticCurves = parse_elliptic_curve_list(EllipticCurveList),
dec_hello_extensions(Rest, Acc#hello_extensions{elliptic_curves =
#elliptic_curves{elliptic_curve_list =
EllipticCurves}});
@@ -1867,3 +1867,16 @@ handle_psk_identity(_PSKIdentity, LookupFun)
error;
handle_psk_identity(PSKIdentity, {Fun, UserState}) ->
Fun(psk, PSKIdentity, UserState).
+
+
+% Safe elliptic curve list parser
+parse_elliptic_curve_list(<<X:16, Rest/binary>>) ->
+ try tls_v1:enum_to_oid(X) of
+ Curve -> % supported curve
+ [Curve | parse_elliptic_curve_list(Rest)]
+ catch
+ error:function_clause -> % unsupported curve
+ parse_elliptic_curve_list(Rest)
+ end;
+parse_elliptic_curve_list(<<_/binary>>) -> % Maybe just <<>> should be here
+ [].
View
12 lib/ssl/test/ssl_handshake_SUITE.erl
@@ -34,6 +34,7 @@ suite() -> [{ct_hooks,[ts_install_cth]}].
all() -> [decode_hello_handshake,
decode_single_hello_extension_correctly,
+ decode_supported_elliptic_curves_hello_extension_correctly,
decode_unknown_hello_extension_correctly,
encode_single_hello_sni_extension_correctly].
@@ -67,6 +68,17 @@ decode_single_hello_extension_correctly(_Config) ->
#renegotiation_info{renegotiated_connection = <<0>>}
= Extensions#hello_extensions.renegotiation_info.
+decode_supported_elliptic_curves_hello_extension_correctly(_Config) ->
+ % List of supported and unsupported curves (RFC4492:S5.1.1)
+ ClientEllipticCurves = [0, tls_v1:oid_to_enum(?sect233k1), 37, tls_v1:oid_to_enum(?sect193r2), 16#badc],
+ % Construct extension binary - modified version of ssl_handshake:encode_hello_extensions([#elliptic_curves{}], _)
+ EllipticCurveList = << <<X:16>> || X <- ClientEllipticCurves>>,
+ ListLen = byte_size(EllipticCurveList),
+ Len = ListLen + 2,
+ Extension = <<?UINT16(?ELLIPTIC_CURVES_EXT), ?UINT16(Len), ?UINT16(ListLen), EllipticCurveList/binary>>,
+ % after decoding we should see only valid curves
+ #hello_extensions{elliptic_curves = DecodedCurves} = ssl_handshake:decode_hello_extensions(Extension),
+ #elliptic_curves{elliptic_curve_list = [?sect233k1, ?sect193r2]} = DecodedCurves.
decode_unknown_hello_extension_correctly(_Config) ->
FourByteUnknown = <<16#CA,16#FE, ?UINT16(4), 3, 0, 1, 2>>,
Something went wrong with that request. Please try again.