From bca3782d735eb7840c549d4d9519bba16b4741cf Mon Sep 17 00:00:00 2001 From: Ilya Averyanov Date: Mon, 13 May 2024 23:46:05 +0300 Subject: [PATCH] fix(auth_http): fix query encoding * ignore authenticator if JSON format is set up for requests, but non-utf8 data is going to be sent * use application/json format by default * fix encoding of query part of the requests --- .../src/emqx_authn/emqx_authn_utils.erl | 56 ++++++++- apps/emqx_auth_http/src/emqx_authn_http.erl | 114 +++++++++++------- .../test/emqx_authn_http_SUITE.erl | 113 ++++++++++++++++- .../src/emqx_authn_mongodb.erl | 2 +- apps/emqx_utils/src/emqx_template.erl | 22 +++- 5 files changed, 252 insertions(+), 55 deletions(-) diff --git a/apps/emqx_auth/src/emqx_authn/emqx_authn_utils.erl b/apps/emqx_auth/src/emqx_authn/emqx_authn_utils.erl index ad7cf9a92..56865517b 100644 --- a/apps/emqx_auth/src/emqx_authn/emqx_authn_utils.erl +++ b/apps/emqx_auth/src/emqx_authn/emqx_authn_utils.erl @@ -28,7 +28,8 @@ parse_str/1, parse_str/2, parse_sql/2, - render_deep/2, + render_deep_for_json/2, + render_deep_for_url/2, render_str/2, render_urlencoded_str/2, render_sql_params/2, @@ -166,13 +167,23 @@ prerender_disallowed_placeholders(Template) -> }), Result. -render_deep(Template, Credential) -> +render_deep_for_json(Template, Credential) -> % NOTE % Ignoring errors here, undefined bindings will be replaced with empty string. {Term, _Errors} = emqx_template:render( Template, mapping_credential(Credential), - #{var_trans => fun to_string/2} + #{var_trans => fun to_string_for_json/2} + ), + Term. + +render_deep_for_url(Template, Credential) -> + % NOTE + % Ignoring errors here, undefined bindings will be replaced with empty string. + {Term, _Errors} = emqx_template:render( + Template, + mapping_credential(Credential), + #{var_trans => fun to_string_for_urlencode/2} ), Term. @@ -202,7 +213,7 @@ render_sql_params(ParamList, Credential) -> {Row, _Errors} = emqx_template:render( ParamList, mapping_credential(Credential), - #{var_trans => fun to_sql_valaue/2} + #{var_trans => fun to_sql_value/2} ), Row. @@ -328,12 +339,43 @@ without_password(Credential, [Name | Rest]) -> end. to_urlencoded_string(Name, Value) -> - emqx_http_lib:uri_encode(to_string(Name, Value)). + <<"q=", EncodedValue/binary>> = uri_string:compose_query([{<<"q">>, to_string(Name, Value)}]), + EncodedValue. to_string(Name, Value) -> emqx_template:to_string(render_var(Name, Value)). -to_sql_valaue(Name, Value) -> +%% Any data may be urlencoded, so we allow non-unicode binaries here. + +to_string_for_urlencode(Name, Value) -> + to_string_for_urlencode(render_var(Name, Value)). + +to_string_for_urlencode(Value) when is_binary(Value) -> + Value; +to_string_for_urlencode(Value) when is_list(Value) -> + unicode:characters_to_binary(Value); +to_string_for_urlencode(Value) -> + emqx_template:to_string(Value). + +%% JSON strings are sequences of unicode characters, not bytes. +%% So we force all rendered data to be unicode. + +to_string_for_json(Name, Value) -> + to_unicode_string(Name, render_var(Name, Value)). + +to_unicode_string(Name, Value) when is_list(Value) orelse is_binary(Value) -> + try unicode:characters_to_binary(Value) of + Encoded when is_binary(Encoded) -> + Encoded; + _ -> + error({encode_error, {non_unicode_data, Name}}) + catch error:badarg -> + error({encode_error, {non_unicode_data, Name}}) + end; +to_unicode_string(_Name, Value) -> + emqx_template:to_string(Value). + +to_sql_value(Name, Value) -> emqx_utils_sql:to_sql_value(render_var(Name, Value)). render_var(_, undefined) -> @@ -343,6 +385,8 @@ render_var(_, undefined) -> <<>>; render_var(?VAR_PEERHOST, Value) -> inet:ntoa(Value); +render_var(?VAR_PASSWORD, Value) -> + iolist_to_binary(Value); render_var(_Name, Value) -> Value. diff --git a/apps/emqx_auth_http/src/emqx_authn_http.erl b/apps/emqx_auth_http/src/emqx_authn_http.erl index 3b04a29d2..81421dab3 100644 --- a/apps/emqx_auth_http/src/emqx_authn_http.erl +++ b/apps/emqx_auth_http/src/emqx_authn_http.erl @@ -28,6 +28,8 @@ destroy/1 ]). +-define(DEFAULT_CONTENT_TYPE, <<"application/json">>). + %%------------------------------------------------------------------------------ %% APIs %%------------------------------------------------------------------------------ @@ -68,23 +70,34 @@ authenticate( request_timeout := RequestTimeout } = State ) -> - Request = generate_request(Credential, State), - Response = emqx_resource:simple_sync_query(ResourceId, {Method, Request, RequestTimeout}), - ?TRACE_AUTHN_PROVIDER("http_response", #{ - request => request_for_log(Credential, State), - response => response_for_log(Response), - resource => ResourceId - }), - case Response of - {ok, 204, _Headers} -> - {ok, #{is_superuser => false}}; - {ok, 200, Headers, Body} -> - handle_response(Headers, Body); - {ok, _StatusCode, _Headers} = Response -> - ignore; - {ok, _StatusCode, _Headers, _Body} = Response -> - ignore; - {error, _Reason} -> + case generate_request(Credential, State) of + {ok, Request} -> + Response = emqx_resource:simple_sync_query( + ResourceId, {Method, Request, RequestTimeout} + ), + ?TRACE_AUTHN_PROVIDER("http_response", #{ + request => request_for_log(Credential, State), + response => response_for_log(Response), + resource => ResourceId + }), + case Response of + {ok, 204, _Headers} -> + {ok, #{is_superuser => false}}; + {ok, 200, Headers, Body} -> + handle_response(Headers, Body); + {ok, _StatusCode, _Headers} = Response -> + ignore; + {ok, _StatusCode, _Headers, _Body} = Response -> + ignore; + {error, _Reason} -> + ignore + end; + {error, Reason} -> + ?TRACE_AUTHN_PROVIDER( + error, + "generate_http_request_failed", + #{reason => Reason, credential => emqx_authn_utils:without_password(Credential)} + ), ignore end. @@ -99,7 +112,8 @@ destroy(#{resource_id := ResourceId}) -> with_validated_config(Config, Fun) -> Pipeline = [ fun check_ssl_opts/1, - fun check_headers/1, + fun normalize_headers/1, + fun check_method_headers/1, fun parse_config/1 ], case emqx_utils:pipeline(Pipeline, Config, undefined) of @@ -116,15 +130,23 @@ check_ssl_opts(#{url := <<"https://", _/binary>>, ssl := #{enable := false}}) -> check_ssl_opts(_) -> ok. -check_headers(#{headers := Headers, method := get}) -> +normalize_headers(#{headers := Headers} = Config) -> + {ok, Config#{headers => ensure_binary_names(Headers)}, undefined}. + +check_method_headers(#{headers := Headers, method := get}) -> case maps:is_key(<<"content-type">>, Headers) of false -> ok; true -> {error, {invalid_headers, <<"HTTP GET requests cannot include content-type header.">>}} end; -check_headers(_) -> - ok. +check_method_headers(#{headers := Headers, method := post} = Config) -> + {ok, + Config#{ + headers => + maps:merge(#{<<"content-type">> => ?DEFAULT_CONTENT_TYPE}, Headers) + }, + undefined}. parse_config( #{ @@ -134,16 +156,20 @@ parse_config( request_timeout := RequestTimeout } = Config ) -> + ct:print("parse_config: ~p~n", [Config]), {RequestBase, Path, Query} = emqx_auth_utils:parse_url(RawUrl), State = #{ method => Method, path => Path, - headers => ensure_header_name_type(Headers), + headers => Headers, base_path_template => emqx_authn_utils:parse_str(Path), base_query_template => emqx_authn_utils:parse_deep( cow_qs:parse_qs(Query) ), - body_template => emqx_authn_utils:parse_deep(maps:get(body, Config, #{})), + body_template => + emqx_authn_utils:parse_deep( + emqx_utils_maps:binary_key_map(maps:get(body, Config, #{})) + ), request_timeout => RequestTimeout, url => RawUrl }, @@ -163,36 +189,38 @@ generate_request(Credential, #{ }) -> Headers = maps:to_list(Headers0), Path = emqx_authn_utils:render_urlencoded_str(BasePathTemplate, Credential), - Query = emqx_authn_utils:render_deep(BaseQueryTemplate, Credential), - Body = emqx_authn_utils:render_deep(BodyTemplate, Credential), + Query = emqx_authn_utils:render_deep_for_url(BaseQueryTemplate, Credential), case Method of get -> + Body = emqx_authn_utils:render_deep_for_url(BodyTemplate, Credential), NPathQuery = append_query(to_list(Path), to_list(Query) ++ maps:to_list(Body)), - {NPathQuery, Headers}; + {ok, {NPathQuery, Headers}}; post -> - NPathQuery = append_query(to_list(Path), to_list(Query)), - ContentType = proplists:get_value(<<"content-type">>, Headers), - NBody = serialize_body(ContentType, Body), - {NPathQuery, Headers, NBody} + ContentType = post_request_content_type(Headers), + try + Body = serialize_body(ContentType, BodyTemplate, Credential), + NPathQuery = append_query(to_list(Path), to_list(Query)), + {ok, {NPathQuery, Headers, Body}} + catch + error:{encode_error, _} = Reason -> + {error, Reason} + end end. append_query(Path, []) -> Path; append_query(Path, Query) -> - Path ++ "?" ++ binary_to_list(qs(Query)). + ct:print("append_query: ~p~n", [Query]), + Path ++ "?" ++ qs(Query). qs(KVs) -> - qs(KVs, []). + uri_string:compose_query(KVs). -qs([], Acc) -> - <<$&, Qs/binary>> = iolist_to_binary(lists:reverse(Acc)), - Qs; -qs([{K, V} | More], Acc) -> - qs(More, [["&", uri_encode(K), "=", uri_encode(V)] | Acc]). - -serialize_body(<<"application/json">>, Body) -> +serialize_body(<<"application/json">>, BodyTemplate, Credential) -> + Body = emqx_authn_utils:render_deep_for_json(BodyTemplate, Credential), emqx_utils_json:encode(Body); -serialize_body(<<"application/x-www-form-urlencoded">>, Body) -> +serialize_body(<<"application/x-www-form-urlencoded">>, BodyTemplate, Credential) -> + Body = emqx_authn_utils:render_deep_for_url(BodyTemplate, Credential), qs(maps:to_list(Body)). handle_response(Headers, Body) -> @@ -239,8 +267,8 @@ parse_body(<<"application/x-www-form-urlencoded", _/binary>>, Body) -> parse_body(ContentType, _) -> {error, {unsupported_content_type, ContentType}}. -uri_encode(T) -> - emqx_http_lib:uri_encode(to_list(T)). +post_request_content_type(Headers) -> + proplists:get_value(<<"content-type">>, Headers, ?DEFAULT_CONTENT_TYPE). request_for_log(Credential, #{url := Url, method := Method} = State) -> SafeCredential = emqx_authn_utils:without_password(Credential), @@ -276,7 +304,7 @@ to_list(B) when is_binary(B) -> to_list(L) when is_list(L) -> L. -ensure_header_name_type(Headers) -> +ensure_binary_names(Headers) -> Fun = fun (Key, _Val, Acc) when is_binary(Key) -> Acc; 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 73422ec1d..2382ac4c6 100644 --- a/apps/emqx_auth_http/test/emqx_authn_http_SUITE.erl +++ b/apps/emqx_auth_http/test/emqx_authn_http_SUITE.erl @@ -152,8 +152,9 @@ test_user_auth(#{ handler := Handler, config_params := SpecificConfgParams, result := Expect -}) -> - Result = perform_user_auth(SpecificConfgParams, Handler, ?CREDENTIALS), +} = Sample) -> + Credentials = maps:merge(?CREDENTIALS, maps:get(credentials, Sample, #{})), + Result = perform_user_auth(SpecificConfgParams, Handler, Credentials), ?assertEqual(Expect, Result). perform_user_auth(SpecificConfgParams, Handler, Credentials) -> @@ -180,7 +181,7 @@ t_authenticate_path_placeholders(_Config) -> fun(Req0, State) -> Req = case cowboy_req:path(Req0) of - <<"/auth/p%20ath//us%20er/auth//">> -> + <<"/auth/p%20ath//us+er/auth//">> -> cowboy_req:reply( 200, #{<<"content-type">> => <<"application/json">>}, @@ -563,6 +564,31 @@ samples() -> result => {ok, #{is_superuser => true, client_attrs => #{<<"fid">> => <<"n11">>}}} }, + %% get request with non-utf8 password + #{ + handler => fun(Req0, State) -> + #{ + password := <<255, 255, 255>> + } = cowboy_req:match_qs([password], Req0), + Req = cowboy_req:reply( + 200, + #{<<"content-type">> => <<"application/json">>}, + emqx_utils_json:encode(#{ + result => allow, + is_superuser => true, + client_attrs => #{} + }), + Req0 + ), + {ok, Req, State} + end, + config_params => #{}, + credentials => #{ + password => <<255, 255, 255>> + }, + result => {ok, #{is_superuser => true, client_attrs => #{}}} + }, + %% get request with url-form-encoded body response #{ handler => fun(Req0, State) -> @@ -623,6 +649,31 @@ samples() -> result => {ok, #{is_superuser => false, client_attrs => #{}}} }, + %% post request, no content-type header + #{ + handler => fun(Req0, State) -> + {ok, RawBody, Req1} = cowboy_req:read_body(Req0), + #{ + <<"username">> := <<"plain">>, + <<"password">> := <<"plain">> + } = emqx_utils_json:decode(RawBody, [return_maps]), + ct:print("headers: ~p", [cowboy_req:headers(Req0)]), + <<"application/json">> = cowboy_req:header(<<"content-type">>, Req0), + Req = cowboy_req:reply( + 200, + #{<<"content-type">> => <<"application/json">>}, + emqx_utils_json:encode(#{result => allow, is_superuser => false}), + Req1 + ), + {ok, Req, State} + end, + config_params => #{ + <<"method">> => <<"post">>, + <<"headers">> => #{} + }, + result => {ok, #{is_superuser => false, client_attrs => #{}}} + }, + %% simple post request, application/x-www-form-urlencoded #{ handler => fun(Req0, State) -> @@ -686,6 +737,62 @@ samples() -> result => {ok, #{is_superuser => false, client_attrs => #{}}} }, + %% post request with non-utf8 password, application/json + #{ + handler => fun(Req0, State) -> + Req = cowboy_req:reply( + 200, + #{<<"content-type">> => <<"application/json">>}, + emqx_utils_json:encode(#{result => allow, is_superuser => false}), + Req0 + ), + {ok, Req, State} + end, + config_params => #{ + <<"method">> => <<"post">>, + <<"headers">> => #{<<"content-type">> => <<"application/json">>}, + <<"body">> => #{ + <<"password">> => ?PH_PASSWORD + } + }, + credentials => #{ + password => <<255, 255, 255>> + }, + %% non-utf8 password cannot be encoded in json + result => {error, not_authorized} + }, + + %% post request with non-utf8 password, form urlencoded + #{ + handler => fun(Req0, State) -> + {ok, PostVars, Req1} = cowboy_req:read_urlencoded_body(Req0), + #{ + <<"password">> := <<255, 255, 255>> + } = maps:from_list(PostVars), + Req = cowboy_req:reply( + 200, + #{<<"content-type">> => <<"application/json">>}, + emqx_utils_json:encode(#{result => allow, is_superuser => false}), + Req1 + ), + {ok, Req, State} + end, + config_params => #{ + <<"method">> => <<"post">>, + <<"headers">> => #{ + <<"content-type">> => + <<"application/x-www-form-urlencoded">> + }, + <<"body">> => #{ + <<"password">> => ?PH_PASSWORD + } + }, + credentials => #{ + password => <<255, 255, 255>> + }, + result => {ok, #{is_superuser => false, client_attrs => #{}}} + }, + %% custom headers #{ handler => fun(Req0, State) -> diff --git a/apps/emqx_auth_mongodb/src/emqx_authn_mongodb.erl b/apps/emqx_auth_mongodb/src/emqx_authn_mongodb.erl index 75a474c0c..0910d9e02 100644 --- a/apps/emqx_auth_mongodb/src/emqx_authn_mongodb.erl +++ b/apps/emqx_auth_mongodb/src/emqx_authn_mongodb.erl @@ -68,7 +68,7 @@ authenticate( resource_id := ResourceId } = State ) -> - Filter = emqx_authn_utils:render_deep(FilterTemplate, Credential), + Filter = emqx_authn_utils:render_deep_for_json(FilterTemplate, Credential), case emqx_resource:simple_sync_query(ResourceId, {find_one, Collection, Filter, #{}}) of {ok, undefined} -> ignore; diff --git a/apps/emqx_utils/src/emqx_template.erl b/apps/emqx_utils/src/emqx_template.erl index 1383f90e1..85d2e16e8 100644 --- a/apps/emqx_utils/src/emqx_template.erl +++ b/apps/emqx_utils/src/emqx_template.erl @@ -65,7 +65,7 @@ -type accessor() :: [binary()]. -type varname() :: string(). --type scalar() :: atom() | unicode:chardata() | number(). +-type scalar() :: atom() | unicode:chardata() | binary() | number(). -type binding() :: scalar() | list(scalar()) | bindings(). -type bindings() :: #{atom() | binary() => binding()}. @@ -346,7 +346,7 @@ render_deep({tuple, Template}, Context, Opts) when is_list(Template) -> {list_to_tuple(Term), Errors}; render_deep(Template, Context, Opts) when is_list(Template) -> {String, Errors} = render(Template, Context, Opts), - {unicode:characters_to_binary(String), Errors}; + {character_segments_to_binary(String), Errors}; render_deep(Term, _Bindings, _Opts) -> {Term, []}. @@ -424,3 +424,21 @@ to_string(List) when is_list(List) -> true -> List; false -> emqx_utils_json:encode(List) end. + +character_segments_to_binary(StringSegments) -> + ct:print("characters_to_binary: ~p~n", [StringSegments]), + iolist_to_binary( + lists:map( + fun + ($$) -> + $$; + (Bin) when is_binary(Bin) -> Bin; + (Chars) when is_list(Chars) -> + case unicode:characters_to_binary(Chars) of + Bin when is_binary(Bin) -> Bin; + _ -> emqx_utils_json:encode(Chars) + end + end, + StringSegments + ) + ).