git » chasquid » commit b3a500d

diff --git a/internal/smtpsrv/conn.go b/internal/smtpsrv/conn.go
index a35bae2..51ec6f7 100644
--- a/internal/smtpsrv/conn.go
+++ b/internal/smtpsrv/conn.go
@@ -214,7 +214,7 @@ func (c *Conn) Handle() {
 
 		cstate := tc.ConnectionState()
 		c.tlsConnState = &cstate
-		if name := c.tlsConnState.ServerName; name != "" {
+		if name := c.validServerName(); name != "" {
 			c.hostname = name
 		}
 	}
@@ -837,6 +837,35 @@ func (c *Conn) addReceivedHeader() {
 	}
 }
 
+// validServerName returns the server name the client requested via TLS SNI,
+// if one of our certificates is valid for it; otherwise it returns "".
+//
+// The SNI is controlled by the client, and Go accepts almost any value (e.g.
+// with spaces, ";", or CR/LF), so we must not use it blindly: it ends up in
+// our greeting, and the Received and Authentication-Results headers.
+// VerifyHostname only accepts valid hostnames, and then checks them against
+// the certificate.
+//
+// TODO: Once we can depend on Go 1.27, use ConnectionState.LocalCertificate
+// to check against the certificate we actually presented. Today they are
+// almost always the same, and when not, the name is still valid for one of
+// our certificates.
+func (c *Conn) validServerName() string {
+	name := c.tlsConnState.ServerName
+	if name == "" {
+		return ""
+	}
+
+	for _, cert := range c.tlsConfig.Certificates {
+		if cert.Leaf != nil && cert.Leaf.VerifyHostname(name) == nil {
+			return name
+		}
+	}
+
+	c.tr.Debugf("ignoring SNI %q, no certificate is valid for it", name)
+	return ""
+}
+
 // Escape "(" and ")" with quoted-pairs, for use inside header comments.
 // https://datatracker.ietf.org/doc/html/rfc5322#section-3.2.2
 var commentEscaper = strings.NewReplacer("(", `\(`, ")", `\)`)
