infrastructure/mox
1
0
Fork 0
mirror of https://github.com/mjl-/mox.git synced 2025-06-27 23:08:14 +03:00
Code Issues Packages Projects Releases Wiki Activity

add config option to disable tls client auth during tls handshakes

Browse Source 
to work around clients, like the gmail smtp client, that tries to
authenticate with a webpki-issued certificate (which we don't know).

i tried specifying a list of accepted (subjects of) CA certs during the
tls handshake (with just 1 entry, with "xmox.nl" as common name), which
clients can use to influence their cert selection.  however, the gmail
smtp client ignores it, so not a solution for the issue where this was
raised. also, specifying a list of accepted certs could cause other
clients to not send their client cert anymore, breaking existing setups.

i also considered only asking for tls client auth when at least one
account has a tls pubkey configured. but decided against it since any
account can add one on their own (without system admin interaction),
changing behaviour of the system and potentially breaking existing
submission/tls configs.

we now also print the "subject" and "issuer" of certs when tls client
auth fails, should be useful for future debugging.

for issue #359
This commit is contained in:
Mechiel Lukkien 2025-06-09 12:26:58 +02:00
parent f5b8c64b84
commit 833a67fe3d
9 changed files with 55 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
config/config.go
View File

@ -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.

5
config/doc.go
View File

@ -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

2
imapserver/authenticate_test.go
View File

@ -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)
}()

2
imapserver/fuzz_test.go
View File

@ -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++
}

35
imapserver/server.go
View File

@ -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

2
imapserver/server_test.go
View File

@ -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

2
smtpserver/fuzz_test.go
View File

@ -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++
}

35
smtpserver/server.go
View File

@ -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)

6
smtpserver/server_test.go
View File

@ -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)
}()