From 1e0d466f198842cfed14f4fae906381c39bd2050 Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Wed, 19 Sep 2012 12:14:20 +0200 Subject: ssl: Changed default behaviour of next protocol negotiation to make more "sense" (be true to the specification). --- lib/ssl/doc/src/ssl.xml | 39 +++++++++++++++---------- lib/ssl/src/ssl.erl | 26 ++++++++++------- lib/ssl/src/ssl_handshake.erl | 8 ++--- lib/ssl/src/ssl_handshake.hrl | 2 ++ lib/ssl/test/ssl_npn_handshake_SUITE.erl | 50 +++++++++++++++++++------------- lib/ssl/test/ssl_npn_hello_SUITE.erl | 2 +- lib/ssl/test/ssl_to_openssl_SUITE.erl | 31 +++++++++++--------- 7 files changed, 94 insertions(+), 64 deletions(-) diff --git a/lib/ssl/doc/src/ssl.xml b/lib/ssl/doc/src/ssl.xml index 9fc357f1fd..f0eac76264 100644 --- a/lib/ssl/doc/src/ssl.xml +++ b/lib/ssl/doc/src/ssl.xml @@ -303,22 +303,29 @@ fun(OtpCert :: #'OTPCertificate'{}, Event :: {bad_cert, Reason :: atom()} | when possible. - {client_preferred_next_protocols, Fallback :: binary(), Order :: client | server, - PreferenceList :: list(binary())} - Indicates the client will try to perform Next Protocol Negotiation. The - client will attempt to match a protocol in the PreferenceList with a protocol - the server advertises. If the Order is client a protocol earlier in the - PreferenceList will have precendence over a protocol later in the PreferenceList. - Otherwise if the Order is server protocol precendence is determined by the - order the server advertises its protocols. If the server does not advertise a - protocol in the PreferenceList then the Fallback protocol - will be chosen. Fallback must not be an empty binary and PreferenceList - must not contain empty binaries. If the client negotiates a Next Protocol it can be accessed - using negotiated_next_protocol/1 method. - - - - + {client_preferred_next_protocols, Precedence:: server | client, ClientPrefs::[binary()]} + {client_preferred_next_protocols, Precedence:: server | client, ClientPrefs::[binary()] , Default :: binary()}} + +

Indicates the client will try to perform Next Protocol + Negotiation.

+ +

If precedence is server the negaotiated protocol will be the + first protocol that appears on the server advertised list that is + also on the clients preference list.

+ +

If the precedence is client the negaotiated protocol will be the + first protocol that appears on the clients preference list that is + also on the server advertised list.

+ +

If the client does not support any of the servers advertised + protocols or the server does not advertise any protocols the + client will fallback to the first protocol in its list or if a + default is supplied it will fallback to that instead. If the + server does not support next protocol renegotiation the + connection will be aborted if no default protocol is supplied.

+ + +
SSL OPTION DESCRIPTIONS - SERVER SIDE diff --git a/lib/ssl/src/ssl.erl b/lib/ssl/src/ssl.erl index 9ed6acd43a..c72c4d7a93 100644 --- a/lib/ssl/src/ssl.erl +++ b/lib/ssl/src/ssl.erl @@ -39,6 +39,7 @@ -include("ssl_internal.hrl"). -include("ssl_record.hrl"). -include("ssl_cipher.hrl"). +-include("ssl_handshake.hrl"). -include_lib("public_key/include/public_key.hrl"). @@ -745,11 +746,16 @@ validate_option(hibernate_after, Value) when is_integer(Value), Value >= 0 -> validate_option(erl_dist,Value) when Value == true; Value == false -> Value; -validate_option(client_preferred_next_protocols, {FallbackProtocol, Order, PreferredProtocols} = Value) - when is_list(PreferredProtocols), is_binary(FallbackProtocol), - byte_size(FallbackProtocol) > 0, byte_size(FallbackProtocol) < 256 -> +validate_option(client_preferred_next_protocols, {Precedence, PreferredProtocols}) + when is_list(PreferredProtocols) -> validate_binary_list(client_preferred_next_protocols, PreferredProtocols), - validate_npn_ordering(Order), + validate_npn_ordering(Precedence), + {Precedence, PreferredProtocols, ?NO_PROTOCOL}; +validate_option(client_preferred_next_protocols, {Precedence, PreferredProtocols, Default} = Value) + when is_list(PreferredProtocols), is_binary(Default), + byte_size(Default) > 0, byte_size(Default) < 256 -> + validate_binary_list(client_preferred_next_protocols, PreferredProtocols), + validate_npn_ordering(Precedence), Value; validate_option(client_preferred_next_protocols, undefined) -> undefined; @@ -766,7 +772,7 @@ validate_npn_ordering(client) -> validate_npn_ordering(server) -> ok; validate_npn_ordering(Value) -> - throw({error, {eoptions, {client_preferred_next_protocols, Value}}}). + throw({error, {eoptions, {client_preferred_next_protocols, {invalid_precedence, Value}}}}). validate_binary_list(Opt, List) -> lists:foreach( @@ -775,7 +781,7 @@ validate_binary_list(Opt, List) -> byte_size(Bin) < 256 -> ok; (Bin) -> - throw({error, {eoptions, {Opt, Bin}}}) + throw({error, {eoptions, {Opt, {invalid_protocol, Bin}}}}) end, List). validate_versions([], Versions) -> @@ -896,18 +902,18 @@ detect(Pred, [H|T]) -> make_next_protocol_selector(undefined) -> undefined; -make_next_protocol_selector({FallbackProtocol, client, AllProtocols}) -> +make_next_protocol_selector({client, AllProtocols, DefaultProtocol}) -> fun(AdvertisedProtocols) -> case detect(fun(PreferredProtocol) -> lists:member(PreferredProtocol, AdvertisedProtocols) end, AllProtocols) of - undefined -> FallbackProtocol; + undefined -> DefaultProtocol; PreferredProtocol -> PreferredProtocol end end; -make_next_protocol_selector({FallbackProtocol, server, AllProtocols}) -> +make_next_protocol_selector({server, AllProtocols, DefaultProtocol}) -> fun(AdvertisedProtocols) -> case detect(fun(PreferredProtocol) -> lists:member(PreferredProtocol, AllProtocols) end, AdvertisedProtocols) of - undefined -> FallbackProtocol; + undefined -> DefaultProtocol; PreferredProtocol -> PreferredProtocol end end. diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl index 3b114c1f9c..695721d51a 100644 --- a/lib/ssl/src/ssl_handshake.erl +++ b/lib/ssl/src/ssl_handshake.erl @@ -741,10 +741,10 @@ select_next_protocol({error, _Reason}, _NextProtocolSelector) -> ?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE); select_next_protocol(Protocols, NextProtocolSelector) -> case NextProtocolSelector(Protocols) of - Protocol when is_binary(Protocol), byte_size(Protocol) > 0 -> - Protocol; - _ -> - ?ALERT_REC(?FATAL, ?INTERNAL_ERROR) % we are broken internally :( api presently stops this but we might let it happen in future + ?NO_PROTOCOL -> + ?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE); + Protocol when is_binary(Protocol) -> + Protocol end. handle_renegotiation_info(_, #renegotiation_info{renegotiated_connection = ?byte(0)}, diff --git a/lib/ssl/src/ssl_handshake.hrl b/lib/ssl/src/ssl_handshake.hrl index bd583e5d92..a4ce48c291 100644 --- a/lib/ssl/src/ssl_handshake.hrl +++ b/lib/ssl/src/ssl_handshake.hrl @@ -33,6 +33,8 @@ -type public_key_info() :: {algo_oid(), #'RSAPublicKey'{} | integer() , public_key_params()}. -type tls_handshake_history() :: {[binary()], [binary()]}. +-define(NO_PROTOCOL, <<>>). + %% Signature algorithms -define(ANON, 0). -define(RSA, 1). diff --git a/lib/ssl/test/ssl_npn_handshake_SUITE.erl b/lib/ssl/test/ssl_npn_handshake_SUITE.erl index f2327756c3..8bef2d8d22 100644 --- a/lib/ssl/test/ssl_npn_handshake_SUITE.erl +++ b/lib/ssl/test/ssl_npn_handshake_SUITE.erl @@ -28,16 +28,16 @@ suite() -> [{ct_hooks,[ts_install_cth]}]. init_per_suite(Config) -> try crypto:start() of - ok -> - application:start(public_key), - ssl:start(), - Result = - (catch make_certs:all(?config(data_dir, Config), - ?config(priv_dir, Config))), - test_server:format("Make certs ~p~n", [Result]), - ssl_test_lib:cert_options(Config) + ok -> + application:start(public_key), + ssl:start(), + Result = + (catch make_certs:all(?config(data_dir, Config), + ?config(priv_dir, Config))), + test_server:format("Make certs ~p~n", [Result]), + ssl_test_lib:cert_options(Config) catch _:_ -> - {skip, "Crypto did not start"} + {skip, "Crypto did not start"} end. end_per_suite(_Config) -> @@ -62,9 +62,14 @@ connection_info_result(Socket) -> ssl:connection_info(Socket). validate_empty_protocols_are_not_allowed_test(_Config) -> - {error, {eoptions, {next_protocols_advertised, <<>>}}} = (catch ssl:listen(9443, [{next_protocols_advertised, [<<"foo/1">>, <<"">>]}])), - {error, {eoptions, {client_preferred_next_protocols, <<>>}}} = (catch ssl:connect({127,0,0,1}, 9443, [{client_preferred_next_protocols, {<<"foox/1">>, client, [<<"foo/1">>, <<"">>]}}], infinity)), - Option = {client_preferred_next_protocols, {<<"">>, client, [<<"foo/1">>, <<"blah/1">>]}}, + {error, {eoptions, {next_protocols_advertised, {invalid_protocol, <<>>}}}} + = (catch ssl:listen(9443, + [{next_protocols_advertised, [<<"foo/1">>, <<"">>]}])), + {error, {eoptions, {client_preferred_next_protocols, {invalid_protocol, <<>>}}}} + = (catch ssl:connect({127,0,0,1}, 9443, + [{client_preferred_next_protocols, + {client, [<<"foo/1">>, <<"">>], <<"foox/1">>}}], infinity)), + Option = {client_preferred_next_protocols, {invalid_protocol, <<"">>}}, {error, {eoptions, Option}} = (catch ssl:connect({127,0,0,1}, 9443, [Option], infinity)). validate_empty_advertisement_list_is_allowed_test(_Config) -> @@ -90,32 +95,34 @@ perform_client_does_not_try_to_negotiate_but_server_supports_npn_test(Config) -> perform_client_tries_to_negotiate_but_server_does_not_support_test(Config) -> run_npn_handshake_test(Config, - [{client_preferred_next_protocols, {<<"http/1.1">>, client, [<<"spdy/2">>]}}], + [{client_preferred_next_protocols, {client, [<<"spdy/2">>], <<"http/1.1">>}}], [], {error, next_protocol_not_negotiated}). perform_fallback_npn_handshake_test(Config) -> run_npn_handshake_test(Config, - [{client_preferred_next_protocols, {<<"http/1.1">>, client, [<<"spdy/2">>]}}], + [{client_preferred_next_protocols, {client, [<<"spdy/2">>], <<"http/1.1">>}}], [{next_protocols_advertised, [<<"spdy/1">>, <<"http/1.1">>, <<"http/1.0">>]}], {ok, <<"http/1.1">>}). perform_fallback_npn_handshake_server_preference_test(Config) -> run_npn_handshake_test(Config, - [{client_preferred_next_protocols, {<<"http/1.1">>, server, [<<"spdy/2">>]}}], + [{client_preferred_next_protocols, {server, [<<"spdy/2">>], <<"http/1.1">>}}], [{next_protocols_advertised, [<<"spdy/1">>, <<"http/1.1">>, <<"http/1.0">>]}], {ok, <<"http/1.1">>}). perform_normal_npn_handshake_client_preference_test(Config) -> run_npn_handshake_test(Config, - [{client_preferred_next_protocols, {<<"http/1.1">>, client, [<<"http/1.0">>, <<"http/1.1">>]}}], + [{client_preferred_next_protocols, + {client, [<<"http/1.0">>, <<"http/1.1">>], <<"http/1.1">>}}], [{next_protocols_advertised, [<<"spdy/2">>, <<"http/1.1">>, <<"http/1.0">>]}], {ok, <<"http/1.0">>}). perform_normal_npn_handshake_server_preference_test(Config) -> run_npn_handshake_test(Config, - [{client_preferred_next_protocols, {<<"http/1.1">>, server, [<<"http/1.0">>, <<"http/1.1">>]}}], + [{client_preferred_next_protocols, + {server, [<<"http/1.0">>, <<"http/1.1">>], <<"http/1.1">>}}], [{next_protocols_advertised, [<<"spdy/2">>, <<"http/1.1">>, <<"http/1.0">>]}], {ok, <<"http/1.1">>}). @@ -124,9 +131,11 @@ perform_renegotiate_from_client_after_npn_handshake(Config) -> Data = "hello world", ClientOpts0 = ?config(client_opts, Config), - ClientOpts = [{client_preferred_next_protocols, {<<"http/1.1">>, client, [<<"http/1.0">>]}}] ++ ClientOpts0, + ClientOpts = [{client_preferred_next_protocols, + {client, [<<"http/1.0">>], <<"http/1.1">>}}] ++ ClientOpts0, ServerOpts0 = ?config(server_opts, Config), - ServerOpts = [{next_protocols_advertised, [<<"spdy/2">>, <<"http/1.1">>, <<"http/1.0">>]}] ++ ServerOpts0, + ServerOpts = [{next_protocols_advertised, + [<<"spdy/2">>, <<"http/1.1">>, <<"http/1.0">>]}] ++ ServerOpts0, ExpectedProtocol = {ok, <<"http/1.0">>}, {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config), @@ -170,7 +179,8 @@ run_npn_handshake_test(Config, ClientExtraOpts, ServerExtraOpts, ExpectedProtoco ssl_test_lib:check_result(Server, ok, Client, ok). assert_npn(Socket, Protocol) -> - test_server:format("Negotiated Protocol ~p, Expecting: ~p ~n", [ssl:negotiated_next_protocol(Socket), Protocol]), + test_server:format("Negotiated Protocol ~p, Expecting: ~p ~n", + [ssl:negotiated_next_protocol(Socket), Protocol]), Protocol = ssl:negotiated_next_protocol(Socket). assert_npn_and_renegotiate_and_send_data(Socket, Protocol, Data) -> diff --git a/lib/ssl/test/ssl_npn_hello_SUITE.erl b/lib/ssl/test/ssl_npn_hello_SUITE.erl index f177778178..0bca8bbeb4 100644 --- a/lib/ssl/test/ssl_npn_hello_SUITE.erl +++ b/lib/ssl/test/ssl_npn_hello_SUITE.erl @@ -60,7 +60,7 @@ encode_and_decode_client_hello_test(_Config) -> encode_and_decode_npn_client_hello_test(_Config) -> HandShakeData = create_client_handshake(#next_protocol_negotiation{extension_data = <<>>}), Version = ssl_record:protocol_version(ssl_record:highest_protocol_version([])), - {[{DecodedHandshakeMessage, _Raw}], _} = ssl_handshake:get_tls_handshake(Version¸ list_to_binary(HandShakeData), <<>>), + {[{DecodedHandshakeMessage, _Raw}], _} = ssl_handshake:get_tls_handshake(Version, list_to_binary(HandShakeData), <<>>), NextProtocolNegotiation = DecodedHandshakeMessage#client_hello.next_protocol_negotiation, NextProtocolNegotiation = #next_protocol_negotiation{extension_data = <<>>}. diff --git a/lib/ssl/test/ssl_to_openssl_SUITE.erl b/lib/ssl/test/ssl_to_openssl_SUITE.erl index cc3c6439ac..30f8f60156 100644 --- a/lib/ssl/test/ssl_to_openssl_SUITE.erl +++ b/lib/ssl/test/ssl_to_openssl_SUITE.erl @@ -29,7 +29,7 @@ -define(TIMEOUT, 120000). -define(LONG_TIMEOUT, 600000). -define(SLEEP, 1000). --define(OPENSSL_RENEGOTIATE, "r\n"). +-define(OPENSSL_RENEGOTIATE, "R\n"). -define(OPENSSL_QUIT, "Q\n"). -define(OPENSSL_GARBAGE, "P\n"). -define(EXPIRE, 10). @@ -172,9 +172,9 @@ all() -> groups() -> [{basic, [], basic_tests()}, - {'tlsv1.2', [], all_versions_tests()}, - {'tlsv1.1', [], all_versions_tests()}, - {'tlsv1', [], all_versions_tests()}, + {'tlsv1.2', [], all_versions_tests() ++ npn_tests()}, + {'tlsv1.1', [], all_versions_tests() ++ npn_tests()}, + {'tlsv1', [], all_versions_tests()++ npn_tests()}, {'sslv3', [], all_versions_tests()}]. basic_tests() -> @@ -199,8 +199,10 @@ all_versions_tests() -> ciphers_dsa_signed_certs, erlang_client_bad_openssl_server, expired_session, - ssl2_erlang_server_openssl_client, - erlang_client_openssl_server_npn_negotiation, + ssl2_erlang_server_openssl_client]. + +npn_tests() -> + [erlang_client_openssl_server_npn_negotiation, erlang_server_openssl_client_npn_negotiation, erlang_server_openssl_client_npn_negotiation_and_renegotiate, erlang_client_openssl_server_npn_negotiation_and_renegotiate, @@ -1164,7 +1166,7 @@ erlang_client_openssl_server_npn_negotiate_only_on_server(Config) when is_list(C erlang_client_openssl_server_npn_negotiate_only_on_client(Config) when is_list(Config) -> Data = "From openssl to erlang", - start_erlang_client_and_openssl_server_with_opts(Config, [{client_preferred_next_protocols, {<<"http/1.1">>, client, [<<"spdy/2">>]}}], "", Data, fun(Server, OpensslPort) -> + start_erlang_client_and_openssl_server_with_opts(Config, [{client_preferred_next_protocols, {client, [<<"spdy/2">>], <<"http/1.1">>}}], "", Data, fun(Server, OpensslPort) -> port_command(OpensslPort, Data), ssl_test_lib:check_result(Server, ok) end), @@ -1202,9 +1204,11 @@ start_erlang_client_and_openssl_server_with_opts(Config, ErlangClientOpts, Opens Port = ssl_test_lib:inet_port(node()), CertFile = proplists:get_value(certfile, ServerOpts), KeyFile = proplists:get_value(keyfile, ServerOpts), + Version = ssl_record:protocol_version(ssl_record:highest_protocol_version([])), - Cmd = "openssl s_server " ++ OpensslServerOpts ++ " -accept " ++ integer_to_list(Port) ++ - " -cert " ++ CertFile ++ " -key " ++ KeyFile, + Cmd = "openssl s_server " ++ OpensslServerOpts ++ " -accept " ++ + integer_to_list(Port) ++ version_flag(Version) ++ + " -cert " ++ CertFile ++ " -key " ++ KeyFile, test_server:format("openssl cmd: ~p~n", [Cmd]), @@ -1231,7 +1235,7 @@ start_erlang_client_and_openssl_server_for_npn_negotiation(Config, Data, Callbac process_flag(trap_exit, true), ServerOpts = ?config(server_opts, Config), ClientOpts0 = ?config(client_opts, Config), - ClientOpts = [{client_preferred_next_protocols, {<<"http/1.1">>, client, [<<"spdy/2">>]}} | ClientOpts0], + ClientOpts = [{client_preferred_next_protocols, {client, [<<"spdy/2">>], <<"http/1.1">>}} | ClientOpts0], {ClientNode, _, Hostname} = ssl_test_lib:run_where(Config), @@ -1240,8 +1244,9 @@ start_erlang_client_and_openssl_server_for_npn_negotiation(Config, Data, Callbac Port = ssl_test_lib:inet_port(node()), CertFile = proplists:get_value(certfile, ServerOpts), KeyFile = proplists:get_value(keyfile, ServerOpts), + Version = ssl_record:protocol_version(ssl_record:highest_protocol_version([])), - Cmd = "openssl s_server -msg -nextprotoneg http/1.1,spdy/2 -accept " ++ integer_to_list(Port) ++ + Cmd = "openssl s_server -msg -nextprotoneg http/1.1,spdy/2 -accept " ++ integer_to_list(Port) ++ version_flag(Version) ++ " -cert " ++ CertFile ++ " -key " ++ KeyFile, test_server:format("openssl cmd: ~p~n", [Cmd]), @@ -1278,8 +1283,8 @@ start_erlang_server_and_openssl_client_for_npn_negotiation(Config, Data, Callbac {mfa, {?MODULE, erlang_ssl_receive_and_assert_npn, [<<"spdy/2">>, Data]}}, {options, ServerOpts}]), Port = ssl_test_lib:inet_port(Server), - - Cmd = "openssl s_client -nextprotoneg http/1.0,spdy/2 -msg -port " ++ integer_to_list(Port) ++ + Version = ssl_record:protocol_version(ssl_record:highest_protocol_version([])), + Cmd = "openssl s_client -nextprotoneg http/1.0,spdy/2 -msg -port " ++ integer_to_list(Port) ++ version_flag(Version) ++ " -host localhost", test_server:format("openssl cmd: ~p~n", [Cmd]), -- cgit v1.2.3