Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Xeps/extensible sasl #4102

Merged
merged 7 commits into from
Sep 4, 2023
Merged

Xeps/extensible sasl #4102

merged 7 commits into from
Sep 4, 2023

Conversation

NelsonVides
Copy link
Collaborator

@NelsonVides NelsonVides commented Aug 15, 2023

This PR introduces mod_sasl2, which of all points stated in the XEP-0388, for now only provides with improvements over the number of round-trips and the removal of the redundant stream restart. It also introduces stream-management session resumption within the inline features of SASL2. So it at least already introduces a faster session establishment, that is, 6 round-trips in the usual way vs only 2 round-trips now.

Uses #4101, so it merges on top of it.

@NelsonVides NelsonVides force-pushed the xeps/extensible_sasl branch 5 times, most recently from 3961800 to 213c87f Compare August 15, 2023 19:23
@mongoose-im

This comment was marked as outdated.

@codecov
Copy link

codecov bot commented Aug 15, 2023

Codecov Report

Patch coverage: 95.63% and project coverage change: +0.09% 🎉

Comparison is base (abb89da) 83.91% compared to head (b45636c) 84.01%.
Report is 1 commits behind head on feature/sasl2.

Additional details and impacted files
@@                Coverage Diff                @@
##           feature/sasl2    #4102      +/-   ##
=================================================
+ Coverage          83.91%   84.01%   +0.09%     
=================================================
  Files                552      553       +1     
  Lines              33642    33818     +176     
=================================================
+ Hits               28232    28411     +179     
+ Misses              5410     5407       -3     
Files Changed Coverage Δ
src/c2s/mongoose_c2s_acc.erl 83.33% <50.00%> (-1.45%) ⬇️
src/stream_management/mod_stream_management.erl 91.06% <94.87%> (+1.36%) ⬆️
src/mod_sasl2.erl 96.05% <96.05%> (ø)
src/c2s/mongoose_c2s.erl 87.63% <100.00%> (+0.48%) ⬆️
src/c2s/mongoose_c2s_stanzas.erl 100.00% <100.00%> (+3.63%) ⬆️
src/hooks/mongoose_hooks.erl 95.03% <100.00%> (+0.11%) ⬆️

... and 8 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

Base automatically changed from c2s/sasl to feature/sasl2 August 24, 2023 11:40
@mongoose-im

This comment was marked as outdated.

@NelsonVides NelsonVides force-pushed the xeps/extensible_sasl branch 2 times, most recently from a43a4b1 to f79924f Compare August 25, 2023 10:03
@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

@NelsonVides NelsonVides force-pushed the xeps/extensible_sasl branch 2 times, most recently from c9e30d8 to f4eeb60 Compare August 27, 2023 10:51
@mongoose-im

This comment was marked as outdated.

Note that 'handle_resume' now returns the values that has changed in a map on
its own, instead of actually modifying the data or the accumulator, because it
will be called from two places which have slightly different semantics, so we
let the caller prepare the final return value instead.
@mongoose-im
Copy link
Collaborator

mongoose-im commented Aug 30, 2023

elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 2ec78f1
Reports root/ big
OK: 369 / Failed: 0 / User-skipped: 38 / Auto-skipped: 0


small_tests_24 / small_tests / 2ec78f1
Reports root / small


small_tests_25 / small_tests / 2ec78f1
Reports root / small


small_tests_25_arm64 / small_tests / 2ec78f1
Reports root / small


ldap_mnesia_25 / ldap_mnesia / 2ec78f1
Reports root/ big
OK: 2287 / Failed: 0 / User-skipped: 835 / Auto-skipped: 0


dynamic_domains_mysql_redis_25 / mysql_redis / 2ec78f1
Reports root/ big
OK: 4218 / Failed: 0 / User-skipped: 116 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 2ec78f1
Reports root/ big
OK: 4250 / Failed: 0 / User-skipped: 84 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / 2ec78f1
Reports root/ big
OK: 2287 / Failed: 0 / User-skipped: 835 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 2ec78f1
Reports root/ big
OK: 4250 / Failed: 0 / User-skipped: 84 / Auto-skipped: 0


internal_mnesia_25 / internal_mnesia / 2ec78f1
Reports root/ big
OK: 2437 / Failed: 0 / User-skipped: 685 / Auto-skipped: 0


mysql_redis_25 / mysql_redis / 2ec78f1
Reports root/ big
OK: 4618 / Failed: 1 / User-skipped: 111 / Auto-skipped: 0

