Merge pull request #4527 from esl/check-and-fix-shapers

pawlooss1 · web-flow · commit 5453f6dbb406 · 2025-05-12T09:03:24.000+02:00
Validate and fix XMPP traffic shapers
diff --git a/big_tests/tests/mod_global_distrib_SUITE.erl b/big_tests/tests/mod_global_distrib_SUITE.erl
@@ -159,11 +159,13 @@ init_per_group(start_checks, Config) ->
     dynamic_modules:ensure_modules(NodeSpec, domain(), [{mod_global_distrib, stopped}]),
     Config1;
 init_per_group(multi_connection, Config) ->
+    Config1 = mongoose_helper:backup_and_set_config_option(
+                Config, [shaper, normal, max_rate], 100000), % needed to send many messages quickly
     ExtraConfig = #{bounce => #{resend_after_ms => 20000},
                     connections => #{%% Disable unused feature to avoid interference
                                      connections_per_endpoint => 100,
                                      disabled_gc_interval => 10000}},
-    init_per_group_generic([{extra_config, ExtraConfig} | Config]);
+    init_per_group_generic([{extra_config, ExtraConfig} | Config1]);
 init_per_group(invalidation, Config) ->
     Config1 = init_per_group(invalidation_generic, Config),
     NodeBin = <<"fake_node@localhost">>,
@@ -255,6 +257,9 @@ end_per_group(invalidation, Config) ->
     redis_query(europe_node1, [<<"HDEL">>, ?config(nodes_key, Config),
                             ?config(node_to_expire, Config)]),
     end_per_group_generic(Config);
+end_per_group(multi_connection, Config) ->
+    mongoose_helper:restore_config_option(Config, [shaper, normal, max_rate]),
+    end_per_group_generic(Config);
 end_per_group(_, Config) ->
     end_per_group_generic(Config).
 
