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

mod_auth_fast SASL downgrade protection trigger #4331

Open
mightyBroccoli opened this issue Dec 20, 2024 · 13 comments
Open

mod_auth_fast SASL downgrade protection trigger #4331

mightyBroccoli opened this issue Dec 20, 2024 · 13 comments

Comments

@mightyBroccoli
Copy link
Contributor

Environment

  • ejabberd version: 24.12
  • Erlang version: Erlang (SMP,ASYNC_THREADS) (BEAM) emulator version 13.1.5
  • OS: Debian 12.8
  • Installed from: source

Configuration (only if needed): grep -Ev '^$|^\s*#' ejabberd.yml

modules:
   mod_auth_fast:
     db_type: mnesia

Errors from error.log/crash.log

No errors

Bug description

Monal (Version 6.4.7 build 1003) is not able to connect with the module active because it detects a Man in the Middle through SSDP

Presumably the problem is that the FAST mechanism isn't part of the SSDP hash.

@weiss @tmolitor-stud-tu

@mightyBroccoli mightyBroccoli changed the title mod_auth_fast SASL downgrade protection interference mod_auth_fast SASL downgrade protection trigger Dec 20, 2024
@mdosch
Copy link

mdosch commented Dec 21, 2024

Same was happening to a go-sendxmpp user who is running ejabberd 24.12.
As it could be fixed by disabling FAST in ejabberd or disabling SSDP in go-sendxmpp it looks like your suspicion is right.

@ravermeister
Copy link

The same applies for movim too

@prefiks
Copy link
Member

prefiks commented Dec 21, 2024

Diagnosis is partially correct, and it's from actually taking fast mechanisms into account when calculating ssdp hash. But after carefully reading ssdp spec, i think that was wrong, those aren't reported inside regular sasl mechanisms list, but in external element. I will remove those.

@tmolitor-stud-tu
Copy link

When the user already requested a FAST token, FAST is listed in the mechanism list: https://xmpp.org/extensions/xep-0484.html#fast-auth

So BEFORE the client requested a token, the advertising of FAST support should NOT be part of the SSDP hash, but AFTER the client requested a FAST token, the FAST mechanism is part of the SASL2 mechanism list and MUST be part of the SSDP hash, too.

@prefiks
Copy link
Member

prefiks commented Dec 21, 2024

When using fast, ssdp is not send (as in that case we don't really use sasl, so there is no field that can be used to pass it). But reading spec for ssdp i am not sure if this should be included anyway, it says about "Sort all server-advertised SASL mechanisms ", and fast mechanisms aren't advertised like that, they are listed inside element and not inside top level mechanisms like other sasl method are.

@prefiks
Copy link
Member

prefiks commented Dec 21, 2024

I commited this change as 7d5413c

@prefiks
Copy link
Member

prefiks commented Dec 21, 2024

And xmpp change which does real work here is in processone/xmpp@9a56de6

@tmolitor-stud-tu
Copy link

as in that case we don't really use sasl

that's wrong, FAST uses SASL2, too. You presumably mean: we don't really use SCRAM

@tmolitor-stud-tu
Copy link

tmolitor-stud-tu commented Dec 21, 2024

and fast mechanisms aren't advertised like that

that's wrong, too. FAST mechanisms are advertised as SASL2 mechanism just like any other SASL mechanism, too. They are advertised in a special element only (and not in the SASL2 mechanism list), if there was no FAST token requested by the client yet.

@tmolitor-stud-tu
Copy link

okay, I stand corrected: you were right: even after handing out a token, the complete FAST stuff is in its own inline element.

SSDP talks about all mechanisms, but it doesn't make clear if only SASL2 is meant or if FAST should be included, too.

I'll update SSDP to include FAST, too (otherwise a MITM attacker could try to downgrade from FAST to some other SASL2 mechanism like SCRAM).

mdosch added a commit to xmppo/go-xmpp that referenced this issue Dec 21, 2024
This will probably added to the SSDP XEP, see processone/ejabberd#4331 (comment)
@prefiks
Copy link
Member

prefiks commented Dec 21, 2024

If you extend this to include those hash, it would be good to have a way to make clients using older version without knowing about fast mechanisms, to still work, otherwise there will be issue like this again.

@tmolitor-stud-tu
Copy link

tmolitor-stud-tu commented Dec 21, 2024

@prefiks after thinking longer about this, adding the FAST mechanisms to the SSDP hash, introduces an ugly dependency on FAST even if the client does not implement it (but the server). To me that sounds very hackish and dirty and I don't like protocols that aren't designed cleanly.

On the other hand, there is the hypothetical danger of a MITM downgrading from a channel-binding FAST token to a non-channel-binding SASL mechanism, by removing the advertised FAST mechanisms or replacing them with dummy ones unknown to the client.

But that requires the server to support channel-binding for FAST but not for SCRAM etc. Otherwise the attacker would have to remove all scram -PLUS mechanisms, too, and that would still be caught by SSDP. I don't think any server implementation allows that (even though it would be theoretically possible).

FAST tokens currently in use don't have a way to add additional data authenticated by the token (like an SSDP hash). So, using SSDP to catch downgrade attempts between different FAST mechanisms isn't possible. And normally a client doesn't posses tokens for different FAST mechanisms, but only for one, and falls back to SCRAM etc. if that token isn't usable. That means SSDP isn't (yet?) needed for FAST, nor implementable using the current draft-schmaus-kitten-sasl-ht I-D.

IMHO the arguments against including the FAST mechanisms into the SSDP hash outweigh the arguments in favor.

@prefiks
Copy link
Member

prefiks commented Dec 22, 2024

I also was thinking that those don't need to be in ssdp hash, fast has only auxilary role to regular sasl methods (they can be only generated when other sasl method was used already, and clients are able to function without them). Plus if someone could mitm connection they could just "disable" fast without needing to disable fast features - all needed here would be responding to fast auth with error about unknown token and not passing that request to server, and that would be something valid for both parties.

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

No branches or pull requests

5 participants