Enabling mod_privilege prevents component from sending un-privileged IQ

[mtstickney]

Environment

• ejabberd version: 24.12
• Erlang version: Erlang (SMP,ASYNC_THREADS) (BEAM) emulator version 15.1.3
• OS: Linux (Arch Linux)
• Installed from: distro package

Configuration (only if needed):

listen:
  -
    port: 5101
    ip: "127.0.0.1"
    module: ejabberd_service
    hosts:
      matridge.aptnet.home.arpa:
        password: <secret>
acl:
  slidge_gateways:
    - server: "matridge.aptnet.home.arpa"
access_rules:
  slidge_gateways_rule:
    allow: slidge_gateways
modules:
  mod_privilege:
    roster:
      both: slidge_gateways_rule

Errors from error.log/crash.log

2025-01-11 01:22:53.438 [info] (#PID<0.703.0>) Accepted connection 127.0.0.1:55922 -> 127.0.0.1:5101
2025-01-11 01:22:53.442 [info] (tcp|<0.703.0>) Accepted external component handshake authentication for matridge.aptnet.home.arpa from 127.0.0.1
2025-01-11 01:22:53.444 [info] Granting permissions to external component 'matridge.aptnet.home.arpa': roster = both, presence = none, message = none,
iq = []
2025-01-11 01:22:55.449 [info] IQ not forwarded: Component tried to send not wrapped IQ stanza.

Bug description

(This is behavior observed with matridge, a slidge-based transport, but I've also replicated it with my own simpler script using the XEP-0114 handshake, because I didn't want to try to figure out how to hack up SASL and the like. I'm not completely sure what component protocol slidge uses (XEP-0225? XEP-0114? something else?), but hopefully the differences won't matter.)

If ejabberd is configured with mod_privilege enabled, even if it is only granting non-IQ permissions like outgoing message support or roster privileges, the server declines to forward non-privileged IQ stanzas, complaining that it is non-wrapped (see logs above). Enabling mod_privilege seems to require components to only send privileged IQs, and prevents them from sending any IQ packet on their own behalf.

An example stanza that triggers the log message:

<iq type="get" to="[email protected]" from="matridge.aptnet.home.arpa"
        id="b212a556567e4a53aa42b5ca9658e9d5">
  <pubsub xmlns="http://jabber.org/protocol/pubsub">
    <items node="urn:xmpp:avatar:metadata" />
  </pubsub>
</iq>

As best I can tell, this is a bug: if mod_privilege is not enabled, the server doesn't complain about forwarding non-privileged IQs -- I can't prove they're actually processed usefully, but I don't see any evidence they aren't. Since components seem to be permitted to send IQs on their own behalf when mod_privilege is disabled, I'd expect them to continue to be able to do so when they've been granted extra privileges (but I'm not an XMPP expert, so maybe I've got some part of this wrong).

[badlop]

Looking at the source code and process flow, this is what I found:

1. When a component sends a stanza, ejabberd_service:handle_authenticated_packet runs the hook component_send_packet.
2. A) When mod_privilege is disabled, that hook does nothing, the packet is returned and routed.
3. B) When mod_privilege is enabled, mod_privilege:component_send_packet is executed,
4. and it calls get_iq_encapsulated_details
5. As that example IQ stanza doesn't have any privileged_iq, the function get_iq_encapsulated_details returns {error, no_privileged_iq, _}
6. Then component_send_packet interprets that as a problem, consequently logs "IQ not forwarded: Component tried to send not wrapped IQ stanza." and drops the stanza.

I agree that the problem is in step 5: if a stanza does not have privileged_iq, then simply mod_privilege should not intervene, as there's nothing in XEP-0356 that describes that case, nothing specifically in the IQ permission section.

Nobody else have complained about this problem because that code is quite recent (four months ago, 81906b7 ) and few software supports it (apparently only Slidge and ejabberd).

In that case, the solution is to let the packet pass as your PR does, and also rewrite the INFO_MSG into a DEBUG that simply informs, something like "Component sent an IQ stanza without wrapped IQ content, so it will be routed"

[mtstickney]

