diff --git a/apps/emqx_authn/src/emqx_authn_schema.erl b/apps/emqx_authn/src/emqx_authn_schema.erl index 112ea2076..a7cdaac5f 100644 --- a/apps/emqx_authn/src/emqx_authn_schema.erl +++ b/apps/emqx_authn/src/emqx_authn_schema.erl @@ -18,10 +18,12 @@ -elvis([{elvis_style, invalid_dynamic_call, disable}]). -include_lib("hocon/include/hoconsc.hrl"). +-include("emqx_authn.hrl"). -export([ common_fields/0, roots/0, + validations/0, tags/0, fields/1, authenticator_type/0, @@ -207,3 +209,27 @@ array(Name) -> array(Name, DescId) -> {Name, ?HOCON(?R_REF(Name), #{desc => ?DESC(DescId)})}. + +validations() -> + [ + {check_http_ssl_opts, fun(Conf) -> + CheckFun = fun emqx_authn_http:check_ssl_opts/1, + validation(Conf, CheckFun) + end}, + {check_http_headers, fun(Conf) -> + CheckFun = fun emqx_authn_http:check_headers/1, + validation(Conf, CheckFun) + end} + ]. + +validation(Conf, CheckFun) when is_map(Conf) -> + validation(hocon_maps:get(?CONF_NS, Conf), CheckFun); +validation(undefined, _) -> + ok; +validation([], _) -> + ok; +validation([AuthN | Tail], CheckFun) -> + case CheckFun(#{?EMQX_AUTHENTICATION_CONFIG_ROOT_NAME_BINARY => AuthN}) of + ok -> validation(Tail, CheckFun); + Error -> Error + end. diff --git a/apps/emqx_authn/src/simple_authn/emqx_authn_http.erl b/apps/emqx_authn/src/simple_authn/emqx_authn_http.erl index 3c34d878e..43701cbc7 100644 --- a/apps/emqx_authn/src/simple_authn/emqx_authn_http.erl +++ b/apps/emqx_authn/src/simple_authn/emqx_authn_http.erl @@ -38,6 +38,8 @@ headers/1 ]). +-export([check_headers/1, check_ssl_opts/1]). + -export([ refs/0, union_member_selector/1, @@ -106,8 +108,8 @@ common_fields() -> validations() -> [ - {check_ssl_opts, fun check_ssl_opts/1}, - {check_headers, fun check_headers/1} + {check_ssl_opts, fun ?MODULE:check_ssl_opts/1}, + {check_headers, fun ?MODULE:check_headers/1} ]. url(type) -> binary(); @@ -261,21 +263,47 @@ transform_header_name(Headers) -> ). check_ssl_opts(Conf) -> - {BaseUrl, _Path, _Query} = parse_url(get_conf_val("url", Conf)), - case BaseUrl of - <<"https://", _/binary>> -> - case get_conf_val("ssl.enable", Conf) of - true -> ok; - false -> false + case is_backend_http(Conf) of + true -> + Url = get_conf_val("url", Conf), + {BaseUrl, _Path, _Query} = parse_url(Url), + case BaseUrl of + <<"https://", _/binary>> -> + case get_conf_val("ssl.enable", Conf) of + true -> + ok; + false -> + <<"it's required to enable the TLS option to establish a https connection">> + end; + <<"http://", _/binary>> -> + ok end; - <<"http://", _/binary>> -> + false -> ok end. check_headers(Conf) -> - Method = to_bin(get_conf_val("method", Conf)), - Headers = get_conf_val("headers", Conf), - Method =:= <<"post">> orelse (not maps:is_key(<<"content-type">>, Headers)). + case is_backend_http(Conf) of + true -> + Headers = get_conf_val("headers", Conf), + case to_bin(get_conf_val("method", Conf)) of + <<"post">> -> + ok; + <<"get">> -> + case maps:is_key(<<"content-type">>, Headers) of + false -> ok; + true -> <<"HTTP GET requests cannot include content-type header.">> + end + end; + false -> + ok + end. + +is_backend_http(Conf) -> + case get_conf_val("backend", Conf) of + http -> true; + _ -> false + end. parse_url(Url) -> case string:split(Url, "//", leading) of @@ -310,7 +338,7 @@ parse_config( method => Method, path => Path, headers => ensure_header_name_type(Headers), - base_path_templete => emqx_authn_utils:parse_str(Path), + base_path_template => emqx_authn_utils:parse_str(Path), base_query_template => emqx_authn_utils:parse_deep( cow_qs:parse_qs(to_bin(Query)) ), @@ -323,7 +351,7 @@ parse_config( generate_request(Credential, #{ method := Method, headers := Headers0, - base_path_templete := BasePathTemplate, + base_path_template := BasePathTemplate, base_query_template := BaseQueryTemplate, body_template := BodyTemplate }) -> diff --git a/apps/emqx_conf/test/emqx_conf_schema_tests.erl b/apps/emqx_conf/test/emqx_conf_schema_tests.erl index 3653b9d19..192aa0e93 100644 --- a/apps/emqx_conf/test/emqx_conf_schema_tests.erl +++ b/apps/emqx_conf/test/emqx_conf_schema_tests.erl @@ -6,6 +6,116 @@ -include_lib("eunit/include/eunit.hrl"). +%% erlfmt-ignore +-define(BASE_CONF, + """ + node { + name = \"emqx1@127.0.0.1\" + cookie = \"emqxsecretcookie\" + data_dir = \"data\" + } + cluster { + name = emqxcl + discovery_strategy = static + static.seeds = ~p + core_nodes = ~p + } + """). + +array_nodes_test() -> + ExpectNodes = ['emqx1@127.0.0.1', 'emqx2@127.0.0.1'], + lists:foreach( + fun({Seeds, Nodes}) -> + ConfFile = to_bin(?BASE_CONF, [Seeds, Nodes]), + {ok, Conf} = hocon:binary(ConfFile, #{format => richmap}), + ConfList = hocon_tconf:generate(emqx_conf_schema, Conf), + ClusterDiscovery = proplists:get_value( + cluster_discovery, proplists:get_value(ekka, ConfList) + ), + ?assertEqual( + {static, [{seeds, ExpectNodes}]}, + ClusterDiscovery, + Nodes + ), + ?assertEqual( + ExpectNodes, + proplists:get_value(core_nodes, proplists:get_value(mria, ConfList)), + Nodes + ) + end, + [{["emqx1@127.0.0.1", "emqx2@127.0.0.1"], "emqx1@127.0.0.1, emqx2@127.0.0.1"}] + ), + ok. + +%% erlfmt-ignore +-define(BASE_AUTHN_ARRAY, + """ + authentication = [ + {backend = \"http\" + body {password = \"${password}\", username = \"${username}\"} + connect_timeout = \"15s\" + enable_pipelining = 100 + headers {\"content-type\" = \"application/json\"} + mechanism = \"password_based\" + method = \"~p\" + pool_size = 8 + request_timeout = \"5s\" + ssl {enable = ~p, verify = \"verify_peer\"} + url = \"~ts\" + } + ] + """ +). + +-define(ERROR(Reason), + {emqx_conf_schema, [ + #{ + kind := validation_error, + reason := integrity_validation_failure, + result := _, + validation_name := Reason + } + ]} +). + +authn_validations_test() -> + BaseConf = to_bin(?BASE_CONF, [["emqx1@127.0.0.1"], "emqx1@127.0.0.1,emqx1@127.0.0.1"]), + + OKHttps = to_bin(?BASE_AUTHN_ARRAY, [post, true, <<"https://127.0.0.1:8080">>]), + Conf0 = <>, + {ok, ConfMap0} = hocon:binary(Conf0, #{format => richmap}), + ?assert(is_list(hocon_tconf:generate(emqx_conf_schema, ConfMap0))), + + OKHttp = to_bin(?BASE_AUTHN_ARRAY, [post, false, <<"http://127.0.0.1:8080">>]), + Conf1 = <>, + {ok, ConfMap1} = hocon:binary(Conf1, #{format => richmap}), + ?assert(is_list(hocon_tconf:generate(emqx_conf_schema, ConfMap1))), + + DisableSSLWithHttps = to_bin(?BASE_AUTHN_ARRAY, [post, false, <<"https://127.0.0.1:8080">>]), + Conf2 = <>, + {ok, ConfMap2} = hocon:binary(Conf2, #{format => richmap}), + ?assertThrow( + ?ERROR(check_http_ssl_opts), + hocon_tconf:generate(emqx_conf_schema, ConfMap2) + ), + + BadHeader = to_bin(?BASE_AUTHN_ARRAY, [get, true, <<"https://127.0.0.1:8080">>]), + Conf3 = <>, + {ok, ConfMap3} = hocon:binary(Conf3, #{format => richmap}), + ?assertThrow( + ?ERROR(check_http_headers), + hocon_tconf:generate(emqx_conf_schema, ConfMap3) + ), + + BadHeaderWithTuple = binary:replace(BadHeader, [<<"[">>, <<"]">>], <<"">>, [global]), + Conf4 = <>, + {ok, ConfMap4} = hocon:binary(Conf4, #{format => richmap}), + ?assertThrow( + ?ERROR(check_http_headers), + hocon_tconf:generate(emqx_conf_schema, ConfMap4) + ), + ok. + doc_gen_test() -> %% the json file too large to encode. { @@ -26,3 +136,6 @@ doc_gen_test() -> ok end }. + +to_bin(Format, Args) -> + iolist_to_binary(io_lib:format(Format, Args)). diff --git a/apps/emqx_prometheus/TODO b/apps/emqx_prometheus/TODO deleted file mode 100644 index a868fba7e..000000000 --- a/apps/emqx_prometheus/TODO +++ /dev/null @@ -1,2 +0,0 @@ -1. Add more VM Metrics -2. Add more emqx Metrics diff --git a/changes/ce/fix-10449.en.md b/changes/ce/fix-10449.en.md new file mode 100644 index 000000000..005dea73c --- /dev/null +++ b/changes/ce/fix-10449.en.md @@ -0,0 +1,2 @@ +Validate the ssl_options and header configurations when creating authentication http (`authn_http`). +Prior to this, incorrect `ssl` configuration could result in successful creation but the entire authn being unusable.