processone / ejabberd

Robust, Ubiquitous and Massively Scalable Messaging Platform (XMPP, MQTT, SIP Server)
https://www.process-one.net/en/ejabberd/
Other
6k stars 1.5k forks source link

SASL2: Stable resource is used if no user-agent id is set. #4195

Closed mdosch closed 1 month ago

mdosch commented 2 months ago

Environment

@weiss might fill the gaps if necessary.

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

unknown

Errors from error.log/crash.log

unknown

Bug description

Clients SHOULD also include a element, informing the server about the connecting client. The 'id' attribute is RECOMMENDED, and if present contains a unique stable identifier for the client installation. The contents of the 'id' attribute MUST be a UUID v4. This allows the server to provide functionality such as deriving stable resource identifiers (see Bind 2.0 (XEP-0386) [5]). https://xmpp.org/extensions/xep-0388.html#initiation

Ejabberd is providing a stable resource although no id attribute is set in the user-agent element.

prefiks commented 2 months ago

I fixed this recently in https://github.com/processone/xmpp/commit/1b1418ab2f79b26c31a6cd6c91d32fb477b56850

mdosch commented 2 months ago

This fixes crash in case of client sending bind2 tag but no user-agent, as we tried to use missing user-agent to generate stable resource.

But in my scenario there is a user-tag but no ID. Still ejabberd gives me a stable resource.

prefiks commented 2 months ago

This change is not yet available in ejabberd, it was added to library that ejabberd uses, but ejabberd uses version of that library from before that change was introduced.

And this change should also be triggered case where there is missing/empty id in user-agent

weiss commented 2 months ago

@mdosch, messaging.one now has the commit in question, just in case you'd like to test it. If things look fine, this issue could be closed I guess.

mdosch commented 2 months ago

Seems to still set a stable resource in absence of user-agent id:

<authenticate xmlns='urn:xmpp:sasl:2' mechanism='SCRAM-SHA-1-PLUS'><initial-response>cD10bHMtZXhwb3J0ZXIsLG49bWRvc2NoLHI9ODk2OWQ3ZjcyOWQzOTRiZA==</initial-response><user-agent><software>go-sendxmpp</software></user-agent><bind xmlns='urn:xmpp:bind:0'><tag>go-sendxmpp</tag></bind></authenticate>
[…]
<success xmlns='urn:xmpp:sasl:2'><bound xmlns='urn:xmpp:bind:0'/><authorization-identifier>mdosch@messaging.one/go-sendxmpp.xuIUkF6Ve2</authorization-identifier><additional-data>dj1PdHVqU0dCSWo5WnUwMHJEWEh6NWFsWHljR3c9</additional-data></success>
<authenticate xmlns='urn:xmpp:sasl:2' mechanism='SCRAM-SHA-1-PLUS'><initial-response>cD10bHMtZXhwb3J0ZXIsLG49bWRvc2NoLHI9OGJiYzYxODE1NmM1ZmRlOA==</initial-response><user-agent><software>go-sendxmpp</software></user-agent><bind xmlns='urn:xmpp:bind:0'><tag>go-sendxmpp</tag></bind></authenticate>
[…]
<success xmlns='urn:xmpp:sasl:2'><bound xmlns='urn:xmpp:bind:0'/><authorization-identifier>mdosch@messaging.one/go-sendxmpp.xuIUkF6Ve2</authorization-identifier><additional-data>dj04NkV2OXlUYk1VckdDdjRmR1RXZGd6UWd4YlU9</additional-data></success>
prefiks commented 2 months ago

Hm, i will see if i can reproduce that then, but from looking at what code does i though we should be skip code that does that without id.

prefiks commented 2 months ago

Looks like i had a typo that didn't properly detect empty id - fixed in https://github.com/processone/xmpp/commit/12a2e3feadc0dfdf6a56a2b17925c1275adc2ef9

mdosch commented 2 months ago

Thanks, I can confirm that it works now as expected.