From 0b4600c293eed4b4db65a4373fcd0b48f596caa8 Mon Sep 17 00:00:00 2001 From: Ilya Averyanov Date: Wed, 18 Oct 2023 15:46:43 +0300 Subject: [PATCH] feat(auth): improve redis command parsing and validation --- .../src/emqx_auth_redis_validations.erl | 71 ++++++++++ apps/emqx_auth_redis/src/emqx_authn_redis.erl | 67 +++++---- .../src/emqx_authn_redis_schema.erl | 2 +- apps/emqx_auth_redis/src/emqx_authz_redis.erl | 31 ++++- .../test/emqx_authn_redis_SUITE.erl | 17 ++- .../test/emqx_authz_redis_SUITE.erl | 22 ++- apps/emqx_redis/src/emqx_redis_command.erl | 129 ++++++++++++++++++ apps/emqx_redis/test/emqx_redis_SUITE.erl | 34 ++--- .../test/emqx_redis_command_SUITE.erl | 76 +++++++++++ .../test/props/prop_emqx_redis_command.erl | 31 +++++ .../test/emqx_telemetry_SUITE.erl | 2 +- changes/ce/feat-11790.en.md | 3 + 12 files changed, 422 insertions(+), 63 deletions(-) create mode 100644 apps/emqx_auth_redis/src/emqx_auth_redis_validations.erl create mode 100644 apps/emqx_redis/src/emqx_redis_command.erl create mode 100644 apps/emqx_redis/test/emqx_redis_command_SUITE.erl create mode 100644 apps/emqx_redis/test/props/prop_emqx_redis_command.erl create mode 100644 changes/ce/feat-11790.en.md diff --git a/apps/emqx_auth_redis/src/emqx_auth_redis_validations.erl b/apps/emqx_auth_redis/src/emqx_auth_redis_validations.erl new file mode 100644 index 000000000..e94b67c40 --- /dev/null +++ b/apps/emqx_auth_redis/src/emqx_auth_redis_validations.erl @@ -0,0 +1,71 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2020-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_auth_redis_validations). + +-export([ + validate_command/2 +]). + +validate_command([], _Command) -> + ok; +validate_command([Validation | Rest], Command) -> + case validate(Validation, Command) of + ok -> + validate_command(Rest, Command); + {error, _} = Error -> + Error + end. + +validate(not_empty, []) -> + {error, empty_command}; +validate(not_empty, _) -> + ok; +validate({command_name, AllowedNames}, [Name | _]) -> + IsAllowed = lists:any( + fun(AllowedName) -> + string:equal(AllowedName, Name, true, none) + end, + AllowedNames + ), + case IsAllowed of + true -> + ok; + false -> + {error, {invalid_command_name, Name}} + end; +validate({command_name, _}, _) -> + {error, invalid_command_name}; +validate({allowed_fields, AllowedFields}, [_CmdName, _CmdKey | Args]) -> + Unknown = lists:filter(fun(Arg) -> not lists:member(Arg, AllowedFields) end, Args), + case Unknown of + [] -> + ok; + _ -> + {error, {unknown_fields, Unknown}} + end; +validate({allowed_fields, _}, _) -> + ok; +validate({required_field_one_of, Required}, [_CmdName, _CmdKey | Args]) -> + HasRequired = lists:any(fun(Field) -> lists:member(Field, Args) end, Required), + case HasRequired of + true -> + ok; + false -> + {error, {missing_required_field, Required}} + end; +validate({required_field_one_of, Required}, _) -> + {error, {missing_required_field, Required}}. diff --git a/apps/emqx_auth_redis/src/emqx_authn_redis.erl b/apps/emqx_auth_redis/src/emqx_authn_redis.erl index 960308ac9..b7324e251 100644 --- a/apps/emqx_auth_redis/src/emqx_authn_redis.erl +++ b/apps/emqx_auth_redis/src/emqx_authn_redis.erl @@ -118,54 +118,51 @@ authenticate( parse_config( #{ - cmd := Cmd, + cmd := CmdStr, password_hash_algorithm := Algorithm } = Config ) -> - try - NCmd = parse_cmd(Cmd), - ok = emqx_authn_password_hashing:init(Algorithm), - ok = emqx_authn_utils:ensure_apps_started(Algorithm), - State = maps:with([password_hash_algorithm, salt_position], Config), - {Config, State#{cmd => NCmd}} - catch - error:{unsupported_cmd, _Cmd} -> - {error, {unsupported_cmd, Cmd}}; - error:missing_password_hash -> - {error, missing_password_hash}; - error:{unsupported_fields, Fields} -> - {error, {unsupported_fields, Fields}} + case parse_cmd(CmdStr) of + {ok, Cmd} -> + ok = emqx_authn_password_hashing:init(Algorithm), + ok = emqx_authn_utils:ensure_apps_started(Algorithm), + State = maps:with([password_hash_algorithm, salt_position], Config), + {Config, State#{cmd => Cmd}}; + {error, _} = Error -> + Error end. -%% Only support HGET and HMGET -parse_cmd(Cmd) -> - case string:tokens(Cmd, " ") of - [Command, Key, Field | Fields] when Command =:= "HGET" orelse Command =:= "HMGET" -> - NFields = [Field | Fields], - check_fields(NFields), - KeyTemplate = emqx_authn_utils:parse_str(list_to_binary(Key)), - {Command, KeyTemplate, NFields}; - _ -> - error({unsupported_cmd, Cmd}) +parse_cmd(CmdStr) -> + case emqx_redis_command:split(CmdStr) of + {ok, Cmd} -> + case validate_cmd(Cmd) of + ok -> + [CommandName, Key | Fields] = Cmd, + {ok, {CommandName, emqx_authn_utils:parse_str(Key), Fields}}; + {error, _} = Error -> + Error + end; + {error, _} = Error -> + Error end. -check_fields(Fields) -> - HasPassHash = lists:member("password_hash", Fields) orelse lists:member("password", Fields), - KnownFields = ["password_hash", "password", "salt", "is_superuser"], - UnknownFields = [F || F <- Fields, not lists:member(F, KnownFields)], - - case {HasPassHash, UnknownFields} of - {true, []} -> ok; - {true, _} -> error({unsupported_fields, UnknownFields}); - {false, _} -> error(missing_password_hash) - end. +validate_cmd(Cmd) -> + emqx_auth_redis_validations:validate_command( + [ + not_empty, + {command_name, [<<"hget">>, <<"hmget">>]}, + {allowed_fields, [<<"password_hash">>, <<"password">>, <<"salt">>, <<"is_superuser">>]}, + {required_field_one_of, [<<"password_hash">>, <<"password">>]} + ], + Cmd + ). merge(Fields, Value) when not is_list(Value) -> merge(Fields, [Value]); merge(Fields, Values) -> maps:from_list( [ - {list_to_binary(K), V} + {K, V} || {K, V} <- lists:zip(Fields, Values), V =/= undefined ] ). diff --git a/apps/emqx_auth_redis/src/emqx_authn_redis_schema.erl b/apps/emqx_auth_redis/src/emqx_authn_redis_schema.erl index 4f1b63633..7b5794c48 100644 --- a/apps/emqx_auth_redis/src/emqx_authn_redis_schema.erl +++ b/apps/emqx_auth_redis/src/emqx_authn_redis_schema.erl @@ -85,7 +85,7 @@ common_fields() -> {password_hash_algorithm, fun emqx_authn_password_hashing:type_ro/1} ] ++ emqx_authn_schema:common_fields(). -cmd(type) -> string(); +cmd(type) -> binary(); cmd(desc) -> ?DESC(?FUNCTION_NAME); cmd(required) -> true; cmd(_) -> undefined. diff --git a/apps/emqx_auth_redis/src/emqx_authz_redis.erl b/apps/emqx_auth_redis/src/emqx_authz_redis.erl index be83223e4..9b69f508a 100644 --- a/apps/emqx_auth_redis/src/emqx_authz_redis.erl +++ b/apps/emqx_auth_redis/src/emqx_authz_redis.erl @@ -47,15 +47,13 @@ description() -> "AuthZ with Redis". create(#{cmd := CmdStr} = Source) -> - Cmd = tokens(CmdStr), + CmdTemplate = parse_cmd(CmdStr), ResourceId = emqx_authz_utils:make_resource_id(?MODULE), - CmdTemplate = emqx_authz_utils:parse_deep(Cmd, ?PLACEHOLDERS), {ok, _Data} = emqx_authz_utils:create_resource(ResourceId, emqx_redis, Source), Source#{annotations => #{id => ResourceId}, cmd_template => CmdTemplate}. update(#{cmd := CmdStr} = Source) -> - Cmd = tokens(CmdStr), - CmdTemplate = emqx_authz_utils:parse_deep(Cmd, ?PLACEHOLDERS), + CmdTemplate = parse_cmd(CmdStr), case emqx_authz_utils:update_resource(emqx_redis, Source) of {error, Reason} -> error({load_config_error, Reason}); @@ -131,9 +129,28 @@ compile_rule(RuleBin, TopicFilterRaw) -> error(Reason) end. -tokens(Query) -> - Tokens = binary:split(Query, <<" ">>, [global]), - [Token || Token <- Tokens, size(Token) > 0]. +parse_cmd(Query) -> + case emqx_redis_command:split(Query) of + {ok, Cmd} -> + ok = validate_cmd(Cmd), + emqx_authz_utils:parse_deep(Cmd, ?PLACEHOLDERS); + {error, Reason} -> + error({invalid_redis_cmd, Reason, Query}) + end. + +validate_cmd(Cmd) -> + case + emqx_auth_redis_validations:validate_command( + [ + not_empty, + {command_name, [<<"hmget">>, <<"hgetall">>]} + ], + Cmd + ) + of + ok -> ok; + {error, Reason} -> error({invalid_redis_cmd, Reason, Cmd}) + end. parse_rule(<<"publish">>) -> #{<<"action">> => <<"publish">>}; diff --git a/apps/emqx_auth_redis/test/emqx_authn_redis_SUITE.erl b/apps/emqx_auth_redis/test/emqx_authn_redis_SUITE.erl index b3f4a15a3..081c4e641 100644 --- a/apps/emqx_auth_redis/test/emqx_authn_redis_SUITE.erl +++ b/apps/emqx_auth_redis/test/emqx_authn_redis_SUITE.erl @@ -336,7 +336,22 @@ user_seeds() -> config_params => #{}, result => {ok, #{is_superuser => true}} }, - + #{ + data => #{ + password_hash => <<"plainsalt">>, + salt => <<"salt">>, + is_superuser => <<"1">> + }, + credentials => #{ + username => <<"plain">>, + password => <<"plain">> + }, + key => <<"mqtt_user:plain">>, + config_params => #{ + <<"cmd">> => <<"HmGeT mqtt_user:${username} password_hash salt is_superuser">> + }, + result => {ok, #{is_superuser => true}} + }, #{ data => #{ password_hash => <<"9b4d0c43d206d48279e69b9ad7132e22">>, diff --git a/apps/emqx_auth_redis/test/emqx_authz_redis_SUITE.erl b/apps/emqx_auth_redis/test/emqx_authz_redis_SUITE.erl index 962333cd2..1c52cee17 100644 --- a/apps/emqx_auth_redis/test/emqx_authz_redis_SUITE.erl +++ b/apps/emqx_auth_redis/test/emqx_authz_redis_SUITE.erl @@ -112,7 +112,9 @@ t_create_invalid_config(_Config) -> ). t_redis_error(_Config) -> - ok = setup_config(#{<<"cmd">> => <<"INVALID COMMAND">>}), + q([<<"SET">>, <<"notahash">>, <<"stringvalue">>]), + + ok = setup_config(#{<<"cmd">> => <<"HGETALL notahash">>}), ClientInfo = emqx_authz_test_lib:base_client_info(), @@ -121,6 +123,24 @@ t_redis_error(_Config) -> emqx_access_control:authorize(ClientInfo, ?AUTHZ_SUBSCRIBE, <<"a">>) ). +t_invalid_command(_Config) -> + Config = raw_redis_authz_config(), + + ?assertMatch( + {error, _}, + emqx_authz:update(?CMD_REPLACE, [Config#{<<"cmd">> => <<"HGET key">>}]) + ), + + ?assertMatch( + {ok, _}, + emqx_authz:update(?CMD_REPLACE, [Config#{<<"cmd">> => <<"HGETALL key">>}]) + ), + + ?assertMatch( + {error, _}, + emqx_authz:update({?CMD_REPLACE, redis}, Config#{<<"cmd">> => <<"HGET key">>}) + ). + %%------------------------------------------------------------------------------ %% Cases %%------------------------------------------------------------------------------ diff --git a/apps/emqx_redis/src/emqx_redis_command.erl b/apps/emqx_redis/src/emqx_redis_command.erl new file mode 100644 index 000000000..7de80e1fa --- /dev/null +++ b/apps/emqx_redis/src/emqx_redis_command.erl @@ -0,0 +1,129 @@ +%%-------------------------------------------------------------------- +%% 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. +%%-------------------------------------------------------------------- + +%% @doc `split/1` function reimplements the one used by Redis itself for `redis-cli`. +%% See `sdssplitargs` function, https://github.com/redis/redis/blob/unstable/src/sds.c. + +-module(emqx_redis_command). + +-export([split/1]). + +-define(CH_SPACE, 32). +-define(CH_N, 10). +-define(CH_R, 13). +-define(CH_T, 9). +-define(CH_B, 8). +-define(CH_A, 11). + +-define(IS_CH_HEX_DIGIT(C), + (((C >= $a) andalso (C =< $f)) orelse + ((C >= $A) andalso (C =< $F)) orelse + ((C >= $0) andalso (C =< $9))) +). +-define(IS_CH_SPACE(C), + (C =:= ?CH_SPACE orelse + C =:= ?CH_N orelse + C =:= ?CH_R orelse + C =:= ?CH_T orelse + C =:= ?CH_B orelse + C =:= ?CH_A) +). + +split(Line) when is_binary(Line) -> + case split(binary_to_list(Line)) of + {ok, Args} -> + {ok, [list_to_binary(Arg) || Arg <- Args]}; + {error, _} = Error -> + Error + end; +split(Line) -> + split(Line, []). + +split([], Acc) -> + {ok, lists:reverse(Acc)}; +split([C | Rest] = Line, Acc) -> + case ?IS_CH_SPACE(C) of + true -> split(Rest, Acc); + false -> split_noq([], Line, Acc) + end. + +hex_digit_to_int(C) when (C >= $a) andalso (C =< $f) -> 10 + C - $a; +hex_digit_to_int(C) when (C >= $A) andalso (C =< $F) -> 10 + C - $A; +hex_digit_to_int(C) when (C >= $0) andalso (C =< $9) -> C - $0. + +maybe_special_char($n) -> ?CH_N; +maybe_special_char($r) -> ?CH_R; +maybe_special_char($t) -> ?CH_T; +maybe_special_char($b) -> ?CH_B; +maybe_special_char($a) -> ?CH_A; +maybe_special_char(C) -> C. + +%% Inside double quotes +split_inq(CurAcc, Line, Acc) -> + case Line of + [$\\, $x, HD1, HD2 | LineRest] when ?IS_CH_HEX_DIGIT(HD1) andalso ?IS_CH_HEX_DIGIT(HD2) -> + C = hex_digit_to_int(HD1) * 16 + hex_digit_to_int(HD2), + NewCurAcc = [C | CurAcc], + split_inq(NewCurAcc, LineRest, Acc); + [$\\, SC | LineRest] -> + C = maybe_special_char(SC), + NewCurAcc = [C | CurAcc], + split_inq(NewCurAcc, LineRest, Acc); + [$", C | _] when not ?IS_CH_SPACE(C) -> + {error, trailing_after_quote}; + [$" | LineRest] -> + split(LineRest, [lists:reverse(CurAcc) | Acc]); + [] -> + {error, unterminated_quote}; + [C | LineRest] -> + NewCurAcc = [C | CurAcc], + split_inq(NewCurAcc, LineRest, Acc) + end. + +%% Inside single quotes +split_insq(CurAcc, Line, Acc) -> + case Line of + [$\\, $' | LineRest] -> + NewCurAcc = [$' | CurAcc], + split_insq(NewCurAcc, LineRest, Acc); + [$', C | _] when not ?IS_CH_SPACE(C) -> + {error, trailing_after_single_quote}; + [$' | LineRest] -> + split(LineRest, [lists:reverse(CurAcc) | Acc]); + [] -> + {error, unterminated_single_quote}; + [C | LineRest] -> + NewCurAcc = [C | CurAcc], + split_insq(NewCurAcc, LineRest, Acc) + end. + +%% Outside quotes +split_noq(CurAcc, Line, Acc) -> + case Line of + [C | LineRest] when + ?IS_CH_SPACE(C); C =:= ?CH_N; C =:= ?CH_R; C =:= ?CH_T + -> + split(LineRest, [lists:reverse(CurAcc) | Acc]); + [] -> + split([], [lists:reverse(CurAcc) | Acc]); + [$' | LineRest] -> + split_insq(CurAcc, LineRest, Acc); + [$" | LineRest] -> + split_inq(CurAcc, LineRest, Acc); + [C | LineRest] -> + NewCurAcc = [C | CurAcc], + split_noq(NewCurAcc, LineRest, Acc) + end. diff --git a/apps/emqx_redis/test/emqx_redis_SUITE.erl b/apps/emqx_redis/test/emqx_redis_SUITE.erl index e03b05921..8fcbf2b63 100644 --- a/apps/emqx_redis/test/emqx_redis_SUITE.erl +++ b/apps/emqx_redis/test/emqx_redis_SUITE.erl @@ -1,17 +1,17 @@ -% %%-------------------------------------------------------------------- -% %% Copyright (c) 2020-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. -% %%-------------------------------------------------------------------- +%%-------------------------------------------------------------------- +%% Copyright (c) 2020-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_redis_SUITE). @@ -190,9 +190,9 @@ perform_lifecycle_check(ResourceId, InitialConfig, RedisCommand) -> % Should not even be able to get the resource data out of ets now unlike just stopping. ?assertEqual({error, not_found}, emqx_resource:get_instance(ResourceId)). -% %%------------------------------------------------------------------------------ -% %% Helpers -% %%------------------------------------------------------------------------------ +%%------------------------------------------------------------------------------ +%% Helpers +%%------------------------------------------------------------------------------ redis_config_single() -> redis_config_base("single", "server"). diff --git a/apps/emqx_redis/test/emqx_redis_command_SUITE.erl b/apps/emqx_redis/test/emqx_redis_command_SUITE.erl new file mode 100644 index 000000000..1c6f87eff --- /dev/null +++ b/apps/emqx_redis/test/emqx_redis_command_SUITE.erl @@ -0,0 +1,76 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2020-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_redis_command_SUITE). + +-compile(nowarn_export_all). +-compile(export_all). + +-include_lib("eunit/include/eunit.hrl"). + +all() -> + emqx_common_test_helpers:all(?MODULE). + +t_split_ok(_Config) -> + ?assertEqual( + {ok, [<<"ab">>, <<"cd">>, <<"ef">>]}, + emqx_redis_command:split(<<" \"ab\" 'cd' ef ">>) + ), + ?assertEqual( + {ok, [<<"ab">>, <<"cd">>, <<"ef">>]}, + emqx_redis_command:split(<<" ab\tcd ef">>) + ), + ?assertEqual( + {ok, [<<"abc'd">>, <<"ef">>]}, + emqx_redis_command:split(<<"ab\"c'd\" ef">>) + ), + ?assertEqual( + {ok, [<<"abc\"d">>, <<"ef">>]}, + emqx_redis_command:split(<<"ab'c\"d' ef">>) + ), + ?assertEqual( + {ok, [<<"IJK">>, <<"\\x49\\x4a\\x4B">>]}, + emqx_redis_command:split(<<"\"\\x49\\x4a\\x4B\" \\x49\\x4a\\x4B">>) + ), + ?assertEqual( + {ok, [<<"x\t\n\r\b\v">>]}, + emqx_redis_command:split(<<"\"\\x\\t\\n\\r\\b\\a\"">>) + ), + ?assertEqual( + {ok, [<<"abc\'d">>, <<"ef">>]}, + emqx_redis_command:split(<<"'abc\\'d' ef">>) + ), + ?assertEqual( + {ok, [<<>>, <<>>]}, + emqx_redis_command:split(<<" '' \"\" ">>) + ). + +t_split_error(_Config) -> + ?assertEqual( + {error, trailing_after_quote}, + emqx_redis_command:split(<<"\"a\"b">>) + ), + ?assertEqual( + {error, unterminated_quote}, + emqx_redis_command:split(<<"\"ab">>) + ), + ?assertEqual( + {error, trailing_after_single_quote}, + emqx_redis_command:split(<<"'a'b'c">>) + ), + ?assertEqual( + {error, unterminated_single_quote}, + emqx_redis_command:split(<<"'ab">>) + ). diff --git a/apps/emqx_redis/test/props/prop_emqx_redis_command.erl b/apps/emqx_redis/test/props/prop_emqx_redis_command.erl new file mode 100644 index 000000000..dc7ce2ada --- /dev/null +++ b/apps/emqx_redis/test/props/prop_emqx_redis_command.erl @@ -0,0 +1,31 @@ +%%-------------------------------------------------------------------- +%% 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(prop_emqx_redis_command). + +-include_lib("proper/include/proper.hrl"). + +%%-------------------------------------------------------------------- +%% Properties +%%-------------------------------------------------------------------- + +prop_split() -> + ?FORALL( + Cmd, + binary(), + %% Should terminate and not crash + is_tuple(emqx_redis_command:split(Cmd)) + ). diff --git a/apps/emqx_telemetry/test/emqx_telemetry_SUITE.erl b/apps/emqx_telemetry/test/emqx_telemetry_SUITE.erl index 92839d06a..47d31c4de 100644 --- a/apps/emqx_telemetry/test/emqx_telemetry_SUITE.erl +++ b/apps/emqx_telemetry/test/emqx_telemetry_SUITE.erl @@ -738,7 +738,7 @@ create_authn(ChainName, redis) -> backend => redis, enable => true, user_id_type => username, - cmd => "HMGET mqtt_user:${username} password_hash salt is_superuser", + cmd => <<"HMGET mqtt_user:${username} password_hash salt is_superuser">>, password_hash_algorithm => #{ name => plain, salt_position => suffix diff --git a/changes/ce/feat-11790.en.md b/changes/ce/feat-11790.en.md new file mode 100644 index 000000000..c2ceee216 --- /dev/null +++ b/changes/ce/feat-11790.en.md @@ -0,0 +1,3 @@ +Added validation of Redis commands configured in Redis authorization source. +Also, improved Redis command parsing in authentication and authorization +so that it is `redis-cli` compatible and supports quoted arguments.