From b2c3d8366da17b8829516a7580519136090ac46b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=91=A8=E5=AD=90=E5=8D=9A?= <349832309@qq.com> Date: Mon, 3 Dec 2018 13:57:37 +0800 Subject: [PATCH 1/3] Add logs for malformed acl configuration file --- src/emqx_access_rule.erl | 10 +++++++--- src/emqx_acl_internal.erl | 28 ++++++++++++++++++---------- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/src/emqx_access_rule.erl b/src/emqx_access_rule.erl index 3fb6dd7ef..1d3154735 100644 --- a/src/emqx_access_rule.erl +++ b/src/emqx_access_rule.erl @@ -34,16 +34,20 @@ -export([match/3]). -define(ALLOW_DENY(A), ((A =:= allow) orelse (A =:= deny))). +-define(PUBSUB(A), ((A =:= subscribe) orelse (A =:= publish) orelse (A =:= pubsub))). %% @doc Compile Access Rule. compile({A, all}) when ?ALLOW_DENY(A) -> {A, all}; -compile({A, Who, Access, Topic}) when ?ALLOW_DENY(A), is_binary(Topic) -> +compile({A, Who, Access, Topic}) when ?ALLOW_DENY(A), ?PUBSUB(Access), is_binary(Topic) -> {A, compile(who, Who), Access, [compile(topic, Topic)]}; -compile({A, Who, Access, TopicFilters}) when ?ALLOW_DENY(A) -> - {A, compile(who, Who), Access, [compile(topic, Topic) || Topic <- TopicFilters]}. +compile({A, Who, Access, TopicFilters}) when ?ALLOW_DENY(A), ?PUBSUB(Access) -> + {A, compile(who, Who), Access, [compile(topic, Topic) || Topic <- TopicFilters]}; + +compile(Rule) -> + emqx_logger:error("[ACCESS_RULE] Malformed rule: ~p", [Rule]). compile(who, all) -> all; diff --git a/src/emqx_acl_internal.erl b/src/emqx_acl_internal.erl index eee7e6c18..383967030 100644 --- a/src/emqx_acl_internal.erl +++ b/src/emqx_acl_internal.erl @@ -46,18 +46,24 @@ all_rules() -> -spec(init([File :: string()]) -> {ok, #{}}). init([File]) -> _ = emqx_tables:new(?ACL_RULE_TAB, [set, public, {read_concurrency, true}]), - true = load_rules_from_file(File), + ok = load_rules_from_file(File), {ok, #{acl_file => File}}. load_rules_from_file(AclFile) -> - {ok, Terms} = file:consult(AclFile), - Rules = [emqx_access_rule:compile(Term) || Term <- Terms], - lists:foreach(fun(PubSub) -> - ets:insert(?ACL_RULE_TAB, {PubSub, - lists:filter(fun(Rule) -> filter(PubSub, Rule) end, Rules)}) - end, [publish, subscribe]), - ets:insert(?ACL_RULE_TAB, {all_rules, Terms}). - + case file:consult(AclFile) of + {ok, Terms} -> + Rules = [emqx_access_rule:compile(Term) || Term <- Terms], + lists:foreach(fun(PubSub) -> + ets:insert(?ACL_RULE_TAB, {PubSub, + lists:filter(fun(Rule) -> filter(PubSub, Rule) end, Rules)}) + end, [publish, subscribe]), + ets:insert(?ACL_RULE_TAB, {all_rules, Terms}), + ok; + {error, Reason} -> + emqx_logger:error("[ACL_INTERNAL] Consult failed: ~p", [Reason]), + {error, Reason} + end. + filter(_PubSub, {allow, all}) -> true; filter(_PubSub, {deny, all}) -> @@ -100,9 +106,11 @@ match(Credentials, Topic, [Rule|Rules]) -> -spec(reload_acl(state()) -> ok | {error, term()}). reload_acl(#{acl_file := AclFile}) -> case catch load_rules_from_file(AclFile) of - true -> + ok -> emqx_logger:info("Reload acl_file ~s successfully", [AclFile]), ok; + {error, Error} -> + {error, Error}; {'EXIT', Error} -> {error, Error} end. From 35e699e54ea83df6bbcfa5865ef9a3ff169ff29b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=91=A8=E5=AD=90=E5=8D=9A?= <349832309@qq.com> Date: Tue, 4 Dec 2018 16:11:25 +0800 Subject: [PATCH 2/3] Make sure test case of emqx_banned passes --- src/emqx_banned.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/emqx_banned.erl b/src/emqx_banned.erl index 49a698384..aae4f8c7a 100644 --- a/src/emqx_banned.erl +++ b/src/emqx_banned.erl @@ -105,7 +105,7 @@ code_change(_OldVsn, State, _Extra) -> -ifdef(TEST). ensure_expiry_timer(State) -> - State#{expiry_timer := emqx_misc:start_timer(timer:seconds(2), expire)}. + State#{expiry_timer := emqx_misc:start_timer(timer:seconds(1), expire)}. -else. ensure_expiry_timer(State) -> State#{expiry_timer := emqx_misc:start_timer(timer:minutes(5), expire)}. From ec2e28977600d131cf3b5eca4e5be5fbc71d3f23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=91=A8=E5=AD=90=E5=8D=9A?= <349832309@qq.com> Date: Mon, 10 Dec 2018 11:13:25 +0800 Subject: [PATCH 3/3] Fix crash in emqx_acl_internal:filter/2 --- src/emqx_access_rule.erl | 3 ++- src/emqx_acl_internal.erl | 6 ++++-- test/emqx_access_SUITE.erl | 3 ++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/emqx_access_rule.erl b/src/emqx_access_rule.erl index 1d3154735..47b20676b 100644 --- a/src/emqx_access_rule.erl +++ b/src/emqx_access_rule.erl @@ -47,7 +47,8 @@ compile({A, Who, Access, TopicFilters}) when ?ALLOW_DENY(A), ?PUBSUB(Access) -> {A, compile(who, Who), Access, [compile(topic, Topic) || Topic <- TopicFilters]}; compile(Rule) -> - emqx_logger:error("[ACCESS_RULE] Malformed rule: ~p", [Rule]). + emqx_logger:error("[ACCESS_RULE] Malformed rule: ~p", [Rule]), + {error, bad_rule}. compile(who, all) -> all; diff --git a/src/emqx_acl_internal.erl b/src/emqx_acl_internal.erl index 383967030..328993a78 100644 --- a/src/emqx_acl_internal.erl +++ b/src/emqx_acl_internal.erl @@ -60,10 +60,12 @@ load_rules_from_file(AclFile) -> ets:insert(?ACL_RULE_TAB, {all_rules, Terms}), ok; {error, Reason} -> - emqx_logger:error("[ACL_INTERNAL] Consult failed: ~p", [Reason]), + emqx_logger:error("[ACL_INTERNAL] Failed to read ~s: ~p", [AclFile, Reason]), {error, Reason} end. - + +filter(_PubSub, {error, _}) -> + false; filter(_PubSub, {allow, all}) -> true; filter(_PubSub, {deny, all}) -> diff --git a/test/emqx_access_SUITE.erl b/test/emqx_access_SUITE.erl index b49c90d80..5d0bcf049 100644 --- a/test/emqx_access_SUITE.erl +++ b/test/emqx_access_SUITE.erl @@ -355,7 +355,8 @@ compile_rule(_) -> {deny, all, subscribe, [ [<<"$SYS">>, '#'], ['#'] ]} = compile({deny, all, subscribe, ["$SYS/#", "#"]}), {allow, all} = compile({allow, all}), - {deny, all} = compile({deny, all}). + {deny, all} = compile({deny, all}), + {error, bad_rule} = compile({test, malformed}). match_rule(_) -> User = #{client_id => <<"testClient">>, username => <<"TestUser">>, peername => {{127,0,0,1}, 2948}},