git » chasquid » commit f3374e7

diff --git a/docs/hooks.md b/docs/hooks.md
index 97fb127..bf8a7d1 100644
--- a/docs/hooks.md
+++ b/docs/hooks.md
@@ -42,9 +42,10 @@ The environment will contain the following variables:
  - `$PWD`: The working directory, which will be the config directory.
  - `$REMOTE_ADDR`: IP address of the remote side of the connection.
  - `$EHLO_DOMAIN`: EHLO/HELO domain, as given by the client; sanitized for
-   safety.
- - `$EHLO_DOMAIN_RAW`: Same as `$EHLO_DOMAIN`, but not sanitized; be careful as
-   it can contain problematic characters.
+   safety (only letters, digits, and `-.[]:` are kept). If there are no valid
+   characters, it is `no-valid-characters.invalid`.
+ - `$EHLO_DOMAIN_RAW`: Same as `$EHLO_DOMAIN`. Kept for backwards
+   compatibility; it used to be the unsanitized value.
  - `$MAIL_FROM`: MAIL FROM address.
  - `$RCPT_TO`: RCPT TO addresses, space separated.
  - `$AUTH_AS`: Authenticated user; empty if the connection has not
diff --git a/internal/smtpsrv/conn.go b/internal/smtpsrv/conn.go
index c4b1934..ba01e0e 100644
--- a/internal/smtpsrv/conn.go
+++ b/internal/smtpsrv/conn.go
@@ -356,7 +356,7 @@ func (c *Conn) HELO(params string) (code int, msg string) {
 	if len(strings.TrimSpace(params)) == 0 {
 		return 501, "Invisible customers are not welcome!"
 	}
-	c.ehloDomain = strings.Fields(params)[0]
+	c.setEHLODomain(params)
 
 	types := []string{
 		"general store", "used armor dealership", "second-hand bookstore",
@@ -374,7 +374,7 @@ func (c *Conn) EHLO(params string) (code int, msg string) {
 	if len(strings.TrimSpace(params)) == 0 {
 		return 501, "Invisible customers are not welcome!"
 	}
-	c.ehloDomain = strings.Fields(params)[0]
+	c.setEHLODomain(params)
 	c.isESMTP = true
 
 	buf := bytes.NewBuffer(nil)
@@ -871,11 +871,26 @@ func checkData(data []byte) error {
 	return nil
 }
 
+// Placeholder EHLO domain, used when the one given by the client has no valid
+// characters. The .invalid TLD is reserved for names that are guaranteed to
+// be invalid (RFC 6761).
+const invalidEHLODomain = "no-valid-characters.invalid"
+
+// setEHLODomain sets the EHLO domain from the (non-empty) HELO/EHLO
+// parameters, sanitizing it.
+func (c *Conn) setEHLODomain(params string) {
+	c.ehloDomain = sanitizeEHLODomain(strings.Fields(params)[0])
+	if c.ehloDomain == "" {
+		c.ehloDomain = invalidEHLODomain
+	}
+}
+
 // Sanitize HELO/EHLO domain.
 // RFC is extremely flexible with EHLO domain values, allowing all printable
 // ASCII characters. They can be tricky to use in shell scripts (commonly used
-// as post-data hooks), so this function sanitizes the value to make it
-// shell-safe.
+// as post-data hooks), and in headers (e.g. "(", ")" and ";" have special
+// meanings in the Received header), so this function sanitizes the value to
+// make it safe for both.
 func sanitizeEHLODomain(s string) string {
 	var n strings.Builder
 	for _, c := range s {
@@ -918,7 +933,10 @@ func (c *Conn) runPostDataHook(data []byte) ([]byte, bool, error) {
 		cmd.Env = append(cmd.Env, v+"="+os.Getenv(v))
 	}
 	cmd.Env = append(cmd.Env, "REMOTE_ADDR="+c.remoteAddr.String())
-	cmd.Env = append(cmd.Env, "EHLO_DOMAIN="+sanitizeEHLODomain(c.ehloDomain))
+	cmd.Env = append(cmd.Env, "EHLO_DOMAIN="+c.ehloDomain)
+
+	// The EHLO domain is already sanitized, so this is the same as
+	// EHLO_DOMAIN. We keep it for backwards compatibility.
 	cmd.Env = append(cmd.Env, "EHLO_DOMAIN_RAW="+c.ehloDomain)
 	cmd.Env = append(cmd.Env, "MAIL_FROM="+c.mailFrom)
 	cmd.Env = append(cmd.Env, "RCPT_TO="+strings.Join(c.rcptTo, " "))
diff --git a/internal/smtpsrv/conn_test.go b/internal/smtpsrv/conn_test.go
index fad3bb1..75e538a 100644
--- a/internal/smtpsrv/conn_test.go
+++ b/internal/smtpsrv/conn_test.go
@@ -142,3 +142,29 @@ func TestSanitizeEHLODomain(t *testing.T) {
 		}
 	}
 }
+
+func TestEHLODomainIsSanitized(t *testing.T) {
+	cases := []struct {
+		params   string
+		expected string
+	}{
+		{"mx.example.com", "mx.example.com"},
+		{"[1.2.3.4] extra params", "[1.2.3.4]"},
+		{"evil;(x", "evilx"},
+		{"!!!", invalidEHLODomain},
+	}
+	for _, c := range cases {
+		for _, cmd := range []string{"HELO", "EHLO"} {
+			conn := &Conn{}
+			if cmd == "HELO" {
+				conn.HELO(c.params)
+			} else {
+				conn.EHLO(c.params)
+			}
+			if conn.ehloDomain != c.expected {
+				t.Errorf("%s %q: got EHLO domain %q, expected %q",
+					cmd, c.params, conn.ehloDomain, c.expected)
+			}
+		}
+	}
+}