From 570800a54077e2a3788768894dff909f854c435b Mon Sep 17 00:00:00 2001 From: Nathan Bruning Date: Sat, 27 Oct 2018 15:26:37 +0200 Subject: [PATCH 1/3] Fix mod_privacy race condition mod_privacy updates the c2s state in user_receive_packet, which tracks the *result* of the IQ set for active privacy lists. When a second stanza is sent directly after a privacy list request, the second stanza will be processed using the old privacy list, because the IQ result has not yet been routed. --- src/mod_privacy.erl | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/mod_privacy.erl b/src/mod_privacy.erl index 0b534d272..0204e0917 100644 --- a/src/mod_privacy.erl +++ b/src/mod_privacy.erl @@ -419,6 +419,27 @@ user_send_packet({#iq{type = Type, false -> IQ end, {NewIQ, State}; + +user_send_packet({#iq{type = Type, + from = #jid{luser = U, lserver = S}, + to = #jid{luser = U, lserver = S}, + sub_els = [_]} = IQ, + State}) + when Type == set -> + case xmpp:get_subtag(IQ, #privacy_query{}) of + #privacy_query{active = undefined} -> {IQ, State}; + #privacy_query{default = undefined, active = Active} -> + case get_user_list(U, S, Active) of + {ok, _} -> + % Adjust the client's state directly, so the next to-be-processed + % packet will take the active list into account. + {IQ, State#{privacy_active_list => Active}}; + true -> + {IQ, State} + end; + _ -> {IQ, State} + end; + user_send_packet(Acc) -> Acc. From aa489c5a8bac54e7f2fe4b68b42ddfb4d7b82b37 Mon Sep 17 00:00:00 2001 From: Nathan Bruning Date: Wed, 28 Nov 2018 18:34:16 +0100 Subject: [PATCH 2/3] Fix user_send_packet in mod_privacy; was failing on newly created users --- src/mod_privacy.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mod_privacy.erl b/src/mod_privacy.erl index 0204e0917..dc52825a1 100644 --- a/src/mod_privacy.erl +++ b/src/mod_privacy.erl @@ -434,7 +434,7 @@ user_send_packet({#iq{type = Type, % Adjust the client's state directly, so the next to-be-processed % packet will take the active list into account. {IQ, State#{privacy_active_list => Active}}; - true -> + _ -> {IQ, State} end; _ -> {IQ, State} From 8410a203ec81f0b0b9e4885221abf1d7ba6bd9f1 Mon Sep 17 00:00:00 2001 From: Nathan Bruning Date: Tue, 11 Dec 2018 00:19:54 +0100 Subject: [PATCH 3/3] Refactor mod_privacy patch; move logic user_receive_packet to user_send_packet. --- src/mod_privacy.erl | 68 ++++++++++++++++++++++----------------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/src/mod_privacy.erl b/src/mod_privacy.erl index dc52825a1..a25d30515 100644 --- a/src/mod_privacy.erl +++ b/src/mod_privacy.erl @@ -36,7 +36,7 @@ check_packet/4, remove_user/2, encode_list_item/1, get_user_lists/2, get_user_list/3, set_list/1, set_list/4, set_default_list/3, - user_send_packet/1, user_receive_packet/1, + user_send_packet/1, import_start/2, import_stop/2, import/5, import_info/0, mod_opt_type/1, mod_options/1, depends/2]). @@ -78,8 +78,6 @@ start(Host, Opts) -> c2s_copy_session, 50), ejabberd_hooks:add(user_send_packet, Host, ?MODULE, user_send_packet, 50), - ejabberd_hooks:add(user_receive_packet, Host, ?MODULE, - user_receive_packet, 50), ejabberd_hooks:add(privacy_check_packet, Host, ?MODULE, check_packet, 50), ejabberd_hooks:add(remove_user, Host, ?MODULE, @@ -94,8 +92,6 @@ stop(Host) -> c2s_copy_session, 50), ejabberd_hooks:delete(user_send_packet, Host, ?MODULE, user_send_packet, 50), - ejabberd_hooks:delete(user_receive_packet, Host, ?MODULE, - user_receive_packet, 50), ejabberd_hooks:delete(privacy_check_packet, Host, ?MODULE, check_packet, 50), ejabberd_hooks:delete(remove_user, Host, ?MODULE, @@ -407,6 +403,34 @@ c2s_copy_session(State, #{privacy_active_list := List}) -> c2s_copy_session(State, _) -> State. +% Adjust the client's state, so next packets (which can be already queued) +% will take the active list into account. +-spec update_c2s_state_with_privacy_list(stanza(), c2s_state()) -> c2s_state(). +update_c2s_state_with_privacy_list(#iq{ + type = set, + to = #jid{luser = U, lserver = S, lresource = <<"">>} = To} = IQ, + State) -> + % match a IQ set containing a new active privacy list + case xmpp:get_subtag(IQ, #privacy_query{}) of + #privacy_query{default = undefined, active = Active} -> + case Active of + none -> + ?DEBUG("Removing active privacy list for user ~p", [jid:encode(To)]), + State#{privacy_active_list => none}; + _ -> + case get_user_list(U, S, Active) of + {ok, _} -> + ?DEBUG("Setting active privacy list ~p for user ~p", [Active, jid:encode(To)]), + State#{privacy_active_list => Active}; + _ -> State % unknown privacy list name + end + end; + _ -> State + end; + +update_c2s_state_with_privacy_list(_Packet, State) -> State. + +% add the active privacy list to packet metadata -spec user_send_packet({stanza(), c2s_state()}) -> {stanza(), c2s_state()}. user_send_packet({#iq{type = Type, to = #jid{luser = U, lserver = S, lresource = <<"">>}, @@ -418,37 +442,13 @@ user_send_packet({#iq{type = Type, true -> xmpp:put_meta(IQ, privacy_active_list, Name); false -> IQ end, - {NewIQ, State}; + {NewIQ, update_c2s_state_with_privacy_list(IQ, State)}; -user_send_packet({#iq{type = Type, - from = #jid{luser = U, lserver = S}, - to = #jid{luser = U, lserver = S}, - sub_els = [_]} = IQ, - State}) - when Type == set -> - case xmpp:get_subtag(IQ, #privacy_query{}) of - #privacy_query{active = undefined} -> {IQ, State}; - #privacy_query{default = undefined, active = Active} -> - case get_user_list(U, S, Active) of - {ok, _} -> - % Adjust the client's state directly, so the next to-be-processed - % packet will take the active list into account. - {IQ, State#{privacy_active_list => Active}}; - _ -> - {IQ, State} - end; - _ -> {IQ, State} - end; +% for client with no active privacy list, see if there is +% one about to be activated in this packet and update client state +user_send_packet({Packet, State}) -> + {Packet, update_c2s_state_with_privacy_list(Packet, State)}. -user_send_packet(Acc) -> - Acc. - --spec user_receive_packet({stanza(), c2s_state()}) -> {stanza(), c2s_state()}. -user_receive_packet({#iq{type = result, - meta = #{privacy_active_list := Name}} = IQ, State}) -> - {IQ, State#{privacy_active_list => Name}}; -user_receive_packet(Acc) -> - Acc. -spec set_list(binary(), binary(), binary(), [listitem()]) -> ok | {error, any()}. set_list(LUser, LServer, Name, List) ->