Skip to content

Commit

Permalink
Merge pull request #12931 from zmstone/0425-fix-acl.conf-topic-templa…
Browse files Browse the repository at this point in the history
…te-render-fialure-handling

0425 fix acl.conf topic template render fialure handling
  • Loading branch information
zmstone committed Apr 25, 2024
2 parents 47c0bdf + 0192314 commit 6af651c
Show file tree
Hide file tree
Showing 4 changed files with 179 additions and 26 deletions.
9 changes: 9 additions & 0 deletions apps/emqx/src/emqx_channel.erl
Expand Up @@ -1638,6 +1638,15 @@ initialize_client_attrs(Inits, ClientInfo) ->
fun(#{expression := Variform, set_as_attr := Name}, Acc) ->
Attrs = maps:get(client_attrs, ClientInfo, #{}),
case emqx_variform:render(Variform, ClientInfo) of
{ok, <<>>} ->
?SLOG(
debug,
#{
msg => "client_attr_rednered_to_empty_string",
set_as_attr => Name
}
),
Acc;
{ok, Value} ->
?SLOG(
debug,
Expand Down
137 changes: 114 additions & 23 deletions apps/emqx_auth/etc/acl.conf
@@ -1,32 +1,123 @@
%%--------------------------------------------------------------------
%% -type(ipaddr() :: {ipaddr, string()}).
%%
%% -type(ipaddrs() :: {ipaddrs, [string()]}).
%%
%% -type(username() :: {user | username, string()} | {user | username, {re, regex()}}).
%%-------------- Default ACL rules -------------------------------------------------------

{allow, {username, {re, "^dashboard$"}}, subscribe, ["$SYS/#"]}.

{allow, {ipaddr, "127.0.0.1"}, all, ["$SYS/#", "#"]}.

{deny, all, subscribe, ["$SYS/#", {eq, "#"}]}.

{allow, all}.
%% NOTE! when deploy in production:
%% - Change the last rule to `{deny, all}.`
%% - Set config `authorization.no_match = deny`

%% See docs below
%%
%% -type(clientid() :: {client | clientid, string()} | {client | clientid, {re, regex()}}).
%% ------------ The formal spec ----------------------------------------------------------
%%
%% -type(who() :: ipaddr() | ipaddrs() | username() | clientid() |
%% -type ipaddr() :: {ipaddr, string()}.
%% -type ipaddrs() :: {ipaddrs, [string()]}.
%% -type username() :: {user | username, string()} | {user | username, {re, regex()}}.
%% -type clientid() :: {client | clientid, string()} | {client | clientid, {re, regex()}}.
%% -type who() :: ipaddr() | ipaddrs() | username() | clientid() |
%% {'and', [ipaddr() | ipaddrs() | username() | clientid()]} |
%% {'or', [ipaddr() | ipaddrs() | username() | clientid()]} |
%% all).
%% all.
%% -type simple_action() :: subscribe | publish | all.
%% -type complex_action() :: {simple_action(), [{qos, 0..2}, {retain, true|false|all}]}.
%% -type action() :: simple_action() | complex_action().
%% -type topic() :: string().
%% -type topic_filter() :: string().
%% -type topic_match() :: topic() | topic_filter() | {eq, topic() | topic_filter()}.
%% -type perm() :: allow | deny.
%% -type rule() :: {perm(), who(), action(), [topic_match()]} | {perm(), all}.

%%-------------- Viusal aid for the spec -------------------------------------------------
%%
%% -type(action() :: subscribe | publish | all).
%% rule()
%% ├── {perm(), who(), action(), [topic_match()]}
%% │ │ │ │ ├── topic() :: string()
%% │ │ │ │ ├── topic_filter() :: string()
%% │ │ │ │ └── {eq, topic() | topic_filter()}
%% │ │ │ │
%% │ │ │ ├── simple_action()
%% │ │ │ │ ├── publish
%% │ │ │ │ ├── subscribe
%% │ │ │ │ └── all
%% │ │ │ └── {simple_action(), [{qos,0..2},{retain,true|false|all}]}
%% │ │ │
%% │ │ ├── ipaddr()
%% │ │ │ └── {ipaddr, string()}
%% │ │ ├── ipaddrs()
%% │ │ │ └── {ipaddrs, [string()]}
%% │ │ ├── username()
%% │ │ │ ├── {user | username, string()}
%% │ │ │ └── {user | username, {re, regex()}}
%% │ │ ├── clientid()
%% │ │ │ ├── {client | clientid, string()}
%% │ │ │ └── {client | clientid, {re, regex()}}
%% │ │ ├── {'and', [ipaddr() | ipaddrs() | username() | clientid()]}
%% │ │ ├── {'or', [ipaddr() | ipaddrs() | username() | clientid()]}
%% │ │ └── all
%% │ │
%% │ ├── allow
%% │ └── deny
%% │
%% └── {perm(), all}
%%
%% -type(topic_filters() :: string()).

%% This file defines a set of ACL rules for MQTT client pub/sub authorization.
%% The content is of Erlang-term format.
%% Each Erlang-term is a tuple `{...}` terminated by dot `.`
%%
%% -type(topics() :: [topic_filters() | {eq, topic_filters()}]).
%% NOTE: When deploy to production, the last rule should be changed to {deny, all}.
%%
%% -type(permission() :: allow | deny).
%% NOTE: It's a good practice to keep the nubmer of rules small, because in worst case
%% scenarios, all rules have to be traversed for each message publish.
%%
%% -type(rule() :: {permission(), who(), action(), topics()} | {permission(), all}).
%%--------------------------------------------------------------------

{allow, {username, {re, "^dashboard$"}}, subscribe, ["$SYS/#"]}.

{allow, {ipaddr, "127.0.0.1"}, all, ["$SYS/#", "#"]}.

{deny, all, subscribe, ["$SYS/#", {eq, "#"}]}.

{allow, all}.
%% A rule is a 4-element tuple.
%% For example, `{allow, {username, "Jon"}, subscribe, ["#"]}` allows Jon to subscribe to
%% any topic they want.
%%
%% Below is an explanation:
%%
%% - `perm()`: The permission.
%% Defines whether this is an `allow` or `deny` rule.
%%
%% - `who()`: The MQTT client matching condition.
%% - `all`: A rule which applies to all clients.
%% - `{ipaddr, IpAddress}`: Matches a client by source IP address. CIDR notation is allowed.
%% - `{ipaddrs, [IpAddress]}`: Matches clients by a set of IP addresses. CIDR notation is allowed.
%% - `{clientid, ClientID}`: Matches a client by ID.
%% - `{username, Username}`: Matches a client by username.
%% - `{..., {re, ..}}`: Regular expression to match either clientid or username.
%% - `{'and', [...]}`: Combines a list of matching conditions.
%% - `{'or', [...]}`: Combines a list of matching conditions.
%%
%% - `action()`: Matches publish or subscribe actions (or both).
%% Applies the rule to `publish` or `subscribe` actions.
%% The special value `all` denotes allowing or denying both `publish` and `subscribe`.
%% It can also be associated with `qos` and `retain` flags to match the action with
%% more specifics. For example, `{publish, [{qos,0},{retain,false}]}` should only
%% match the `publish` action when the message has QoS 0, and without retained flag set.
%%
%% - `[topic_match()]`:
%% A list of topics, topic-filters, or template rendering to match the topic being
%% subscribed to or published.
%% For example, `{allow, {username, "Jan"}, publish, ["jan/#"]}` permits Jan to publish
%% to any topic matching the wildcard pattern "jan/#".
%% A special tuple `{eq, topic_match()}` is useful to allow or deny the specific wildcard
%% subscription instead of performing a topic match.
%% A `topic_match()` can also contain a placeholder rendered with actual value at runtime,
%% for example, `{allow, all, publish, "${clientid}/#"}` allows all clients to publish to
%% topics prefixed by their own client ID.
%%
%% Supported placeholders are:
%% - `${cn}`: TLS certificate common name.
%% - `${clientid}`: The client ID.
%% - `${username}`: The username.
%% - `${client_attrs.NAME}`: A client attribute named `NAME`, which can be initialized by
%% `mqtt.client_attrs_init` config or extended by certain authentication backends.
%% NOTE: Placeholder is not rendered as empty string if the referencing value is not
%% foud. For example, `${client_attrs.group}/#` is not rendered as `/#` if the
%% client does not have a `group` attribute.
18 changes: 16 additions & 2 deletions apps/emqx_auth/src/emqx_authz/emqx_authz_rule.erl
Expand Up @@ -351,8 +351,9 @@ match_who(_, _) ->
match_topics(_ClientInfo, _Topic, []) ->
false;
match_topics(ClientInfo, Topic, [{pattern, PatternFilter} | Filters]) ->
TopicFilter = bin(emqx_template:render_strict(PatternFilter, ClientInfo)),
match_topic(emqx_topic:words(Topic), emqx_topic:words(TopicFilter)) orelse
TopicFilter = render_topic(PatternFilter, ClientInfo),
(is_binary(TopicFilter) andalso
match_topic(emqx_topic:words(Topic), emqx_topic:words(TopicFilter))) orelse
match_topics(ClientInfo, Topic, Filters);
match_topics(ClientInfo, Topic, [TopicFilter | Filters]) ->
match_topic(emqx_topic:words(Topic), TopicFilter) orelse
Expand All @@ -362,3 +363,16 @@ match_topic(Topic, {'eq', TopicFilter}) ->
Topic =:= TopicFilter;
match_topic(Topic, TopicFilter) ->
emqx_topic:match(Topic, TopicFilter).

render_topic(Topic, ClientInfo) ->
try
bin(emqx_template:render_strict(Topic, ClientInfo))
catch
error:Reason ->
?SLOG(debug, #{
msg => "failed_to_render_topic_template",
template => Topic,
reason => Reason
}),
error
end.
41 changes: 40 additions & 1 deletion apps/emqx_auth/test/emqx_authz/emqx_authz_SUITE.erl
Expand Up @@ -173,7 +173,16 @@ end_per_testcase(_TestCase, _Config) ->
-define(SOURCE_FILE_CLIENT_ATTR,
?SOURCE_FILE(
<<
"{allow,all,all,[\"${client_attrs.alias}/#\"]}.\n"
"{allow,all,all,[\"${client_attrs.alias}/#\",\"client_attrs_backup\"]}.\n"
"{deny, all}."
>>
)
).

-define(SOURCE_FILE_CLIENT_NO_SUCH_ATTR,
?SOURCE_FILE(
<<
"{allow,all,all,[\"${client_attrs.nonexist}/#\",\"client_attrs_backup\"]}.\n"
"{deny, all}."
>>
)
Expand Down Expand Up @@ -572,11 +581,41 @@ t_alias_prefix(_Config) ->
?assertMatch({ok, _}, emqtt:connect(C)),
?assertMatch({ok, _, [?RC_SUCCESS]}, emqtt:subscribe(C, SubTopic)),
?assertMatch({ok, _, [?RC_NOT_AUTHORIZED]}, emqtt:subscribe(C, SubTopicNotAllowed)),
?assertMatch({ok, _, [?RC_NOT_AUTHORIZED]}, emqtt:subscribe(C, <<"/#">>)),
unlink(C),
emqtt:stop(C),
NonMatching = <<"clientid_which_has_no_dash">>,
{ok, C2} = emqtt:start_link([{clientid, NonMatching}, {proto_ver, v5}]),
?assertMatch({ok, _}, emqtt:connect(C2)),
?assertMatch({ok, _, [?RC_SUCCESS]}, emqtt:subscribe(C2, <<"client_attrs_backup">>)),
%% assert '${client_attrs.alias}/#' is not rendered as '/#'
?assertMatch({ok, _, [?RC_NOT_AUTHORIZED]}, emqtt:subscribe(C2, <<"/#">>)),
unlink(C2),
emqtt:stop(C2),
emqx_config:put_zone_conf(default, [mqtt, client_attrs_init], []),
ok.

t_non_existing_attr(_Config) ->
{ok, _} = emqx_authz:update(?CMD_REPLACE, [?SOURCE_FILE_CLIENT_NO_SUCH_ATTR]),
%% '^.*-(.*)$': extract the suffix after the last '-'
{ok, Compiled} = emqx_variform:compile("concat(regex_extract(clientid,'^.*-(.*)$'))"),
emqx_config:put_zone_conf(default, [mqtt, client_attrs_init], [
#{
expression => Compiled,
%% this is intended to be different from 'nonexist'
set_as_attr => <<"existing">>
}
]),
ClientId = <<"org1-name3">>,
{ok, C} = emqtt:start_link([{clientid, ClientId}, {proto_ver, v5}]),
?assertMatch({ok, _}, emqtt:connect(C)),
?assertMatch({ok, _, [?RC_SUCCESS]}, emqtt:subscribe(C, <<"client_attrs_backup">>)),
%% assert '${client_attrs.nonexist}/#' is not rendered as '/#'
?assertMatch({ok, _, [?RC_NOT_AUTHORIZED]}, emqtt:subscribe(C, <<"/#">>)),
unlink(C),
emqtt:stop(C),
ok.

%% client is allowed by ACL to publish to its LWT topic, is connected,
%% and then gets banned and kicked out while connected. Should not
%% publish LWT.
Expand Down

0 comments on commit 6af651c

Please sign in to comment.