From b016695cb1a30ad5f3d68a1a66539e531428f73b Mon Sep 17 00:00:00 2001 From: firest Date: Thu, 22 Dec 2022 15:39:38 +0800 Subject: [PATCH 1/3] fix(bridges): obfuscate the password in bridges API responses --- apps/emqx/src/emqx_misc.erl | 127 ++++++++++++++++++++++- apps/emqx_bridge/src/emqx_bridge_api.erl | 36 ++++++- 2 files changed, 157 insertions(+), 6 deletions(-) diff --git a/apps/emqx/src/emqx_misc.erl b/apps/emqx/src/emqx_misc.erl index 0e7b29869..483b99587 100644 --- a/apps/emqx/src/emqx_misc.erl +++ b/apps/emqx/src/emqx_misc.erl @@ -68,7 +68,7 @@ nolink_apply/2 ]). --export([clamp/3]). +-export([clamp/3, redact/1, redact/2, is_redacted/2, is_redacted/3]). -dialyzer({nowarn_function, [nolink_apply/2]}). @@ -556,6 +556,75 @@ try_to_existing_atom(Convert, Data, Encoding) -> _:Reason -> {error, Reason} end. +is_sensitive_key(token) -> true; +is_sensitive_key("token") -> true; +is_sensitive_key(<<"token">>) -> true; +is_sensitive_key(password) -> true; +is_sensitive_key("password") -> true; +is_sensitive_key(<<"password">>) -> true; +is_sensitive_key(secret) -> true; +is_sensitive_key("secret") -> true; +is_sensitive_key(<<"secret">>) -> true; +is_sensitive_key(_) -> false. + +redact(Term) -> + do_redact(Term, fun is_sensitive_key/1). + +redact(Term, Checker) -> + do_redact(Term, fun(V) -> + is_sensitive_key(V) orelse Checker(V) + end). + +do_redact(L, Checker) when is_list(L) -> + lists:map(fun(E) -> do_redact(E, Checker) end, L); +do_redact(M, Checker) when is_map(M) -> + maps:map( + fun(K, V) -> + do_redact(K, V, Checker) + end, + M + ); +do_redact({Key, Value}, Checker) -> + case Checker(Key) of + true -> + {Key, redact_v(Value)}; + false -> + {do_redact(Key, Checker), do_redact(Value, Checker)} + end; +do_redact(T, Checker) when is_tuple(T) -> + Elements = erlang:tuple_to_list(T), + Redact = do_redact(Elements, Checker), + erlang:list_to_tuple(Redact); +do_redact(Any, _Checker) -> + Any. + +do_redact(K, V, Checker) -> + case Checker(K) of + true -> + redact_v(V); + false -> + do_redact(V, Checker) + end. + +-define(REDACT_VAL, "******"). +redact_v(V) when is_binary(V) -> <>; +redact_v(_V) -> ?REDACT_VAL. + +is_redacted(K, V) -> + do_is_redacted(K, V, fun is_sensitive_key/1). + +is_redacted(K, V, Fun) -> + do_is_redacted(K, V, fun(E) -> + is_sensitive_key(E) orelse Fun(E) + end). + +do_is_redacted(K, ?REDACT_VAL, Fun) -> + Fun(K); +do_is_redacted(K, <>, Fun) -> + Fun(K); +do_is_redacted(_K, _V, _Fun) -> + false. + -ifdef(TEST). -include_lib("eunit/include/eunit.hrl"). @@ -568,6 +637,62 @@ ipv6_probe_test() -> ok end. +redact_test_() -> + Case = fun(Type, KeyT) -> + Key = + case Type of + atom -> KeyT; + string -> erlang:atom_to_list(KeyT); + binary -> erlang:atom_to_binary(KeyT) + end, + + ?assert(is_sensitive_key(Key)), + + %% direct + ?assertEqual({Key, ?REDACT_VAL}, redact({Key, foo})), + ?assertEqual(#{Key => ?REDACT_VAL}, redact(#{Key => foo})), + ?assertEqual({Key, Key, Key}, redact({Key, Key, Key})), + ?assertEqual({[{Key, ?REDACT_VAL}], bar}, redact({[{Key, foo}], bar})), + + %% 1 level nested + ?assertEqual([{Key, ?REDACT_VAL}], redact([{Key, foo}])), + ?assertEqual([#{Key => ?REDACT_VAL}], redact([#{Key => foo}])), + + %% 2 level nested + ?assertEqual(#{opts => [{Key, ?REDACT_VAL}]}, redact(#{opts => [{Key, foo}]})), + ?assertEqual(#{opts => #{Key => ?REDACT_VAL}}, redact(#{opts => #{Key => foo}})), + ?assertEqual({opts, [{Key, ?REDACT_VAL}]}, redact({opts, [{Key, foo}]})), + + %% 3 level nested + ?assertEqual([#{opts => [{Key, ?REDACT_VAL}]}], redact([#{opts => [{Key, foo}]}])), + ?assertEqual([{opts, [{Key, ?REDACT_VAL}]}], redact([{opts, [{Key, foo}]}])), + ?assertEqual([{opts, [#{Key => ?REDACT_VAL}]}], redact([{opts, [#{Key => foo}]}])) + end, + + Types = [atom, string, binary], + Keys = [ + token, + password, + secret + ], + [{case_name(Type, Key), fun() -> Case(Type, Key) end} || Key <- Keys, Type <- Types]. + +redact2_test_() -> + Case = fun(Key, Checker) -> + ?assertEqual({Key, ?REDACT_VAL}, redact({Key, foo}, Checker)), + ?assertEqual(#{Key => ?REDACT_VAL}, redact(#{Key => foo}, Checker)), + ?assertEqual({Key, Key, Key}, redact({Key, Key, Key}, Checker)), + ?assertEqual({[{Key, ?REDACT_VAL}], bar}, redact({[{Key, foo}], bar}, Checker)) + end, + + Checker = fun(E) -> E =:= passcode end, + + Keys = [secret, passcode], + [{case_name(atom, Key), fun() -> Case(Key, Checker) end} || Key <- Keys]. + +case_name(Type, Key) -> + lists:concat([Type, "-", Key]). + -endif. pub_props_to_packet(Properties) -> diff --git a/apps/emqx_bridge/src/emqx_bridge_api.erl b/apps/emqx_bridge/src/emqx_bridge_api.erl index e649e5215..1debc90c4 100644 --- a/apps/emqx_bridge/src/emqx_bridge_api.erl +++ b/apps/emqx_bridge/src/emqx_bridge_api.erl @@ -409,11 +409,13 @@ schema("/nodes/:node/bridges/:id/operation/:operation") -> '/bridges/:id'(get, #{bindings := #{id := Id}}) -> ?TRY_PARSE_ID(Id, lookup_from_all_nodes(BridgeType, BridgeName, 200)); '/bridges/:id'(put, #{bindings := #{id := Id}, body := Conf0}) -> - Conf = filter_out_request_body(Conf0), + Conf1 = filter_out_request_body(Conf0), ?TRY_PARSE_ID( Id, case emqx_bridge:lookup(BridgeType, BridgeName) of {ok, _} -> + RawConf = emqx:get_raw_config([bridges, BridgeType, BridgeName], #{}), + Conf = deobfuscate(Conf1, RawConf), case ensure_bridge_created(BridgeType, BridgeName, Conf) of ok -> lookup_from_all_nodes(BridgeType, BridgeName, 200); @@ -604,12 +606,12 @@ format_bridge_info([FirstBridge | _] = Bridges) -> Res = maps:remove(node, FirstBridge), NodeStatus = collect_status(Bridges), NodeMetrics = collect_metrics(Bridges), - Res#{ + redact(Res#{ status => aggregate_status(NodeStatus), node_status => NodeStatus, metrics => aggregate_metrics(NodeMetrics), node_metrics => NodeMetrics - }. + }). collect_status(Bridges) -> [maps:with([node, status], B) || B <- Bridges]. @@ -676,13 +678,13 @@ format_resp( Node ) -> RawConfFull = fill_defaults(Type, RawConf), - RawConfFull#{ + redact(RawConfFull#{ type => Type, name => maps:get(<<"name">>, RawConf, BridgeName), node => Node, status => Status, metrics => format_metrics(Metrics) - }. + }). format_metrics(#{ counters := #{ @@ -806,3 +808,27 @@ call_operation(Node, OperFunc, BridgeType, BridgeName) -> {error, _} -> {400, error_msg('INVALID_NODE', <<"invalid node">>)} end. + +redact(Term) -> + emqx_misc:redact(Term). + +deobfuscate(NewConf, OldConf) -> + maps:fold( + fun(K, V, Acc) -> + case maps:find(K, OldConf) of + error -> + Acc#{K => V}; + {ok, OldV} when is_map(V), is_map(OldV) -> + Acc#{K => deobfuscate(V, OldV)}; + {ok, OldV} -> + case emqx_misc:is_redacted(K, V) of + true -> + Acc#{K => OldV}; + _ -> + Acc#{K => V} + end + end + end, + #{}, + NewConf + ). From b43be50a987d72a09c0b323bf7a59bae7c5af647 Mon Sep 17 00:00:00 2001 From: firest Date: Fri, 30 Dec 2022 16:35:04 +0800 Subject: [PATCH 2/3] test: add the `redacted` test case for bridges api --- .../test/emqx_bridge_api_SUITE.erl | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl b/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl index 74e712c6f..60f770df8 100644 --- a/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl +++ b/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl @@ -514,6 +514,39 @@ t_reset_bridges(Config) -> {ok, 204, <<>>} = request(delete, uri(["bridges", BridgeID]), []), {ok, 200, <<"[]">>} = request(get, uri(["bridges"]), []). +t_with_redact_update(_Config) -> + Name = <<"redact_update">>, + Type = <<"mqtt">>, + Password = <<"123456">>, + Template = #{ + <<"type">> => Type, + <<"name">> => Name, + <<"server">> => <<"127.0.0.1:1883">>, + <<"username">> => <<"test">>, + <<"password">> => Password, + <<"ingress">> => + #{<<"remote">> => #{<<"topic">> => <<"t/#">>}} + }, + + {ok, 201, _} = request( + post, + uri(["bridges"]), + Template + ), + + %% update with redacted config + Conf = emqx_misc:redact(Template), + BridgeID = emqx_bridge_resource:bridge_id(Type, Name), + {ok, 200, _ResBin} = request( + put, + uri(["bridges", BridgeID]), + Conf + ), + RawConf = emqx:get_raw_config([bridges, Type, Name]), + Value = maps:get(<<"password">>, RawConf), + ?assertEqual(Password, Value), + ok. + request(Method, Url, Body) -> request(<<"bridge_admin">>, Method, Url, Body). From 69c7b41b5227a744789d371d1d1b002725b37fdb Mon Sep 17 00:00:00 2001 From: firest Date: Mon, 2 Jan 2023 21:52:11 +0800 Subject: [PATCH 3/3] chore: bump version && update changes --- changes/v5.0.14-en.md | 2 ++ changes/v5.0.14-zh.md | 2 ++ 2 files changed, 4 insertions(+) diff --git a/changes/v5.0.14-en.md b/changes/v5.0.14-en.md index 7ca8200b6..f61211bb8 100644 --- a/changes/v5.0.14-en.md +++ b/changes/v5.0.14-en.md @@ -7,6 +7,8 @@ `env EMQX_BRIDGES__MQTT__XYZ__SERVER='"localhost:1883"'`. Now it's possible to set it without quote as `env EMQX_BRIDGES__MQTT__XYZ__SERVER='localhost:1883'`. +- Obfuscated sensitive data in the response when querying `bridges` information by API [#9593](https://github.com/emqx/emqx/pull/9593/). + ## Bug Fixes - Fix an issue where testing the GCP PubSub could leak memory, and an issue where its JWT token would fail to refresh a second time. [#9641](https://github.com/emqx/emqx/pull/9641) diff --git a/changes/v5.0.14-zh.md b/changes/v5.0.14-zh.md index 2667fabef..c96bef305 100644 --- a/changes/v5.0.14-zh.md +++ b/changes/v5.0.14-zh.md @@ -7,6 +7,8 @@ `env EMQX_BRIDGES__MQTT__XYZ__SERVER='"localhost:1883"'`。 此修复后，可以不使用引号，例如 `env EMQX_BRIDGES__MQTT__XYZ__SERVER='localhost:1883'`。 +- 通过 API 查询 `bridges` 信息时将混淆响应中的敏感数据 [#9593](https://github.com/emqx/emqx/pull/9593/)。 + ## 修复 - 修复了测试GCP PubSub可能泄露内存的问题，以及其JWT令牌第二次刷新失败的问题。 [#9640](https://github.com/emqx/emqx/pull/9640)