Merge pull request #6314 from JimMoen/fix-re-escape

fix(authz): placehodler regular escape
2021-11-30 10:21:28 +08:00 · 2021-11-30 10:21:28 +08:00 · 36cece7a49
parent e6d7e59c18 1d3558ebe0
commit 36cece7a49
8 changed files with 79 additions and 52 deletions
--- a/apps/emqx/src/emqx_topic.erl
+++ b/apps/emqx/src/emqx_topic.erl
@ -54,11 +54,11 @@ wildcard(Topic) when is_binary(Topic) ->
    wildcard(words(Topic));
 wildcard([]) ->
    false;
-wildcard(['#'|_]) ->
+wildcard(['#' | _]) ->
    true;
-wildcard(['+'|_]) ->
+wildcard(['+' | _]) ->
    true;
-wildcard([_H|T]) ->
+wildcard([_H | T]) ->
    wildcard(T).

 %% @doc Match Topic name with filter.
@ -73,17 +73,17 @@ match(Name, Filter) when is_binary(Name), is_binary(Filter) ->
    match(words(Name), words(Filter));
 match([], []) ->
    true;
-match([H|T1], [H|T2]) ->
+match([H | T1], [H | T2]) ->
    match(T1, T2);
-match([_H|T1], ['+'|T2]) ->
+match([_H | T1], ['+' | T2]) ->
    match(T1, T2);
 match(_, ['#']) ->
    true;
-match([_H1|_], [_H2|_]) ->
+match([_H1 | _], [_H2 | _]) ->
    false;
-match([_H1|_], []) ->
+match([_H1 | _], []) ->
    false;
-match([], [_H|_T2]) ->
+match([], [_H | _T2]) ->
    false.

 %% @doc Validate topic name or filter
@ -110,13 +110,13 @@ validate2([]) ->
    true;
 validate2(['#']) -> % end with '#'
    true;
-validate2(['#'|Words]) when length(Words) > 0 ->
+validate2(['#' | Words]) when length(Words) > 0 ->
    error('topic_invalid_#');
-validate2([''|Words]) ->
+validate2(['' | Words]) ->
    validate2(Words);
-validate2(['+'|Words]) ->
+validate2(['+' | Words]) ->
    validate2(Words);
-validate2([W|Words]) ->
+validate2([W | Words]) ->
    validate3(W) andalso validate2(Words).

 validate3(<<>>) ->
@ -164,7 +164,7 @@ word(<<"#">>) -> '#';
 word(Bin)     -> Bin.

 %% @doc '$SYS' Topic.
-spec(systop(atom()|string()|binary()) -> topic()).
+-spec(systop(atom() | string() | binary()) -> topic()).
 systop(Name) when is_atom(Name); is_list(Name) ->
    iolist_to_binary(lists:concat(["$SYS/brokers/", node(), "/", Name]));
 systop(Name) when is_binary(Name) ->
@ -175,10 +175,10 @@ feed_var(Var, Val, Topic) ->
    feed_var(Var, Val, words(Topic), []).
 feed_var(_Var, _Val, [], Acc) ->
    join(lists:reverse(Acc));
-feed_var(Var, Val, [Var|Words], Acc) ->
-    feed_var(Var, Val, Words, [Val|Acc]);
-feed_var(Var, Val, [W|Words], Acc) ->
-    feed_var(Var, Val, Words, [W|Acc]).
+feed_var(Var, Val, [Var | Words], Acc) ->
+    feed_var(Var, Val, Words, [Val | Acc]);
+feed_var(Var, Val, [W | Words], Acc) ->
+    feed_var(Var, Val, Words, [W | Acc]).

 -spec(join(list(binary())) -> binary()).
 join([]) ->
@ -218,4 +218,3 @@ parse(TopicFilter = <<"$share/", Rest/binary>>, Options) ->
    end;
 parse(TopicFilter, Options) ->
    {TopicFilter, Options}.
-
--- a/apps/emqx/test/emqx_topic_SUITE.erl
+++ b/apps/emqx/test/emqx_topic_SUITE.erl
@ -20,6 +20,7 @@
 -compile(nowarn_export_all).

 -include_lib("eunit/include/eunit.hrl").
+-include_lib("emqx/include/emqx_placeholder.hrl").

 -import(emqx_topic,
        [ wildcard/1
@ -183,9 +184,11 @@ t_feed_var(_) ->
    ?assertEqual(<<"$queue/client/clientId">>,
                 feed_var(<<"$c">>, <<"clientId">>, <<"$queue/client/$c">>)),
    ?assertEqual(<<"username/test/client/x">>,
-                 feed_var(<<"%u">>, <<"test">>, <<"username/%u/client/x">>)),
+                 feed_var( ?PH_USERNAME, <<"test">>
+                         , <<"username/", ?PH_USERNAME/binary, "/client/x">>)),
    ?assertEqual(<<"username/test/client/clientId">>,
-                 feed_var(<<"%c">>, <<"clientId">>, <<"username/test/client/%c">>)).
+                 feed_var( ?PH_CLIENTID, <<"clientId">>
+                         , <<"username/test/client/", ?PH_CLIENTID/binary>>)).

 long_topic() ->
    iolist_to_binary([[integer_to_list(I), "/"] || I <- lists:seq(0, 66666)]).
