From 88364945429a7835427c16d38c1d330dd4a16d45 Mon Sep 17 00:00:00 2001 From: Erik Timan Date: Wed, 25 Jan 2023 11:50:12 +0100 Subject: [PATCH 1/3] fix: redact influxdb tokens in a few logs --- apps/emqx_bridge/src/emqx_bridge.erl | 2 +- apps/emqx_bridge/src/emqx_bridge_resource.erl | 6 +++--- apps/emqx_conf/src/emqx_cluster_rpc.erl | 8 ++++---- apps/emqx_conf/src/emqx_conf.app.src | 2 +- .../src/emqx_ee_connector_influxdb.erl | 19 +++++++++++++------ 5 files changed, 22 insertions(+), 15 deletions(-) diff --git a/apps/emqx_bridge/src/emqx_bridge.erl b/apps/emqx_bridge/src/emqx_bridge.erl index 5b3fe796b..f6677bd09 100644 --- a/apps/emqx_bridge/src/emqx_bridge.erl +++ b/apps/emqx_bridge/src/emqx_bridge.erl @@ -255,7 +255,7 @@ create(BridgeType, BridgeName, RawConf) -> brige_action => create, bridge_type => BridgeType, bridge_name => BridgeName, - bridge_raw_config => RawConf + bridge_raw_config => emqx_misc:redact(RawConf) }), emqx_conf:update( emqx_bridge:config_key_path() ++ [BridgeType, BridgeName], diff --git a/apps/emqx_bridge/src/emqx_bridge_resource.erl b/apps/emqx_bridge/src/emqx_bridge_resource.erl index cbff85df3..d228f2281 100644 --- a/apps/emqx_bridge/src/emqx_bridge_resource.erl +++ b/apps/emqx_bridge/src/emqx_bridge_resource.erl @@ -137,7 +137,7 @@ create(Type, Name, Conf, Opts0) -> msg => "create bridge", type => Type, name => Name, - config => Conf + config => emqx_misc:redact(Conf) }), Opts = override_start_after_created(Conf, Opts0), {ok, _Data} = emqx_resource:create_local( @@ -172,7 +172,7 @@ update(Type, Name, {OldConf, Conf}, Opts0) -> msg => "update bridge", type => Type, name => Name, - config => Conf + config => emqx_misc:redact(Conf) }), case recreate(Type, Name, Conf, Opts) of {ok, _} -> @@ -182,7 +182,7 @@ update(Type, Name, {OldConf, Conf}, Opts0) -> msg => "updating_a_non_existing_bridge", type => Type, name => Name, - config => Conf + config => emqx_misc:redact(Conf) }), create(Type, Name, Conf, Opts); {error, Reason} -> diff --git a/apps/emqx_conf/src/emqx_cluster_rpc.erl b/apps/emqx_conf/src/emqx_cluster_rpc.erl index fe701049c..c285e09b8 100644 --- a/apps/emqx_conf/src/emqx_cluster_rpc.erl +++ b/apps/emqx_conf/src/emqx_cluster_rpc.erl @@ -495,15 +495,15 @@ log_and_alarm(IsSuccess, Res, #{kind := ?APPLY_KIND_INITIATE} = Meta) -> %% because nothing is committed case IsSuccess of true -> - ?SLOG(debug, Meta#{msg => "cluster_rpc_apply_result", result => Res}); + ?SLOG(debug, Meta#{msg => "cluster_rpc_apply_result", result => emqx_misc:redact(Res)}); false -> - ?SLOG(warning, Meta#{msg => "cluster_rpc_apply_result", result => Res}) + ?SLOG(warning, Meta#{msg => "cluster_rpc_apply_result", result => emqx_misc:redact(Res)}) end; log_and_alarm(true, Res, Meta) -> - ?SLOG(debug, Meta#{msg => "cluster_rpc_apply_ok", result => Res}), + ?SLOG(debug, Meta#{msg => "cluster_rpc_apply_ok", result => emqx_misc:redact(Res)}), do_alarm(deactivate, Res, Meta); log_and_alarm(false, Res, Meta) -> - ?SLOG(error, Meta#{msg => "cluster_rpc_apply_failed", result => Res}), + ?SLOG(error, Meta#{msg => "cluster_rpc_apply_failed", result => emqx_misc:redact(Res)}), do_alarm(activate, Res, Meta). do_alarm(Fun, Res, #{tnx_id := Id} = Meta) -> diff --git a/apps/emqx_conf/src/emqx_conf.app.src b/apps/emqx_conf/src/emqx_conf.app.src index b13c0d055..f7fd33e3b 100644 --- a/apps/emqx_conf/src/emqx_conf.app.src +++ b/apps/emqx_conf/src/emqx_conf.app.src @@ -1,6 +1,6 @@ {application, emqx_conf, [ {description, "EMQX configuration management"}, - {vsn, "0.1.10"}, + {vsn, "0.1.11"}, {registered, []}, {mod, {emqx_conf_app, []}}, {applications, [kernel, stdlib]}, diff --git a/lib-ee/emqx_ee_connector/src/emqx_ee_connector_influxdb.erl b/lib-ee/emqx_ee_connector/src/emqx_ee_connector_influxdb.erl index 824233a6d..ef1b2edc9 100644 --- a/lib-ee/emqx_ee_connector/src/emqx_ee_connector_influxdb.erl +++ b/lib-ee/emqx_ee_connector/src/emqx_ee_connector_influxdb.erl @@ -200,8 +200,8 @@ start_client(InstId, Config) -> ?SLOG(info, #{ msg => "starting influxdb connector", connector => InstId, - config => Config, - client_config => ClientConfig + config => emqx_misc:redact(Config), + client_config => emqx_misc:redact(ClientConfig) }), try do_start_client(InstId, ClientConfig, Config) @@ -236,8 +236,8 @@ do_start_client( ?SLOG(info, #{ msg => "starting influxdb connector success", connector => InstId, - client => Client, - state => State + client => redact_auth(Client), + state => redact_auth(State) }), {ok, State}; false -> @@ -245,7 +245,7 @@ do_start_client( ?SLOG(error, #{ msg => "starting influxdb connector failed", connector => InstId, - client => Client, + client => redact_auth(Client), reason => "client is not alive" }), {error, influxdb_client_not_alive} @@ -255,7 +255,7 @@ do_start_client( ?SLOG(info, #{ msg => "restarting influxdb connector, found already started client", connector => InstId, - old_client => Client0 + old_client => redact_auth(Client0) }), _ = influxdb:stop_client(Client0), do_start_client(InstId, ClientConfig, Config); @@ -338,6 +338,13 @@ password(#{password := Password}) -> password(_) -> []. +redact_auth(Term) -> + emqx_misc:redact(Term, fun is_auth_key/1). + +is_auth_key(<<"Authorization">>) -> true; +is_auth_key(<<"authorization">>) -> true; +is_auth_key(_) -> false. + %% ------------------------------------------------------------------------------------------------- %% Query do_query(InstId, Client, Points) -> From 805d08e823364f02f1d7ab7c7958994f68f8e661 Mon Sep 17 00:00:00 2001 From: Erik Timan Date: Wed, 25 Jan 2023 11:56:27 +0100 Subject: [PATCH 2/3] fix: reduce log level from error to warning in several places This reduces the log level from error to warning in places that are connected to the influxdb bridge. Transient errors for external resources should not render an error log. --- apps/emqx_resource/src/emqx_resource_manager.erl | 4 ++-- lib-ee/emqx_ee_connector/src/emqx_ee_connector_influxdb.erl | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/apps/emqx_resource/src/emqx_resource_manager.erl b/apps/emqx_resource/src/emqx_resource_manager.erl index 8098dbe42..47e0cf658 100644 --- a/apps/emqx_resource/src/emqx_resource_manager.erl +++ b/apps/emqx_resource/src/emqx_resource_manager.erl @@ -487,7 +487,7 @@ start_resource(Data, From) -> Actions = maybe_reply([{state_timeout, 0, health_check}], From, ok), {next_state, connecting, UpdatedData, Actions}; {error, Reason} = Err -> - ?SLOG(error, #{ + ?SLOG(warning, #{ msg => start_resource_failed, id => Data#data.id, reason => Reason @@ -546,7 +546,7 @@ handle_connected_health_check(Data) -> Actions = [{state_timeout, health_check_interval(Data#data.opts), health_check}], {keep_state, UpdatedData, Actions}; (Status, UpdatedData) -> - ?SLOG(error, #{ + ?SLOG(warning, #{ msg => health_check_failed, id => Data#data.id, status => Status diff --git a/lib-ee/emqx_ee_connector/src/emqx_ee_connector_influxdb.erl b/lib-ee/emqx_ee_connector/src/emqx_ee_connector_influxdb.erl index ef1b2edc9..1370ed2c2 100644 --- a/lib-ee/emqx_ee_connector/src/emqx_ee_connector_influxdb.erl +++ b/lib-ee/emqx_ee_connector/src/emqx_ee_connector_influxdb.erl @@ -208,7 +208,7 @@ start_client(InstId, Config) -> catch E:R:S -> ?tp(influxdb_connector_start_exception, #{error => {E, R}}), - ?SLOG(error, #{ + ?SLOG(warning, #{ msg => "start influxdb connector error", connector => InstId, error => E, @@ -242,7 +242,7 @@ do_start_client( {ok, State}; false -> ?tp(influxdb_connector_start_failed, #{error => influxdb_client_not_alive}), - ?SLOG(error, #{ + ?SLOG(warning, #{ msg => "starting influxdb connector failed", connector => InstId, client => redact_auth(Client), @@ -261,7 +261,7 @@ do_start_client( do_start_client(InstId, ClientConfig, Config); {error, Reason} -> ?tp(influxdb_connector_start_failed, #{error => Reason}), - ?SLOG(error, #{ + ?SLOG(warning, #{ msg => "starting influxdb connector failed", connector => InstId, reason => Reason From 1f235ffee935696e15fb06c2da8aa7c7e31c9810 Mon Sep 17 00:00:00 2001 From: Erik Timan Date: Thu, 26 Jan 2023 10:30:13 +0100 Subject: [PATCH 3/3] refactor(emqx_ee_connector): redo readact key function --- .../src/emqx_ee_connector_influxdb.erl | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/lib-ee/emqx_ee_connector/src/emqx_ee_connector_influxdb.erl b/lib-ee/emqx_ee_connector/src/emqx_ee_connector_influxdb.erl index 1370ed2c2..402202b11 100644 --- a/lib-ee/emqx_ee_connector/src/emqx_ee_connector_influxdb.erl +++ b/lib-ee/emqx_ee_connector/src/emqx_ee_connector_influxdb.erl @@ -341,9 +341,10 @@ password(_) -> redact_auth(Term) -> emqx_misc:redact(Term, fun is_auth_key/1). -is_auth_key(<<"Authorization">>) -> true; -is_auth_key(<<"authorization">>) -> true; -is_auth_key(_) -> false. +is_auth_key(Key) when is_binary(Key) -> + string:equal("authorization", Key, true); +is_auth_key(_) -> + false. %% ------------------------------------------------------------------------------------------------- %% Query @@ -628,6 +629,13 @@ is_unrecoverable_error(_) -> -ifdef(TEST). -include_lib("eunit/include/eunit.hrl"). +is_auth_key_test_() -> + [ + ?_assert(is_auth_key(<<"Authorization">>)), + ?_assertNot(is_auth_key(<<"Something">>)), + ?_assertNot(is_auth_key(89)) + ]. + %% for coverage desc_test_() -> [