diff --git a/doc/configuration/access.md b/doc/configuration/access.md
@@ -50,40 +50,6 @@ The `blocked` access class can be defined in the [`acl` section](acl.md) and mat
 
 For this rule to take effect, it needs to be referenced in the options of a [C2S listener](../listeners/listen-c2s.md#listenc2saccess).
 
-### C2S Shaper
-
-The `c2s_shaper` rule is used to determine the shaper used to limit the incoming traffic on C2S connections:
-
-```toml
-  c2s_shaper = [
-    {acl = "admin", value = "none"},
-    {acl = "all", value = "normal"}
-  ]
-```
-
-It has the following logic:
-
-* if the access class is `admin`, the returned value is `"none"`,
-* otherwise, the returned value is `"normal"`.
-
-The `admin` access class can be defined in the `acl` to specify admin users who will bypass the `normal` shaper.
-
-For this rule to take effect, it needs to be referenced in the options of a [C2S listener](../listeners/listen-c2s.md#listenc2sshaper).
-
-### S2S Shaper
-
-The `s2s_shaper` rule is used to determine the shaper used to limit the incoming traffic on C2S connections:
-
-```toml
-  s2s_shaper = [
-    {acl = "all", value = "fast"}
-  ]
-```
-
-It assigns the `fast` shaper to all S2S connections.
-
-For this rule to take effect, it needs to be referenced in the options of an [S2S listener](../listeners/listen-s2s.md#listens2sshaper).
-
 ### MUC
 
 The following rules manage the permissions of MUC operations:
diff --git a/doc/configuration/listen.md b/doc/configuration/listen.md
@@ -95,7 +95,7 @@ The number of processes accepting new connections on the listening socket.
 ### `listen.*.shaper`
 * **Syntax:** string, shaper name
 * **Default:** `"none"` (no shaper)
-* **Example:** `shaper = "c2s_shaper"`
+* **Example:** `shaper = "normal"`
 
 The shaper name that determines what traffic shaper is used to limit the incoming XMPP traffic to prevent the server from being flooded with incoming data.
 The shaper referenced here needs to be defined in the [`shaper`](../configuration/shaper.md) configuration section.
diff --git a/doc/configuration/shaper.md b/doc/configuration/shaper.md
@@ -31,8 +31,7 @@ This is the typical definition of an XMPP shaper, which accepts the maximum data
   max_rate = 1000
 ```
 
-To make use of it, the [corresponding rule](access.md#c2s-shaper) should be defined in the `access` section.
-Finally, the C2S listener has to be configured to use the defined shaper - see the [C2S Example](../listeners/listen-c2s.md#c2s-listener-configuration-example).
+To make use of it, the C2S listener has to be configured to use the defined shaper - see the [C2S Example](../listeners/listen-c2s.md#c2s-listener-configuration-example).
 
 ### S2S Shaper
 
@@ -43,8 +42,7 @@ For S2S connections we need to increase the limit as they receive the accumulate
   max_rate = 50_000
 ```
 
-To make use of it, the [corresponding rule](access.md#s2s-shaper) should be defined in the `access` section.
-Finally, the S2S listener has to be configured to use the defined shaper - see the [S2S Example](../listeners/listen-s2s.md#s2s-listener-configuration-example).
+To make use of it, the S2S listener has to be configured to use the defined shaper - see the [S2S Example](../listeners/listen-s2s.md#s2s-listener-configuration-example).
 
 ### MAM Shapers
 
diff --git a/doc/listeners/listen-c2s.md b/doc/listeners/listen-c2s.md
@@ -164,19 +164,20 @@ The following section configures two C2S listeners.
 [[listen.c2s]]
   port = 5222
   access = "c2s"
-  shaper = "c2s_shaper"
+  shaper = "normal"
   max_stanza_size = 65536
   tls.certfile = "server.pem"
   tls.dhfile = "dh_server.pem"
 
 [[listen.c2s]]
   port = 5223
   access = "c2s"
-  shaper = "c2s_shaper"
+  shaper = "normal"
   max_stanza_size = 65536
 ```
 
 * One at port 5222, which accepts a plain TCP connection and allows to use StartTLS for upgrading it to an encrypted one. The files containing the certificate and the DH parameter are also provided.
 * One at port 5223, which accepts only encrypted TLS connections. It is called Direct TLS.
 
-Both listeners use `c2s` and `c2s_shaper` rules for access management and traffic shaping, respectively.
+Both listeners use the `c2s` access rule and the `normal` traffic shaper.
+They need to be defined in the [`access`](../configuration/access.md) and [`shaper`](../configuration/shaper.md) sections, respectively (the default configuration file includes them).
diff --git a/doc/listeners/listen-s2s.md b/doc/listeners/listen-s2s.md
@@ -16,12 +16,12 @@ They have the same semantics as the corresponding [c2s options](listen-c2s.md#tl
 ## S2S listener configuration example
 
 The following section configures an S2S listener with some basic settings set up.
-The `s2s_shaper` access rule is used, which requires a definition in the [`access`](../configuration/access.md) section.
+The `fast` shaper is used, which requires a definition in the [`shaper`](../configuration/shaper.md) section (the default configuration file includes it).
 
 ```toml
 [[listen.s2s]]
   port = 5269
-  shaper = "s2s_shaper"
+  shaper = "fast"
   max_stanza_size = 131072
   tls.dhfile = "dh_server.pem"
 ```
diff --git a/rebar.config b/rebar.config
@@ -60,7 +60,7 @@
   {prometheus_cowboy, "0.1.9"},
 
   %%% Stateless libraries
-  {opuntia, "1.1.0"},
+  {opuntia, "1.1.2"},
   {fast_scram, "0.7.0"},
   {idna, "6.1.1"},
   {uuid, "2.0.7", {pkg, uuid_erl}},
diff --git a/rebar.lock b/rebar.lock
@@ -79,7 +79,7 @@
        {ref,"e55adf8a54b8d9262ce157cf2d8f88b4b94b7023"}},
   0},
  {<<"observer_cli">>,{pkg,<<"observer_cli">>,<<"1.8.2">>},0},
- {<<"opuntia">>,{pkg,<<"opuntia">>,<<"1.1.0">>},0},
+ {<<"opuntia">>,{pkg,<<"opuntia">>,<<"1.1.2">>},0},
  {<<"p1_utils">>,{pkg,<<"p1_utils">>,<<"1.0.26">>},1},
  {<<"pa">>,
   {git,"https://github.com/erszcz/pa.git",
@@ -160,7 +160,7 @@
  {<<"mimerl">>, <<"D0CD9FC04B9061F82490F6581E0128379830E78535E017F7780F37FEA7545726">>},
  {<<"mysql">>, <<"3BAD7F35107E141989B0B4FAB47C9E73626BC5FEF38EDF8CCFDA5EC3EFA83B30">>},
  {<<"observer_cli">>, <<"9962E6818E75774EC829875016BE61A6BCA87E95AD18ECD7FBCF4F23F1278C57">>},
- {<<"opuntia">>, <<"96DBDF5A1A1D9CEEE6649F08E27B7A4AC8FFFFCFA88404242327130122355766">>},
+ {<<"opuntia">>, <<"A781E231032F892F930E60ACB00F8DF6AFDE3F035F66A5C760320C8E0FA41A3F">>},
  {<<"p1_utils">>, <<"67B0C4AC9FA3BA3EF563B31AA111B0A004439A37FAC85E027F1C3617E1C7EC6C">>},
  {<<"parse_trans">>, <<"BB87AC362A03CA674EBB7D9D498F45C03256ADED7214C9101F7035EF44B798C7">>},
  {<<"pooler">>, <<"898CD1FA301FC42D4A8ED598CE139B71CA85B54C16AB161152B5CC5FBDCFA1A8">>},
@@ -228,7 +228,7 @@
  {<<"mimerl">>, <<"A1E15A50D1887217DE95F0B9B0793E32853F7C258A5CD227650889B38839FE9D">>},
  {<<"mysql">>, <<"186D7E6DF9CAD33BD9F8F61856F45F2F1962F80B645F163DA744F66C626DA63E">>},
  {<<"observer_cli">>, <<"93AE523D42D566B176F7AE77A0BF36802DAB8BB51A6086316CCE66A7CFB5D81F">>},
- {<<"opuntia">>, <<"8F70DAA6BD35AE58D96061CDB0FC2AF79DB5C73B0F7564B8BEC2B2DEB703EB8F">>},
+ {<<"opuntia">>, <<"76D3C52362E63D1A55CF76209E1324D80A2E853600C2485D44D6F57E963F1A10">>},
  {<<"p1_utils">>, <<"D0379E8C1156B98BD64F8129C1DE022FCCA4F2FDB7486CE73BF0ED2C3376B04C">>},
  {<<"parse_trans">>, <<"F99E368830BEA44552224E37E04943A54874F08B8590485DE8D13832B63A2DC3">>},
  {<<"pooler">>, <<"058D85C5081289B90E97E4DDDBC3BB5A3B4A19A728AB3BC88C689EFCC36A07C7">>},
diff --git a/rel/files/mongooseim.toml b/rel/files/mongooseim.toml
@@ -127,7 +127,7 @@
 [[listen.c2s]]
   port = {{{c2s_port}}}
   access = "c2s"
-  shaper = "c2s_shaper"
+  shaper = "normal"
   max_stanza_size = 65536
   {{#tls_config}}
   {{{tls_config}}}
@@ -145,7 +145,7 @@
 
 [[listen.s2s]]
   port = {{{incoming_s2s_port}}}
-  shaper = "s2s_shaper"
+  shaper = "fast"
   max_stanza_size = 131072
   {{#tls_config}}
   {{{tls_config}}}
@@ -304,15 +304,6 @@
     {acl = "all", value = "allow"}
   ]
 
-  c2s_shaper = [
-    {acl = "admin", value = "none"},
-    {acl = "all", value = "normal"}
-  ]
-
-  s2s_shaper = [
-    {acl = "all", value = "fast"}
-  ]
-
   muc_admin = [
     {acl = "admin", value = "allow"}
   ]
diff --git a/rel/mim1.vars-toml.config b/rel/mim1.vars-toml.config
@@ -68,7 +68,7 @@
   "[[listen.c2s]]
   port = {{ c2s_tls_port }}
   access = \"c2s\"
-  shaper = \"c2s_shaper\"
+  shaper = \"normal\"
   max_stanza_size = 65536
   tls.certfile = \"priv/ssl/fake_server.pem\"
   tls.cacertfile = \"priv/ssl/cacert.pem\"
diff --git a/rel/mim2.vars-toml.config b/rel/mim2.vars-toml.config
@@ -33,7 +33,7 @@
   "[[listen.c2s]]
   port = {{ c2s_tls_port }}
   access = \"c2s\"
-  shaper = \"c2s_shaper\"
+  shaper = \"normal\"
   max_stanza_size = 65536
   tls.cacertfile = \"priv/ssl/cacert.pem\"
   tls.certfile = \"priv/ssl/fake_server.pem\"
diff --git a/rel/mim3.vars-toml.config b/rel/mim3.vars-toml.config
@@ -44,7 +44,7 @@
   "[[listen.c2s]]
   port = {{ c2s_tls_port }}
   access = \"c2s\"
-  shaper = \"c2s_shaper\"
+  shaper = \"normal\"
   max_stanza_size = 65536
   tls.cacertfile = \"priv/ssl/cacert.pem\"
   tls.certfile = \"priv/ssl/fake_server.pem\"
diff --git a/src/config/mongoose_config_spec.erl b/src/config/mongoose_config_spec.erl
@@ -10,6 +10,7 @@
 
 %% callbacks for the 'process' step
 -export([process_dynamic_domains/1,
+         process_shapers/1,
          process_host/1,
          process_general/1,
          process_listener/2,
@@ -105,7 +106,8 @@ root() ->
                 },
        defaults = #{<<"internal_databases">> => default_internal_databases()},
        required = [<<"general">>],
-       process = fun ?MODULE:process_dynamic_domains/1,
+       process = [fun ?MODULE:process_dynamic_domains/1,
+                  fun ?MODULE:process_shapers/1],
        wrap = none,
        format_items = list
       }.
@@ -926,6 +928,25 @@ process_dynamic_domains(Items) ->
             Items
     end.
 
+%% Check that all referenced shapers are defined in the 'shaper' section
+process_shapers(Items) ->
+    S2SShapers = [Shaper || {{s2s, _}, #{outgoing := #{shaper := Shaper}}} <- Items],
+    ListenerShapers = [Shaper || #{shaper := Shaper} <- proplists:get_value(listen, Items)],
+    ReferencedShapers = lists:usort(S2SShapers ++ ListenerShapers) -- [none],
+    DefinedShapers = case proplists:lookup(shaper, Items) of
+                         none -> [];
+                         {_, ShaperOpts} -> maps:keys(ShaperOpts)
+                     end,
+    case ReferencedShapers -- DefinedShapers of
+        [] ->
+            Items;
+        UndefinedShapers ->
+            error(#{what => undefined_shapers,
+                    text => ("The configuration references undefined shapers. "
+                             "You need to define them in the 'shaper' section."),
+                    undefined_shapers => UndefinedShapers})
+    end.
+
 unsupported_auth_methods(KVs) ->
     [Method || Method <- extract_auth_methods(KVs),
                not ejabberd_auth:does_method_support(Method, dynamic_domains)].
diff --git a/src/listeners/mongoose_listener.erl b/src/listeners/mongoose_listener.erl
@@ -39,7 +39,7 @@
                      num_acceptors => pos_integer(),
                      proxy_protocol => boolean(),
                      reuse_port => boolean(),
-                     shaper => atom(),
+                     shaper => mongoose_shaper:name(),
                      %% C2S
                      allowed_auth_methods => [atom()],
                      backwards_compatible_session => boolean(),
diff --git a/src/mongoose_shaper.erl b/src/mongoose_shaper.erl
@@ -4,8 +4,9 @@
 -export([new/1, update/2, wait/5, reset_all_shapers/1]).
 -ignore_xref([reset_all_shapers/1]).
 
+-type name() :: atom(). % special value 'none' means no shaper
 -type shaper() :: opuntia:shaper().
--export_type([shaper/0]).
+-export_type([name/0, shaper/0]).
 
 -spec child_spec() -> supervisor:child_spec().
 child_spec() ->
@@ -16,7 +17,7 @@ child_spec() ->
       shutdown => infinity,
       type => supervisor}.
 
--spec new(atom()) -> opuntia:shaper().
+-spec new(name()) -> opuntia:shaper().
 new(Name) ->
     opuntia:new(get_shaper_config(Name)).
 
@@ -49,22 +50,17 @@ reset_all_shapers(_HostType) ->
 -spec get_shaper_name(HostType :: mongooseim:host_type_or_global(),
                       Domain :: global | jid:server(),
                       Action :: atom(),
-                      FromJID :: jid:jid()) -> allow | none.
+                      FromJID :: jid:jid()) -> name().
 get_shaper_name(HostType, Domain, Action, FromJID) ->
     case acl:match_rule(HostType, Domain, Action, FromJID) of
-        deny -> default_shaper();
-        Value -> Value
+        deny -> none;
+        ShaperName -> ShaperName
     end.
 
--spec get_shaper_config(atom()) -> number().
+-spec get_shaper_config(name()) -> opuntia:config().
+get_shaper_config(none) ->
+    0;
 get_shaper_config(Name) ->
-    case mongoose_config:lookup_opt([shaper, Name]) of
-        {ok, #{max_rate := MaxRatePerSecond}} ->
-            #{bucket_size => MaxRatePerSecond, rate => MaxRatePerSecond,
-              time_unit => second, start_full => true};
-        {error, not_found} ->
-            0
-    end.
-
-default_shaper() ->
-    none.
+    MaxRatePerSecond = mongoose_config:get_opt([shaper, Name, max_rate]),
+    #{bucket_size => MaxRatePerSecond, rate => MaxRatePerSecond,
+      time_unit => second, start_full => true}.
diff --git a/test/common/config_parser_helper.erl b/test/common/config_parser_helper.erl
@@ -152,7 +152,7 @@ options("mongooseim-pgsql") ->
       [config([listen, c2s],
               #{port => 5222,
                 access => c2s,
-                shaper => c2s_shaper,
+                shaper => normal,
                 max_stanza_size => 65536,
                 tls => #{certfile => "priv/cert.pem",
                          keyfile => "priv/dc1.pem",
@@ -161,7 +161,7 @@ options("mongooseim-pgsql") ->
        config([listen, c2s],
               #{port => 5223,
                 access => c2s,
-                shaper => c2s_shaper,
+                shaper => normal,
                 max_stanza_size => 65536
                }),
        config([listen, component],
@@ -241,7 +241,7 @@ options("mongooseim-pgsql") ->
                }),
        config([listen, s2s],
               #{port => 5269,
-                shaper => s2s_shaper,
+                shaper => fast,
                 max_stanza_size => 131072,
                 tls => #{cacertfile => "priv/ca.pem",
                          certfile => "priv/cert.pem",
@@ -775,8 +775,6 @@ custom_s2s() ->
 pgsql_access() ->
     #{c2s => [#{acl => blocked, value => deny},
               #{acl => all, value => allow}],
-      c2s_shaper => [#{acl => admin, value => none},
-                     #{acl => all, value => normal}],
       local => [#{acl => local, value => allow}],
       mam_get_prefs => [#{acl => all, value => default}],
       mam_get_prefs_global_shaper => [#{acl => all, value => mam_global_shaper}],
@@ -793,8 +791,7 @@ pgsql_access() ->
       muc => [#{acl => all, value => allow}],
       muc_admin => [#{acl => admin, value => allow}],
       muc_create => [#{acl => local, value => allow}],
-      register => [#{acl => all, value => allow}],
-      s2s_shaper => [#{acl => all, value => fast}]}.
+      register => [#{acl => all, value => allow}]}.
 
 pool_config(PoolIn = #{type := Type}) ->
     config([outgoing_pools, Type, maps:get(tag, PoolIn, default)], PoolIn).
diff --git a/test/config_parser_SUITE.erl b/test/config_parser_SUITE.erl
diff --git a/test/config_parser_SUITE_data/mongooseim-pgsql.toml b/test/config_parser_SUITE_data/mongooseim-pgsql.toml
