diff --git a/README.md b/README.md index 49ab6a9..26d1ad0 100644 --- a/README.md +++ b/README.md @@ -115,10 +115,8 @@ https://nlnet.nl/project/Mox/. ## Roadmap -- Quota support in backend -- SASL SCRAM TLS binding -- Improve documentation - Integrate account page into webmail +- Improve documentation - Per-domain webmail and IMAP/SMTP host name (and TLS cert) and client settings - Authentication other than HTTP-basic for webmail/webadmin - Improve SMTP delivery from queue diff --git a/apidiff/v0.0.9.txt b/apidiff/v0.0.9.txt index c1c221b..e949119 100644 --- a/apidiff/v0.0.9.txt +++ b/apidiff/v0.0.9.txt @@ -42,9 +42,13 @@ Below are the incompatible changes between v0.0.8 and v0.0.9, per package. # ratelimit # sasl +- NewClientSCRAMSHA1: changed from func(string, string) Client to func(string, string, bool) Client +- NewClientSCRAMSHA256: changed from func(string, string) Client to func(string, string, bool) Client # scram - HMAC: removed +- NewClient: changed from func(func() hash.Hash, string, string) *Client to func(func() hash.Hash, string, string, bool, *crypto/tls.ConnectionState) *Client +- NewServer: changed from func(func() hash.Hash, []byte) (*Server, error) to func(func() hash.Hash, []byte, *crypto/tls.ConnectionState, bool) (*Server, error) # smtp @@ -55,6 +59,7 @@ Below are the incompatible changes between v0.0.8 and v0.0.9, per package. - GatherIPs: changed from func(context.Context, *github.com/mjl-/mox/mlog.Log, github.com/mjl-/mox/dns.Resolver, github.com/mjl-/mox/dns.IPDomain, map[string][]net.IP) (bool, bool, github.com/mjl-/mox/dns.Domain, []net.IP, bool, error) to func(context.Context, *golang.org/x/exp/slog.Logger, github.com/mjl-/mox/dns.Resolver, github.com/mjl-/mox/dns.IPDomain, map[string][]net.IP) (bool, bool, github.com/mjl-/mox/dns.Domain, []net.IP, bool, error) - GatherTLSA: changed from func(context.Context, *github.com/mjl-/mox/mlog.Log, github.com/mjl-/mox/dns.Resolver, github.com/mjl-/mox/dns.Domain, bool, github.com/mjl-/mox/dns.Domain) (bool, []github.com/mjl-/adns.TLSA, github.com/mjl-/mox/dns.Domain, error) to func(context.Context, *golang.org/x/exp/slog.Logger, github.com/mjl-/mox/dns.Resolver, github.com/mjl-/mox/dns.Domain, bool, github.com/mjl-/mox/dns.Domain) (bool, []github.com/mjl-/adns.TLSA, github.com/mjl-/mox/dns.Domain, error) - New: changed from func(context.Context, *github.com/mjl-/mox/mlog.Log, net.Conn, TLSMode, bool, github.com/mjl-/mox/dns.Domain, github.com/mjl-/mox/dns.Domain, Opts) (*Client, error) to func(context.Context, *golang.org/x/exp/slog.Logger, net.Conn, TLSMode, bool, github.com/mjl-/mox/dns.Domain, github.com/mjl-/mox/dns.Domain, Opts) (*Client, error) +- Opts.Auth: changed from []github.com/mjl-/mox/sasl.Client to func([]string, *crypto/tls.ConnectionState) (github.com/mjl-/mox/sasl.Client, error) # spf - Evaluate: changed from func(context.Context, *Record, github.com/mjl-/mox/dns.Resolver, Args) (Status, string, string, bool, error) to func(context.Context, *golang.org/x/exp/slog.Logger, *Record, github.com/mjl-/mox/dns.Resolver, Args) (Status, string, string, bool, error) diff --git a/config/config.go b/config/config.go index 4721f3d..a9c335d 100644 --- a/config/config.go +++ b/config/config.go @@ -260,7 +260,7 @@ type TransportSMTP struct { type SMTPAuth struct { Username string Password string - Mechanisms []string `sconf:"optional" sconf-doc:"Allowed authentication mechanisms. Defaults to SCRAM-SHA-256, SCRAM-SHA-1, CRAM-MD5. Not included by default: PLAIN."` + Mechanisms []string `sconf:"optional" sconf-doc:"Allowed authentication mechanisms. Defaults to SCRAM-SHA-256-PLUS, SCRAM-SHA-256, SCRAM-SHA-1-PLUS, SCRAM-SHA-1, CRAM-MD5. Not included by default: PLAIN. Specify the strongest mechanism known to be implemented by the server to prevent mechanism downgrade attacks."` EffectiveMechanisms []string `sconf:"-" json:"-"` } diff --git a/config/doc.go b/config/doc.go index 1069fae..aee588e 100644 --- a/config/doc.go +++ b/config/doc.go @@ -501,8 +501,10 @@ describe-static" and "mox config describe-domains": Username: Password: - # Allowed authentication mechanisms. Defaults to SCRAM-SHA-256, SCRAM-SHA-1, - # CRAM-MD5. Not included by default: PLAIN. (optional) + # Allowed authentication mechanisms. Defaults to SCRAM-SHA-256-PLUS, + # SCRAM-SHA-256, SCRAM-SHA-1-PLUS, SCRAM-SHA-1, CRAM-MD5. Not included by default: + # PLAIN. Specify the strongest mechanism known to be implemented by the server to + # prevent mechanism downgrade attacks. (optional) Mechanisms: - @@ -532,8 +534,10 @@ describe-static" and "mox config describe-domains": Username: Password: - # Allowed authentication mechanisms. Defaults to SCRAM-SHA-256, SCRAM-SHA-1, - # CRAM-MD5. Not included by default: PLAIN. (optional) + # Allowed authentication mechanisms. Defaults to SCRAM-SHA-256-PLUS, + # SCRAM-SHA-256, SCRAM-SHA-1-PLUS, SCRAM-SHA-1, CRAM-MD5. Not included by default: + # PLAIN. Specify the strongest mechanism known to be implemented by the server to + # prevent mechanism downgrade attacks. (optional) Mechanisms: - @@ -563,8 +567,10 @@ describe-static" and "mox config describe-domains": Username: Password: - # Allowed authentication mechanisms. Defaults to SCRAM-SHA-256, SCRAM-SHA-1, - # CRAM-MD5. Not included by default: PLAIN. (optional) + # Allowed authentication mechanisms. Defaults to SCRAM-SHA-256-PLUS, + # SCRAM-SHA-256, SCRAM-SHA-1-PLUS, SCRAM-SHA-1, CRAM-MD5. Not included by default: + # PLAIN. Specify the strongest mechanism known to be implemented by the server to + # prevent mechanism downgrade attacks. (optional) Mechanisms: - diff --git a/imapclient/client.go b/imapclient/client.go index 91bfe09..c9addb5 100644 --- a/imapclient/client.go +++ b/imapclient/client.go @@ -16,6 +16,7 @@ package imapclient import ( "bufio" + "crypto/tls" "fmt" "net" "reflect" @@ -117,6 +118,15 @@ func (c *Conn) xcheck(err error) { } } +// TLSConnectionState returns the TLS connection state if the connection uses TLS. +func (c *Conn) TLSConnectionState() *tls.ConnectionState { + if conn, ok := c.conn.(*tls.Conn); ok { + cs := conn.ConnectionState() + return &cs + } + return nil +} + // Commandf writes a free-form IMAP command to the server. // If tag is empty, a next unique tag is assigned. func (c *Conn) Commandf(tag string, format string, args ...any) (rerr error) { diff --git a/imapclient/cmds.go b/imapclient/cmds.go index e220bcf..1d9c7ea 100644 --- a/imapclient/cmds.go +++ b/imapclient/cmds.go @@ -61,13 +61,26 @@ func (c *Conn) AuthenticatePlain(username, password string) (untagged []Untagged return } -// Authenticate with SCRAM-SHA-1 or SCRAM-SHA-256, where the password is not -// exchanged in original plaintext form, but only derived hashes are exchanged by -// both parties as proof of knowledge of password. +// Authenticate with SCRAM-SHA-256(-PLUS) or SCRAM-SHA-1(-PLUS). With SCRAM, the +// password is not exchanged in plaintext form, but only derived hashes are +// exchanged by both parties as proof of knowledge of password. +// +// The PLUS variants bind the authentication exchange to the TLS connection, +// detecting MitM attacks. func (c *Conn) AuthenticateSCRAM(method string, h func() hash.Hash, username, password string) (untagged []Untagged, result Result, rerr error) { defer c.recover(&rerr) - sc := scram.NewClient(h, username, "") + var cs *tls.ConnectionState + lmethod := strings.ToLower(method) + if strings.HasSuffix(lmethod, "-plus") { + tlsConn, ok := c.conn.(*tls.Conn) + if !ok { + c.xerrorf("cannot use scram plus without tls") + } + xcs := tlsConn.ConnectionState() + cs = &xcs + } + sc := scram.NewClient(h, username, "", false, cs) clientFirst, err := sc.ClientFirst() c.xcheckf(err, "scram clientFirst") c.LastTag = c.nextTag() diff --git a/imapserver/authenticate_test.go b/imapserver/authenticate_test.go index 8180cc7..07d629f 100644 --- a/imapserver/authenticate_test.go +++ b/imapserver/authenticate_test.go @@ -60,22 +60,31 @@ func TestAuthenticatePlain(t *testing.T) { } func TestAuthenticateSCRAMSHA1(t *testing.T) { - testAuthenticateSCRAM(t, "SCRAM-SHA-1", sha1.New) + testAuthenticateSCRAM(t, false, "SCRAM-SHA-1", sha1.New) } func TestAuthenticateSCRAMSHA256(t *testing.T) { - testAuthenticateSCRAM(t, "SCRAM-SHA-256", sha256.New) + testAuthenticateSCRAM(t, false, "SCRAM-SHA-256", sha256.New) } -func testAuthenticateSCRAM(t *testing.T, method string, h func() hash.Hash) { - tc := start(t) +func TestAuthenticateSCRAMSHA1PLUS(t *testing.T) { + testAuthenticateSCRAM(t, true, "SCRAM-SHA-1-PLUS", sha1.New) +} + +func TestAuthenticateSCRAMSHA256PLUS(t *testing.T) { + testAuthenticateSCRAM(t, true, "SCRAM-SHA-256-PLUS", sha256.New) +} + +func testAuthenticateSCRAM(t *testing.T, tls bool, method string, h func() hash.Hash) { + tc := startArgs(t, true, tls, true, true, "mjl") tc.client.AuthenticateSCRAM(method, h, "mjl@mox.example", "testtest") tc.close() auth := func(status string, serverFinalError error, username, password string) { t.Helper() - sc := scram.NewClient(h, username, "") + noServerPlus := false + sc := scram.NewClient(h, username, "", noServerPlus, tc.client.TLSConnectionState()) clientFirst, err := sc.ClientFirst() tc.check(err, "scram clientFirst") tc.client.LastTag = "x001" @@ -116,7 +125,7 @@ func testAuthenticateSCRAM(t *testing.T, method string, h func() hash.Hash) { } } - tc = start(t) + tc = startArgs(t, true, tls, true, true, "mjl") auth("no", scram.ErrInvalidProof, "mjl@mox.example", "badpass") auth("no", scram.ErrInvalidProof, "mjl@mox.example", "") // todo: server aborts due to invalid username. we should probably make client continue with fake determinisitically generated salt and result in error in the end. diff --git a/imapserver/server.go b/imapserver/server.go index a197161..b0eca2a 100644 --- a/imapserver/server.go +++ b/imapserver/server.go @@ -150,13 +150,18 @@ var authFailDelay = time.Second // After authentication failure. // SPECIAL-USE: ../rfc/6154 // LIST-STATUS: ../rfc/5819 // ID: ../rfc/2971 -// AUTH=SCRAM-SHA-256: ../rfc/7677 ../rfc/5802 -// AUTH=SCRAM-SHA-1: ../rfc/5802 +// AUTH=SCRAM-SHA-256-PLUS and AUTH=SCRAM-SHA-256: ../rfc/7677 ../rfc/5802 +// AUTH=SCRAM-SHA-1-PLUS and AUTH=SCRAM-SHA-1: ../rfc/5802 // AUTH=CRAM-MD5: ../rfc/2195 // APPENDLIMIT, we support the max possible size, 1<<63 - 1: ../rfc/7889:129 // CONDSTORE: ../rfc/7162:411 // QRESYNC: ../rfc/7162:1323 -const serverCapabilities = "IMAP4rev2 IMAP4rev1 ENABLE LITERAL+ IDLE SASL-IR BINARY UNSELECT UIDPLUS ESEARCH SEARCHRES MOVE UTF8=ONLY LIST-EXTENDED SPECIAL-USE LIST-STATUS AUTH=SCRAM-SHA-256 AUTH=SCRAM-SHA-1 AUTH=CRAM-MD5 ID APPENDLIMIT=9223372036854775807 CONDSTORE QRESYNC" +// +// We always announce support for SCRAM PLUS-variants, also on connections without +// TLS. The client should not be selecting PLUS variants on non-TLS connections, +// instead opting to do the bare SCRAM variant without indicating the server claims +// to support the PLUS variant (skipping the server downgrade detection check). +const serverCapabilities = "IMAP4rev2 IMAP4rev1 ENABLE LITERAL+ IDLE SASL-IR BINARY UNSELECT UIDPLUS ESEARCH SEARCHRES MOVE UTF8=ONLY LIST-EXTENDED SPECIAL-USE LIST-STATUS AUTH=SCRAM-SHA-256-PLUS AUTH=SCRAM-SHA-256 AUTH=SCRAM-SHA-1-PLUS AUTH=SCRAM-SHA-1 AUTH=CRAM-MD5 ID APPENDLIMIT=9223372036854775807 CONDSTORE QRESYNC" type conn struct { cid int64 @@ -1654,22 +1659,34 @@ func (c *conn) cmdAuthenticate(tag, cmd string, p *parser) { acc = nil // Cancel cleanup. c.username = addr - case "SCRAM-SHA-1", "SCRAM-SHA-256": + case "SCRAM-SHA-256-PLUS", "SCRAM-SHA-256", "SCRAM-SHA-1-PLUS", "SCRAM-SHA-1": // todo: improve handling of errors during scram. e.g. invalid parameters. should we abort the imap command, or continue until the end and respond with a scram-level error? // todo: use single implementation between ../imapserver/server.go and ../smtpserver/server.go - authVariant = strings.ToLower(authType) - var h func() hash.Hash - if authVariant == "scram-sha-1" { - h = sha1.New - } else { - h = sha256.New - } - // No plaintext credentials, we can log these normally. + authVariant = strings.ToLower(authType) + var h func() hash.Hash + switch authVariant { + case "scram-sha-1", "scram-sha-1-plus": + h = sha1.New + case "scram-sha-256", "scram-sha-256-plus": + h = sha256.New + default: + xserverErrorf("missing case for scram variant") + } + + var cs *tls.ConnectionState + requireChannelBinding := strings.HasSuffix(authVariant, "-plus") + if requireChannelBinding && !c.tls { + xuserErrorf("cannot use plus variant with tls channel binding without tls") + } + if c.tls { + xcs := c.conn.(*tls.Conn).ConnectionState() + cs = &xcs + } c0 := xreadInitial() - ss, err := scram.NewServer(h, c0) + ss, err := scram.NewServer(h, c0, cs, requireChannelBinding) if err != nil { xsyntaxErrorf("starting scram: %s", err) } @@ -1694,10 +1711,13 @@ func (c *conn) cmdAuthenticate(tag, cmd string, p *parser) { acc.WithRLock(func() { err := acc.DB.Read(context.TODO(), func(tx *bstore.Tx) error { password, err := bstore.QueryTx[store.Password](tx).Get() - if authVariant == "scram-sha-1" { + switch authVariant { + case "scram-sha-1", "scram-sha-1-plus": xscram = password.SCRAMSHA1 - } else { + case "scram-sha-256", "scram-sha-256-plus": xscram = password.SCRAMSHA256 + default: + xserverErrorf("missing case for scram credentials") } if err == bstore.ErrAbsent || err == nil && (len(xscram.Salt) == 0 || xscram.Iterations == 0 || len(xscram.SaltedPassword) == 0) { c.log.Info("scram auth attempt without derived secrets set, save password again to store secrets", slog.String("address", ss.Authentication)) diff --git a/integration_test.go b/integration_test.go index a98297c..7c2e577 100644 --- a/integration_test.go +++ b/integration_test.go @@ -130,7 +130,9 @@ Subject: test message This is the message. `, mailfrom, rcptto) msg = strings.ReplaceAll(msg, "\n", "\r\n") - auth := []sasl.Client{sasl.NewClientPlain(mailfrom, password)} + auth := func(mechanisms []string, cs *tls.ConnectionState) (sasl.Client, error) { + return sasl.NewClientPlain(mailfrom, password), nil + } c, err := smtpclient.New(mox.Context, log.Logger, conn, smtpclient.TLSSkip, false, ourHostname, dns.Domain{ASCII: desthost}, smtpclient.Opts{Auth: auth}) tcheck(t, err, "smtp hello") err = c.Deliver(mox.Context, mailfrom, rcptto, int64(len(msg)), strings.NewReader(msg), false, false, false) diff --git a/mox-/config.go b/mox-/config.go index d96362e..d595a43 100644 --- a/mox-/config.go +++ b/mox-/config.go @@ -881,7 +881,9 @@ func PrepareStaticConfig(ctx context.Context, log mlog.Log, configFile string, c } seen[m] = true switch m { + case "SCRAM-SHA-256-PLUS": case "SCRAM-SHA-256": + case "SCRAM-SHA-1-PLUS": case "SCRAM-SHA-1": case "CRAM-MD5": case "PLAIN": @@ -892,7 +894,7 @@ func PrepareStaticConfig(ctx context.Context, log mlog.Log, configFile string, c t.Auth.EffectiveMechanisms = t.Auth.Mechanisms if len(t.Auth.EffectiveMechanisms) == 0 { - t.Auth.EffectiveMechanisms = []string{"SCRAM-SHA-256", "SCRAM-SHA-1", "CRAM-MD5"} + t.Auth.EffectiveMechanisms = []string{"SCRAM-SHA-256-PLUS", "SCRAM-SHA-256", "SCRAM-SHA-1-PLUS", "SCRAM-SHA-1", "CRAM-MD5"} } } diff --git a/queue/submit.go b/queue/submit.go index 40ed32a..45cd3ae 100644 --- a/queue/submit.go +++ b/queue/submit.go @@ -3,6 +3,7 @@ package queue import ( "bytes" "context" + "crypto/tls" "errors" "fmt" "io" @@ -10,6 +11,7 @@ import ( "os" "time" + "golang.org/x/exp/slices" "golang.org/x/exp/slog" "github.com/mjl-/mox/config" @@ -40,7 +42,7 @@ func deliverSubmit(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale if dialTLS { tlsMode = smtpclient.TLSImmediate } else if transport.STARTTLSInsecureSkipVerify { - tlsMode = smtpclient.TLSOpportunistic + tlsMode = smtpclient.TLSRequiredStartTLS tlsPKIX = false } else if transport.NoSTARTTLS { tlsMode = smtpclient.TLSSkip @@ -119,26 +121,39 @@ func deliverSubmit(qlog mlog.Log, resolver dns.Resolver, dialer smtpclient.Diale } dialcancel() - var auth []sasl.Client + var auth func(mechanisms []string, cs *tls.ConnectionState) (sasl.Client, error) if transport.Auth != nil { a := transport.Auth - for _, mech := range a.EffectiveMechanisms { - switch mech { - case "PLAIN": - auth = append(auth, sasl.NewClientPlain(a.Username, a.Password)) - case "CRAM-MD5": - auth = append(auth, sasl.NewClientCRAMMD5(a.Username, a.Password)) - case "SCRAM-SHA-1": - auth = append(auth, sasl.NewClientSCRAMSHA1(a.Username, a.Password)) - case "SCRAM-SHA-256": - auth = append(auth, sasl.NewClientSCRAMSHA256(a.Username, a.Password)) - default: - // Should not happen. - qlog.Error("missing smtp authentication mechanisms implementation", slog.String("mechanism", mech)) - errmsg = fmt.Sprintf("transport %s: authentication mechanisms %q not implemented", transportName, mech) - fail(ctx, qlog, m, backoff, false, dsn.NameIP{}, "", errmsg) - return + auth = func(mechanisms []string, cs *tls.ConnectionState) (sasl.Client, error) { + var supportsscramsha1plus, supportsscramsha256plus bool + for _, mech := range a.EffectiveMechanisms { + if !slices.Contains(mechanisms, mech) { + switch mech { + case "SCRAM-SHA-1-PLUS": + supportsscramsha1plus = cs != nil + case "SCRAM-SHA-256-PLUS": + supportsscramsha256plus = cs != nil + } + continue + } + if mech == "SCRAM-SHA-256-PLUS" && cs != nil { + return sasl.NewClientSCRAMSHA256PLUS(a.Username, a.Password, *cs), nil + } else if mech == "SCRAM-SHA-256" { + return sasl.NewClientSCRAMSHA256(a.Username, a.Password, supportsscramsha256plus), nil + } else if mech == "SCRAM-SHA-1-PLUS" && cs != nil { + return sasl.NewClientSCRAMSHA1PLUS(a.Username, a.Password, *cs), nil + } else if mech == "SCRAM-SHA-1" { + return sasl.NewClientSCRAMSHA1(a.Username, a.Password, supportsscramsha1plus), nil + } else if mech == "CRAM-MD5" { + return sasl.NewClientCRAMMD5(a.Username, a.Password), nil + } else if mech == "PLAIN" { + return sasl.NewClientPlain(a.Username, a.Password), nil + } + return nil, fmt.Errorf("internal error: unrecognized authentication mechanism %q for transport %s", mech, transportName) } + + // No mutually supported algorithm. + return nil, nil } } clientctx, clientcancel := context.WithTimeout(context.Background(), 60*time.Second) diff --git a/rfc/index.txt b/rfc/index.txt index e9ba2b9..574d6f5 100644 --- a/rfc/index.txt +++ b/rfc/index.txt @@ -286,12 +286,18 @@ See implementation guide, https://jmap.io/server.html 5518 Vouch By Reference # TLS +5056 On the Use of Channel Bindings to Secure Channels +5705 Keying Material Exporters for Transport Layer Security (TLS) +5929 Channel Bindings for TLS 6125 Representation and Verification of Domain-Based Application Service Identity within Internet Public Key Infrastructure Using X.509 (PKIX) Certificates in the Context of Transport Layer Security (TLS) 7250 Using Raw Public Keys in Transport Layer Security (TLS) and Datagram Transport Layer Security (DTLS) 7525 Recommendations for Secure Use of Transport Layer Security (TLS) and Datagram Transport Layer Security (DTLS) +7627 Transport Layer Security (TLS) Session Hash and Extended Master Secret Extension 8314 Cleartext Considered Obsolete: Use of Transport Layer Security (TLS) for Email Submission and Access +8446 The Transport Layer Security (TLS) Protocol Version 1.3 8996 Deprecating TLS 1.0 and TLS 1.1 8997 Deprecation of TLS 1.1 for Email Submission and Access +9266 Channel Bindings for TLS 1.3 # SASL diff --git a/sasl/sasl.go b/sasl/sasl.go index 2594e8b..96b87dd 100644 --- a/sasl/sasl.go +++ b/sasl/sasl.go @@ -5,6 +5,7 @@ import ( "crypto/md5" "crypto/sha1" "crypto/sha256" + "crypto/tls" "fmt" "hash" "strings" @@ -171,6 +172,15 @@ func (a *clientCRAMMD5) Next(fromServer []byte) (toServer []byte, last bool, rer type clientSCRAMSHA struct { Username, Password string + hash func() hash.Hash + + plus bool + cs tls.ConnectionState + + // When not doing PLUS variant, this field indicates whether that is because the + // server doesn't support the PLUS variant. Used for detecting MitM attempts. + noServerPlus bool + name string step int scram *scram.Client @@ -180,18 +190,52 @@ var _ Client = (*clientSCRAMSHA)(nil) // NewClientSCRAMSHA1 returns a client for SASL SCRAM-SHA-1 authentication. // -// SCRAM-SHA-1 is specified in RFC 5802, Salted Challenge Response Authentication -// Mechanism (SCRAM) SASL and GSS-API Mechanisms. -func NewClientSCRAMSHA1(username, password string) Client { - return &clientSCRAMSHA{username, password, "SCRAM-SHA-1", 0, nil} +// Clients should prefer using the PLUS-variant with TLS channel binding, if +// supported by a server. If noServerPlus is set, this mechanism was chosen because +// the PLUS-variant was not supported by the server. If the server actually does +// implement the PLUS variant, this can indicate a MitM attempt, which is detected +// by the server and causes the authentication attempt to be aborted. +// +// SCRAM-SHA-1 is specified in RFC 5802, "Salted Challenge Response Authentication +// Mechanism (SCRAM) SASL and GSS-API Mechanisms". +func NewClientSCRAMSHA1(username, password string, noServerPlus bool) Client { + return &clientSCRAMSHA{username, password, sha1.New, false, tls.ConnectionState{}, noServerPlus, "SCRAM-SHA-1", 0, nil} +} + +// NewClientSCRAMSHA1PLUS returns a client for SASL SCRAM-SHA-1-PLUS authentication. +// +// The PLUS-variant binds the authentication exchange to the TLS connection, +// detecting any MitM attempt. +// +// SCRAM-SHA-1-PLUS is specified in RFC 5802, "Salted Challenge Response +// Authentication Mechanism (SCRAM) SASL and GSS-API Mechanisms". +func NewClientSCRAMSHA1PLUS(username, password string, cs tls.ConnectionState) Client { + return &clientSCRAMSHA{username, password, sha1.New, true, cs, false, "SCRAM-SHA-1-PLUS", 0, nil} } // NewClientSCRAMSHA256 returns a client for SASL SCRAM-SHA-256 authentication. // -// SCRAM-SHA-256 is specified in RFC 7677, SCRAM-SHA-256 and SCRAM-SHA-256-PLUS -// Simple Authentication and Security Layer (SASL) Mechanisms. -func NewClientSCRAMSHA256(username, password string) Client { - return &clientSCRAMSHA{username, password, "SCRAM-SHA-256", 0, nil} +// Clients should prefer using the PLUS-variant with TLS channel binding, if +// supported by a server. If noServerPlus is set, this mechanism was chosen because +// the PLUS-variant was not supported by the server. If the server actually does +// implement the PLUS variant, this can indicate a MitM attempt, which is detected +// by the server and causes the authentication attempt to be aborted. +// +// SCRAM-SHA-256 is specified in RFC 7677, "SCRAM-SHA-256 and SCRAM-SHA-256-PLUS +// Simple Authentication and Security Layer (SASL) Mechanisms". +func NewClientSCRAMSHA256(username, password string, noServerPlus bool) Client { + return &clientSCRAMSHA{username, password, sha256.New, false, tls.ConnectionState{}, noServerPlus, "SCRAM-SHA-256", 0, nil} +} + +// NewClientSCRAMSHA256PLUS returns a client for SASL SCRAM-SHA-256-PLUS authentication. +// +// The PLUS-variant binds the authentication exchange to the TLS connection, +// detecting any MitM attempt. +// +// SCRAM-SHA-256-PLUS is specified in RFC 7677, "SCRAM-SHA-256 and SCRAM-SHA-256-PLUS +// Simple Authentication and Security Layer (SASL) Mechanisms". +func NewClientSCRAMSHA256PLUS(username, password string, cs tls.ConnectionState) Client { + return &clientSCRAMSHA{username, password, sha256.New, true, cs, false, "SCRAM-SHA-256-PLUS", 0, nil} } func (a *clientSCRAMSHA) Info() (name string, hasCleartextCredentials bool) { @@ -202,17 +246,11 @@ func (a *clientSCRAMSHA) Next(fromServer []byte) (toServer []byte, last bool, re defer func() { a.step++ }() switch a.step { case 0: - var h func() hash.Hash - switch a.name { - case "SCRAM-SHA-1": - h = sha1.New - case "SCRAM-SHA-256": - h = sha256.New - default: - return nil, false, fmt.Errorf("invalid SCRAM-SHA variant %q", a.name) + var cs *tls.ConnectionState + if a.plus { + cs = &a.cs } - - a.scram = scram.NewClient(h, a.Username, "") + a.scram = scram.NewClient(a.hash, a.Username, "", a.noServerPlus, cs) toserver, err := a.scram.ClientFirst() return []byte(toserver), false, err diff --git a/scram/examples_test.go b/scram/examples_test.go index f978419..c15136c 100644 --- a/scram/examples_test.go +++ b/scram/examples_test.go @@ -30,12 +30,12 @@ func Example() { // Make a new client for authenticating user mjl with SCRAM-SHA-256. username := "mjl" authz := "" - client := scram.NewClient(sha256.New, username, authz) + client := scram.NewClient(sha256.New, username, authz, false, nil) clientFirst, err := client.ClientFirst() check(err, "client.ClientFirst") - // Instantia a new server with the initial message from the client. - server, err := scram.NewServer(sha256.New, []byte(clientFirst)) + // Instantiate a new server with the initial message from the client. + server, err := scram.NewServer(sha256.New, []byte(clientFirst), nil, false) check(err, "NewServer") // Generate first message from server to client, with a challenge. diff --git a/scram/parse.go b/scram/parse.go index 28cd517..5de3ff8 100644 --- a/scram/parse.go +++ b/scram/parse.go @@ -223,9 +223,27 @@ func (p *parser) xsaslname() string { return r } -func (p *parser) xchannelBinding() string { +// ../rfc/5802:889 +func (p *parser) xcbname() string { + o := p.o + for ; o < len(p.s); o++ { + c := p.s[o] + if c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c >= '0' && c <= '9' || c == '.' || c == '-' { + continue + } + break + } + if o == p.o { + p.xerrorf("empty channel binding name") + } + r := p.s[p.o:o] + p.o = o + return string(r) +} + +func (p *parser) xchannelBinding() []byte { p.xtake("c=") - return string(p.xbase64()) + return p.xbase64() } func (p *parser) xproof() []byte { diff --git a/scram/scram.go b/scram/scram.go index 16db9c1..cff3f50 100644 --- a/scram/scram.go +++ b/scram/scram.go @@ -14,6 +14,7 @@ import ( "bytes" "crypto/hmac" cryptorand "crypto/rand" + "crypto/tls" "encoding/base64" "errors" "fmt" @@ -102,6 +103,20 @@ func xor(a, b []byte) { } } +func channelBindData(cs *tls.ConnectionState) ([]byte, error) { + if cs.Version <= tls.VersionTLS12 { + if cs.TLSUnique == nil { + return nil, fmt.Errorf("no channel binding data available") + } + return cs.TLSUnique, nil + } + + // "tls-exporter", ../rfc/9266:95 + // Since TLS 1.3, a zero-length and absent context have the same behaviour. ../rfc/8446:5385 ../rfc/8446:5405 + // This is different from TLS 1.2 and earlier. ../rfc/5705:206 ../rfc/5705:245 + return cs.ExportKeyingMaterial("EXPORTER-Channel-Binding", []byte{}, 32) +} + // Server represents the server-side of a SCRAM-SHA-* authentication. type Server struct { Authentication string // Username for authentication, "authc". Always set and non-empty. @@ -118,15 +133,24 @@ type Server struct { clientNonce string // Client-part of the nonce. serverNonceOverride string // If set, server does not generate random nonce, but uses this. For tests with the test vector. nonce string // Full client + server nonce. + channelBinding []byte } // NewServer returns a server given the first SCRAM message from a client. // +// If cs is set, the PLUS variant can be negotiated, binding the authentication +// exchange to the TLS channel (preventing MitM attempts). If a client +// indicates it supports the PLUS variant, but thinks the server does not, the +// authentication attempt will fail. +// +// If channelBindingRequired is set, the client has indicated it will do channel +// binding and not doing so will cause the authentication to fail. +// // The sequence for data and calls on a server: // // - Read initial data from client, call NewServer (this call), then ServerFirst and write to the client. // - Read response from client, call Finish or FinishFinal and write the resulting string. -func NewServer(h func() hash.Hash, clientFirst []byte) (server *Server, rerr error) { +func NewServer(h func() hash.Hash, clientFirst []byte, cs *tls.ConnectionState, channelBindingRequired bool) (server *Server, rerr error) { p := newParser(clientFirst) defer p.recover(&rerr) @@ -135,9 +159,58 @@ func NewServer(h func() hash.Hash, clientFirst []byte) (server *Server, rerr err // ../rfc/5802:949 ../rfc/5802:910 gs2cbindFlag := p.xbyte() switch gs2cbindFlag { - case 'n', 'y': + case 'n': + // Client does not support channel binding. + if channelBindingRequired { + p.xerrorf("channel binding is required when specifying scram plus: %w", ErrChannelBindingsDontMatch) + } + case 'y': + // Client supports channel binding but thinks we as server do not. + p.xerrorf("gs2 channel bind flag is y, client believes server does not support channel binding: %w", ErrServerDoesSupportChannelBinding) case 'p': - p.xerrorf("gs2 header with p: %w", ErrChannelBindingNotSupported) + // Use channel binding. + // It seems a cyrus-sasl client tells a server it is using the bare (non-PLUS) + // scram authentication mechanism, but then does use channel binding. It seems to + // use the server announcement of the plus variant only to learn the server + // supports channel binding. + p.xtake("=") + cbname := p.xcbname() + // Assume the channel binding name is case-sensitive, and lower-case as used in + // examples. The ABNF rule accepts both lower and upper case. But the ABNF for + // attribute names also allows that, while the text claims they are case + // sensitive... ../rfc/5802:547 + switch cbname { + case "tls-unique": + if cs == nil { + p.xerrorf("no tls connection: %w", ErrChannelBindingsDontMatch) + } else if cs.Version >= tls.VersionTLS13 { + // ../rfc/9266:122 + p.xerrorf("tls-unique not defined for tls 1.3 and later, use tls-exporter: %w", ErrChannelBindingsDontMatch) + } else if cs.TLSUnique == nil { + // As noted in the crypto/tls documentation. + p.xerrorf("no tls-unique channel binding value for this tls connection, possibly due to missing extended master key support and/or resumed connection: %w", ErrChannelBindingsDontMatch) + } + case "tls-exporter": + if cs == nil { + p.xerrorf("no tls connection: %w", ErrChannelBindingsDontMatch) + } else if cs.Version < tls.VersionTLS13 { + // Using tls-exporter with pre-1.3 TLS would require more precautions. Perhaps later. + // ../rfc/9266:201 + p.xerrorf("tls-exporter with tls before 1.3 not implemented, use tls-unique: %w", ErrChannelBindingsDontMatch) + } + default: + p.xerrorf("unknown parameter p %s: %w", cbname, ErrUnsupportedChannelBindingType) + } + cb, err := channelBindData(cs) + if err != nil { + // We can pass back the error, it should never contain sensitive data, and only + // happen due to incorrect calling or a TLS config that is currently impossible + // (renegotiation enabled). + p.xerrorf("error fetching channel binding data: %v: %w", err, ErrOtherError) + } + server.channelBinding = cb + default: + p.xerrorf("unrecognized gs2 channel bind flag") } p.xtake(",") if !p.take(",") { @@ -150,9 +223,10 @@ func NewServer(h func() hash.Hash, clientFirst []byte) (server *Server, rerr err server.gs2header = p.s[:p.o] server.clientFirstBare = p.s[p.o:] - // ../rfc/5802:945 + // ../rfc/5802:632 + // ../rfc/5802:946 if p.take("m=") { - p.xerrorf("unexpected mandatory extension: %w", ErrExtensionsNotSupported) + p.xerrorf("unexpected mandatory extension: %w", ErrExtensionsNotSupported) // ../rfc/5802:973 } server.Authentication = p.xusername() if norm.NFC.String(server.Authentication) != server.Authentication { @@ -192,8 +266,12 @@ func (s *Server) Finish(clientFinal []byte, saltedPassword []byte) (serverFinal p := newParser(clientFinal) defer p.recover(&rerr) + // If there is any channel binding, and it doesn't match, this may be a + // MitM-attack. If the MitM would replace the channel binding, the signature + // calculated below would not match. cbind := p.xchannelBinding() - if cbind != s.gs2header { + cbindExp := append([]byte(s.gs2header), s.channelBinding...) + if !bytes.Equal(cbind, cbindExp) { return "e=" + string(ErrChannelBindingsDontMatch), ErrChannelBindingsDontMatch } p.xtake(",") @@ -210,21 +288,21 @@ func (s *Server) Finish(clientFinal []byte, saltedPassword []byte) (serverFinal proof := p.xproof() p.xempty() - msg := s.clientFirstBare + "," + s.serverFirst + "," + s.clientFinalWithoutProof + authMsg := s.clientFirstBare + "," + s.serverFirst + "," + s.clientFinalWithoutProof clientKey := hmac0(s.h, saltedPassword, "Client Key") h := s.h() h.Write(clientKey) storedKey := h.Sum(nil) - clientSig := hmac0(s.h, storedKey, msg) + clientSig := hmac0(s.h, storedKey, authMsg) xor(clientSig, clientKey) // Now clientProof. if !bytes.Equal(clientSig, proof) { return "e=" + string(ErrInvalidProof), ErrInvalidProof } serverKey := hmac0(s.h, saltedPassword, "Server Key") - serverSig := hmac0(s.h, serverKey, msg) + serverSig := hmac0(s.h, serverKey, authMsg) return fmt.Sprintf("v=%s", base64.StdEncoding.EncodeToString(serverSig)), nil } @@ -239,7 +317,9 @@ type Client struct { authc string authz string - h func() hash.Hash // sha1.New or sha256.New + h func() hash.Hash // sha1.New or sha256.New + noServerPlus bool // Server did not announce support for PLUS-variant. + cs *tls.ConnectionState // If set, use PLUS-variant. // Messages used in hash calculations. clientFirstBare string @@ -247,31 +327,69 @@ type Client struct { clientFinalWithoutProof string authMessage string - gs2header string - clientNonce string - nonce string // Full client + server nonce. - saltedPassword []byte + gs2header string + clientNonce string + nonce string // Full client + server nonce. + saltedPassword []byte + channelBindData []byte // For PLUS-variant. } // NewClient returns a client for authentication authc, optionally for // authorization with role authz, for the hash (sha1.New or sha256.New). // +// If noServerPlus is true, the client would like to have used the PLUS-variant, +// that binds the authentication attempt to the TLS connection, but the client did +// not see support for the PLUS variant announced by the server. Used during +// negotiation to detect possible MitM attempt. +// +// If cs is not nil, the SCRAM PLUS-variant is negotiated, with channel binding to +// the unique TLS connection, either using "tls-exporter" for TLS 1.3 and later, or +// "tls-unique" otherwise. +// +// If cs is nil, no channel binding is done. If noServerPlus is also false, the +// client is configured to not attempt/"support" the PLUS-variant, ensuring servers +// that do support the PLUS-variant do not abort the connection. +// // The sequence for data and calls on a client: // // - ClientFirst, write result to server. // - Read response from server, feed to ServerFirst, write response to server. // - Read response from server, feed to ServerFinal. -func NewClient(h func() hash.Hash, authc, authz string) *Client { +func NewClient(h func() hash.Hash, authc, authz string, noServerPlus bool, cs *tls.ConnectionState) *Client { authc = norm.NFC.String(authc) authz = norm.NFC.String(authz) - return &Client{authc: authc, authz: authz, h: h} + return &Client{authc: authc, authz: authz, h: h, noServerPlus: noServerPlus, cs: cs} } // ClientFirst returns the first client message to write to the server. // No channel binding is done/supported. // A random nonce is generated. func (c *Client) ClientFirst() (clientFirst string, rerr error) { - c.gs2header = fmt.Sprintf("n,%s,", saslname(c.authz)) + if c.noServerPlus && c.cs != nil { + return "", fmt.Errorf("cannot set both claim channel binding is not supported, and use channel binding") + } + // The first byte of the gs2header indicates if/how channel binding should be used. + // ../rfc/5802:903 + if c.cs != nil { + if c.cs.Version >= tls.VersionTLS13 { + c.gs2header = "p=tls-exporter" + } else { + c.gs2header = "p=tls-unique" + } + cbdata, err := channelBindData(c.cs) + if err != nil { + return "", fmt.Errorf("get channel binding data: %v", err) + } + c.channelBindData = cbdata + } else if c.noServerPlus { + // We support it, but we think server does not. If server does support it, we may + // have been downgraded, and the server will tell us. + c.gs2header = "y" + } else { + // We don't want to do channel binding. + c.gs2header = "n" + } + c.gs2header += fmt.Sprintf(",%s,", saslname(c.authz)) if c.clientNonce == "" { c.clientNonce = base64.StdEncoding.EncodeToString(MakeRandom()) } @@ -288,9 +406,10 @@ func (c *Client) ServerFirst(serverFirst []byte, password string) (clientFinal s p := newParser(serverFirst) defer p.recover(&rerr) + // ../rfc/5802:632 // ../rfc/5802:959 if p.take("m=") { - p.xerrorf("unsupported mandatory extension: %w", ErrExtensionsNotSupported) + p.xerrorf("unsupported mandatory extension: %w", ErrExtensionsNotSupported) // ../rfc/5802:973 } c.nonce = p.xnonce() @@ -317,7 +436,12 @@ func (c *Client) ServerFirst(serverFirst []byte, password string) (clientFinal s return "", fmt.Errorf("%w: too few iterations", ErrUnsafe) } - c.clientFinalWithoutProof = fmt.Sprintf("c=%s,r=%s", base64.StdEncoding.EncodeToString([]byte(c.gs2header)), c.nonce) + // We send our channel binding data if present. If the server has different values, + // we'll get an error. If any MitM would try to modify the channel binding data, + // the server cannot verify our signature and will fail the attempt. + // ../rfc/5802:925 ../rfc/5802:1015 + cbindInput := append([]byte(c.gs2header), c.channelBindData...) + c.clientFinalWithoutProof = fmt.Sprintf("c=%s,r=%s", base64.StdEncoding.EncodeToString(cbindInput), c.nonce) c.authMessage = c.clientFirstBare + "," + c.serverFirst + "," + c.clientFinalWithoutProof diff --git a/scram/scram_test.go b/scram/scram_test.go index 1a315a1..ac7646e 100644 --- a/scram/scram_test.go +++ b/scram/scram_test.go @@ -1,11 +1,19 @@ package scram import ( + "crypto/ed25519" + cryptorand "crypto/rand" "crypto/sha1" "crypto/sha256" + "crypto/tls" + "crypto/x509" "encoding/base64" "errors" + "hash" + "math/big" + "net" "testing" + "time" ) func base64Decode(s string) []byte { @@ -28,7 +36,7 @@ func TestSCRAMSHA1Server(t *testing.T) { salt := base64Decode("QSXCR+Q6sek8bf92") saltedPassword := SaltPassword(sha1.New, "pencil", salt, 4096) - server, err := NewServer(sha1.New, []byte("n,,n=user,r=fyko+d2lbbFgONRv9qkxdawL")) + server, err := NewServer(sha1.New, []byte("n,,n=user,r=fyko+d2lbbFgONRv9qkxdawL"), nil, false) server.serverNonceOverride = "3rfcNHYJY1ZVvWVs7j" tcheck(t, err, "newserver") resp, err := server.ServerFirst(4096, salt) @@ -48,7 +56,7 @@ func TestSCRAMSHA256Server(t *testing.T) { salt := base64Decode("W22ZaJ0SNY7soEsUEjb6gQ==") saltedPassword := SaltPassword(sha256.New, "pencil", salt, 4096) - server, err := NewServer(sha256.New, []byte("n,,n=user,r=rOprNGfwEbeRWgbNEkqO")) + server, err := NewServer(sha256.New, []byte("n,,n=user,r=rOprNGfwEbeRWgbNEkqO"), nil, false) server.serverNonceOverride = "%hvYDpWUa2RaTCAfuxFIlj)hNlF$k0" tcheck(t, err, "newserver") resp, err := server.ServerFirst(4096, salt) @@ -68,7 +76,7 @@ func TestScramServerBadPassword(t *testing.T) { salt := base64Decode("W22ZaJ0SNY7soEsUEjb6gQ==") saltedPassword := SaltPassword(sha256.New, "marker", salt, 4096) - server, err := NewServer(sha256.New, []byte("n,,n=user,r=rOprNGfwEbeRWgbNEkqO")) + server, err := NewServer(sha256.New, []byte("n,,n=user,r=rOprNGfwEbeRWgbNEkqO"), nil, false) server.serverNonceOverride = "%hvYDpWUa2RaTCAfuxFIlj)hNlF$k0" tcheck(t, err, "newserver") _, err = server.ServerFirst(4096, salt) @@ -84,7 +92,7 @@ func TestScramServerBadIterations(t *testing.T) { salt := base64Decode("W22ZaJ0SNY7soEsUEjb6gQ==") saltedPassword := SaltPassword(sha256.New, "pencil", salt, 2048) - server, err := NewServer(sha256.New, []byte("n,,n=user,r=rOprNGfwEbeRWgbNEkqO")) + server, err := NewServer(sha256.New, []byte("n,,n=user,r=rOprNGfwEbeRWgbNEkqO"), nil, false) server.serverNonceOverride = "%hvYDpWUa2RaTCAfuxFIlj)hNlF$k0" tcheck(t, err, "newserver") _, err = server.ServerFirst(4096, salt) @@ -100,7 +108,7 @@ func TestScramServerBad(t *testing.T) { salt := base64Decode("W22ZaJ0SNY7soEsUEjb6gQ==") saltedPassword := SaltPassword(sha256.New, "pencil", salt, 4096) - server, err := NewServer(sha256.New, []byte("n,,n=user,r=rOprNGfwEbeRWgbNEkqO")) + server, err := NewServer(sha256.New, []byte("n,,n=user,r=rOprNGfwEbeRWgbNEkqO"), nil, false) tcheck(t, err, "newserver") _, err = server.ServerFirst(4096, salt) tcheck(t, err, "server first") @@ -111,7 +119,7 @@ func TestScramServerBad(t *testing.T) { } func TestScramClient(t *testing.T) { - c := NewClient(sha256.New, "user", "") + c := NewClient(sha256.New, "user", "", false, nil) c.clientNonce = "rOprNGfwEbeRWgbNEkqO" clientFirst, err := c.ClientFirst() tcheck(t, err, "ClientFirst") @@ -128,7 +136,7 @@ func TestScramClient(t *testing.T) { } func TestScram(t *testing.T) { - run := func(expErr error, username, authzid, password string, iterations int, clientNonce, serverNonce string) { + runHash := func(h func() hash.Hash, expErr error, username, authzid, password string, iterations int, clientNonce, serverNonce string, noServerPlus bool, clientcs, servercs *tls.ConnectionState) { t.Helper() defer func() { @@ -151,14 +159,14 @@ func TestScram(t *testing.T) { } salt := MakeRandom() - saltedPassword := SaltPassword(sha256.New, password, salt, iterations) + saltedPassword := SaltPassword(h, password, salt, iterations) - client := NewClient(sha256.New, username, "") + client := NewClient(h, username, "", noServerPlus, clientcs) client.clientNonce = clientNonce clientFirst, err := client.ClientFirst() xerr(err, "client.ClientFirst") - server, err := NewServer(sha256.New, []byte(clientFirst)) + server, err := NewServer(h, []byte(clientFirst), servercs, servercs != nil) xerr(err, "NewServer") server.serverNonceOverride = serverNonce @@ -179,13 +187,114 @@ func TestScram(t *testing.T) { } } + makeState := func(maxTLSVersion uint16) (tls.ConnectionState, tls.ConnectionState) { + client, server := net.Pipe() + defer client.Close() + defer server.Close() + tlsClient := tls.Client(client, &tls.Config{ + InsecureSkipVerify: true, + MaxVersion: maxTLSVersion, + }) + tlsServer := tls.Server(server, &tls.Config{ + Certificates: []tls.Certificate{fakeCert(t, "mox.example", false)}, + MaxVersion: maxTLSVersion, + }) + errc := make(chan error, 1) + go func() { + errc <- tlsServer.Handshake() + }() + err := tlsClient.Handshake() + tcheck(t, err, "tls handshake") + err = <-errc + tcheck(t, err, "server tls handshake") + clientcs := tlsClient.ConnectionState() + servercs := tlsServer.ConnectionState() + + return clientcs, servercs + } + + runPlus := func(maxTLSVersion uint16, expErr error, username, authzid, password string, iterations int, clientNonce, serverNonce string) { + t.Helper() + + // PLUS variants. + clientcs, servercs := makeState(maxTLSVersion) + runHash(sha1.New, expErr, username, authzid, password, iterations, clientNonce, serverNonce, false, &clientcs, &servercs) + runHash(sha256.New, expErr, username, authzid, password, iterations, clientNonce, serverNonce, false, &clientcs, &servercs) + } + + run := func(expErr error, username, authzid, password string, iterations int, clientNonce, serverNonce string) { + t.Helper() + + // Bare variants + runHash(sha1.New, expErr, username, authzid, password, iterations, clientNonce, serverNonce, false, nil, nil) + runHash(sha256.New, expErr, username, authzid, password, iterations, clientNonce, serverNonce, false, nil, nil) + + // Check with both TLS 1.2 for "tls-unique", and latest TLS for "tls-exporter". + runPlus(tls.VersionTLS12, expErr, username, authzid, password, iterations, clientNonce, serverNonce) + runPlus(0, expErr, username, authzid, password, iterations, clientNonce, serverNonce) + } + run(nil, "user", "", "pencil", 4096, "", "") run(nil, "mjl@mox.example", "", "testtest", 4096, "", "") run(nil, "mjl@mox.example", "", "short", 4096, "", "") run(nil, "mjl@mox.example", "", "short", 2048, "", "") run(nil, "mjl@mox.example", "mjl@mox.example", "testtest", 4096, "", "") run(nil, "mjl@mox.example", "other@mox.example", "testtest", 4096, "", "") + run(nil, "mjl@mox.example", "other@mox.example", "testtest", 4096, "", "") run(ErrUnsafe, "user", "", "pencil", 1, "", "") // Few iterations. run(ErrUnsafe, "user", "", "pencil", 2048, "short", "") // Short client nonce. run(ErrUnsafe, "user", "", "pencil", 2048, "test1234", "test") // Server added too few random data. + + // Test mechanism downgrade attacks are detected. + runHash(sha1.New, ErrServerDoesSupportChannelBinding, "user", "", "pencil", 4096, "", "", true, nil, nil) + runHash(sha256.New, ErrServerDoesSupportChannelBinding, "user", "", "pencil", 4096, "", "", true, nil, nil) + + // Test channel binding, detecting MitM attacks. + runChannelBind := func(maxTLSVersion uint16) { + t.Helper() + + clientcs0, _ := makeState(maxTLSVersion) + _, servercs1 := makeState(maxTLSVersion) + runHash(sha1.New, ErrChannelBindingsDontMatch, "user", "", "pencil", 4096, "", "", false, &clientcs0, &servercs1) + runHash(sha256.New, ErrChannelBindingsDontMatch, "user", "", "pencil", 4096, "", "", false, &clientcs0, &servercs1) + + // Client thinks it is on a TLS connection and server is not. + runHash(sha1.New, ErrChannelBindingsDontMatch, "user", "", "pencil", 4096, "", "", false, &clientcs0, nil) + runHash(sha256.New, ErrChannelBindingsDontMatch, "user", "", "pencil", 4096, "", "", false, &clientcs0, nil) + } + + runChannelBind(0) + runChannelBind(tls.VersionTLS12) +} + +// Just a cert that appears valid. +func fakeCert(t *testing.T, name string, expired bool) tls.Certificate { + notAfter := time.Now() + if expired { + notAfter = notAfter.Add(-time.Hour) + } else { + notAfter = notAfter.Add(time.Hour) + } + + privKey := ed25519.NewKeyFromSeed(make([]byte, ed25519.SeedSize)) // Fake key, don't use this for real! + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), // Required field... + DNSNames: []string{name}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: notAfter, + } + localCertBuf, err := x509.CreateCertificate(cryptorand.Reader, template, template, privKey.Public(), privKey) + if err != nil { + t.Fatalf("making certificate: %s", err) + } + cert, err := x509.ParseCertificate(localCertBuf) + if err != nil { + t.Fatalf("parsing generated certificate: %s", err) + } + c := tls.Certificate{ + Certificate: [][]byte{localCertBuf}, + PrivateKey: privKey, + Leaf: cert, + } + return c } diff --git a/sendmail.go b/sendmail.go index f27df75..5148256 100644 --- a/sendmail.go +++ b/sendmail.go @@ -3,6 +3,7 @@ package main import ( "bufio" "context" + "crypto/tls" "errors" "fmt" "io" @@ -14,6 +15,8 @@ import ( "strings" "time" + "golang.org/x/exp/slices" + "github.com/mjl-/sconf" "github.com/mjl-/mox/dns" @@ -30,8 +33,8 @@ var submitconf struct { TLS bool `sconf-doc:"Connect with TLS. Usually for connections to port 465."` STARTTLS bool `sconf-doc:"After starting in plain text, use STARTTLS to enable TLS. For port 587 and 25."` Username string `sconf-doc:"For SMTP authentication."` - Password string `sconf-doc:"For password-based SMTP authentication, e.g. SCRAM-SHA-256, SCRAM-SHA-1, CRAM-MD5, PLAIN."` - AuthMethod string `sconf-doc:"If set, only attempt this authentication mechanism. E.g. SCRAM-SHA-256. If not set, any mutually supported algorithm can be used, in order of most to least secure."` + Password string `sconf-doc:"For password-based SMTP authentication, e.g. SCRAM-SHA-256-PLUS, CRAM-MD5, PLAIN."` + AuthMethod string `sconf-doc:"If set, only attempt this authentication mechanism. E.g. SCRAM-SHA-256-PLUS, SCRAM-SHA-256, SCRAM-SHA-1-PLUS, SCRAM-SHA-1, CRAM-MD5, PLAIN. If not set, any mutually supported algorithm can be used, in order listed, from most to least secure. It is recommended to specify the strongest authentication mechanism known to be implemented by the server, to prevent mechanism downgrade attacks."` From string `sconf-doc:"Address for MAIL FROM in SMTP and From-header in message."` DefaultDestination string `sconf:"optional" sconf-doc:"Used when specified address does not contain an @ and may be a local user (eg root)."` RequireTLS RequireTLSOption `sconf:"optional" sconf-doc:"If yes, submission server must implement SMTP REQUIRETLS extension, and connection to submission server must use verified TLS. If no, a TLS-Required header with value no is added to the message, allowing fallback to unverified TLS or plain text delivery despite recpient domain policies. By default, the submission server will follow the policies of the recipient domain (MTA-STS and/or DANE), and apply unverified opportunistic TLS with STARTTLS."` @@ -252,23 +255,45 @@ binary should be setgid that group: conn, err := d.Dial("tcp", addr) xsavecheckf(err, "dial submit server") - var auth []sasl.Client - switch submitconf.AuthMethod { - case "SCRAM-SHA-256": - auth = []sasl.Client{sasl.NewClientSCRAMSHA256(submitconf.Username, submitconf.Password)} - case "SCRAM-SHA-1": - auth = []sasl.Client{sasl.NewClientSCRAMSHA1(submitconf.Username, submitconf.Password)} - case "CRAM-MD5": - auth = []sasl.Client{sasl.NewClientCRAMMD5(submitconf.Username, submitconf.Password)} - case "PLAIN": - auth = []sasl.Client{sasl.NewClientPlain(submitconf.Username, submitconf.Password)} - default: - auth = []sasl.Client{ - sasl.NewClientSCRAMSHA256(submitconf.Username, submitconf.Password), - sasl.NewClientSCRAMSHA1(submitconf.Username, submitconf.Password), - sasl.NewClientCRAMMD5(submitconf.Username, submitconf.Password), - sasl.NewClientPlain(submitconf.Username, submitconf.Password), + auth := func(mechanisms []string, cs *tls.ConnectionState) (sasl.Client, error) { + // Check explicitly configured mechanisms. + switch submitconf.AuthMethod { + case "SCRAM-SHA-256-PLUS": + if cs == nil { + return nil, fmt.Errorf("scram plus authentication mechanism requires tls") + } + return sasl.NewClientSCRAMSHA256PLUS(submitconf.Username, submitconf.Password, *cs), nil + case "SCRAM-SHA-256": + return sasl.NewClientSCRAMSHA256(submitconf.Username, submitconf.Password, false), nil + case "SCRAM-SHA-1-PLUS": + if cs == nil { + return nil, fmt.Errorf("scram plus authentication mechanism requires tls") + } + return sasl.NewClientSCRAMSHA1PLUS(submitconf.Username, submitconf.Password, *cs), nil + case "SCRAM-SHA-1": + return sasl.NewClientSCRAMSHA1(submitconf.Username, submitconf.Password, false), nil + case "CRAM-MD5": + return sasl.NewClientCRAMMD5(submitconf.Username, submitconf.Password), nil + case "PLAIN": + return sasl.NewClientPlain(submitconf.Username, submitconf.Password), nil } + + // Try the defaults, from more to less secure. + if cs != nil && slices.Contains(mechanisms, "SCRAM-SHA-256-PLUS") { + return sasl.NewClientSCRAMSHA256PLUS(submitconf.Username, submitconf.Password, *cs), nil + } else if slices.Contains(mechanisms, "SCRAM-SHA-256") { + return sasl.NewClientSCRAMSHA256(submitconf.Username, submitconf.Password, true), nil + } else if cs != nil && slices.Contains(mechanisms, "SCRAM-SHA-1-PLUS") { + return sasl.NewClientSCRAMSHA1PLUS(submitconf.Username, submitconf.Password, *cs), nil + } else if slices.Contains(mechanisms, "SCRAM-SHA-1") { + return sasl.NewClientSCRAMSHA1(submitconf.Username, submitconf.Password, true), nil + } else if slices.Contains(mechanisms, "CRAM-MD5") { + return sasl.NewClientCRAMMD5(submitconf.Username, submitconf.Password), nil + } else if slices.Contains(mechanisms, "PLAIN") { + return sasl.NewClientPlain(submitconf.Username, submitconf.Password), nil + } + // No mutually supported mechanism. + return nil, nil } ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) diff --git a/smtpclient/client.go b/smtpclient/client.go index 8e6a770..8f5f0de 100644 --- a/smtpclient/client.go +++ b/smtpclient/client.go @@ -192,10 +192,17 @@ func (e Error) Error() string { // Opts influence behaviour of Client. type Opts struct { - // If auth is non-empty, authentication will be done with the first algorithm - // supported by the server. If none of the algorithms are supported, an error is - // returned. - Auth []sasl.Client + // If auth is non-nil, authentication will be done with the returned sasl client. + // The function should select the preferred mechanism. Mechanisms are in upper + // case. + // + // The TLS connection state can be used for the SCRAM PLUS mechanisms, binding the + // authentication exchange to a TLS connection. It is only present for TLS + // connections. + // + // If no mechanism is supported, a nil client and nil error can be returned, and + // the connection will fail. + Auth func(mechanisms []string, cs *tls.ConnectionState) (sasl.Client, error) DANERecords []adns.TLSA // If not nil, DANE records to verify. DANEMoreHostnames []dns.Domain // For use with DANE, where additional certificate host names are allowed. @@ -666,7 +673,7 @@ func (c *Client) recover(rerr *error) { *rerr = cerr } -func (c *Client) hello(ctx context.Context, tlsMode TLSMode, ehloHostname dns.Domain, auth []sasl.Client) (rerr error) { +func (c *Client) hello(ctx context.Context, tlsMode TLSMode, ehloHostname dns.Domain, auth func(mechanisms []string, cs *tls.ConnectionState) (sasl.Client, error)) (rerr error) { defer c.recover(&rerr) // perform EHLO handshake, falling back to HELO if server does not appear to @@ -808,7 +815,7 @@ func (c *Client) hello(ctx context.Context, tlsMode TLSMode, ehloHostname dns.Do c.tlsResultAddFailureDetails(0, 0, c.tlsrptFailureDetails(tlsrpt.ResultSTARTTLSNotSupported, "")) } - if len(auth) > 0 { + if auth != nil { return c.auth(auth) } return @@ -859,27 +866,23 @@ func (c *Client) tlsResultAddFailureDetails(success, failure int64, fds ...tlsrp } // ../rfc/4954:139 -func (c *Client) auth(auth []sasl.Client) (rerr error) { +func (c *Client) auth(auth func(mechanisms []string, cs *tls.ConnectionState) (sasl.Client, error)) (rerr error) { defer c.recover(&rerr) c.cmds[0] = "auth" c.cmdStart = time.Now() - var a sasl.Client - var name string - var cleartextCreds bool - for _, x := range auth { - name, cleartextCreds = x.Info() - for _, s := range c.extAuthMechanisms { - if s == name { - a = x - break - } - } + mechanisms := make([]string, len(c.extAuthMechanisms)) + for i, m := range c.extAuthMechanisms { + mechanisms[i] = strings.ToUpper(m) } - if a == nil { + a, err := auth(mechanisms, c.TLSConnectionState()) + if err != nil { + c.xerrorf(true, 0, "", "", "get authentication mechanism: %s, server supports %s", err, strings.Join(c.extAuthMechanisms, ", ")) + } else if a == nil { c.xerrorf(true, 0, "", "", "no matching authentication mechanisms, server supports %s", strings.Join(c.extAuthMechanisms, ", ")) } + name, cleartextCreds := a.Info() abort := func() (int, string, string) { // Abort authentication. ../rfc/4954:193 diff --git a/smtpclient/client_test.go b/smtpclient/client_test.go index 0728e48..5105449 100644 --- a/smtpclient/client_test.go +++ b/smtpclient/client_test.go @@ -69,7 +69,7 @@ func TestClient(t *testing.T) { Certificates: []tls.Certificate{cert}, } - test := func(msg string, opts options, auths []sasl.Client, expClientErr, expDeliverErr, expServerErr error) { + test := func(msg string, opts options, auth func(l []string, cs *tls.ConnectionState) (sasl.Client, error), expClientErr, expDeliverErr, expServerErr error) { t.Helper() if opts.tlsMode == "" { @@ -193,17 +193,32 @@ func TestClient(t *testing.T) { writeline("334 " + base64.StdEncoding.EncodeToString([]byte("<123.1234@host>"))) readline("") // Proof writeline("235 2.7.0 auth ok") - case "SCRAM-SHA-1", "SCRAM-SHA-256": + case "SCRAM-SHA-256-PLUS", "SCRAM-SHA-256", "SCRAM-SHA-1-PLUS", "SCRAM-SHA-1": // Cannot fake/hardcode scram interactions. var h func() hash.Hash salt := scram.MakeRandom() var iterations int - if t[0] == "SCRAM-SHA-1" { + switch t[0] { + case "SCRAM-SHA-1-PLUS", "SCRAM-SHA-1": h = sha1.New iterations = 2 * 4096 - } else { + case "SCRAM-SHA-256-PLUS", "SCRAM-SHA-256": h = sha256.New iterations = 4096 + default: + panic("missing case for scram") + } + var cs *tls.ConnectionState + if strings.HasSuffix(t[0], "-PLUS") { + if !haveTLS { + writeline("501 scram plus without tls not possible") + readline("QUIT") + writeline("221 ok") + result <- nil + return + } + xcs := serverConn.(*tls.Conn).ConnectionState() + cs = &xcs } saltedPassword := scram.SaltPassword(h, "test", salt, iterations) @@ -211,7 +226,7 @@ func TestClient(t *testing.T) { if err != nil { fail("bad base64: %w", err) } - s, err := scram.NewServer(h, clientFirst) + s, err := scram.NewServer(h, clientFirst, cs, cs != nil) if err != nil { fail("scram new server: %w", err) } @@ -283,7 +298,7 @@ func TestClient(t *testing.T) { result <- err panic("stop") } - c, err := New(ctx, log.Logger, clientConn, opts.tlsMode, opts.tlsPKIX, localhost, opts.tlsHostname, Opts{Auth: auths, RootCAs: opts.roots}) + client, err := New(ctx, log.Logger, clientConn, opts.tlsMode, opts.tlsPKIX, localhost, opts.tlsHostname, Opts{Auth: auth, RootCAs: opts.roots}) if (err == nil) != (expClientErr == nil) || err != nil && !errors.As(err, reflect.New(reflect.ValueOf(expClientErr).Type()).Interface()) && !errors.Is(err, expClientErr) { fail("new client: got err %v, expected %#v", err, expClientErr) } @@ -291,21 +306,21 @@ func TestClient(t *testing.T) { result <- nil return } - err = c.Deliver(ctx, "postmaster@mox.example", "mjl@mox.example", int64(len(msg)), strings.NewReader(msg), opts.need8bitmime, opts.needsmtputf8, opts.needsrequiretls) + err = client.Deliver(ctx, "postmaster@mox.example", "mjl@mox.example", int64(len(msg)), strings.NewReader(msg), opts.need8bitmime, opts.needsmtputf8, opts.needsrequiretls) if (err == nil) != (expDeliverErr == nil) || err != nil && !errors.Is(err, expDeliverErr) { fail("first deliver: got err %v, expected %v", err, expDeliverErr) } if err == nil { - err = c.Reset() + err = client.Reset() if err != nil { fail("reset: %v", err) } - err = c.Deliver(ctx, "postmaster@mox.example", "mjl@mox.example", int64(len(msg)), strings.NewReader(msg), opts.need8bitmime, opts.needsmtputf8, opts.needsrequiretls) + err = client.Deliver(ctx, "postmaster@mox.example", "mjl@mox.example", int64(len(msg)), strings.NewReader(msg), opts.need8bitmime, opts.needsmtputf8, opts.needsrequiretls) if (err == nil) != (expDeliverErr == nil) || err != nil && !errors.Is(err, expDeliverErr) { fail("second deliver: got err %v, expected %v", err, expDeliverErr) } } - err = c.Close() + err = client.Close() if err != nil { fail("close client: %v", err) } @@ -357,11 +372,39 @@ test test(msg, options{ehlo: true, smtputf8: false, needsmtputf8: true, nodeliver: true}, nil, nil, ErrSMTPUTF8Unsupported, nil) test(msg, options{ehlo: true, starttls: true, tlsMode: TLSRequiredStartTLS, tlsPKIX: true, tlsHostname: dns.Domain{ASCII: "mismatch.example"}, nodeliver: true}, nil, ErrTLS, nil, &net.OpError{}) // Server TLS handshake is a net.OpError with "remote error" as text. test(msg, options{ehlo: true, maxSize: len(msg) - 1, nodeliver: true}, nil, nil, ErrSize, nil) - test(msg, options{ehlo: true, auths: []string{"PLAIN"}}, []sasl.Client{sasl.NewClientPlain("test", "test")}, nil, nil, nil) - test(msg, options{ehlo: true, auths: []string{"CRAM-MD5"}}, []sasl.Client{sasl.NewClientCRAMMD5("test", "test")}, nil, nil, nil) - test(msg, options{ehlo: true, auths: []string{"SCRAM-SHA-1"}}, []sasl.Client{sasl.NewClientSCRAMSHA1("test", "test")}, nil, nil, nil) - test(msg, options{ehlo: true, auths: []string{"SCRAM-SHA-256"}}, []sasl.Client{sasl.NewClientSCRAMSHA256("test", "test")}, nil, nil, nil) + + authPlain := func(l []string, cs *tls.ConnectionState) (sasl.Client, error) { + return sasl.NewClientPlain("test", "test"), nil + } + test(msg, options{ehlo: true, auths: []string{"PLAIN"}}, authPlain, nil, nil, nil) + + authCRAMMD5 := func(l []string, cs *tls.ConnectionState) (sasl.Client, error) { + return sasl.NewClientCRAMMD5("test", "test"), nil + } + test(msg, options{ehlo: true, auths: []string{"CRAM-MD5"}}, authCRAMMD5, nil, nil, nil) + // todo: add tests for failing authentication, also at various stages in SCRAM + + authSCRAMSHA1 := func(l []string, cs *tls.ConnectionState) (sasl.Client, error) { + return sasl.NewClientSCRAMSHA1("test", "test", false), nil + } + test(msg, options{ehlo: true, auths: []string{"SCRAM-SHA-1"}}, authSCRAMSHA1, nil, nil, nil) + + authSCRAMSHA1PLUS := func(l []string, cs *tls.ConnectionState) (sasl.Client, error) { + return sasl.NewClientSCRAMSHA1PLUS("test", "test", *cs), nil + } + test(msg, options{ehlo: true, starttls: true, auths: []string{"SCRAM-SHA-1-PLUS"}}, authSCRAMSHA1PLUS, nil, nil, nil) + + authSCRAMSHA256 := func(l []string, cs *tls.ConnectionState) (sasl.Client, error) { + return sasl.NewClientSCRAMSHA256("test", "test", false), nil + } + test(msg, options{ehlo: true, auths: []string{"SCRAM-SHA-256"}}, authSCRAMSHA256, nil, nil, nil) + + authSCRAMSHA256PLUS := func(l []string, cs *tls.ConnectionState) (sasl.Client, error) { + return sasl.NewClientSCRAMSHA256PLUS("test", "test", *cs), nil + } + test(msg, options{ehlo: true, starttls: true, auths: []string{"SCRAM-SHA-256-PLUS"}}, authSCRAMSHA256PLUS, nil, nil, nil) + test(msg, options{ehlo: true, requiretls: false, needsrequiretls: true, nodeliver: true}, nil, nil, ErrRequireTLSUnsupported, nil) // Set an expired certificate. For non-strict TLS, we should still accept it. diff --git a/smtpclient/examples_test.go b/smtpclient/examples_test.go index 1f159db..fdeab89 100644 --- a/smtpclient/examples_test.go +++ b/smtpclient/examples_test.go @@ -2,10 +2,12 @@ package smtpclient_test import ( "context" + "crypto/tls" "log" "net" "strings" + "golang.org/x/exp/slices" "golang.org/x/exp/slog" "github.com/mjl-/mox/dns" @@ -29,11 +31,31 @@ func ExampleClient() { ctx := context.Background() tlsVerifyPKIX := true opts := smtpclient.Opts{ - Auth: []sasl.Client{ + Auth: func(mechanisms []string, cs *tls.ConnectionState) (sasl.Client, error) { + // If the server is known to support a SCRAM PLUS variant, you should only use + // that, detecting and preventing authentication mechanism downgrade attacks + // through TLS channel binding. + username := "mjl" + password := "test1234" + // Prefer strongest authentication mechanism, allow up to older CRAM-MD5. - sasl.NewClientSCRAMSHA256("mjl", "test1234"), - sasl.NewClientSCRAMSHA1("mjl", "test1234"), - sasl.NewClientCRAMMD5("mjl", "test1234"), + if cs != nil && slices.Contains(mechanisms, "SCRAM-SHA-256-PLUS") { + return sasl.NewClientSCRAMSHA256PLUS(username, password, *cs), nil + } + if slices.Contains(mechanisms, "SCRAM-SHA-256") { + return sasl.NewClientSCRAMSHA256(username, password, true), nil + } + if cs != nil && slices.Contains(mechanisms, "SCRAM-SHA-1-PLUS") { + return sasl.NewClientSCRAMSHA1PLUS(username, password, *cs), nil + } + if slices.Contains(mechanisms, "SCRAM-SHA-1") { + return sasl.NewClientSCRAMSHA1(username, password, true), nil + } + if slices.Contains(mechanisms, "CRAM-MD5") { + return sasl.NewClientCRAMMD5(username, password), nil + } + // No mutually supported mechanism found, connection will fail. + return nil, nil }, } localname := dns.Domain{ASCII: "localhost"} diff --git a/smtpserver/server.go b/smtpserver/server.go index e9be0f6..890b38b 100644 --- a/smtpserver/server.go +++ b/smtpserver/server.go @@ -869,7 +869,12 @@ func (c *conn) cmdHello(p *parser, ehlo bool) { if c.submission { // ../rfc/4954:123 if c.tls || !c.requireTLSForAuth { - c.bwritelinef("250-AUTH SCRAM-SHA-256 SCRAM-SHA-1 CRAM-MD5 PLAIN LOGIN") + // We always mention the SCRAM PLUS variants, even if TLS is not active: It is a + // hint to the client that a TLS connection can use TLS channel binding during + // authentication. The client should select the bare variant when TLS isn't + // present, and also not indicate the server supports the PLUS variant in that + // case, or it would trigger the mechanism downgrade detection. + c.bwritelinef("250-AUTH SCRAM-SHA-256-PLUS SCRAM-SHA-256 SCRAM-SHA-1-PLUS SCRAM-SHA-1 CRAM-MD5 PLAIN LOGIN") } else { c.bwritelinef("250-AUTH ") } @@ -1190,22 +1195,35 @@ func (c *conn) cmdAuth(p *parser) { // ../rfc/4954:276 c.writecodeline(smtp.C235AuthSuccess, smtp.SePol7Other0, "nice", nil) - case "SCRAM-SHA-1", "SCRAM-SHA-256": + case "SCRAM-SHA-256-PLUS", "SCRAM-SHA-256", "SCRAM-SHA-1-PLUS", "SCRAM-SHA-1": // todo: improve handling of errors during scram. e.g. invalid parameters. should we abort the imap command, or continue until the end and respond with a scram-level error? // todo: use single implementation between ../imapserver/server.go and ../smtpserver/server.go - authVariant = strings.ToLower(mech) - var h func() hash.Hash - if authVariant == "scram-sha-1" { - h = sha1.New - } else { - h = sha256.New - } - // Passwords cannot be retrieved or replayed from the trace. + authVariant = strings.ToLower(mech) + var h func() hash.Hash + switch authVariant { + case "scram-sha-1", "scram-sha-1-plus": + h = sha1.New + case "scram-sha-256", "scram-sha-256-plus": + h = sha256.New + default: + xsmtpServerErrorf(codes{smtp.C554TransactionFailed, smtp.SeSys3Other0}, "missing scram auth method case") + } + + var cs *tls.ConnectionState + channelBindingRequired := strings.HasSuffix(authVariant, "-plus") + if channelBindingRequired && !c.tls { + // ../rfc/4954:630 + xsmtpUserErrorf(smtp.C538EncReqForAuth, smtp.SePol7EncReqForAuth11, "scram plus mechanism requires tls connection") + } + if c.tls { + xcs := c.conn.(*tls.Conn).ConnectionState() + cs = &xcs + } c0 := xreadInitial() - ss, err := scram.NewServer(h, c0) + ss, err := scram.NewServer(h, c0, cs, channelBindingRequired) xcheckf(err, "starting scram") c.log.Debug("scram auth", slog.String("authentication", ss.Authentication)) acc, _, err := store.OpenEmail(c.log, ss.Authentication) @@ -1229,10 +1247,13 @@ func (c *conn) cmdAuth(p *parser) { acc.WithRLock(func() { err := acc.DB.Read(context.TODO(), func(tx *bstore.Tx) error { password, err := bstore.QueryTx[store.Password](tx).Get() - if authVariant == "scram-sha-1" { + switch authVariant { + case "scram-sha-1", "scram-sha-1-plus": xscram = password.SCRAMSHA1 - } else { + case "scram-sha-256", "scram-sha-256-plus": xscram = password.SCRAMSHA256 + default: + xsmtpServerErrorf(codes{smtp.C554TransactionFailed, smtp.SeSys3Other0}, "missing scram auth credentials case") } if err == bstore.ErrAbsent || err == nil && (len(xscram.Salt) == 0 || xscram.Iterations == 0 || len(xscram.SaltedPassword) == 0) { c.log.Info("scram auth attempt without derived secrets set, save password again to store secrets", slog.String("address", ss.Authentication)) diff --git a/smtpserver/server_test.go b/smtpserver/server_test.go index af3c08c..e1e79b2 100644 --- a/smtpserver/server_test.go +++ b/smtpserver/server_test.go @@ -89,7 +89,7 @@ type testserver struct { comm *store.Comm cid int64 resolver dns.Resolver - auth []sasl.Client + auth func(mechanisms []string, cs *tls.ConnectionState) (sasl.Client, error) user, pass string submission bool requiretls bool @@ -159,11 +159,11 @@ func (ts *testserver) run(fn func(helloErr error, client *smtpclient.Client)) { close(serverdone) }() - var auth []sasl.Client - if len(ts.auth) > 0 { - auth = ts.auth - } else if ts.user != "" { - auth = append(auth, sasl.NewClientPlain(ts.user, ts.pass)) + auth := ts.auth + if auth == nil && ts.user != "" { + auth = func(mechanisms []string, cs *tls.ConnectionState) (sasl.Client, error) { + return sasl.NewClientPlain(ts.user, ts.pass), nil + } } ourHostname := mox.Conf.Static.HostnameDomain @@ -234,10 +234,12 @@ func TestSubmission(t *testing.T) { } mox.Conf.Dynamic.Domains["mox.example"] = dom - testAuth := func(authfn func(user, pass string) sasl.Client, user, pass string, expErr *smtpclient.Error) { + testAuth := func(authfn func(user, pass string, cs *tls.ConnectionState) sasl.Client, user, pass string, expErr *smtpclient.Error) { t.Helper() if authfn != nil { - ts.auth = []sasl.Client{authfn(user, pass)} + ts.auth = func(mechanisms []string, cs *tls.ConnectionState) (sasl.Client, error) { + return authfn(user, pass, cs), nil + } } else { ts.auth = nil } @@ -258,12 +260,22 @@ func TestSubmission(t *testing.T) { ts.submission = true testAuth(nil, "", "", &smtpclient.Error{Permanent: true, Code: smtp.C530SecurityRequired, Secode: smtp.SePol7Other0}) - authfns := []func(user, pass string) sasl.Client{ - sasl.NewClientPlain, - sasl.NewClientLogin, - sasl.NewClientCRAMMD5, - sasl.NewClientSCRAMSHA1, - sasl.NewClientSCRAMSHA256, + authfns := []func(user, pass string, cs *tls.ConnectionState) sasl.Client{ + func(user, pass string, cs *tls.ConnectionState) sasl.Client { return sasl.NewClientPlain(user, pass) }, + func(user, pass string, cs *tls.ConnectionState) sasl.Client { return sasl.NewClientLogin(user, pass) }, + func(user, pass string, cs *tls.ConnectionState) sasl.Client { return sasl.NewClientCRAMMD5(user, pass) }, + func(user, pass string, cs *tls.ConnectionState) sasl.Client { + return sasl.NewClientSCRAMSHA1(user, pass, false) + }, + func(user, pass string, cs *tls.ConnectionState) sasl.Client { + return sasl.NewClientSCRAMSHA256(user, pass, false) + }, + func(user, pass string, cs *tls.ConnectionState) sasl.Client { + return sasl.NewClientSCRAMSHA1PLUS(user, pass, *cs) + }, + func(user, pass string, cs *tls.ConnectionState) sasl.Client { + return sasl.NewClientSCRAMSHA256PLUS(user, pass, *cs) + }, } for _, fn := range authfns { testAuth(fn, "mjl@mox.example", "test", &smtpclient.Error{Secode: smtp.SePol7AuthBadCreds8}) // Bad (short) password.