From af6760a44668835a5ac69d6af808dad3d5d44d23 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Tue, 21 Feb 2023 11:23:53 +0100 Subject: [PATCH 1/2] Fix announcing session establishment feature An XMPP client won't try the old session establishment step if it doesn't see it announced in the server features, even when the server is strictly waiting for it, so this would deadlock all connections. --- src/c2s/mongoose_c2s.erl | 2 +- src/c2s/mongoose_c2s_stanzas.erl | 14 +++++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/c2s/mongoose_c2s.erl b/src/c2s/mongoose_c2s.erl index 37adcd04e6f..92abc125774 100644 --- a/src/c2s/mongoose_c2s.erl +++ b/src/c2s/mongoose_c2s.erl @@ -451,7 +451,7 @@ stream_start_features_before_auth(#c2s_data{host_type = HostType, lserver = LSer stream_start_features_after_auth(#c2s_data{host_type = HostType, lserver = LServer, lang = Lang, listener_opts = LOpts} = S) -> send_header(S, LServer, <<"1.0">>, Lang), - StreamFeatures = mongoose_c2s_stanzas:stream_features_after_auth(HostType, LServer), + StreamFeatures = mongoose_c2s_stanzas:stream_features_after_auth(HostType, LServer, LOpts), send_element_from_server_jid(S, StreamFeatures), {next_state, {wait_for_feature_after_auth, ?BIND_RETRIES}, S, state_timeout(LOpts)}. diff --git a/src/c2s/mongoose_c2s_stanzas.erl b/src/c2s/mongoose_c2s_stanzas.erl index 199757a968c..d2908f40cf3 100644 --- a/src/c2s/mongoose_c2s_stanzas.erl +++ b/src/c2s/mongoose_c2s_stanzas.erl @@ -7,7 +7,7 @@ stream_header/4, stream_features_before_auth/4, tls_proceed/0, - stream_features_after_auth/2, + stream_features_after_auth/3, sasl_success_stanza/1, sasl_failure_stanza/1, sasl_challenge_stanza/1, @@ -87,11 +87,19 @@ tls_proceed() -> #xmlel{name = <<"proceed">>, attrs = [{<<"xmlns">>, ?NS_TLS}]}. --spec stream_features_after_auth(mongooseim:host_type(), jid:lserver()) -> exml:element(). -stream_features_after_auth(HostType, LServer) -> +-spec stream_features_after_auth(mongooseim:host_type(), jid:lserver(), mongoose_listener:options()) -> + exml:element(). +stream_features_after_auth(HostType, LServer, #{backwards_compatible_session := false}) -> Features = [#xmlel{name = <<"bind">>, attrs = [{<<"xmlns">>, ?NS_BIND}]} | hook_enabled_features(HostType, LServer)], + stream_features(Features); +stream_features_after_auth(HostType, LServer, #{backwards_compatible_session := true}) -> + Features = [#xmlel{name = <<"session">>, + attrs = [{<<"xmlns">>, ?NS_SESSION}]}, + #xmlel{name = <<"bind">>, + attrs = [{<<"xmlns">>, ?NS_BIND}]} + | hook_enabled_features(HostType, LServer)], stream_features(Features). hook_enabled_features(HostType, LServer) -> From 62a6af839b83adde502cb83471953c777a07475b Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Tue, 21 Feb 2023 16:36:12 +0100 Subject: [PATCH 2/2] Test session feature announcement --- big_tests/tests/mim_c2s_SUITE.erl | 63 +++++++++++++++++++++++-------- 1 file changed, 48 insertions(+), 15 deletions(-) diff --git a/big_tests/tests/mim_c2s_SUITE.erl b/big_tests/tests/mim_c2s_SUITE.erl index a178b88e6b9..a5c938314b0 100644 --- a/big_tests/tests/mim_c2s_SUITE.erl +++ b/big_tests/tests/mim_c2s_SUITE.erl @@ -4,6 +4,9 @@ -include_lib("common_test/include/ct.hrl"). -include_lib("eunit/include/eunit.hrl"). +-include_lib("exml/include/exml.hrl"). +-include_lib("escalus/include/escalus.hrl"). +-include_lib("escalus/include/escalus_xmlns.hrl"). -import(distributed_helper, [mim/0]). @@ -13,7 +16,8 @@ all() -> [ - {group, basic} + {group, basic}, + {group, backwards_compatible_session} ]. groups() -> @@ -21,7 +25,12 @@ groups() -> {basic, [parallel], [ two_users_can_log_and_chat, - too_big_stanza_rejected + too_big_stanza_rejected, + verify_session_establishment_is_not_announced + ]}, + {backwards_compatible_session, [parallel], + [ + verify_session_establishment_is_announced ]} ]. @@ -30,34 +39,39 @@ groups() -> %%-------------------------------------------------------------------- init_per_suite(Config) -> HostType = domain_helper:host_type(), - Steps = [start_stream, stream_features, maybe_use_ssl, authenticate, bind], + Config1 = dynamic_modules:save_modules(HostType, Config), + dynamic_modules:ensure_stopped(HostType, [mod_presence]), EscalusOverrides = [{initial_activity, fun(_) -> ok end}, {start_ready_clients, fun ?MODULE:escalus_start/2}], - Config1 = save_c2s_listener(Config), - Config2 = dynamic_modules:save_modules(HostType, Config1), - dynamic_modules:ensure_stopped(HostType, [mod_presence]), - Config3 = escalus_users:update_userspec(Config2, alice, connection_steps, Steps), - Config4 = escalus_users:update_userspec(Config3, bob, connection_steps, Steps), - configure_c2s_listener(Config4, #{backwards_compatible_session => false, max_stanza_size => 1024}), - escalus:init_per_suite([{escalus_overrides, EscalusOverrides} | Config4 ]). + escalus:init_per_suite([{escalus_overrides, EscalusOverrides} | Config1 ]). end_per_suite(Config) -> - restore_c2s_listener(Config), - escalus_fresh:clean(), dynamic_modules:restore_modules(Config), + mongoose_helper:restore_config(Config), escalus:end_per_suite(Config). -init_per_group(_, Config) -> +init_per_group(basic, Config) -> + Steps = [start_stream, stream_features, maybe_use_ssl, authenticate, bind], + Config1 = save_c2s_listener(Config), + Config2 = escalus_users:update_userspec(Config1, alice, connection_steps, Steps), + Config3 = escalus_users:update_userspec(Config2, bob, connection_steps, Steps), + configure_c2s_listener(Config3, #{backwards_compatible_session => false, max_stanza_size => 1024}), + Config3; +init_per_group(backwards_compatible_session, Config) -> Config. -end_per_group(_GroupName, Config) -> +end_per_group(basic, Config) -> + escalus_fresh:clean(), + restore_c2s_listener(Config), + Config; +end_per_group(backwards_compatible_session, Config) -> + escalus_fresh:clean(), Config. init_per_testcase(Name, Config) -> escalus:init_per_testcase(Name, Config). end_per_testcase(Name, Config) -> - mongoose_helper:restore_config(Config), escalus:end_per_testcase(Name, Config). %%-------------------------------------------------------------------- @@ -80,6 +94,25 @@ too_big_stanza_rejected(Config) -> escalus:assert(is_stream_end, escalus_client:wait_for_stanza(Alice)), true = escalus_connection:wait_for_close(Alice, timer:seconds(1)). +verify_session_establishment_is_not_announced(Config) -> + MaybeSessionFeature = start_connection_maybe_get_session_feature(Config), + ?assertEqual(undefined, MaybeSessionFeature). + +verify_session_establishment_is_announced(Config) -> + MaybeSessionFeature = start_connection_maybe_get_session_feature(Config), + ?assertNotEqual(undefined, MaybeSessionFeature). + +start_connection_maybe_get_session_feature(Config) -> + Steps = [start_stream, stream_features], + AliceSpec = escalus_fresh:create_fresh_user(Config, alice), + {ok, Client = #client{props = Props}, _} = escalus_connection:start(AliceSpec, Steps), + ok = escalus_auth:auth_plain(Client, Props), + escalus_connection:reset_parser(Client), + Client1 = escalus_session:start_stream(Client), + Features = escalus_connection:get_stanza(Client1, wait_for_features), + escalus_connection:stop(Client1), + exml_query:path(Features, [{element_with_ns, <<"session">>, ?NS_SESSION}]). + %%-------------------------------------------------------------------- %% helpers %%--------------------------------------------------------------------