aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMagnus Henoch <[email protected]>2016-01-29 18:47:43 +0000
committerMagnus Henoch <[email protected]>2016-02-17 10:05:26 +0000
commita567dca5ea418a0aaaed8fb4359032b11f28cccd (patch)
tree06e623b56e43f70e14b4019dc8a4783e0f966a1b
parent6945881b99aeadaf9ed4ec1f8c7811538cee1405 (diff)
downloadotp-a567dca5ea418a0aaaed8fb4359032b11f28cccd.tar.gz
otp-a567dca5ea418a0aaaed8fb4359032b11f28cccd.tar.bz2
otp-a567dca5ea418a0aaaed8fb4359032b11f28cccd.zip
ssl: with verify_none, accept critical extensions
When establishing a TLS connection with {verify, verify_none}, if the server has a certificate with a critical extension, for example a "Netscape Cert Type" extension, certificate verification would fail, which is surprising given that the name of the option suggests that no verification would be performed. With this change, certificate extensions marked as critical are ignored when using verify_none.
-rw-r--r--lib/ssl/doc/src/ssl.xml8
-rw-r--r--lib/ssl/src/ssl.erl6
-rw-r--r--lib/ssl/test/ssl_certificate_verify_SUITE.erl119
3 files changed, 131 insertions, 2 deletions
diff --git a/lib/ssl/doc/src/ssl.xml b/lib/ssl/doc/src/ssl.xml
index bf87644116..ca5d2afc24 100644
--- a/lib/ssl/doc/src/ssl.xml
+++ b/lib/ssl/doc/src/ssl.xml
@@ -269,7 +269,11 @@ atom()}} |
terminate regarding verification failures and the connection is
established.</p></item>
<item><p>If called with an extension unknown to the user application,
- return value <c>{unknown, UserState}</c> is to be used.</p></item>
+ return value <c>{unknown, UserState}</c> is to be used.</p>
+
+ <p>Note that if the fun returns <c>unknown</c> for an extension marked
+ as critical, validation will fail.</p>
+ </item>
</list>
<p>Default option <c>verify_fun</c> in <c>verify_peer mode</c>:</p>
@@ -291,6 +295,8 @@ atom()}} |
<code>
{fun(_,{bad_cert, _}, UserState) ->
{valid, UserState};
+ (_,{extension, #'Extension'{critical = true}}, UserState) ->
+ {valid, UserState};
(_,{extension, _}, UserState) ->
{unknown, UserState};
(_, valid, UserState) ->
diff --git a/lib/ssl/src/ssl.erl b/lib/ssl/src/ssl.erl
index 6551308935..401ee63908 100644
--- a/lib/ssl/src/ssl.erl
+++ b/lib/ssl/src/ssl.erl
@@ -1280,6 +1280,12 @@ handle_verify_options(Opts, CaCerts) ->
DefaultVerifyNoneFun =
{fun(_,{bad_cert, _}, UserState) ->
{valid, UserState};
+ (_,{extension, #'Extension'{critical = true}}, UserState) ->
+ %% This extension is marked as critical, so
+ %% certificate verification should fail if we don't
+ %% understand the extension. However, this is
+ %% `verify_none', so let's accept it anyway.
+ {valid, UserState};
(_,{extension, _}, UserState) ->
{unknown, UserState};
(_, valid, UserState) ->
diff --git a/lib/ssl/test/ssl_certificate_verify_SUITE.erl b/lib/ssl/test/ssl_certificate_verify_SUITE.erl
index 5940a86a7f..123a7f4cbf 100644
--- a/lib/ssl/test/ssl_certificate_verify_SUITE.erl
+++ b/lib/ssl/test/ssl_certificate_verify_SUITE.erl
@@ -66,7 +66,9 @@ tests() ->
invalid_signature_client,
invalid_signature_server,
extended_key_usage_verify_peer,
- extended_key_usage_verify_none].
+ extended_key_usage_verify_none,
+ critical_extension_verify_peer,
+ critical_extension_verify_none].
error_handling_tests()->
[client_with_cert_cipher_suites_handshake,
@@ -794,6 +796,121 @@ extended_key_usage_verify_none(Config) when is_list(Config) ->
ssl_test_lib:close(Client).
%%--------------------------------------------------------------------
+critical_extension_verify_peer() ->
+ [{doc,"Test cert that has a critical unknown extension in verify_peer mode"}].
+
+critical_extension_verify_peer(Config) when is_list(Config) ->
+ ClientOpts = ?config(client_verification_opts, Config),
+ ServerOpts = ?config(server_verification_opts, Config),
+ PrivDir = ?config(priv_dir, Config),
+ Active = ?config(active, Config),
+ ReceiveFunction = ?config(receive_function, Config),
+
+ KeyFile = filename:join(PrivDir, "otpCA/private/key.pem"),
+ NewCertName = integer_to_list(erlang:unique_integer()) ++ ".pem",
+
+ ServerCertFile = proplists:get_value(certfile, ServerOpts),
+ NewServerCertFile = filename:join([PrivDir, "server", NewCertName]),
+ add_critical_netscape_cert_type(ServerCertFile, NewServerCertFile, KeyFile),
+ NewServerOpts = [{certfile, NewServerCertFile} | proplists:delete(certfile, ServerOpts)],
+
+ ClientCertFile = proplists:get_value(certfile, ClientOpts),
+ NewClientCertFile = filename:join([PrivDir, "client", NewCertName]),
+ add_critical_netscape_cert_type(ClientCertFile, NewClientCertFile, KeyFile),
+ NewClientOpts = [{certfile, NewClientCertFile} | proplists:delete(certfile, ClientOpts)],
+
+ {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config),
+
+ Server = ssl_test_lib:start_server_error(
+ [{node, ServerNode}, {port, 0},
+ {from, self()},
+ {mfa, {ssl_test_lib, ReceiveFunction, []}},
+ {options, [{verify, verify_peer}, {active, Active} | NewServerOpts]}]),
+ Port = ssl_test_lib:inet_port(Server),
+ Client = ssl_test_lib:start_client_error(
+ [{node, ClientNode}, {port, Port},
+ {host, Hostname},
+ {from, self()},
+ {mfa, {ssl_test_lib, ReceiveFunction, []}},
+ {options, [{verify, verify_peer}, {active, Active} | NewClientOpts]}]),
+
+ %% This certificate has a critical extension that we don't
+ %% understand. Therefore, verification should fail.
+ tcp_delivery_workaround(Server, {error, {tls_alert, "unsupported certificate"}},
+ Client, {error, {tls_alert, "unsupported certificate"}}),
+
+ ssl_test_lib:close(Server),
+ ok.
+
+%%--------------------------------------------------------------------
+critical_extension_verify_none() ->
+ [{doc,"Test cert that has a critical unknown extension in verify_none mode"}].
+
+critical_extension_verify_none(Config) when is_list(Config) ->
+ ClientOpts = ?config(client_verification_opts, Config),
+ ServerOpts = ?config(server_verification_opts, Config),
+ PrivDir = ?config(priv_dir, Config),
+ Active = ?config(active, Config),
+ ReceiveFunction = ?config(receive_function, Config),
+
+ KeyFile = filename:join(PrivDir, "otpCA/private/key.pem"),
+ NewCertName = integer_to_list(erlang:unique_integer()) ++ ".pem",
+
+ ServerCertFile = proplists:get_value(certfile, ServerOpts),
+ NewServerCertFile = filename:join([PrivDir, "server", NewCertName]),
+ add_critical_netscape_cert_type(ServerCertFile, NewServerCertFile, KeyFile),
+ NewServerOpts = [{certfile, NewServerCertFile} | proplists:delete(certfile, ServerOpts)],
+
+ ClientCertFile = proplists:get_value(certfile, ClientOpts),
+ NewClientCertFile = filename:join([PrivDir, "client", NewCertName]),
+ add_critical_netscape_cert_type(ClientCertFile, NewClientCertFile, KeyFile),
+ NewClientOpts = [{certfile, NewClientCertFile} | proplists:delete(certfile, ClientOpts)],
+
+ {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config),
+
+ Server = ssl_test_lib:start_server(
+ [{node, ServerNode}, {port, 0},
+ {from, self()},
+ {mfa, {ssl_test_lib, ReceiveFunction, []}},
+ {options, [{verify, verify_none}, {active, Active} | NewServerOpts]}]),
+ Port = ssl_test_lib:inet_port(Server),
+ Client = ssl_test_lib:start_client(
+ [{node, ClientNode}, {port, Port},
+ {host, Hostname},
+ {from, self()},
+ {mfa, {ssl_test_lib, ReceiveFunction, []}},
+ {options, [{verify, verify_none}, {active, Active} | NewClientOpts]}]),
+
+ %% This certificate has a critical extension that we don't
+ %% understand. But we're using `verify_none', so verification
+ %% shouldn't fail.
+ ssl_test_lib:check_result(Server, ok, Client, ok),
+
+ ssl_test_lib:close(Server),
+ ssl_test_lib:close(Client),
+ ok.
+
+add_critical_netscape_cert_type(CertFile, NewCertFile, KeyFile) ->
+ [KeyEntry] = ssl_test_lib:pem_to_der(KeyFile),
+ Key = ssl_test_lib:public_key(public_key:pem_entry_decode(KeyEntry)),
+
+ [{'Certificate', DerCert, _}] = ssl_test_lib:pem_to_der(CertFile),
+ OTPCert = public_key:pkix_decode_cert(DerCert, otp),
+ %% This is the "Netscape Cert Type" extension, telling us that the
+ %% certificate can be used for SSL clients and SSL servers.
+ NetscapeCertTypeExt = #'Extension'{
+ extnID = {2,16,840,1,113730,1,1},
+ critical = true,
+ extnValue = <<3,2,6,192>>},
+ OTPTbsCert = OTPCert#'OTPCertificate'.tbsCertificate,
+ Extensions = OTPTbsCert#'OTPTBSCertificate'.extensions,
+ NewOTPTbsCert = OTPTbsCert#'OTPTBSCertificate'{
+ extensions = [NetscapeCertTypeExt] ++ Extensions},
+ NewDerCert = public_key:pkix_sign(NewOTPTbsCert, Key),
+ ssl_test_lib:der_to_pem(NewCertFile, [{'Certificate', NewDerCert, not_encrypted}]),
+ ok.
+
+%%--------------------------------------------------------------------
no_authority_key_identifier() ->
[{doc, "Test cert that does not have authorityKeyIdentifier extension"
" but are present in trusted certs db."}].