mod_block_strangers shoud bounce instead of silently dropping

[ge0rg]

What version of ejabberd are you using?

17.09 (on a remote server that I'm trying to contact)

What did not work as expected? Are there error messages in the log? What
was the unexpected behavior? What was the expected result?

The server is deploying mod_block_strangers in the default configuration.

Messages sent to (existing) users on that server silently disappear. It is not clear to the sender if the messages are stored in offline storage, if the user just doesn't react, or if the messages are blocked.

Messages sent to non-existing users are bounced with 503 - cancel: Service unavailable.

This imposes two problems:

• you don't know if your receipient got your message (which is really bad for XMPP, where we struggle with the reliability of message delivery already)
• it is possible to enumerate user accounts by differentiating between silence and 503 responses.

Please change the behavior of mod_block_strangers to:

• bounce blocked messages to existing users with an error
• bounce messages to non-existing accounts with the same error

A naive suggestion would be: policy-violation "Messages from strangers are blocked on this server."
(https://xmpp.org/rfcs/rfc6120.html#stanzas-error-conditions-policy-violation)

[horazont]

"Account does not exist or rejects messages from strangers" would be a bit more useful for diagnostics while still not giving away any more information.

[badlop]

I guess something like this. But if it wasn't implemented that way initially, I imagine it was for a good reason...

diff --git a/src/mod_block_strangers.erl b/src/mod_block_strangers.erl
index b2c56f36b..3e0233802 100644
--- a/src/mod_block_strangers.erl
+++ b/src/mod_block_strangers.erl
@@ -65,7 +65,9 @@ filter_packet({#message{from = From} = Msg, State} = Acc) ->
 	false ->
 	    case check_message(Msg) of
 		allow -> Acc;
-		deny -> {stop, {drop, State}}
+		deny ->
+		    ejabberd_router:route_error(Msg, xmpp:err_service_unavailable()),
+		    {stop, {drop, State}}
 	    end;
 	true ->
 	    Acc

[weiss]

you don't know if your receipient got your message (which is really bad for XMPP, where we struggle with the reliability of message delivery already)

I agree we should return an error message for this reason. But ...

it is possible to enumerate user accounts by differentiating between silence and 503 responses.

Is this really a problem? Assuming mod_offline and/or mod_mam is enabled, you can do the same thing without mod_block_strangers, no? So I don't think we need to bounce messages to non-existing accounts with the same error.

[ge0rg]

P.S: please don't bounce type=error and let type=groupchat pass through ;-)

[weiss]

please don't bounce type=error

I don't think we do?

let type=groupchat pass through

I'm not sure we really want that. What's the advantage?

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.