DRAFT: fix: When NoTLS is set, don't do StartTLS by selfhoster1312 · Pull Request #201 · xmppo/go-xmpp

selfhoster1312 · 2025-10-24T11:24:41Z

When using prosody in CI i really want to use plaintext connection. Maybe i should just use random certs and bypass verification but that seemed wrong.

This is the PR that got CI running for matterbridge in matterbridge-org/matterbridge#4

However, i'm not sure if it still applies correctly with the recent updates. I'll come back here once i'm sure.

Also, maybe NoTLS is not the correct option for plaintext auth? It's not clear to me what the mental model of all connection options is in this library. We should probably add docs about the specific order in which they are applied and the option combinations required to reach a server with certain settings.

mdosch · 2025-10-24T16:28:43Z

Thank you very much for suggesting a change, but I think this already possible. NoTLS is for configuring direct TLS and StartTLS is for configuring StartTLS. So I guess you can achieve your goal by setting NoTLS to true and StartTLS to false. Maybe you also need to set InsecureAllowUnencryptedAuth to true also.

See the docs for the options for more details:

// InsecureAllowUnencryptedAuth permits authentication over a TCP connection that has not been promoted to
// TLS by STARTTLS; this could leak authentication information over the network, or permit man in the middle
// attacks.
InsecureAllowUnencryptedAuth bool

// NoTLS directs go-xmpp to not use TLS initially to contact the server; instead, a plain old unencrypted
// TCP connection should be used. (Can be combined with StartTLS to support STARTTLS-based servers.)
NoTLS bool

// StartTLS directs go-xmpp to STARTTLS if the server supports it; go-xmpp will automatically STARTTLS
// if the server requires it regardless of this option.
StartTLS bool

Best regards
Martin

selfhoster1312 · 2025-10-24T16:32:38Z

Hello, thanks for replying so quickly.

Here's what i did: https://github.com/matterbridge-org/matterbridge/blob/2bb66da51a25c7f1da606adfb3543b8c1cf96897/tests/xmpp/xmpp.go#L112

It was definitely not working without this patch, but it was based on really old go-xmpp (as you are aware). I'll try the same settings on the new version and let you know.

selfhoster1312 · 2025-10-24T16:44:51Z

I can confirm it does not work without the patch: https://github.com/matterbridge-org/matterbridge/actions/runs/18786159951/job/53604836999?pr=8

I get on matterbridge side:

time="2025-10-24T16:42:06Z" level=debug msg="&errors.errorString{s:"no viable authentication method available: [PLAIN]"}" func=Connect file="matterbridge/bridge/xmpp/xmpp.go:47" prefix=xmpp

Here's the prosody debug logs:

connnopzx4024Ay4                                     debug	New connection FD 15 (::1, 34746, ::1, 52222) on server FD 9 (::, 52222)
connnopzx4024Ay4                                     debug	Connected (FD 15 (::1, 34746, ::1, 52222))
c2s559db5426a90                                      info	Client connected
runnerFWaC9ddmtzV5                                   debug	creating new coroutine
c2s559db5426a90                                      debug	Client sent opening <stream:stream> to matterbridge-test.localhost
c2s559db5426a90                                      debug	Sending[c2s_unauthed]: <?xml version='1.0'?>
c2s559db5426a90                                      debug	Sent reply <stream:stream> to client
c2s559db5426a90                                      debug	SASL mechanisms supported by handler: {PLAIN}
c2s559db5426a90                                      debug	Offering usable mechanisms: {PLAIN}
c2s559db5426a90                                      debug	Sending[c2s_unauthed]: <stream:features>
connnopzx4024Ay4                                     debug	Sent 635 out of 635 buffered bytes
connnopzx4024Ay4                                     debug	Connection closed by remote
c2s559db5426a90                                      info	Client disconnected: closed
c2s559db5426a90                                      debug	Destroying unbound session for <(unknown)@matterbridge-test.localhost>: closed

I'm still not sure this PR is how to properly fix it but i'm sure there's a problem :) :)

mdosch · 2025-10-24T16:55:54Z

Looks this is about the auth mechanism, not about encryption. But that's weird as NoPLAIN should default to no…

// NoPLAIN forbids authentication using plain passwords
NoPLAIN bool

I'll need to have a closer look, but I am not sure whether I get something done today.

selfhoster1312 · 2025-10-24T17:07:14Z

My understanding is that tlsConnOK evaluates to false, which makes it impossible to select the PLAIN mechanism on go-xmpp side.

Don't worry if you don't have time. There are more important things in life than TLS issues :D

And anyway this is on a test branch at the moment because i'm starting to use it to test matterbridge with a real prosody server. This has no incidence on matterbridge's main branch which i should be able to update.

mdosch · 2025-10-24T17:08:40Z

xmpp.go

 				mechanism = "X-OAUTH2"
 				// Do not use PLAIN auth if NoPlain is set.
-			case slices.Contains(mechSlice, "PLAIN") && tlsConnOK && !o.NoPLAIN:
+			case slices.Contains(mechSlice, "PLAIN") && (tlsConnOK || o.NoTLS) && !o.NoPLAIN:


I think from a security point of view we should make it very hard to use PLAIN auth over an unencrypted connection. So maybe better not allow PLAIN here, but set set Mechanism in Options to PLAIN, then the else clause won't be used.

mdosch · 2025-10-24T17:09:45Z

xmpp.go

-	if f, err = c.startTLSIfRequired(f, o, domain); err != nil {
-		return err
+	// unless the client has specified NoTLS
+	if !o.NoTLS {


I am not sure if we should use the directTLS setting to control whether StartTLS is used. I'll check if there is an alternative.

mdosch · 2025-10-24T17:26:53Z

I think the startTLSIfRequired function is buggy, can you try this branch with setting Mechanism := "PLAIN"?

selfhoster1312 · 2025-10-24T17:32:28Z

Not responding in the review because there's overlapping points.

I think the mental model is something like:

establish connection (for consistent logic, should probably be a switch/case and not linear ifs, or at least the ifs should be closer and documented)
- by default, with startTLS
- if directTLS is requested, with directTLS not startTLS (less common, and incompatible with startTLS)
- if DisableEncryption (or something like this) is used, with neither
authenticate with a specific mechanism
- we should support SASL2 https://xmpp.org/extensions/xep-0388.html
- applying this setting should probably be 100% independent of TLS settings

Are we on the same page?

selfhoster1312 · 2025-10-24T17:33:21Z

I think the startTLSIfRequired function is buggy, can you try this branch with setting Mechanism := "PLAIN"?

That looks good! I'll give it a go (though maybe not tonight)

selfhoster1312 · 2025-10-24T17:44:56Z

Nope, still fails: https://github.com/matterbridge-org/matterbridge/pull/8/checks?check_run_id=53609622388

mdosch · 2025-10-24T17:49:14Z

I think the mental model is something like:

* establish connection (for consistent logic, should probably be a switch/case and not linear ifs, or at least the ifs should be closer and documented)

There is a lot that can be improved, but I am only a little bit working on this library as I am a user of it. So don't expect me to do big reworks. :)

  * by default, with startTLS
  * if directTLS is requested, with directTLS not startTLS (less common, and incompatible with startTLS)

If you check SRV records you might want to be explicit about whether you want to use direct TLS or StartTLS. Also I am reluctant to change the default behavior of options that are in use for a long time.

  * if DisableEncryption (or something like this) is used, with neither

There is no DisableEncryption, not sure if need as NoTLS, StartTLS and InsecureAllowUnencryptedAuth should be enough options for that. :)

* authenticate with a specific mechanism
  
  * we should support SASL2 https://xmpp.org/extensions/xep-0388.html
  * applying this setting should probably be 100% independent of TLS settings

I find it reasonable to disable PLAIN on unencrypted connections and -PLUS variants require encryption so there will be always a relation between mechanisms and encryption.

Are we on the same page?

Mostly I guess.

mdosch · 2025-10-24T17:51:26Z

Nope, still fails: https://github.com/matterbridge-org/matterbridge/pull/8/checks?check_run_id=53609622388

Can you show me the error? Somehow this doesn't fully render for me and the output ends at

go: downloading github.com/zfjagann/golang-ring v0.0.0-20220330170733-19bcea1b6289
go: downloading github.com/bwmarrin/discordgo v0.28.1

selfhoster1312 · 2025-10-24T17:54:06Z

Can you show me the error? Somehow this doesn't fully render for me

Same errors. It's not visible in the test.sh job, but in the setup.log job (prosody log) and matterbridge.log job.

selfhoster1312 · 2025-10-24T17:56:21Z

If you check SRV records you might want to be explicit about whether you want to use direct TLS or StartTLS

Only directTLS should be explicit, because few servers support it. startTLS should be the implied default.

InsecureAllowUnencryptedAuth

That's a good enough name!

I find it reasonable to disable PLAIN on unencrypted connections and -PLUS variants require encryption so there will be always a relation between mechanisms and encryption.

I was using PLAIN on unencrypted connection (localhost). I'm aware some variants require specific settings but i believe this depends on the server implementation so the best thing to do is have good defaults (don't manually set the mechanisms) and let the user configure it as they like for very specific cases such as mine, and deal with server errors if any.

I was suggesting to remove the link between those two kinds of settings because it requires more complex conditions handling. I think having proper errors emanating from the underlying libraries or the remote server would be more user-friendly than simply failing because the condition chain is so complex we have a logic bug. What do you think?

mdosch · 2025-10-24T18:02:33Z

If you check SRV records you might want to be explicit about whether you want to use direct TLS or StartTLS

Only directTLS should be explicit, because few servers support it. startTLS should be the implied default.

It is, if you set neither StartTLS will be done if supported by the server.

I find it reasonable to disable PLAIN on unencrypted connections and -PLUS variants require encryption so there will be always a relation between mechanisms and encryption.

I was using PLAIN on unencrypted connection (localhost). I'm aware some variants require specific settings but i believe this depends on the server implementation so the best thing to do is have good defaults (don't manually set the mechanisms) and let the user configure it as they like for very specific cases such as mine, and deal with server errors if any.

Maybe we disagree about good defaults. Not using PLAIN over an unencrypted connection is a reasonable default for me. For your use case I find it ok, to have to set some extra settings. It's only that there currently is a bug somewhere.

I was suggesting to remove the link between those two kinds of settings because it requires more complex conditions handling. I think having proper errors emanating from the underlying libraries or the remote server would be more user-friendly than simply failing because the condition chain is so complex we have a logic bug. What do you think?

Not sure what link you mean, can you elaborate?

mdosch · 2025-10-24T18:07:51Z

Can you show me the error? Somehow this doesn't fully render for me

Same errors. It's not visible in the test.sh job, but in the setup.log job (prosody log) and matterbridge.log job.

no viable authentication method available: [PLAIN]

That's weird, that means mechanism is not set. Can you doublecheck you have set Mechanism to PLAIN?

mdosch · 2025-10-24T19:24:23Z

I set up a local prosody and I was able to connect using the updated branch with the latest commits and these settings:

NoTLS = true,
StartTLS = false,
InsecureAllowUnencryptedAuth = true
Mechanism = "PLAIN"

Please check if it is also fixed for you.

selfhoster1312 · 2025-10-25T08:48:52Z

Not using PLAIN over an unencrypted connection is a reasonable default for me.

I agree :)

Not sure what link you mean, can you elaborate?

I meant my proposed change (case slices.Contains(mechSlice, "PLAIN") && (tlsConnOK || o.NoTLS) && !o.NoPLAIN:) touches on code that mixes SASL mechanisms and TLS state/config. I was wondering if these should be separate concerns entirely. You're not wrong when you say they're linked anyway, i'm pondering if we can reduce the amount of logic on go-xmpp side and use error messages from the server to spot misconfigurations. That's definitely not the core of the issue, though.

Maybe my brain is not working at the moment. I've rebased my branch on your latest commits, but still nothing… https://github.com/matterbridge-org/matterbridge/actions/runs/18800711155/job/53647792486?pr=8

mdosch · 2025-10-25T11:05:22Z

I wanted to check your prosody config on my machine but seems there is something missing:

prosodyctl adduser test@matterbridge-test.localhost test
** Unable to connect to server - is it running? Is mod_admin_shell enabled?
** Connection error: No such file or directory

How do you create your test user?

mdosch · 2025-10-25T11:30:33Z

Ok, seems prosody doesn't let me log in. I fixed it by changing authentication to internal_hashed and adding the admin_shell module. Then I could add a testuser by prosodyctl adduser test@matterbridge-test.localhost and the login process worked.
No idea why prosody didn't give an helpful error though.

2025-10-25 13:12:44 INFO connecting to host=matterbridge-test.localhost:52222
SEND <?xml version='1.0'?><stream:stream to='matterbridge-test.localhost' xmlns='jabber:client' xmlns:stream='http://etherx.jabber.org/streams' version='1.0'>
RECV <?xml version='1.0'?><stream:stream xmlns:stream='http://etherx.jabber.org/streams' from='matterbridge-test.localhost' xmlns='jabber:client' xml:lang='en' id='df9688cf-bdea-4214-aaba-632bfb25ba6e' version='1.0'><stream:features><register xmlns='urn:xmpp:invite'/><register xmlns='urn:xmpp:ibr-token:0'/><starttls xmlns='urn:ietf:params:xml:ns:xmpp-tls'/><register xmlns='http://jabber.org/features/iq-register'/><limits xmlns='urn:xmpp:stream-limits:0'><max-bytes>262144</max-bytes><idle-seconds>840</idle-seconds></limits></stream:features>
2025-10-25 13:12:44 INFO giving up trying to connect…
2025-10-25 13:12:44 connect: failed to connect to server: no viable authentication method available: []

It just didn't offer any authentication mechanisms, after creating a user it does:

2025-10-25 13:24:45 INFO connecting to host=matterbridge-test.localhost:52222
SEND <?xml version='1.0'?><stream:stream to='matterbridge-test.localhost' xmlns='jabber:client' xmlns:stream='http://etherx.jabber.org/streams' version='1.0'>
RECV <?xml version='1.0'?><stream:stream xml:lang='en' xmlns:stream='http://etherx.jabber.org/streams' id='05460ae4-c966-43f9-b389-027ddaaf0925' xmlns='jabber:client' version='1.0' from='matterbridge-test.localhost'><stream:features><starttls xmlns='urn:ietf:params:xml:ns:xmpp-tls'/><mechanisms xmlns='urn:ietf:params:xml:ns:xmpp-sasl'><mechanism>OAUTHBEARER</mechanism><mechanism>SCRAM-SHA-1</mechanism><mechanism>PLAIN</mechanism></mechanisms><register xmlns='http://jabber.org/features/iq-register'/><register xmlns='urn:xmpp:invite'/><register xmlns='urn:xmpp:ibr-token:0'/><limits xmlns='urn:xmpp:stream-limits:0'><max-bytes>262144</max-bytes><idle-seconds>840</idle-seconds></limits></stream:features>
SEND <auth xmlns='urn:ietf:params:xml:ns:xmpp-sasl' mechanism='SCRAM-SHA-1'>biwsbj10ZXN0LHI9MTQ5Y2I2NWEzNTRiMGZmNQ==</auth>
RECV <challenge xmlns='urn:ietf:params:xml:ns:xmpp-sasl'>cj0xNDljYjY1YTM1NGIwZmY1NjdmNGZiN2UtMTFhMS00NDY4LThiOWYtMDBkMDc2NTBjMWUxLHM9WkdFMk1HRXlZbUl0TUdJM05TMDBNVGhtTFdFNFptVXROR1V5TkdVM056WTRZVGt3LGk9MTAwMDA=</challenge>
SEND <response xmlns='urn:ietf:params:xml:ns:xmpp-sasl'>Yz1iaXdzLHI9MTQ5Y2I2NWEzNTRiMGZmNTY3ZjRmYjdlLTExYTEtNDQ2OC04YjlmLTAwZDA3NjUwYzFlMSxwPThhUm85andzWGR1Nlk2dU5wWGhFSGdsRHRVYz0=</response>
RECV <success xmlns='urn:ietf:params:xml:ns:xmpp-sasl'>dj1IWHlMRTdwUEFERVpBUGVQU3kwU3o5dkVaWXM9</success>

mdosch · 2025-10-25T12:15:33Z

Maybe you wanted to use this module but failed to install it?
After installing it, loading it and setting authentication back to any login also works with non-existing users and the settings I recommended somewhere above.
So for me it looks like the fix in startTLSIfRequired works as intended and I will merge it to master, but please report whether it also works for you now. :)

It was not possible to use plain auth with an unencrypted connection (e.g. for test setups), this is now possible. See #201 for context.

selfhoster1312 · 2025-10-25T12:29:08Z

How do you create your test user?

I don't. Any login is considered valid as that makes orchestration easier:

authentication = "any"

The original reason i wanted to try PLAIN auth was to make everything simpler which it's definitely not doing. Also it appears auth_any only supports PLAIN mechanism because the server doesn't know the password beforehand (according to prosody devs).

mdosch · 2025-10-25T12:50:30Z

The original reason i wanted to try PLAIN auth was to make everything simpler which it's definitely not doing. Also it appears auth_any only supports PLAIN mechanism because the server doesn't know the password beforehand (according to prosody devs).

That you have to pull some strings to use PLAIN over an unencrypted connection is intentional as it is really dangerous in general.
That it still did not work after setting everything up was a bug in the StartTLS logic that, according to my local tests, should be fixed now.

selfhoster1312 · 2025-10-25T14:59:29Z

That it still did not work after setting everything up was a bug in the StartTLS logic that, according to my local tests, should be fixed now.

After taking a break away from the computer i believe you are right. I'll try something :)

selfhoster1312 · 2025-10-25T15:24:24Z

Can you maybe share your go-sendxmpp command? I still can't get it to work:

echo test | ./go-sendxmpp --allow-plain --no-sasl-upgrade --no-tls-verify --debug  testroom@m
uc.matterbridge-test.localhost
SEND <?xml version='1.0'?><stream:stream to='matterbridge-test.localhost' xmlns='jabber:client' xmlns:stream='http://etherx.jabber.org/streams' version='1.0'>
RECV <?xml version='1.0'?><stream:stream xmlns:stream='http://etherx.jabber.org/streams' version='1.0' from='matterbridge-test.localhost' xmlns='jabber:client' xml:lang='en' id='5c912372-8496-4092-8e05-5ae0153c2829'><stream:features><mechanisms xmlns='urn:ietf:params:xml:ns:xmpp-sasl'><mechanism>PLAIN</mechanism></mechanisms><starttls xmlns='urn:ietf:params:xml:ns:xmpp-tls'/><register xmlns='urn:xmpp:invite'/><register xmlns='urn:xmpp:ibr-token:0'/><limits xmlns='urn:xmpp:stream-limits:0'><max-bytes>262144</max-bytes><idle-seconds>840</idle-seconds></limits></stream:features>
SEND <starttls xmlns='urn:ietf:params:xml:ns:xmpp-tls'/>
RECV <proceed xmlns='urn:ietf:params:xml:ns:xmpp-tls'/>
2025-10-25 17:18:41 connect: failed to connect to server: starttls handshake: remote error: tls: protocol version not supported

mdosch · 2025-10-25T15:30:42Z

You'll need the insecure version: `go install ***@***.***` And run the command like that: `fortune|~/go/bin/go-sendxmpp -dj matterbridge-test.localhost:52222 -u ***@***.*** -p test333 --allow-plain ***@***.***`

mdosch · 2025-10-25T15:32:03Z

Ah, the TLS version is too low. Go-sendxmpp uses TLSv1.3 as default, but you can downgrade it: `--tls-version 12` will use TLSv1.2

selfhoster1312 · 2025-10-25T15:45:56Z

Yes sorry i was on the wrong branch. I now see i need to specify Mechanism PLAIN even with InsecureAllowUnencryptedAuth. I believe you already mentioned that but could we maybe automatically add it when InsecureAllowUnencryptedAuth is set?

When i set it manually on the new branch i can indeed connect. Sorry for wasting your neurons :)

mdosch · 2025-10-25T15:53:30Z

On Sat Oct 25, 2025 at 17:46 CEST, selfhoster1312 wrote: Yes sorry i was on the wrong branch. I now see i need to specify Mechanism PLAIN even with `InsecureAllowUnencryptedAuth`. I believe you already mentioned that but could we maybe automatically add it when InsecureAllowUnencryptedAuth is set?

I need to think about that. It would then fail if a server doesn't offer PLAIN but only better methods if we pin the mechanism to PLAIN. Maybe there is a way to differently fix that, let me check.

When i set it manually on the new branch i can indeed connect. Sorry for wasting your neurons :)

No problem, sometimes debugging is like this.

mdosch · 2025-10-25T16:02:22Z

Ok, I pushed a new commit to go-xmpp and the insecure go-sendxmpp branch, can you check?

selfhoster1312 · 2025-10-25T16:37:37Z

That's perfect, thank you so much!

fix: When NoTLS is set, don't do StartTLS

80d061d

mdosch reviewed Oct 24, 2025

View reviewed changes

mdosch added a commit that referenced this pull request Oct 25, 2025

Allow unencrypted plain auth.

c1a74f2

It was not possible to use plain auth with an unencrypted connection (e.g. for test setups), this is now possible. See #201 for context.

selfhoster1312 closed this Oct 25, 2025

Conversation

selfhoster1312 commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mdosch commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

selfhoster1312 commented Oct 24, 2025

Uh oh!

selfhoster1312 commented Oct 24, 2025

Uh oh!

mdosch commented Oct 24, 2025

Uh oh!

selfhoster1312 commented Oct 24, 2025

Uh oh!

mdosch Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

mdosch Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

mdosch commented Oct 24, 2025

Uh oh!

selfhoster1312 commented Oct 24, 2025

Uh oh!

selfhoster1312 commented Oct 24, 2025

Uh oh!

selfhoster1312 commented Oct 24, 2025

Uh oh!

mdosch commented Oct 24, 2025

Uh oh!

mdosch commented Oct 24, 2025

Uh oh!

selfhoster1312 commented Oct 24, 2025

Uh oh!

selfhoster1312 commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mdosch commented Oct 24, 2025

Uh oh!

mdosch commented Oct 24, 2025

Uh oh!

mdosch commented Oct 24, 2025

Uh oh!

selfhoster1312 commented Oct 25, 2025

Uh oh!

mdosch commented Oct 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mdosch commented Oct 25, 2025

Uh oh!

mdosch commented Oct 25, 2025

Uh oh!

selfhoster1312 commented Oct 25, 2025

Uh oh!

mdosch commented Oct 25, 2025

Uh oh!

selfhoster1312 commented Oct 25, 2025

Uh oh!

selfhoster1312 commented Oct 25, 2025

Uh oh!

mdosch commented Oct 25, 2025 via email

Uh oh!

mdosch commented Oct 25, 2025 via email

Uh oh!

selfhoster1312 commented Oct 25, 2025

Uh oh!

mdosch commented Oct 25, 2025 via email

Uh oh!

mdosch commented Oct 25, 2025 via email

Uh oh!

selfhoster1312 commented Oct 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

selfhoster1312 commented Oct 24, 2025 •

edited

Loading

mdosch commented Oct 24, 2025 •

edited

Loading

selfhoster1312 commented Oct 24, 2025 •

edited

Loading

mdosch commented Oct 25, 2025 •

edited

Loading