Problem with ip_access option for mod_register when using append_host_config

[Dunedan]

I want to disallow registrations for one virtual host from certain networks. However, I noticed this only works when configuring the ACL for all hosts, but gets silently ignored when configuring it as host-specific option using append_host_config.

I'm able to reproduce that behavior with the ghcr.io/processone/ejabberd Docker image with the following diff compared to the default config:

--- a/ejabberd.yml.orig
+++ b/ejabberd.yml
@@ -88,37 +88,50 @@ listen:
 
 s2s_use_starttls: optional
 
 acl:
   local:
     user_regexp: ""
   loopback:
     ip:
       - 127.0.0.0/8
       - ::1/128
+#  blocked_networks:
+#    ip:
+#      - 0.0.0.0/0
+#      - ::/0
+
+append_host_config:
+  localhost:
+    acl:
+      blocked_networks:
+        ip:
+          - 0.0.0.0/0
+          - ::/0
 
 access_rules:
   local:
     allow: local
   c2s:
     deny: blocked
     allow: all
   announce:
     allow: admin
   configure:
     allow: admin
   muc_create:
     allow: local
   pubsub_createnode:
     allow: local
   trusted_network:
-    allow: loopback
+    deny: blocked_networks
+    allow: all
 
 api_permissions:
   "console commands":
     from:
       - ejabberd_ctl
     who: all
     what: "*"
   "admin access":
     who:
       access:

When using the commented out blocked_networks ACL, registrations get properly blocked, however when using the one in append_host_config instead, registrations are still possible.

Am I doing something wrong here or is this a bug?
Is the host maybe not considered for unregistered/unauthenticated connections?

[Neustradamus]

@badlop, @prefiks, @weiss: What do you think?

[prefiks]

mod_register has also option access that i think you could use like this:

modules:
  mod_register:
    access:
      - deny:
        - server: "blocked.domain.com"
      - allow: all

[Dunedan]

I'm trying to block by IP CIDRs, so I don't get how using the access option instead would help. Aside from that: Wouldn't I encounter the same problem regarding the use of virtual hosts with that as well?

[prefiks]

Ah, didn't notice that "blocking only some ip" part. I think you could do that by having different value of ip_acces in mod_register per host:

append_host_config:
  "host1.example.org"
    modules:
      mod_register:
        ip_access: access_for_host1
  "host2.example.org"
    modules:
      mod_register:
        ip_access: access_for_host2

[Dunedan]

While that to works, this doesn't have the CIDRs defined as part of the host config anymore.

From my understanding having them as part of the host config would be a prerequisite for being able to move them to a separate file and include them using include_config_file, which is something I'd like to do to keep the main config file short and clear. Is there a way to achieve that?

[badlop]

I think this patch solves that problem. I include in the patch the configuration change that works for me:

That configuration blocks registration from 127.0.0.1 in example.com, and allows registration from 127.0.0.1 in example.org

If you are still around, interested, and capable of testing it, please comment your results.

From 0a9cb59961d07a479fc250fa60b22da7251aaa93 Mon Sep 17 00:00:00 2001
From: Badlop <badlop@process-one.net>
Date: Tue, 27 Aug 2024 13:33:27 +0200
Subject: [PATCH] Add support to block certain IPs in a specific vhost using
 append_host_config

---
 ejabberd.yml.example | 13 ++++++++++++-
 src/mod_register.erl | 12 ++++++------
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/ejabberd.yml.example b/ejabberd.yml.example
index 27cf8e4c5..466416f9a 100644
--- a/ejabberd.yml.example
+++ b/ejabberd.yml.example
@@ -16,6 +16,8 @@
 
 hosts:
   - localhost
+  - example.com
+  - example.org
 
 loglevel: info
 
@@ -92,6 +94,14 @@ acl:
       - 127.0.0.0/8
       - ::1/128
 
+append_host_config:
+  example.com:
+    acl:
+      blocked_networks:
+        ip:
+          - 127.0.0.0/8
+          - ::1/128
+
 access_rules:
   local:
     allow: local
@@ -107,7 +117,8 @@ access_rules:
   pubsub_createnode:
     allow: local
   trusted_network:
-    allow: loopback
+    deny: blocked_networks
+    allow: all
 
 api_permissions:
   "console commands":
diff --git a/src/mod_register.erl b/src/mod_register.erl
index d656263bf..6baea8cbb 100644
--- a/src/mod_register.erl
+++ b/src/mod_register.erl
@@ -573,24 +573,24 @@ may_remove_resource(From) -> From.
 get_ip_access(Host) ->
     mod_register_opt:ip_access(Host).
 
-check_ip_access({User, Server, Resource}, IPAccess) ->
+check_ip_access(Server, {User, Server, Resource}, IPAccess) ->
     case ejabberd_sm:get_user_ip(User, Server, Resource) of
         {IPAddress, _PortNumber} ->
-            check_ip_access(IPAddress, IPAccess);
+            check_ip_access(Server, IPAddress, IPAccess);
         _ ->
             deny
     end;
-check_ip_access(undefined, _IPAccess) ->
+check_ip_access(_Server, undefined, _IPAccess) ->
     deny;
-check_ip_access(IPAddress, IPAccess) ->
-    acl:match_rule(global, IPAccess, IPAddress).
+check_ip_access(Server, IPAddress, IPAccess) ->
+    acl:match_rule(Server, IPAccess, IPAddress).
 
 check_access(User, Server, Source) ->
     JID = jid:make(User, Server),
     Access = mod_register_opt:access(Server),
     IPAccess = get_ip_access(Server),
     case acl:match_rule(Server, Access, JID) of
-	allow -> check_ip_access(Source, IPAccess);
+	allow -> check_ip_access(Server, Source, IPAccess);
 	deny -> deny
     end.
 
-- 
2.43.0

[Dunedan]

I tried this patch out and it works like a charm. 👍
I'd be very happy if it'd get included into ejabberd.

[badlop]

Ok, I've pushed the patch, will be included in the next ejabberd release: 15d73b9