handle scram errors more gracefully, not aborting the connection

for some errors during the scram authentication protocol, we would treat some
errors that a client connection could induce as server errors, printing a stack
trace and aborting the connection.

this change recognizes those errors and sends regular "authentication failed"
or "protocol error" error messages to the client.

for issue #222 by wneessen, thanks for reporting

imapserver/authenticate_test.go

@@ -147,7 +147,7 @@ func testAuthenticateSCRAM(t *testing.T, tls bool, method string, h func() hash.
 
 	tc.transactf("no", "authenticate bogus ")
 	tc.transactf("bad", "authenticate %s not base64...", method)
-	tc.transactf("bad", "authenticate %s %s", method, base64.StdEncoding.EncodeToString([]byte("bad data")))
+	tc.transactf("no", "authenticate %s %s", method, base64.StdEncoding.EncodeToString([]byte("bad data")))
 
 	// NFD username, with PRECIS-cleaned password.
 	auth("ok", nil, "mo\u0301x@mox.example", password1)

imapserver/server.go

@@ -1708,7 +1708,8 @@ func (c *conn) cmdAuthenticate(tag, cmd string, p *parser) {
 		c0 := xreadInitial()
 		ss, err := scram.NewServer(h, c0, cs, requireChannelBinding)
 		if err != nil {
-			xsyntaxErrorf("starting scram: %s", err)
+			c.log.Infox("scram protocol error", err, slog.Any("remote", c.remoteIP))
+			xuserErrorf("scram protocol error: %s", err)
 		}
 		c.log.Debug("scram auth", slog.String("authentication", ss.Authentication))
 		acc, _, err := store.OpenEmail(c.log, ss.Authentication)
@@ -1767,6 +1768,13 @@ func (c *conn) cmdAuthenticate(tag, cmd string, p *parser) {
 				authResult = "badcreds"
 				c.log.Info("failed authentication attempt", slog.String("username", ss.Authentication), slog.Any("remote", c.remoteIP))
 				xusercodeErrorf("AUTHENTICATIONFAILED", "bad credentials")
+			} else if errors.Is(err, scram.ErrChannelBindingsDontMatch) {
+				authResult = "badchanbind"
+				c.log.Warn("bad channel binding during authentication, potential mitm", slog.String("username", ss.Authentication), slog.Any("remote", c.remoteIP))
+				xusercodeErrorf("AUTHENTICATIONFAILED", "channel bindings do not match, potential mitm")
+			} else if errors.Is(err, scram.ErrInvalidEncoding) {
+				c.log.Infox("bad scram protocol message", err, slog.String("username", ss.Authentication), slog.Any("remote", c.remoteIP))
+				xuserErrorf("bad scram protocol message: %s", err)
 			}
 			xuserErrorf("server final: %w", err)
 		}

metrics/auth.go

@@ -16,7 +16,7 @@ var (
 			"kind",    // submission, imap, webmail, webapi, webaccount, webadmin (formerly httpaccount, httpadmin)
 			"variant", // login, plain, scram-sha-256, scram-sha-1, cram-md5, weblogin, websessionuse, httpbasic.
 			// todo: we currently only use badcreds, but known baduser can be helpful
-			"result", // ok, baduser, badpassword, badcreds, error, aborted
+			"result", // ok, baduser, badpassword, badcreds, badchanbind, error, aborted
 		},
 	)
 

smtpserver/server.go

@@ -1186,11 +1186,9 @@ func (c *conn) cmdAuth(p *parser) {
 		addr := norm.NFC.String(t[0])
 		c.log.Debug("cram-md5 auth", slog.String("address", addr))
 		acc, _, err := store.OpenEmail(c.log, addr)
-		if err != nil {
+		if err != nil && errors.Is(err, store.ErrUnknownCredentials) {
-			if errors.Is(err, store.ErrUnknownCredentials) {
+			c.log.Info("failed authentication attempt", slog.String("username", addr), slog.Any("remote", c.remoteIP))
-				c.log.Info("failed authentication attempt", slog.String("username", addr), slog.Any("remote", c.remoteIP))
+			xsmtpUserErrorf(smtp.C535AuthBadCreds, smtp.SePol7AuthBadCreds8, "bad user/pass")
-				xsmtpUserErrorf(smtp.C535AuthBadCreds, smtp.SePol7AuthBadCreds8, "bad user/pass")
-			}
 		}
 		xcheckf(err, "looking up address")
 		defer func() {
@@ -1271,7 +1269,10 @@ func (c *conn) cmdAuth(p *parser) {
 		}
 		c0 := xreadInitial("")
 		ss, err := scram.NewServer(h, c0, cs, channelBindingRequired)
-		xcheckf(err, "starting scram")
+		if err != nil {
+			c.log.Infox("scram protocol error", err, slog.Any("remote", c.remoteIP))
+			xsmtpUserErrorf(smtp.C455BadParams, smtp.SePol7Other0, "scram protocol error: %s", err)
+		}
 		authc := norm.NFC.String(ss.Authentication)
 		c.log.Debug("scram auth", slog.String("authentication", authc))
 		acc, _, err := store.OpenEmail(c.log, authc)
@@ -1332,6 +1333,13 @@ func (c *conn) cmdAuth(p *parser) {
 				authResult = "badcreds"
 				c.log.Info("failed authentication attempt", slog.String("username", authc), slog.Any("remote", c.remoteIP))
 				xsmtpUserErrorf(smtp.C535AuthBadCreds, smtp.SePol7AuthBadCreds8, "bad credentials")
+			} else if errors.Is(err, scram.ErrChannelBindingsDontMatch) {
+				authResult = "badchanbind"
+				c.log.Warn("bad channel binding during authentication, potential mitm", slog.String("username", authc), slog.Any("remote", c.remoteIP))
+				xsmtpUserErrorf(smtp.C535AuthBadCreds, smtp.SePol7MsgIntegrity7, "channel bindings do not match, potential mitm")
+			} else if errors.Is(err, scram.ErrInvalidEncoding) {
+				c.log.Infox("bad scram protocol message", err, slog.String("username", authc), slog.Any("remote", c.remoteIP))
+				xsmtpUserErrorf(smtp.C535AuthBadCreds, smtp.SePol7Other0, "bad scram protocol message")
 			}
 			xcheckf(err, "server final")
 		}