@@ -1173,7 +1202,7 @@ func (c *Conn) STARTTLS(params string) (code int, msg string) {
 
 	// If the client requested a specific server and we complied, that's our
 	// identity from now on.
-	if name := c.tlsConnState.ServerName; name != "" {
+	if name := c.validServerName(); name != "" {
 		c.hostname = name
 	}
 
diff --git a/internal/smtpsrv/conn_test.go b/internal/smtpsrv/conn_test.go
index 552c452..d73554e 100644
--- a/internal/smtpsrv/conn_test.go
+++ b/internal/smtpsrv/conn_test.go
@@ -1,9 +1,14 @@
 package smtpsrv
 
 import (
+	"crypto/ed25519"
+	"crypto/tls"
+	"crypto/x509"
+	"math/big"
 	"net"
 	"strings"
 	"testing"
+	"time"
 
 	"blitiri.com.ar/go/chasquid/internal/domaininfo"
 	"blitiri.com.ar/go/chasquid/internal/trace"
@@ -153,6 +158,71 @@ func TestReceivedHeaderEnvelopeFrom(t *testing.T) {
 	}
 }
 
+func TestValidServerName(t *testing.T) {
+	// Self-signed certificate valid for mx.example.com and *.example.net.
+	pub, priv, err := ed25519.GenerateKey(nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	tmpl := &x509.Certificate{
+		SerialNumber: big.NewInt(1),
+		DNSNames:     []string{"mx.example.com", "*.example.net"},
+		NotBefore:    time.Now(),
+		NotAfter:     time.Now().Add(time.Hour),
+	}
+	der, err := x509.CreateCertificate(nil, tmpl, tmpl, pub, priv)
+	if err != nil {
+		t.Fatal(err)
+	}
+	leaf, err := x509.ParseCertificate(der)
+	if err != nil {
+		t.Fatal(err)
+	}
+	cert := tls.Certificate{
+		Certificate: [][]byte{der},
+		PrivateKey:  priv,
+		Leaf:        leaf,
+	}
+
+	cases := []struct {
+		sni  string
+		want string
+	}{
+		{"", ""},
+		{"mx.example.com", "mx.example.com"},
+		{"MX.Example.COM", "MX.Example.COM"},
+		{"a.example.net", "a.example.net"},
+		{"a.b.example.net", ""},
+		{"other.example.com", ""},
+		{"mx.example.com; dkim=pass header.d=example.org", ""},
+		{"mx.example.com\r\nX-Injected: yes", ""},
+		{"a;b.example.net", ""},
+	}
+	for _, c := range cases {
+		conn := &Conn{
+			tlsConfig:    &tls.Config{Certificates: []tls.Certificate{cert}},
+			tlsConnState: &tls.ConnectionState{ServerName: c.sni},
+			tr:           trace.New("test", "TestValidServerName"),
+		}
+		if got := conn.validServerName(); got != c.want {
+			t.Errorf("SNI %q: got %q, want %q", c.sni, got, c.want)
+		}
+		conn.tr.Finish()
+	}
+
+	// Without the parsed leaf, we can't check, so the SNI is not used.
+	cert.Leaf = nil
+	conn := &Conn{
+		tlsConfig:    &tls.Config{Certificates: []tls.Certificate{cert}},
+		tlsConnState: &tls.ConnectionState{ServerName: "mx.example.com"},
+		tr:           trace.New("test", "TestValidServerName"),
+	}
+	if got := conn.validServerName(); got != "" {
+		t.Errorf("without leaf: got %q, want \"\"", got)
+	}
+	conn.tr.Finish()
+}
+
 func TestSanitizeEHLODomain(t *testing.T) {
 	equal := []string{
 		"domain", "do.main", "do-main",
diff --git a/internal/smtpsrv/server_test.go b/internal/smtpsrv/server_test.go
index 3540237..e7bc502 100644
--- a/internal/smtpsrv/server_test.go
+++ b/internal/smtpsrv/server_test.go
@@ -550,6 +550,55 @@ func TestStartTLSOnTLS(t *testing.T) {
 	}
 }
 
+// The server name requested via SNI must only be used as our hostname if one
+// of our certificates is valid for it.
+func TestServerNameFromSNI(t *testing.T) {
+	cases := []struct {
+		sni  string
+		want string
+	}{
+		{"localhost", "localhost"},
+		{"other.example.com", "localhost"},
+		{"localhost; dkim=pass header.d=example.com", "localhost"},
+	}
+
+	for _, c := range cases {
+		cfg := tlsConfig.Clone()
+		cfg.ServerName = c.sni
+		cfg.InsecureSkipVerify = true
+
+		// STARTTLS, and then EHLO, whose reply starts with our hostname.
+		sc, err := smtp.Dial(smtpAddr)
+		if err != nil {
+			t.Fatalf("smtp.Dial: %v", err)
+		}
+		if err := sc.StartTLS(cfg); err != nil {
+			t.Fatalf("%q: StartTLS: %v", c.sni, err)
+		}
+		msg := simpleCmd(t, sc, "EHLO test", 250)
+		if !strings.HasPrefix(msg, c.want+" ") {
+			t.Errorf("%q: STARTTLS: EHLO reply %q, expected hostname %q",
+				c.sni, msg, c.want)
+		}
+		sc.Close()
+
+		// TLS-wrapped, where the greeting contains our hostname.
+		conn, err := tls.Dial("tcp", submissionTLSAddr, cfg)
+		if err != nil {
+			t.Fatalf("%q: tls.Dial: %v", c.sni, err)
+		}
+		_, msg, err = textproto.NewConn(conn).ReadResponse(220)
+		if err != nil {
+			t.Fatalf("%q: reading greeting: %v", c.sni, err)
+		}
+		if !strings.HasPrefix(msg, c.want+" ") {
+			t.Errorf("%q: TLS: greeting %q, expected hostname %q",
+				c.sni, msg, c.want)
+		}
+		conn.Close()
+	}
+}
+
 func TestAddDKIMSigner(t *testing.T) {
 	s := NewServer()
 	err := s.AddDKIMSigner("example.com", "selector", "keyfile-does-not-exist")