diff --git a/config/config.go b/config/config.go index 8d39e31..5e90a2f 100644 --- a/config/config.go +++ b/config/config.go @@ -544,6 +544,7 @@ type TLS struct { KeyCerts []KeyCert `sconf:"optional" sconf-doc:"Keys and certificates to use for this listener. The files are opened by the privileged root process and passed to the unprivileged mox process, so no special permissions are required on the files. If the private key will not be replaced when refreshing certificates, also consider adding the private key to HostPrivateKeyFiles and configuring DANE TLSA DNS records."` MinVersion string `sconf:"optional" sconf-doc:"Minimum TLS version. Default: TLSv1.2."` HostPrivateKeyFiles []string `sconf:"optional" sconf-doc:"Private keys used for ACME certificates. Specified explicitly so DANE TLSA DNS records can be generated, even before the certificates are requested. DANE is a mechanism to authenticate remote TLS certificates based on a public key or certificate specified in DNS, protected with DNSSEC. DANE is opportunistic and attempted when delivering SMTP with STARTTLS. The private key files must be in PEM format. PKCS8 is recommended, but PKCS1 and EC private keys are recognized as well. Only RSA 2048 bit and ECDSA P-256 keys are currently used. The first of each is used when requesting new certificates through ACME."` + ClientAuthDisabled bool `sconf:"optional" sconf-doc:"Disable TLS client authentication with certificates/keys, preventing the TLS server from requesting a TLS certificate from clients. Useful for working around clients that don't handle TLS client authentication well."` Config *tls.Config `sconf:"-" json:"-"` // TLS config for non-ACME-verification connections, i.e. SMTP and IMAP, and not port 443. Connections without SNI will use a certificate for the hostname of the listener, connections with an SNI hostname that isn't allowed will be rejected. ConfigFallback *tls.Config `sconf:"-" json:"-"` // Like Config, but uses the certificate for the listener hostname when the requested SNI hostname is not allowed, instead of causing the connection to fail. diff --git a/config/doc.go b/config/doc.go index 6760419..45c7f88 100644 --- a/config/doc.go +++ b/config/doc.go @@ -217,6 +217,11 @@ See https://pkg.go.dev/github.com/mjl-/sconf for details. HostPrivateKeyFiles: - + # Disable TLS client authentication with certificates/keys, preventing the TLS + # server from requesting a TLS certificate from clients. Useful for working around + # clients that don't handle TLS client authentication well. (optional) + ClientAuthDisabled: false + # Maximum size in bytes for incoming and outgoing messages. Default is 100MB. # (optional) SMTPMaxMessageSize: 0 diff --git a/imapserver/authenticate_test.go b/imapserver/authenticate_test.go index e73d504..e75041a 100644 --- a/imapserver/authenticate_test.go +++ b/imapserver/authenticate_test.go @@ -362,7 +362,7 @@ func TestAuthenticateTLSClientCert(t *testing.T) { cid := connCounter go func() { defer serverConn.Close() - serve("test", cid, &serverConfig, serverConn, true, false, false, "") + serve("test", cid, &serverConfig, serverConn, true, false, false, false, "") close(done) }() diff --git a/imapserver/fuzz_test.go b/imapserver/fuzz_test.go index 271964d..2b1548f 100644 --- a/imapserver/fuzz_test.go +++ b/imapserver/fuzz_test.go @@ -144,7 +144,7 @@ func FuzzServer(f *testing.F) { err = serverConn.SetDeadline(time.Now().Add(time.Second)) flog(err, "set server deadline") - serve("test", cid, nil, serverConn, false, true, false, "") + serve("test", cid, nil, serverConn, false, false, true, false, "") cid++ } diff --git a/imapserver/server.go b/imapserver/server.go index d5455c8..d308204 100644 --- a/imapserver/server.go +++ b/imapserver/server.go @@ -192,9 +192,10 @@ type conn struct { cid int64 state state conn net.Conn - connBroken bool // Once broken, we won't flush any more data. - tls bool // Whether TLS has been initialized. - viaHTTPS bool // Whether this connection came in via HTTPS (using TLS ALPN). + connBroken bool // Once broken, we won't flush any more data. + tls bool // Whether TLS has been initialized. + viaHTTPS bool // Whether this connection came in via HTTPS (using TLS ALPN). + noTLSClientAuth bool br *bufio.Reader // From remote, with TLS unwrapped in case of TLS, and possibly wrapping inflate. tr *moxio.TraceReader // Kept to change trace level when reading/writing cmd/auth/data. line chan lineErr // If set, instead of reading from br, a line is read from this channel. For reading a line in IDLE while also waiting for mailbox/account updates. @@ -384,21 +385,23 @@ func Listen() { listener := mox.Conf.Static.Listeners[name] var tlsConfig *tls.Config + var noTLSClientAuth bool if listener.TLS != nil { tlsConfig = listener.TLS.Config + noTLSClientAuth = listener.TLS.ClientAuthDisabled } if listener.IMAP.Enabled { port := config.Port(listener.IMAP.Port, 143) for _, ip := range listener.IPs { - listen1("imap", name, ip, port, tlsConfig, false, listener.IMAP.NoRequireSTARTTLS) + listen1("imap", name, ip, port, tlsConfig, false, noTLSClientAuth, listener.IMAP.NoRequireSTARTTLS) } } if listener.IMAPS.Enabled { port := config.Port(listener.IMAPS.Port, 993) for _, ip := range listener.IPs { - listen1("imaps", name, ip, port, tlsConfig, true, false) + listen1("imaps", name, ip, port, tlsConfig, true, noTLSClientAuth, false) } } } @@ -406,7 +409,7 @@ func Listen() { var servers []func() -func listen1(protocol, listenerName, ip string, port int, tlsConfig *tls.Config, xtls, noRequireSTARTTLS bool) { +func listen1(protocol, listenerName, ip string, port int, tlsConfig *tls.Config, xtls, noTLSClientAuth, noRequireSTARTTLS bool) { log := mlog.New("imapserver", nil) addr := net.JoinHostPort(ip, fmt.Sprintf("%d", port)) if os.Getuid() == 0 { @@ -439,7 +442,7 @@ func listen1(protocol, listenerName, ip string, port int, tlsConfig *tls.Config, } metricIMAPConnection.WithLabelValues(protocol).Inc() - go serve(listenerName, mox.Cid(), tlsConfig, conn, xtls, noRequireSTARTTLS, false, "") + go serve(listenerName, mox.Cid(), tlsConfig, conn, xtls, noTLSClientAuth, noRequireSTARTTLS, false, "") } } @@ -448,11 +451,11 @@ func listen1(protocol, listenerName, ip string, port int, tlsConfig *tls.Config, // ServeTLSConn serves IMAP on a TLS connection. func ServeTLSConn(listenerName string, conn *tls.Conn, tlsConfig *tls.Config) { - serve(listenerName, mox.Cid(), tlsConfig, conn, true, false, true, "") + serve(listenerName, mox.Cid(), tlsConfig, conn, true, true, false, true, "") } func ServeConnPreauth(listenerName string, cid int64, conn net.Conn, preauthAddress string) { - serve(listenerName, cid, nil, conn, false, true, false, preauthAddress) + serve(listenerName, cid, nil, conn, false, true, true, false, preauthAddress) } // Serve starts serving on all listeners, launching a goroutine per listener. @@ -766,7 +769,7 @@ var cleanClose struct{} // Sentinel value for panic/recover indicating clean clo // preauthenticated. // // The connection is closed before returning. -func serve(listenerName string, cid int64, tlsConfig *tls.Config, nc net.Conn, xtls, noRequireSTARTTLS, viaHTTPS bool, preauthAddress string) { +func serve(listenerName string, cid int64, tlsConfig *tls.Config, nc net.Conn, xtls, noTLSClientAuth, noRequireSTARTTLS, viaHTTPS bool, preauthAddress string) { var remoteIP net.IP if a, ok := nc.RemoteAddr().(*net.TCPAddr); ok { remoteIP = a.IP @@ -780,6 +783,7 @@ func serve(listenerName string, cid int64, tlsConfig *tls.Config, nc net.Conn, x conn: nc, tls: xtls, viaHTTPS: viaHTTPS, + noTLSClientAuth: noTLSClientAuth, lastlog: time.Now(), baseTLSConfig: tlsConfig, remoteIP: remoteIP, @@ -990,6 +994,10 @@ func (c *conn) makeTLSConfig() *tls.Config { // config, so they can be used for this connection too. tlsConf := c.baseTLSConfig.Clone() + if c.noTLSClientAuth { + return tlsConf + } + // Allow client certificate authentication, for use with the sasl "external" // authentication mechanism. tlsConf.ClientAuth = tls.RequestClientCert @@ -1089,7 +1097,7 @@ func (c *conn) tlsClientAuthVerifyPeerCertParsed(cert *x509.Certificate) error { if err == bstore.ErrAbsent { c.loginAttempt.Result = store.AuthBadCredentials } - return fmt.Errorf("looking up tls public key with fingerprint %s: %v", fp, err) + return fmt.Errorf("looking up tls public key with fingerprint %s, subject %q, issuer %q: %v", fp, cert.Subject, cert.Issuer, err) } c.loginAttempt.LoginAddress = pubKey.LoginAddress @@ -1152,7 +1160,7 @@ func (c *conn) xtlsHandshakeAndAuthenticate(conn net.Conn) { cancel() cs := tlsConn.ConnectionState() - if cs.DidResume && len(cs.PeerCertificates) > 0 { + if cs.DidResume && len(cs.PeerCertificates) > 0 && !c.noTLSClientAuth { // Verify client after session resumption. err := c.tlsClientAuthVerifyPeerCertParsed(cs.PeerCertificates[0]) if err != nil { @@ -1167,6 +1175,7 @@ func (c *conn) xtlsHandshakeAndAuthenticate(conn net.Conn) { slog.String("ciphersuite", ciphersuite), slog.String("sni", cs.ServerName), slog.Bool("resumed", cs.DidResume), + slog.Bool("notlsclientauth", c.noTLSClientAuth), slog.Int("clientcerts", len(cs.PeerCertificates)), } if c.account != nil { @@ -2390,7 +2399,7 @@ func (c *conn) capabilities() string { } else { caps += " LOGINDISABLED" } - if c.tls && len(c.conn.(*tls.Conn).ConnectionState().PeerCertificates) > 0 && !c.viaHTTPS { + if c.tls && len(c.conn.(*tls.Conn).ConnectionState().PeerCertificates) > 0 && !c.viaHTTPS && !c.noTLSClientAuth { caps += " AUTH=EXTERNAL" } return caps diff --git a/imapserver/server_test.go b/imapserver/server_test.go index c64809d..f326946 100644 --- a/imapserver/server_test.go +++ b/imapserver/server_test.go @@ -556,7 +556,7 @@ func startArgsMore(t *testing.T, uidonly, first, immediateTLS bool, serverConfig cid := connCounter - 1 go func() { const viaHTTPS = false - serve("test", cid, serverConfig, serverConn, immediateTLS, allowLoginWithoutTLS, viaHTTPS, "") + serve("test", cid, serverConfig, serverConn, immediateTLS, false, allowLoginWithoutTLS, viaHTTPS, "") close(done) }() var tc *testconn diff --git a/smtpserver/fuzz_test.go b/smtpserver/fuzz_test.go index b532685..5fc05a9 100644 --- a/smtpserver/fuzz_test.go +++ b/smtpserver/fuzz_test.go @@ -113,7 +113,7 @@ func FuzzServer(f *testing.F) { const viaHTTPS = false err := serverConn.SetDeadline(time.Now().Add(time.Second)) flog(err, "set server deadline") - serve("test", cid, dns.Domain{ASCII: "mox.example"}, nil, serverConn, resolver, submission, false, viaHTTPS, 100<<10, false, false, false, nil, 0) + serve("test", cid, dns.Domain{ASCII: "mox.example"}, nil, serverConn, resolver, submission, false, viaHTTPS, false, 100<<10, false, false, false, nil, 0) cid++ } diff --git a/smtpserver/server.go b/smtpserver/server.go index 721ba6f..66776f6 100644 --- a/smtpserver/server.go +++ b/smtpserver/server.go @@ -206,12 +206,14 @@ func Listen() { listener := mox.Conf.Static.Listeners[name] var tlsConfig, tlsConfigDelivery *tls.Config + var noTLSClientAuth bool if listener.TLS != nil { tlsConfig = listener.TLS.Config // For SMTP delivery, if we get a TLS handshake for an SNI hostname that we don't // allow, we'll fallback to a certificate for the listener hostname instead of // causing the connection to fail. May improve interoperability. tlsConfigDelivery = listener.TLS.ConfigFallback + noTLSClientAuth = listener.TLS.ClientAuthDisabled } maxMsgSize := listener.SMTPMaxMessageSize @@ -234,7 +236,7 @@ func Listen() { // https://github.com/golang/go/issues/70232. tlsConfigDelivery.SessionTicketsDisabled = listener.SMTP.TLSSessionTicketsDisabled == nil || *listener.SMTP.TLSSessionTicketsDisabled } - listen1("smtp", name, ip, port, hostname, tlsConfigDelivery, false, false, maxMsgSize, false, listener.SMTP.RequireSTARTTLS, !listener.SMTP.NoRequireTLS, listener.SMTP.DNSBLZones, firstTimeSenderDelay) + listen1("smtp", name, ip, port, hostname, tlsConfigDelivery, false, false, noTLSClientAuth, maxMsgSize, false, listener.SMTP.RequireSTARTTLS, !listener.SMTP.NoRequireTLS, listener.SMTP.DNSBLZones, firstTimeSenderDelay) } } if listener.Submission.Enabled { @@ -244,7 +246,7 @@ func Listen() { } port := config.Port(listener.Submission.Port, 587) for _, ip := range listener.IPs { - listen1("submission", name, ip, port, hostname, tlsConfig, true, false, maxMsgSize, !listener.Submission.NoRequireSTARTTLS, !listener.Submission.NoRequireSTARTTLS, true, nil, 0) + listen1("submission", name, ip, port, hostname, tlsConfig, true, false, noTLSClientAuth, maxMsgSize, !listener.Submission.NoRequireSTARTTLS, !listener.Submission.NoRequireSTARTTLS, true, nil, 0) } } @@ -255,7 +257,7 @@ func Listen() { } port := config.Port(listener.Submissions.Port, 465) for _, ip := range listener.IPs { - listen1("submissions", name, ip, port, hostname, tlsConfig, true, true, maxMsgSize, true, true, true, nil, 0) + listen1("submissions", name, ip, port, hostname, tlsConfig, true, true, noTLSClientAuth, maxMsgSize, true, true, true, nil, 0) } } } @@ -263,7 +265,7 @@ func Listen() { var servers []func() -func listen1(protocol, name, ip string, port int, hostname dns.Domain, tlsConfig *tls.Config, submission, xtls bool, maxMessageSize int64, requireTLSForAuth, requireTLSForDelivery, requireTLS bool, dnsBLs []dns.Domain, firstTimeSenderDelay time.Duration) { +func listen1(protocol, name, ip string, port int, hostname dns.Domain, tlsConfig *tls.Config, submission, xtls, noTLSClientAuth bool, maxMessageSize int64, requireTLSForAuth, requireTLSForDelivery, requireTLS bool, dnsBLs []dns.Domain, firstTimeSenderDelay time.Duration) { log := mlog.New("smtpserver", nil) addr := net.JoinHostPort(ip, fmt.Sprintf("%d", port)) if os.Getuid() == 0 { @@ -297,7 +299,7 @@ func listen1(protocol, name, ip string, port int, hostname dns.Domain, tlsConfig // Package is set on the resolver by the dkim/spf/dmarc/etc packages. resolver := dns.StrictResolver{Log: log.Logger} - go serve(name, mox.Cid(), hostname, tlsConfig, conn, resolver, submission, xtls, false, maxMessageSize, requireTLSForAuth, requireTLSForDelivery, requireTLS, dnsBLs, firstTimeSenderDelay) + go serve(name, mox.Cid(), hostname, tlsConfig, conn, resolver, submission, xtls, false, noTLSClientAuth, maxMessageSize, requireTLSForAuth, requireTLSForDelivery, requireTLS, dnsBLs, firstTimeSenderDelay) } } @@ -321,10 +323,11 @@ type conn struct { origConn net.Conn conn net.Conn - tls bool - extRequireTLS bool // Whether to announce and allow the REQUIRETLS extension. - viaHTTPS bool // Whether the connection came in via the HTTPS port (using TLS ALPN). - resolver dns.Resolver + tls bool + extRequireTLS bool // Whether to announce and allow the REQUIRETLS extension. + viaHTTPS bool // Whether the connection came in via the HTTPS port (using TLS ALPN). + noTLSClientAuth bool + resolver dns.Resolver // The "x" in the readers and writes indicate Read and Write errors use panic to // propagate the error. xbr *bufio.Reader @@ -435,7 +438,7 @@ func (c *conn) loginAttempt(useTLS bool, authMech string) store.LoginAttempt { // makeTLSConfig makes a new tls config that is bound to the connection for // possible client certificate authentication in case of submission. func (c *conn) makeTLSConfig() *tls.Config { - if !c.submission { + if !c.submission || c.noTLSClientAuth { return c.baseTLSConfig } @@ -540,7 +543,7 @@ func (c *conn) tlsClientAuthVerifyPeerCertParsed(cert *x509.Certificate) error { if err == bstore.ErrAbsent { la.Result = store.AuthBadCredentials } - return fmt.Errorf("looking up tls public key with fingerprint %s: %v", fp, err) + return fmt.Errorf("looking up tls public key with fingerprint %s, subject %q, issuer %q: %v", fp, cert.Subject, cert.Issuer, err) } la.LoginAddress = pubKey.LoginAddress @@ -620,7 +623,7 @@ func (c *conn) xtlsHandshakeAndAuthenticate(conn net.Conn) { cancel() cs := tlsConn.ConnectionState() - if cs.DidResume && len(cs.PeerCertificates) > 0 { + if cs.DidResume && len(cs.PeerCertificates) > 0 && !c.noTLSClientAuth { // Verify client after session resumption. err := c.tlsClientAuthVerifyPeerCertParsed(cs.PeerCertificates[0]) if err != nil { @@ -634,6 +637,7 @@ func (c *conn) xtlsHandshakeAndAuthenticate(conn net.Conn) { slog.String("ciphersuite", ciphersuite), slog.String("sni", cs.ServerName), slog.Bool("resumed", cs.DidResume), + slog.Bool("notlsclientauth", c.noTLSClientAuth), slog.Int("clientcerts", len(cs.PeerCertificates)), } if c.account != nil { @@ -860,10 +864,10 @@ var cleanClose struct{} // Sentinel value for panic/recover indicating clean clo func ServeTLSConn(listenerName string, hostname dns.Domain, conn *tls.Conn, tlsConfig *tls.Config, submission, viaHTTPS bool, maxMsgSize int64, requireTLS bool) { log := mlog.New("smtpserver", nil) resolver := dns.StrictResolver{Log: log.Logger} - serve(listenerName, mox.Cid(), hostname, tlsConfig, conn, resolver, submission, true, viaHTTPS, maxMsgSize, true, true, requireTLS, nil, 0) + serve(listenerName, mox.Cid(), hostname, tlsConfig, conn, resolver, submission, true, viaHTTPS, true, maxMsgSize, true, true, requireTLS, nil, 0) } -func serve(listenerName string, cid int64, hostname dns.Domain, tlsConfig *tls.Config, nc net.Conn, resolver dns.Resolver, submission, xtls, viaHTTPS bool, maxMessageSize int64, requireTLSForAuth, requireTLSForDelivery, requireTLS bool, dnsBLs []dns.Domain, firstTimeSenderDelay time.Duration) { +func serve(listenerName string, cid int64, hostname dns.Domain, tlsConfig *tls.Config, nc net.Conn, resolver dns.Resolver, submission, xtls, viaHTTPS, noTLSClientAuth bool, maxMessageSize int64, requireTLSForAuth, requireTLSForDelivery, requireTLS bool, dnsBLs []dns.Domain, firstTimeSenderDelay time.Duration) { var localIP, remoteIP net.IP if a, ok := nc.LocalAddr().(*net.TCPAddr); ok { localIP = a.IP @@ -890,6 +894,7 @@ func serve(listenerName string, cid int64, hostname dns.Domain, tlsConfig *tls.C submission: submission, tls: xtls, viaHTTPS: viaHTTPS, + noTLSClientAuth: noTLSClientAuth, extRequireTLS: requireTLS, resolver: resolver, lastlog: time.Now(), @@ -1209,7 +1214,7 @@ func (c *conn) cmdHello(p *parser, ehlo bool) { // case, or it would trigger the mechanism downgrade detection. mechs = "SCRAM-SHA-256-PLUS SCRAM-SHA-256 SCRAM-SHA-1-PLUS SCRAM-SHA-1 CRAM-MD5 PLAIN LOGIN" } - if c.tls && len(c.conn.(*tls.Conn).ConnectionState().PeerCertificates) > 0 && !c.viaHTTPS { + if c.tls && len(c.conn.(*tls.Conn).ConnectionState().PeerCertificates) > 0 && !c.viaHTTPS && !c.noTLSClientAuth { mechs = "EXTERNAL " + mechs } c.xbwritelinef("250-AUTH %s", mechs) diff --git a/smtpserver/server_test.go b/smtpserver/server_test.go index 69a2ac2..58fc756 100644 --- a/smtpserver/server_test.go +++ b/smtpserver/server_test.go @@ -257,7 +257,7 @@ func (ts *testserver) runRaw(fn func(clientConn net.Conn)) { defer func() { <-serverdone }() go func() { - serve("test", ts.cid-2, dns.Domain{ASCII: "mox.example"}, ts.serverConfig, serverConn, ts.resolver, ts.submission, ts.immediateTLS, false, 100<<20, false, false, ts.requiretls, ts.dnsbls, 0) + serve("test", ts.cid-2, dns.Domain{ASCII: "mox.example"}, ts.serverConfig, serverConn, ts.resolver, ts.submission, ts.immediateTLS, false, false, 100<<20, false, false, ts.requiretls, ts.dnsbls, 0) close(serverdone) }() @@ -476,7 +476,7 @@ func TestSubmission(t *testing.T) { tlsConfig := &tls.Config{ Certificates: []tls.Certificate{fakeCert(ts.t, false)}, } - serve("test", ts.cid-2, dns.Domain{ASCII: "mox.example"}, tlsConfig, serverConn, ts.resolver, ts.submission, ts.immediateTLS, false, 100<<20, false, false, false, ts.dnsbls, 0) + serve("test", ts.cid-2, dns.Domain{ASCII: "mox.example"}, tlsConfig, serverConn, ts.resolver, ts.submission, ts.immediateTLS, false, false, 100<<20, false, false, false, ts.dnsbls, 0) close(serverdone) }() @@ -1426,7 +1426,7 @@ func TestNonSMTP(t *testing.T) { tlsConfig := &tls.Config{ Certificates: []tls.Certificate{fakeCert(ts.t, false)}, } - serve("test", ts.cid-2, dns.Domain{ASCII: "mox.example"}, tlsConfig, serverConn, ts.resolver, ts.submission, false, false, 100<<20, false, false, false, ts.dnsbls, 0) + serve("test", ts.cid-2, dns.Domain{ASCII: "mox.example"}, tlsConfig, serverConn, ts.resolver, ts.submission, false, false, false, 100<<20, false, false, false, ts.dnsbls, 0) close(serverdone) }()