From 5fa060a43c05083760cff7ef5abeb941732f3394 Mon Sep 17 00:00:00 2001 From: William Yang Date: Wed, 3 May 2023 14:10:44 +0200 Subject: [PATCH 1/6] feat: enhanced tls handshake --- priv/emqx.schema | 6 + src/emqx_const_v2.erl | 77 ++++++ src/emqx_listeners.erl | 3 +- src/emqx_tls_lib.erl | 22 ++ ...mqx_listener_tls_verify_keyusage_SUITE.erl | 234 ++++++++++++++++++ test/emqx_test_tls_certs_helper.erl | 3 +- 6 files changed, 343 insertions(+), 2 deletions(-) create mode 100644 test/emqx_listener_tls_verify_keyusage_SUITE.erl diff --git a/priv/emqx.schema b/priv/emqx.schema index b3846dc0d..e53c18d6e 100644 --- a/priv/emqx.schema +++ b/priv/emqx.schema @@ -1650,6 +1650,11 @@ end}. {datatype, atom} ]}. +{mapping, "listener.ssl.$name.verify_peer_ext_key_usage", "emqx.listeners", [ + {datatype, string}, + {default, ""} +]}. + {mapping, "listener.ssl.$name.fail_if_no_peer_cert", "emqx.listeners", [ {datatype, {enum, [true, false]}} ]}. @@ -2382,6 +2387,7 @@ end}. {cacertfile, cuttlefish:conf_get(Prefix ++ ".cacertfile", Conf, undefined)}, {verify, cuttlefish:conf_get(Prefix ++ ".verify", Conf, undefined)}, {partial_chain, cuttlefish:conf_get(Prefix ++ ".partial_chain", Conf, undefined)}, + {verify_peer_ext_key_usage, cuttlefish:conf_get(Prefix ++ ".verify_peer_ext_key_usage", Conf, undefined)}, {fail_if_no_peer_cert, cuttlefish:conf_get(Prefix ++ ".fail_if_no_peer_cert", Conf, undefined)}, {secure_renegotiate, cuttlefish:conf_get(Prefix ++ ".secure_renegotiate", Conf, undefined)}, {reuse_sessions, cuttlefish:conf_get(Prefix ++ ".reuse_sessions", Conf, undefined)}, diff --git a/src/emqx_const_v2.erl b/src/emqx_const_v2.erl index d692dd3b4..f122e6d52 100644 --- a/src/emqx_const_v2.erl +++ b/src/emqx_const_v2.erl @@ -19,8 +19,10 @@ -module(emqx_const_v2). -export([ make_tls_root_fun/2 + , make_tls_verify_fun/2 ]). +-include_lib("public_key/include/public_key.hrl"). %% @doc Build a root fun for verify TLS partial_chain. %% The `InputChain' is composed by OTP SSL with local cert store %% AND the cert (chain if any) from the client. @@ -43,3 +45,78 @@ make_tls_root_fun(cacert_from_cacertfile, [TrustedOne, TrustedTwo]) -> {trusted_ca, TrustedTwo} end end. + +make_tls_verify_fun(verify_cert_extKeyUsage, KeyUsages) -> + AllowedKeyUsages = ext_key_opts(KeyUsages), + fun(A, B, C) -> + verify_fun_peer_extKeyUsage(A, B, C, AllowedKeyUsages) + end. + +verify_fun_peer_extKeyUsage(_, {bad_cert, invalid_ext_key_usage}, UserState, AllowedKeyUsages) -> + %% !! Override OTP verify peer default + %% OTP SSL is unhappy with the ext_key_usage but we will check on ower own. + {unknown, UserState}; +verify_fun_peer_extKeyUsage(_, {bad_cert, _} = Reason, _, AllowedKeyUsages) -> + %% OTP verify_peer default + {fail, Reason}; +verify_fun_peer_extKeyUsage(_, {extension, _}, UserState, _AllowedKeyUsages) -> + %% OTP verify_peer default + {unknown, UserState}; +verify_fun_peer_extKeyUsage(_, valid, UserState, _AllowedKeyUsages) -> + %% OTP verify_peer default + {valid, UserState}; +verify_fun_peer_extKeyUsage(#'OTPCertificate'{tbsCertificate = #'OTPTBSCertificate'{extensions = ExtL}}, + valid_peer, %% valid peer cert + UserState, AllowedKeyUsages) -> + %% override OTP verify_peer default + %% must have id-ce-extKeyUsage + case lists:keyfind(?'id-ce-extKeyUsage', 2, ExtL) of + #'Extension'{extnID = ?'id-ce-extKeyUsage', extnValue = VL} -> + case do_verify_ext_key_usage(VL, AllowedKeyUsages) of + true -> + %% pass the check, + %% fallback to OTP verify_peer default + {valid, UserState}; + false -> + {fail, extKeyUsage_unmatched} + end; + _ -> + {fail, extKeyUsage_not_set} + end. + +%% @doc check required extkeyUsages are presented in the cert +do_verify_ext_key_usage(_, []) -> + %% Verify finished + true; +do_verify_ext_key_usage(CertExtL, [Usage | T] = _Required) -> + case lists:member(Usage, CertExtL) of + true -> + do_verify_ext_key_usage(CertExtL, T); + false -> + false + end. + +%% @doc Helper tls cert extension +-spec ext_key_opts(string()) -> [OidString::string() | public_key:oid()]; + (undefined) -> undefined. +ext_key_opts(undefined) -> + %% disabled + undefined; +ext_key_opts(Str) -> + Usages = string:tokens(Str, ","), + lists:map(fun("clientAuth") -> + ?'id-kp-clientAuth'; + ("serverAuth") -> + ?'id-kp-serverAuth'; + ("codeSigning") -> + ?'id-kp-codeSigning'; + ("emailProtection") -> + ?'id-kp-emailProtection'; + ("timeStamping") -> + ?'id-kp-timeStamping'; + ("ocspSigning") -> + ?'id-kp-OCSPSigning'; + ([$O,$I,$D,$: | OidStr]) -> + OidList = string:tokens(OidStr, "."), + list_to_tuple(lists:map(fun list_to_integer/1, OidList)) + end, Usages). diff --git a/src/emqx_listeners.erl b/src/emqx_listeners.erl index 19963e8b6..baa840d04 100644 --- a/src/emqx_listeners.erl +++ b/src/emqx_listeners.erl @@ -139,7 +139,8 @@ start_listener(Proto, ListenOn, Options0) when Proto == ssl; Proto == tls -> ListenerID = proplists:get_value(listener_id, Options0), Options1 = proplists:delete(listener_id, Options0), Options2 = emqx_ocsp_cache:inject_sni_fun(ListenerID, Options1), - Options = emqx_tls_lib:inject_root_fun(Options2), + Options3 = emqx_tls_lib:inject_root_fun(Options2), + Options = emqx_tls_lib:inject_verify_fun(Options3), ok = maybe_register_crl_urls(Options), start_mqtt_listener('mqtt:ssl', ListenOn, Options); diff --git a/src/emqx_tls_lib.erl b/src/emqx_tls_lib.erl index 720c886a9..92682cebf 100644 --- a/src/emqx_tls_lib.erl +++ b/src/emqx_tls_lib.erl @@ -23,10 +23,13 @@ , integral_ciphers/2 , drop_tls13_for_old_otp/1 , inject_root_fun/1 + , inject_verify_fun/1 , opt_partial_chain/1 + , opt_verify_fun/1 ]). -include("logger.hrl"). +-include_lib("public_key/include/public_key.hrl"). %% non-empty string -define(IS_STRING(L), (is_list(L) andalso L =/= [] andalso is_integer(hd(L)))). @@ -182,6 +185,14 @@ inject_root_fun(Options) -> replace(Options, ssl_options, opt_partial_chain(SslOpts)) end. +inject_verify_fun(Options) -> + case proplists:get_value(ssl_options, Options) of + undefined -> + Options; + SslOpts -> + replace(Options, ssl_options, emqx_tls_lib:opt_verify_fun(SslOpts)) + end. + %% @doc enable TLS partial_chain validation if set. -spec opt_partial_chain(SslOpts :: proplists:proplist()) -> NewSslOpts :: proplists:proplist(). opt_partial_chain(SslOpts) -> @@ -196,6 +207,17 @@ opt_partial_chain(SslOpts) -> replace(SslOpts, partial_chain, rootfun_trusted_ca_from_cacertfile(2, SslOpts)) end. + +-spec opt_verify_fun(SslOpts :: proplists:proplist()) -> NewSslOpts :: proplists:proplist(). +opt_verify_fun(SslOpts) -> + case proplists:get_value(verify_peer_ext_key_usage, SslOpts, undefined) of + undefined -> + SslOpts; + V -> + Fun = emqx_const_v2:make_tls_verify_fun(verify_cert_extKeyUsage, V), + replace(SslOpts, verify_fun, {Fun, #{}}) + end. + replace(Opts, Key, Value) -> [{Key, Value} | proplists:delete(Key, Opts)]. %% @doc Helper, make TLS root_fun diff --git a/test/emqx_listener_tls_verify_keyusage_SUITE.erl b/test/emqx_listener_tls_verify_keyusage_SUITE.erl new file mode 100644 index 000000000..e3134bb99 --- /dev/null +++ b/test/emqx_listener_tls_verify_keyusage_SUITE.erl @@ -0,0 +1,234 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2023 EMQ Technologies Co., Ltd. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%%-------------------------------------------------------------------- +-module(emqx_listener_tls_verify_keyusage_SUITE). + +-compile(export_all). +-compile(nowarn_export_all). + +-include_lib("emqx/include/emqx.hrl"). +-include_lib("emqx/include/emqx_mqtt.hrl"). +-include_lib("eunit/include/eunit.hrl"). +-include_lib("common_test/include/ct.hrl"). + +-import(emqx_test_tls_certs_helper, [ fail_when_ssl_error/1 + , fail_when_no_ssl_alert/2 + , generate_tls_certs/1 + , gen_host_cert/4 + ]). + + +all() -> + [ {group, full_chain} + , {group, partial_chain} + ]. + +all_tc() -> + emqx_ct:all(?MODULE). + +groups() -> + [ {partial_chain, [], all_tc()} + , {full_chain, [], all_tc()} + ]. + +init_per_suite(Config) -> + dbg:tracer(process, {fun dbg:dhandler/2,group_leader()}), + dbg:p(all,c), + dbg:tpl(emqx_tls_lib, opt_verify_fun, cx), + dbg:tpl(emqx_const_v2, verify_fun_peer_extKeyUsage, cx), + dbg:tpl(emqx_const_v2, do_verify_ext_key_usage,cx), + generate_tls_certs(Config), + application:ensure_all_started(esockd), + Config. + +end_per_suite(_Config) -> + application:stop(esockd). + +init_per_group(full_chain, Config)-> + [{ssl_config, ssl_config_verify_peer_full_chain(Config)} | Config]; +init_per_group(partial_chain, Config)-> + [{ssl_config, ssl_config_verify_peer_partial_chain(Config)} | Config]; +init_per_group(_, Config) -> + Config. + +end_per_group(_, Config) -> + Config. + +t_conn_success_verify_peer_ext_key_usage_unset(Config) -> + Port = emqx_test_tls_certs_helper:select_free_port(ssl), + DataDir = ?config(data_dir, Config), + Options = [{ssl_options, ?config(ssl_config, Config)}], + emqx_listeners:start_listener(ssl, Port, Options), + {ok, Socket} = ssl:connect({127, 0, 0, 1}, Port, [{keyfile, filename:join(DataDir, "client1.key")}, + {certfile, filename:join(DataDir, "client1.pem")} + ], 1000), + fail_when_ssl_error(Socket), + ok = ssl:close(Socket). + +t_conn_success_verify_peer_ext_key_usage_undefined(Config) -> + Port = emqx_test_tls_certs_helper:select_free_port(ssl), + DataDir = ?config(data_dir, Config), + Options = [{ssl_options, [ {verify_peer_ext_key_usage, undefined} + | ?config(ssl_config, Config) + ]}], + emqx_listeners:start_listener(ssl, Port, Options), + {ok, Socket} = ssl:connect({127, 0, 0, 1}, Port, [{keyfile, filename:join(DataDir, "client1.key")}, + {certfile, filename:join(DataDir, "client1.pem")} + ], 1000), + fail_when_ssl_error(Socket), + ok = ssl:close(Socket). + +t_conn_success_verify_peer_ext_key_usage_matched_predefined(Config) -> + Port = emqx_test_tls_certs_helper:select_free_port(ssl), + DataDir = ?config(data_dir, Config), + gen_client_cert_ext_keyusage(?FUNCTION_NAME, "intermediate1", DataDir, "clientAuth"), + Options = [{ssl_options, [ {verify_peer_ext_key_usage, "clientAuth"} + | ?config(ssl_config, Config) + ]}], + emqx_listeners:start_listener(ssl, Port, Options), + {ok, Socket} = ssl:connect({127, 0, 0, 1}, Port, [{keyfile, client_key_file(DataDir, ?FUNCTION_NAME)}, + {certfile, client_pem_file(DataDir, ?FUNCTION_NAME)} + ], 1000), + fail_when_ssl_error(Socket), + ok = ssl:close(Socket). + +t_conn_success_verify_peer_ext_key_usage_matched_raw_oid(Config) -> + Port = emqx_test_tls_certs_helper:select_free_port(ssl), + DataDir = ?config(data_dir, Config), + gen_client_cert_ext_keyusage(?FUNCTION_NAME, "intermediate1", DataDir, "clientAuth"), + Options = [{ssl_options, [ {verify_peer_ext_key_usage, "OID:1.3.6.1.5.5.7.3.2"} %% from OTP-PUB-KEY.hrl + | ?config(ssl_config, Config) + ]}], + emqx_listeners:start_listener(ssl, Port, Options), + {ok, Socket} = ssl:connect({127, 0, 0, 1}, Port, [{keyfile, client_key_file(DataDir, ?FUNCTION_NAME)}, + {certfile, client_pem_file(DataDir, ?FUNCTION_NAME)} + ], 1000), + fail_when_ssl_error(Socket), + ok = ssl:close(Socket). + +t_conn_success_verify_peer_ext_key_usage_matched_unorded_list(Config) -> + Port = emqx_test_tls_certs_helper:select_free_port(ssl), + DataDir = ?config(data_dir, Config), + gen_client_cert_ext_keyusage(?FUNCTION_NAME, "intermediate1", DataDir, "clientAuth,serverAuth"), + Options = [{ssl_options, [ {verify_peer_ext_key_usage, "clientAuth,serverAuth"} + | ?config(ssl_config, Config) + ]}], + emqx_listeners:start_listener(ssl, Port, Options), + {ok, Socket} = ssl:connect({127, 0, 0, 1}, Port, [{keyfile, client_key_file(DataDir, ?FUNCTION_NAME)}, + {certfile, client_pem_file(DataDir, ?FUNCTION_NAME)} + ], 1000), + fail_when_ssl_error(Socket), + ok = ssl:close(Socket). + +t_conn_success_verify_peer_ext_key_usage_matched_ordered_list(Config) -> + Port = emqx_test_tls_certs_helper:select_free_port(ssl), + DataDir = ?config(data_dir, Config), + gen_client_cert_ext_keyusage(?FUNCTION_NAME, "intermediate1", DataDir, "clientAuth,serverAuth"), + Options = [{ssl_options, [ {verify_peer_ext_key_usage, "serverAuth,clientAuth"} + | ?config(ssl_config, Config) + ]}], + emqx_listeners:start_listener(ssl, Port, Options), + {ok, Socket} = ssl:connect({127, 0, 0, 1}, Port, [{keyfile, client_key_file(DataDir, ?FUNCTION_NAME)}, + {certfile, client_pem_file(DataDir, ?FUNCTION_NAME)} + ], 1000), + fail_when_ssl_error(Socket), + ok = ssl:close(Socket). + +t_conn_fail_verify_peer_ext_key_usage_unmatched_raw_oid(Config) -> + Port = emqx_test_tls_certs_helper:select_free_port(ssl), + DataDir = ?config(data_dir, Config), + gen_client_cert_ext_keyusage(?FUNCTION_NAME, "intermediate1", DataDir, "clientAuth"), + Options = [{ssl_options, [ {verify_peer_ext_key_usage, "OID:1.3.6.1.5.5.7.3.1"} + | ?config(ssl_config, Config) + ]}], + emqx_listeners:start_listener(ssl, Port, Options), + {ok, Socket} = ssl:connect({127, 0, 0, 1}, Port, [{keyfile, client_key_file(DataDir, ?FUNCTION_NAME)}, + {certfile, client_pem_file(DataDir, ?FUNCTION_NAME)} + ], 1000), + fail_when_no_ssl_alert(Socket, handshake_failure), + ok = ssl:close(Socket). + +t_conn_fail_verify_peer_ext_key_usage_empty_str(Config) -> + Port = emqx_test_tls_certs_helper:select_free_port(ssl), + DataDir = ?config(data_dir, Config), + Options = [{ssl_options, [ {verify_peer_ext_key_usage, ""} + | ?config(ssl_config, Config) + ]}], + emqx_listeners:start_listener(ssl, Port, Options), + {ok, Socket} = ssl:connect({127, 0, 0, 1}, Port, [{keyfile, filename:join(DataDir, "client1.key")}, + {certfile, filename:join(DataDir, "client1.pem")} + ], 1000), + fail_when_no_ssl_alert(Socket, handshake_failure), + ok = ssl:close(Socket). + +t_conn_fail_client_keyusage_unmatch(Config) -> + Port = emqx_test_tls_certs_helper:select_free_port(ssl), + DataDir = ?config(data_dir, Config), + gen_client_cert_ext_keyusage(?FUNCTION_NAME, "intermediate1", DataDir, "codeSigning"), + Options = [{ssl_options, [ {verify_peer_ext_key_usage, "clientAuth"} + | ?config(ssl_config, Config) + ]}], + emqx_listeners:start_listener(ssl, Port, Options), + {ok, Socket} = ssl:connect({127, 0, 0, 1}, Port, [{keyfile, client_key_file(DataDir, ?FUNCTION_NAME)}, + {certfile, client_pem_file(DataDir, ?FUNCTION_NAME)} + ], 1000), + fail_when_no_ssl_alert(Socket, handshake_failure), + ok = ssl:close(Socket). + +t_conn_fail_client_keyusage_incomplete(Config) -> + Port = emqx_test_tls_certs_helper:select_free_port(ssl), + DataDir = ?config(data_dir, Config), + Options = [{ssl_options, [ {verify_peer_ext_key_usage, "codeSigning,clientAuth"} + | ?config(ssl_config, Config) + ]}], + emqx_listeners:start_listener(ssl, Port, Options), + {ok, Socket} = ssl:connect({127, 0, 0, 1}, Port, [{keyfile, filename:join(DataDir, "client1.key")}, + {certfile, filename:join(DataDir, "client1.pem")} + ], 1000), + fail_when_no_ssl_alert(Socket, handshake_failure), + ok = ssl:close(Socket). + + +%%% +%%% Helpers +%%% +gen_client_cert_ext_keyusage(Name, CA, DataDir, Usage) when is_atom(Name) -> + gen_client_cert_ext_keyusage(atom_to_list(Name), CA, DataDir, Usage); +gen_client_cert_ext_keyusage(Name, CA, DataDir, Usage) -> + gen_host_cert(Name, CA, DataDir, #{ext => "extendedKeyUsage=" ++ Usage}). + +client_key_file(DataDir, Name) -> + filename:join(DataDir, Name)++".key". + +client_pem_file(DataDir, Name) -> + filename:join(DataDir, Name)++".pem". + +ssl_config_verify_peer_full_chain(Config) -> + [ {cacertfile, filename:join(?config(data_dir, Config), "intermediate1-root-bundle.pem")} + | ssl_config_verify_peer(Config)]. +ssl_config_verify_peer_partial_chain(Config) -> + [ {cacertfile, filename:join(?config(data_dir, Config), "intermediate1.pem")} + , {partial_chain, true} + | ssl_config_verify_peer(Config)]. + +ssl_config_verify_peer(Config) -> + DataDir = ?config(data_dir, Config), + [ {verify, verify_peer} + , {fail_if_no_peer_cert, true} + , {keyfile, filename:join(DataDir, "server1.key")} + , {certfile, filename:join(DataDir, "server1.pem")} + , {log_level, debug} + ]. + diff --git a/test/emqx_test_tls_certs_helper.erl b/test/emqx_test_tls_certs_helper.erl index 8882f9f40..f64bda0b4 100644 --- a/test/emqx_test_tls_certs_helper.erl +++ b/test/emqx_test_tls_certs_helper.erl @@ -89,8 +89,9 @@ gen_host_cert(H, CaName, Path, Opts) -> HEXT, "keyUsage=digitalSignature,keyAgreement,keyCertSign\n" "basicConstraints=CA:TRUE \n" + "~s \n" "subjectAltName=DNS:~s\n", - [CN] + [maps:get(ext, Opts, ""), CN] ), CSR_Cmd = csr_cmd(PasswordArg, ECKeyFile, HKey, HCSR, CN), From 7346dfe51007253888bd78fcfa705f63308731f4 Mon Sep 17 00:00:00 2001 From: William Yang Date: Fri, 5 May 2023 18:08:00 +0200 Subject: [PATCH 2/6] refactor: verify_fun_peer_extKeyUsage/3 --- src/emqx_const_v2.erl | 18 ++++++++---------- src/emqx_tls_lib.erl | 4 ++-- ...emqx_listener_tls_verify_keyusage_SUITE.erl | 7 +------ 3 files changed, 11 insertions(+), 18 deletions(-) diff --git a/src/emqx_const_v2.erl b/src/emqx_const_v2.erl index f122e6d52..9efeb6dde 100644 --- a/src/emqx_const_v2.erl +++ b/src/emqx_const_v2.erl @@ -47,27 +47,25 @@ make_tls_root_fun(cacert_from_cacertfile, [TrustedOne, TrustedTwo]) -> end. make_tls_verify_fun(verify_cert_extKeyUsage, KeyUsages) -> - AllowedKeyUsages = ext_key_opts(KeyUsages), - fun(A, B, C) -> - verify_fun_peer_extKeyUsage(A, B, C, AllowedKeyUsages) - end. + AllowedKeyUsages = ext_key_opts(KeyUsages), + {fun verify_fun_peer_extKeyUsage/3, AllowedKeyUsages}. -verify_fun_peer_extKeyUsage(_, {bad_cert, invalid_ext_key_usage}, UserState, AllowedKeyUsages) -> +verify_fun_peer_extKeyUsage(_, {bad_cert, invalid_ext_key_usage}, UserState) -> %% !! Override OTP verify peer default %% OTP SSL is unhappy with the ext_key_usage but we will check on ower own. {unknown, UserState}; -verify_fun_peer_extKeyUsage(_, {bad_cert, _} = Reason, _, AllowedKeyUsages) -> +verify_fun_peer_extKeyUsage(_, {bad_cert, _} = Reason, _UserState) -> %% OTP verify_peer default {fail, Reason}; -verify_fun_peer_extKeyUsage(_, {extension, _}, UserState, _AllowedKeyUsages) -> +verify_fun_peer_extKeyUsage(_, {extension, _}, UserState) -> %% OTP verify_peer default {unknown, UserState}; -verify_fun_peer_extKeyUsage(_, valid, UserState, _AllowedKeyUsages) -> +verify_fun_peer_extKeyUsage(_, valid, UserState) -> %% OTP verify_peer default {valid, UserState}; verify_fun_peer_extKeyUsage(#'OTPCertificate'{tbsCertificate = #'OTPTBSCertificate'{extensions = ExtL}}, valid_peer, %% valid peer cert - UserState, AllowedKeyUsages) -> + AllowedKeyUsages) -> %% override OTP verify_peer default %% must have id-ce-extKeyUsage case lists:keyfind(?'id-ce-extKeyUsage', 2, ExtL) of @@ -76,7 +74,7 @@ verify_fun_peer_extKeyUsage(#'OTPCertificate'{tbsCertificate = #'OTPTBSCertifica true -> %% pass the check, %% fallback to OTP verify_peer default - {valid, UserState}; + {valid, AllowedKeyUsages}; false -> {fail, extKeyUsage_unmatched} end; diff --git a/src/emqx_tls_lib.erl b/src/emqx_tls_lib.erl index 92682cebf..bebc891d6 100644 --- a/src/emqx_tls_lib.erl +++ b/src/emqx_tls_lib.erl @@ -214,8 +214,8 @@ opt_verify_fun(SslOpts) -> undefined -> SslOpts; V -> - Fun = emqx_const_v2:make_tls_verify_fun(verify_cert_extKeyUsage, V), - replace(SslOpts, verify_fun, {Fun, #{}}) + VerifyFun = emqx_const_v2:make_tls_verify_fun(verify_cert_extKeyUsage, V), + replace(SslOpts, verify_fun, VerifyFun) end. replace(Opts, Key, Value) -> [{Key, Value} | proplists:delete(Key, Opts)]. diff --git a/test/emqx_listener_tls_verify_keyusage_SUITE.erl b/test/emqx_listener_tls_verify_keyusage_SUITE.erl index e3134bb99..3ba089aae 100644 --- a/test/emqx_listener_tls_verify_keyusage_SUITE.erl +++ b/test/emqx_listener_tls_verify_keyusage_SUITE.erl @@ -44,11 +44,6 @@ groups() -> ]. init_per_suite(Config) -> - dbg:tracer(process, {fun dbg:dhandler/2,group_leader()}), - dbg:p(all,c), - dbg:tpl(emqx_tls_lib, opt_verify_fun, cx), - dbg:tpl(emqx_const_v2, verify_fun_peer_extKeyUsage, cx), - dbg:tpl(emqx_const_v2, do_verify_ext_key_usage,cx), generate_tls_certs(Config), application:ensure_all_started(esockd), Config. @@ -229,6 +224,6 @@ ssl_config_verify_peer(Config) -> , {fail_if_no_peer_cert, true} , {keyfile, filename:join(DataDir, "server1.key")} , {certfile, filename:join(DataDir, "server1.pem")} - , {log_level, debug} + %% , {log_level, debug} ]. From 535d040554b2d9a8ca7bc320ec8f95beee139e25 Mon Sep 17 00:00:00 2001 From: William Yang Date: Thu, 11 May 2023 12:01:54 +0200 Subject: [PATCH 3/6] docs: changelog for tls client cert keyusage validation --- changes/v4.4.19-en.md | 2 ++ changes/v4.4.19-zh.md | 2 ++ 2 files changed, 4 insertions(+) diff --git a/changes/v4.4.19-en.md b/changes/v4.4.19-en.md index 93f604213..7cf076dfb 100644 --- a/changes/v4.4.19-en.md +++ b/changes/v4.4.19-en.md @@ -20,6 +20,8 @@ - Adds a new feature to enable partial certificate chain validation for TLS listeners[#10553](https://github.com/emqx/emqx/pull/10553). If partial_chain is set to 'true', the last certificate in cacertfile is treated as the terminal of the certificate trust-chain. That is, the TLS handshake does not require full trust-chain, and EMQX will not try to validate the chain all the way up to the root CA. +- Adds a new feature to enable client certificate extended key usage validation for TLS listeners[#10669](https://github.com/emqx/emqx/pull/10669). + ## Bug fixes - Fixed an issue where the rule engine was unable to access variables exported by `FOREACH` in the `DO` clause [#10620](https://github.com/emqx/emqx/pull/10620). diff --git a/changes/v4.4.19-zh.md b/changes/v4.4.19-zh.md index dc5a77fac..d94996920 100644 --- a/changes/v4.4.19-zh.md +++ b/changes/v4.4.19-zh.md @@ -19,6 +19,8 @@ - 增加了一个新的功能,为 TLS 监听器启用部分证书链验证[#10553](https://github.com/emqx/emqx/pull/10553)。 如果 partial_chain 设置为“true”,cacertfile 中的最后一个证书将被视为证书信任链的顶端证书。 也就是说,TLS 握手不需要完整的链,并且 EMQX 不会尝试一直验证链直到根 CA。 + +- 增加了一个新功能,为 TLS 监听器启用客户端证书扩展密钥使用验证 [#10669](https://github.com/emqx/emqx/pull/10669)。 ## 修复 From 4ac2f6d2058e8d87bd26b9ddd61fbc3702ce1cd1 Mon Sep 17 00:00:00 2001 From: William Yang Date: Fri, 12 May 2023 09:24:16 +0200 Subject: [PATCH 4/6] fix: default value --- priv/emqx.schema | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/priv/emqx.schema b/priv/emqx.schema index e53c18d6e..700752d6d 100644 --- a/priv/emqx.schema +++ b/priv/emqx.schema @@ -1652,7 +1652,7 @@ end}. {mapping, "listener.ssl.$name.verify_peer_ext_key_usage", "emqx.listeners", [ {datatype, string}, - {default, ""} + {default, undefined} ]}. {mapping, "listener.ssl.$name.fail_if_no_peer_cert", "emqx.listeners", [ From 64955e9083611792e88c307c8e5361f35157636a Mon Sep 17 00:00:00 2001 From: William Yang Date: Fri, 12 May 2023 11:33:04 +0200 Subject: [PATCH 5/6] test(tls-keyusage): add some comments --- ...mqx_listener_tls_verify_keyusage_SUITE.erl | 73 ++++++++++++++----- 1 file changed, 54 insertions(+), 19 deletions(-) diff --git a/test/emqx_listener_tls_verify_keyusage_SUITE.erl b/test/emqx_listener_tls_verify_keyusage_SUITE.erl index 3ba089aae..197373e8d 100644 --- a/test/emqx_listener_tls_verify_keyusage_SUITE.erl +++ b/test/emqx_listener_tls_verify_keyusage_SUITE.erl @@ -64,94 +64,119 @@ end_per_group(_, Config) -> t_conn_success_verify_peer_ext_key_usage_unset(Config) -> Port = emqx_test_tls_certs_helper:select_free_port(ssl), DataDir = ?config(data_dir, Config), + %% Given listener keyusage unset Options = [{ssl_options, ?config(ssl_config, Config)}], emqx_listeners:start_listener(ssl, Port, Options), + %% when client connect with cert without keyusage ext {ok, Socket} = ssl:connect({127, 0, 0, 1}, Port, [{keyfile, filename:join(DataDir, "client1.key")}, {certfile, filename:join(DataDir, "client1.pem")} ], 1000), + %% Then connection success fail_when_ssl_error(Socket), ok = ssl:close(Socket). t_conn_success_verify_peer_ext_key_usage_undefined(Config) -> Port = emqx_test_tls_certs_helper:select_free_port(ssl), DataDir = ?config(data_dir, Config), + %% Give listener keyusage is set to undefined Options = [{ssl_options, [ {verify_peer_ext_key_usage, undefined} | ?config(ssl_config, Config) ]}], emqx_listeners:start_listener(ssl, Port, Options), + %% when client connect with cert without keyusages ext {ok, Socket} = ssl:connect({127, 0, 0, 1}, Port, [{keyfile, filename:join(DataDir, "client1.key")}, {certfile, filename:join(DataDir, "client1.pem")} ], 1000), + %% Then connection success fail_when_ssl_error(Socket), ok = ssl:close(Socket). t_conn_success_verify_peer_ext_key_usage_matched_predefined(Config) -> Port = emqx_test_tls_certs_helper:select_free_port(ssl), DataDir = ?config(data_dir, Config), - gen_client_cert_ext_keyusage(?FUNCTION_NAME, "intermediate1", DataDir, "clientAuth"), + %% Give listener keyusage is set to clientAuth Options = [{ssl_options, [ {verify_peer_ext_key_usage, "clientAuth"} | ?config(ssl_config, Config) ]}], + + %% When client cert has clientAuth that is matched + gen_client_cert_ext_keyusage(?FUNCTION_NAME, "intermediate1", DataDir, "clientAuth"), emqx_listeners:start_listener(ssl, Port, Options), {ok, Socket} = ssl:connect({127, 0, 0, 1}, Port, [{keyfile, client_key_file(DataDir, ?FUNCTION_NAME)}, {certfile, client_pem_file(DataDir, ?FUNCTION_NAME)} ], 1000), + %% Then connection success fail_when_ssl_error(Socket), ok = ssl:close(Socket). t_conn_success_verify_peer_ext_key_usage_matched_raw_oid(Config) -> Port = emqx_test_tls_certs_helper:select_free_port(ssl), DataDir = ?config(data_dir, Config), - gen_client_cert_ext_keyusage(?FUNCTION_NAME, "intermediate1", DataDir, "clientAuth"), + %% Give listener keyusage is set to raw OID Options = [{ssl_options, [ {verify_peer_ext_key_usage, "OID:1.3.6.1.5.5.7.3.2"} %% from OTP-PUB-KEY.hrl | ?config(ssl_config, Config) ]}], emqx_listeners:start_listener(ssl, Port, Options), + %% When client cert has keyusage and matched. + gen_client_cert_ext_keyusage(?FUNCTION_NAME, "intermediate1", DataDir, "clientAuth"), {ok, Socket} = ssl:connect({127, 0, 0, 1}, Port, [{keyfile, client_key_file(DataDir, ?FUNCTION_NAME)}, {certfile, client_pem_file(DataDir, ?FUNCTION_NAME)} ], 1000), - fail_when_ssl_error(Socket), - ok = ssl:close(Socket). - -t_conn_success_verify_peer_ext_key_usage_matched_unorded_list(Config) -> - Port = emqx_test_tls_certs_helper:select_free_port(ssl), - DataDir = ?config(data_dir, Config), - gen_client_cert_ext_keyusage(?FUNCTION_NAME, "intermediate1", DataDir, "clientAuth,serverAuth"), - Options = [{ssl_options, [ {verify_peer_ext_key_usage, "clientAuth,serverAuth"} - | ?config(ssl_config, Config) - ]}], - emqx_listeners:start_listener(ssl, Port, Options), - {ok, Socket} = ssl:connect({127, 0, 0, 1}, Port, [{keyfile, client_key_file(DataDir, ?FUNCTION_NAME)}, - {certfile, client_pem_file(DataDir, ?FUNCTION_NAME)} - ], 1000), + %% Then connection success fail_when_ssl_error(Socket), ok = ssl:close(Socket). t_conn_success_verify_peer_ext_key_usage_matched_ordered_list(Config) -> Port = emqx_test_tls_certs_helper:select_free_port(ssl), DataDir = ?config(data_dir, Config), + + %% Give listener keyusage is clientAuth,serverAuth + Options = [{ssl_options, [ {verify_peer_ext_key_usage, "clientAuth,serverAuth"} + | ?config(ssl_config, Config) + ]}], + emqx_listeners:start_listener(ssl, Port, Options), + %% When client cert has the same keyusage ext list gen_client_cert_ext_keyusage(?FUNCTION_NAME, "intermediate1", DataDir, "clientAuth,serverAuth"), + {ok, Socket} = ssl:connect({127, 0, 0, 1}, Port, [{keyfile, client_key_file(DataDir, ?FUNCTION_NAME)}, + {certfile, client_pem_file(DataDir, ?FUNCTION_NAME)} + ], 1000), + %% Then connection success + fail_when_ssl_error(Socket), + ok = ssl:close(Socket). + +t_conn_success_verify_peer_ext_key_usage_matched_unordered_list(Config) -> + Port = emqx_test_tls_certs_helper:select_free_port(ssl), + DataDir = ?config(data_dir, Config), + %% Give listener keyusage is clientAuth,serverAuth Options = [{ssl_options, [ {verify_peer_ext_key_usage, "serverAuth,clientAuth"} | ?config(ssl_config, Config) ]}], emqx_listeners:start_listener(ssl, Port, Options), + %% When client cert has the same keyusage ext list but different order + gen_client_cert_ext_keyusage(?FUNCTION_NAME, "intermediate1", DataDir, "clientAuth,serverAuth"), {ok, Socket} = ssl:connect({127, 0, 0, 1}, Port, [{keyfile, client_key_file(DataDir, ?FUNCTION_NAME)}, {certfile, client_pem_file(DataDir, ?FUNCTION_NAME)} ], 1000), + %% Then connection success fail_when_ssl_error(Socket), ok = ssl:close(Socket). t_conn_fail_verify_peer_ext_key_usage_unmatched_raw_oid(Config) -> Port = emqx_test_tls_certs_helper:select_free_port(ssl), DataDir = ?config(data_dir, Config), - gen_client_cert_ext_keyusage(?FUNCTION_NAME, "intermediate1", DataDir, "clientAuth"), + %% Give listener keyusage is using OID Options = [{ssl_options, [ {verify_peer_ext_key_usage, "OID:1.3.6.1.5.5.7.3.1"} | ?config(ssl_config, Config) ]}], emqx_listeners:start_listener(ssl, Port, Options), + + %% When client cert has the keyusage but not matching OID + gen_client_cert_ext_keyusage(?FUNCTION_NAME, "intermediate1", DataDir, "clientAuth"), {ok, Socket} = ssl:connect({127, 0, 0, 1}, Port, [{keyfile, client_key_file(DataDir, ?FUNCTION_NAME)}, {certfile, client_pem_file(DataDir, ?FUNCTION_NAME)} ], 1000), + + %% Then connecion should fail. fail_when_no_ssl_alert(Socket, handshake_failure), ok = ssl:close(Socket). @@ -161,41 +186,51 @@ t_conn_fail_verify_peer_ext_key_usage_empty_str(Config) -> Options = [{ssl_options, [ {verify_peer_ext_key_usage, ""} | ?config(ssl_config, Config) ]}], + %% Give listener keyusage is empty string emqx_listeners:start_listener(ssl, Port, Options), + %% When client connect with cert without keyusage {ok, Socket} = ssl:connect({127, 0, 0, 1}, Port, [{keyfile, filename:join(DataDir, "client1.key")}, {certfile, filename:join(DataDir, "client1.pem")} ], 1000), + %% Then connecion should fail. fail_when_no_ssl_alert(Socket, handshake_failure), ok = ssl:close(Socket). t_conn_fail_client_keyusage_unmatch(Config) -> Port = emqx_test_tls_certs_helper:select_free_port(ssl), DataDir = ?config(data_dir, Config), - gen_client_cert_ext_keyusage(?FUNCTION_NAME, "intermediate1", DataDir, "codeSigning"), + + %% Give listener keyusage is clientAuth Options = [{ssl_options, [ {verify_peer_ext_key_usage, "clientAuth"} | ?config(ssl_config, Config) ]}], emqx_listeners:start_listener(ssl, Port, Options), + %% When client connect with mismatch cert keyusage = codeSigning + gen_client_cert_ext_keyusage(?FUNCTION_NAME, "intermediate1", DataDir, "codeSigning"), {ok, Socket} = ssl:connect({127, 0, 0, 1}, Port, [{keyfile, client_key_file(DataDir, ?FUNCTION_NAME)}, {certfile, client_pem_file(DataDir, ?FUNCTION_NAME)} ], 1000), + %% Then connecion should fail. fail_when_no_ssl_alert(Socket, handshake_failure), ok = ssl:close(Socket). t_conn_fail_client_keyusage_incomplete(Config) -> Port = emqx_test_tls_certs_helper:select_free_port(ssl), DataDir = ?config(data_dir, Config), + %% Give listener keyusage is codeSigning,clientAuth Options = [{ssl_options, [ {verify_peer_ext_key_usage, "codeSigning,clientAuth"} | ?config(ssl_config, Config) ]}], emqx_listeners:start_listener(ssl, Port, Options), + %% When client connect with cert keyusage = clientAuth + gen_client_cert_ext_keyusage(?FUNCTION_NAME, "intermediate1", DataDir, "codeSigning"), {ok, Socket} = ssl:connect({127, 0, 0, 1}, Port, [{keyfile, filename:join(DataDir, "client1.key")}, {certfile, filename:join(DataDir, "client1.pem")} ], 1000), + %% Then connection should fail fail_when_no_ssl_alert(Socket, handshake_failure), ok = ssl:close(Socket). - %%% %%% Helpers %%% From dfabc7ca725f1e040ae08a020bcec50ff3c9f2d9 Mon Sep 17 00:00:00 2001 From: William Yang Date: Fri, 12 May 2023 18:13:21 +0200 Subject: [PATCH 6/6] chore: improve coverage --- src/emqx_const_v2.erl | 5 +---- test/emqx_listener_tls_verify_keyusage_SUITE.erl | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/emqx_const_v2.erl b/src/emqx_const_v2.erl index 9efeb6dde..cb7043955 100644 --- a/src/emqx_const_v2.erl +++ b/src/emqx_const_v2.erl @@ -52,7 +52,7 @@ make_tls_verify_fun(verify_cert_extKeyUsage, KeyUsages) -> verify_fun_peer_extKeyUsage(_, {bad_cert, invalid_ext_key_usage}, UserState) -> %% !! Override OTP verify peer default - %% OTP SSL is unhappy with the ext_key_usage but we will check on ower own. + %% OTP SSL is unhappy with the ext_key_usage but we will check on our own. {unknown, UserState}; verify_fun_peer_extKeyUsage(_, {bad_cert, _} = Reason, _UserState) -> %% OTP verify_peer default @@ -97,9 +97,6 @@ do_verify_ext_key_usage(CertExtL, [Usage | T] = _Required) -> %% @doc Helper tls cert extension -spec ext_key_opts(string()) -> [OidString::string() | public_key:oid()]; (undefined) -> undefined. -ext_key_opts(undefined) -> - %% disabled - undefined; ext_key_opts(Str) -> Usages = string:tokens(Str, ","), lists:map(fun("clientAuth") -> diff --git a/test/emqx_listener_tls_verify_keyusage_SUITE.erl b/test/emqx_listener_tls_verify_keyusage_SUITE.erl index 197373e8d..07acc0eef 100644 --- a/test/emqx_listener_tls_verify_keyusage_SUITE.erl +++ b/test/emqx_listener_tls_verify_keyusage_SUITE.erl @@ -218,7 +218,7 @@ t_conn_fail_client_keyusage_incomplete(Config) -> Port = emqx_test_tls_certs_helper:select_free_port(ssl), DataDir = ?config(data_dir, Config), %% Give listener keyusage is codeSigning,clientAuth - Options = [{ssl_options, [ {verify_peer_ext_key_usage, "codeSigning,clientAuth"} + Options = [{ssl_options, [ {verify_peer_ext_key_usage, "serverAuth,clientAuth,codeSigning,emailProtection,timeStamping,ocspSigning"} | ?config(ssl_config, Config) ]}], emqx_listeners:start_listener(ssl, Port, Options),