--- a/apps/emqx_authz/README.md
+++ b/apps/emqx_authz/README.md
@ -23,7 +23,7 @@ authz:{
                keyfile: "etc/certs/client-key.pem"
              }
           }
-           sql: "select ipaddress, username, clientid, action, permission, topic from mqtt_authz where ipaddr = '%a' or username = '%u' or clientid = '%c'"
+           sql: "select ipaddress, username, clientid, action, permission, topic from mqtt_authz where ipaddr = ${peerhost} or username = ${username} or clientid = ${clientid}"
       },
       {
           type: postgresql
@ -36,7 +36,7 @@ authz:{
              auto_reconnect: true
              ssl: {enable: false}
           }
-           sql: "select ipaddress, username, clientid, action, permission, topic from mqtt_authz where ipaddr = '%a' or username = '%u' or username = '$all' or clientid = '%c'"
+           sql: "select ipaddress, username, clientid, action, permission, topic from mqtt_authz where ipaddr = ${peerhost} or username = ${username} or username = '$all' or clientid = ${clientid}"
       },
       {
           type: redis
@ -48,7 +48,7 @@ authz:{
              auto_reconnect: true
              ssl: {enable: false}
           }
-           cmd: "HGETALL mqtt_authz:%u"
+           cmd: "HGETALL mqtt_authz:${username}"
       },
       {
           principal: {username: "^admin?"}
--- a/apps/emqx_authz/etc/emqx_authz.conf
+++ b/apps/emqx_authz/etc/emqx_authz.conf
@ -22,7 +22,7 @@ authorization {
       #       certfile: "{{ platform_etc_dir }}/certs/client-cert.pem"
       #       keyfile: "{{ platform_etc_dir }}/certs/client-key.pem"
       #     }
-       #     query: "select ipaddress, username, clientid, action, permission, topic from mqtt_authz where ipaddr = '%a' or username = '%u' or clientid = '%c'"
+       #     query: "select ipaddress, username, clientid, action, permission, topic from mqtt_authz where ipaddr = ${peerhost} or username = ${username} or clientid = ${clientid}"
       # },
       # {
       #     type: postgresql
@ -33,7 +33,7 @@ authorization {
       #     password: public
       #     auto_reconnect: true
       #     ssl: {enable: false}
-       #     query: "select ipaddress, username, clientid, action, permission, topic from mqtt_authz where ipaddr = '%a' or username = '%u' or username = '$all' or clientid = '%c'"
+       #     query: "select ipaddress, username, clientid, action, permission, topic from mqtt_authz where ipaddr = ${peerhost} or username = ${username} or username = '$all' or clientid = ${clientid}"
       # },
       # {
       #     type: redis
@ -43,7 +43,7 @@ authorization {
       #     password: public
       #     auto_reconnect: true
       #     ssl: {enable: false}
-       #     cmd: "HGETALL mqtt_authz:%u"
+       #     cmd: "HGETALL mqtt_authz:${username}"
       # },
       # {
       #     type: mongodb
@ -53,7 +53,7 @@ authorization {
       #     database: mqtt
       #     ssl: {enable: false}
       #     collection: mqtt_authz
-       #     selector: { "$or": [ { "username": "%u" }, { "clientid": "%c" } ] }
+       #     selector: { "$or": [ { "username": "${username}" }, { "clientid": "${clientid}" } ] }
       # },
       {
            type: built-in-database
--- a/apps/emqx_authz/src/emqx_authz.erl
+++ b/apps/emqx_authz/src/emqx_authz.erl
@ -40,6 +40,8 @@

 -export([acl_conf_file/0]).

+-export([ph_to_re/1]).
+
 -spec(register_metrics() -> ok).
 register_metrics() ->
    lists:foreach(fun emqx_metrics:ensure/1, ?AUTHZ_METRICS).
@ -64,11 +66,14 @@ move(Type, Cmd) ->
    move(Type, Cmd, #{}).

 move(Type, #{<<"before">> := Before}, Opts) ->
-    emqx:update_config(?CONF_KEY_PATH, {?CMD_MOVE, type(Type), ?CMD_MOVE_BEFORE(type(Before))}, Opts);
+    emqx:update_config( ?CONF_KEY_PATH
+                      , {?CMD_MOVE, type(Type), ?CMD_MOVE_BEFORE(type(Before))}, Opts);
 move(Type, #{<<"after">> := After}, Opts) ->
-    emqx:update_config(?CONF_KEY_PATH, {?CMD_MOVE, type(Type), ?CMD_MOVE_AFTER(type(After))}, Opts);
+    emqx:update_config( ?CONF_KEY_PATH
+                      , {?CMD_MOVE, type(Type), ?CMD_MOVE_AFTER(type(After))}, Opts);
 move(Type, Position, Opts) ->
-    emqx:update_config(?CONF_KEY_PATH, {?CMD_MOVE, type(Type), Position}, Opts).
+    emqx:update_config( ?CONF_KEY_PATH
+                      , {?CMD_MOVE, type(Type), Position}, Opts).

 update(Cmd, Sources) ->
    update(Cmd, Sources, #{}).
@ -155,7 +160,8 @@ do_post_update({{?CMD_REPLACE, Type}, Source}, _NewSources) when is_map(Source)
    {OldSource, Front, Rear} = take(Type, OldInitedSources),
    ok = ensure_resource_deleted(OldSource),
    InitedSources = init_sources(check_sources([Source])),
-    ok = emqx_hooks:put('client.authorize', {?MODULE, authorize, [Front ++ InitedSources ++ Rear]}, -1),
+    ok = emqx_hooks:put( 'client.authorize'
+                       , {?MODULE, authorize, [Front ++ InitedSources ++ Rear]}, -1),
    ok = emqx_authz_cache:drain_cache();
 do_post_update({{?CMD_DELETE, Type}, _Source}, _NewSources) ->
    OldInitedSources = lookup(),
@ -267,7 +273,7 @@ init_source(#{type := DB,
        {error, Reason} -> error({load_config_error, Reason});
        Id -> Source#{annotations =>
                      #{id => Id,
-                        query => Mod:parse_query(SQL)
+                        query => erlang:apply(Mod, parse_query, [SQL])
                       }
                   }
    end.
@ -277,22 +283,36 @@ init_source(#{type := DB,
 %%--------------------------------------------------------------------

 %% @doc Check AuthZ
-spec(authorize(emqx_types:clientinfo(), emqx_types:all(), emqx_types:topic(), allow | deny, sources())
+-spec(authorize( emqx_types:clientinfo()
+               , emqx_types:all()
+               , emqx_types:topic()
+               , allow | deny
+               , sources())
      -> {stop, allow} | {ok, deny}).
 authorize(#{username := Username,
            peerhost := IpAddress
           } = Client, PubSub, Topic, DefaultResult, Sources) ->
    case do_authorize(Client, PubSub, Topic, Sources) of
        {matched, allow} ->
-            ?SLOG(info, #{msg => "authorization_permission_allowed", username => Username, ipaddr => IpAddress, topic => Topic}),
+            ?SLOG(info, #{msg => "authorization_permission_allowed",
+                          username => Username,
+                          ipaddr => IpAddress,
+                          topic => Topic}),
            emqx_metrics:inc(?AUTHZ_METRICS(allow)),
            {stop, allow};
        {matched, deny} ->
-            ?SLOG(info, #{msg => "authorization_permission_denied", username => Username, ipaddr => IpAddress, topic => Topic}),
+            ?SLOG(info, #{msg => "authorization_permission_denied",
+                          username => Username,
+                          ipaddr => IpAddress,
+                          topic => Topic}),
            emqx_metrics:inc(?AUTHZ_METRICS(deny)),
            {stop, deny};
        nomatch ->
-            ?SLOG(info, #{msg => "authorization_failed_nomatch", username => Username, ipaddr => IpAddress, topic => Topic, reason => "no-match rule"}),
+            ?SLOG(info, #{msg => "authorization_failed_nomatch",
+                          username => Username,
+                          ipaddr => IpAddress,
+                          topic => Topic,
+                          reason => "no-match rule"}),
            {stop, DefaultResult}
    end.

@ -309,7 +329,7 @@ do_authorize(Client, PubSub, Topic, [#{type := file} = F | Tail]) ->
 do_authorize(Client, PubSub, Topic,
               [Connector = #{type := Type} | Tail] ) ->
    Mod = authz_module(Type),
-    case Mod:authorize(Client, PubSub, Topic, Connector) of
+    case erlang:apply(Mod, authorize, [Client, PubSub, Topic, Connector]) of
        nomatch -> do_authorize(Client, PubSub, Topic, Tail);
        Matched -> Matched
    end.
@ -381,8 +401,12 @@ type(postgresql) -> postgresql;
 type(<<"postgresql">>) -> postgresql;
 type('built-in-database') -> 'built-in-database';
 type(<<"built-in-database">>) -> 'built-in-database';
-type(Unknown) -> error({unknown_authz_source_type, Unknown}). % should never happend if the input is type-checked by hocon schema
+%% should never happend if the input is type-checked by hocon schema
+type(Unknown) -> error({unknown_authz_source_type, Unknown}).

 %% @doc where the acl.conf file is stored.
 acl_conf_file() ->
    filename:join([emqx:data_dir(), "authz", "acl.conf"]).
+
+ph_to_re(VarPH) ->
+    re:replace(VarPH, "[\\$\\{\\}]", "\\\\&", [global, {return, list}]).
--- a/apps/emqx_authz/src/emqx_authz_http.erl
+++ b/apps/emqx_authz/src/emqx_authz_http.erl
@ -87,19 +87,19 @@ replvar(Str0, PubSub, Topic,
         }) when is_list(Str0);
                 is_binary(Str0) ->
    NTopic = emqx_http_lib:uri_encode(Topic),
-    Str1 = re:replace( Str0, ?PH_S_CLIENTID
+    Str1 = re:replace( Str0, emqx_authz:ph_to_re(?PH_S_CLIENTID)
                     , Clientid, [global, {return, binary}]),
-    Str2 = re:replace( Str1, ?PH_S_USERNAME
+    Str2 = re:replace( Str1, emqx_authz:ph_to_re(?PH_S_USERNAME)
                     , bin(Username), [global, {return, binary}]),
-    Str3 = re:replace( Str2, ?PH_S_HOST
+    Str3 = re:replace( Str2, emqx_authz:ph_to_re(?PH_S_HOST)
                     , inet_parse:ntoa(IpAddress), [global, {return, binary}]),
-    Str4 = re:replace( Str3, ?PH_S_PROTONAME
+    Str4 = re:replace( Str3, emqx_authz:ph_to_re(?PH_S_PROTONAME)
                     , bin(Protocol), [global, {return, binary}]),
-    Str5 = re:replace( Str4, ?PH_S_MOUNTPOINT
+    Str5 = re:replace( Str4, emqx_authz:ph_to_re(?PH_S_MOUNTPOINT)
                     , Mountpoint, [global, {return, binary}]),
-    Str6 = re:replace( Str5, ?PH_S_TOPIC
+    Str6 = re:replace( Str5, emqx_authz:ph_to_re(?PH_S_TOPIC)
                     , NTopic, [global, {return, binary}]),
-    Str7 = re:replace( Str6, ?PH_S_ACTION
+    Str7 = re:replace( Str6, emqx_authz:ph_to_re(?PH_S_ACTION)
                     , bin(PubSub), [global, {return, binary}]),
    Str7.

--- a/apps/emqx_authz/src/emqx_authz_mongodb.erl
+++ b/apps/emqx_authz/src/emqx_authz_mongodb.erl
@ -76,11 +76,11 @@ replvar(Selector, #{clientid := Clientid,
                                end || M <- V],
                           AccIn);
              InFun(K, V, AccIn) when is_binary(V) ->
-                  V1 = re:replace( V,  ?PH_S_CLIENTID
+                  V1 = re:replace( V,  emqx_authz:ph_to_re(?PH_S_CLIENTID)
                                 , bin(Clientid), [global, {return, binary}]),
-                  V2 = re:replace( V1, ?PH_S_USERNAME
+                  V2 = re:replace( V1, emqx_authz:ph_to_re(?PH_S_USERNAME)
                                 , bin(Username), [global, {return, binary}]),
-                  V3 = re:replace( V2, ?PH_S_HOST
+                  V3 = re:replace( V2, emqx_authz:ph_to_re(?PH_S_HOST)
                                 , inet_parse:ntoa(IpAddress), [global, {return, binary}]),
                  maps:put(K, V3, AccIn);
              InFun(K, V, AccIn) -> maps:put(K, V, AccIn)
--- a/apps/emqx_authz/src/emqx_authz_redis.erl
+++ b/apps/emqx_authz/src/emqx_authz_redis.erl
@ -71,8 +71,9 @@ replvar(Cmd, Client = #{username := Username}) ->
 replvar(Cmd, _) ->
    Cmd.

-repl(S, _Var, undefined) ->
+repl(S, _VarPH, undefined) ->
    S;
-repl(S, Var, Val) ->
-    NVal = re:replace(Val, "&", "\\\\&", [global, {return, list}]),
-    re:replace(S, Var, NVal, [{return, list}]).
+repl(S, VarPH, Val) ->
+    NVal   = re:replace(Val, "&", "\\\\&", [global, {return, list}]),
+    NVarPH = emqx_authz:ph_to_re(VarPH),
+    re:replace(S, NVarPH, NVal, [{return, list}]).