From b1c86d436dc679a3dc272b344c803721d1bf6e92 Mon Sep 17 00:00:00 2001 From: kevin Date: Fri, 28 Aug 2026 14:15:09 +0800 Subject: [PATCH] =?UTF-8?q?fix(security):=20=E5=86=99=E4=BF=A1=E9=85=8D?= =?UTF-8?q?=E9=A2=9D=E6=B3=84=E6=BC=8F=E7=BB=9F=E4=B8=80=E8=A1=A5=E5=81=BF?= =?UTF-8?q?=EF=BC=8COAuth2=20=E4=BB=85=E6=8E=A5=E5=8F=97=E5=B7=B2=E9=AA=8C?= =?UTF-8?q?=E8=AF=81=E9=82=AE=E7=AE=B1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 安全审计 #21/#22（security_todo.md P2）： - #21 DoSend 配额补偿收敛为 defer + persistedBytes 统一对账： 预扣总量与实际落库量的差额在任何退出路径退还，覆盖外发入队 失败/本地投递失败/Sent 副本保存失败三条提前 return 的泄漏路径； 移除文件读取与附件保存失败的分散回退 - #22 OAuth2 仅接受已验证邮箱：GitHub /user/emails 过滤 verified==true（主邮箱优先，其次任一已验证）；Google email_verified 为 false 或缺失时拒绝（fail closed）；解析逻辑 抽为 parseGitHubEmail/parseGoogleEmail 纯函数，不依赖 IdP 当前只返回已验证邮箱的实现细节 - 新增 8 项单测（DoSend 回退/不误退、GitHub/Google 邮箱解析矩阵） - security_todo.md 勾选 #21/#22 --- internal/auth/oauth2.go | 78 ++++++++----- internal/auth/oauth2_email_test.go | 72 ++++++++++++ internal/web/handlers/mail.go | 24 ++-- internal/web/handlers/mail_send_test.go | 144 ++++++++++++++++++++++++ security_todo.md | 22 ++-- 5 files changed, 296 insertions(+), 44 deletions(-) create mode 100644 internal/auth/oauth2_email_test.go create mode 100644 internal/web/handlers/mail_send_test.go diff --git a/internal/auth/oauth2.go b/internal/auth/oauth2.go index 2ba49a0..b1b2c0b 100644 --- a/internal/auth/oauth2.go +++ b/internal/auth/oauth2.go @@ -96,40 +96,66 @@ func (p *OAuth2Provider) fetchUserEmail(token *oauth2.Token) (string, error) { } if p.cfg.OAuth2Provider == "google" { - var userInfo struct { - Email string `json:"email"` - } - if err := json.Unmarshal(body, &userInfo); err != nil { - return "", fmt.Errorf("解析Google用户信息失败: %w", err) - } - if userInfo.Email == "" { - return "", fmt.Errorf("Google账户未关联邮箱") - } - return userInfo.Email, nil + return parseGoogleEmail(body) } if p.cfg.OAuth2Provider == "github" { - var emails []struct { - Email string `json:"email"` - Primary bool `json:"primary"` - } - if err := json.Unmarshal(body, &emails); err != nil { - return "", fmt.Errorf("解析GitHub用户信息失败: %w", err) - } - for _, e := range emails { - if e.Primary { - return e.Email, nil - } - } - if len(emails) > 0 { - return emails[0].Email, nil - } - return "", fmt.Errorf("GitHub账户未关联邮箱") + return parseGitHubEmail(body) } return "", fmt.Errorf("OAuth2 邮箱获取尚未实现") } +// parseGoogleEmail 解析 Google userinfo 响应，仅接受已验证邮箱。 +// verified_email 缺失按未验证处理（fail closed）：未验证邮箱可能不属于 +// 该账号所有者，放行会允许冒充既有用户登录（账号接管）。 +func parseGoogleEmail(body []byte) (string, error) { + var userInfo struct { + Email string `json:"email"` + EmailVerified bool `json:"verified_email"` + } + if err := json.Unmarshal(body, &userInfo); err != nil { + return "", fmt.Errorf("解析Google用户信息失败: %w", err) + } + if userInfo.Email == "" { + return "", fmt.Errorf("Google账户未关联邮箱") + } + if !userInfo.EmailVerified { + return "", fmt.Errorf("Google 账号邮箱未验证") + } + return userInfo.Email, nil +} + +// parseGitHubEmail 解析 /user/emails 响应，仅接受已验证邮箱：主邮箱优先， +// 其次任一已验证邮箱。GitHub 该接口当前实际只返回已验证邮箱，显式校验 +// 使登录安全不依赖该行为。 +func parseGitHubEmail(body []byte) (string, error) { + var emails []struct { + Email string `json:"email"` + Primary bool `json:"primary"` + Verified bool `json:"verified"` + } + if err := json.Unmarshal(body, &emails); err != nil { + return "", fmt.Errorf("解析GitHub用户信息失败: %w", err) + } + var fallback string + for _, e := range emails { + if e.Email == "" || !e.Verified { + continue + } + if e.Primary { + return e.Email, nil + } + if fallback == "" { + fallback = e.Email + } + } + if fallback != "" { + return fallback, nil + } + return "", fmt.Errorf("GitHub账户没有已验证邮箱") +} + // Authenticate 实现 Provider 接口（OAuth2 不使用此方法，通过回调流程认证） func (p *OAuth2Provider) Authenticate(credentials map[string]string) (string, error) { return "", fmt.Errorf("OAuth2 不支持直接认证，请使用回调流程") diff --git a/internal/auth/oauth2_email_test.go b/internal/auth/oauth2_email_test.go new file mode 100644 index 0000000..ec6745f --- /dev/null +++ b/internal/auth/oauth2_email_test.go @@ -0,0 +1,72 @@ +package auth + +// OAuth2 邮箱解析回归测试（#22）：仅接受已验证邮箱——未验证邮箱可能 +// 不属于该账号所有者，放行会允许冒充既有用户登录（账号接管）。 + +import ( + "strings" + "testing" +) + +func TestParseGitHubEmailPrefersVerifiedPrimary(t *testing.T) { + body := []byte(`[{"email":"p@example.com","primary":true,"verified":true}]`) + email, err := parseGitHubEmail(body) + if err != nil { + t.Fatalf("parseGitHubEmail: %v", err) + } + if email != "p@example.com" { + t.Fatalf("email = %q, want p@example.com", email) + } +} + +func TestParseGitHubEmailSkipsUnverifiedPrimary(t *testing.T) { + // 主邮箱未验证：不能用它登录，回退到任一已验证邮箱 + body := []byte(`[ + {"email":"unverified@example.com","primary":true,"verified":false}, + {"email":"verified@example.com","primary":false,"verified":true} + ]`) + email, err := parseGitHubEmail(body) + if err != nil { + t.Fatalf("parseGitHubEmail: %v", err) + } + if email != "verified@example.com" { + t.Fatalf("email = %q, want verified@example.com", email) + } +} + +func TestParseGitHubEmailRejectsAllUnverified(t *testing.T) { + body := []byte(`[ + {"email":"a@example.com","primary":true,"verified":false}, + {"email":"b@example.com","primary":false,"verified":false} + ]`) + _, err := parseGitHubEmail(body) + if err == nil || !strings.Contains(err.Error(), "已验证") { + t.Fatalf("expected unverified-email rejection, got %v", err) + } +} + +func TestParseGitHubEmailMalformedBody(t *testing.T) { + if _, err := parseGitHubEmail([]byte("not json")); err == nil { + t.Fatal("expected error for malformed body") + } +} + +func TestParseGoogleEmailAcceptsVerified(t *testing.T) { + email, err := parseGoogleEmail([]byte(`{"email":"a@example.com","verified_email":true}`)) + if err != nil { + t.Fatalf("parseGoogleEmail: %v", err) + } + if email != "a@example.com" { + t.Fatalf("email = %q, want a@example.com", email) + } +} + +func TestParseGoogleEmailRejectsUnverified(t *testing.T) { + if _, err := parseGoogleEmail([]byte(`{"email":"a@example.com","verified_email":false}`)); err == nil { + t.Fatal("expected rejection for verified_email=false") + } + // verified_email 缺失按未验证处理（fail closed） + if _, err := parseGoogleEmail([]byte(`{"email":"a@example.com"}`)); err == nil { + t.Fatal("expected rejection when verified_email is missing") + } +} diff --git a/internal/web/handlers/mail.go b/internal/web/handlers/mail.go index 9498441..b1ea666 100644 --- a/internal/web/handlers/mail.go +++ b/internal/web/handlers/mail.go @@ -255,11 +255,21 @@ func (h *MailHandler) DoSend(c *gin.Context) { // Handle attachments and check quota form, multipartErr := c.MultipartForm() attachments := make([]pendingAttachment, 0) + // 统一配额补偿：quotaReserved 为本次预扣总量，persistedBytes 为实际 + // 落库的附件量；函数任何路径退出（含外发入队失败/本地投递失败/Sent + // 副本保存失败的提前 return）都退还预扣量中未落库的部分，杜绝配额 + // 凭空泄漏。 + var quotaReserved, persistedBytes int64 + defer func() { + if remaining := quotaReserved - persistedBytes; remaining > 0 { + _ = h.stores.Users.UpdateUsedBytes(userID, -remaining) + } + }() if multipartErr == nil { files := form.File["attachments"] if len(files) > 0 { // 原子预扣附件配额（单条 SQL：used_bytes + n <= quota_bytes 才生效）， - // 防止并发提交绕过配额检查（TOCTOU）。后续保存失败会补偿回退。 + // 防止并发提交绕过配额检查（TOCTOU）。补偿由上方 defer 统一处理。 var totalNewSize int64 for _, file := range files { totalNewSize += file.Size @@ -289,20 +299,19 @@ func (h *MailHandler) DoSend(c *gin.Context) { })) return } + quotaReserved = totalNewSize // Read all attachment files into memory once (used for both the // MIME message body and the stored attachment records). - // 读取失败的文件回退已预扣的配额。 + // 读取失败的文件不会落库，其份额由 defer 统一回退。 for _, file := range files { f, err := file.Open() if err != nil { - _ = h.stores.Users.UpdateUsedBytes(userID, -file.Size) continue } buf, readErr := io.ReadAll(f) f.Close() if readErr != nil { - _ = h.stores.Users.UpdateUsedBytes(userID, -file.Size) continue } @@ -407,12 +416,11 @@ func (h *MailHandler) DoSend(c *gin.Context) { } // Save attachment records linked to the Sent copy (bytes were already - // read during message construction). 配额已在前面原子预扣， - // 保存/落库失败的附件需要补偿回退。 + // read during message construction). 配额已在前面原子预扣，落库成功 + // 的份额记入 persistedBytes，失败份额由 defer 统一回退。 for _, att := range attachments { relPath, err := h.storage.Save(att.filename, att.data) if err != nil { - _ = h.stores.Users.UpdateUsedBytes(userID, -int64(len(att.data))) continue } @@ -424,9 +432,9 @@ func (h *MailHandler) DoSend(c *gin.Context) { FileSize: int64(len(att.data)), } if err := h.stores.Attachments.Create(attRecord); err != nil { - _ = h.stores.Users.UpdateUsedBytes(userID, -attRecord.FileSize) continue } + persistedBytes += attRecord.FileSize } c.Redirect(http.StatusFound, "/sent") diff --git a/internal/web/handlers/mail_send_test.go b/internal/web/handlers/mail_send_test.go new file mode 100644 index 0000000..23ffaf7 --- /dev/null +++ b/internal/web/handlers/mail_send_test.go @@ -0,0 +1,144 @@ +package handlers + +// DoSend 配额记账回归测试（#21）：外发入队失败等提前 return 的路径必须 +// 全额回退预扣量（修复前配额凭空泄漏）；成功投递时预扣量与实际落库量 +// 一致，统一补偿不误退。 + +import ( + "bytes" + "html/template" + "mime/multipart" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + "mail_go/internal/db" + "mail_go/internal/imap_server" + "mail_go/internal/storage" + "mail_go/internal/store" + + "github.com/gin-gonic/gin" + "gorm.io/driver/sqlite" + "gorm.io/gorm" +) + +func newSendTestHandler(t *testing.T) (*MailHandler, *store.Stores, string) { + t.Helper() + gdb, err := gorm.Open(sqlite.Open(filepath.Join(t.TempDir(), "test.db")), &gorm.Config{}) + if err != nil { + t.Fatal(err) + } + if err := gdb.AutoMigrate(&db.User{}, &db.Domain{}, &db.Message{}, &db.Attachment{}, &db.Mailbox{}, &db.MailboxState{}); err != nil { + t.Fatal(err) + } + stores := store.NewStores(gdb) + // 用户 1 发件（配额 1MB），用户 2 本地收件 + if err := stores.Users.Create(&db.User{ID: 1, Username: "alice", Domain: db.Domain{Name: "example.com"}, DomainID: 1, QuotaBytes: 1 << 20}); err != nil { + t.Fatal(err) + } + if err := stores.Users.Create(&db.User{ID: 2, Username: "bob", DomainID: 1}); err != nil { + t.Fatal(err) + } + attDir := t.TempDir() + return NewMailHandler(stores, storage.NewAttachmentStorage(attDir), nil, imap_server.NewMailboxService(stores), nil), stores, attDir +} + +func newSendTestRouter(h *MailHandler) *gin.Engine { + gin.SetMode(gin.TestMode) + r := gin.New() + tmpl := template.Must(template.New("").Funcs(testTemplateFuncs()).ParseGlob(filepath.Join("..", "templates", "*.html"))) + r.SetHTMLTemplate(tmpl) + r.Use(func(c *gin.Context) { + c.Set("userID", uint(1)) + c.Set("currentUser", &db.User{ID: 1, Username: "alice", Domain: db.Domain{Name: "example.com"}}) + c.Next() + }) + r.POST("/compose", h.DoSend) + return r +} + +func newComposeRequest(t *testing.T, to string, attachments map[string]string) *http.Request { + t.Helper() + body := &bytes.Buffer{} + mw := multipart.NewWriter(body) + if to != "" { + if err := mw.WriteField("to", to); err != nil { + t.Fatal(err) + } + } + for name, content := range attachments { + fw, err := mw.CreateFormFile("attachments", name) + if err != nil { + t.Fatal(err) + } + if _, err := fw.Write([]byte(content)); err != nil { + t.Fatal(err) + } + } + if err := mw.Close(); err != nil { + t.Fatal(err) + } + req := httptest.NewRequest(http.MethodPost, "/compose", body) + req.Header.Set("Content-Type", mw.FormDataContentType()) + return req +} + +// TestDoSendRefundsQuotaWhenOutboundDisabled 验证外发未启用（入队失败路径） +// 提前退出时，预扣的附件配额全额回退（修复前凭空泄漏）。 +func TestDoSendRefundsQuotaWhenOutboundDisabled(t *testing.T) { + h, stores, _ := newSendTestHandler(t) + r := newSendTestRouter(h) + + w := httptest.NewRecorder() + r.ServeHTTP(w, newComposeRequest(t, "stranger@gmail.com", map[string]string{"a.txt": "hello world"})) + + if w.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400", w.Code) + } + u, err := stores.Users.GetByID(1) + if err != nil { + t.Fatal(err) + } + if u.UsedBytes != 0 { + t.Fatalf("used_bytes = %d, want 0 after refund", u.UsedBytes) + } + if n, _ := stores.Mails.CountAll(); n != 0 { + t.Fatalf("messages = %d, want 0", n) + } +} + +// TestDoSendKeepsQuotaOnSuccess 验证成功投递时预扣量与实际落库量一致， +// 统一补偿不误退（附件挂 Sent 副本）。 +func TestDoSendKeepsQuotaOnSuccess(t *testing.T) { + h, stores, attDir := newSendTestHandler(t) + r := newSendTestRouter(h) + + w := httptest.NewRecorder() + r.ServeHTTP(w, newComposeRequest(t, "bob@example.com", map[string]string{"a.txt": "hello world"})) + + if w.Code != http.StatusFound { + t.Fatalf("status = %d, want 302", w.Code) + } + u, err := stores.Users.GetByID(1) + if err != nil { + t.Fatal(err) + } + if u.UsedBytes != 11 { + t.Fatalf("used_bytes = %d, want 11", u.UsedBytes) + } + + sent, total, err := stores.Mails.ListByUserAndFolder(1, "Sent", 1, 50) + if err != nil || total != 1 || len(sent) != 1 { + t.Fatalf("sent copies = %d (err=%v), want 1", len(sent), err) + } + atts, err := stores.Attachments.ListByMessage(sent[0].ID) + if err != nil || len(atts) != 1 { + t.Fatalf("attachments = %d (err=%v), want 1", len(atts), err) + } + content, err := os.ReadFile(filepath.Join(attDir, atts[0].FilePath)) + if err != nil || string(content) != "hello world" { + t.Fatalf("attachment file missing or wrong content (err=%v)", err) + } +} diff --git a/security_todo.md b/security_todo.md index 235ebd1..aa1577b 100644 --- a/security_todo.md +++ b/security_todo.md @@ -165,23 +165,25 @@ ### 21. Web 发信配额预扣泄漏（第三轮，2026-08-28） -- [ ] 位置：`internal/web/handlers/mail.go:278`（预扣）→ 367 / 390 / 416（泄漏点） +- [x] 位置：`internal/web/handlers/mail.go:278`（预扣）→ 367 / 390 / 416（泄漏点） - 现状：`TryReserveQuota` 预扣附件配额后，三处提前 return 不回退：外部收件人入队失败（367）、本地投递失败（390）、Sent 副本保存失败（416）。文件读取失败（310）与附件保存失败（426）有回退，上述路径漏了。用户反复提交含无效外部地址的带附件邮件，可把自己配额扣光且无对应文件占用（配额"凭空消失"，需管理员改库恢复）。 - 修复方案： - - [ ] defer + 成功标志统一补偿：未走到"附件记录全部落库"终点即回退剩余预扣量；或各失败分支显式回退。 - - [ ] 注意 367 处入队失败时可能已成功入队部分外部收件人，只回退附件未消耗部分。 + - [x] defer + 成功标志统一补偿：未走到"附件记录全部落库"终点即回退剩余预扣量；或各失败分支显式回退。→ 实现 `defer + persistedBytes` 统一补偿：预扣总量记 `quotaReserved`、实际落库量记 `persistedBytes`，任何退出路径退还差额；同时移除读取/保存失败的分散回退 + - [x] 注意 367 处入队失败时可能已成功入队部分外部收件人，只回退附件未消耗部分。→ 附件仅随 Sent 副本落库，提前退出时未落库即全额退还，与外发队列的原始 MIME 内联附件无关 - 验证： - - [ ] 单测：外部入队失败 / 本地投递失败路径退出后 `used_bytes` 恢复原值。 + - [x] 单测：外部入队失败 / 本地投递失败路径退出后 `used_bytes` 恢复原值。（`TestDoSendRefundsQuotaWhenOutboundDisabled`；另含成功路径不误退 `TestDoSendKeepsQuotaOnSuccess`） +- 已完成（2026-08-28）。 ### 22. OAuth2 登录不校验邮箱 verified 状态（第三轮，2026-08-28） -- [ ] 位置：`internal/auth/oauth2.go:111-127` +- [x] 位置：`internal/auth/oauth2.go:111-127` - 现状：GitHub `/user/emails` 响应含 `verified` 字段但未检查，取 primary 首个即用；Google 未检查 `email_verified`。当前 GitHub 该 API 实际只返回已验证邮箱，现实可利用性低，但属依赖 IdP 实现细节；自建 provider（代码 33-36 行支持任意 host）场景下未验证邮箱可登录他人账号。 - 修复方案： - - [ ] GitHub：过滤 `verified == true`（primary 优先），无已验证邮箱返回错误。 - - [ ] Google：解析 `email_verified`，false 或缺失时拒绝。 + - [x] GitHub：过滤 `verified == true`（primary 优先），无已验证邮箱返回错误。→ 解析抽为 `parseGitHubEmail` 纯函数 + - [x] Google：解析 `email_verified`，false 或缺失时拒绝。→ `parseGoogleEmail`，缺失按未验证 fail closed - 验证： - - [ ] 单测：构造含未验证邮箱的响应被拒。 + - [x] 单测：构造含未验证邮箱的响应被拒。（`oauth2_email_test.go` 6 项：主邮箱已验证优先、跳过未验证主邮箱回退、全部未验证拒绝、畸形响应、Google verified_email 缺失/为 false 拒绝） +- 已完成（2026-08-28）。 ## P3 低危 / 加固 @@ -332,8 +334,8 @@ 1. ~~**#20**（先修删除退配额，数据一致性基础）~~ 已完成 2026-08-28 2. ~~**#19**（入站配额强制，依赖 #20）~~ 已完成 2026-08-28 -3. **#21**（Web 预扣泄漏） +3. ~~**#21**（Web 预扣泄漏）~~ 已完成 2026-08-28 4. #24、#23（一行校验类） -5. #22、#25 +5. ~~#22（OAuth2 verified）~~ 已完成 2026-08-28；#25 待修 6. #26 按需排期 7. #27（存量丢失更新，约 3 行改动，可顺手修）