From e291dcdd18279369042279f5445c9b978ba45bf4 Mon Sep 17 00:00:00 2001 From: Kjell Winblad Date: Mon, 1 Jul 2024 13:11:19 +0200 Subject: [PATCH 01/41] fix: default value for max_conn_rate etc should be set to infinity Before this commit the default value for the fields max_conn_rate, messages_rate and bytes_rate were not set. This is fixed by setting the default value to infinity. This breaks the corresponding dashboard fields (they can not be edited) so the dashboard also needs to be updated. Fixes: https://emqx.atlassian.net/browse/EMQX-12514 --- .../emqx_limiter/src/emqx_limiter_schema.erl | 1 + .../test/emqx_mgmt_api_listeners_SUITE.erl | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/apps/emqx/src/emqx_limiter/src/emqx_limiter_schema.erl b/apps/emqx/src/emqx_limiter/src/emqx_limiter_schema.erl index 8a10a7be2..b9bfda166 100644 --- a/apps/emqx/src/emqx_limiter/src/emqx_limiter_schema.erl +++ b/apps/emqx/src/emqx_limiter/src/emqx_limiter_schema.erl @@ -215,6 +215,7 @@ short_paths_fields(Importance) -> ?HOCON(rate_type(), #{ desc => ?DESC(Name), required => false, + default => infinity, importance => Importance, example => Example })} diff --git a/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl index 02a098f28..227ae0107 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl @@ -418,6 +418,35 @@ t_update_listener_zone(_Config) -> ?assertMatch({error, {_, 400, _}}, request(put, Path, [], AddConf1)), ?assertMatch(#{<<"zone">> := <<"zone1">>}, request(put, Path, [], AddConf2)). +t_update_listener_max_conn_rate({init, Config}) -> + Config; +t_update_listener_max_conn_rate({'end', _Config}) -> + ok; +t_update_listener_max_conn_rate(_Config) -> + ListenerId = <<"tcp:default">>, + Path = emqx_mgmt_api_test_util:api_path(["listeners", ListenerId]), + Conf = request(get, Path, [], []), + %% Check that default is infinity + ?assertMatch(#{<<"max_conn_rate">> := <<"infinity">>}, Conf), + %% Update to infinity + UpdateConfToInfinity = Conf#{<<"max_conn_rate">> => <<"infinity">>}, + ?assertMatch( + #{<<"max_conn_rate">> := <<"infinity">>}, + request(put, Path, [], UpdateConfToInfinity) + ), + %% Update to 42/s + UpdateConfTo42PerSec = Conf#{<<"max_conn_rate">> => <<"42/s">>}, + ?assertMatch( + #{<<"max_conn_rate">> := <<"42/s">>}, + request(put, Path, [], UpdateConfTo42PerSec) + ), + %% Update back to infinity + UpdateConfToInfinity = Conf#{<<"max_conn_rate">> => <<"infinity">>}, + ?assertMatch( + #{<<"max_conn_rate">> := <<"infinity">>}, + request(put, Path, [], UpdateConfToInfinity) + ). + t_delete_nonexistent_listener(Config) when is_list(Config) -> NonExist = emqx_mgmt_api_test_util:api_path(["listeners", "tcp:nonexistent"]), ?assertMatch( From 82bb03a2a3be987ed2d2775e19c857c65b31fa71 Mon Sep 17 00:00:00 2001 From: Kjell Winblad Date: Mon, 1 Jul 2024 13:30:51 +0200 Subject: [PATCH 02/41] docs: add change log entry --- changes/ce/fix-13375.en.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/ce/fix-13375.en.md diff --git a/changes/ce/fix-13375.en.md b/changes/ce/fix-13375.en.md new file mode 100644 index 000000000..e09bc649b --- /dev/null +++ b/changes/ce/fix-13375.en.md @@ -0,0 +1 @@ +The value infinity has been added as default value to the listener configuration fields max_conn_rate, messages_rate and bytes_rate. From c3579f338b1104146714f455824830b8903838dd Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Wed, 3 Jul 2024 10:16:18 -0300 Subject: [PATCH 03/41] fix(authz api): add new `ignore` metric to status response Fixes https://emqx.atlassian.net/browse/EMQX-12411 --- .../src/emqx_authz/emqx_authz_api_sources.erl | 9 +++- .../src/emqx_authz/emqx_authz_schema.erl | 1 + .../test/emqx_authz_http_SUITE.erl | 47 +++++++++++++++++-- .../test/emqx_mgmt_api_test_util.erl | 8 ++++ rel/i18n/emqx_authz_schema.hocon | 5 ++ 5 files changed, 66 insertions(+), 4 deletions(-) diff --git a/apps/emqx_auth/src/emqx_authz/emqx_authz_api_sources.erl b/apps/emqx_auth/src/emqx_authz/emqx_authz_api_sources.erl index 27ef53d14..a8f91a302 100644 --- a/apps/emqx_auth/src/emqx_authz/emqx_authz_api_sources.erl +++ b/apps/emqx_auth/src/emqx_authz/emqx_authz_api_sources.erl @@ -467,7 +467,13 @@ make_result_map(ResList) -> lists:foldl(Fun, {maps:new(), maps:new(), maps:new(), maps:new()}, ResList). restructure_map(#{ - counters := #{deny := Failed, total := Total, allow := Succ, nomatch := Nomatch}, + counters := #{ + ignore := Ignore, + deny := Failed, + total := Total, + allow := Succ, + nomatch := Nomatch + }, rate := #{total := #{current := Rate, last5m := Rate5m, max := RateMax}} }) -> #{ @@ -475,6 +481,7 @@ restructure_map(#{ allow => Succ, deny => Failed, nomatch => Nomatch, + ignore => Ignore, rate => Rate, rate_last5m => Rate5m, rate_max => RateMax diff --git a/apps/emqx_auth/src/emqx_authz/emqx_authz_schema.erl b/apps/emqx_auth/src/emqx_authz/emqx_authz_schema.erl index f7bd59b82..24deb0161 100644 --- a/apps/emqx_auth/src/emqx_authz/emqx_authz_schema.erl +++ b/apps/emqx_auth/src/emqx_authz/emqx_authz_schema.erl @@ -88,6 +88,7 @@ fields("metrics_status_fields") -> fields("metrics") -> [ {"total", ?HOCON(integer(), #{desc => ?DESC("metrics_total")})}, + {"ignore", ?HOCON(integer(), #{desc => ?DESC("ignore")})}, {"allow", ?HOCON(integer(), #{desc => ?DESC("allow")})}, {"deny", ?HOCON(integer(), #{desc => ?DESC("deny")})}, {"nomatch", ?HOCON(float(), #{desc => ?DESC("nomatch")})} diff --git a/apps/emqx_auth_http/test/emqx_authz_http_SUITE.erl b/apps/emqx_auth_http/test/emqx_authz_http_SUITE.erl index 2efbbb031..2628a4309 100644 --- a/apps/emqx_auth_http/test/emqx_authz_http_SUITE.erl +++ b/apps/emqx_auth_http/test/emqx_authz_http_SUITE.erl @@ -48,7 +48,7 @@ init_per_suite(Config) -> emqx_auth, emqx_auth_http ], - #{work_dir => ?config(priv_dir, Config)} + #{work_dir => emqx_cth_suite:work_dir(Config)} ), [{suite_apps, Apps} | Config]. @@ -56,12 +56,22 @@ end_per_suite(_Config) -> ok = emqx_authz_test_lib:restore_authorizers(), emqx_cth_suite:stop(?config(suite_apps, _Config)). -init_per_testcase(_Case, Config) -> +init_per_testcase(t_bad_response = TestCase, Config) -> + TCApps = emqx_cth_suite:start_apps( + [emqx_management, emqx_mgmt_api_test_util:emqx_dashboard()], + #{work_dir => emqx_cth_suite:work_dir(TestCase, Config)} + ), + init_per_testcase(common, [{tc_apps, TCApps} | Config]); +init_per_testcase(_TestCase, Config) -> ok = emqx_authz_test_lib:reset_authorizers(), {ok, _} = emqx_authz_http_test_server:start_link(?HTTP_PORT, ?HTTP_PATH), Config. -end_per_testcase(_Case, _Config) -> +end_per_testcase(t_bad_response, Config) -> + TCApps = ?config(tc_apps, Config), + emqx_cth_suite:stop_apps(TCApps), + end_per_testcase(common, Config); +end_per_testcase(_TestCase, _Config) -> _ = emqx_authz:set_feature_available(rich_actions, true), try ok = emqx_authz_http_test_server:stop() @@ -589,6 +599,29 @@ t_bad_response(_Config) -> }, get_metrics() ), + ?assertMatch( + {200, #{ + <<"metrics">> := #{ + <<"ignore">> := 1, + <<"nomatch">> := 0, + <<"allow">> := 0, + <<"deny">> := 0, + <<"total">> := 1 + }, + <<"node_metrics">> := [ + #{ + <<"metrics">> := #{ + <<"ignore">> := 1, + <<"nomatch">> := 0, + <<"allow">> := 0, + <<"deny">> := 0, + <<"total">> := 1 + } + } + ] + }}, + get_status_api() + ), ok. t_no_value_for_placeholder(_Config) -> @@ -806,3 +839,11 @@ get_metrics() -> 'authorization.nomatch' ] ). + +get_status_api() -> + Path = emqx_mgmt_api_test_util:uri(["authorization", "sources", "http", "status"]), + Auth = emqx_mgmt_api_test_util:auth_header_(), + Opts = #{return_all => true}, + Res0 = emqx_mgmt_api_test_util:request_api(get, Path, _QParams = [], Auth, _Body = [], Opts), + {Status, RawBody} = emqx_mgmt_api_test_util:simplify_result(Res0), + {Status, emqx_utils_json:decode(RawBody, [return_maps])}. diff --git a/apps/emqx_management/test/emqx_mgmt_api_test_util.erl b/apps/emqx_management/test/emqx_mgmt_api_test_util.erl index 49f356316..a14bf9235 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_test_util.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_test_util.erl @@ -154,6 +154,14 @@ do_request_api(Method, Request, Opts) -> {error, Reason} end. +simplify_result(Res) -> + case Res of + {error, {{_, Status, _}, _, Body}} -> + {Status, Body}; + {ok, {{_, Status, _}, _, Body}} -> + {Status, Body} + end. + auth_header_() -> emqx_common_test_http:default_auth_header(). diff --git a/rel/i18n/emqx_authz_schema.hocon b/rel/i18n/emqx_authz_schema.hocon index 5e11f26a6..7a454f046 100644 --- a/rel/i18n/emqx_authz_schema.hocon +++ b/rel/i18n/emqx_authz_schema.hocon @@ -78,6 +78,11 @@ failed.desc: failed.label: """Failed""" +ignore.desc: +"""Count of query ignored. This counter is increased whenever the authorization source attempts to authorize a request, but either it's not applicable, or an error was encountered and the result is undecidable""" +ignore.label: +"""Ignored""" + metrics.desc: """The metrics of the resource.""" From 91947569631e751ea2276fe54076d01d3f635c27 Mon Sep 17 00:00:00 2001 From: zmstone Date: Thu, 6 Jun 2024 09:34:41 +0200 Subject: [PATCH 04/41] feat(auth): support HTTP authn return ACL rules --- .../src/emqx_authz/emqx_authz_rule.erl | 3 +- .../src/emqx_authz/emqx_authz_rule_raw.erl | 23 ++- .../emqx_auth_http/src/emqx_auth_http.app.src | 2 +- apps/emqx_auth_http/src/emqx_authn_http.erl | 99 +++++++++-- .../test/emqx_authn_http_SUITE.erl | 163 ++++++++++++++++++ apps/emqx_auth_jwt/src/emqx_auth_jwt.app.src | 2 +- apps/emqx_auth_jwt/src/emqx_authn_jwt.erl | 15 +- 7 files changed, 276 insertions(+), 31 deletions(-) diff --git a/apps/emqx_auth/src/emqx_authz/emqx_authz_rule.erl b/apps/emqx_auth/src/emqx_authz/emqx_authz_rule.erl index b3bddada4..8d7e12fcd 100644 --- a/apps/emqx_auth/src/emqx_authz/emqx_authz_rule.erl +++ b/apps/emqx_auth/src/emqx_authz/emqx_authz_rule.erl @@ -73,7 +73,8 @@ permission/0, who_condition/0, action_condition/0, - topic_condition/0 + topic_condition/0, + rule/0 ]). -type action_precompile() :: diff --git a/apps/emqx_auth/src/emqx_authz/emqx_authz_rule_raw.erl b/apps/emqx_auth/src/emqx_authz/emqx_authz_rule_raw.erl index 0f66b3ade..05b52c99b 100644 --- a/apps/emqx_auth/src/emqx_authz/emqx_authz_rule_raw.erl +++ b/apps/emqx_auth/src/emqx_authz/emqx_authz_rule_raw.erl @@ -21,7 +21,7 @@ -module(emqx_authz_rule_raw). --export([parse_rule/1, format_rule/1]). +-export([parse_rule/1, parse_and_compile_rules/1, format_rule/1]). -include("emqx_authz.hrl"). @@ -31,6 +31,27 @@ %% API %%-------------------------------------------------------------------- +%% @doc Parse and compile raw ACL rules. +%% If any bad rule is found, `{bad_acl_rule, ..}' is thrown. +-spec parse_and_compile_rules([rule_raw()]) -> [emqx_authz_rule:rule()]. +parse_and_compile_rules(Rules) -> + lists:map( + fun(Rule) -> + case parse_rule(Rule) of + {ok, {Permission, Action, Topics}} -> + try + emqx_authz_rule:compile({Permission, all, Action, Topics}) + catch + throw:Reason -> + throw({bad_acl_rule, Reason}) + end; + {error, Reason} -> + throw({bad_acl_rule, Reason}) + end + end, + Rules + ). + -spec parse_rule(rule_raw()) -> {ok, { emqx_authz_rule:permission(), diff --git a/apps/emqx_auth_http/src/emqx_auth_http.app.src b/apps/emqx_auth_http/src/emqx_auth_http.app.src index 4f625140b..9cf62ae15 100644 --- a/apps/emqx_auth_http/src/emqx_auth_http.app.src +++ b/apps/emqx_auth_http/src/emqx_auth_http.app.src @@ -1,7 +1,7 @@ %% -*- mode: erlang -*- {application, emqx_auth_http, [ {description, "EMQX External HTTP API Authentication and Authorization"}, - {vsn, "0.2.2"}, + {vsn, "0.3.0"}, {registered, []}, {mod, {emqx_auth_http_app, []}}, {applications, [ diff --git a/apps/emqx_auth_http/src/emqx_authn_http.erl b/apps/emqx_auth_http/src/emqx_authn_http.erl index 18995cb9d..d9c5c5ed5 100644 --- a/apps/emqx_auth_http/src/emqx_authn_http.erl +++ b/apps/emqx_auth_http/src/emqx_authn_http.erl @@ -201,19 +201,7 @@ handle_response(Headers, Body) -> ContentType = proplists:get_value(<<"content-type">>, Headers), case safely_parse_body(ContentType, Body) of {ok, NBody} -> - case maps:get(<<"result">>, NBody, <<"ignore">>) of - <<"allow">> -> - IsSuperuser = emqx_authn_utils:is_superuser(NBody), - Attrs = emqx_authn_utils:client_attrs(NBody), - Result = maps:merge(IsSuperuser, Attrs), - {ok, Result}; - <<"deny">> -> - {error, not_authorized}; - <<"ignore">> -> - ignore; - _ -> - ignore - end; + body_to_auth_data(NBody); {error, Reason} -> ?TRACE_AUTHN_PROVIDER( error, @@ -223,6 +211,91 @@ handle_response(Headers, Body) -> ignore end. +body_to_auth_data(Body) -> + case maps:get(<<"result">>, Body, <<"ignore">>) of + <<"allow">> -> + IsSuperuser = emqx_authn_utils:is_superuser(Body), + Attrs = emqx_authn_utils:client_attrs(Body), + try + ExpireAt = expire_at(Body), + ACL = acl(ExpireAt, Body), + Result = merge_maps([ExpireAt, IsSuperuser, ACL, Attrs]), + {ok, Result} + catch + throw:{bad_acl_rule, Reason} -> + %% it's a invalid token, so ok to log + ?TRACE_AUTHN_PROVIDER("bad_acl_rule", Reason#{http_body => Body}), + {error, bad_username_or_password}; + throw:Reason -> + ?TRACE_AUTHN_PROVIDER("bad_response_body", Reason#{http_body => Body}), + {error, bad_username_or_password} + end; + <<"deny">> -> + {error, not_authorized}; + <<"ignore">> -> + ignore; + _ -> + ignore + end. + +merge_maps([]) -> #{}; +merge_maps([Map | Maps]) -> maps:merge(Map, merge_maps(Maps)). + +%% Return either an empty map, or a map with `expire_at` at millisecond precision +%% Millisecond precision timestamp is required by `auth_expire_at` +%% emqx_channel:schedule_connection_expire/1 +expire_at(Body) -> + case expire_sec(Body) of + undefined -> + #{}; + Sec -> + #{expire_at => erlang:convert_time_unit(Sec, second, millisecond)} + end. + +expire_sec(#{<<"expire_at">> := ExpireTime}) when is_integer(ExpireTime) -> + Now = erlang:system_time(second), + NowMs = erlang:convert_time_unit(Now, second, millisecond), + case ExpireTime < Now of + true -> + throw(#{ + cause => "'expire_at' is in the past.", + system_time => Now, + expire_at => ExpireTime + }); + false when ExpireTime > (NowMs div 2) -> + throw(#{ + cause => "'expire_at' does not appear to be a Unix epoch time in seconds.", + system_time => Now, + expire_at => ExpireTime + }); + false -> + ExpireTime + end; +expire_sec(#{<<"expire_at">> := _}) -> + throw(#{cause => "'expire_at' is not an integer (Unix epoch time in seconds)."}); +expire_sec(_) -> + undefined. + +acl(#{expire_at := ExpireTimeMs}, #{<<"acl">> := Rules}) -> + #{ + acl => #{ + source_for_logging => http, + rules => emqx_authz_rule_raw:parse_and_compile_rules(Rules), + %% It's seconds level precision (like JWT) for authz + %% see emqx_authz_client_info:check/1 + expire => erlang:convert_time_unit(ExpireTimeMs, millisecond, second) + } + }; +acl(_NoExpire, #{<<"acl">> := Rules}) -> + #{ + acl => #{ + source_for_logging => http, + rules => emqx_authz_rule_raw:parse_and_compile_rules(Rules) + } + }; +acl(_, _) -> + #{}. + safely_parse_body(ContentType, Body) -> try parse_body(ContentType, Body) diff --git a/apps/emqx_auth_http/test/emqx_authn_http_SUITE.erl b/apps/emqx_auth_http/test/emqx_authn_http_SUITE.erl index dc1443b19..861e8492a 100644 --- a/apps/emqx_auth_http/test/emqx_authn_http_SUITE.erl +++ b/apps/emqx_auth_http/test/emqx_authn_http_SUITE.erl @@ -23,6 +23,7 @@ -include_lib("eunit/include/eunit.hrl"). -include_lib("common_test/include/ct.hrl"). -include_lib("emqx/include/emqx_placeholder.hrl"). +-include_lib("emqx/include/emqx_mqtt.hrl"). -define(PATH, [?CONF_NS_ATOM]). @@ -49,6 +50,21 @@ }) ). +-define(SERVER_RESPONSE_WITH_ACL_JSON(ACL), + emqx_utils_json:encode(#{ + result => allow, + acl => ACL + }) +). + +-define(SERVER_RESPONSE_WITH_ACL_JSON(ACL, Expire), + emqx_utils_json:encode(#{ + result => allow, + acl => ACL, + expire_at => Expire + }) +). + -define(SERVER_RESPONSE_URLENCODE(Result, IsSuperuser), list_to_binary( "result=" ++ @@ -506,6 +522,129 @@ test_ignore_allow_deny({ExpectedValue, ServerResponse}) -> ) end. +t_acl(_Config) -> + ACL = acl_rules(), + Config = raw_http_auth_config(), + {ok, _} = emqx:update_config( + ?PATH, + {create_authenticator, ?GLOBAL, Config} + ), + ok = emqx_authn_http_test_server:set_handler( + fun(Req0, State) -> + Req = cowboy_req:reply( + 200, + #{<<"content-type">> => <<"application/json">>}, + ?SERVER_RESPONSE_WITH_ACL_JSON(ACL), + Req0 + ), + {ok, Req, State} + end + ), + {ok, C} = emqtt:start_link( + [ + {clean_start, true}, + {proto_ver, v5}, + {clientid, <<"clientid">>}, + {username, <<"username">>}, + {password, <<"password">>} + ] + ), + {ok, _} = emqtt:connect(C), + Cases = [ + {allow, <<"http-authn-acl/#">>}, + {deny, <<"http-authn-acl/1">>}, + {deny, <<"t/#">>} + ], + try + lists:foreach( + fun(Case) -> + test_acl(Case, C) + end, + Cases + ) + after + ok = emqtt:disconnect(C) + end. + +t_auth_expire(_Config) -> + ACL = acl_rules(), + Config = raw_http_auth_config(), + {ok, _} = emqx:update_config( + ?PATH, + {create_authenticator, ?GLOBAL, Config} + ), + ExpireSec = 3, + WaitTime = timer:seconds(ExpireSec + 1), + Tests = [ + {<<"ok-to-connect-but-expire-on-pub">>, erlang:system_time(second) + ExpireSec, fun(C) -> + {ok, _} = emqtt:connect(C), + receive + {'DOWN', _Ref, process, C, Reason} -> + ?assertMatch({disconnected, ?RC_NOT_AUTHORIZED, _}, Reason) + after WaitTime -> + error(timeout) + end + end}, + {<<"past">>, erlang:system_time(second) - 1, fun(C) -> + ?assertMatch({error, {bad_username_or_password, _}}, emqtt:connect(C)), + receive + {'DOWN', _Ref, process, C, Reason} -> + ?assertMatch({shutdown, bad_username_or_password}, Reason) + end + end}, + {<<"invalid">>, erlang:system_time(millisecond), fun(C) -> + ?assertMatch({error, {bad_username_or_password, _}}, emqtt:connect(C)), + receive + {'DOWN', _Ref, process, C, Reason} -> + ?assertMatch({shutdown, bad_username_or_password}, Reason) + end + end} + ], + ok = emqx_authn_http_test_server:set_handler( + fun(Req0, State) -> + QS = cowboy_req:parse_qs(Req0), + {_, Username} = lists:keyfind(<<"username">>, 1, QS), + {_, ExpireTime, _} = lists:keyfind(Username, 1, Tests), + Req = cowboy_req:reply( + 200, + #{<<"content-type">> => <<"application/json">>}, + ?SERVER_RESPONSE_WITH_ACL_JSON(ACL, ExpireTime), + Req0 + ), + {ok, Req, State} + end + ), + lists:foreach(fun test_auth_expire/1, Tests). + +test_auth_expire({Username, _ExpireTime, TestFn}) -> + {ok, C} = emqtt:start_link( + [ + {clean_start, true}, + {proto_ver, v5}, + {clientid, <<"clientid">>}, + {username, Username}, + {password, <<"password">>} + ] + ), + _ = monitor(process, C), + unlink(C), + try + TestFn(C) + after + [ok = emqtt:disconnect(C) || is_process_alive(C)] + end. + +test_acl({allow, Topic}, C) -> + ?assertMatch( + {ok, #{}, [0]}, + emqtt:subscribe(C, Topic) + ); +test_acl({deny, Topic}, C) -> + ?assertMatch( + {ok, #{}, [?RC_NOT_AUTHORIZED]}, + emqtt:subscribe(C, Topic) + ). + %%------------------------------------------------------------------------------ %% Helpers %%------------------------------------------------------------------------------ @@ -765,3 +904,27 @@ to_list(B) when is_binary(B) -> binary_to_list(B); to_list(L) when is_list(L) -> L. + +acl_rules() -> + [ + #{ + <<"permission">> => <<"allow">>, + <<"action">> => <<"pub">>, + <<"topics">> => [ + <<"http-authn-acl/1">> + ] + }, + #{ + <<"permission">> => <<"allow">>, + <<"action">> => <<"sub">>, + <<"topics">> => + [ + <<"eq http-authn-acl/#">> + ] + }, + #{ + <<"permission">> => <<"deny">>, + <<"action">> => <<"all">>, + <<"topics">> => [<<"#">>] + } + ]. diff --git a/apps/emqx_auth_jwt/src/emqx_auth_jwt.app.src b/apps/emqx_auth_jwt/src/emqx_auth_jwt.app.src index 84659fce8..1edb5fc67 100644 --- a/apps/emqx_auth_jwt/src/emqx_auth_jwt.app.src +++ b/apps/emqx_auth_jwt/src/emqx_auth_jwt.app.src @@ -1,7 +1,7 @@ %% -*- mode: erlang -*- {application, emqx_auth_jwt, [ {description, "EMQX JWT Authentication and Authorization"}, - {vsn, "0.3.1"}, + {vsn, "0.3.2"}, {registered, []}, {mod, {emqx_auth_jwt_app, []}}, {applications, [ diff --git a/apps/emqx_auth_jwt/src/emqx_authn_jwt.erl b/apps/emqx_auth_jwt/src/emqx_authn_jwt.erl index ceaa2dfc2..33e7ee645 100644 --- a/apps/emqx_auth_jwt/src/emqx_authn_jwt.erl +++ b/apps/emqx_auth_jwt/src/emqx_authn_jwt.erl @@ -402,20 +402,7 @@ binary_to_number(Bin) -> parse_rules(Rules) when is_map(Rules) -> Rules; parse_rules(Rules) when is_list(Rules) -> - lists:map(fun parse_rule/1, Rules). - -parse_rule(Rule) -> - case emqx_authz_rule_raw:parse_rule(Rule) of - {ok, {Permission, Action, Topics}} -> - try - emqx_authz_rule:compile({Permission, all, Action, Topics}) - catch - throw:Reason -> - throw({bad_acl_rule, Reason}) - end; - {error, Reason} -> - throw({bad_acl_rule, Reason}) - end. + emqx_authz_rule_raw:parse_and_compile_rules(Rules). merge_maps([]) -> #{}; merge_maps([Map | Maps]) -> maps:merge(Map, merge_maps(Maps)). From 72579f90142f09a463c57372f2d11ca5892403b7 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Wed, 3 Jul 2024 15:05:00 -0300 Subject: [PATCH 05/41] test(postgres bridge): attempt to stabilize flaky test ``` %%% emqx_bridge_pgsql_SUITE ==> tcp.sync.with_batch.t_table_removed: FAILED %%% emqx_bridge_pgsql_SUITE ==> {{panic, #{msg => "Unexpected result", result => {run_stage_failed,exit, {test_case_failed, "unexpected result: {error,{recoverable_error,sync_required}}"}, [{emqx_bridge_pgsql_SUITE,'-t_table_removed/1-fun-3-',3, [{file, "/emqx/apps/emqx_bridge_pgsql/test/emqx_bridge_pgsql_SUITE.erl"}, {line,822}]}, ``` ``` Error: -03T17:52:54.046809+00:00 [error] Generic server <0.352770.0> terminating. Reason: {'module could not be loaded',[{undefined,handle_message,[90,<<"I">>,{state,ssl,{sslsocket,{gen_tcp,#Port<0.1671>,tls_connection,undefined},[<0.352774.0>,<0.352773.0>]},<<>>,{336,-2093820527},on_message,{codec,#{},[null,undefined],{oid_db,#{16 => ... 2024-07-03T17:52:54.075446+00:00 [critical] Run stage failed: exit:{test_case_failed,"unexpected result: {error,\n {resource_error,\n #{reason => exception,\n msg =>\n #{error =>\n {exit,\n {{undef,\n [{undefined,handle_message,\n [90,<<\"I\">>,\n {state,ssl,\n {sslsocket,\n {gen_tcp,#Port<0.1671>,tls_connection,\n ``` --- apps/emqx_bridge_pgsql/test/emqx_bridge_pgsql_SUITE.erl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/emqx_bridge_pgsql/test/emqx_bridge_pgsql_SUITE.erl b/apps/emqx_bridge_pgsql/test/emqx_bridge_pgsql_SUITE.erl index f671c90df..a88cddf76 100644 --- a/apps/emqx_bridge_pgsql/test/emqx_bridge_pgsql_SUITE.erl +++ b/apps/emqx_bridge_pgsql/test/emqx_bridge_pgsql_SUITE.erl @@ -728,8 +728,8 @@ t_prepared_statement_exists(Config) -> emqx_common_test_helpers:on_exit(fun() -> meck:unload() end), - MeckOpts = [passthrough, no_link, no_history, non_strict], - meck:new(emqx_postgresql, MeckOpts), + MeckOpts = [passthrough, no_link, no_history], + meck:new(epgsql, MeckOpts), InsertPrepStatementDupAndThenRemoveMeck = fun(Conn, Key, SQL, List) -> meck:passthrough([Conn, Key, SQL, List]), @@ -795,6 +795,7 @@ t_prepared_statement_exists(Config) -> ok end ), + meck:unload(), ok. t_table_removed(Config) -> From eaaee725c21aa4d711b2cfdb36217255ea3e45a7 Mon Sep 17 00:00:00 2001 From: zmstone Date: Wed, 3 Jul 2024 16:48:26 +0200 Subject: [PATCH 06/41] fix: upgrade to hocon 0.43.1 included 3 changes since 0.42.2 - allow validation of map keys - improve crash stacktrace report - avoid dumping array environment variable values --- apps/emqx/rebar.config | 2 +- mix.exs | 2 +- rebar.config | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/emqx/rebar.config b/apps/emqx/rebar.config index f3089d11f..0816b013e 100644 --- a/apps/emqx/rebar.config +++ b/apps/emqx/rebar.config @@ -30,7 +30,7 @@ {esockd, {git, "https://github.com/emqx/esockd", {tag, "5.11.2"}}}, {ekka, {git, "https://github.com/emqx/ekka", {tag, "0.19.5"}}}, {gen_rpc, {git, "https://github.com/emqx/gen_rpc", {tag, "3.3.1"}}}, - {hocon, {git, "https://github.com/emqx/hocon.git", {tag, "0.42.2"}}}, + {hocon, {git, "https://github.com/emqx/hocon.git", {tag, "0.43.1"}}}, {emqx_http_lib, {git, "https://github.com/emqx/emqx_http_lib.git", {tag, "0.5.3"}}}, {pbkdf2, {git, "https://github.com/emqx/erlang-pbkdf2.git", {tag, "2.0.4"}}}, {recon, {git, "https://github.com/ferd/recon", {tag, "2.5.1"}}}, diff --git a/mix.exs b/mix.exs index 41ab01b37..313059bde 100644 --- a/mix.exs +++ b/mix.exs @@ -177,7 +177,7 @@ defmodule EMQXUmbrella.MixProject do def common_dep(:ekka), do: {:ekka, github: "emqx/ekka", tag: "0.19.5", override: true} def common_dep(:esockd), do: {:esockd, github: "emqx/esockd", tag: "5.11.2", override: true} def common_dep(:gproc), do: {:gproc, github: "emqx/gproc", tag: "0.9.0.1", override: true} - def common_dep(:hocon), do: {:hocon, github: "emqx/hocon", tag: "0.42.2", override: true} + def common_dep(:hocon), do: {:hocon, github: "emqx/hocon", tag: "0.43.1", override: true} def common_dep(:lc), do: {:lc, github: "emqx/lc", tag: "0.3.2", override: true} # in conflict by ehttpc and emqtt def common_dep(:gun), do: {:gun, github: "emqx/gun", tag: "1.3.11", override: true} diff --git a/rebar.config b/rebar.config index 3b097f6fa..8f689ea3d 100644 --- a/rebar.config +++ b/rebar.config @@ -97,7 +97,7 @@ {system_monitor, {git, "https://github.com/ieQu1/system_monitor", {tag, "3.0.5"}}}, {getopt, "1.0.2"}, {snabbkaffe, {git, "https://github.com/kafka4beam/snabbkaffe.git", {tag, "1.0.10"}}}, - {hocon, {git, "https://github.com/emqx/hocon.git", {tag, "0.42.2"}}}, + {hocon, {git, "https://github.com/emqx/hocon.git", {tag, "0.43.1"}}}, {emqx_http_lib, {git, "https://github.com/emqx/emqx_http_lib.git", {tag, "0.5.3"}}}, {esasl, {git, "https://github.com/emqx/esasl", {tag, "0.2.1"}}}, {jose, {git, "https://github.com/potatosalad/erlang-jose", {tag, "1.11.2"}}}, From 5446bc305f56fe676490a0e1d0c1736318873a4c Mon Sep 17 00:00:00 2001 From: zmstone Date: Wed, 3 Jul 2024 17:08:41 +0200 Subject: [PATCH 07/41] docs: add changelog for PR 13403 --- changes/ce/fix-13403.en.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/ce/fix-13403.en.md diff --git a/changes/ce/fix-13403.en.md b/changes/ce/fix-13403.en.md new file mode 100644 index 000000000..1a02fffe7 --- /dev/null +++ b/changes/ce/fix-13403.en.md @@ -0,0 +1 @@ +Fixed environment variable config override logging behaviour to avoid logging passwords. From 947cddb2eb73f0bbd57f1a5513a3008ab8da3d2d Mon Sep 17 00:00:00 2001 From: zmstone Date: Wed, 3 Jul 2024 22:56:44 +0200 Subject: [PATCH 08/41] test: invalid map key is caught by hocon now that hocon has a built-in map key validation, some of the resource name validations are cought by hocon --- apps/emqx_bridge/test/emqx_bridge_SUITE.erl | 2 +- apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl | 4 ++-- .../test/emqx_bridge_v1_compatibility_layer_SUITE.erl | 7 +++---- apps/emqx_connector/test/emqx_connector_SUITE.erl | 2 +- apps/emqx_connector/test/emqx_connector_api_SUITE.erl | 2 +- 5 files changed, 8 insertions(+), 9 deletions(-) diff --git a/apps/emqx_bridge/test/emqx_bridge_SUITE.erl b/apps/emqx_bridge/test/emqx_bridge_SUITE.erl index 7273df000..e510dda7f 100644 --- a/apps/emqx_bridge/test/emqx_bridge_SUITE.erl +++ b/apps/emqx_bridge/test/emqx_bridge_SUITE.erl @@ -216,7 +216,7 @@ t_create_with_bad_name(_Config) -> ok. t_create_with_bad_name_root(_Config) -> - BadBridgeName = <<"test_哈哈">>, + BadBridgeName = <<"test_哈哈"/utf8>>, BridgeConf = #{ <<"bridge_mode">> => false, <<"clean_start">> => true, diff --git a/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl b/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl index 08b3270ea..6b160f3b3 100644 --- a/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl +++ b/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl @@ -1431,7 +1431,7 @@ t_cluster_later_join_metrics(Config) -> t_create_with_bad_name(Config) -> Port = ?config(port, Config), URL1 = ?URL(Port, "path1"), - Name = <<"test_哈哈">>, + Name = <<"test_哈哈"/utf8>>, BadBridgeParams = emqx_utils_maps:deep_merge( ?HTTP_BRIDGE(URL1, Name), @@ -1457,7 +1457,7 @@ t_create_with_bad_name(Config) -> ?assertMatch( #{ <<"kind">> := <<"validation_error">>, - <<"reason">> := <<"Invalid name format.", _/binary>> + <<"reason">> := <<"invalid_map_key">> }, Msg ), diff --git a/apps/emqx_bridge/test/emqx_bridge_v1_compatibility_layer_SUITE.erl b/apps/emqx_bridge/test/emqx_bridge_v1_compatibility_layer_SUITE.erl index c0b8acc0d..0fc13ffb0 100644 --- a/apps/emqx_bridge/test/emqx_bridge_v1_compatibility_layer_SUITE.erl +++ b/apps/emqx_bridge/test/emqx_bridge_v1_compatibility_layer_SUITE.erl @@ -604,8 +604,7 @@ deprecated_config() -> t_name_too_long(_Config) -> LongName = list_to_binary(lists:duplicate(256, $a)), ?assertMatch( - {error, - {{_, 400, _}, _, #{<<"message">> := #{<<"reason">> := <<"Name is too long", _/binary>>}}}}, + {error, {{_, 400, _}, _, #{<<"message">> := #{<<"reason">> := <<"invalid_map_key">>}}}}, create_bridge_http_api_v1(#{name => LongName}) ), ok. @@ -942,7 +941,7 @@ t_scenario_2(Config) -> ok. t_create_with_bad_name(_Config) -> - BadBridgeName = <<"test_哈哈">>, + BadBridgeName = <<"test_哈哈"/utf8>>, %% Note: must contain SSL options to trigger bug. Cacertfile = emqx_common_test_helpers:app_path( emqx, @@ -960,7 +959,7 @@ t_create_with_bad_name(_Config) -> <<"code">> := <<"BAD_REQUEST">>, <<"message">> := #{ <<"kind">> := <<"validation_error">>, - <<"reason">> := <<"Invalid name format.", _/binary>> + <<"reason">> := <<"invalid_map_key">> } }}} = create_bridge_http_api_v1(Opts), ok. diff --git a/apps/emqx_connector/test/emqx_connector_SUITE.erl b/apps/emqx_connector/test/emqx_connector_SUITE.erl index 1b210e7fb..fbdece6ff 100644 --- a/apps/emqx_connector/test/emqx_connector_SUITE.erl +++ b/apps/emqx_connector/test/emqx_connector_SUITE.erl @@ -275,7 +275,7 @@ t_create_with_bad_name_root_path({'end', _Config}) -> ok; t_create_with_bad_name_root_path(_Config) -> Path = [connectors], - BadConnectorName = <<"test_哈哈">>, + BadConnectorName = <<"test_哈哈"/utf8>>, ConnConfig0 = connector_config(), %% Note: must contain SSL options to trigger original bug. Cacertfile = emqx_common_test_helpers:app_path( diff --git a/apps/emqx_connector/test/emqx_connector_api_SUITE.erl b/apps/emqx_connector/test/emqx_connector_api_SUITE.erl index 7c7bc432c..f3e91ef12 100644 --- a/apps/emqx_connector/test/emqx_connector_api_SUITE.erl +++ b/apps/emqx_connector/test/emqx_connector_api_SUITE.erl @@ -697,7 +697,7 @@ t_connectors_probe(Config) -> ok. t_create_with_bad_name(Config) -> - ConnectorName = <<"test_哈哈">>, + ConnectorName = <<"test_哈哈"/utf8>>, Conf0 = ?KAFKA_CONNECTOR(ConnectorName), %% Note: must contain SSL options to trigger original bug. Cacertfile = emqx_common_test_helpers:app_path( From 913e0ce18b8ca6a2a75057c85fd0afaef7e9d25f Mon Sep 17 00:00:00 2001 From: firest Date: Mon, 1 Jul 2024 18:36:33 +0800 Subject: [PATCH 09/41] feat(banned): add a bootstrap file for banned --- apps/emqx/src/emqx_banned.erl | 150 +++++++++++++++++- apps/emqx/src/emqx_schema.erl | 18 +++ apps/emqx/test/data/banned/error.csv | 4 + apps/emqx/test/data/banned/full.csv | 3 + apps/emqx/test/data/banned/full2.csv | 3 + apps/emqx/test/data/banned/omitted.csv | 3 + apps/emqx/test/data/banned/optional.csv | 3 + apps/emqx/test/emqx_banned_SUITE.erl | 53 +++++++ .../src/emqx_mgmt_api_banned.erl | 2 +- apps/emqx_utils/src/emqx_utils_stream.erl | 59 ++++++- rel/i18n/emqx_schema.hocon | 18 +++ 11 files changed, 305 insertions(+), 11 deletions(-) create mode 100644 apps/emqx/test/data/banned/error.csv create mode 100644 apps/emqx/test/data/banned/full.csv create mode 100644 apps/emqx/test/data/banned/full2.csv create mode 100644 apps/emqx/test/data/banned/omitted.csv create mode 100644 apps/emqx/test/data/banned/optional.csv diff --git a/apps/emqx/src/emqx_banned.erl b/apps/emqx/src/emqx_banned.erl index f6e9a908d..5e3545b93 100644 --- a/apps/emqx/src/emqx_banned.erl +++ b/apps/emqx/src/emqx_banned.erl @@ -16,6 +16,8 @@ -module(emqx_banned). +-feature(maybe_expr, enable). + -behaviour(gen_server). -behaviour(emqx_db_backup). @@ -48,6 +50,7 @@ handle_call/3, handle_cast/2, handle_info/2, + handle_continue/2, terminate/2, code_change/3 ]). @@ -132,7 +135,7 @@ format(#banned{ until => to_rfc3339(Until) }. --spec parse(map()) -> emqx_types:banned() | {error, term()}. +-spec parse(map()) -> {ok, emqx_types:banned()} | {error, term()}. parse(Params) -> case parse_who(Params) of {error, Reason} -> @@ -144,13 +147,13 @@ parse(Params) -> Until = maps:get(<<"until">>, Params, At + ?EXPIRATION_TIME), case Until > erlang:system_time(second) of true -> - #banned{ + {ok, #banned{ who = Who, by = By, reason = Reason, at = At, until = Until - }; + }}; false -> ErrorReason = io_lib:format("Cannot create expired banned, ~p to ~p", [At, Until]), @@ -234,12 +237,137 @@ who(peerhost_net, CIDR) when is_tuple(CIDR) -> {peerhost_net, CIDR}; who(peerhost_net, CIDR) when is_binary(CIDR) -> {peerhost_net, esockd_cidr:parse(binary_to_list(CIDR), true)}. +%%-------------------------------------------------------------------- +%% Import From CSV +%%-------------------------------------------------------------------- +init_from_csv(<<>>) -> + ok; +init_from_csv(File) -> + maybe + core ?= mria_rlog:role(), + '$end_of_table' ?= mnesia:dirty_first(?BANNED_RULE_TAB), + '$end_of_table' ?= mnesia:dirty_first(?BANNED_INDIVIDUAL_TAB), + {ok, Bin} ?= file:read_file(File), + Stream = emqx_utils_stream:csv(Bin, #{nullable => true, filter_null => true}), + {ok, List} ?= parse_stream(Stream), + import_from_stream(List), + ?SLOG(info, #{ + msg => "load_banned_bootstrap_file_succeeded", + file => File + }) + else + replicant -> + ok; + {Name, _} when + Name == peerhost; + Name == peerhost_net; + Name == clientid_re; + Name == username_re; + Name == clientid; + Name == username + -> + ok; + {error, Reason} = Error -> + ?SLOG(error, #{ + msg => "load_banned_bootstrap_file_failed", + reason => Reason, + file => File + }), + Error + end. + +import_from_stream(Stream) -> + Groups = maps:groups_from_list( + fun(#banned{who = Who}) -> table(Who) end, Stream + ), + maps:foreach( + fun(Tab, Items) -> + Trans = fun() -> + lists:foreach( + fun(Item) -> + mnesia:write(Tab, Item, write) + end, + Items + ) + end, + + case trans(Trans) of + {ok, _} -> + ?SLOG(info, #{ + msg => "import_banned_from_stream_succeeded", + items => Items + }); + {error, Reason} -> + ?SLOG(error, #{ + msg => "import_banned_from_stream_failed", + reason => Reason, + items => Items + }) + end + end, + Groups + ). + +parse_stream(Stream) -> + try + List = emqx_utils_stream:consume(Stream), + parse_stream(List, [], []) + catch + error:Reason -> + {error, Reason} + end. + +parse_stream([Item | List], Ok, Error) -> + maybe + {ok, Item1} ?= normalize_parse_item(Item), + {ok, Banned} ?= parse(Item1), + parse_stream(List, [Banned | Ok], Error) + else + {error, _} -> + parse_stream(List, Ok, [Item | Error]) + end; +parse_stream([], Ok, []) -> + {ok, Ok}; +parse_stream([], Ok, Error) -> + ?SLOG(warning, #{ + msg => "invalid_banned_items", + items => Error + }), + {ok, Ok}. + +normalize_parse_item(#{<<"as">> := As} = Item) -> + ParseTime = fun(Name, Input) -> + maybe + #{Name := Time} ?= Input, + {ok, Epoch} ?= emqx_utils_calendar:to_epoch_second(emqx_utils_conv:str(Time)), + {ok, Input#{Name := Epoch}} + else + {error, _} = Error -> + Error; + NoTime when is_map(NoTime) -> + {ok, NoTime} + end + end, + + maybe + {ok, Type} ?= emqx_utils:safe_to_existing_atom(As), + {ok, Item1} ?= ParseTime(<<"at">>, Item#{<<"as">> := Type}), + ParseTime(<<"until">>, Item1) + end; +normalize_parse_item(_Item) -> + {error, invalid_item}. + %%-------------------------------------------------------------------- %% gen_server callbacks %%-------------------------------------------------------------------- init([]) -> - {ok, ensure_expiry_timer(#{expiry_timer => undefined})}. + {ok, ensure_expiry_timer(#{expiry_timer => undefined}), {continue, init_from_csv}}. + +handle_continue(init_from_csv, State) -> + File = emqx_schema:naive_env_interpolation(emqx:get_config([banned, bootstrap_file], <<>>)), + _ = init_from_csv(File), + {noreply, State}. handle_call(Req, _From, State) -> ?SLOG(error, #{msg => "unexpected_call", call => Req}), @@ -250,7 +378,7 @@ handle_cast(Msg, State) -> {noreply, State}. handle_info({timeout, TRef, expire}, State = #{expiry_timer := TRef}) -> - _ = mria:transaction(?COMMON_SHARD, fun ?MODULE:expire_banned_items/1, [ + _ = trans(fun ?MODULE:expire_banned_items/1, [ erlang:system_time(second) ]), {noreply, ensure_expiry_timer(State), hibernate}; @@ -391,3 +519,15 @@ on_banned(_) -> all_rules() -> ets:tab2list(?BANNED_RULE_TAB). + +trans(Fun) -> + case mria:transaction(?COMMON_SHARD, Fun) of + {atomic, Res} -> {ok, Res}; + {aborted, Reason} -> {error, Reason} + end. + +trans(Fun, Args) -> + case mria:transaction(?COMMON_SHARD, Fun, Args) of + {atomic, Res} -> {ok, Res}; + {aborted, Reason} -> {error, Reason} + end. diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index 6b02a4d4b..1ba71fb00 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/apps/emqx/src/emqx_schema.erl @@ -319,6 +319,11 @@ roots(low) -> sc( ref("crl_cache"), #{importance => ?IMPORTANCE_HIDDEN} + )}, + {banned, + sc( + ref("banned"), + #{importance => ?IMPORTANCE_HIDDEN} )} ]. @@ -1762,6 +1767,17 @@ fields("client_attrs_init") -> desc => ?DESC("client_attrs_init_set_as_attr"), validator => fun restricted_string/1 })} + ]; +fields("banned") -> + [ + {bootstrap_file, + sc( + binary(), + #{ + desc => ?DESC("banned_bootstrap_file"), + default => <<>> + } + )} ]. compile_variform(undefined, _Opts) -> @@ -2105,6 +2121,8 @@ desc(durable_storage) -> ?DESC(durable_storage); desc("client_attrs_init") -> ?DESC(client_attrs_init); +desc("banned") -> + "Banned ."; desc(_) -> undefined. diff --git a/apps/emqx/test/data/banned/error.csv b/apps/emqx/test/data/banned/error.csv new file mode 100644 index 000000000..49046a5a3 --- /dev/null +++ b/apps/emqx/test/data/banned/error.csv @@ -0,0 +1,4 @@ +as,who,reason,at,until,by +clientid,c1,right,2021-10-25T21:53:47+08:00,2025-10-25T21:53:47+08:00,boot +username,u1,reason 1,abc,2025-10-25T21:53:47+08:00,boot +usernamx,u2,reason 2,2021-10-25T21:53:47+08:00,2025-10-25T21:53:47+08:00,boot diff --git a/apps/emqx/test/data/banned/full.csv b/apps/emqx/test/data/banned/full.csv new file mode 100644 index 000000000..f6abf9de2 --- /dev/null +++ b/apps/emqx/test/data/banned/full.csv @@ -0,0 +1,3 @@ +as,who,reason,at,until,by +clientid,c1,reason 1,2021-10-25T21:53:47+08:00,2025-10-25T21:53:47+08:00,boot +username,u1,reason 2,2021-10-25T21:53:47+08:00,2025-10-25T21:53:47+08:00,boot diff --git a/apps/emqx/test/data/banned/full2.csv b/apps/emqx/test/data/banned/full2.csv new file mode 100644 index 000000000..e033d2e60 --- /dev/null +++ b/apps/emqx/test/data/banned/full2.csv @@ -0,0 +1,3 @@ +as,who,reason,at,until,by +clientid,c2,reason 1,2021-10-25T21:53:47+08:00,2025-10-25T21:53:47+08:00,boot +username,u2,reason 2,2021-10-25T21:53:47+08:00,2025-10-25T21:53:47+08:00,boot diff --git a/apps/emqx/test/data/banned/omitted.csv b/apps/emqx/test/data/banned/omitted.csv new file mode 100644 index 000000000..5db11d7cf --- /dev/null +++ b/apps/emqx/test/data/banned/omitted.csv @@ -0,0 +1,3 @@ +as,who,reason,at,until,by +clientid,c1,,2021-10-25T21:53:47+08:00,2025-10-25T21:53:47+08:00, +username,u1,,2021-10-25T21:53:47+08:00,2025-10-25T21:53:47+08:00, diff --git a/apps/emqx/test/data/banned/optional.csv b/apps/emqx/test/data/banned/optional.csv new file mode 100644 index 000000000..4e752e29a --- /dev/null +++ b/apps/emqx/test/data/banned/optional.csv @@ -0,0 +1,3 @@ +as,who +clientid,c1 +username,u1 diff --git a/apps/emqx/test/emqx_banned_SUITE.erl b/apps/emqx/test/emqx_banned_SUITE.erl index d73907c57..d2d031124 100644 --- a/apps/emqx/test/emqx_banned_SUITE.erl +++ b/apps/emqx/test/emqx_banned_SUITE.erl @@ -246,6 +246,45 @@ t_session_taken(_) -> {ok, #{}, [0]} = emqtt:unsubscribe(C3, Topic), ok = emqtt:disconnect(C3). +t_full_bootstrap_file(_) -> + emqx_banned:clear(), + ?assertEqual(ok, emqx_banned:init_from_csv(mk_bootstrap_file(<<"full.csv">>))), + FullDatas = lists:sort([ + {banned, {username, <<"u1">>}, <<"boot">>, <<"reason 2">>, 1635170027, 1761400427}, + {banned, {clientid, <<"c1">>}, <<"boot">>, <<"reason 1">>, 1635170027, 1761400427} + ]), + ?assertMatch(FullDatas, lists:sort(get_banned_list())), + + ?assertEqual(ok, emqx_banned:init_from_csv(mk_bootstrap_file(<<"full2.csv">>))), + ?assertMatch(FullDatas, lists:sort(get_banned_list())), + ok. + +t_optional_bootstrap_file(_) -> + emqx_banned:clear(), + ?assertEqual(ok, emqx_banned:init_from_csv(mk_bootstrap_file(<<"optional.csv">>))), + Keys = lists:sort([{username, <<"u1">>}, {clientid, <<"c1">>}]), + ?assertMatch(Keys, lists:sort([element(2, Data) || Data <- get_banned_list()])), + ok. + +t_omitted_bootstrap_file(_) -> + emqx_banned:clear(), + ?assertEqual(ok, emqx_banned:init_from_csv(mk_bootstrap_file(<<"omitted.csv">>))), + Keys = lists:sort([{username, <<"u1">>}, {clientid, <<"c1">>}]), + ?assertMatch(Keys, lists:sort([element(2, Data) || Data <- get_banned_list()])), + ok. + +t_error_bootstrap_file(_) -> + emqx_banned:clear(), + ?assertEqual( + {error, enoent}, emqx_banned:init_from_csv(mk_bootstrap_file(<<"not_exists.csv">>)) + ), + ?assertEqual( + ok, emqx_banned:init_from_csv(mk_bootstrap_file(<<"error.csv">>)) + ), + Keys = [{clientid, <<"c1">>}], + ?assertMatch(Keys, [element(2, Data) || Data <- get_banned_list()]), + ok. + receive_messages(Count) -> receive_messages(Count, []). receive_messages(0, Msgs) -> @@ -261,3 +300,17 @@ receive_messages(Count, Msgs) -> after 1200 -> Msgs end. + +mk_bootstrap_file(File) -> + Dir = code:lib_dir(emqx, test), + filename:join([Dir, <<"data/banned">>, File]). + +get_banned_list() -> + Tabs = emqx_banned:tables(), + lists:foldl( + fun(Tab, Acc) -> + Acc ++ ets:tab2list(Tab) + end, + [], + Tabs + ). diff --git a/apps/emqx_management/src/emqx_mgmt_api_banned.erl b/apps/emqx_management/src/emqx_mgmt_api_banned.erl index 0a24dab15..659eed48a 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_banned.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_banned.erl @@ -171,7 +171,7 @@ banned(post, #{body := Body}) -> {error, Reason} -> ErrorReason = io_lib:format("~p", [Reason]), {400, 'BAD_REQUEST', list_to_binary(ErrorReason)}; - Ban -> + {ok, Ban} -> case emqx_banned:create(Ban) of {ok, Banned} -> {200, format(Banned)}; diff --git a/apps/emqx_utils/src/emqx_utils_stream.erl b/apps/emqx_utils/src/emqx_utils_stream.erl index 8b6db8292..6e3ff079b 100644 --- a/apps/emqx_utils/src/emqx_utils_stream.erl +++ b/apps/emqx_utils/src/emqx_utils_stream.erl @@ -45,15 +45,18 @@ %% Streams from .csv data -export([ - csv/1 + csv/1, + csv/2 ]). --export_type([stream/1]). +-export_type([stream/1, csv_parse_opts/0]). %% @doc A stream is essentially a lazy list. -type stream(T) :: fun(() -> next(T) | []). -type next(T) :: nonempty_improper_list(T, stream(T)). +-type csv_parse_opts() :: #{nullable => boolean(), filter_null => boolean()}. + -dialyzer(no_improper_lists). -elvis([{elvis_style, nesting_level, disable}]). @@ -261,13 +264,42 @@ ets(Cont, ContF) -> %% @doc Make a stream out of a .csv binary, where the .csv binary is loaded in all at once. %% The .csv binary is assumed to be in UTF-8 encoding and to have a header row. -spec csv(binary()) -> stream(map()). -csv(Bin) when is_binary(Bin) -> +csv(Bin) -> + csv(Bin, #{}). + +-spec csv(binary(), csv_parse_opts()) -> stream(map()). +csv(Bin, Opts) when is_binary(Bin) -> + Liner = + case Opts of + #{nullable := true} -> + fun csv_read_nullable_line/1; + _ -> + fun csv_read_line/1 + end, + Maper = + case Opts of + #{filter_null := true} -> + fun(Headers, Fields) -> + maps:from_list( + lists:filter( + fun({_, Value}) -> + Value =/= undefined + end, + lists:zip(Headers, Fields) + ) + ) + end; + _ -> + fun(Headers, Fields) -> + maps:from_list(lists:zip(Headers, Fields)) + end + end, Reader = fun _Iter(Headers, Lines) -> - case csv_read_line(Lines) of + case Liner(Lines) of {Fields, Rest} -> case length(Fields) == length(Headers) of true -> - User = maps:from_list(lists:zip(Headers, Fields)), + User = Maper(Headers, Fields), [User | fun() -> _Iter(Headers, Rest) end]; false -> error(bad_format) @@ -291,6 +323,23 @@ csv_read_line([Line | Lines]) -> csv_read_line([]) -> eof. +csv_read_nullable_line([Line | Lines]) -> + %% XXX: not support ' ' for the field value + Fields = lists:map( + fun(Bin) -> + case string:trim(Bin, both) of + <<>> -> + undefined; + Any -> + Any + end + end, + binary:split(Line, [<<",">>], [global]) + ), + {Fields, Lines}; +csv_read_nullable_line([]) -> + eof. + do_interleave(_Cont, _, [], []) -> []; do_interleave(Cont, N, [{N, S} | Rest], Rev) -> diff --git a/rel/i18n/emqx_schema.hocon b/rel/i18n/emqx_schema.hocon index f9978fe6f..cea151a87 100644 --- a/rel/i18n/emqx_schema.hocon +++ b/rel/i18n/emqx_schema.hocon @@ -1630,4 +1630,22 @@ client_attrs_init_set_as_attr { The extracted attribute will be stored in the `client_attrs` property with this name.""" } +banned_bootstrap_file.desc: +"""The bootstrap file is a CSV file used to batch loading banned data when initializing a single node or cluster, in other words, the import operation is performed only if there is no data in the database. + +The delimiter for this file is `,`. + +The first line of this file must be a header line. All valid headers are listed here: +- as :: required +- who :: required +- by :: optional +- reason :: optional +- at :: optional +- until :: optional + +See the documentation for details on each field. + +Each row in the rest of this file must contain the same number of columns as the header line, +and column can be omitted then its value will be `undefined`.""" + } From a912751458d6a3e7f53a551bf3f8186a1c90c3f5 Mon Sep 17 00:00:00 2001 From: firest Date: Tue, 2 Jul 2024 13:47:33 +0800 Subject: [PATCH 10/41] chore: update changes --- changes/ee/feat-13386.en.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 changes/ee/feat-13386.en.md diff --git a/changes/ee/feat-13386.en.md b/changes/ee/feat-13386.en.md new file mode 100644 index 000000000..e038e0144 --- /dev/null +++ b/changes/ee/feat-13386.en.md @@ -0,0 +1,17 @@ +Added a bootstrap file to batch loading banned data when initializing a single node or cluster, in other words, the import operation is performed only if there is no data in the database. + + +This file is a CSV file with `,` as its delimiter. + +The first line of this file must be a header line. All valid headers are listed here: +- as :: required +- who :: required +- by :: optional +- reason :: optional +- at :: optional +- until :: optional + +See the documentation for details on each field. + +Each row in the rest of this file must contain the same number of columns as the header line, +and column can be omitted then its value will be `undefined`. From 8c6cd69caa6dffacd4552ed8f7fac1bce17aa903 Mon Sep 17 00:00:00 2001 From: JimMoen Date: Thu, 4 Jul 2024 14:30:04 +0800 Subject: [PATCH 11/41] fix: obtain cert expiry epoch failed due to formated `generalTime` --- .../src/emqx_prometheus.app.src | 2 +- apps/emqx_prometheus/src/emqx_prometheus.erl | 26 +++++++------------ changes/fix-13412.en.md | 1 + 3 files changed, 12 insertions(+), 17 deletions(-) create mode 100644 changes/fix-13412.en.md diff --git a/apps/emqx_prometheus/src/emqx_prometheus.app.src b/apps/emqx_prometheus/src/emqx_prometheus.app.src index 713a3e511..e5bb770cd 100644 --- a/apps/emqx_prometheus/src/emqx_prometheus.app.src +++ b/apps/emqx_prometheus/src/emqx_prometheus.app.src @@ -2,7 +2,7 @@ {application, emqx_prometheus, [ {description, "Prometheus for EMQX"}, % strict semver, bump manually! - {vsn, "5.2.2"}, + {vsn, "5.2.3"}, {modules, []}, {registered, [emqx_prometheus_sup]}, {applications, [kernel, stdlib, prometheus, emqx, emqx_auth, emqx_resource, emqx_management]}, diff --git a/apps/emqx_prometheus/src/emqx_prometheus.erl b/apps/emqx_prometheus/src/emqx_prometheus.erl index f4d0ff2c0..fd2faf11a 100644 --- a/apps/emqx_prometheus/src/emqx_prometheus.erl +++ b/apps/emqx_prometheus/src/emqx_prometheus.erl @@ -944,9 +944,7 @@ cert_expiry_at_from_path(Path0) -> [CertEntry | _] = public_key:pem_decode(PemBin), Cert = public_key:pem_entry_decode(CertEntry), %% TODO: Not fully tested for all certs type - {'utcTime', NotAfterUtc} = - Cert#'Certificate'.'tbsCertificate'#'TBSCertificate'.validity#'Validity'.'notAfter', - utc_time_to_epoch(NotAfterUtc); + not_after_epoch(Cert); {error, Reason} -> ?SLOG(error, #{ msg => "read_cert_file_failed", @@ -969,21 +967,17 @@ cert_expiry_at_from_path(Path0) -> 0 end. -utc_time_to_epoch(UtcTime) -> - date_to_expiry_epoch(utc_time_to_datetime(UtcTime)). - -utc_time_to_datetime(Str) -> - {ok, [Year, Month, Day, Hour, Minute, Second], _} = io_lib:fread( - "~2d~2d~2d~2d~2d~2dZ", Str - ), - %% Always Assuming YY is in 2000 - {{2000 + Year, Month, Day}, {Hour, Minute, Second}}. - %% 62167219200 =:= calendar:datetime_to_gregorian_seconds({{1970, 1, 1}, {0, 0, 0}}). -define(EPOCH_START, 62167219200). --spec date_to_expiry_epoch(calendar:datetime()) -> Seconds :: non_neg_integer(). -date_to_expiry_epoch(DateTime) -> - calendar:datetime_to_gregorian_seconds(DateTime) - ?EPOCH_START. +not_after_epoch(#'Certificate'{ + 'tbsCertificate' = #'TBSCertificate'{ + validity = + #'Validity'{'notAfter' = NotAfter} + } +}) -> + pubkey_cert:'time_str_2_gregorian_sec'(NotAfter) - ?EPOCH_START; +not_after_epoch(_) -> + 0. %%======================================== %% Mria diff --git a/changes/fix-13412.en.md b/changes/fix-13412.en.md new file mode 100644 index 000000000..0afc6cceb --- /dev/null +++ b/changes/fix-13412.en.md @@ -0,0 +1 @@ +Fixed an issue in the Prometheus API where the certificate expiration time format incorrectly returned `0` due to the use of `generalTime`. From d206d24975987774f8f95395c23ca813bae19d19 Mon Sep 17 00:00:00 2001 From: Kjell Winblad Date: Thu, 4 Jul 2024 14:32:10 +0200 Subject: [PATCH 12/41] fix: only set default for max_conn_rate and update test case This revert the change in commit e291dcd for all listener "short path fields" except the field max_conn_rate so they no longer have a default value. It also updates a test case that assume that no listener config is created by default but this is no longer the case when there is a default value for the max_conn_rate field. --- .../emqx_limiter/src/emqx_limiter_schema.erl | 26 ++++++++++++++----- apps/emqx/test/emqx_ratelimiter_SUITE.erl | 4 +-- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/apps/emqx/src/emqx_limiter/src/emqx_limiter_schema.erl b/apps/emqx/src/emqx_limiter/src/emqx_limiter_schema.erl index b9bfda166..32c92c8f0 100644 --- a/apps/emqx/src/emqx_limiter/src/emqx_limiter_schema.erl +++ b/apps/emqx/src/emqx_limiter/src/emqx_limiter_schema.erl @@ -212,17 +212,29 @@ short_paths_fields() -> short_paths_fields(Importance) -> [ {Name, - ?HOCON(rate_type(), #{ - desc => ?DESC(Name), - required => false, - default => infinity, - importance => Importance, - example => Example - })} + ?HOCON( + rate_type(), + maps:merge( + #{ + desc => ?DESC(Name), + required => false, + importance => Importance, + example => Example + }, + short_paths_fields_extra(Name) + ) + )} || {Name, Example} <- lists:zip(short_paths(), [<<"1000/s">>, <<"1000/s">>, <<"100MB/s">>]) ]. +short_paths_fields_extra(max_conn_rate) -> + #{ + default => infinity + }; +short_paths_fields_extra(_Name) -> + #{}. + desc(limiter) -> "Settings for the rate limiter."; desc(node_opts) -> diff --git a/apps/emqx/test/emqx_ratelimiter_SUITE.erl b/apps/emqx/test/emqx_ratelimiter_SUITE.erl index 28a05ce23..b76c5dd33 100644 --- a/apps/emqx/test/emqx_ratelimiter_SUITE.erl +++ b/apps/emqx/test/emqx_ratelimiter_SUITE.erl @@ -816,8 +816,8 @@ t_no_limiter_for_listener(_) -> CfgStr = <<>>, ok = emqx_common_test_helpers:load_config(emqx_schema, CfgStr), ListenerOpt = emqx:get_config([listeners, tcp, default]), - ?assertEqual( - undefined, + ?assertMatch( + #{connection := #{rate := infinity}}, emqx_limiter_utils:get_listener_opts(ListenerOpt) ). From d84d31cbc572f0b00856ed646fcfa1cac714b351 Mon Sep 17 00:00:00 2001 From: JimMoen Date: Thu, 4 Jul 2024 18:18:13 +0800 Subject: [PATCH 13/41] test: cert expiry epoch with `generalTime` formatted --- apps/emqx_prometheus/.gitignore | 1 - apps/emqx_prometheus/src/emqx_prometheus.erl | 6 +++++- apps/emqx_prometheus/test/data/cert.crt | 21 +++++++++++++++++++ .../test/emqx_prometheus_SUITE.erl | 14 +++++++++++++ 4 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 apps/emqx_prometheus/test/data/cert.crt diff --git a/apps/emqx_prometheus/.gitignore b/apps/emqx_prometheus/.gitignore index b89cbebfd..233b9f887 100644 --- a/apps/emqx_prometheus/.gitignore +++ b/apps/emqx_prometheus/.gitignore @@ -13,7 +13,6 @@ rel/example_project emqx_prometheus.d ct.coverdata logs/ -data/ test/ct.cover.spec cover/ erlang.mk diff --git a/apps/emqx_prometheus/src/emqx_prometheus.erl b/apps/emqx_prometheus/src/emqx_prometheus.erl index fd2faf11a..1474164f6 100644 --- a/apps/emqx_prometheus/src/emqx_prometheus.erl +++ b/apps/emqx_prometheus/src/emqx_prometheus.erl @@ -78,6 +78,10 @@ do_stop/0 ]). +-ifdef(TEST). +-export([cert_expiry_at_from_path/1]). +-endif. + %%-------------------------------------------------------------------- %% Macros %%-------------------------------------------------------------------- @@ -943,7 +947,7 @@ cert_expiry_at_from_path(Path0) -> {ok, PemBin} -> [CertEntry | _] = public_key:pem_decode(PemBin), Cert = public_key:pem_entry_decode(CertEntry), - %% TODO: Not fully tested for all certs type + %% XXX: Only pem cert supported by listeners not_after_epoch(Cert); {error, Reason} -> ?SLOG(error, #{ diff --git a/apps/emqx_prometheus/test/data/cert.crt b/apps/emqx_prometheus/test/data/cert.crt new file mode 100644 index 000000000..6f09b5609 --- /dev/null +++ b/apps/emqx_prometheus/test/data/cert.crt @@ -0,0 +1,21 @@ +-----BEGIN CERTIFICATE----- +MIIDfzCCAmegAwIBAgIUJ3pE/Dwffa5gKNHY2L8HmazicmowDQYJKoZIhvcNAQEL +BQAwZzELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFjAUBgNVBAcM +DVNhbiBGcmFuY2lzY28xFTATBgNVBAoMDEV4YW1wbGUgSW5jLjEUMBIGA1UEAwwL +ZXhhbXBsZS5jb20wIBcNMjQwNzAzMTAyOTMzWhgPMjA1NDA2MjYxMDI5MzNaMGcx +CzAJBgNVBAYTAlVTMRMwEQYDVQQIDApDYWxpZm9ybmlhMRYwFAYDVQQHDA1TYW4g +RnJhbmNpc2NvMRUwEwYDVQQKDAxFeGFtcGxlIEluYy4xFDASBgNVBAMMC2V4YW1w +bGUuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEArFZKxzsxCaGP +rVhilTd4PKk9jVrBLQ4xaFG6tmmlzjBCp+E35EulND4gpWZSUs9bYO/C+qykKmrL +J7TddGBVXe6lbl6mMHqZzHUp9mJdvPBSHcqOHc2E/UiBwOpN4tatx6UdK+VEQySr +z+dtc0Az5Itkoy/SvAu1Zzdq3d3MfxaTUvCmWfeR2huTalNQkG1jQ0C2CjCU9Z1f +Ex+y1MzxNhVrrdExC8Vwrb4TDlue8/XwJ4A4gBJYNbVAwALcSKnF56nRib3evE3J +Irvy2Rt4aC694JawWLPzJ1e2Rz8WBzCRPJAmaV4iD66sU8BMkmbCV+mMmF673s3R +sS4kGqklvQIDAQABoyEwHzAdBgNVHQ4EFgQU0tDKnCDey6fKrzs7caDfS41Dii4w +DQYJKoZIhvcNAQELBQADggEBAEIKvrSuUgpkIEUDV+UMr/5xUKkDyjNi4rwkBA6X +Ej0HskXg6u9wOIkBKwpQbleDFICdyqXMhGMjN4050PQCizaInBJBz77ah47UwGGQ +P+wavbcdHR9cbhewhCo6EtbCclPY1LXq4OFkgHMToLFzXC4S/kLX/KrhVApGHskO +Ad4U4gmMtIalruz5Mzc4YuSaAjbRI9v0IxhvS8JU0uoOwhIstkrMlFc26SU6EcZ9 +k88gVmmqEnsvmJi4gn4XPgvJB8hPs0/OMDBCVjAM8VaxZZ6sqlTT9FTGaKbIJdDc +KjT7VdbhVcuZo4s1u9gQzJNU2WHlHLwZi1wCjTC1vTE/HrQ= +-----END CERTIFICATE----- diff --git a/apps/emqx_prometheus/test/emqx_prometheus_SUITE.erl b/apps/emqx_prometheus/test/emqx_prometheus_SUITE.erl index c97913ec2..dd125f97b 100644 --- a/apps/emqx_prometheus/test/emqx_prometheus_SUITE.erl +++ b/apps/emqx_prometheus/test/emqx_prometheus_SUITE.erl @@ -211,6 +211,16 @@ t_push_gateway(_) -> ok. +t_cert_expiry_epoch(_) -> + Path = some_pem_path(), + ?assertEqual( + 2666082573, + emqx_prometheus:cert_expiry_at_from_path(Path) + ). + +%%-------------------------------------------------------------------- +%% Helper functions + start_mock_pushgateway(Port) -> ensure_loaded(cowboy), ensure_loaded(ranch), @@ -249,3 +259,7 @@ init(Req0, Opts) -> RespHeader = #{<<"content-type">> => <<"text/plain; charset=utf-8">>}, Req = cowboy_req:reply(200, RespHeader, <<"OK">>, Req0), {ok, Req, Opts}. + +some_pem_path() -> + Dir = code:lib_dir(emqx_prometheus, test), + _Path = filename:join([Dir, "data", "cert.crt"]). From d3d3303dcbe4d6fd38473d1da4df4990cf8c23d7 Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Thu, 4 Jul 2024 10:14:21 +0800 Subject: [PATCH 14/41] chore: improve auth error for invalid salt/password type --- apps/emqx/src/emqx_passwd.erl | 6 +++++- apps/emqx/test/emqx_passwd_SUITE.erl | 16 +++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/apps/emqx/src/emqx_passwd.erl b/apps/emqx/src/emqx_passwd.erl index c243442ba..dc3622411 100644 --- a/apps/emqx/src/emqx_passwd.erl +++ b/apps/emqx/src/emqx_passwd.erl @@ -102,7 +102,11 @@ hash({SimpleHash, _Salt, disable}, Password) when is_binary(Password) -> hash({SimpleHash, Salt, prefix}, Password) when is_binary(Password), is_binary(Salt) -> hash_data(SimpleHash, <>); hash({SimpleHash, Salt, suffix}, Password) when is_binary(Password), is_binary(Salt) -> - hash_data(SimpleHash, <>). + hash_data(SimpleHash, <>); +hash({_SimpleHash, Salt, _SaltPos}, _Password) when not is_binary(Salt) -> + error({salt_not_string, Salt}); +hash({_SimpleHash, _Salt, _SaltPos}, Password) when not is_binary(Password) -> + error({password_not_string, Password}). -spec hash_data(hash_type(), binary()) -> binary(). hash_data(plain, Data) when is_binary(Data) -> diff --git a/apps/emqx/test/emqx_passwd_SUITE.erl b/apps/emqx/test/emqx_passwd_SUITE.erl index fd032bdb1..3078a5805 100644 --- a/apps/emqx/test/emqx_passwd_SUITE.erl +++ b/apps/emqx/test/emqx_passwd_SUITE.erl @@ -124,4 +124,18 @@ t_hash(_) -> false = emqx_passwd:check_pass({pbkdf2, sha, Pbkdf2Salt, 2, BadDKlen}, Pbkdf2, Password), %% Invalid derived_length, pbkdf2 fails - ?assertException(error, _, emqx_passwd:hash({pbkdf2, sha, Pbkdf2Salt, 2, BadDKlen}, Password)). + ?assertException(error, _, emqx_passwd:hash({pbkdf2, sha, Pbkdf2Salt, 2, BadDKlen}, Password)), + + %% invalid salt (not binary) + ?assertException( + error, + {salt_not_string, false}, + emqx_passwd:hash({sha256, false, suffix}, Password) + ), + + %% invalid password (not binary) + ?assertException( + error, + {password_not_string, bad_password_type}, + emqx_passwd:hash({sha256, Salt, suffix}, bad_password_type) + ). From 755d6c9e0f29fbcfd47b7bc35063314b5de399cc Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Fri, 5 Jul 2024 10:21:04 +0800 Subject: [PATCH 15/41] chore: add changelog for 13398 and 13408 --- changes/ce/fix-13398.en.md | 1 + changes/ce/fix-13408.en.md | 1 + 2 files changed, 2 insertions(+) create mode 100644 changes/ce/fix-13398.en.md create mode 100644 changes/ce/fix-13408.en.md diff --git a/changes/ce/fix-13398.en.md b/changes/ce/fix-13398.en.md new file mode 100644 index 000000000..fb2f891e8 --- /dev/null +++ b/changes/ce/fix-13398.en.md @@ -0,0 +1 @@ +Fix acl rule clearing when reloading built-in-database for authorization using command line. diff --git a/changes/ce/fix-13408.en.md b/changes/ce/fix-13408.en.md new file mode 100644 index 000000000..e27482d91 --- /dev/null +++ b/changes/ce/fix-13408.en.md @@ -0,0 +1 @@ +Fix function_clause crash that occurs when attempting to authenticate with an invalid type of salt or password. From 3e69a52596d1fae57003436be21d00ee1c77f854 Mon Sep 17 00:00:00 2001 From: JimMoen Date: Fri, 5 Jul 2024 14:06:58 +0800 Subject: [PATCH 16/41] build: avoid warnings during docker build - See also: https://docs.docker.com/reference/build-checks/from-as-casing/ --- deploy/docker/Dockerfile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/deploy/docker/Dockerfile b/deploy/docker/Dockerfile index 4f068fab6..89524b4a7 100644 --- a/deploy/docker/Dockerfile +++ b/deploy/docker/Dockerfile @@ -2,17 +2,17 @@ ARG BUILD_FROM=ghcr.io/emqx/emqx-builder/5.3-9:1.15.7-26.2.5-3-debian12 ARG RUN_FROM=public.ecr.aws/debian/debian:stable-20240612-slim ARG SOURCE_TYPE=src # tgz -FROM ${BUILD_FROM} as builder_src +FROM ${BUILD_FROM} AS builder_src ONBUILD COPY . /emqx -FROM ${RUN_FROM} as builder_tgz +FROM ${RUN_FROM} AS builder_tgz ARG PROFILE=emqx ARG PKG_VSN ARG SUFFIX ARG TARGETARCH ONBUILD COPY ${PROFILE}-${PKG_VSN}${SUFFIX}-debian12-$TARGETARCH.tar.gz /${PROFILE}.tar.gz -FROM builder_${SOURCE_TYPE} as builder +FROM builder_${SOURCE_TYPE} AS builder ARG PROFILE=emqx ARG IS_ELIXIR=no From b4cffc581b27c5151cd5f79d75202ef48a596a4b Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Wed, 12 Jun 2024 14:41:34 +0800 Subject: [PATCH 17/41] fix: ws/wss's max_frame_size should > 0 --- apps/emqx/src/emqx_schema.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index 1ba71fb00..607f74451 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/apps/emqx/src/emqx_schema.erl @@ -890,7 +890,7 @@ fields("ws_opts") -> )}, {"max_frame_size", sc( - hoconsc:union([infinity, integer()]), + hoconsc:union([infinity, pos_integer()]), #{ default => infinity, desc => ?DESC(fields_ws_opts_max_frame_size) From 8aab919f74413c7cf5075b8a02ea593ad5123644 Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Wed, 12 Jun 2024 15:11:09 +0800 Subject: [PATCH 18/41] fix: load bad configs return unknown msg --- apps/emqx_conf/src/emqx_conf_cli.erl | 6 ++++++ apps/emqx_management/src/emqx_mgmt_api_configs.erl | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/apps/emqx_conf/src/emqx_conf_cli.erl b/apps/emqx_conf/src/emqx_conf_cli.erl index 257cc7453..f2b9d63b0 100644 --- a/apps/emqx_conf/src/emqx_conf_cli.erl +++ b/apps/emqx_conf/src/emqx_conf_cli.erl @@ -237,6 +237,12 @@ load_config(Bin, Opts) when is_binary(Bin) -> case hocon:binary(Bin) of {ok, RawConf} -> load_config_from_raw(RawConf, Opts); + %% Type is scan_error, parse_error... + {error, {Type, Meta = #{reason := Reason}}} -> + {error, Meta#{ + reason => unicode:characters_to_binary(Reason), + type => Type + }}; {error, Reason} -> {error, Reason} end. diff --git a/apps/emqx_management/src/emqx_mgmt_api_configs.erl b/apps/emqx_management/src/emqx_mgmt_api_configs.erl index 75341facc..56e1f9bc1 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_configs.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_configs.erl @@ -364,10 +364,10 @@ configs(get, #{query_string := QueryStr, headers := Headers}, _Req) -> {error, _} = Error -> {400, #{code => 'INVALID_ACCEPT', message => ?ERR_MSG(Error)}} end; configs(put, #{body := Conf, query_string := #{<<"mode">> := Mode} = QS}, _Req) -> - IngnoreReadonly = maps:get(<<"ignore_readonly">>, QS, false), + IgnoreReadonly = maps:get(<<"ignore_readonly">>, QS, false), case emqx_conf_cli:load_config(Conf, #{ - mode => Mode, log => none, ignore_readonly => IngnoreReadonly + mode => Mode, log => none, ignore_readonly => IgnoreReadonly }) of ok -> From f0a1d785cad3424cff36c90b3808f4c37134eea5 Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Wed, 12 Jun 2024 15:22:56 +0800 Subject: [PATCH 19/41] fix: don't allow set active_n to negative int --- apps/emqx/src/emqx_schema.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index 607f74451..b6be28d21 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/apps/emqx/src/emqx_schema.erl @@ -970,7 +970,7 @@ fields("tcp_opts") -> [ {"active_n", sc( - integer(), + non_neg_integer(), #{ default => 100, desc => ?DESC(fields_tcp_opts_active_n) From ba3097dc56c5130b0d6d66a11fcb390123566c17 Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Wed, 12 Jun 2024 17:04:52 +0800 Subject: [PATCH 20/41] fix: observer command crash when can't find object code --- apps/emqx_modules/src/emqx_observer_cli.erl | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/apps/emqx_modules/src/emqx_observer_cli.erl b/apps/emqx_modules/src/emqx_observer_cli.erl index 9359392c6..043b1430e 100644 --- a/apps/emqx_modules/src/emqx_observer_cli.erl +++ b/apps/emqx_modules/src/emqx_observer_cli.erl @@ -46,8 +46,13 @@ cmd(["load", Mod]) -> Nodes -> case emqx_utils:safe_to_existing_atom(Mod) of {ok, Module} -> - Res = recon:remote_load(Nodes, Module), - emqx_ctl:print("Loaded ~p module on ~p: ~p~n", [Module, Nodes, Res]); + case code:get_object_code(Module) of + error -> + emqx_ctl:print("Module(~s)'s object code not found~n", [Mod]); + _ -> + Res = recon:remote_load(Nodes, Module), + emqx_ctl:print("Loaded ~p module on ~p: ~p~n", [Module, Nodes, Res]) + end; {error, Reason} -> emqx_ctl:print("Module(~s) not found: ~p~n", [Mod, Reason]) end From d94fcb9cfd65e1323c13352543e5e38cd584e18b Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Thu, 13 Jun 2024 10:44:29 +0800 Subject: [PATCH 21/41] test: fix api_config SUITE failed --- .../test/emqx_mgmt_api_configs_SUITE.erl | 37 ++++++++++++++++--- 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl index a2d4d21af..10906183b 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl @@ -310,7 +310,7 @@ t_configs_key(_Config) -> ReadOnlyBin = iolist_to_binary(hocon_pp:do(ReadOnlyConf, #{})), {error, ReadOnlyError} = update_configs_with_binary(ReadOnlyBin), ?assertEqual(<<"{\"errors\":\"Cannot update read-only key 'cluster'.\"}">>, ReadOnlyError), - ?assertMatch({ok, <<>>}, update_configs_with_binary(ReadOnlyBin, _InogreReadonly = true)), + ?assertMatch({ok, <<>>}, update_configs_with_binary(ReadOnlyBin, _IgnoreReadonly = true)), ok. t_get_configs_in_different_accept(_Config) -> @@ -443,13 +443,38 @@ t_create_webhook_v1_bridges_api(Config) -> ok. t_config_update_parse_error(_Config) -> - ?assertMatch( - {error, <<"{\"errors\":\"{parse_error,", _/binary>>}, - update_configs_with_binary(<<"not an object">>) + BadHoconList = [ + <<"not an object">>, + <<"a = \"tlsv1\"\"\"3e-01">> + ], + lists:map( + fun(BadHocon) -> + {error, ParseError} = update_configs_with_binary(BadHocon), + ?assertMatch( + #{ + <<"errors">> := + #{ + <<"line">> := 1, + <<"reason">> := _, + <<"type">> := <<"parse_error">> + } + }, + emqx_utils_json:decode(ParseError) + ) + end, + BadHoconList ), + + {error, ScanError} = update_configs_with_binary(<<"a=测试"/utf8>>), ?assertMatch( - {error, <<"{\"errors\":\"{parse_error,", _/binary>>}, - update_configs_with_binary(<<"a = \"tlsv1\"\"\"3e-01">>) + #{ + <<"errors">> := #{ + <<"line">> := 1, + <<"reason">> := _, + <<"type">> := <<"scan_error">> + } + }, + emqx_utils_json:decode(ScanError) ). t_config_update_unknown_root(_Config) -> From 9ffe6420c2b571ab0a301fa0a9733983e8ab5838 Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Fri, 5 Jul 2024 17:41:02 +0800 Subject: [PATCH 22/41] chore: add changelog for 13419 --- changes/ce/fix-13419.en.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/ce/fix-13419.en.md diff --git a/changes/ce/fix-13419.en.md b/changes/ce/fix-13419.en.md new file mode 100644 index 000000000..95f82e970 --- /dev/null +++ b/changes/ce/fix-13419.en.md @@ -0,0 +1 @@ +Fix garbled hints in crash log message when calling /configs API From 7d851872ec91f41d6aabeeb7ca35178e65b60cf5 Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Fri, 5 Jul 2024 19:21:28 +0800 Subject: [PATCH 23/41] chore: update emqx_module's app version --- apps/emqx_modules/src/emqx_modules.app.src | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/emqx_modules/src/emqx_modules.app.src b/apps/emqx_modules/src/emqx_modules.app.src index 233adf7ad..19caf6763 100644 --- a/apps/emqx_modules/src/emqx_modules.app.src +++ b/apps/emqx_modules/src/emqx_modules.app.src @@ -1,7 +1,7 @@ %% -*- mode: erlang -*- {application, emqx_modules, [ {description, "EMQX Modules"}, - {vsn, "5.0.26"}, + {vsn, "5.0.27"}, {modules, []}, {applications, [kernel, stdlib, emqx, emqx_ctl, observer_cli]}, {mod, {emqx_modules_app, []}}, From e99fee68c098b65ca9946e784ccc97e5b86d60e3 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Fri, 5 Jul 2024 09:46:20 -0300 Subject: [PATCH 24/41] fix(message transformation): forbid empty topic filter list Fixes https://emqx.atlassian.net/browse/EMQX-12646 --- .../src/emqx_message_transformation_schema.erl | 2 ++ .../test/emqx_message_transformation_tests.erl | 13 +++++++++++++ 2 files changed, 15 insertions(+) diff --git a/apps/emqx_message_transformation/src/emqx_message_transformation_schema.erl b/apps/emqx_message_transformation/src/emqx_message_transformation_schema.erl index 9dee9d5b0..169743e5b 100644 --- a/apps/emqx_message_transformation/src/emqx_message_transformation_schema.erl +++ b/apps/emqx_message_transformation/src/emqx_message_transformation_schema.erl @@ -231,6 +231,8 @@ do_validate_unique_names([#{<<"name">> := Name} | _Rest], Acc) when is_map_key(N do_validate_unique_names([#{<<"name">> := Name} | Rest], Acc) -> do_validate_unique_names(Rest, Acc#{Name => true}). +validate_unique_topics([]) -> + {error, <<"at least one topic filter must be defined">>}; validate_unique_topics(Topics) -> Grouped = maps:groups_from_list( fun(T) -> T end, diff --git a/apps/emqx_message_transformation/test/emqx_message_transformation_tests.erl b/apps/emqx_message_transformation/test/emqx_message_transformation_tests.erl index 3e86e3862..545773c00 100644 --- a/apps/emqx_message_transformation/test/emqx_message_transformation_tests.erl +++ b/apps/emqx_message_transformation/test/emqx_message_transformation_tests.erl @@ -87,6 +87,19 @@ schema_test_() -> ) ]) )}, + {"topics must be non-empty", + ?_assertThrow( + {_Schema, [ + #{ + reason := <<"at least one topic filter must be defined", _/binary>>, + value := [], + kind := validation_error + } + ]}, + parse_and_check([ + transformation(<<"foo">>, [dummy_operation()], #{<<"topics">> => []}) + ]) + )}, {"names are unique", ?_assertThrow( {_Schema, [ From e7351d949d578d6f59e6fddceff8147d2004fcba Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Fri, 5 Jul 2024 09:49:09 -0300 Subject: [PATCH 25/41] fix(schema validation): forbid empty topic filter list --- .../src/emqx_schema_validation.app.src | 2 +- .../src/emqx_schema_validation_schema.erl | 2 ++ .../test/emqx_schema_validation_tests.erl | 13 +++++++++++++ changes/ee/fix-13420.en.md | 1 + 4 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 changes/ee/fix-13420.en.md diff --git a/apps/emqx_schema_validation/src/emqx_schema_validation.app.src b/apps/emqx_schema_validation/src/emqx_schema_validation.app.src index 2dfe710db..31b2a30fc 100644 --- a/apps/emqx_schema_validation/src/emqx_schema_validation.app.src +++ b/apps/emqx_schema_validation/src/emqx_schema_validation.app.src @@ -1,6 +1,6 @@ {application, emqx_schema_validation, [ {description, "EMQX Schema Validation"}, - {vsn, "0.1.1"}, + {vsn, "0.1.2"}, {registered, [emqx_schema_validation_sup, emqx_schema_validation_registry]}, {mod, {emqx_schema_validation_app, []}}, {applications, [ diff --git a/apps/emqx_schema_validation/src/emqx_schema_validation_schema.erl b/apps/emqx_schema_validation/src/emqx_schema_validation_schema.erl index fa9461745..8984d1066 100644 --- a/apps/emqx_schema_validation/src/emqx_schema_validation_schema.erl +++ b/apps/emqx_schema_validation/src/emqx_schema_validation_schema.erl @@ -259,6 +259,8 @@ do_validate_unique_schema_checks( do_validate_unique_schema_checks([_Check | Rest], Seen, Duplicated) -> do_validate_unique_schema_checks(Rest, Seen, Duplicated). +validate_unique_topics([]) -> + {error, <<"at least one topic filter must be defined">>}; validate_unique_topics(Topics) -> Grouped = maps:groups_from_list( fun(T) -> T end, diff --git a/apps/emqx_schema_validation/test/emqx_schema_validation_tests.erl b/apps/emqx_schema_validation/test/emqx_schema_validation_tests.erl index a75e4b556..d6e18da92 100644 --- a/apps/emqx_schema_validation/test/emqx_schema_validation_tests.erl +++ b/apps/emqx_schema_validation/test/emqx_schema_validation_tests.erl @@ -117,6 +117,19 @@ schema_test_() -> ) ]) )}, + {"topics must be non-empty", + ?_assertThrow( + {_Schema, [ + #{ + reason := <<"at least one topic filter must be defined", _/binary>>, + value := [], + kind := validation_error + } + ]}, + parse_and_check([ + validation(<<"foo">>, [sql_check()], #{<<"topics">> => []}) + ]) + )}, {"foreach expression is not allowed", ?_assertThrow( {_Schema, [ diff --git a/changes/ee/fix-13420.en.md b/changes/ee/fix-13420.en.md new file mode 100644 index 000000000..994ee8a88 --- /dev/null +++ b/changes/ee/fix-13420.en.md @@ -0,0 +1 @@ +Added a schema validation to forbid empty topic filter lists when configuring a Schema Validation. From 36ee7bed77fc32e41e844fb612a587f5dd2d727c Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Fri, 5 Jul 2024 09:59:27 -0300 Subject: [PATCH 26/41] docs(message transformation): add api examples Fixes https://emqx.atlassian.net/browse/EMQX-12645 --- .../emqx_message_transformation_http_api.erl | 52 +++++++++---------- 1 file changed, 24 insertions(+), 28 deletions(-) diff --git a/apps/emqx_message_transformation/src/emqx_message_transformation_http_api.erl b/apps/emqx_message_transformation/src/emqx_message_transformation_http_api.erl index 1ba5cee8a..9b4afb914 100644 --- a/apps/emqx_message_transformation/src/emqx_message_transformation_http_api.erl +++ b/apps/emqx_message_transformation/src/emqx_message_transformation_http_api.erl @@ -452,18 +452,12 @@ ref(Struct) -> hoconsc:ref(?MODULE, Struct). mk(Type, Opts) -> hoconsc:mk(Type, Opts). array(Type) -> hoconsc:array(Type). -%% FIXME: all examples example_input_create() -> #{ - <<"sql_check">> => + <<"message_transformation">> => #{ - summary => <<"Using a SQL check">>, - value => example_transformation([example_sql_check()]) - }, - <<"avro_check">> => - #{ - summary => <<"Using an Avro schema check">>, - value => example_transformation([example_avro_check()]) + summary => <<"Simple message transformation">>, + value => example_transformation() } }. @@ -472,7 +466,7 @@ example_input_update() -> <<"update">> => #{ summary => <<"Update">>, - value => example_transformation([example_sql_check()]) + value => example_transformation() } }. @@ -493,20 +487,28 @@ example_input_dryrun_transformation() -> #{ summary => <<"Test an input against a configuration">>, value => #{ - todo => true + message => #{ + client_attrs => #{}, + payload => <<"{}">>, + qos => 2, + retain => true, + topic => <<"t/u/v">>, + user_property => #{} + }, + transformation => example_transformation() } } }. example_return_list() -> - OtherVal0 = example_transformation([example_avro_check()]), + OtherVal0 = example_transformation(), OtherVal = OtherVal0#{name => <<"other_transformation">>}, #{ <<"list">> => #{ summary => <<"List">>, value => [ - example_transformation([example_sql_check()]), + example_transformation(), OtherVal ] } @@ -547,29 +549,23 @@ example_return_metrics() -> } }. -example_transformation(Checks) -> +example_transformation() -> #{ name => <<"my_transformation">>, enable => true, description => <<"my transformation">>, tags => [<<"transformation">>], topics => [<<"t/+">>], - strategy => <<"all_pass">>, failure_action => <<"drop">>, log_failure => #{<<"level">> => <<"info">>}, - checks => Checks - }. - -example_sql_check() -> - #{ - type => <<"sql">>, - sql => <<"select payload.temp as t where t > 10">> - }. - -example_avro_check() -> - #{ - type => <<"avro">>, - schema => <<"my_avro_schema">> + payload_decoder => #{<<"type">> => <<"json">>}, + payload_encoder => #{<<"type">> => <<"json">>}, + operations => [ + #{ + key => <<"topic">>, + value => <<"concat([topic, '/', payload.t])">> + } + ] }. error_schema(Code, Message) -> From 70fab51354e5fe6e2f33291a6a84b735f49b5336 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Fri, 5 Jul 2024 10:23:21 -0300 Subject: [PATCH 27/41] fix: handle `max_heap_size` = 0 Fixes https://github.com/emqx/emqx/issues/13417 Fixes https://emqx.atlassian.net/browse/EMQX-12659 --- apps/emqx_utils/src/emqx_utils.erl | 4 +++- apps/emqx_utils/test/emqx_utils_SUITE.erl | 16 ++++++++++++++++ changes/ce/fix-13422.en.md | 1 + rel/i18n/emqx_schema.hocon | 2 +- 4 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 changes/ce/fix-13422.en.md diff --git a/apps/emqx_utils/src/emqx_utils.erl b/apps/emqx_utils/src/emqx_utils.erl index e4f0d91d1..de74d06d6 100644 --- a/apps/emqx_utils/src/emqx_utils.erl +++ b/apps/emqx_utils/src/emqx_utils.erl @@ -289,8 +289,10 @@ do_check_oom([{Val, Max, Reason} | Rest]) -> end. tune_heap_size(#{enable := false}) -> - ok; + ignore; %% If the max_heap_size is set to zero, the limit is disabled. +tune_heap_size(#{max_heap_size := 0}) -> + ignore; tune_heap_size(#{max_heap_size := MaxHeapSize}) when MaxHeapSize > 0 -> MaxSize = case erlang:system_info(wordsize) of diff --git a/apps/emqx_utils/test/emqx_utils_SUITE.erl b/apps/emqx_utils/test/emqx_utils_SUITE.erl index c124f5384..9887fc41c 100644 --- a/apps/emqx_utils/test/emqx_utils_SUITE.erl +++ b/apps/emqx_utils/test/emqx_utils_SUITE.erl @@ -154,6 +154,22 @@ t_check(_) -> emqx_utils:check_oom(Policy) ). +t_tune_heap_size(_Config) -> + Policy = #{ + max_mailbox_size => 10, + max_heap_size => 1024 * 1024 * 8, + enable => true + }, + ?assertEqual(ignore, emqx_utils:tune_heap_size(Policy#{enable := false})), + %% Setting it to 0 disables the check. + ?assertEqual(ignore, emqx_utils:tune_heap_size(Policy#{max_heap_size := 0})), + {max_heap_size, PreviousHeapSize} = process_info(self(), max_heap_size), + try + ?assertMatch(PreviousHeapSize, emqx_utils:tune_heap_size(Policy)) + after + process_flag(max_heap_size, PreviousHeapSize) + end. + t_rand_seed(_) -> ?assert(is_tuple(emqx_utils:rand_seed())). diff --git a/changes/ce/fix-13422.en.md b/changes/ce/fix-13422.en.md new file mode 100644 index 000000000..78b66c72a --- /dev/null +++ b/changes/ce/fix-13422.en.md @@ -0,0 +1 @@ +Fixed an issue where the option `force_shutdown.max_heap_size` could not be set to 0 to disable this tuning. diff --git a/rel/i18n/emqx_schema.hocon b/rel/i18n/emqx_schema.hocon index cea151a87..e2ec44cc9 100644 --- a/rel/i18n/emqx_schema.hocon +++ b/rel/i18n/emqx_schema.hocon @@ -801,7 +801,7 @@ mqtt_max_topic_levels.label: """Max Topic Levels""" force_shutdown_max_heap_size.desc: -"""Total heap size""" +"""Total heap size. Setting this to 0 disables this limitation.""" force_shutdown_max_heap_size.label: """Total heap size""" From f1b4467fe1b1e5dd09028ad96ce1511287979dd9 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Fri, 5 Jul 2024 14:17:42 -0300 Subject: [PATCH 28/41] test(plugins): fix flaky test Hypothesis is that both peer nodes were using the same directory and stepping on each other's toes. --- apps/emqx_plugins/test/emqx_plugins_SUITE.erl | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/apps/emqx_plugins/test/emqx_plugins_SUITE.erl b/apps/emqx_plugins/test/emqx_plugins_SUITE.erl index aa97c6584..f98bf5b5f 100644 --- a/apps/emqx_plugins/test/emqx_plugins_SUITE.erl +++ b/apps/emqx_plugins/test/emqx_plugins_SUITE.erl @@ -906,7 +906,8 @@ group_t_cluster_leave(Config) -> %% hooks added by the plugin's `application:start/2' callback are indeed in place. %% See also: https://github.com/emqx/emqx/issues/13378 t_start_node_with_plugin_enabled({init, Config}) -> - #{package := Package, shdir := InstallDir} = get_demo_plugin_package(), + #{package := Package} = get_demo_plugin_package(), + Basename = filename:basename(Package), NameVsn = filename:basename(Package, ?PACKAGE_SUFFIX), AppSpecs = [ emqx, @@ -917,7 +918,7 @@ t_start_node_with_plugin_enabled({init, Config}) -> #{ plugins => #{ - install_dir => InstallDir, + install_dir => <<"plugins">>, states => [ #{ @@ -938,6 +939,14 @@ t_start_node_with_plugin_enabled({init, Config}) -> ], #{work_dir => emqx_cth_suite:work_dir(?FUNCTION_NAME, Config)} ), + lists:foreach( + fun(#{work_dir := WorkDir}) -> + Destination = filename:join([WorkDir, "plugins", Basename]), + ok = filelib:ensure_dir(Destination), + {ok, _} = file:copy(Package, Destination) + end, + Specs + ), Names = [Name1, Name2], Nodes = [emqx_cth_cluster:node_name(N) || N <- Names], [ @@ -955,7 +964,9 @@ t_start_node_with_plugin_enabled(Config) when is_list(Config) -> ?check_trace( #{timetrap => 10_000}, begin - [N1, N2 | _] = emqx_cth_cluster:start(NodeSpecs), + %% Hack: we use `restart' here to disable the clean slate verification, as we + %% just created and populated the `plugins' directory... + [N1, N2 | _] = lists:flatmap(fun emqx_cth_cluster:restart/1, NodeSpecs), ?ON(N1, assert_started_and_hooks_loaded()), ?ON(N2, assert_started_and_hooks_loaded()), %% Now make them join. From f76444fbf8690c1d876a80a680ce6564c49ceae4 Mon Sep 17 00:00:00 2001 From: JimMoen Date: Mon, 8 Jul 2024 15:35:20 +0800 Subject: [PATCH 29/41] fix: create authn jwt with bad public key --- apps/emqx_auth_jwt/src/emqx_authn_jwt.erl | 69 ++++++++++++++++------- changes/fix-13432.en.md | 1 + 2 files changed, 49 insertions(+), 21 deletions(-) create mode 100644 changes/fix-13432.en.md diff --git a/apps/emqx_auth_jwt/src/emqx_authn_jwt.erl b/apps/emqx_auth_jwt/src/emqx_authn_jwt.erl index 33e7ee645..18c954ab5 100644 --- a/apps/emqx_auth_jwt/src/emqx_authn_jwt.erl +++ b/apps/emqx_auth_jwt/src/emqx_authn_jwt.erl @@ -18,6 +18,7 @@ -include_lib("emqx_auth/include/emqx_authn.hrl"). -include_lib("emqx/include/logger.hrl"). +-include_lib("jose/include/jose_jwk.hrl"). -export([ create/2, @@ -38,7 +39,7 @@ create(_AuthenticatorID, Config) -> create(Config). create(#{verify_claims := VerifyClaims} = Config) -> - create2(Config#{verify_claims => handle_verify_claims(VerifyClaims)}). + do_create(Config#{verify_claims => handle_verify_claims(VerifyClaims)}). update( #{use_jwks := false} = Config, @@ -83,6 +84,7 @@ authenticate( } ) -> JWT = maps:get(From, Credential), + %% XXX: Only supports single public key JWKs = [JWK], VerifyClaims = replace_placeholder(VerifyClaims0, Credential), verify(JWT, JWKs, VerifyClaims, AclClaimName, DisconnectAfterExpire); @@ -119,7 +121,7 @@ destroy(_) -> %% Internal functions %%-------------------------------------------------------------------- -create2(#{ +do_create(#{ use_jwks := false, algorithm := 'hmac-based', secret := Secret0, @@ -142,24 +144,35 @@ create2(#{ from => From }} end; -create2(#{ - use_jwks := false, - algorithm := 'public-key', - public_key := PublicKey, - verify_claims := VerifyClaims, - disconnect_after_expire := DisconnectAfterExpire, - acl_claim_name := AclClaimName, - from := From -}) -> - JWK = create_jwk_from_public_key(PublicKey), - {ok, #{ - jwk => JWK, - verify_claims => VerifyClaims, - disconnect_after_expire => DisconnectAfterExpire, - acl_claim_name => AclClaimName, - from => From - }}; -create2( +do_create( + #{ + use_jwks := false, + algorithm := 'public-key', + public_key := PublicKey, + verify_claims := VerifyClaims, + disconnect_after_expire := DisconnectAfterExpire, + acl_claim_name := AclClaimName, + from := From + } = Config +) -> + case + create_jwk_from_public_key( + maps:get(enable, Config, false), + PublicKey + ) + of + {ok, JWK} -> + {ok, #{ + jwk => JWK, + verify_claims => VerifyClaims, + disconnect_after_expire => DisconnectAfterExpire, + acl_claim_name => AclClaimName, + from => From + }}; + {error, _Reason} = Err -> + Err + end; +do_create( #{ use_jwks := true, verify_claims := VerifyClaims, @@ -183,9 +196,23 @@ create2( from => From }}. -create_jwk_from_public_key(PublicKey) when +create_jwk_from_public_key(true, PublicKey) when is_binary(PublicKey); is_list(PublicKey) -> + try do_create_jwk_from_public_key(PublicKey) of + %% XXX: Only supports single public key + #jose_jwk{} = Res -> + {ok, Res}; + _ -> + {error, invalid_public_key} + catch + _:_ -> + {error, invalid_public_key} + end; +create_jwk_from_public_key(false, _PublicKey) -> + {ok, []}. + +do_create_jwk_from_public_key(PublicKey) -> case filelib:is_file(PublicKey) of true -> jose_jwk:from_pem_file(PublicKey); diff --git a/changes/fix-13432.en.md b/changes/fix-13432.en.md new file mode 100644 index 000000000..d1f56f052 --- /dev/null +++ b/changes/fix-13432.en.md @@ -0,0 +1 @@ +Fixed the issue where JWT authentication was silently bypassed when an invalid public key (or invalid public key file path) was used. From ae3b8fe146dff2b1c533f28d2cac82878c6a182c Mon Sep 17 00:00:00 2001 From: JimMoen Date: Mon, 8 Jul 2024 16:34:24 +0800 Subject: [PATCH 30/41] test: create jwt authenticator with bad public key --- .../test/data/bad_public_key_file.pem | 3 ++ .../test/emqx_authn_jwt_SUITE.erl | 46 +++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 apps/emqx_auth/test/data/bad_public_key_file.pem diff --git a/apps/emqx_auth/test/data/bad_public_key_file.pem b/apps/emqx_auth/test/data/bad_public_key_file.pem new file mode 100644 index 000000000..526dbf577 --- /dev/null +++ b/apps/emqx_auth/test/data/bad_public_key_file.pem @@ -0,0 +1,3 @@ +-----BEGIN PUBLIC KEY----- +XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX +-----END PUBLIC KEY----- diff --git a/apps/emqx_auth_jwt/test/emqx_authn_jwt_SUITE.erl b/apps/emqx_auth_jwt/test/emqx_authn_jwt_SUITE.erl index 8bf0cc68a..48b6d3887 100644 --- a/apps/emqx_auth_jwt/test/emqx_authn_jwt_SUITE.erl +++ b/apps/emqx_auth_jwt/test/emqx_authn_jwt_SUITE.erl @@ -178,6 +178,7 @@ t_public_key(_) -> from => password, acl_claim_name => <<"acl">>, use_jwks => false, + enable => true, algorithm => 'public-key', public_key => PublicKey, verify_claims => [], @@ -199,6 +200,51 @@ t_public_key(_) -> ?assertEqual(ok, emqx_authn_jwt:destroy(State)), ok. +t_bad_public_keys(_) -> + BaseConfig = #{ + mechanism => jwt, + from => password, + acl_claim_name => <<"acl">>, + use_jwks => false, + algorithm => 'public-key', + verify_claims => [], + disconnect_after_expire => false + }, + + %% try create with invalid public key + ?assertMatch( + {error, invalid_public_key}, + emqx_authn_jwt:create(?AUTHN_ID, BaseConfig#{ + enable => true, + public_key => <<"bad_public_key">> + }) + ), + + %% no such file + ?assertMatch( + {error, invalid_public_key}, + emqx_authn_jwt:create(?AUTHN_ID, BaseConfig#{ + enable => true, + public_key => data_file("bad_flie_path.pem") + }) + ), + + %% bad public key file content + ?assertMatch( + {error, invalid_public_key}, + emqx_authn_jwt:create(?AUTHN_ID, BaseConfig#{ + enable => true, + public_key => data_file("bad_public_key_file.pem") + }) + ), + + %% assume jwk authenticator is disabled + {ok, State} = + emqx_authn_jwt:create(?AUTHN_ID, BaseConfig#{public_key => <<"bad_public_key">>}), + + ?assertEqual(ok, emqx_authn_jwt:destroy(State)), + ok. + t_jwt_in_username(_) -> Secret = <<"abcdef">>, Config = #{ From 893630aee3dc2f7b59df10c3ccad151d4744cb36 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Mon, 8 Jul 2024 10:18:12 -0300 Subject: [PATCH 31/41] docs: add breaking change entry Fixes https://github.com/emqx/emqx/pull/13420#issuecomment-2213957235 --- changes/ee/breaking-13420.en.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/ee/breaking-13420.en.md diff --git a/changes/ee/breaking-13420.en.md b/changes/ee/breaking-13420.en.md new file mode 100644 index 000000000..dc7a05134 --- /dev/null +++ b/changes/ee/breaking-13420.en.md @@ -0,0 +1 @@ +Added a schema validation that prevents configuring an empty set of topic filters for a Schema Validation. Any such configurations will have to define at least one topic filter to be valid. Such configurations, though, are probably very rare, as a Schema Validation with empty topics is essentially the same as having no validation at all. From 811184ddad9a14ec4956e8d96ccb1c45ce61c743 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Mon, 8 Jul 2024 13:29:21 -0300 Subject: [PATCH 32/41] feat(jwks): allow specifying custom request headers Fixes https://emqx.atlassian.net/browse/EMQX-12655 --- apps/emqx/src/emqx_schema.erl | 4 +- .../src/emqx_authn_jwks_client.erl | 15 ++- .../src/emqx_authn_jwt_schema.erl | 32 +++++ .../test/emqx_authn_jwt_SUITE.erl | 116 +++++++++++++++++- changes/ce/feat-13436.en.md | 1 + rel/i18n/emqx_authn_jwt_schema.hocon | 5 + 6 files changed, 170 insertions(+), 3 deletions(-) create mode 100644 changes/ce/feat-13436.en.md diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index b6be28d21..d639523bb 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/apps/emqx/src/emqx_schema.erl @@ -63,6 +63,7 @@ -type json_binary() :: binary(). -type template() :: binary(). -type template_str() :: string(). +-type binary_kv() :: #{binary() => binary()}. -typerefl_from_string({duration/0, emqx_schema, to_duration}). -typerefl_from_string({duration_s/0, emqx_schema, to_duration_s}). @@ -167,7 +168,8 @@ json_binary/0, port_number/0, template/0, - template_str/0 + template_str/0, + binary_kv/0 ]). -export([namespace/0, roots/0, roots/1, fields/1, desc/1, tags/0]). diff --git a/apps/emqx_auth_jwt/src/emqx_authn_jwks_client.erl b/apps/emqx_auth_jwt/src/emqx_authn_jwks_client.erl index dc03cafef..f574a421f 100644 --- a/apps/emqx_auth_jwt/src/emqx_authn_jwks_client.erl +++ b/apps/emqx_auth_jwt/src/emqx_authn_jwks_client.erl @@ -133,11 +133,13 @@ code_change(_OldVsn, State, _Extra) -> handle_options(#{ endpoint := Endpoint, + headers := Headers, refresh_interval := RefreshInterval0, ssl_opts := SSLOpts }) -> #{ endpoint => Endpoint, + headers => to_httpc_headers(Headers), refresh_interval => limit_refresh_interval(RefreshInterval0), ssl_opts => maps:to_list(SSLOpts), jwks => [], @@ -147,6 +149,7 @@ handle_options(#{ refresh_jwks( #{ endpoint := Endpoint, + headers := Headers, ssl_opts := SSLOpts } = State ) -> @@ -159,7 +162,7 @@ refresh_jwks( case httpc:request( get, - {Endpoint, [{"Accept", "application/json"}]}, + {Endpoint, Headers}, HTTPOpts, [{body_format, binary}, {sync, false}, {receiver, self()}] ) @@ -185,6 +188,9 @@ limit_refresh_interval(Interval) when Interval < 10 -> limit_refresh_interval(Interval) -> Interval. +to_httpc_headers(Headers) -> + [{binary_to_list(bin(K)), V} || {K, V} <- maps:to_list(Headers)]. + cancel_http_request(#{request_id := undefined} = State) -> State; cancel_http_request(#{request_id := RequestID} = State) -> @@ -195,3 +201,10 @@ cancel_http_request(#{request_id := RequestID} = State) -> ok end, State#{request_id => undefined}. + +bin(List) when is_list(List) -> + unicode:characters_to_binary(List, utf8); +bin(Atom) when is_atom(Atom) -> + erlang:atom_to_binary(Atom); +bin(Bin) when is_binary(Bin) -> + Bin. diff --git a/apps/emqx_auth_jwt/src/emqx_authn_jwt_schema.erl b/apps/emqx_auth_jwt/src/emqx_authn_jwt_schema.erl index aff0f12c7..2c49121f9 100644 --- a/apps/emqx_auth_jwt/src/emqx_authn_jwt_schema.erl +++ b/apps/emqx_auth_jwt/src/emqx_authn_jwt_schema.erl @@ -95,6 +95,15 @@ fields(jwt_jwks) -> [ {use_jwks, sc(hoconsc:enum([true]), #{required => true, desc => ?DESC(use_jwks)})}, {endpoint, fun endpoint/1}, + {headers, + sc( + typerefl:alias("map", emqx_schema:binary_kv()), + #{ + default => #{<<"Accept">> => <<"application/json">>}, + validator => fun validate_headers/1, + desc => ?DESC("jwks_headers") + } + )}, {pool_size, fun emqx_connector_schema_lib:pool_size/1}, {refresh_interval, fun refresh_interval/1}, {ssl, #{ @@ -225,3 +234,26 @@ to_binary(B) when is_binary(B) -> B. sc(Type, Meta) -> hoconsc:mk(Type, Meta). + +validate_headers(undefined) -> + ok; +validate_headers(Headers) -> + BadKeys0 = + lists:filter( + fun(K) -> + re:run(K, <<"[^-0-9a-zA-Z_ ]">>, [{capture, none}]) =:= match + end, + maps:keys(Headers) + ), + case BadKeys0 of + [] -> + ok; + _ -> + BadKeys = lists:join(", ", BadKeys0), + Msg0 = io_lib:format( + "headers should contain only characters matching [-0-9a-zA-Z_ ]; bad headers: ~s", + [BadKeys] + ), + Msg = iolist_to_binary(Msg0), + {error, Msg} + end. diff --git a/apps/emqx_auth_jwt/test/emqx_authn_jwt_SUITE.erl b/apps/emqx_auth_jwt/test/emqx_authn_jwt_SUITE.erl index 8bf0cc68a..ef6efcdf2 100644 --- a/apps/emqx_auth_jwt/test/emqx_authn_jwt_SUITE.erl +++ b/apps/emqx_auth_jwt/test/emqx_authn_jwt_SUITE.erl @@ -22,18 +22,21 @@ -include_lib("common_test/include/ct.hrl"). -include_lib("eunit/include/eunit.hrl"). -include_lib("snabbkaffe/include/snabbkaffe.hrl"). +-include_lib("emqx/include/asserts.hrl"). -define(AUTHN_ID, <<"mechanism:jwt">>). -define(JWKS_PORT, 31333). -define(JWKS_PATH, "/jwks.json"). +-import(emqx_common_test_helpers, [on_exit/1]). + all() -> emqx_common_test_helpers:all(?MODULE). init_per_suite(Config) -> Apps = emqx_cth_suite:start([emqx, emqx_conf, emqx_auth, emqx_auth_jwt], #{ - work_dir => ?config(priv_dir, Config) + work_dir => emqx_cth_suite:work_dir(Config) }), [{apps, Apps} | Config]. @@ -41,6 +44,10 @@ end_per_suite(Config) -> ok = emqx_cth_suite:stop(?config(apps, Config)), ok. +end_per_testcase(_TestCase, _Config) -> + emqx_common_test_helpers:call_janitor(), + ok. + %%------------------------------------------------------------------------------ %% Tests %%------------------------------------------------------------------------------ @@ -244,6 +251,7 @@ t_jwks_renewal(_Config) -> disconnect_after_expire => false, use_jwks => true, endpoint => "https://127.0.0.1:" ++ integer_to_list(?JWKS_PORT + 1) ++ ?JWKS_PATH, + headers => #{<<"Accept">> => <<"application/json">>}, refresh_interval => 1000, pool_size => 1 }, @@ -328,6 +336,102 @@ t_jwks_renewal(_Config) -> ?assertEqual(ok, emqx_authn_jwt:destroy(State2)), ok = emqx_authn_http_test_server:stop(). +t_jwks_custom_headers(_Config) -> + {ok, _} = emqx_authn_http_test_server:start_link(?JWKS_PORT, ?JWKS_PATH, server_ssl_opts()), + on_exit(fun() -> ok = emqx_authn_http_test_server:stop() end), + ok = emqx_authn_http_test_server:set_handler(jwks_handler_spy()), + + PrivateKey = test_rsa_key(private), + Payload = #{ + <<"username">> => <<"myuser">>, + <<"foo">> => <<"myuser">>, + <<"exp">> => erlang:system_time(second) + 10 + }, + Endpoint = iolist_to_binary("https://127.0.0.1:" ++ integer_to_list(?JWKS_PORT) ++ ?JWKS_PATH), + Config0 = #{ + <<"mechanism">> => <<"jwt">>, + <<"use_jwks">> => true, + <<"from">> => <<"password">>, + <<"endpoint">> => Endpoint, + <<"headers">> => #{ + <<"Accept">> => <<"application/json">>, + <<"Content-Type">> => <<>>, + <<"foo">> => <<"bar">> + }, + <<"pool_size">> => 1, + <<"refresh_interval">> => 1_000, + <<"ssl">> => #{ + <<"keyfile">> => cert_file("client.key"), + <<"certfile">> => cert_file("client.crt"), + <<"cacertfile">> => cert_file("ca.crt"), + <<"enable">> => true, + <<"verify">> => <<"verify_peer">>, + <<"server_name_indication">> => <<"authn-server">> + }, + <<"verify_claims">> => #{<<"foo">> => <<"${username}">>} + }, + {ok, Config} = hocon:binary(hocon_pp:do(Config0, #{})), + ChainName = 'mqtt:global', + AuthenticatorId = <<"jwt">>, + ?check_trace( + #{timetrap => 10_000}, + begin + %% bad header keys + BadConfig1 = emqx_utils_maps:deep_put( + [<<"headers">>, <<"ça-va"/utf8>>], Config, <<"bien">> + ), + ?assertMatch( + {error, #{ + kind := validation_error, + reason := <<"headers should contain only characters matching ", _/binary>> + }}, + emqx_authn_api:update_config( + [authentication], + {create_authenticator, ChainName, BadConfig1} + ) + ), + BadConfig2 = emqx_utils_maps:deep_put( + [<<"headers">>, <<"test_哈哈"/utf8>>], + Config, + <<"test_haha">> + ), + ?assertMatch( + {error, #{ + kind := validation_error, + reason := <<"headers should contain only characters matching ", _/binary>> + }}, + emqx_authn_api:update_config( + [authentication], + {create_authenticator, ChainName, BadConfig2} + ) + ), + {{ok, _}, {ok, _}} = + ?wait_async_action( + emqx_authn_api:update_config( + [authentication], + {create_authenticator, ChainName, Config} + ), + #{?snk_kind := jwks_endpoint_response}, + 5_000 + ), + ?assertReceive( + {http_request, #{ + headers := #{ + <<"accept">> := <<"application/json">>, + <<"foo">> := <<"bar">> + } + }} + ), + {ok, _} = emqx_authn_api:update_config( + [authentication], + {delete_authenticator, ChainName, AuthenticatorId} + ), + ok + end, + [] + ), + ok. + t_verify_claims(_) -> Secret = <<"abcdef">>, Config0 = #{ @@ -469,6 +573,16 @@ jwks_handler(Req0, State) -> ), {ok, Req, State}. +jwks_handler_spy() -> + TestPid = self(), + fun(Req, State) -> + ReqHeaders = cowboy_req:headers(Req), + ReqMap = #{headers => ReqHeaders}, + ct:pal("jwks request:\n ~p", [ReqMap]), + TestPid ! {http_request, ReqMap}, + jwks_handler(Req, State) + end. + test_rsa_key(public) -> data_file("public_key.pem"); test_rsa_key(private) -> diff --git a/changes/ce/feat-13436.en.md b/changes/ce/feat-13436.en.md new file mode 100644 index 000000000..cda52a137 --- /dev/null +++ b/changes/ce/feat-13436.en.md @@ -0,0 +1 @@ +Added the option to add custom request headers to JWKS requests. diff --git a/rel/i18n/emqx_authn_jwt_schema.hocon b/rel/i18n/emqx_authn_jwt_schema.hocon index a7a0aad09..aadab1b68 100644 --- a/rel/i18n/emqx_authn_jwt_schema.hocon +++ b/rel/i18n/emqx_authn_jwt_schema.hocon @@ -145,4 +145,9 @@ disconnect_after_expire.desc: disconnect_after_expire.label: """Disconnect After Expire""" +jwks_headers.label: +"""HTTP Headers""" +jwks_headers.desc: +"""List of HTTP headers to send with the JWKS request.""" + } From 9f8a1885a7450c1c9d208ca7aa1d7704b3c137e1 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Tue, 9 Jul 2024 14:13:46 -0300 Subject: [PATCH 33/41] fix(resource manager): disentangle connector and channel health check frequencies Fixes https://emqx.atlassian.net/browse/EMQX-12674 --- .../src/emqx_resource_manager.erl | 182 ++++++++++++------ changes/ce/fix-13442.en.md | 1 + 2 files changed, 123 insertions(+), 60 deletions(-) create mode 100644 changes/ce/fix-13442.en.md diff --git a/apps/emqx_resource/src/emqx_resource_manager.erl b/apps/emqx_resource/src/emqx_resource_manager.erl index c3b746d8e..50a25620c 100644 --- a/apps/emqx_resource/src/emqx_resource_manager.erl +++ b/apps/emqx_resource/src/emqx_resource_manager.erl @@ -143,6 +143,15 @@ perform_health_check => boolean() }. +%% calls/casts/generic timeouts +-record(add_channel, {channel_id :: channel_id(), config :: map()}). +-record(start_channel_health_check, {channel_id :: channel_id()}). + +-type generic_timeout(Id, Content) :: {{timeout, Id}, timeout(), Content}. +-type start_channel_health_check_action() :: generic_timeout( + #start_channel_health_check{}, #start_channel_health_check{} +). + %%------------------------------------------------------------------------------ %% API %%------------------------------------------------------------------------------ @@ -405,7 +414,7 @@ add_channel(ResId, ChannelId, Config) -> ) -> ok | {error, term()}. add_channel(ResId, ChannelId, Config, Opts) -> - Result = safe_call(ResId, {add_channel, ChannelId, Config}, ?T_OPERATION), + Result = safe_call(ResId, #add_channel{channel_id = ChannelId, config = Config}, ?T_OPERATION), maybe true ?= maps:get(perform_health_check, Opts, true), %% Wait for health_check to finish @@ -570,7 +579,9 @@ handle_event({call, From}, health_check, _State, Data) -> handle_manual_resource_health_check(From, Data); handle_event({call, From}, {channel_health_check, ChannelId}, _State, Data) -> handle_manual_channel_health_check(From, Data, ChannelId); -% State: CONNECTING +%%-------------------------- +%% State: CONNECTING +%%-------------------------- handle_event(enter, _OldState, ?state_connecting = State, Data) -> ok = log_status_consistency(State, Data), {keep_state_and_data, [{state_timeout, 0, health_check}]}; @@ -582,25 +593,39 @@ handle_event( {call, From}, {remove_channel, ChannelId}, ?state_connecting = _State, Data ) -> handle_remove_channel(From, ChannelId, Data); +%%-------------------------- %% State: CONNECTED %% The connected state is entered after a successful on_start/2 of the callback mod %% and successful health_checks +%%-------------------------- handle_event(enter, _OldState, ?state_connected = State, Data) -> ok = log_status_consistency(State, Data), _ = emqx_alarm:safe_deactivate(Data#data.id), ?tp(resource_connected_enter, #{}), - {keep_state_and_data, health_check_actions(Data)}; + {keep_state_and_data, resource_health_check_actions(Data)}; handle_event(state_timeout, health_check, ?state_connected, Data) -> start_resource_health_check(Data); handle_event( - {call, From}, {add_channel, ChannelId, Config}, ?state_connected = _State, Data + {call, From}, + #add_channel{channel_id = ChannelId, config = Config}, + ?state_connected = _State, + Data ) -> handle_add_channel(From, Data, ChannelId, Config); handle_event( {call, From}, {remove_channel, ChannelId}, ?state_connected = _State, Data ) -> handle_remove_channel(From, ChannelId, Data); +handle_event( + {timeout, #start_channel_health_check{channel_id = ChannelId}}, + _, + ?state_connected = _State, + Data +) -> + handle_start_channel_health_check(Data, ChannelId); +%%-------------------------- %% State: DISCONNECTED +%%-------------------------- handle_event(enter, _OldState, ?state_disconnected = State, Data) -> ok = log_status_consistency(State, Data), ?tp(resource_disconnected_enter, #{}), @@ -608,14 +633,18 @@ handle_event(enter, _OldState, ?state_disconnected = State, Data) -> handle_event(state_timeout, auto_retry, ?state_disconnected, Data) -> ?tp(resource_auto_reconnect, #{}), start_resource(Data, undefined); +%%-------------------------- %% State: STOPPED %% The stopped state is entered after the resource has been explicitly stopped +%%-------------------------- handle_event(enter, _OldState, ?state_stopped = State, Data) -> ok = log_status_consistency(State, Data), {keep_state_and_data, []}; +%%-------------------------- %% The following events can be handled in any other state +%%-------------------------- handle_event( - {call, From}, {add_channel, ChannelId, Config}, State, Data + {call, From}, #add_channel{channel_id = ChannelId, config = Config}, State, Data ) -> handle_not_connected_add_channel(From, ChannelId, Config, State, Data); handle_event( @@ -645,6 +674,9 @@ handle_event( is_map_key(Pid, CHCWorkers) -> handle_channel_health_check_worker_down(Data0, Pid, Res); +handle_event({timeout, #start_channel_health_check{channel_id = _}}, _, _State, _Data) -> + %% Stale health check action; currently, we only probe channel health when connected. + keep_state_and_data; % Ignore all other events handle_event(EventType, EventData, State, Data) -> ?SLOG( @@ -702,7 +734,7 @@ retry_actions(Data) -> [{state_timeout, RetryInterval, auto_retry}] end. -health_check_actions(Data) -> +resource_health_check_actions(Data) -> [{state_timeout, health_check_interval(Data#data.opts), health_check}]. handle_remove_event(From, ClearMetrics, Data) -> @@ -1079,7 +1111,7 @@ continue_resource_health_check_connected(NewStatus, Data0) -> {Replies, Data1} = reply_pending_resource_health_check_callers(NewStatus, Data0), Data2 = channels_health_check(?status_connected, Data1), Data = update_state(Data2, Data0), - Actions = Replies ++ health_check_actions(Data), + Actions = Replies ++ resource_health_check_actions(Data), {keep_state, Data, Actions}; _ -> ?SLOG(warning, #{ @@ -1091,23 +1123,28 @@ continue_resource_health_check_connected(NewStatus, Data0) -> %% subset of resource manager state... But there should be a conversion %% between the two here, as resource manager also has `stopped', which is %% not a valid status at the time of writing. - {Replies, Data} = reply_pending_resource_health_check_callers(NewStatus, Data0), - {next_state, NewStatus, channels_health_check(NewStatus, Data), Replies} + {Replies, Data1} = reply_pending_resource_health_check_callers(NewStatus, Data0), + Data = channels_health_check(NewStatus, Data1), + Actions = Replies, + {next_state, NewStatus, Data, Actions} end. %% Continuation to be used when the current resource state is not `?state_connected'. continue_resource_health_check_not_connected(NewStatus, Data0) -> - {Replies, Data} = reply_pending_resource_health_check_callers(NewStatus, Data0), + {Replies, Data1} = reply_pending_resource_health_check_callers(NewStatus, Data0), case NewStatus of ?status_connected -> - {next_state, ?state_connected, channels_health_check(?status_connected, Data), Replies}; + Data = channels_health_check(?status_connected, Data1), + Actions = Replies, + {next_state, ?state_connected, Data, Actions}; ?status_connecting -> - Actions = Replies ++ health_check_actions(Data), - {next_state, ?status_connecting, channels_health_check(?status_connecting, Data), - Actions}; + Data = channels_health_check(?status_connecting, Data1), + Actions = Replies ++ resource_health_check_actions(Data), + {next_state, ?status_connecting, Data, Actions}; ?status_disconnected -> - {next_state, ?state_disconnected, channels_health_check(?status_disconnected, Data), - Replies} + Data = channels_health_check(?status_disconnected, Data1), + Actions = Replies, + {next_state, ?state_disconnected, Data, Actions} end. handle_manual_channel_health_check(From, #data{state = undefined}, _ChannelId) -> @@ -1269,38 +1306,60 @@ resource_not_connected_channel_error_msg(ResourceStatus, ChannelId, Data1) -> ) ). +-spec generic_timeout_action(Id, timeout(), Content) -> generic_timeout(Id, Content). +generic_timeout_action(Id, Timeout, Content) -> + {{timeout, Id}, Timeout, Content}. + +-spec start_channel_health_check_action(channel_id(), map(), map(), data() | timeout()) -> + [start_channel_health_check_action()]. +start_channel_health_check_action(ChannelId, NewChanStatus, PreviousChanStatus, Data = #data{}) -> + Timeout = get_channel_health_check_interval(ChannelId, NewChanStatus, PreviousChanStatus, Data), + Event = #start_channel_health_check{channel_id = ChannelId}, + [generic_timeout_action(Event, Timeout, Event)]. + +get_channel_health_check_interval(ChannelId, NewChanStatus, PreviousChanStatus, Data) -> + emqx_utils:foldl_while( + fun + (#{config := #{resource_opts := #{health_check_interval := HCInterval}}}, _Acc) -> + {halt, HCInterval}; + (_, Acc) -> + {cont, Acc} + end, + ?HEALTHCHECK_INTERVAL, + [ + NewChanStatus, + PreviousChanStatus, + maps:get(ChannelId, Data#data.added_channels, #{}) + ] + ). + %% Currently, we only call resource channel health checks when the underlying resource is %% `?status_connected'. -spec trigger_health_check_for_added_channels(data()) -> data(). trigger_health_check_for_added_channels(Data0 = #data{hc_workers = HCWorkers0}) -> #{ - channel := CHCWorkers0 = + channel := #{ - pending := CPending0, + %% TODO: rm pending + %% pending := CPending0, ongoing := Ongoing0 } } = HCWorkers0, NewOngoing = maps:filter( fun(ChannelId, OldStatus) -> - not is_map_key(ChannelId, Ongoing0) and + (not is_map_key(ChannelId, Ongoing0)) andalso channel_status_is_channel_added(OldStatus) end, Data0#data.added_channels ), ChannelsToCheck = maps:keys(NewOngoing), - case ChannelsToCheck of - [] -> - %% Nothing to do. - Data0; - [ChannelId | Rest] -> - %% Shooting one check at a time. We could increase concurrency in the future. - CHCWorkers = CHCWorkers0#{ - pending := CPending0 ++ Rest, - ongoing := maps:merge(Ongoing0, NewOngoing) - }, - Data1 = Data0#data{hc_workers = HCWorkers0#{channel := CHCWorkers}}, - start_channel_health_check(Data1, ChannelId) - end. + lists:foldl( + fun(ChannelId, Acc) -> + start_channel_health_check(Acc, ChannelId) + end, + Data0, + ChannelsToCheck + ). -spec continue_channel_health_check_connected( channel_id(), channel_status_map(), channel_status_map(), data() @@ -1338,12 +1397,29 @@ continue_channel_health_check_connected_no_update_during_check(ChannelId, OldSta end, Data. +-spec handle_start_channel_health_check(data(), channel_id()) -> + gen_statem:event_handler_result(state(), data()). +handle_start_channel_health_check(Data0, ChannelId) -> + Data = start_channel_health_check(Data0, ChannelId), + {keep_state, Data}. + -spec start_channel_health_check(data(), channel_id()) -> data(). -start_channel_health_check(#data{} = Data0, ChannelId) -> +start_channel_health_check( + #data{added_channels = AddedChannels, hc_workers = #{channel := #{ongoing := CHCOngoing0}}} = + Data0, + ChannelId +) when + is_map_key(ChannelId, AddedChannels) andalso (not is_map_key(ChannelId, CHCOngoing0)) +-> #data{hc_workers = HCWorkers0 = #{channel := CHCWorkers0}} = Data0, WorkerPid = spawn_channel_health_check_worker(Data0, ChannelId), - HCWorkers = HCWorkers0#{channel := CHCWorkers0#{WorkerPid => ChannelId}}, - Data0#data{hc_workers = HCWorkers}. + ChannelStatus = maps:get(ChannelId, AddedChannels), + CHCOngoing = CHCOngoing0#{ChannelId => ChannelStatus}, + CHCWorkers = CHCWorkers0#{WorkerPid => ChannelId, ongoing := CHCOngoing}, + HCWorkers = HCWorkers0#{channel := CHCWorkers}, + Data0#data{hc_workers = HCWorkers}; +start_channel_health_check(Data, _ChannelId) -> + Data. -spec spawn_channel_health_check_worker(data(), channel_id()) -> pid(). spawn_channel_health_check_worker(#data{} = Data, ChannelId) -> @@ -1380,33 +1456,19 @@ handle_channel_health_check_worker_down(Data0, WorkerRef, ExitResult) -> #{ongoing := Ongoing0} = CHCWorkers1, {PreviousChanStatus, Ongoing1} = maps:take(ChannelId, Ongoing0), CHCWorkers2 = CHCWorkers1#{ongoing := Ongoing1}, - CHCWorkers3 = emqx_utils_maps:deep_remove([ongoing, ChannelId], CHCWorkers2), Data1 = Data0#data{added_channels = AddedChannels}, {Replies, Data2} = reply_pending_channel_health_check_callers(ChannelId, NewStatus, Data1), - case CHCWorkers1 of - #{pending := [NextChannelId | Rest]} -> - CHCWorkers = CHCWorkers3#{pending := Rest}, - HCWorkers = HCWorkers0#{channel := CHCWorkers}, - Data3 = Data2#data{hc_workers = HCWorkers}, - Data4 = continue_channel_health_check_connected( - ChannelId, - PreviousChanStatus, - CurrentStatus, - Data3 - ), - Data = start_channel_health_check(Data4, NextChannelId), - {keep_state, update_state(Data, Data0), Replies}; - #{pending := []} -> - HCWorkers = HCWorkers0#{channel := CHCWorkers3}, - Data3 = Data2#data{hc_workers = HCWorkers}, - Data = continue_channel_health_check_connected( - ChannelId, - PreviousChanStatus, - CurrentStatus, - Data3 - ), - {keep_state, update_state(Data, Data0), Replies} - end. + HCWorkers = HCWorkers0#{channel := CHCWorkers2}, + Data3 = Data2#data{hc_workers = HCWorkers}, + Data = continue_channel_health_check_connected( + ChannelId, + PreviousChanStatus, + CurrentStatus, + Data3 + ), + CHCActions = start_channel_health_check_action(ChannelId, NewStatus, PreviousChanStatus, Data), + Actions = Replies ++ CHCActions, + {keep_state, update_state(Data, Data0), Actions}. handle_channel_health_check_worker_down_new_channels_and_status( ChannelId, diff --git a/changes/ce/fix-13442.en.md b/changes/ce/fix-13442.en.md new file mode 100644 index 000000000..05aaee8a0 --- /dev/null +++ b/changes/ce/fix-13442.en.md @@ -0,0 +1 @@ +Fixed an issue where the health check interval values of actions/sources were not being taken into account. From b0e3e405cf7b2444871f4c05cab486b71e72a735 Mon Sep 17 00:00:00 2001 From: firest Date: Wed, 10 Jul 2024 15:22:43 +0800 Subject: [PATCH 34/41] fix(oidc): Avoid crashes and avoid deleting jwks on update --- apps/emqx_dashboard_sso/src/emqx_dashboard_sso_oidc.erl | 8 ++++++-- .../src/emqx_dashboard_sso_oidc_session.erl | 7 ++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_oidc.erl b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_oidc.erl index dbc0d7f0b..1d2520d0f 100644 --- a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_oidc.erl +++ b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_oidc.erl @@ -181,12 +181,16 @@ create(#{name_var := NameVar} = Config) -> end. update(Config, State) -> - destroy(State), + destroy(State, false), create(Config). destroy(State) -> + destroy(State, true). + +destroy(State, TryDelete) -> emqx_dashboard_sso_oidc_session:stop(), - try_delete_jwks_file(State). + _ = TryDelete andalso try_delete_jwks_file(State), + ok. -dialyzer({nowarn_function, login/2}). login( diff --git a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_oidc_session.erl b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_oidc_session.erl index b28bcc64d..c5756def9 100644 --- a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_oidc_session.erl +++ b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_oidc_session.erl @@ -35,6 +35,8 @@ -define(DEFAULT_RANDOM_LEN, 32). -define(NOW, erlang:system_time(millisecond)). +-define(BACKOFF_MIN, 5000). +-define(BACKOFF_MAX, 10000). %%------------------------------------------------------------------------------ %% API @@ -49,7 +51,10 @@ start(Name, #{issuer := Issuer, session_expiry := SessionExpiry0}) -> [ #{ issuer => Issuer, - name => {local, Name} + name => {local, Name}, + backoff_min => ?BACKOFF_MIN, + backoff_max => ?BACKOFF_MAX, + backoff_type => random } ] ) From 3004e3247339567dd7ac4c1b591a4fd93999bce7 Mon Sep 17 00:00:00 2001 From: Ivan Dyachkov Date: Thu, 11 Jul 2024 10:37:19 +0200 Subject: [PATCH 35/41] ci: fix dashboard tests (again) --- scripts/ui-tests/dashboard_test.py | 52 ++++++++++++++++++---------- scripts/ui-tests/docker-compose.yaml | 4 +-- 2 files changed, 36 insertions(+), 20 deletions(-) diff --git a/scripts/ui-tests/dashboard_test.py b/scripts/ui-tests/dashboard_test.py index 6005a9403..c349739b6 100644 --- a/scripts/ui-tests/dashboard_test.py +++ b/scripts/ui-tests/dashboard_test.py @@ -3,6 +3,7 @@ import time import unittest import pytest import requests +import logging from urllib.parse import urljoin from selenium import webdriver from selenium.webdriver.common.by import By @@ -12,6 +13,9 @@ from selenium.webdriver.support.wait import WebDriverWait from selenium.webdriver.common import utils from selenium.common.exceptions import NoSuchElementException +logger = logging.getLogger() +logger.setLevel(logging.INFO) + @pytest.fixture def driver(): options = Options() @@ -31,39 +35,52 @@ def dashboard_url(dashboard_host, dashboard_port): time.sleep(1) return f"http://{dashboard_host}:{dashboard_port}" -@pytest.fixture def login(driver, dashboard_url): # admin is set in CI jobs, hence as default value password = os.getenv("EMQX_DASHBOARD__DEFAULT_PASSWORD", "admin") driver.get(dashboard_url) assert "EMQX Dashboard" == driver.title assert f"{dashboard_url}/#/login?to=/dashboard/overview" == driver.current_url - driver.find_element(By.XPATH, "//div[@class='login']//form[1]//input[@type='text']").send_keys("admin") - driver.find_element(By.XPATH, "//div[@class='login']//form[1]//input[@type='password']").send_keys(password) - driver.find_element(By.XPATH, "//div[@class='login']//form[1]//button[1]").click() + driver.execute_script("window.localStorage.setItem('licenseTipVisible','false');") + driver.find_element(By.XPATH, "//div[@class='login']//form//input[@type='text']").send_keys("admin") + driver.find_element(By.XPATH, "//div[@class='login']//form//input[@type='password']").send_keys(password) + driver.find_element(By.XPATH, "//div[@class='login']//form//button").click() dest_url = urljoin(dashboard_url, "/#/dashboard/overview") - driver.get(dest_url) ensure_current_url(driver, dest_url) + assert len(driver.find_elements(By.XPATH, "//div[@class='login']")) == 0 + logger.info(f"Logged in to {dashboard_url}") -def ensure_current_url(driver, url): +def ensure_current_url(d, url): count = 0 - while url != driver.current_url: + while url != d.current_url: if count == 10: raise Exception(f"Failed to load {url}") count += 1 time.sleep(1) -def title(driver): - return driver.find_element("xpath", "//div[@id='app']//h1[@class='header-title']") +def title(d): + title = '' + for _ in range(5): + try: + title = d.find_element("xpath", "//div[@id='app']//h1[@class='header-title']") + break + except NoSuchElementException: + time.sleep(1) + else: + raise AssertionError("Cannot find the title element") + return title -def wait_title_text(driver, text): - return WebDriverWait(driver, 10).until(lambda x: title(x).text == text) +def wait_title_text(d, text): + return WebDriverWait(d, 10).until(lambda x: title(x).text == text) -def test_basic(driver, login, dashboard_url): - driver.get(dashboard_url) +def test_basic(driver, dashboard_url): + login(driver, dashboard_url) + logger.info(f"Current URL: {driver.current_url}") wait_title_text(driver, "Cluster Overview") -def test_log(driver, login, dashboard_url): +def test_log(driver, dashboard_url): + login(driver, dashboard_url) + logger.info(f"Current URL: {driver.current_url}") dest_url = urljoin(dashboard_url, "/#/log") driver.get(dest_url) ensure_current_url(driver, dest_url) @@ -95,10 +112,9 @@ def fetch_version(url): version_str = info['rel_vsn'] return parse_version(version_str) -def test_docs_link(driver, login, dashboard_url): - dest_url = urljoin(dashboard_url, "/#/dashboard/overview") - driver.get(dest_url) - ensure_current_url(driver, dest_url) +def test_docs_link(driver, dashboard_url): + login(driver, dashboard_url) + logger.info(f"Current URL: {driver.current_url}") xpath_link_help = "//div[@id='app']//div[@class='nav-header']//a[contains(@class, 'link-help')]" # retry up to 5 times for _ in range(5): diff --git a/scripts/ui-tests/docker-compose.yaml b/scripts/ui-tests/docker-compose.yaml index 952b0f382..ce87613d2 100644 --- a/scripts/ui-tests/docker-compose.yaml +++ b/scripts/ui-tests/docker-compose.yaml @@ -1,14 +1,14 @@ -version: '3.9' - services: emqx: image: ${_EMQX_DOCKER_IMAGE_TAG:-emqx/emqx:latest} environment: EMQX_DASHBOARD__DEFAULT_PASSWORD: admin + EMQX_LOG__CONSOLE__LEVEL: debug selenium: shm_size: '2gb' image: ghcr.io/emqx/selenium-chrome:1.0.0 + platform: linux/amd64 volumes: - ./:/app depends_on: From d9b5c5863bcd08cf537b04488e7d60e5f9a990c0 Mon Sep 17 00:00:00 2001 From: firest Date: Thu, 11 Jul 2024 18:12:38 +0800 Subject: [PATCH 36/41] fix: do not convert a empty file name to a empty list --- apps/emqx/src/emqx_schema.erl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index d639523bb..379696c05 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/apps/emqx/src/emqx_schema.erl @@ -3355,6 +3355,8 @@ default_listener(SSLListener) -> %% otherwise always return string. naive_env_interpolation(undefined) -> undefined; +naive_env_interpolation(<<>>) -> + <<>>; naive_env_interpolation(Bin) when is_binary(Bin) -> naive_env_interpolation(unicode:characters_to_list(Bin, utf8)); naive_env_interpolation("$" ++ Maybe = Original) -> From 04b547d6f5356c169c0ef1c4b23bc32168ec4ab7 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Thu, 11 Jul 2024 14:35:31 -0300 Subject: [PATCH 37/41] fix(schema registry): handle large names during lookup Fixes https://emqx.atlassian.net/browse/EMQX-12692 --- .../src/emqx_schema_registry.erl | 30 +++++++++++++------ .../emqx_schema_registry_http_api_SUITE.erl | 10 ++++++- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/apps/emqx_schema_registry/src/emqx_schema_registry.erl b/apps/emqx_schema_registry/src/emqx_schema_registry.erl index 6d894ab90..f8d760ddc 100644 --- a/apps/emqx_schema_registry/src/emqx_schema_registry.erl +++ b/apps/emqx_schema_registry/src/emqx_schema_registry.erl @@ -44,6 +44,12 @@ get_serde/1 ]). +%%------------------------------------------------------------------------------------------------- +%% Type definitions +%%------------------------------------------------------------------------------------------------- + +-define(BAD_SCHEMA_NAME, <<"bad_schema_name">>). + -type schema() :: #{ type := serde_type(), source := binary(), @@ -87,6 +93,8 @@ get_schema(SchemaName) -> Config -> {ok, Config} catch + throw:#{reason := ?BAD_SCHEMA_NAME} -> + {error, not_found}; throw:not_found -> {error, not_found} end. @@ -343,16 +351,20 @@ to_bin(A) when is_atom(A) -> atom_to_binary(A); to_bin(B) when is_binary(B) -> B. schema_name_bin_to_atom(Bin) when size(Bin) > 255 -> - throw( - iolist_to_binary( - io_lib:format( - "Name is is too long." - " Please provide a shorter name (<= 255 bytes)." - " The name that is too long: \"~s\"", - [Bin] - ) + Msg = iolist_to_binary( + io_lib:format( + "Name is is too long." + " Please provide a shorter name (<= 255 bytes)." + " The name that is too long: \"~s\"", + [Bin] ) - ); + ), + Reason = #{ + kind => validation_error, + reason => ?BAD_SCHEMA_NAME, + hint => Msg + }, + throw(Reason); schema_name_bin_to_atom(Bin) -> try binary_to_existing_atom(Bin, utf8) diff --git a/apps/emqx_schema_registry/test/emqx_schema_registry_http_api_SUITE.erl b/apps/emqx_schema_registry/test/emqx_schema_registry_http_api_SUITE.erl index 2ab05a7e8..4cfb2fa46 100644 --- a/apps/emqx_schema_registry/test/emqx_schema_registry_http_api_SUITE.erl +++ b/apps/emqx_schema_registry/test/emqx_schema_registry_http_api_SUITE.erl @@ -347,7 +347,8 @@ t_crud(Config) -> ok. -%% Tests that we can't create names that are too long and get a decent error message. +%% Tests that we can't create or lookup names that are too long and get a decent error +%% message. t_name_too_long(Config) -> SerdeType = ?config(serde_type, Config), SourceBin = ?config(schema_source, Config), @@ -368,4 +369,11 @@ t_name_too_long(Config) -> }}, request({post, Params}) ), + ?assertMatch( + {ok, 404, #{ + <<"code">> := <<"NOT_FOUND">>, + <<"message">> := <<"Schema not found">> + }}, + request({get, SchemaName}) + ), ok. From 83cc3ffeb0a570f2a8fd101f9501fad6c7e0415f Mon Sep 17 00:00:00 2001 From: firest Date: Fri, 12 Jul 2024 13:58:14 +0800 Subject: [PATCH 38/41] fix(banned): let the bootfile of banned be optional --- apps/emqx/src/emqx_banned.erl | 6 ++++-- apps/emqx/src/emqx_schema.erl | 4 +--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/apps/emqx/src/emqx_banned.erl b/apps/emqx/src/emqx_banned.erl index 5e3545b93..4d7a37869 100644 --- a/apps/emqx/src/emqx_banned.erl +++ b/apps/emqx/src/emqx_banned.erl @@ -240,7 +240,7 @@ who(peerhost_net, CIDR) when is_binary(CIDR) -> %%-------------------------------------------------------------------- %% Import From CSV %%-------------------------------------------------------------------- -init_from_csv(<<>>) -> +init_from_csv(undefined) -> ok; init_from_csv(File) -> maybe @@ -365,7 +365,9 @@ init([]) -> {ok, ensure_expiry_timer(#{expiry_timer => undefined}), {continue, init_from_csv}}. handle_continue(init_from_csv, State) -> - File = emqx_schema:naive_env_interpolation(emqx:get_config([banned, bootstrap_file], <<>>)), + File = emqx_schema:naive_env_interpolation( + emqx:get_config([banned, bootstrap_file], undefined) + ), _ = init_from_csv(File), {noreply, State}. diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index 379696c05..0204b7de4 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/apps/emqx/src/emqx_schema.erl @@ -1777,7 +1777,7 @@ fields("banned") -> binary(), #{ desc => ?DESC("banned_bootstrap_file"), - default => <<>> + require => false } )} ]. @@ -3355,8 +3355,6 @@ default_listener(SSLListener) -> %% otherwise always return string. naive_env_interpolation(undefined) -> undefined; -naive_env_interpolation(<<>>) -> - <<>>; naive_env_interpolation(Bin) when is_binary(Bin) -> naive_env_interpolation(unicode:characters_to_list(Bin, utf8)); naive_env_interpolation("$" ++ Maybe = Original) -> From 854754eb6038d9ff063ee087e9354614d99e1f61 Mon Sep 17 00:00:00 2001 From: firest Date: Tue, 9 Jul 2024 17:15:42 +0800 Subject: [PATCH 39/41] feat(coap): use content-sensitive udp proxy for coap --- apps/emqx/rebar.config | 2 +- .../src/bhvrs/emqx_gateway_conn.erl | 52 ++++++++++++++-- apps/emqx_gateway/src/emqx_gateway.app.src | 2 +- apps/emqx_gateway/src/emqx_gateway_schema.erl | 19 +++++- apps/emqx_gateway/src/emqx_gateway_utils.erl | 10 +++- .../test/emqx_gateway_authn_SUITE.erl | 1 + .../src/emqx_coap_channel.erl | 46 +-------------- .../src/emqx_coap_proxy_conn.erl | 59 +++++++++++++++++++ .../src/emqx_gateway_coap.erl | 17 +++++- .../test/emqx_coap_SUITE.erl | 31 ++++++---- mix.exs | 2 +- rebar.config | 2 +- rel/i18n/emqx_gateway_schema.hocon | 9 +++ 13 files changed, 181 insertions(+), 71 deletions(-) create mode 100644 apps/emqx_gateway_coap/src/emqx_coap_proxy_conn.erl diff --git a/apps/emqx/rebar.config b/apps/emqx/rebar.config index 0816b013e..3c6a0c516 100644 --- a/apps/emqx/rebar.config +++ b/apps/emqx/rebar.config @@ -27,7 +27,7 @@ {lc, {git, "https://github.com/emqx/lc.git", {tag, "0.3.2"}}}, {gproc, {git, "https://github.com/emqx/gproc", {tag, "0.9.0.1"}}}, {cowboy, {git, "https://github.com/emqx/cowboy", {tag, "2.9.2"}}}, - {esockd, {git, "https://github.com/emqx/esockd", {tag, "5.11.2"}}}, + {esockd, {git, "https://github.com/emqx/esockd", {tag, "5.11.3"}}}, {ekka, {git, "https://github.com/emqx/ekka", {tag, "0.19.5"}}}, {gen_rpc, {git, "https://github.com/emqx/gen_rpc", {tag, "3.3.1"}}}, {hocon, {git, "https://github.com/emqx/hocon.git", {tag, "0.43.1"}}}, diff --git a/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl b/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl index 710148b94..e2aeef6fc 100644 --- a/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl +++ b/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl @@ -57,7 +57,7 @@ -record(state, { %% TCP/SSL/UDP/DTLS Wrapped Socket - socket :: {esockd_transport, esockd:socket()} | {udp, _, _}, + socket :: {esockd_transport, esockd:socket()} | {udp, _, _} | {esockd_udp_proxy, _, _}, %% Peername of the connection peername :: emqx_types:peername(), %% Sockname of the connection @@ -122,6 +122,9 @@ start_link(Socket = {udp, _SockPid, _Sock}, Peername, Options) -> start_link(esockd_transport, Sock, Options) -> Socket = {esockd_transport, Sock}, Args = [self(), Socket, undefined, Options] ++ callback_modules(Options), + {ok, proc_lib:spawn_link(?MODULE, init, Args)}; +start_link(Socket = {esockd_udp_proxy, _ProxyId, _Sock}, Peername, Options) -> + Args = [self(), Socket, Peername, Options] ++ callback_modules(Options), {ok, proc_lib:spawn_link(?MODULE, init, Args)}. callback_modules(Options) -> @@ -196,10 +199,14 @@ esockd_peername({udp, _SockPid, _Sock}, Peername) -> Peername; esockd_peername({esockd_transport, Sock}, _Peername) -> {ok, Peername} = esockd_transport:ensure_ok_or_exit(peername, [Sock]), + Peername; +esockd_peername({esockd_udp_proxy, _ProxyId, _Sock}, Peername) -> Peername. esockd_wait(Socket = {udp, _SockPid, _Sock}) -> {ok, Socket}; +esockd_wait(Socket = {esockd_udp_proxy, _ProxyId, _Sock}) -> + {ok, Socket}; esockd_wait({esockd_transport, Sock}) -> case esockd_transport:wait(Sock) of {ok, NSock} -> {ok, {esockd_transport, NSock}}; @@ -211,29 +218,41 @@ esockd_close({udp, _SockPid, _Sock}) -> %%gen_udp:close(Sock); ok; esockd_close({esockd_transport, Sock}) -> - esockd_transport:fast_close(Sock). + esockd_transport:fast_close(Sock); +esockd_close({esockd_udp_proxy, ProxyId, _Sock}) -> + esockd_udp_proxy:close(ProxyId). esockd_ensure_ok_or_exit(peercert, {udp, _SockPid, _Sock}) -> nossl; esockd_ensure_ok_or_exit(Fun, {udp, _SockPid, Sock}) -> esockd_transport:ensure_ok_or_exit(Fun, [Sock]); esockd_ensure_ok_or_exit(Fun, {esockd_transport, Socket}) -> - esockd_transport:ensure_ok_or_exit(Fun, [Socket]). + esockd_transport:ensure_ok_or_exit(Fun, [Socket]); +esockd_ensure_ok_or_exit(Fun, {esockd_udp_proxy, _ProxyId, Sock}) -> + esockd_transport:ensure_ok_or_exit(Fun, [Sock]). esockd_type({udp, _, _}) -> udp; esockd_type({esockd_transport, Socket}) -> - esockd_transport:type(Socket). + esockd_transport:type(Socket); +esockd_type({esockd_udp_proxy, _ProxyId, Sock}) when is_port(Sock) -> + udp; +esockd_type({esockd_udp_proxy, _ProxyId, _Sock}) -> + ssl. esockd_setopts({udp, _, _}, _) -> ok; esockd_setopts({esockd_transport, Socket}, Opts) -> %% FIXME: DTLS works?? + esockd_transport:setopts(Socket, Opts); +esockd_setopts({esockd_udp_proxy, _ProxyId, Socket}, Opts) -> esockd_transport:setopts(Socket, Opts). esockd_getstat({udp, _SockPid, Sock}, Stats) -> inet:getstat(Sock, Stats); esockd_getstat({esockd_transport, Sock}, Stats) -> + esockd_transport:getstat(Sock, Stats); +esockd_getstat({esockd_udp_proxy, _ProxyId, Sock}, Stats) -> esockd_transport:getstat(Sock, Stats). esockd_send(Data, #state{ @@ -242,7 +261,9 @@ esockd_send(Data, #state{ }) -> gen_udp:send(Sock, Ip, Port, Data); esockd_send(Data, #state{socket = {esockd_transport, Sock}}) -> - esockd_transport:send(Sock, Data). + esockd_transport:send(Sock, Data); +esockd_send(Data, #state{socket = {esockd_udp_proxy, ProxyId, _Sock}}) -> + esockd_udp_proxy:send(ProxyId, Data). keepalive_stats(recv) -> emqx_pd:get_counter(recv_pkt); @@ -250,7 +271,8 @@ keepalive_stats(send) -> emqx_pd:get_counter(send_pkt). is_datadram_socket({esockd_transport, _}) -> false; -is_datadram_socket({udp, _, _}) -> true. +is_datadram_socket({udp, _, _}) -> true; +is_datadram_socket({esockd_udp_proxy, _ProxyId, Sock}) -> erlang:is_port(Sock). %%-------------------------------------------------------------------- %% callbacks @@ -461,6 +483,21 @@ handle_msg({'$gen_cast', Req}, State) -> with_channel(handle_cast, [Req], State); handle_msg({datagram, _SockPid, Data}, State) -> parse_incoming(Data, State); +handle_msg( + {{esockd_udp_proxy, _ProxyId, _Socket} = NSock, Data, Packets}, + State = #state{ + chann_mod = ChannMod, + channel = Channel + } +) -> + ?SLOG(debug, #{msg => "RECV_data", data => Data}), + Oct = iolist_size(Data), + inc_counter(incoming_bytes, Oct), + Ctx = ChannMod:info(ctx, Channel), + ok = emqx_gateway_ctx:metrics_inc(Ctx, 'bytes.received', Oct), + + NState = State#state{socket = NSock}, + {ok, next_incoming_msgs(Packets), NState}; handle_msg({Inet, _Sock, Data}, State) when Inet == tcp; Inet == ssl @@ -508,6 +545,9 @@ handle_msg({inet_reply, _Sock, {error, Reason}}, State) -> handle_msg({close, Reason}, State) -> ?tp(debug, force_socket_close, #{reason => Reason}), handle_info({sock_closed, Reason}, close_socket(State)); +handle_msg(udp_proxy_closed, State) -> + ?tp(debug, udp_proxy_closed, #{reason => normal}), + handle_info({sock_closed, normal}, close_socket(State)); handle_msg( {event, connected}, State = #state{ diff --git a/apps/emqx_gateway/src/emqx_gateway.app.src b/apps/emqx_gateway/src/emqx_gateway.app.src index 25a81801b..6c1a3a8a6 100644 --- a/apps/emqx_gateway/src/emqx_gateway.app.src +++ b/apps/emqx_gateway/src/emqx_gateway.app.src @@ -1,7 +1,7 @@ %% -*- mode: erlang -*- {application, emqx_gateway, [ {description, "The Gateway management application"}, - {vsn, "0.1.33"}, + {vsn, "0.1.34"}, {registered, []}, {mod, {emqx_gateway_app, []}}, {applications, [ diff --git a/apps/emqx_gateway/src/emqx_gateway_schema.erl b/apps/emqx_gateway/src/emqx_gateway_schema.erl index 00555c67a..11488d1a3 100644 --- a/apps/emqx_gateway/src/emqx_gateway_schema.erl +++ b/apps/emqx_gateway/src/emqx_gateway_schema.erl @@ -139,6 +139,16 @@ fields(websocket) -> fields(udp_listener) -> [ %% some special configs for udp listener + {health_check, + sc( + ref(udp_health_check), + #{ + desc => ?DESC( + udp_health_check + ), + required => false + } + )} ] ++ udp_opts() ++ common_listener_opts(); @@ -175,7 +185,12 @@ fields(dtls_opts) -> versions => dtls_all_available }, _IsRanchListener = false - ). + ); +fields(udp_health_check) -> + [ + {request, sc(binary(), #{desc => ?DESC(udp_health_check_request), required => false})}, + {reply, sc(binary(), #{desc => ?DESC(udp_health_check_reply), required => false})} + ]. desc(gateway) -> "EMQX Gateway configuration root."; @@ -201,6 +216,8 @@ desc(dtls_opts) -> "Settings for DTLS protocol."; desc(websocket) -> "Websocket options"; +desc(udp_health_check) -> + "UDP health check"; desc(_) -> undefined. diff --git a/apps/emqx_gateway/src/emqx_gateway_utils.erl b/apps/emqx_gateway/src/emqx_gateway_utils.erl index e6a5be8ab..88a537613 100644 --- a/apps/emqx_gateway/src/emqx_gateway_utils.erl +++ b/apps/emqx_gateway/src/emqx_gateway_utils.erl @@ -151,7 +151,12 @@ find_sup_child(Sup, ChildId) -> {ok, [pid()]} | {error, term()} when - ModCfg :: #{frame_mod := atom(), chann_mod := atom(), connection_mod => atom()}. + ModCfg :: #{ + frame_mod := atom(), + chann_mod := atom(), + connection_mod => atom(), + esockd_proxy_opts => map() + }. start_listeners(Listeners, GwName, Ctx, ModCfg) -> start_listeners(Listeners, GwName, Ctx, ModCfg, []). @@ -519,7 +524,8 @@ esockd_opts(Type, Opts0) when ?IS_ESOCKD_LISTENER(Type) -> max_connections, max_conn_rate, proxy_protocol, - proxy_protocol_timeout + proxy_protocol_timeout, + health_check ], Opts0 ), diff --git a/apps/emqx_gateway/test/emqx_gateway_authn_SUITE.erl b/apps/emqx_gateway/test/emqx_gateway_authn_SUITE.erl index 936856c2e..fb7cc49d3 100644 --- a/apps/emqx_gateway/test/emqx_gateway_authn_SUITE.erl +++ b/apps/emqx_gateway/test/emqx_gateway_authn_SUITE.erl @@ -91,6 +91,7 @@ end_per_suite(Config) -> %%------------------------------------------------------------------------------ t_case_coap(_) -> + emqx_coap_SUITE:restart_coap_with_connection_mode(false), Login = fun(URI, Checker) -> Action = fun(Channel) -> Req = emqx_coap_SUITE:make_req(post), diff --git a/apps/emqx_gateway_coap/src/emqx_coap_channel.erl b/apps/emqx_gateway_coap/src/emqx_coap_channel.erl index 844677d12..a10fa04b2 100644 --- a/apps/emqx_gateway_coap/src/emqx_coap_channel.erl +++ b/apps/emqx_gateway_coap/src/emqx_coap_channel.erl @@ -410,19 +410,6 @@ is_create_connection_request(Msg = #coap_message{method = Method}) when is_create_connection_request(_Msg) -> false. -is_delete_connection_request(Msg = #coap_message{method = Method}) when - is_atom(Method) andalso Method =/= undefined --> - URIPath = emqx_coap_message:get_option(uri_path, Msg, []), - case URIPath of - [<<"mqtt">>, <<"connection">>] when Method == delete -> - true; - _ -> - false - end; -is_delete_connection_request(_Msg) -> - false. - check_token( Msg, #channel{ @@ -430,7 +417,6 @@ check_token( clientinfo = ClientInfo } = Channel ) -> - IsDeleteConn = is_delete_connection_request(Msg), #{clientid := ClientId} = ClientInfo, case emqx_coap_message:extract_uri_query(Msg) of #{ @@ -438,39 +424,10 @@ check_token( <<"token">> := Token } -> call_session(handle_request, Msg, Channel); - #{<<"clientid">> := ReqClientId, <<"token">> := ReqToken} -> - case emqx_gateway_cm:call(coap, ReqClientId, {check_token, ReqToken}) of - undefined when IsDeleteConn -> - Reply = emqx_coap_message:piggyback({ok, deleted}, Msg), - {shutdown, normal, Reply, Channel}; - undefined -> - ?SLOG(info, #{ - msg => "remote_connection_not_found", - clientid => ReqClientId, - token => ReqToken - }), - Reply = emqx_coap_message:reset(Msg), - {shutdown, normal, Reply, Channel}; - false -> - ?SLOG(info, #{ - msg => "request_token_invalid", clientid => ReqClientId, token => ReqToken - }), - Reply = emqx_coap_message:piggyback({error, unauthorized}, Msg), - {shutdown, normal, Reply, Channel}; - true -> - %% hack: since each message request can spawn a new connection - %% process, we can't rely on the `inc_incoming_stats' call in - %% `emqx_gateway_conn:handle_incoming' to properly keep track of - %% bumping incoming requests for an existing channel. Since this - %% number is used by keepalive, we have to bump it inside the - %% requested channel/connection pid so heartbeats actually work. - emqx_gateway_cm:cast(coap, ReqClientId, inc_recv_pkt), - call_session(handle_request, Msg, Channel) - end; _ -> ErrMsg = <<"Missing token or clientid in connection mode">>, Reply = emqx_coap_message:piggyback({error, bad_request}, ErrMsg, Msg), - {shutdown, normal, Reply, Channel} + {ok, {outgoing, Reply}, Channel} end. run_conn_hooks( @@ -785,6 +742,7 @@ process_connection( ) when ConnState == connected -> + %% TODO should take over the session here Queries = emqx_coap_message:extract_uri_query(Req), ErrMsg0 = case Queries of diff --git a/apps/emqx_gateway_coap/src/emqx_coap_proxy_conn.erl b/apps/emqx_gateway_coap/src/emqx_coap_proxy_conn.erl new file mode 100644 index 000000000..703450d7e --- /dev/null +++ b/apps/emqx_gateway_coap/src/emqx_coap_proxy_conn.erl @@ -0,0 +1,59 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2021-2024 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_coap_proxy_conn). + +-behaviour(esockd_udp_proxy_connection). + +-include("emqx_coap.hrl"). + +-export([initialize/1, create/3, get_connection_id/4, dispatch/3, close/2]). + +%%-------------------------------------------------------------------- +%% Callbacks +%%-------------------------------------------------------------------- +initialize(_Opts) -> + emqx_coap_frame:initial_parse_state(#{}). + +create(Transport, Peer, Opts) -> + emqx_gateway_conn:start_link(Transport, Peer, Opts). + +get_connection_id(_Transport, _Peer, State, Data) -> + case parse_incoming(Data, [], State) of + {[Msg | _] = Packets, NState} -> + case emqx_coap_message:extract_uri_query(Msg) of + #{ + <<"clientid">> := ClientId + } -> + {ok, ClientId, Packets, NState}; + _ -> + invalid + end; + _Error -> + invalid + end. + +dispatch(Pid, _State, Packet) -> + erlang:send(Pid, Packet). + +close(Pid, _State) -> + erlang:send(Pid, udp_proxy_closed). + +parse_incoming(<<>>, Packets, State) -> + {Packets, State}; +parse_incoming(Data, Packets, State) -> + {ok, Packet, Rest, NParseState} = emqx_coap_frame:parse(Data, State), + parse_incoming(Rest, [Packet | Packets], NParseState). diff --git a/apps/emqx_gateway_coap/src/emqx_gateway_coap.erl b/apps/emqx_gateway_coap/src/emqx_gateway_coap.erl index c92103bfc..cc18c3351 100644 --- a/apps/emqx_gateway_coap/src/emqx_gateway_coap.erl +++ b/apps/emqx_gateway_coap/src/emqx_gateway_coap.erl @@ -20,7 +20,7 @@ -include_lib("emqx/include/logger.hrl"). -include_lib("emqx_gateway/include/emqx_gateway.hrl"). -%% define a gateway named stomp +%% define a gateway named coap -gateway(#{ name => coap, callback_module => ?MODULE, @@ -58,10 +58,11 @@ on_gateway_load( Ctx ) -> Listeners = normalize_config(Config), - ModCfg = #{ + ModCfg = maps:merge(connection_opts(Config), #{ frame_mod => emqx_coap_frame, chann_mod => emqx_coap_channel - }, + }), + case start_listeners( Listeners, GwName, Ctx, ModCfg @@ -105,3 +106,13 @@ on_gateway_unload( ) -> Listeners = normalize_config(Config), stop_listeners(GwName, Listeners). + +connection_opts(#{connection_required := false}) -> + #{}; +connection_opts(_) -> + #{ + connection_mod => esockd_udp_proxy, + esockd_proxy_opts => #{ + connection_mod => emqx_coap_proxy_conn + } + }. diff --git a/apps/emqx_gateway_coap/test/emqx_coap_SUITE.erl b/apps/emqx_gateway_coap/test/emqx_coap_SUITE.erl index bd403a463..bb244c8d4 100644 --- a/apps/emqx_gateway_coap/test/emqx_coap_SUITE.erl +++ b/apps/emqx_gateway_coap/test/emqx_coap_SUITE.erl @@ -330,7 +330,8 @@ t_publish(_) -> ?assertEqual(Payload, Msg#message.payload) after 500 -> ?assert(false) - end + end, + true end, with_connection(Topics, Action). @@ -360,7 +361,9 @@ t_publish_with_retain_qos_expiry(_) -> ?assertEqual(Payload, Msg#message.payload) after 500 -> ?assert(false) - end + end, + + true end, with_connection(Topics, Action), @@ -392,7 +395,8 @@ t_subscribe(_) -> #coap_content{payload = PayloadRecv} = Notify, - ?assertEqual(Payload, PayloadRecv) + ?assertEqual(Payload, PayloadRecv), + true end, with_connection(Topics, Fun), @@ -431,7 +435,8 @@ t_subscribe_with_qos_opt(_) -> #coap_content{payload = PayloadRecv} = Notify, - ?assertEqual(Payload, PayloadRecv) + ?assertEqual(Payload, PayloadRecv), + true end, with_connection(Topics, Fun), @@ -468,7 +473,8 @@ t_un_subscribe(_) -> {ok, nocontent, _} = do_request(Channel, URI, UnReq), ?LOGT("un observer topic:~ts~n", [Topic]), timer:sleep(100), - ?assertEqual([], emqx:subscribers(Topic)) + ?assertEqual([], emqx:subscribers(Topic)), + true end, with_connection(Topics, Fun). @@ -497,7 +503,8 @@ t_observe_wildcard(_) -> #coap_content{payload = PayloadRecv} = Notify, - ?assertEqual(Payload, PayloadRecv) + ?assertEqual(Payload, PayloadRecv), + true end, with_connection(Fun). @@ -530,7 +537,8 @@ t_clients_api(_) -> {204, _} = request(delete, "/gateways/coap/clients/client1"), timer:sleep(200), - {200, #{data := []}} = request(get, "/gateways/coap/clients") + {200, #{data := []}} = request(get, "/gateways/coap/clients"), + false end, with_connection(Fun). @@ -560,7 +568,8 @@ t_clients_subscription_api(_) -> {204, _} = request(delete, Path ++ "/tx"), - {200, []} = request(get, Path) + {200, []} = request(get, Path), + true end, with_connection(Fun). @@ -578,7 +587,8 @@ t_clients_get_subscription_api(_) -> observe(Channel, Token, false), - {200, []} = request(get, Path) + {200, []} = request(get, Path), + true end, with_connection(Fun). @@ -773,8 +783,7 @@ with_connection(Action) -> Fun = fun(Channel) -> Token = connection(Channel), timer:sleep(100), - Action(Channel, Token), - disconnection(Channel, Token), + _ = Action(Channel, Token) andalso disconnection(Channel, Token), timer:sleep(100) end, do(Fun). diff --git a/mix.exs b/mix.exs index 313059bde..53e5b304f 100644 --- a/mix.exs +++ b/mix.exs @@ -175,7 +175,7 @@ defmodule EMQXUmbrella.MixProject do end def common_dep(:ekka), do: {:ekka, github: "emqx/ekka", tag: "0.19.5", override: true} - def common_dep(:esockd), do: {:esockd, github: "emqx/esockd", tag: "5.11.2", override: true} + def common_dep(:esockd), do: {:esockd, github: "emqx/esockd", tag: "5.11.3", override: true} def common_dep(:gproc), do: {:gproc, github: "emqx/gproc", tag: "0.9.0.1", override: true} def common_dep(:hocon), do: {:hocon, github: "emqx/hocon", tag: "0.43.1", override: true} def common_dep(:lc), do: {:lc, github: "emqx/lc", tag: "0.3.2", override: true} diff --git a/rebar.config b/rebar.config index 8f689ea3d..115bed3e9 100644 --- a/rebar.config +++ b/rebar.config @@ -81,7 +81,7 @@ {gproc, {git, "https://github.com/emqx/gproc", {tag, "0.9.0.1"}}}, {jiffy, {git, "https://github.com/emqx/jiffy", {tag, "1.0.6"}}}, {cowboy, {git, "https://github.com/emqx/cowboy", {tag, "2.9.2"}}}, - {esockd, {git, "https://github.com/emqx/esockd", {tag, "5.11.2"}}}, + {esockd, {git, "https://github.com/emqx/esockd", {tag, "5.11.3"}}}, {rocksdb, {git, "https://github.com/emqx/erlang-rocksdb", {tag, "1.8.0-emqx-5"}}}, {ekka, {git, "https://github.com/emqx/ekka", {tag, "0.19.5"}}}, {gen_rpc, {git, "https://github.com/emqx/gen_rpc", {tag, "3.3.1"}}}, diff --git a/rel/i18n/emqx_gateway_schema.hocon b/rel/i18n/emqx_gateway_schema.hocon index 1a04d57ac..9f7bd8462 100644 --- a/rel/i18n/emqx_gateway_schema.hocon +++ b/rel/i18n/emqx_gateway_schema.hocon @@ -195,4 +195,13 @@ Relevant when the EMQX cluster is deployed behind a load-balancer.""" fields_ws_opts_proxy_address_header.label: """Proxy address header""" +udp_health_check.desc: +"""Some Cloud platform use a `request-reply` mechanism to check whether a UDP port is healthy, here can configure this pair.""" + +udp_health_check_request.desc: +"""The content of the request.""" + +udp_health_check_reply.desc: +"""The content to reply.""" + } From ec183f1d4cf5f3fa3a2e7d88e660fa477ec700dd Mon Sep 17 00:00:00 2001 From: firest Date: Sat, 13 Jul 2024 18:54:57 +0800 Subject: [PATCH 40/41] test(coap): fix ci errors --- .../test/emqx_gateway_authz_SUITE.erl | 6 ++-- .../src/emqx_coap_channel.erl | 29 ++++++++++++++++--- .../src/emqx_coap_proxy_conn.erl | 16 +++++++--- .../test/emqx_coap_SUITE.erl | 3 +- .../test/emqx_coap_api_SUITE.erl | 3 +- 5 files changed, 45 insertions(+), 12 deletions(-) diff --git a/apps/emqx_gateway/test/emqx_gateway_authz_SUITE.erl b/apps/emqx_gateway/test/emqx_gateway_authz_SUITE.erl index 5b262158f..87db2f64c 100644 --- a/apps/emqx_gateway/test/emqx_gateway_authz_SUITE.erl +++ b/apps/emqx_gateway/test/emqx_gateway_authz_SUITE.erl @@ -97,7 +97,8 @@ t_case_coap_publish(_) -> end, Case = fun(Channel, Token) -> Fun(Channel, Token, <<"/publish">>, ?checkMatch({ok, changed, _})), - Fun(Channel, Token, <<"/badpublish">>, ?checkMatch({error, uauthorized})) + Fun(Channel, Token, <<"/badpublish">>, ?checkMatch({error, uauthorized})), + true end, Mod:with_connection(Case). @@ -113,7 +114,8 @@ t_case_coap_subscribe(_) -> end, Case = fun(Channel, Token) -> Fun(Channel, Token, <<"/subscribe">>, ?checkMatch({ok, content, _})), - Fun(Channel, Token, <<"/badsubscribe">>, ?checkMatch({error, uauthorized})) + Fun(Channel, Token, <<"/badsubscribe">>, ?checkMatch({error, uauthorized})), + true end, Mod:with_connection(Case). diff --git a/apps/emqx_gateway_coap/src/emqx_coap_channel.erl b/apps/emqx_gateway_coap/src/emqx_coap_channel.erl index a10fa04b2..b2aac2a4c 100644 --- a/apps/emqx_gateway_coap/src/emqx_coap_channel.erl +++ b/apps/emqx_gateway_coap/src/emqx_coap_channel.erl @@ -410,6 +410,19 @@ is_create_connection_request(Msg = #coap_message{method = Method}) when is_create_connection_request(_Msg) -> false. +is_delete_connection_request(Msg = #coap_message{method = Method}) when + is_atom(Method) andalso Method =/= undefined +-> + URIPath = emqx_coap_message:get_option(uri_path, Msg, []), + case URIPath of + [<<"mqtt">>, <<"connection">>] when Method == delete -> + true; + _ -> + false + end; +is_delete_connection_request(_Msg) -> + false. + check_token( Msg, #channel{ @@ -424,10 +437,18 @@ check_token( <<"token">> := Token } -> call_session(handle_request, Msg, Channel); - _ -> - ErrMsg = <<"Missing token or clientid in connection mode">>, - Reply = emqx_coap_message:piggyback({error, bad_request}, ErrMsg, Msg), - {ok, {outgoing, Reply}, Channel} + Any -> + %% This channel is create by this DELETE command, so here can safely close this channel + case Token =:= undefined andalso is_delete_connection_request(Msg) of + true -> + Reply = emqx_coap_message:piggyback({ok, deleted}, Msg), + {shutdown, normal, Reply, Channel}; + false -> + io:format(">>> C1:~p, T1:~p~nC2:~p~n", [ClientId, Token, Any]), + ErrMsg = <<"Missing token or clientid in connection mode">>, + Reply = emqx_coap_message:piggyback({error, bad_request}, ErrMsg, Msg), + {ok, {outgoing, Reply}, Channel} + end end. run_conn_hooks( diff --git a/apps/emqx_gateway_coap/src/emqx_coap_proxy_conn.erl b/apps/emqx_gateway_coap/src/emqx_coap_proxy_conn.erl index 703450d7e..2eb8419f4 100644 --- a/apps/emqx_gateway_coap/src/emqx_coap_proxy_conn.erl +++ b/apps/emqx_gateway_coap/src/emqx_coap_proxy_conn.erl @@ -20,7 +20,7 @@ -include("emqx_coap.hrl"). --export([initialize/1, create/3, get_connection_id/4, dispatch/3, close/2]). +-export([initialize/1, find_or_create/4, get_connection_id/4, dispatch/3, close/2]). %%-------------------------------------------------------------------- %% Callbacks @@ -28,8 +28,13 @@ initialize(_Opts) -> emqx_coap_frame:initial_parse_state(#{}). -create(Transport, Peer, Opts) -> - emqx_gateway_conn:start_link(Transport, Peer, Opts). +find_or_create(CId, Transport, Peer, Opts) -> + case emqx_gateway_cm_registry:lookup_channels(coap, CId) of + [Pid] -> + {ok, Pid}; + [] -> + emqx_gateway_conn:start_link(Transport, Peer, Opts) + end. get_connection_id(_Transport, _Peer, State, Data) -> case parse_incoming(Data, [], State) of @@ -40,7 +45,10 @@ get_connection_id(_Transport, _Peer, State, Data) -> } -> {ok, ClientId, Packets, NState}; _ -> - invalid + ErrMsg = <<"Missing token or clientid in connection mode">>, + Reply = emqx_coap_message:piggyback({error, bad_request}, ErrMsg, Msg), + Bin = emqx_coap_frame:serialize_pkt(Reply, emqx_coap_frame:serialize_opts()), + {error, Bin} end; _Error -> invalid diff --git a/apps/emqx_gateway_coap/test/emqx_coap_SUITE.erl b/apps/emqx_gateway_coap/test/emqx_coap_SUITE.erl index bb244c8d4..5a4a027ab 100644 --- a/apps/emqx_gateway_coap/test/emqx_coap_SUITE.erl +++ b/apps/emqx_gateway_coap/test/emqx_coap_SUITE.erl @@ -165,7 +165,8 @@ t_connection(_) -> emqx_gateway_cm_registry:lookup_channels(coap, <<"client1">>) ) end, - do(Action). + do(Action), + ok. t_connection_with_short_param_name(_) -> Action = fun(Channel) -> diff --git a/apps/emqx_gateway_coap/test/emqx_coap_api_SUITE.erl b/apps/emqx_gateway_coap/test/emqx_coap_api_SUITE.erl index 79975d331..6bfa3a90b 100644 --- a/apps/emqx_gateway_coap/test/emqx_coap_api_SUITE.erl +++ b/apps/emqx_gateway_coap/test/emqx_coap_api_SUITE.erl @@ -207,7 +207,8 @@ test_recv_coap_request(UdpSock) -> test_send_coap_response(UdpSock, Host, Port, Code, Content, Request) -> is_list(Host) orelse error("Host is not a string"), {ok, IpAddr} = inet:getaddr(Host, inet), - Response = emqx_coap_message:piggyback(Code, Content, Request), + Response0 = emqx_coap_message:piggyback(Code, Content, Request), + Response = Response0#coap_message{options = #{uri_query => [<<"clientid=client1">>]}}, ?LOGT("test_send_coap_response Response=~p", [Response]), Binary = emqx_coap_frame:serialize_pkt(Response, undefined), ok = gen_udp:send(UdpSock, IpAddr, Port, Binary). From 269f6b29ccc113241fedf8ed9a4672096475dfb9 Mon Sep 17 00:00:00 2001 From: firest Date: Mon, 15 Jul 2024 11:26:55 +0800 Subject: [PATCH 41/41] chore: update changes --- changes/ce/perf-13441.en.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/ce/perf-13441.en.md diff --git a/changes/ce/perf-13441.en.md b/changes/ce/perf-13441.en.md new file mode 100644 index 000000000..949b3601f --- /dev/null +++ b/changes/ce/perf-13441.en.md @@ -0,0 +1 @@ +Enhanced CoAP gateway connection mode, UDP connection will always be bound to the corresponding gateway connection through the `clientid`.