do not log an error for tls requests with ipv6 addresses as sni server name

ip addresses are invalid in server names. for ipv6 addresses, the
autocert.GetCertificate calls would return an error, which we logged, and
increased a metric about. but the alerts for this situation aren't helpful. so
recognize ip addresses early. if we are lenient about unknown server names (for
incoming smtp deliveries), we switch to the fallback hostname, otherwise we
return an error.

this was the error logged:

	l=error m="requesting certificate" err="acme/autocert: server name component count invalid"

for ipv4 addresses, the name wouldn't be in our allowlist and should already
have caused us to switch to the fallback hostname.
This commit is contained in:
Mechiel Lukkien 2025-02-23 10:44:17 +01:00
parent cad585a70e
commit 797c1cf9f0
No known key found for this signature in database
2 changed files with 37 additions and 8 deletions

View File

@ -193,21 +193,41 @@ func Load(name, acmeDir, contactEmail, directoryURL string, eabKeyID string, eab
// optionally falling back to a certificate for fallbackHostname in case SNI is
// absent or for an unknown hostname.
func (m *Manager) loggingGetCertificate(hello *tls.ClientHelloInfo, fallbackHostname dns.Domain, fallbackNoSNI, fallbackUnknownSNI bool) (*tls.Certificate, error) {
log := mlog.New("autotls", nil).WithContext(hello.Context())
log := mlog.New("autotls", nil).WithContext(hello.Context()).With(
slog.Any("localaddr", hello.Conn.LocalAddr()),
slog.Any("supportedprotos", hello.SupportedProtos),
slog.String("servername", hello.ServerName),
)
// If we can't find a certificate (depending on fallback parameters), we return a
// nil certificate and nil error, which crypto/tls turns into a TLS alert
// "unrecognized name", which can be interpreted by clients as a hint that they are
// using the wrong hostname, or a certificate is missing.
// using the wrong hostname, or a certificate is missing. ../rfc/9325:578
// IP addresses for ServerName are not allowed, but happen in practice. If we
// should be lenient (fallbackUnknownSNI), we switch to the fallback hostname,
// otherwise we return an error. We don't want to pass IP addresses to
// GetCertificate because it will return an error for IPv6 addresses.
// ../rfc/6066:367 ../rfc/4366:535
if net.ParseIP(hello.ServerName) != nil {
if fallbackUnknownSNI {
hello.ServerName = fallbackHostname.ASCII
log = log.With(slog.String("servername", hello.ServerName))
} else {
log.Debug("tls request with ip for server name, rejecting")
return nil, fmt.Errorf("invalid ip address for sni server name")
}
}
if hello.ServerName == "" && fallbackNoSNI {
hello.ServerName = fallbackHostname.ASCII
log = log.With(slog.String("servername", hello.ServerName))
}
// Handle missing SNI to prevent logging an error below.
if hello.ServerName == "" {
metricMissingServerName.Inc()
log.Debug("tls request without sni servername, rejecting", slog.Any("localaddr", hello.Conn.LocalAddr()), slog.Any("supportedprotos", hello.SupportedProtos))
log.Debug("tls request without sni server name, rejecting")
return nil, nil
}
@ -215,23 +235,29 @@ func (m *Manager) loggingGetCertificate(hello *tls.ClientHelloInfo, fallbackHost
if err != nil && errors.Is(err, errHostNotAllowed) {
if !fallbackUnknownSNI {
metricUnknownServerName.Inc()
log.Debugx("requesting certificate", err, slog.String("host", hello.ServerName))
log.Debugx("requesting certificate", err)
return nil, nil
}
log.Debug("certificate for unknown hostname, using fallback hostname", slog.String("host", hello.ServerName))
// Some legitimate email deliveries over SMTP use an unknown SNI, e.g. a bare
// domain instead of the MX hostname. We "should" return an error, but that would
// break email delivery, so we use the fallback name if it is configured.
// ../rfc/9325:589
log = log.With(slog.String("servername", hello.ServerName))
log.Debug("certificate for unknown hostname, using fallback hostname")
hello.ServerName = fallbackHostname.ASCII
cert, err = m.Manager.GetCertificate(hello)
if err != nil {
metricCertRequestErrors.Inc()
log.Errorx("requesting certificate for fallback hostname", err, slog.String("host", hello.ServerName))
log.Errorx("requesting certificate for fallback hostname", err)
} else {
log.Debug("using certificate for fallback hostname", slog.String("host", hello.ServerName))
log.Debug("using certificate for fallback hostname")
}
return cert, err
} else if err != nil {
metricCertRequestErrors.Inc()
log.Errorx("requesting certificate", err, slog.String("host", hello.ServerName))
log.Errorx("requesting certificate", err)
}
return cert, err
}

View File

@ -365,9 +365,11 @@ See implementation guide, https://jmap.io/server.html
8616 Yes - Email Authentication for Internationalized Mail
# TLS
4366 - Obs (RFC 6066) Transport Layer Security (TLS) Extensions
5056 Yes - On the Use of Channel Bindings to Secure Channels
5705 Yes - Keying Material Exporters for Transport Layer Security (TLS)
5929 Yes - Channel Bindings for TLS
6066 - - Transport Layer Security (TLS) Extensions: Extension Definitions
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 -No - 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)
@ -377,6 +379,7 @@ See implementation guide, https://jmap.io/server.html
8996 Yes - Deprecating TLS 1.0 and TLS 1.1
8997 Yes - Deprecation of TLS 1.1 for Email Submission and Access
9266 Yes - Channel Bindings for TLS 1.3
9325 -? - Recommendations for Secure Use of Transport Layer Security (TLS) and Datagram Transport Layer Security (DTLS)
# ACME
8555 Yes - Automatic Certificate Management Environment (ACME)