fix(email): build an RFC 5322 message with a proper From address and name (#5941)

The notification/test email carried only From/To/Subject/MIME headers, and the From header was the raw SMTP username. Two problems: - When the SMTP login is not a bare email address (common with relays and submission services), the From header has no valid address and strict receivers reject the message — e.g. Gmail returns "550-5.7.1 ... Messages missing a valid address in From: header". - There was no Date (mandatory per RFC 5322 section 3.6) and no Message-ID, which also raises spam score. Add smtpFrom (sender address) and smtpFromName (display name) settings and assemble the message with net/mail: a name-addr From ("Name" <addr>), a Date, a Message-ID, and an RFC 2047 encoded Subject, in a deterministic header order. From falls back to the username when smtpFrom is empty, so existing setups keep working. Wire the settings through the model, the SMTP send and test paths, the Email settings UI, and all 13 locale files; regenerate the Zod/OpenAPI artifacts. Validate smtpFrom in AllSetting.CheckValid (reject anything net/mail cannot parse), which surfaces a bad address at configuration time and prevents CRLF header injection; strip CR/LF in buildMessage as defense in depth. Add buildMessage and CheckValid tests.
2026-08-26 13:07:14 +00:00 · 2026-07-14 13:55:46 +03:00
parent ae0da4c51f
commit 1cfd7b49b0
25 changed files with 292 additions and 14 deletions
@@ -2,9 +2,13 @@ package email

 import (
 	"context"
+	"crypto/rand"
 	"crypto/tls"
+	"encoding/hex"
 	"fmt"
+	"mime"
 	"net"
+	"net/mail"
 	"net/smtp"
 	"strings"
 	"time"
@@ -41,10 +45,15 @@ func (s *EmailService) Send(subject, body string) error {
 	}
 	username, _ := s.settingService.GetSmtpUsername()
 	password, _ := s.settingService.GetSmtpPassword()
+	fromAddr, _ := s.settingService.GetSmtpFrom()
+	fromName, _ := s.settingService.GetSmtpFromName()
 	toStr, _ := s.settingService.GetSmtpTo()
 	encryptionType, _ := s.settingService.GetSmtpEncryptionType()

-	from := username
+	from := fromAddr
+	if from == "" {
+		from = username
+	}
 	if from == "" {
 		return fmt.Errorf("smtp from not configured")
 	}
@@ -55,7 +64,7 @@ func (s *EmailService) Send(subject, body string) error {
 	}

 	addr := net.JoinHostPort(host, fmt.Sprintf("%d", port))
-	msg := buildMessage(from, recipients, subject, body)
+	msg := buildMessage(from, fromName, recipients, subject, body)

 	// Authenticate only when credentials are set. Go's PlainAuth refuses to run
 	// over the unencrypted "none" transport, so an open relay must use nil auth.
@@ -98,10 +107,15 @@ func (s *EmailService) TestConnection() SMTPTestResult {
 	}
 	username, _ := s.settingService.GetSmtpUsername()
 	password, _ := s.settingService.GetSmtpPassword()
+	fromAddr, _ := s.settingService.GetSmtpFrom()
+	fromName, _ := s.settingService.GetSmtpFromName()
 	toStr, _ := s.settingService.GetSmtpTo()
 	encryptionType, _ := s.settingService.GetSmtpEncryptionType()

-	from := username
+	from := fromAddr
+	if from == "" {
+		from = username
+	}

 	recipients := parseRecipients(toStr)
 	if len(recipients) == 0 {
@@ -166,7 +180,7 @@ func (s *EmailService) TestConnection() SMTPTestResult {
 		}
 	}

-	msg := buildMessage(from, recipients, "[3x-ui] Test email",
+	msg := buildMessage(from, fromName, recipients, "[3x-ui] Test email",
 		`<html><body style="font-family:monospace;font-size:14px">
 <h2>Test email from 3x-ui</h2>
 <p>If you received this, SMTP is configured correctly.</p>
@@ -280,18 +294,37 @@ func parseRecipients(toStr string) []string {
 	return out
 }

-func buildMessage(from string, to []string, subject, body string) []byte {
-	headers := map[string]string{
-		"From":         from,
-		"To":           strings.Join(to, ","),
-		"Subject":      subject,
-		"MIME-Version": "1.0",
-		"Content-Type": "text/html; charset=utf-8",
+// buildMessage assembles an RFC 5322 message. It emits the two mandatory
+// header fields (Date, From) plus Message-ID, so strict receivers such as Gmail
+// accept it and spam filters do not penalize a missing date or message id. The
+// From header is a proper name-addr ("Name" <addr>) via net/mail, and a
+// non-ASCII subject is RFC 2047 encoded.
+// headerSanitizer drops CR/LF so a crafted address or name cannot inject extra
+// header lines. Configured addresses are already validated at save time
+// (entity.AllSetting.CheckValid), this is defense in depth for buildMessage.
+var headerSanitizer = strings.NewReplacer("\r", "", "\n", "")
+
+func buildMessage(fromAddr, fromName string, to []string, subject, body string) []byte {
+	fromAddr = headerSanitizer.Replace(fromAddr)
+	fromName = headerSanitizer.Replace(fromName)
+	from := (&mail.Address{Name: fromName, Address: fromAddr}).String()
+
+	domain := "localhost"
+	if at := strings.LastIndex(fromAddr, "@"); at >= 0 && at+1 < len(fromAddr) {
+		domain = fromAddr[at+1:]
 	}
+	var token [16]byte
+	_, _ = rand.Read(token[:])
+	messageID := fmt.Sprintf("<%s@%s>", hex.EncodeToString(token[:]), domain)
+
 	var msg strings.Builder
-	for k, v := range headers {
-		fmt.Fprintf(&msg, "%s: %s\r\n", k, v)
-	}
+	fmt.Fprintf(&msg, "Date: %s\r\n", time.Now().Format(time.RFC1123Z))
+	fmt.Fprintf(&msg, "From: %s\r\n", from)
+	fmt.Fprintf(&msg, "To: %s\r\n", strings.Join(to, ", "))
+	fmt.Fprintf(&msg, "Message-ID: %s\r\n", messageID)
+	fmt.Fprintf(&msg, "Subject: %s\r\n", mime.QEncoding.Encode("utf-8", subject))
+	msg.WriteString("MIME-Version: 1.0\r\n")
+	msg.WriteString("Content-Type: text/html; charset=utf-8\r\n")
 	msg.WriteString("\r\n")
 	msg.WriteString(body)
 	return []byte(msg.String())
@@ -0,0 +1,81 @@
+package email
+
+import (
+	"io"
+	"mime"
+	"net/mail"
+	"strings"
+	"testing"
+)
+
+func TestBuildMessageIsRFC5322(t *testing.T) {
+	raw := buildMessage("panel@example.com", "3x-ui", []string{"a@example.com", "b@example.com"}, "Тест", "<b>hi</b>")
+
+	msg, err := mail.ReadMessage(strings.NewReader(string(raw)))
+	if err != nil {
+		t.Fatalf("message does not parse as RFC 5322: %v", err)
+	}
+
+	from, err := mail.ParseAddress(msg.Header.Get("From"))
+	if err != nil {
+		t.Fatalf("From header does not parse: %v", err)
+	}
+	if from.Name != "3x-ui" || from.Address != "panel@example.com" {
+		t.Errorf("From = %q <%q>, want name %q addr %q", from.Name, from.Address, "3x-ui", "panel@example.com")
+	}
+
+	if _, err := msg.Header.Date(); err != nil {
+		t.Errorf("Date header missing or unparseable: %v", err)
+	}
+
+	id := msg.Header.Get("Message-ID")
+	if !strings.HasPrefix(id, "<") || !strings.HasSuffix(id, "@example.com>") {
+		t.Errorf("Message-ID = %q, want <token@example.com>", id)
+	}
+
+	subject, err := (&mime.WordDecoder{}).DecodeHeader(msg.Header.Get("Subject"))
+	if err != nil {
+		t.Fatalf("Subject does not decode: %v", err)
+	}
+	if subject != "Тест" {
+		t.Errorf("Subject = %q, want %q", subject, "Тест")
+	}
+
+	body, _ := io.ReadAll(msg.Body)
+	if string(body) != "<b>hi</b>" {
+		t.Errorf("body = %q, want %q", body, "<b>hi</b>")
+	}
+}
+
+func TestBuildMessageFromWithoutName(t *testing.T) {
+	raw := buildMessage("panel@example.com", "", []string{"a@example.com"}, "s", "b")
+	msg, err := mail.ReadMessage(strings.NewReader(string(raw)))
+	if err != nil {
+		t.Fatalf("parse: %v", err)
+	}
+	from, err := mail.ParseAddress(msg.Header.Get("From"))
+	if err != nil {
+		t.Fatalf("From header does not parse: %v", err)
+	}
+	if from.Name != "" || from.Address != "panel@example.com" {
+		t.Errorf("From = %q <%q>, want bare addr", from.Name, from.Address)
+	}
+}
+
+func TestBuildMessageStripsHeaderInjection(t *testing.T) {
+	raw := buildMessage(
+		"panel@example.com\r\nBcc: evil@example.com",
+		"Name\r\nX-Evil: 1",
+		[]string{"a@example.com"}, "s", "b",
+	)
+	msg, err := mail.ReadMessage(strings.NewReader(string(raw)))
+	if err != nil {
+		t.Fatalf("parse: %v", err)
+	}
+	if got := msg.Header.Get("Bcc"); got != "" {
+		t.Errorf("injected Bcc header leaked: %q", got)
+	}
+	if got := msg.Header.Get("X-Evil"); got != "" {
+		t.Errorf("injected X-Evil header leaked: %q", got)
+	}
+}
@@ -148,6 +148,8 @@ var defaultValueMap = map[string]string{
 	"smtpPort":           "587",
 	"smtpUsername":       "",
 	"smtpPassword":       "",
+	"smtpFrom":           "",
+	"smtpFromName":       "",
 	"smtpTo":             "",
 	"smtpEncryptionType": "starttls", // no, starttls, tls
 }
@@ -1045,6 +1047,22 @@ func (s *SettingService) SetSmtpUsername(value string) error {
 	return s.setString("smtpUsername", value)
 }

+func (s *SettingService) GetSmtpFrom() (string, error) {
+	return s.getString("smtpFrom")
+}
+
+func (s *SettingService) SetSmtpFrom(value string) error {
+	return s.setString("smtpFrom", value)
+}
+
+func (s *SettingService) GetSmtpFromName() (string, error) {
+	return s.getString("smtpFromName")
+}
+
+func (s *SettingService) SetSmtpFromName(value string) error {
+	return s.setString("smtpFromName", value)
+}
+
 func (s *SettingService) GetSmtpPassword() (string, error) {
 	return s.getString("smtpPassword")
 }