From 0aeb2cd77ff33088e2c2a90de3fc5c8f98dfc0c4 Mon Sep 17 00:00:00 2001
From: Kjell Winblad <kjellwinblad@gmail.com>
Date: Fri, 10 May 2024 12:27:04 +0200
Subject: [PATCH] fix: listener crash if access_rules config option is
 incorrect

Previously when changing the access_rules configuration option of a
listener to something that was not a valid rule, the listener would
crash. This has now been fixed by the addition of a configuration
validator that checks the access_rules field.

Additionally, a configuration option converter has been added to the
access_rules field so that one can specify several rules in a single
string by using "," (comma) as separator.

Fixes:
https://emqx.atlassian.net/browse/EMQX-12315
---
 apps/emqx/src/emqx_schema.erl | 46 ++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl
index b4ba08bbc..9b3450a83 100644
--- a/apps/emqx/src/emqx_schema.erl
+++ b/apps/emqx/src/emqx_schema.erl
@@ -1781,7 +1781,9 @@ mqtt_listener(Bind) ->
                     hoconsc:array(string()),
                     #{
                         desc => ?DESC(mqtt_listener_access_rules),
-                        default => [<<"allow all">>]
+                        default => [<<"allow all">>],
+                        converter => fun access_rules_converter/1,
+                        validator => fun access_rules_validator/1
                     }
                 )},
             {"proxy_protocol",
@@ -1802,6 +1804,48 @@ mqtt_listener(Bind) ->
                 )}
         ] ++ emqx_schema_hooks:injection_point('mqtt.listener').
 
+access_rules_converter(AccessRules) ->
+    DeepRules =
+        lists:foldr(
+            fun(Rule, Acc) ->
+                Rules = re:split(Rule, <<"\\s*,\\s*">>, [{return, binary}]),
+                [Rules | Acc]
+            end,
+            [],
+            AccessRules
+        ),
+    [unicode:characters_to_list(RuleBin) || RuleBin <- lists:flatten(DeepRules)].
+
+access_rules_validator(AccessRules) ->
+    InvalidRules = [Rule || Rule <- AccessRules, is_invalid_rule(Rule)],
+    case InvalidRules of
+        [] ->
+            ok;
+        _ ->
+            MsgStr = io_lib:format("invalid_rule(s): ~ts", [string:join(InvalidRules, ", ")]),
+            MsgBin = unicode:characters_to_binary(MsgStr),
+            {error, MsgBin}
+    end.
+
+is_invalid_rule(S) ->
+    try
+        [Action, CIDR] = string:tokens(S, " "),
+        case Action of
+            "allow" -> ok;
+            "deny" -> ok
+        end,
+        case CIDR of
+            "all" ->
+                ok;
+            _ ->
+                %% should not crash
+                _ = esockd_cidr:parse(CIDR, true)
+        end,
+        false
+    catch
+        _:_ -> true
+    end.
+
 base_listener(Bind) ->
     [
         {"enable",