From 6a224e307e83862380a263d21a6ac39299a3402a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 26 May 2022 13:04:17 +0200 Subject: [PATCH] WIP --- src/config/mongoose_config_parser_toml.erl | 3 - src/config/mongoose_config_spec.erl | 23 +++---- src/ejabberd_commands.erl | 73 +++++++++------------- src/ejabberd_ctl.erl | 13 ++-- test/commands_SUITE.erl | 23 ++++--- test/common/config_parser_helper.erl | 15 +++-- test/config_parser_SUITE.erl | 31 ++++----- test/mongoose_config_SUITE.erl | 2 +- 8 files changed, 77 insertions(+), 106 deletions(-) diff --git a/src/config/mongoose_config_parser_toml.erl b/src/config/mongoose_config_parser_toml.erl index be078aaaf67..31f62f2910b 100644 --- a/src/config/mongoose_config_parser_toml.erl +++ b/src/config/mongoose_config_parser_toml.erl @@ -253,9 +253,6 @@ wrap(_Path, V, item) -> [V]; wrap(_Path, _V, remove) -> []; -wrap([Key|_], V, prepend_key) -> - L = [b2a(Key) | tuple_to_list(V)], - [list_to_tuple(L)]; wrap(_Path, V, none) when is_list(V) -> V. diff --git a/src/config/mongoose_config_spec.erl b/src/config/mongoose_config_spec.erl index 698b5eae25b..90e6c245b35 100644 --- a/src/config/mongoose_config_spec.erl +++ b/src/config/mongoose_config_spec.erl @@ -13,7 +13,6 @@ -export([process_root/1, process_host/1, process_general/1, - process_ctl_access_rule/1, process_listener/2, process_c2s_tls/1, process_fast_tls/1, @@ -60,8 +59,7 @@ | item % [Value] | remove % [] - the item is ignored | none % just Value - injects elements of Value into the parent section/list - | {kv, NewKey :: term()} % [{NewKey, Value}] - replaces the key with NewKey - | prepend_key. % [{Key, V1, ..., Vn}] when Value = {V1, ..., Vn} + | {kv, NewKey :: term()}. % [{NewKey, Value}] - replaces the key with NewKey %% This option allows to put list/section items in a map -type format_items() :: @@ -194,7 +192,6 @@ general() -> wrap = host_config}, <<"mongooseimctl_access_commands">> => #section{ items = #{default => ctl_access_rule()}, - format_items = list, wrap = global_config}, <<"routing_modules">> => #list{items = #option{type = atom, validate = module}, @@ -221,23 +218,20 @@ general_defaults() -> <<"all_metrics_are_global">> => false, <<"sm_backend">> => mnesia, <<"rdbms_server_type">> => generic, - <<"mongooseimctl_access_commands">> => [], + <<"mongooseimctl_access_commands">> => #{}, <<"routing_modules">> => mongoose_router:default_routing_modules(), <<"replaced_wait_timeout">> => 2000, <<"hide_service_name">> => false}. ctl_access_rule() -> #section{ - items = #{<<"commands">> => #list{items = #option{type = string}}, - <<"argument_restrictions">> => #section{ - items = #{default => #option{type = string}}, - format_items = list - } + items = #{<<"commands">> => #list{items = #option{type = atom, + validate = non_empty}}, + <<"argument_restrictions">> => + #section{items = #{default => #option{type = string}}} }, defaults = #{<<"commands">> => all, - <<"argument_restrictions">> => []}, - process = fun ?MODULE:process_ctl_access_rule/1, - wrap = prepend_key + <<"argument_restrictions">> => #{}} }. %% path: general.domain_certfile @@ -975,9 +969,6 @@ is_host_type_item({{_, HostType}, _}, HostTypes) -> is_host_type_item(_, _) -> false. -process_ctl_access_rule(#{commands := Commands, argument_restrictions := ArgRestrictions}) -> - {Commands, ArgRestrictions}. - process_host(Host) -> Node = jid:nodeprep(Host), true = Node =/= error, diff --git a/src/ejabberd_commands.erl b/src/ejabberd_commands.erl index 1b08bf55359..5077a47d9f2 100644 --- a/src/ejabberd_commands.erl +++ b/src/ejabberd_commands.erl @@ -231,20 +231,22 @@ -type cmd_error() :: command_unknown | account_unprivileged | invalid_account_data | no_auth_provided. --type access_cmd() :: {Access :: atom(), - CommandNames :: [atom()], - Arguments :: [term()] - }. +-type access_commands() :: #{acl:rule_name() => command_rules()}. +-type command_rules() :: #{commands := all | [atom()], + argument_restrictions := argument_restrictions()}. + +%% Currently only string arguments can have restrictions +-type argument_restrictions() :: #{ArgName :: atom() => Value :: string()}. + -type list_cmd() :: {Name::atom(), Args::[aterm()], Desc::string()}. -export_type([rterm/0, aterm/0, cmd/0, auth/0, - access_cmd/0, + access_commands/0, list_cmd/0]). - init() -> case ets:info(ejabberd_commands) of undefined -> @@ -254,7 +256,6 @@ init() -> ok end. - %% @doc Register ejabberd commands. If a command is already registered, a %% warning is printed and the old command is preserved. -spec register_commands([cmd()]) -> ok. @@ -269,7 +270,6 @@ register_commands(Commands) -> end, Commands). - %% @doc Unregister ejabberd commands. -spec unregister_commands([cmd()]) -> ok. unregister_commands(Commands) -> @@ -279,7 +279,6 @@ unregister_commands(Commands) -> end, Commands). - %% @doc Get a list of all the available commands, arguments and description. -spec list_commands() -> [list_cmd()]. list_commands() -> @@ -290,7 +289,6 @@ list_commands() -> _ = '_'}), [{A, B, C} || [A, B, C] <- Commands]. - %% @doc Get the format of arguments and result of a command. -spec get_command_format(Name::atom()) -> {Args::[aterm()], Result::rterm()} | {error, command_unknown}. @@ -307,7 +305,6 @@ get_command_format(Name) -> {Args, Result} end. - %% @doc Get the definition record of a command. -spec get_command_definition(Name::atom()) -> cmd() | 'command_not_found'. get_command_definition(Name) -> @@ -316,16 +313,14 @@ get_command_definition(Name) -> [] -> command_not_found end. - %% @doc Execute a command. -spec execute_command(Name :: atom(), Arguments :: list() ) -> Result :: term() | {error, command_unknown}. execute_command(Name, Arguments) -> - execute_command([], noauth, Name, Arguments). - + execute_command(#{}, noauth, Name, Arguments). --spec execute_command(AccessCommands :: [access_cmd()], +-spec execute_command(AccessCommands :: access_commands(), Auth :: auth(), Name :: atom(), Arguments :: [term()] @@ -341,7 +336,6 @@ execute_command(AccessCommands, Auth, Name, Arguments) -> [] -> {error, command_unknown} end. - %% @private execute_command2(Command, Arguments) -> Module = Command#ejabberd_commands.module, @@ -352,7 +346,6 @@ execute_command2(Command, Arguments) -> command_args => Arguments}), apply(Module, Function, Arguments). - %% @doc Get all the tags and associated commands. -spec get_tags_commands() -> [{Tag::string(), [CommandName::string()]}]. get_tags_commands() -> @@ -381,42 +374,38 @@ get_tags_commands() -> CommandTags), orddict:to_list(Dict). - %% ----------------------------- %% Access verification %% ----------------------------- - %% @doc Check access is allowed to that command. %% At least one AccessCommand must be satisfied. %% May throw {error, account_unprivileged | invalid_account_data} --spec check_access_commands(AccessCommands :: [ access_cmd() ], +-spec check_access_commands(AccessCommands :: access_commands(), Auth :: auth(), Method :: atom(), Command :: tuple(), Arguments :: [any()] ) -> ok | none(). -check_access_commands([], _Auth, _Method, _Command, _Arguments) -> - ok; +check_access_commands(AccessCommands, _Auth, _Method, _Command, _Arguments) + when AccessCommands =:= #{} -> ok; check_access_commands(AccessCommands, Auth, Method, Command, Arguments) -> AccessCommandsAllowed = - lists:filter( - fun({Access, Commands, ArgumentRestrictions}) -> + maps:filter( + fun(Access, CommandSpec) -> case check_access(Access, Auth) of true -> - check_access_command(Commands, Command, ArgumentRestrictions, - Method, Arguments); + check_access_command(Command, CommandSpec, Method, Arguments); false -> false end end, AccessCommands), - case AccessCommandsAllowed of - [] -> throw({error, account_unprivileged}); - L when is_list(L) -> ok + case AccessCommandsAllowed =:= #{} of + true -> throw({error, account_unprivileged}); + false -> ok end. - %% @private %% May throw {error, invalid_account_data} -spec check_auth(auth()) -> {ok, jid:jid()} | no_return(). @@ -431,13 +420,11 @@ check_auth({User, Server, Password}) -> _ -> throw({error, invalid_account_data}) end. - -spec get_md5(iodata()) -> string(). get_md5(AccountPass) -> lists:flatten([io_lib:format("~.16B", [X]) || X <- binary_to_list(crypto:hash(md5, AccountPass))]). - -spec check_access(Access :: acl:rule_name(), Auth :: auth()) -> boolean(). check_access(all, _) -> true; @@ -453,29 +440,27 @@ check_access(Access, Auth) -> deny -> false end. - --spec check_access_command(_, tuple(), _, _, _) -> boolean(). -check_access_command(Commands, Command, ArgumentRestrictions, Method, Arguments) -> - case Commands==all orelse lists:member(Method, Commands) of +-spec check_access_command(cmd(), command_rules(), atom(), [any()]) -> boolean(). +check_access_command(Command, CommandRules, Method, Arguments) -> + #{commands := Commands, argument_restrictions := ArgumentRestrictions} = CommandRules, + case Commands == all orelse lists:member(Method, Commands) of true -> check_access_arguments(Command, ArgumentRestrictions, Arguments); false -> false end. - -spec check_access_arguments(Command :: cmd(), Restrictions :: [any()], Args :: [any()]) -> boolean(). check_access_arguments(Command, ArgumentRestrictions, Arguments) -> ArgumentsTagged = tag_arguments(Command#ejabberd_commands.args, Arguments), lists:all( - fun({ArgName, ArgAllowedValue}) -> - %% If the call uses the argument, check the value is acceptable - case lists:keysearch(ArgName, 1, ArgumentsTagged) of - {value, {ArgName, ArgValue}} -> ArgValue == ArgAllowedValue; - false -> true + fun({ArgName, ArgValue}) -> + case ArgumentRestrictions of + %% If there is a restriction, check the value is acceptable + #{ArgName := ArgAllowedValue} -> ArgValue =:= ArgAllowedValue; + #{} -> true end - end, ArgumentRestrictions). - + end, ArgumentsTagged). -spec tag_arguments(ArgsDefs :: [{atom(), integer() | string() | {_, _}}], Args :: [any()] ) -> [{_, _}]. diff --git a/src/ejabberd_ctl.erl b/src/ejabberd_ctl.erl index 6946163cf56..1804288f580 100644 --- a/src/ejabberd_ctl.erl +++ b/src/ejabberd_ctl.erl @@ -218,9 +218,8 @@ process(Args) -> Code. --spec process2(Args :: [string()], - AccessCommands :: [ejabberd_commands:access_cmd()] - ) -> {String::string(), Code::integer()}. +-spec process2(Args :: [string()], AccessCommands :: ejabberd_commands:access_commands()) -> + {String::string(), Code::integer()}. process2(["--auth", User, Server, Pass | Args], AccessCommands) -> process2(Args, {list_to_binary(User), list_to_binary(Server), list_to_binary(Pass)}, AccessCommands); @@ -245,7 +244,7 @@ process2(Args, Auth, AccessCommands) -> end. --spec get_accesscommands() -> [char() | tuple()]. +-spec get_accesscommands() -> ejabberd_commands:access_commands(). get_accesscommands() -> mongoose_config:get_opt(mongooseimctl_access_commands). @@ -256,7 +255,7 @@ get_accesscommands() -> -spec try_run_ctp(Args :: [string()], Auth :: ejabberd_commands:auth(), - AccessCommands :: [ejabberd_commands:access_cmd()] + AccessCommands :: ejabberd_commands:access_commands() ) -> string() | integer() | {string(), integer()} | {string(), wrong_command_arguments}. try_run_ctp(Args, Auth, AccessCommands) -> try mongoose_hooks:ejabberd_ctl_process(false, Args) of @@ -281,7 +280,7 @@ try_run_ctp(Args, Auth, AccessCommands) -> -spec try_call_command(Args :: [string()], Auth :: ejabberd_commands:auth(), - AccessCommands :: [ejabberd_commands:access_cmd()] + AccessCommands :: ejabberd_commands:access_commands() ) -> string() | integer() | {string(), integer()} | {string(), wrong_command_arguments}. try_call_command(Args, Auth, AccessCommands) -> try call_command(Args, Auth, AccessCommands) of @@ -297,7 +296,7 @@ try_call_command(Args, Auth, AccessCommands) -> -spec call_command(Args :: [string()], Auth :: ejabberd_commands:auth(), - AccessCommands :: [ejabberd_commands:access_cmd()] + AccessCommands :: ejabberd_commands:access_commands() ) -> string() | integer() | {string(), integer()} | {string(), wrong_command_arguments} | {error, command_unknown}. call_command([CmdString | Args], Auth, AccessCommands) -> diff --git a/test/commands_SUITE.erl b/test/commands_SUITE.erl index e3e890dfdcd..68f49b7d3e9 100644 --- a/test/commands_SUITE.erl +++ b/test/commands_SUITE.erl @@ -127,26 +127,28 @@ old_exec(_C) -> old_access_ctl(_C) -> %% with no auth method it is all fine - checkauth(true, [], noauth), + checkauth(true, #{}, noauth), %% noauth fails if first item is not 'all' (users) - checkauth(account_unprivileged, [{none, none, []}], noauth), + checkauth(account_unprivileged, #{none => command_rules(all)}, noauth), %% if here we allow all commands to noauth - checkauth(true, [{all, all, []}], noauth), + checkauth(true, #{all => command_rules(all)}, noauth), %% and here only command_one - checkauth(true, [{all, [command_one], []}], noauth), + checkauth(true, #{all => command_rules([command_one])}, noauth), %% so this'd fail - checkauth(account_unprivileged, [{all, [command_two], []}], noauth), + checkauth(account_unprivileged, #{all => command_rules([command_two])}, noauth), % now we provide a role name, this requires a user and triggers password and acl check % this fails because password is bad - checkauth(invalid_account_data, [{some_acl_role, [command_one], []}], {<<"zenek">>, <<"localhost">>, <<"bbb">>}), + checkauth(invalid_account_data, #{some_acl_role => command_rules([command_one])}, + {<<"zenek">>, <<"localhost">>, <<"bbb">>}), % this, because of acl - checkauth(account_unprivileged, [{some_acl_role, [command_one], []}], {<<"zenek">>, <<"localhost">>, <<"">>}), + checkauth(account_unprivileged, #{some_acl_role => command_rules([command_one])}, + {<<"zenek">>, <<"localhost">>, <<"">>}), % and this should work, because we define command_one as available to experts only, while acls in config % (see ggo/1) state that experts-only funcs are available to coders and managers, and zenek is a coder, gah. - checkauth(true, [{experts_only, [command_one], []}], {<<"zenek">>, <<"localhost">>, <<"">>}), + checkauth(true, #{experts_only => command_rules([command_one])}, + {<<"zenek">>, <<"localhost">>, <<"">>}), ok. - new_type_checker(_C) -> true = t_check_type({msg, binary}, <<"zzz">>), true = t_check_type({msg, integer}, 127), @@ -603,6 +605,9 @@ mc_holder() -> end, erlang:exit(Pid, kill). +command_rules(Commands) -> + #{commands => Commands, argument_restrictions => #{}}. + checkauth(true, AccessCommands, Auth) -> B = <<"bzzzz">>, B = ejabberd_commands:execute_command(AccessCommands, Auth, command_one, [B]); diff --git a/test/common/config_parser_helper.erl b/test/common/config_parser_helper.erl index 274ceeb52de..27e237b1360 100644 --- a/test/common/config_parser_helper.erl +++ b/test/common/config_parser_helper.erl @@ -17,7 +17,7 @@ options("host_types") -> {language, <<"en">>}, {listen, []}, {loglevel, warning}, - {mongooseimctl_access_commands, []}, + {mongooseimctl_access_commands, #{}}, {outgoing_pools, []}, {rdbms_server_type, generic}, {registration_timeout, 600}, @@ -78,7 +78,8 @@ options("miscellaneous") -> })]}, {loglevel, warning}, {mongooseimctl_access_commands, - [{local, ["join_cluster"], [{node, "mongooseim@prime"}]}]}, + #{local => #{commands => [join_cluster], + argument_restrictions => #{node => "mongooseim@prime"}}}}, {outgoing_pools, []}, {rdbms_server_type, mssql}, {registration_timeout, 600}, @@ -109,7 +110,7 @@ options("modules") -> {language, <<"en">>}, {listen, []}, {loglevel, warning}, - {mongooseimctl_access_commands, []}, + {mongooseimctl_access_commands, #{}}, {outgoing_pools, []}, {rdbms_server_type, generic}, {registration_timeout, 600}, @@ -240,7 +241,7 @@ options("mongooseim-pgsql") -> ]}, {loglevel, warning}, {max_fsm_queue, 1000}, - {mongooseimctl_access_commands, []}, + {mongooseimctl_access_commands, #{}}, {outgoing_pools, lists:map( fun pool_config/1, @@ -318,7 +319,7 @@ options("outgoing_pools") -> {language, <<"en">>}, {listen, []}, {loglevel, warning}, - {mongooseimctl_access_commands, []}, + {mongooseimctl_access_commands, #{}}, {outgoing_pools, lists:map( fun pool_config/1, @@ -389,7 +390,7 @@ options("s2s_only") -> {language, <<"en">>}, {listen, []}, {loglevel, warning}, - {mongooseimctl_access_commands, []}, + {mongooseimctl_access_commands, #{}}, {outgoing_pools, []}, {rdbms_server_type, generic}, {registration_timeout, 600}, @@ -1072,6 +1073,8 @@ extra_service_listener_config() -> hidden_components => false, conflict_behaviour => disconnect}. +default_config([general, mongooseimctl_access_commands, _Key]) -> + #{commands => all, argument_restrictions => #{}}; default_config([listen, http]) -> (common_listener_config())#{module => ejabberd_cowboy, transport => default_config([listen, http, transport]), diff --git a/test/config_parser_SUITE.erl b/test/config_parser_SUITE.erl index e45369e9222..20a172b8c0b 100644 --- a/test/config_parser_SUITE.erl +++ b/test/config_parser_SUITE.erl @@ -408,26 +408,17 @@ route_subdomains(_Config) -> ?errh(#{<<"general">> => #{<<"route_subdomains">> => <<"c2s">>}}). mongooseimctl_access_commands(_Config) -> - ?cfg(mongooseimctl_access_commands, [], #{}), % default - AccessRule = #{<<"commands">> => [<<"join_cluster">>], - <<"argument_restrictions">> => #{<<"node">> => <<"mim1@host1">>}}, - ?cfg(mongooseimctl_access_commands, [{local, ["join_cluster"], [{node, "mim1@host1"}]}], - #{<<"general">> => #{<<"mongooseimctl_access_commands">> => - #{<<"local">> => AccessRule}}}), - ?cfg(mongooseimctl_access_commands, [{local, all, [{node, "mim1@host1"}]}], - #{<<"general">> => #{<<"mongooseimctl_access_commands">> => - #{<<"local">> => maps:remove(<<"commands">>, AccessRule)}}}), - ?cfg(mongooseimctl_access_commands, [{local, ["join_cluster"], []}], - #{<<"general">> => #{<<"mongooseimctl_access_commands">> => - #{<<"local">> => maps:remove(<<"argument_restrictions">>, - AccessRule)}}}), - ?cfg(mongooseimctl_access_commands, [{local, all, []}], - #{<<"general">> => #{<<"mongooseimctl_access_commands">> => #{<<"local">> => #{}}}}), - ?err(#{<<"general">> => #{<<"mongooseimctl_access_commands">> => - #{<<"local">> => #{<<"commands">> => <<"all">>}}}}), - ?err(#{<<"general">> => #{<<"mongooseimctl_access_commands">> => - #{<<"local">> => #{<<"argument_restrictions">> => - [<<"none">>]}}}}). + ?cfg(mongooseimctl_access_commands, #{}, #{}), % default + P = [mongooseimctl_access_commands, local], + T = fun(Opts) -> + #{<<"general">> => #{<<"mongooseimctl_access_commands">> => #{<<"local">> => Opts}}} + end, + ?cfg(P, default_config([general, mongooseimctl_access_commands, local]), T(#{})), + ?cfg(P ++ [commands], [join_cluster], T(#{<<"commands">> => [<<"join_cluster">>]})), + ?cfg(P ++ [argument_restrictions], #{node => "mim1@host1"}, + T(#{<<"argument_restrictions">> => #{<<"node">> => <<"mim1@host1">>}})), + ?err(T(#{<<"commands">> => [<<>>]})), + ?err(T(#{<<"argument_restrictions">> => #{<<"node">> => 1}})). routing_modules(_Config) -> ?cfg(routing_modules, mongoose_router:default_routing_modules(), #{}), % default diff --git a/test/mongoose_config_SUITE.erl b/test/mongoose_config_SUITE.erl index c269d134fa5..65915c35d10 100644 --- a/test/mongoose_config_SUITE.erl +++ b/test/mongoose_config_SUITE.erl @@ -180,7 +180,7 @@ minimal_config_opts() -> {language, <<"en">>}, {listen, []}, {loglevel, warning}, - {mongooseimctl_access_commands, []}, + {mongooseimctl_access_commands, #{}}, {outgoing_pools, []}, {rdbms_server_type, generic}, {registration_timeout, 600},