Oops, I forgot about the log message. I've updated the PR to convert it to a debug message with clearer info than the original thing (if you'd like the message changed, let me know).

I note that roster-query packets do the same thing (get_iq_encapsulated_details returns {error, roster_query, _}, and component_send_packet just returns the original packet for routing) and don't log any messages. Should logging be added there, or is it reasonable to just have it for IQ packets?

[badlop]

Should logging be added there, or is it reasonable to just have it for IQ packets?

Ahh, right! Umm, I guess it's preferable to add DEBUG lines to both cases: this protocol is fairly new, there are few component implementations, only this server implementation, and maybe new component developers use ejabberd to test their component, and the more debug information ejabberd can give, the better.

a debug message with clearer info than the original thing (if you'd like the message changed, let me know).

Notice that the protocol says that the server will send the encapsulated <iq/> as if it were sent by Juliet herself

In ejabberd, all the other INFO_MSG sentences say "IQ forwarded" or "not forwarded": that IQ refers to the encapsulated IQ, it does not refer to the IQ stanza that the component sent, right?

Given that usage of the term "forward", the log sentence "Forwarding non-privileged IQ stanza from privileged component ~ts" is misleading, as the IQ stanza was not "forwarded", in the same sense than the other INFO sentences: the component didn't send any encapsulated IQ to forward.

That's why I didn't use the term "forward" in my example DEBUG sentence, and preferred to use the term "route", and why I gave so much details about what ejabberd received: to help potential component developers.

[mtstickney]

I was adding the log messages in here, and it occurred to me that the roster-query exception is just a special case of the general rule that any IQ stanza whose payload isn't a privileged_iq gets routed normally.

Would it make sense to remove the roster-query return from get_encapsulated_iq_details and the branch for it in component_send_packet, and let it be handled by the general not-wrapped case?

On the one hand you'd lose an opportunity to log something more specific for roster queries, but on the other there's no special treatment for any of the other possible IQ payloads either.

[truenicoco]

I think this might be related to https://codeberg.org/slidge/slidge-whatsapp/issues/1

This IQ sent by slidge is never replied-to:

<iq id="c7146596-06a2-4672-bec0-89d99ff8a571" to="[email protected]" from="whatsapp.6x6.ch" type="set">
 <privileged_iq xmlns="urn:xmpp:privilege:2">
  <iq xmlns="jabber:client" type="set" to="[email protected]" from="[email protected]" id="c7146596-06a2-4672-bec0-89d99ff8a571">
    <pubsub xmlns="http://jabber.org/protocol/pubsub#owner">
     <affiliations node="urn:xmpp:mds:displayed:0">
      <affiliation jid="whatsapp.6x6.ch" affiliation="member" />
</affiliations></pubsub></iq></privileged_iq></iq>

and ejabberd logs "[info] IQ not forwarded: Component tried to send not wrapped IQ stanza."

Apologies if this is not related and/or slidge is actually the culprit here...

[mtstickney]

@truenicoco those are actually two different errors. The "Component tried to send not wrapped IQ stanza" is this issue, but that's only when sending a regular non-privileged IQ payload (the bit you've quoted there is a <privileged_iq> payload, so it won't trigger this). Fetching avatar data is the culprit with matridge, seems possible that's the trigger with slidge-whatsapp too.

The second problem is that ejabberd has some funky behavior with privileged IQ stanzas that are "self-addressed" (the stanza is to the same user that is being impersonated), which presents as never getting a reply to privileged IQs. It's more complicated than that, but this results in a lot of IQ timeouts in matridge.

I was planning to open a discussion about that because I'm not sure why it is implemented that way, but it's separate from this particular problem.

[truenicoco]

Oh OK thanks for the reply. I don't use ejabberd myself and I see that you already went much further than me investigating those mod_privilege issues, so good luck. 😃
Ping me if I can help.

[mtstickney]

I've updated the PR to divide processing into privileged IQs and everything else, rather than preserving the special case for roster-query packets (this includes the debug message added earlier). I can revert and double up the logging messages if you like, but I think this is a cleaner approach.