From d386ded6b06727fffed4248a4d790cefac7fc688 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Sch=C3=A4fer?= Date: Wed, 29 Mar 2023 17:48:26 +0200 Subject: [PATCH 1/2] Delegate authorization from groups.$DOMAIN to $DOMAIN This allows us to set the avatar of circles both from the app and from the web portal (with snikket-web-portal#150). However, this also makes all admins owners in all circles. This may be problematic, or may not be. The upside is that it also automatically allows managing the avatar through the apps. The downside is that it exposes a bunch of dangerous controls (banning, kicking) which desyncs the group membership from MUC membership. We might want a reconciliation loop for that, or figure out something else which overrides outcast-ness or forbids banning or stuff like that, *or* which alternatively reflects that change in the circle UI. --- ansible/files/prosody.cfg.lua | 3 +++ ansible/snikket.yml | 2 +- ansible/tasks/prosody.yml | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/ansible/files/prosody.cfg.lua b/ansible/files/prosody.cfg.lua index 78d0054..5dbb4c2 100644 --- a/ansible/files/prosody.cfg.lua +++ b/ansible/files/prosody.cfg.lua @@ -267,6 +267,9 @@ Component ("groups."..DOMAIN) "muc" restrict_room_creation = "local" muc_local_only = { "general@groups."..DOMAIN } + authorization = "delegate" + authz_delegate_to = DOMAIN + -- Default configuration for rooms (typically overwritten by the client) muc_room_default_allow_member_invites = true muc_room_default_persistent = true diff --git a/ansible/snikket.yml b/ansible/snikket.yml index e10e71c..e2c2c3a 100644 --- a/ansible/snikket.yml +++ b/ansible/snikket.yml @@ -9,7 +9,7 @@ package: "prosody-trunk" snapshot: "2023-03-31" prosody_modules: - revision: "5178c13deb78" + revision: "98d5acb93439" tasks: - import_tasks: tasks/prosody.yml - import_tasks: tasks/supervisor.yml diff --git a/ansible/tasks/prosody.yml b/ansible/tasks/prosody.yml index acb30a0..816e453 100644 --- a/ansible/tasks/prosody.yml +++ b/ansible/tasks/prosody.yml @@ -115,6 +115,7 @@ - mod_measure_malloc - mod_http_xep227 - mod_portcheck + - mod_authz_delegate - name: Enable wanted modules (snikket-modules) file: From 0e2b7fc6290f0e09457b4fc12141394e7e1c8f4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Sch=C3=A4fer?= Date: Fri, 31 Mar 2023 16:57:56 +0200 Subject: [PATCH 2/2] Introduce module to prevent accidental affiliation changes in circles With mod_authz_delegate (see parent commit), all Snikket admins are now owners in all circle MUCs (and non-circle MUCs, for that matter). This implies that they're able to change affiliations. As that is a footgun which may cause desyncs between the circle state and the MUC membership list, we block both invites and affiliation changes, unless triggered by code. --- ansible/files/prosody.cfg.lua | 2 + ansible/tasks/prosody.yml | 1 + .../mod_snikket_circle_muc_protection.lua | 75 +++++++++++++++++++ 3 files changed, 78 insertions(+) create mode 100644 snikket-modules/mod_snikket_circle_muc_protection/mod_snikket_circle_muc_protection.lua diff --git a/ansible/files/prosody.cfg.lua b/ansible/files/prosody.cfg.lua index 5dbb4c2..06f6414 100644 --- a/ansible/files/prosody.cfg.lua +++ b/ansible/files/prosody.cfg.lua @@ -263,9 +263,11 @@ Component ("groups."..DOMAIN) "muc" "muc_offline_delivery"; "snikket_restricted_users"; "muc_auto_reserve_nicks"; + "snikket_circle_muc_protection"; } restrict_room_creation = "local" muc_local_only = { "general@groups."..DOMAIN } + circle_muc_protection_main_domain = DOMAIN authorization = "delegate" authz_delegate_to = DOMAIN diff --git a/ansible/tasks/prosody.yml b/ansible/tasks/prosody.yml index 816e453..609c134 100644 --- a/ansible/tasks/prosody.yml +++ b/ansible/tasks/prosody.yml @@ -130,6 +130,7 @@ - mod_snikket_client_id - mod_snikket_ios_preserve_push - mod_snikket_restricted_users + - mod_snikket_circle_muc_protection - name: "Install lua-ossl for encrypted push notifications" apt: diff --git a/snikket-modules/mod_snikket_circle_muc_protection/mod_snikket_circle_muc_protection.lua b/snikket-modules/mod_snikket_circle_muc_protection/mod_snikket_circle_muc_protection.lua new file mode 100644 index 0000000..e35a792 --- /dev/null +++ b/snikket-modules/mod_snikket_circle_muc_protection/mod_snikket_circle_muc_protection.lua @@ -0,0 +1,75 @@ +--[[ +The purpose of this module is to avoid accidental modification of the member +list by users. + +With mod_authz_delegate enabled, snikket admins suddenly are granted ownership +on circle MUCs; while this is great for setting avatars, it is problematic +when considering that this also allows affiliation changes which would not be +reflected in the circle membership, causing all kinds of desyncs. + +This is a first order solution to this challenge. Future solutions may allow +more modifications as well as synchronisation between MUC affiliation lists +and circle membership or stuff like that. + +We want to get a release shipped which allows setting avatars first :-). +]] +local modulemanager = require"core.modulemanager"; +local st = require "prosody.util.stanza"; + +local main_host_name = module:get_option("circle_muc_protection_main_domain"); +local main_host_groups = nil; + +local function get_groups_module() + if main_host_groups then + return main_host_groups; + end + + module:log("debug", "lazy-initializing MUC protection module"); + if not main_host_name then + return error("main host name required for circle MUC affiliation protection") + end + + local target_module = modulemanager.get_module(main_host_name, "groups_internal"); + if not target_module then + return error("groups_internal not available on "..main_host_name); + end + + main_host_groups = target_module; + return target_module; +end + +local function get_muc_circle(muc_jid) + for group_id in get_groups_module().groups() do + local group_data = main_host_groups.get_info(group_id); + if group_data.muc_jid == muc_jid then + return group_id + end + end + return nil +end + +module:hook("muc-pre-set-affiliation", function(event) + if event.actor == nil then + module:log("debug", "affiliation change in %s granted because the actor is nil.", event.room.jid); + return; + end + local group_id = get_muc_circle(event.room.jid); + if group_id ~= nil then + module:log("warn", "affiliation change blocked as %s is associated with circle %s", event.room.jid, group_id); + event.allowed = false; + else + module:log("debug", "affiliation change not blocked as %s is not associated with any circle", event.room.jid); + end +end); + +module:hook("muc-pre-invite", function(event) + local room = event.room; + local group_id = get_muc_circle(room.jid); + if group_id ~= nil then + module:log("warn", "invite blocked as %s is associated with circle %s", room.jid, group_id); + event.origin.send(st.error_reply(event.stanza, "cancel", "not-allowed", nil, room.jid)); + return true; + else + module:log("debug", "invite not blocked as %s is not associated with any circle", event.room.jid); + end +end, -1); -- run after the main members_only permission check