From 56b40d01c47170fea0d798dcc46fbde7ffc853dc Mon Sep 17 00:00:00 2001 From: Magnus Henoch Date: Thu, 21 Jan 2016 16:06:03 +0000 Subject: Refactor ssl_crl_SUITE: extract crl_verify_error/6 Just like crl_verify_valid/5 checks for a positive result given certain options, crl_verify_error/6 checks for a negative result. --- lib/ssl/test/ssl_crl_SUITE.erl | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) (limited to 'lib/ssl/test') diff --git a/lib/ssl/test/ssl_crl_SUITE.erl b/lib/ssl/test/ssl_crl_SUITE.erl index 44580be1ff..68ed36caa3 100644 --- a/lib/ssl/test/ssl_crl_SUITE.erl +++ b/lib/ssl/test/ssl_crl_SUITE.erl @@ -186,11 +186,6 @@ crl_verify_revoked(Config) when is_list(Config) -> {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config), - Server = ssl_test_lib:start_server_error([{node, ServerNode}, {port, 0}, - {from, self()}, - {options, ServerOpts}]), - Port = ssl_test_lib:inet_port(Server), - ssl_crl_cache:insert({file, filename:join([PrivDir, "erlangCA", "crl.pem"])}), ssl_crl_cache:insert({file, filename:join([PrivDir, "otpCA", "crl.pem"])}), @@ -206,15 +201,8 @@ crl_verify_revoked(Config) when is_list(Config) -> {verify, verify_peer}] end, - Client = ssl_test_lib:start_client_error([{node, ClientNode}, {port, Port}, - {host, Hostname}, - {from, self()}, - {options, ClientOpts}]), - receive - {Server, AlertOrColse} -> - ct:pal("Server Alert or Close ~p", [AlertOrColse]) - end, - ssl_test_lib:check_result(Client, {error, {tls_alert, "certificate revoked"}}). + crl_verify_error(Hostname, ServerNode, ServerOpts, ClientNode, ClientOpts, + "certificate revoked"). crl_verify_valid(Hostname, ServerNode, ServerOpts, ClientNode, ClientOpts) -> @@ -236,6 +224,22 @@ crl_verify_valid(Hostname, ServerNode, ServerOpts, ClientNode, ClientOpts) -> ssl_test_lib:close(Server), ssl_test_lib:close(Client). +crl_verify_error(Hostname, ServerNode, ServerOpts, ClientNode, ClientOpts, ExpectedAlert) -> + Server = ssl_test_lib:start_server_error([{node, ServerNode}, {port, 0}, + {from, self()}, + {options, ServerOpts}]), + Port = ssl_test_lib:inet_port(Server), + + Client = ssl_test_lib:start_client_error([{node, ClientNode}, {port, Port}, + {host, Hostname}, + {from, self()}, + {options, ClientOpts}]), + receive + {Server, AlertOrClose} -> + ct:pal("Server Alert or Close ~p", [AlertOrClose]) + end, + ssl_test_lib:check_result(Client, {error, {tls_alert, ExpectedAlert}}). + %%-------------------------------------------------------------------- %% Internal functions ------------------------------------------------ %%-------------------------------------------------------------------- -- cgit v1.2.3 From a567dca5ea418a0aaaed8fb4359032b11f28cccd Mon Sep 17 00:00:00 2001 From: Magnus Henoch Date: Fri, 29 Jan 2016 18:47:43 +0000 Subject: 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. --- lib/ssl/test/ssl_certificate_verify_SUITE.erl | 119 +++++++++++++++++++++++++- 1 file changed, 118 insertions(+), 1 deletion(-) (limited to 'lib/ssl/test') 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, @@ -793,6 +795,121 @@ extended_key_usage_verify_none(Config) when is_list(Config) -> ssl_test_lib:close(Server), 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" -- cgit v1.2.3 From 331afa5dfaa129a5ea41af5dd76246bb922ac7df Mon Sep 17 00:00:00 2001 From: Magnus Henoch Date: Fri, 22 Jan 2016 12:31:36 +0000 Subject: Be suspicious of certificates without CRL DPs Previously, if certificate revocation checking was turned on, and a certificate didn't contain a CRL Distribution Points extension, and there was no relevant CRL in the cache, then ssl_handshake:crl_check would accept the certificate even if the crl_check option was set to reject certificates for which the revocation status could not be determined. With this change, such certificates will only be accepted if the crl_check option was set to best_effort. The process for CRL validation is described in section 6.3 of RFC 5280. The text doesn't mention any special treatment to be given to certificates without distribution points: it just says "For each distribution point..." (section 6.3.3), which would leave the revocation status undetermined, unless there were "any available CRLs not specified in a distribution point but issued by the certificate issuer". Thus the result of this algorithm should be UNDETERMINED in this case, not UNREVOKED, and the crl_check option should govern how the implementation reacts to this result. --- lib/ssl/test/ssl_crl_SUITE.erl | 50 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) (limited to 'lib/ssl/test') diff --git a/lib/ssl/test/ssl_crl_SUITE.erl b/lib/ssl/test/ssl_crl_SUITE.erl index 68ed36caa3..5b86027210 100644 --- a/lib/ssl/test/ssl_crl_SUITE.erl +++ b/lib/ssl/test/ssl_crl_SUITE.erl @@ -53,7 +53,7 @@ groups() -> {idp_crl, [], basic_tests()}]. basic_tests() -> - [crl_verify_valid, crl_verify_revoked]. + [crl_verify_valid, crl_verify_revoked, crl_verify_no_crl]. init_per_suite(Config) -> @@ -204,6 +204,52 @@ crl_verify_revoked(Config) when is_list(Config) -> crl_verify_error(Hostname, ServerNode, ServerOpts, ClientNode, ClientOpts, "certificate revoked"). +crl_verify_no_crl() -> + [{doc,"Verify a simple CRL chain when the CRL is missing"}]. +crl_verify_no_crl(Config) when is_list(Config) -> + PrivDir = ?config(cert_dir, Config), + Check = ?config(crl_check, Config), + ServerOpts = [{keyfile, filename:join([PrivDir, "server", "key.pem"])}, + {certfile, filename:join([PrivDir, "server", "cert.pem"])}, + {cacertfile, filename:join([PrivDir, "server", "cacerts.pem"])}], + ClientOpts = case ?config(idp_crl, Config) of + true -> + [{cacertfile, filename:join([PrivDir, "server", "cacerts.pem"])}, + {crl_check, Check}, + {crl_cache, {ssl_crl_cache, {internal, [{http, 5000}]}}}, + {verify, verify_peer}]; + false -> + [{cacertfile, filename:join([PrivDir, "server", "cacerts.pem"])}, + {crl_check, Check}, + {verify, verify_peer}] + end, + {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config), + + %% In case we're running an HTTP server that serves CRLs, let's + %% rename those files, so the CRL is absent when we try to verify + %% it. + %% + %% If we're not using an HTTP server, we just need to refrain from + %% adding the CRLs to the cache manually. + rename_crl(filename:join([PrivDir, "erlangCA", "crl.pem"])), + rename_crl(filename:join([PrivDir, "otpCA", "crl.pem"])), + + %% The expected outcome when the CRL is missing depends on the + %% crl_check setting. + case Check of + true -> + %% The error "revocation status undetermined" gets turned + %% into "bad certificate". + crl_verify_error(Hostname, ServerNode, ServerOpts, ClientNode, ClientOpts, + "bad certificate"); + peer -> + crl_verify_error(Hostname, ServerNode, ServerOpts, ClientNode, ClientOpts, + "bad certificate"); + best_effort -> + %% In "best effort" mode, we consider the certificate not + %% to be revoked if we can't find the appropriate CRL. + crl_verify_valid(Hostname, ServerNode, ServerOpts, ClientNode, ClientOpts) + end. crl_verify_valid(Hostname, ServerNode, ServerOpts, ClientNode, ClientOpts) -> Server = ssl_test_lib:start_server([{node, ServerNode}, {port, 0}, @@ -263,3 +309,5 @@ make_dir_path(PathComponents) -> "", PathComponents). +rename_crl(Filename) -> + file:rename(Filename, Filename ++ ".notfound"). -- cgit v1.2.3