carboncopy_SUITE:one2one:dropped_client_doesnt_create_duplicate_carbons
{error,
  {{badmatch,
     [{xmlel,<<"message">>,
        [{<<"from">>,
        <<"alice_dropped_client_doesnt_create_duplicate_carbons_564@localhost">>},
         {<<"to">>,
        <<"alice_dropped_client_doesnt_create_duplicate_carbons_564@localhost/res2">>},
         {<<"xmlns">>,<<"jabber:client">>},
         {<<"type">>,<<"chat">>}],
        [{xmlel,<<"sent">>,
           [{<<"xmlns">>,<<"urn:xmpp:carbons:2">>}],
           [{xmlel,<<"forwarded">>,
            [{<<"xmlns">>,<<"urn:xmpp:forward:0">>}],
            [{xmlel,<<"message">>,
               [{<<"from">>,
                 <<"alice_dropped_client_doesnt_create_duplicate_carbons_564@localhost/res1">>},
                {<<"to">>,
                 <<"bob_dropped_client_doesnt_create_duplicate_carbons_564@localhost/res1">>},
                {<<"type">>,<<"chat">>},
                {<<"xmlns">>,<<"jabber:client">>}],
               [{xmlel,<<"body">>,[],
                  [{xmlcdata,
                     <<"And pious action">>}]}]}]}]}]}]},
   [{carboncopy_SUITE,
      '-dropped_client_doesnt_create_duplicate_carbons/1-fun-0-',4,
      [{file,
         "/home/circleci/project/big_tests/tests/carboncopy_SUITE.erl"},
       {line,189}]},
    {escalus_story,story,4,
      [{file,
         "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
       {line,72}]},
    {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
    {test_server,run_test_case_eval1,6,
      [{file,"test_serv...

Report log


pgsql_cets_25 / pgsql_cets / 2ec78f1
Reports root/ big
OK: 4609 / Failed: 0 / User-skipped: 121 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / 2ec78f1
Reports root/ big
OK: 4639 / Failed: 0 / User-skipped: 91 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 2ec78f1
Reports root/ big
OK: 4246 / Failed: 1 / User-skipped: 87 / Auto-skipped: 0

connect_SUITE:fast_tls:feature_order:cannot_connect_with_proxy_header
{error,
  {thrown,
    {{timeout,stream_end},
     [{escalus_connection,get_stream_end,2,
        [{file,
           "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_connection.erl"},
         {line,370}]},
      {escalus_connection,end_stream,1,
        [{file,
           "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_connection.erl"},
         {line,508}]},
      {escalus_connection,stop,1,
        [{file,
           "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_connection.erl"},
         {line,408}]},
      {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
      {test_server,run_test_case_eval1,6,
        [{file,"test_server.erl"},{line,1291}]},
      {test_server,run_test_case_eval,9,
        [{file,"test_server.erl"},{line,1223}]}]}}}

Report log


pgsql_mnesia_25 / pgsql_mnesia / 2ec78f1
Reports root/ big
OK: 4639 / Failed: 0 / User-skipped: 91 / Auto-skipped: 0


mssql_mnesia_25 / odbc_mssql_mnesia / 2ec78f1
Reports root/ big
OK: 4636 / Failed: 0 / User-skipped: 94 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 2ec78f1
Reports root/ big
OK: 4247 / Failed: 0 / User-skipped: 87 / Auto-skipped: 0


mysql_redis_25 / mysql_redis / 2ec78f1
Reports root/ big
OK: 4619 / Failed: 0 / User-skipped: 111 / Auto-skipped: 0

Copy link
Member

@chrzaszcz chrzaszcz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good in general, I like the usage of hooks. I added some minor comments.

big_tests/tests/sasl2_helper.erl Outdated Show resolved Hide resolved
doc/modules/mod_sasl2.md Outdated Show resolved Hide resolved
src/c2s/mongoose_c2s_acc.erl Outdated Show resolved Hide resolved
src/mod_sasl2.erl Outdated Show resolved Hide resolved
src/mod_sasl2.erl Outdated Show resolved Hide resolved
src/mod_sasl2.erl Outdated Show resolved Hide resolved
src/mod_sasl2.erl Outdated Show resolved Hide resolved
src/mod_sasl2.erl Outdated Show resolved Hide resolved
src/mod_sasl2.erl Outdated Show resolved Hide resolved
@mongoose-im

This comment was marked as outdated.

@mongoose-im
Copy link
Collaborator

mongoose-im commented Sep 1, 2023

elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / b45636c
Reports root/ big
OK: 369 / Failed: 0 / User-skipped: 38 / Auto-skipped: 0


small_tests_24 / small_tests / b45636c
Reports root / small


small_tests_25_arm64 / small_tests / b45636c
Reports root / small


small_tests_25 / small_tests / b45636c
Reports root / small


ldap_mnesia_25 / ldap_mnesia / b45636c
Reports root/ big
OK: 2287 / Failed: 0 / User-skipped: 835 / Auto-skipped: 0


dynamic_domains_mysql_redis_25 / mysql_redis / b45636c
Reports root/ big
OK: 4218 / Failed: 0 / User-skipped: 116 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / b45636c
Reports root/ big
OK: 2287 / Failed: 0 / User-skipped: 835 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / b45636c
Reports root/ big
OK: 4250 / Failed: 0 / User-skipped: 84 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / b45636c
Reports root/ big
OK: 4247 / Failed: 0 / User-skipped: 87 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / b45636c
Reports root/ big
OK: 4250 / Failed: 0 / User-skipped: 84 / Auto-skipped: 0


pgsql_cets_25 / pgsql_cets / b45636c
Reports root/ big
OK: 4609 / Failed: 0 / User-skipped: 121 / Auto-skipped: 0


mysql_redis_25 / mysql_redis / b45636c
Reports root/ big
OK: 4618 / Failed: 1 / User-skipped: 111 / Auto-skipped: 0

carboncopy_SUITE:one2one:unavailable_resources_dont_get_carbons
{error,
  {{assertion_failed,assert_many,true,
     [is_presence,is_presence],
     [{xmlel,<<"message">>,
        [{<<"from">>,
        <<"alice_unavailable_resources_dont_get_carbons_566@localhost">>},
         {<<"to">>,
        <<"alice_unavailable_resources_dont_get_carbons_566@localhost/res1">>},
         {<<"xmlns">>,<<"jabber:client">>},
         {<<"type">>,<<"chat">>}],
        [{xmlel,<<"received">>,
           [{<<"xmlns">>,<<"urn:xmpp:carbons:2">>}],
           [{xmlel,<<"forwarded">>,
            [{<<"xmlns">>,<<"urn:xmpp:forward:0">>}],
            [{xmlel,<<"message">>,
               [{<<"from">>,
                 <<"bob_unavailable_resources_dont_get_carbons_566@localhost/res1">>},
                {<<"to">>,
                 <<"alice_unavailable_resources_dont_get_carbons_566@localhost/res2">>},
                {<<"type">>,<<"chat">>},
                {<<"xmlns">>,<<"jabber:client">>}],
               [{xmlel,<<"body">>,[],
                  [{xmlcdata,<<"And pious action">>}]}]}]}]}]},
      {xmlel,<<"presence">>,
        [{<<"from">>,
        <<"alice_unavailable_resources_dont_get_carbons_566@localhost/res1">>},
         {<<"to">>,
        <<"alice_unavailable_resources_dont_get_carbons_566@localhost/res1">>}],
        []}],
     "   <message from='alice_unavailable_resources_dont_get_carbons_566@localhost' to='alice_unavailable_resources_dont_get_carbons_566@localhost/res1' xmlns='jabber:client' type='chat'><received xmlns='urn:xmpp:carbons:2'><forwarded...

Report log


internal_mnesia_25 / internal_mnesia / b45636c
Reports root/ big
OK: 2437 / Failed: 0 / User-skipped: 685 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / b45636c
Reports root/ big
OK: 4639 / Failed: 0 / User-skipped: 91 / Auto-skipped: 0


pgsql_mnesia_25 / pgsql_mnesia / b45636c
Reports root/ big
OK: 4639 / Failed: 0 / User-skipped: 91 / Auto-skipped: 0


mssql_mnesia_25 / odbc_mssql_mnesia / b45636c
Reports root/ big
OK: 4636 / Failed: 0 / User-skipped: 94 / Auto-skipped: 0


mysql_redis_25 / mysql_redis / b45636c
Reports root/ big
OK: 4619 / Failed: 0 / User-skipped: 111 / Auto-skipped: 0

Copy link
Member

@chrzaszcz chrzaszcz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👌
Thanks for the work, and for the corrections!

@chrzaszcz chrzaszcz merged commit 9e92f64 into feature/sasl2 Sep 4, 2023
4 checks passed
@chrzaszcz chrzaszcz deleted the xeps/extensible_sasl branch September 4, 2023 13:36
@Neustradamus

This comment was marked as spam.

@chrzaszcz chrzaszcz added this to the 6.2.0 milestone Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants