mjl- / mox

modern full-featured open source secure mail server for low-maintenance self-hosted email
https://www.xmox.nl
MIT License
3.57k stars 99 forks source link

Broken SCRAM-SHA-X-PLUS response causes panic #222

Open wneessen opened 1 week ago

wneessen commented 1 week ago

While working on the SCRAM-SHA-X-PLUS support for go-mail I was playing with the authzid parameter and while submitting invalid data, I caused a panic:

Oct 01 11:16:34 dev-vm mox[185464]: l=debug m="scram auth" pkg=smtpserver authentication=wneessen@dev-vm.neessen.local cid=1924727303a delta=3.709543ms
Oct 01 11:16:34 dev-vm mox[185464]: l=debug m="smtp command result" err="server final: channel-bindings-dont-match" pkg=smtpserver kind=submission cmd=auth code=451 ecode=4.3.0 duration=36.973063ms cid=1924727303a delta=35.154895ms
Oct 01 11:16:34 dev-vm mox[185464]: goroutine 328 [running]:
Oct 01 11:16:34 dev-vm mox[185464]: runtime/debug.Stack()
Oct 01 11:16:34 dev-vm mox[185464]:         runtime/debug/stack.go:26 +0x5e
Oct 01 11:16:34 dev-vm mox[185464]: runtime/debug.PrintStack()
Oct 01 11:16:34 dev-vm mox[185464]:         runtime/debug/stack.go:18 +0x13
Oct 01 11:16:34 dev-vm mox[185464]: github.com/mjl-/mox/smtpserver.command.func1()
Oct 01 11:16:34 dev-vm mox[185464]:         github.com/mjl-/mox@v0.0.11/smtpserver/server.go:750 +0x18c
Oct 01 11:16:34 dev-vm mox[185464]: panic({0x10460e0?, 0xc000af98c0?})
Oct 01 11:16:34 dev-vm mox[185464]:         runtime/panic.go:785 +0x132
Oct 01 11:16:34 dev-vm mox[185464]: github.com/mjl-/mox/smtpserver.xcheckf({0x129f8c0, 0xc0006a3c20}, {0x10ba50e?, 0xc0006a3c50?}, {0x0?, 0x20?, 0x20?})
Oct 01 11:16:34 dev-vm mox[185464]:         github.com/mjl-/mox@v0.0.11/smtpserver/error.go:12 +0x17e
Oct 01 11:16:34 dev-vm mox[185464]: github.com/mjl-/mox/smtpserver.(*conn).cmdAuth(0xc0007ea008, 0xc00001ac00)
Oct 01 11:16:34 dev-vm mox[185464]:         github.com/mjl-/mox@v0.0.11/smtpserver/server.go:1325 +0x296e
Oct 01 11:16:34 dev-vm mox[185464]: github.com/mjl-/mox/smtpserver.command(0xc0007ea008)
Oct 01 11:16:34 dev-vm mox[185464]:         github.com/mjl-/mox@v0.0.11/smtpserver/server.go:798 +0x383
Oct 01 11:16:34 dev-vm mox[185464]: github.com/mjl-/mox/smtpserver.serve({0xc0005d0771, 0x6}, 0x1924727303a, {{0xc0000bc5aa?, 0x14?}, {0x0?, 0x0?}}, 0xc0005c3180, {0x12ab390, 0xc000066000}, ...)
Oct 01 11:16:34 dev-vm mox[185464]:         github.com/mjl-/mox@v0.0.11/smtpserver/server.go:700 +0x1385
Oct 01 11:16:34 dev-vm mox[185464]: created by github.com/mjl-/mox/smtpserver.listen1.func1 in goroutine 114
Oct 01 11:16:34 dev-vm mox[185464]:         github.com/mjl-/mox@v0.0.11/smtpserver/server.go:268 +0x45f
Oct 01 11:16:34 dev-vm mox[185464]: l=debug m="smtp command result" pkg=smtpserver kind=submission cmd=quit code=221 ecode=2.0.0 duration="22.34µs" cid=1924727303a delta="871.883µs"
Oct 01 11:16:34 dev-vm mox[185464]: l=info m="connection closed" pkg=smtpserver cid=1924727303a delta="562.406µs"

What I did is, in the response after the initial server response, I added invalid data in the field where a=authzid is expected. The server expects a base64 representation of p=<binding type>,[a=authzid],<bind_data>. I instead did not issue the a= and just put 1234 into that field.

Here is the output of my test showing the full client response to the server. You can see the base64 encoded c= part and the server respone. In the server log it showed the above mentioned panic:

=== RUN   TestClient_AuthSCRAMSHA256PLUS
algo: SCRAM-SHA-256-PLUS
2nd client response: c=cD10bHMtZXhwb3J0ZXIsMTIzNCxg/HNntg56uDGEF6/+7YVMtx52l/mkwaG4Zin43TQlYg==,r=AasuipCKrB0o8BoDBm9SekYS+/HKVNhPhWvArh/UlPybbdPa,p=JCyr69VsNPrHxKGYk9nuV7x94KGWmBvphBWUUlu59a4=
    client_test.go:1859: failed to dial to test server: SMTP AUTH failed: unexpected server response
    client_test.go:1862: failed to send mail: checking SMTP connection: not connected to SMTP server
    client_test.go:1865: failed to close server connection: not connected to SMTP server
--- FAIL: TestClient_AuthSCRAMSHA1 (0.38s)

If I remove the authzid data, everything works fine and authentication works.

mjl- commented 1 week ago

Thanks for reporting! The commit above recognizes the errors that can happen during the scram exchange, and returns regular proper error responses instead of aborting the connection.