diff --git a/apps/emqx_connector/src/emqx_connector_http.erl b/apps/emqx_connector/src/emqx_connector_http.erl index ffb4bd8a9..ebe5b3fdb 100644 --- a/apps/emqx_connector/src/emqx_connector_http.erl +++ b/apps/emqx_connector/src/emqx_connector_http.erl @@ -473,7 +473,7 @@ preprocess_request( method => emqx_plugin_libs_rule:preproc_tmpl(to_bin(Method)), path => emqx_plugin_libs_rule:preproc_tmpl(Path), body => maybe_preproc_tmpl(body, Req), - headers => preproc_headers(Headers), + headers => wrap_auth_header(preproc_headers(Headers)), request_timeout => maps:get(request_timeout, Req, 30000), max_retries => maps:get(max_retries, Req, 2) }. @@ -503,6 +503,36 @@ preproc_headers(Headers) when is_list(Headers) -> Headers ). +wrap_auth_header(Headers) -> + lists:map(fun maybe_wrap_auth_header/1, Headers). + +maybe_wrap_auth_header({[{str, Key}] = StrKey, Val}) -> + {_, MaybeWrapped} = maybe_wrap_auth_header({Key, Val}), + {StrKey, MaybeWrapped}; +maybe_wrap_auth_header({Key, Val} = Header) when + is_binary(Key), (size(Key) =:= 19 orelse size(Key) =:= 13) +-> + %% We check the size of potential keys in the guard above and consider only + %% those that match the number of characters of either "Authorization" or + %% "Proxy-Authorization". + case try_bin_to_lower(Key) of + <<"authorization">> -> + {Key, emqx_secret:wrap(Val)}; + <<"proxy-authorization">> -> + {Key, emqx_secret:wrap(Val)}; + _Other -> + Header + end; +maybe_wrap_auth_header(Header) -> + Header. + +try_bin_to_lower(Bin) -> + try iolist_to_binary(string:lowercase(Bin)) of + LowercaseBin -> LowercaseBin + catch + _:_ -> Bin + end. + maybe_preproc_tmpl(Key, Conf) -> case maps:get(Key, Conf, undefined) of undefined -> undefined; @@ -537,7 +567,7 @@ proc_headers(HeaderTks, Msg) -> fun({K, V}) -> { emqx_plugin_libs_rule:proc_tmpl(K, Msg), - emqx_plugin_libs_rule:proc_tmpl(V, Msg) + emqx_plugin_libs_rule:proc_tmpl(emqx_secret:unwrap(V), Msg) } end, HeaderTks @@ -628,21 +658,13 @@ is_sensitive_key([{str, StringKey}]) -> is_sensitive_key(Atom) when is_atom(Atom) -> is_sensitive_key(erlang:atom_to_binary(Atom)); is_sensitive_key(Bin) when is_binary(Bin), (size(Bin) =:= 19 orelse size(Bin) =:= 13) -> - try - %% This is wrapped in a try-catch since we don't know that Bin is a - %% valid string so string:lowercase/1 might throw an exception. - %% - %% We want to convert this to lowercase since the http header fields - %% are case insensitive, which means that a user of the Webhook bridge - %% can write this field name in many different ways. - LowercaseBin = iolist_to_binary(string:lowercase(Bin)), - case LowercaseBin of - <<"authorization">> -> true; - <<"proxy-authorization">> -> true; - _ -> false - end - catch - _:_ -> false + %% We want to convert this to lowercase since the http header fields + %% are case insensitive, which means that a user of the Webhook bridge + %% can write this field name in many different ways. + case try_bin_to_lower(Bin) of + <<"authorization">> -> true; + <<"proxy-authorization">> -> true; + _ -> false end; is_sensitive_key(_) -> false. diff --git a/apps/emqx_connector/test/emqx_connector_http_tests.erl b/apps/emqx_connector/test/emqx_connector_http_tests.erl new file mode 100644 index 000000000..8d0fa6d2c --- /dev/null +++ b/apps/emqx_connector/test/emqx_connector_http_tests.erl @@ -0,0 +1,90 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2023 EMQ Technologies Co., Ltd. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%%-------------------------------------------------------------------- +-module(emqx_connector_http_tests). + +-include_lib("eunit/include/eunit.hrl"). + +-define(MY_SECRET, <<"my_precious">>). + +wrap_auth_headers_test_() -> + {setup, + fun() -> + meck:expect(ehttpc_sup, start_pool, 2, {ok, foo}), + meck:expect(ehttpc, request, fun(_, _, Req, _, _) -> {ok, 200, Req} end), + meck:expect(ehttpc_pool, pick_worker, 1, self()), + [ehttpc_sup, ehttpc, ehttpc_pool] + end, + fun meck:unload/1, fun(_) -> + Config = #{ + base_url => #{ + scheme => http, + host => "localhost", + port => 18083, + path => "/status" + }, + connect_timeout => 1000, + pool_type => random, + pool_size => 1, + request => #{ + method => get, + path => "/status", + headers => auth_headers() + } + }, + {ok, #{request := #{headers := Headers}} = State} = emqx_connector_http:on_start( + <<"test">>, Config + ), + {ok, 200, Req} = emqx_connector_http:on_query(foo, {send_message, #{}}, State), + Tests = + [ + ?_assert(is_wrapped(V)) + || H <- Headers, is_tuple({K, V} = H), is_auth_header(untmpl(K)) + ], + [ + ?_assertEqual(4, length(Tests)), + ?_assert(is_unwrapped_headers(element(2, Req))) + | Tests + ] + end}. + +auth_headers() -> + [ + {<<"Authorization">>, ?MY_SECRET}, + {<<"authorization">>, ?MY_SECRET}, + {<<"Proxy-Authorization">>, ?MY_SECRET}, + {<<"proxy-authorization">>, ?MY_SECRET}, + {<<"X-Custom-Header">>, <<"foobar">>} + ]. + +is_auth_header(<<"Authorization">>) -> true; +is_auth_header(<<"Proxy-Authorization">>) -> true; +is_auth_header(<<"authorization">>) -> true; +is_auth_header(<<"proxy-authorization">>) -> true; +is_auth_header(_Other) -> false. + +is_wrapped(Secret) when is_function(Secret) -> + untmpl(emqx_secret:unwrap(Secret)) =:= ?MY_SECRET; +is_wrapped(_Other) -> + false. + +untmpl([{_, V} | _]) -> V. + +is_unwrapped_headers(Headers) -> + lists:all(fun is_unwrapped_header/1, Headers). + +is_unwrapped_header({_, V}) when is_function(V) -> false; +is_unwrapped_header({_, [{str, _V}]}) -> throw(unexpected_tmpl_token); +is_unwrapped_header(_) -> true. diff --git a/changes/ce/fix-10556.en.md b/changes/ce/fix-10556.en.md new file mode 100644 index 000000000..019ab3ad9 --- /dev/null +++ b/changes/ce/fix-10556.en.md @@ -0,0 +1 @@ +Wrap potentially sensitive data in `emqx_connector_http` if `Authorization` headers are being passed at initialization.