From 6ee2dd8184c5bd91b59ae4299eb6097a155b0232 Mon Sep 17 00:00:00 2001 From: kevin Date: Thu, 20 Aug 2026 11:25:10 +0800 Subject: [PATCH] =?UTF-8?q?fix(security):=20=E4=BF=AE=E5=A4=8D=20P4=20?= =?UTF-8?q?=E5=B0=81=E7=A6=81=E8=AE=B0=E5=BD=95=E6=95=B0=E6=8D=AE=E9=94=99?= =?UTF-8?q?=E4=BD=8D=20+=20P5=20=E6=9E=9A=E4=B8=BE=E7=88=86=E7=A0=B4?= =?UTF-8?q?=E5=AE=BD=E9=99=90=EF=BC=88=E6=96=B9=E6=A1=88=20A=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P4 #17 手动封禁 Create 非 upsert 导致数据错位： - BanStore 新增 BanIP(ip, reason, duration)：事务内清理该 IP 全部 既有记录（兼容历史脏数据）后插入单条封禁记录，计数清零； DisconnectConnection 改用（原裸 Create 为全仓库唯一调用点） - BanEntry.IPAddress 升级 uniqueIndex；InitDB 在 AutoMigrate 前 dedupeBanEntries 清理历史重复行（MySQL 1093 兼容写法） - IncrementFail 原子化：SQL 侧 fail_count+1，miss 时 OnConflict DoNothing 插入兜底并发竞态，回读计数 P5 #18 方案 A（按失败性质区分宽限）： - RecordAuthFailure 新增 knownUser 参数：用户名不存在（枚举型 爆破）跳过 3 次宽限、首次触发即封第 1 档；用户名存在（真实 用户输错）保留宽限防误封 - 新增 UserStore.LoginExists（邮箱/裸用户名）；五个失败调用点 接线（Web 登录查存在性；SMTP/IMAP/POP3 用登录名；LDAP 侧 存在性不可判定，保守按已知用户处理） - 封禁原因注明「未知用户名，跳过宽限」便于后台审计 新增 8 个测试（-race 通过）：BanIP 单行 upsert、唯一索引约束、 16 协程并发计数精确、dedupe 清理/表不存在静默、未知用户即时 封禁、已知用户宽限回归、LoginExists 矩阵。 --- internal/db/db.go | 25 ++++ internal/db/dedupe_test.go | 84 ++++++++++++ internal/db/models.go | 6 +- internal/imap_server/session.go | 3 +- internal/pop3_server/server.go | 5 +- internal/smtp_server/server.go | 4 +- internal/store/auth_guard.go | 27 +++- internal/store/auth_guard_test.go | 204 +++++++++++++++++++++++++++++- internal/store/ban_store.go | 85 ++++++++++--- internal/store/user_store.go | 23 ++++ internal/web/handlers/admin.go | 12 +- internal/web/handlers/auth.go | 8 +- security_todo.md | 42 +++++- 13 files changed, 481 insertions(+), 47 deletions(-) create mode 100644 internal/db/dedupe_test.go diff --git a/internal/db/db.go b/internal/db/db.go index 7aab395..6a06cf6 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -2,6 +2,7 @@ package db import ( "fmt" + "log" "os" "path/filepath" "strings" @@ -55,6 +56,11 @@ func InitDB(cfg config.DatabaseConfig, storageCfg config.StorageConfig) (*gorm.D return nil, fmt.Errorf("连接数据库失败: %w", err) } + // AutoMigrate 前清理 ban_entries 的历史重复行（保留每 IP 最大 id）： + // ip_address 将升级为唯一索引，重复行会使索引创建失败。 + // 首次安装表不存在时忽略错误（AutoMigrate 会建新表）。 + dedupeBanEntries(db) + // Auto-migrate all models if err := db.AutoMigrate(&User{}, &Domain{}, &Message{}, &Attachment{}, &BanEntry{}, &OutboundMessage{}, &ProtocolLog{}, &MailboxState{}, &Mailbox{}); err != nil { return nil, fmt.Errorf("数据库迁移失败: %w", err) @@ -62,3 +68,22 @@ func InitDB(cfg config.DatabaseConfig, storageCfg config.StorageConfig) (*gorm.D return db, nil } + +// dedupeBanEntries 删除 ban_entries 中同一 ip_address 的重复行（保留 +// 每组 id 最大的一条），为 ip_address 唯一索引的 AutoMigrate 扫清障碍。 +// 表不存在（首次安装）时静默跳过；清理失败仅告警，不阻断启动 +// （索引创建失败会在 AutoMigrate 中显式报错）。 +func dedupeBanEntries(db *gorm.DB) { + // SQLite 与 MySQL 均支持；MySQL 不允许 DELETE 子查询直接引用同表 + // （1093），因此用派生表包一层。 + sql := "DELETE FROM ban_entries WHERE id NOT IN (" + + "SELECT mid FROM (SELECT MAX(id) AS mid FROM ban_entries GROUP BY ip_address) AS t)" + if err := db.Exec(sql).Error; err != nil { + // 表不存在（首次安装）为预期情况 + msg := err.Error() + if strings.Contains(msg, "no such table") || strings.Contains(msg, "doesn't exist") { + return + } + log.Printf("清理 ban_entries 重复行失败（唯一索引可能无法创建）: %v", err) + } +} diff --git a/internal/db/dedupe_test.go b/internal/db/dedupe_test.go new file mode 100644 index 0000000..2cc9a95 --- /dev/null +++ b/internal/db/dedupe_test.go @@ -0,0 +1,84 @@ +package db + +// P4 #17 回归：dedupeBanEntries 在 AutoMigrate 前清理历史重复行， +// 为 ip_address 唯一索引扫清障碍；表不存在时静默跳过。 + +import ( + "path/filepath" + "testing" + + "gorm.io/driver/sqlite" + "gorm.io/gorm" +) + +// createLegacyBanTable 按旧版结构建表（ip_address 无唯一索引）。 +func createLegacyBanTable(t *testing.T) *gorm.DB { + t.Helper() + gdb, err := gorm.Open(sqlite.Open(filepath.Join(t.TempDir(), "test.db")), &gorm.Config{}) + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + if err := gdb.Exec(`CREATE TABLE ban_entries ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + ip_address TEXT NOT NULL, + reason TEXT, + fail_count INTEGER DEFAULT 0, + ban_count INTEGER DEFAULT 0, + expires_at DATETIME, + created_at DATETIME, + updated_at DATETIME + )`).Error; err != nil { + t.Fatalf("create legacy table: %v", err) + } + return gdb +} + +func TestDedupeBanEntriesRemovesDuplicates(t *testing.T) { + gdb := createLegacyBanTable(t) + + // 同一 IP 三条旧记录（id 1,2,3），另一 IP 一条 + for _, rec := range []struct { + ip string + fc int + }{ + {"1.2.3.4", 1}, + {"1.2.3.4", 2}, + {"1.2.3.4", 3}, + {"5.6.7.8", 4}, + } { + if err := gdb.Exec( + "INSERT INTO ban_entries (ip_address, fail_count) VALUES (?, ?)", rec.ip, rec.fc, + ).Error; err != nil { + t.Fatalf("insert: %v", err) + } + } + + dedupeBanEntries(gdb) + + // 每 IP 仅剩 id 最大的一条 + var rows []struct { + IP string `gorm:"column:ip_address"` + FC int `gorm:"column:fail_count"` + } + if err := gdb.Raw("SELECT ip_address, fail_count FROM ban_entries ORDER BY ip_address").Scan(&rows).Error; err != nil { + t.Fatalf("query: %v", err) + } + if len(rows) != 2 { + t.Fatalf("rows after dedupe = %d, want 2", len(rows)) + } + if rows[0].IP != "1.2.3.4" || rows[0].FC != 3 { + t.Fatalf("1.2.3.4 row = %+v, want the max-id row (fail_count=3)", rows[0]) + } + if rows[1].IP != "5.6.7.8" || rows[1].FC != 4 { + t.Fatalf("5.6.7.8 row = %+v, want fail_count=4", rows[1]) + } +} + +func TestDedupeBanEntriesMissingTableSilent(t *testing.T) { + gdb, err := gorm.Open(sqlite.Open(filepath.Join(t.TempDir(), "test.db")), &gorm.Config{}) + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + // 表不存在时不应 panic/报错（首次安装场景） + dedupeBanEntries(gdb) +} diff --git a/internal/db/models.go b/internal/db/models.go index ca6b725..f00f300 100644 --- a/internal/db/models.go +++ b/internal/db/models.go @@ -115,8 +115,10 @@ func (OutboundMessage) TableName() string { // BanEntry represents an IP address that has been banned due to excessive login failures. type BanEntry struct { - ID uint `gorm:"primaryKey" json:"id"` - IPAddress string `gorm:"size:45;index;not null" json:"ip_address"` + ID uint `gorm:"primaryKey" json:"id"` + // IPAddress 唯一索引：每 IP 恰好一条记录（观察计数与封禁状态共用）， + // 防止并发写入产生重复行导致计数与档位读写错位。 + IPAddress string `gorm:"size:45;uniqueIndex;not null" json:"ip_address"` Reason string `gorm:"size:255" json:"reason"` FailCount int `gorm:"default:0" json:"fail_count"` // BanCount 是该 IP 累计达到失败阈值的次数（含未封禁的前几次）。 diff --git a/internal/imap_server/session.go b/internal/imap_server/session.go index 899794d..802d650 100644 --- a/internal/imap_server/session.go +++ b/internal/imap_server/session.go @@ -220,7 +220,8 @@ func (s *imapSession) Login(username, password string) error { user, err := s.srv.stores.Users.AuthenticateLogin(username, password) if err != nil { - s.srv.stores.RecordAuthFailure(clientIP, s.srv.banCfg.MaxFailAttempts, s.srv.banCfg.BanDurationMin, "邮件协议认证失败次数过多") + // 用户名不存在（枚举型爆破）跳过宽限首次触发即封 + s.srv.stores.RecordAuthFailure(clientIP, s.srv.banCfg.MaxFailAttempts, s.srv.banCfg.BanDurationMin, "邮件协议认证失败次数过多", s.srv.stores.Users.LoginExists(username)) s.recordLogin(clientIP, username, false, "用户名或密码错误", "LOGIN 失败", now) return imapserver.ErrAuthFailed } diff --git a/internal/pop3_server/server.go b/internal/pop3_server/server.go index dea5dd0..ff72482 100644 --- a/internal/pop3_server/server.go +++ b/internal/pop3_server/server.go @@ -397,8 +397,9 @@ func (s *POP3Server) handlePASS(conn net.Conn, password string, user *db.User) ( authUser, err := s.stores.Users.AuthenticateLogin(user.Username, password) if err != nil { - // 认证失败计数，达到阈值按档位封禁（与 Web 登录共用 ban_entries） - s.stores.RecordAuthFailure(clientIP, s.banCfg.MaxFailAttempts, s.banCfg.BanDurationMin, "邮件协议认证失败次数过多") + // 认证失败计数，达到阈值按档位封禁（与 Web 登录共用 ban_entries）。 + // 用户名不存在（枚举型爆破）跳过宽限首次触发即封。 + s.stores.RecordAuthFailure(clientIP, s.banCfg.MaxFailAttempts, s.banCfg.BanDurationMin, "邮件协议认证失败次数过多", s.stores.Users.LoginExists(user.Username)) sendResponse(conn, "-ERR authentication failed") return nil, nil, nil } diff --git a/internal/smtp_server/server.go b/internal/smtp_server/server.go index 7591e6e..fc5eef5 100644 --- a/internal/smtp_server/server.go +++ b/internal/smtp_server/server.go @@ -221,12 +221,14 @@ func (s *smtpSession) Auth(mech string) (sasl.Server, error) { user, err := s.backend.server.stores.Users.AuthenticateLogin(username, password) if err != nil { - // 认证失败计数，达到阈值按档位封禁（与 Web 登录共用 ban_entries） + // 认证失败计数，达到阈值按档位封禁（与 Web 登录共用 ban_entries）。 + // 用户名不存在（枚举型爆破）跳过宽限首次触发即封。 s.backend.server.stores.RecordAuthFailure( s.clientIP, s.backend.server.banCfg.MaxFailAttempts, s.backend.server.banCfg.BanDurationMin, "邮件协议认证失败次数过多", + s.backend.server.stores.Users.LoginExists(username), ) s.recordFail("用户名或密码错误") return smtp.ErrAuthFailed diff --git a/internal/store/auth_guard.go b/internal/store/auth_guard.go index ecc3bae..ad4d1e6 100644 --- a/internal/store/auth_guard.go +++ b/internal/store/auth_guard.go @@ -23,12 +23,15 @@ func ClientIPFromAddr(addr net.Addr) string { // 协议层统一入口）： // - 失败计数累加（每 IP 一条记录，upsert）； // - 达到 maxFail 阈值时触发次数 BanCount+1： -// 前 freeTriggers（3）次只计数不封禁； -// 从第 4 次起封禁，时长按档位递增（stageDuration），上限半年； +// knownUser（用户名存在、疑似真实用户输错）前 freeTriggers（3）次 +// 只计数不封禁（防误封）； +// !knownUser（用户名不存在，枚举型爆破）跳过宽限，首次触发即按 +// 第 1 档封禁； +// 封禁时长按档位递增（stageDuration），上限半年； // - reason 为失败场景描述（如“登录失败次数过多”），封禁原因会带上档位。 // // 返回 (是否触发封禁, 当前失败计数)。成功登录后调用 ResetFail 清零。 -func (s *Stores) RecordAuthFailure(ip string, maxFail int, firstBanMin int, reason string) (banned bool, failCount int) { +func (s *Stores) RecordAuthFailure(ip string, maxFail int, firstBanMin int, reason string, knownUser bool) (banned bool, failCount int) { if ip == "" || maxFail <= 0 { return false, 0 } @@ -50,16 +53,26 @@ func (s *Stores) RecordAuthFailure(ip string, maxFail int, firstBanMin int, reas entry.BanCount = banCount entry.FailCount = failCount - // 前 3 次只计数，不封禁（保留零到期时间与空原因） - if banCount <= freeTriggers { + // 未知用户名（枚举型爆破）跳过宽限档：等效于已用完 3 次宽限 + effectiveCount := banCount + if !knownUser { + effectiveCount += freeTriggers + } + + // 宽限期内只计数，不封禁（保留零到期时间与空原因） + if effectiveCount <= freeTriggers { if err := s.Bans.Update(entry); err != nil { return false, failCount } return false, failCount } - banNum := banCount - freeTriggers - entry.Reason = fmt.Sprintf("第%d次封禁：%s（第%d次触发，失败%d次）", banNum, reason, banCount, failCount) + banNum := effectiveCount - freeTriggers + if knownUser { + entry.Reason = fmt.Sprintf("第%d次封禁：%s（第%d次触发，失败%d次）", banNum, reason, banCount, failCount) + } else { + entry.Reason = fmt.Sprintf("第%d次封禁：%s（未知用户名，跳过宽限；第%d次触发，失败%d次）", banNum, reason, banCount, failCount) + } entry.ExpiresAt = time.Now().Add(stageDuration(banNum, firstBanMin)) if err := s.Bans.Update(entry); err != nil { return false, failCount diff --git a/internal/store/auth_guard_test.go b/internal/store/auth_guard_test.go index 15fe47f..d1cade7 100644 --- a/internal/store/auth_guard_test.go +++ b/internal/store/auth_guard_test.go @@ -1,6 +1,7 @@ package store import ( + "sync" "net" "path/filepath" "strings" @@ -33,7 +34,7 @@ func TestRecordAuthFailureFreeTriggers(t *testing.T) { const maxFail = 2 failOnce := func() bool { - banned, _ := s.RecordAuthFailure(ip, maxFail, 30, "登录失败次数过多") + banned, _ := s.RecordAuthFailure(ip, maxFail, 30, "登录失败次数过多", true) return banned } @@ -94,7 +95,7 @@ func TestStagedBanEscalation(t *testing.T) { const maxFail = 2 failOnce := func() bool { - banned, _ := s.RecordAuthFailure(ip, maxFail, 30, "登录失败次数过多") + banned, _ := s.RecordAuthFailure(ip, maxFail, 30, "登录失败次数过多", true) return banned } @@ -228,7 +229,7 @@ func TestBanListOnlyBannedOrExpired(t *testing.T) { // TestRecordAuthFailureEmptyIPSafe 空 IP 不应产生副作用。 func TestRecordAuthFailureEmptyIPSafe(t *testing.T) { s := newTestStores(t) - banned, count := s.RecordAuthFailure("", 3, 30, "登录失败次数过多") + banned, count := s.RecordAuthFailure("", 3, 30, "登录失败次数过多", true) if banned || count != 0 { t.Fatalf("empty IP must be a no-op: banned=%v count=%d", banned, count) } @@ -350,3 +351,200 @@ func TestTryReserveQuotaNonPositiveDelta(t *testing.T) { t.Fatalf("used_bytes = %d, want 0", got.UsedBytes) } } + +// P5 #18 方案 A：未知用户名（枚举型爆破）跳过宽限，首次触发即封。 +func TestRecordAuthFailureUnknownUserSkipsGrace(t *testing.T) { + s := newTestStores(t) + const ip = "203.0.113.77" + const maxFail = 3 + + // 未知用户名：第 1 次触发（累计失败 3 次）即封第 1 档（30 分钟） + for i := 1; i <= maxFail; i++ { + banned, _ := s.RecordAuthFailure(ip, maxFail, 30, "登录失败次数过多", false) + if i < maxFail && banned { + t.Fatalf("attempt %d should not ban before threshold", i) + } + if i == maxFail && !banned { + t.Fatal("unknown user: first trigger must ban immediately") + } + } + banned, entry := s.Bans.IsBanned(ip) + if !banned { + t.Fatal("IP should be banned") + } + // 第 1 档 = 30 分钟 + if entry.ExpiresAt.Before(time.Now().Add(29 * time.Minute)) { + t.Fatalf("first-stage ban duration wrong: expires %v", entry.ExpiresAt) + } + if !strings.Contains(entry.Reason, "未知用户名") { + t.Fatalf("reason should note unknown-user skip: %q", entry.Reason) + } +} + +// P5 #18 方案 A：已知用户名（真实用户输错）保留前 3 次宽限（回归）。 +// 触发语义与 TestRecordAuthFailureFreeTriggers 一致：达到阈值后每次失败 +// 都会触发一次，前 3 次触发（第 2-4 次失败）不封禁，第 4 次触发 +// （第 5 次失败）封第 1 档。 +func TestRecordAuthFailureKnownUserKeepsGrace(t *testing.T) { + s := newTestStores(t) + const ip = "198.51.100.88" + const maxFail = 2 + + // 第 1 次失败：计数，未达阈值 + if banned, _ := s.RecordAuthFailure(ip, maxFail, 30, "登录失败次数过多", true); banned { + t.Fatal("failure 1 must not ban") + } + // 第 2-4 次失败 = 触发 1-3，宽限期内不封禁 + for i := 2; i <= 4; i++ { + banned, _ := s.RecordAuthFailure(ip, maxFail, 30, "登录失败次数过多", true) + if banned { + t.Fatalf("failure %d (trigger within grace) should not ban", i) + } + } + if banned, _ := s.Bans.IsBanned(ip); banned { + t.Fatal("known user must not be banned within 3 free triggers") + } + entry, _ := s.Bans.GetByIP(ip) + if entry.BanCount != 3 { + t.Fatalf("ban_count = %d, want 3", entry.BanCount) + } + + // 第 5 次失败 = 触发 4 -> 第 1 档封禁 + banned, _ := s.RecordAuthFailure(ip, maxFail, 30, "登录失败次数过多", true) + if !banned { + t.Fatal("4th trigger should ban (stage 1)") + } + entry, _ = s.Bans.GetByIP(ip) + if entry.BanCount != 4 { + t.Fatalf("ban count = %d, want 4", entry.BanCount) + } + if !strings.Contains(entry.Reason, "第1次封禁") { + t.Fatalf("reason = %q, want 第1次封禁", entry.Reason) + } +} + +// LoginExists：完整邮箱与裸用户名两种形态。 +func TestLoginExists(t *testing.T) { + s := newTestStores(t) + domain := &db.Domain{Name: "example.com"} + if err := s.Domains.Create(domain); err != nil { + t.Fatal(err) + } + if err := s.Users.Create(&db.User{Username: "alice", PasswordHash: "x", DomainID: domain.ID}); err != nil { + t.Fatal(err) + } + + cases := []struct { + login string + want bool + }{ + {"alice@example.com", true}, + {"alice", true}, + {"bob@example.com", false}, + {"bob", false}, + {"", false}, + } + for _, tc := range cases { + if got := s.Users.LoginExists(tc.login); got != tc.want { + t.Errorf("LoginExists(%q) = %v, want %v", tc.login, got, tc.want) + } + } +} + +// P4 #17：BanIP 为 upsert 语义，已有观察记录的 IP 手动封禁后仅一条记录。 +func TestBanIPUpsertSingleRow(t *testing.T) { + s := newTestStores(t) + const ip = "203.0.113.99" + + // 先产生观察计数记录（未封禁） + for i := 0; i < 2; i++ { + _, _ = s.RecordAuthFailure(ip, 10, 30, "登录失败次数过多", true) + } + if banned, _ := s.Bans.IsBanned(ip); banned { + t.Fatal("should be observation-only at this point") + } + + // 手动封禁 180 天 + if err := s.Bans.BanIP(ip, "管理员手动封禁", 180*24*time.Hour); err != nil { + t.Fatalf("BanIP: %v", err) + } + + // 仅一条记录，且处于封禁状态、计数清零 + entry, err := s.Bans.GetByIP(ip) + if err != nil { + t.Fatalf("GetByIP: %v", err) + } + if entry.Reason != "管理员手动封禁" { + t.Fatalf("reason = %q", entry.Reason) + } + if entry.FailCount != 0 || entry.BanCount != 0 { + t.Fatalf("manual ban should reset counters, got fail=%d ban=%d", entry.FailCount, entry.BanCount) + } + if !entry.ExpiresAt.After(time.Now().Add(179 * 24 * time.Hour)) { + t.Fatalf("manual ban duration wrong: %v", entry.ExpiresAt) + } +} + +// P4 #17：ip_address 唯一索引生效——同一 IP 不允许第二条记录 +// （历史重复行由 InitDB 的 dedupeBanEntries 在 AutoMigrate 前清理， +// BanIP 的事务内“先删后插”兼容既有脏数据）。 +func TestBanEntryUniqueIndex(t *testing.T) { + s := newTestStores(t) + const ip = "198.51.100.3" + + if err := s.Bans.Create(&db.BanEntry{IPAddress: ip, Reason: "first", ExpiresAt: time.Time{}}); err != nil { + t.Fatal(err) + } + // 第二条同 IP 记录必须被唯一约束拒绝 + if err := s.Bans.Create(&db.BanEntry{IPAddress: ip, Reason: "second", ExpiresAt: time.Time{}}); err == nil { + t.Fatal("duplicate ban entry for same IP should be rejected by unique index") + } + + // 唯一记录上的 BanIP/IncrementFail 读写一致（无错位） + if err := s.Bans.BanIP(ip, "管理员手动封禁", time.Hour); err != nil { + t.Fatalf("BanIP: %v", err) + } + entry, err := s.Bans.GetByIP(ip) + if err != nil { + t.Fatal(err) + } + if entry.Reason != "管理员手动封禁" || entry.FailCount != 0 { + t.Fatalf("unexpected entry: %+v", entry) + } + cnt, err := s.Bans.IncrementFail(ip) + if err != nil || cnt != 1 { + t.Fatalf("IncrementFail after BanIP = %d, %v; want 1, nil", cnt, err) + } +} + +// P4 #17：IncrementFail 并发安全（-race 下不产生重复行、计数准确）。 +func TestIncrementFailConcurrent(t *testing.T) { + s := newTestStores(t) + const ip = "203.0.113.100" + const goroutines = 16 + const perG = 5 + + var wg sync.WaitGroup + for g := 0; g < goroutines; g++ { + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; i < perG; i++ { + if _, err := s.Bans.IncrementFail(ip); err != nil { + t.Errorf("IncrementFail: %v", err) + return + } + } + }() + } + wg.Wait() + + entry, err := s.Bans.GetByIP(ip) + if err != nil { + t.Fatalf("GetByIP: %v", err) + } + want := goroutines * perG + if entry.FailCount != want { + t.Fatalf("fail count = %d, want %d (lost updates or duplicate rows)", entry.FailCount, want) + } +} diff --git a/internal/store/ban_store.go b/internal/store/ban_store.go index 9df25a9..dcf1e48 100644 --- a/internal/store/ban_store.go +++ b/internal/store/ban_store.go @@ -1,11 +1,14 @@ package store import ( + "fmt" + "strings" "time" "mail_go/internal/db" "gorm.io/gorm" + "gorm.io/gorm/clause" ) // 阶段封禁档位（分钟/天），从第 4 次触发阈值开始封禁： @@ -41,6 +44,10 @@ func stageDuration(banCount int, firstBanMin int) time.Duration { // BanStore defines the interface for IP ban operations. type BanStore interface { Create(entry *db.BanEntry) error + // BanIP 手动/直接封禁某 IP 指定时长：该 IP 只保留一条记录 + // （既有观察记录一并清理，计数清零），避免产生重复行导致 + // IncrementFail 与 GetByIP 读写错位。 + BanIP(ip, reason string, duration time.Duration) error GetByIP(ip string) (*db.BanEntry, error) Update(entry *db.BanEntry) error Delete(id uint) error @@ -67,6 +74,30 @@ func (s *banStoreGorm) Create(entry *db.BanEntry) error { return s.db.Create(entry).Error } +// BanIP 手动/直接封禁：事务内删除该 IP 的全部既有记录（含历史 bug +// 产生的重复行与观察计数记录）后插入一条封禁记录，计数清零。 +// 与"管理员解封清零"语义一致：手动封禁视为对档位的重新评估。 +func (s *banStoreGorm) BanIP(ip, reason string, duration time.Duration) error { + if ip == "" { + return fmt.Errorf("empty ip") + } + if duration <= 0 { + return fmt.Errorf("invalid ban duration: %v", duration) + } + return s.db.Transaction(func(tx *gorm.DB) error { + if err := tx.Where("ip_address = ?", ip).Delete(&db.BanEntry{}).Error; err != nil { + return err + } + return tx.Create(&db.BanEntry{ + IPAddress: ip, + Reason: reason, + FailCount: 0, + BanCount: 0, + ExpiresAt: time.Now().Add(duration), + }).Error + }) +} + // GetByIP retrieves the most recent ban entry for a given IP address. func (s *banStoreGorm) GetByIP(ip string) (*db.BanEntry, error) { var entry db.BanEntry @@ -118,33 +149,51 @@ func (s *banStoreGorm) IsBanned(ip string) (bool, *db.BanEntry) { return true, &entry } -// IncrementFail increments the fail count for an IP address. -// If no record exists, it creates one with fail_count=1, ban_count=0 and a -// zero expires_at (not yet banned). Existing BanCount is preserved. -// Returns the updated fail count. +// IncrementFail increments the fail count for an IP address atomically +// (SQL-side increment, avoiding read-modify-write races). If no record +// exists it creates one with fail_count=1, ban_count=0 and a zero +// expires_at (not yet banned); a concurrent creator wins and the loser's +// insert becomes a no-op via the unique index. Existing BanCount is +// preserved. Returns the current fail count. func (s *banStoreGorm) IncrementFail(ip string) (int, error) { - var entry db.BanEntry - err := s.db.Where("ip_address = ?", ip).First(&entry).Error - if err != nil { - // No record exists, create a new one - entry = db.BanEntry{ + res := s.db.Model(&db.BanEntry{}). + Where("ip_address = ?", ip). + Update("fail_count", gorm.Expr("fail_count + 1")) + if res.Error != nil { + return 0, res.Error + } + if res.RowsAffected == 0 { + // 无记录：插入首条；ip_address 唯一索引下并发插入用 + // OnConflict DoNothing 兜底，失败方继续走下面的回读。 + err := s.db.Clauses(clause.OnConflict{DoNothing: true}).Create(&db.BanEntry{ IPAddress: ip, FailCount: 1, BanCount: 0, ExpiresAt: time.Time{}, // Zero time, not yet banned + }).Error + if err != nil && !isUniqueConflictErr(err) { + return 0, err } - if createErr := s.db.Create(&entry).Error; createErr != nil { - return 0, createErr - } - return 1, nil } - // Record exists, increment fail count - newCount := entry.FailCount + 1 - if updateErr := s.db.Model(&entry).Update("fail_count", newCount).Error; updateErr != nil { - return 0, updateErr + // 回读计数（并发下取数据库最终值） + var count int64 + if err := s.db.Model(&db.BanEntry{}).Where("ip_address = ?", ip). + Select("fail_count").Scan(&count).Error; err != nil { + return 0, err } - return newCount, nil + return int(count), nil +} + +// isUniqueConflictErr 判断是否为唯一约束冲突（并发插入竞态的预期结果）。 +func isUniqueConflictErr(err error) bool { + if err == nil { + return false + } + msg := err.Error() + return strings.Contains(msg, "UNIQUE constraint") || // SQLite + strings.Contains(msg, "Duplicate entry") || // MySQL + strings.Contains(msg, "unique constraint") // generic } // ResetFail resets the fail count for an IP address by deleting its record. diff --git a/internal/store/user_store.go b/internal/store/user_store.go index 13011ca..63c8639 100644 --- a/internal/store/user_store.go +++ b/internal/store/user_store.go @@ -20,6 +20,10 @@ type UserStore interface { // 但支持裸用户名（如 "kevin"），自动解析到其唯一所属域名；多域名下 // 用户名存在歧义时要求完整邮箱。兼容手机/客户端只填用户名的配置。 AuthenticateLogin(login, password string) (*db.User, error) + // LoginExists 判断登录名（完整邮箱或裸用户名）是否对应系统中的用户， + // 供封禁逻辑区分“真实用户输错密码”（保留宽限）与“枚举型爆破” + // （跳过宽限，见 RecordAuthFailure 的 knownUser 参数）。 + LoginExists(login string) bool Update(user *db.User) error Delete(id uint) error List(domainID uint, page, size int) ([]db.User, int64, error) @@ -127,6 +131,25 @@ func (s *userStoreGorm) AuthenticateLogin(login, password string) (*db.User, err return &user, nil } +// LoginExists 判断登录名是否对应系统中的用户：完整邮箱按邮箱查， +// 裸用户名按用户名全局查（存在即算，歧义不影响存在性判定）。 +// 仅用于封禁分级（knownUser），不做认证。 +func (s *userStoreGorm) LoginExists(login string) bool { + login = strings.TrimSpace(login) + if login == "" { + return false + } + if strings.Contains(login, "@") { + _, err := s.GetByEmail(login) + return err == nil + } + var count int64 + if err := s.db.Model(&db.User{}).Where("username = ?", login).Count(&count).Error; err != nil { + return false + } + return count > 0 +} + // Update saves changes to an existing user record. func (s *userStoreGorm) Update(user *db.User) error { return s.db.Save(user).Error diff --git a/internal/web/handlers/admin.go b/internal/web/handlers/admin.go index 3d67bca..2bce2be 100644 --- a/internal/web/handlers/admin.go +++ b/internal/web/handlers/admin.go @@ -79,14 +79,10 @@ func (h *AdminHandler) DisconnectConnection(c *gin.Context) { return } - // 加入黑名单：180 天封禁（管理员可随时解封） - if err := h.stores.Bans.Create(&db.BanEntry{ - IPAddress: conn.IP, - Reason: "管理员手动封禁（连接断开）", - FailCount: 0, - BanCount: 0, - ExpiresAt: time.Now().Add(manualBanDuration), - }); err != nil { + // 加入黑名单：180 天封禁（管理员可随时解封）。 + // BanIP 为 upsert 语义：清理该 IP 既有观察/重复记录后仅保留一条， + // 避免与阶段性封禁的计数/档位记录错位。 + if err := h.stores.Bans.BanIP(conn.IP, "管理员手动封禁（连接断开）", manualBanDuration); err != nil { c.String(http.StatusInternalServerError, "封禁失败: %v", err) return } diff --git a/internal/web/handlers/auth.go b/internal/web/handlers/auth.go index da7c459..a6d66a0 100644 --- a/internal/web/handlers/auth.go +++ b/internal/web/handlers/auth.go @@ -73,7 +73,10 @@ func (h *AuthHandler) DoLogin(c *gin.Context) { user, err := h.stores.Users.Authenticate(email, password) if err != nil { - banned, failCount := h.stores.RecordAuthFailure(ip, h.banCfg.MaxFailAttempts, h.banCfg.BanDurationMin, "登录失败次数过多") + // 区分失败性质：用户名存在（真实用户输错，保留宽限）vs + // 用户名不存在（枚举型爆破，跳过宽限首次触发即封） + knownUser := h.stores.Users.LoginExists(email) + banned, failCount := h.stores.RecordAuthFailure(ip, h.banCfg.MaxFailAttempts, h.banCfg.BanDurationMin, "登录失败次数过多", knownUser) if banned { entry, _ := h.stores.Bans.GetByIP(ip) c.HTML(http.StatusForbidden, "banned", gin.H{"entry": entry}) @@ -146,7 +149,8 @@ func (h *AuthHandler) LDAPLogin(c *gin.Context) { if err != nil { log.Printf("LDAP 认证失败: %v", err) - banned, failCount := h.stores.RecordAuthFailure(ip, h.banCfg.MaxFailAttempts, h.banCfg.BanDurationMin, "LDAP 登录失败次数过多") + // LDAP 侧用户存在性无法判定，保守按已知用户处理（保留宽限防误封） + banned, failCount := h.stores.RecordAuthFailure(ip, h.banCfg.MaxFailAttempts, h.banCfg.BanDurationMin, "LDAP 登录失败次数过多", true) if banned { entry, _ := h.stores.Bans.GetByIP(ip) c.HTML(http.StatusForbidden, "banned", gin.H{"entry": entry}) diff --git a/security_todo.md b/security_todo.md index d18c8fc..dbfc7cb 100644 --- a/security_todo.md +++ b/security_todo.md @@ -185,7 +185,43 @@ 2. ~~#2、#3、#4（P1）~~ 已完成 2026-08-19 3. ~~#5-#11（P2）~~ 已完成 2026-08-19 4. ~~#12-#16（P3）~~ 已完成 2026-08-19 +5. ~~#17（P4）、#18（P5，方案 A）~~ 已完成 2026-08-20 -**全部安全审计项已修复完成。** 剩余建议（非代码项）： -- 部署侧：Caddy 加固（可选，应用层已加安全头）、8080 端口保持仅本机可达、GitHub 仓库中 3 个 50MB+ 的 exe 文件建议改用 LFS 或删除 -- 线上验证：部署新版后检查登录/收件箱/管理页、协议认证封禁、邮件远程图片加载（CSP 影响） +## P4 低危：第二轮审计发现（2026-08-20，8ea4a62..37b4816） + +### 17. 手动封禁 Create 非 upsert，与阶段性封禁体系数据错位 + +- [x] 位置：`internal/web/handlers/admin.go`（`DisconnectConnection`）、`internal/store/ban_store.go`、`internal/db/models.go`、`internal/db/db.go` +- 现状：`f2493da` 阶段性封禁已改为"每 IP 一条记录 upsert"（`RecordAuthFailure` 内部 GetByIP + Update），但管理员"断开并封禁"仍直接 `Create`。`ip_address` 无唯一索引，当目标 IP 已有失败计数记录时会插入**第二条**记录，造成： + - `IncrementFail` 用 `First`（默认主键升序）更新**旧行**，`GetByIP` 用 `Order("id DESC")` 返回**新行** -> 自动封禁的档位判定（BanCount）与失败计数（FailCount）读写错行； + - `UnbanIP` 按 ID 删除一行后另一行仍在，可能出现"解封后仍被旧记录挡住/计数异常"。 +- 修复方案： + - [x] BanStore 新增 `BanIP(ip, reason, duration)`：事务内删除该 IP 全部既有记录（兼容历史脏数据）后插入单条封禁记录，计数清零（与"管理员解封清零"语义一致）；`DisconnectConnection` 改用该方法。 + - [x] `BanEntry.IPAddress` 升级为 `uniqueIndex`；`InitDB` 在 AutoMigrate 前调用 `dedupeBanEntries` 清理历史重复行（保留每 IP 最大 id，SQLite/MySQL 兼容的派生表写法），表不存在时静默。 + - [x] `IncrementFail` 原子化：SQL 侧 `fail_count + 1`，miss 时 `OnConflict DoNothing` 插入兜底并发竞态，回读计数。 +- 验证： + - [x] 单测：已有观察记录的 IP 手动封禁后仅一条、计数清零、封禁生效（`TestBanIPUpsertSingleRow`）。 + - [x] 单测：同 IP 第二条记录被唯一约束拒绝（`TestBanEntryUniqueIndex`）。 + - [x] 并发单测（`-race`）：16 协程并发 IncrementFail 计数精确无重复行（`TestIncrementFailConcurrent`）。 + - [x] db 包单测：旧表重复行清理保留最大 id、表不存在静默（`dedupe_test.go`）。 + +## P5 备注：产品权衡项（需决策后实施） + +### 18. 阶段性封禁"前 3 次触发不封禁"降低爆破门槛 + +- [x] 位置：`internal/store/auth_guard.go`（`RecordAuthFailure`）、`internal/store/user_store.go`（`LoginExists`）、Web/LDAP/SMTP/IMAP/POP3 五处调用点 +- 现状：为防误封手机客户端（配置向导探测、裸用户名重试等），达到失败阈值记为一次触发，前 3 次**只计数不封禁**。副作用：攻击者每次触发前可"免费"尝试 `max_fail_attempts`（默认 5）次，即约 **15 次失败尝试零封禁**；第 4 次起才进入 30min -> 3h -> 3 个月 -> 半年的递增档位。长期防护足够，但自动化爆破的起步门槛降低。 +- 已实施（方案 A，2026-08-20）： + - [x] `RecordAuthFailure` 新增 `knownUser bool` 参数：用户名存在（真实用户输错）保留前 3 次宽限；用户名不存在（枚举型爆破）跳过宽限、首次触发即按第 1 档封禁，封禁原因注明"未知用户名，跳过宽限"。 + - [x] 新增 `UserStore.LoginExists(login)`（完整邮箱或裸用户名），五个失败调用点按场景传入：Web 登录查邮箱存在性；SMTP/IMAP/POP3 用登录名查；LDAP 侧存在性无法判定，保守按已知用户处理（防误封）。 +- 验证： + - [x] A：未知用户名第 1 次触发即封（30 分钟，reason 含"未知用户名"）（`TestRecordAuthFailureUnknownUserSkipsGrace`）。 + - [x] 已知用户名前 3 次触发不封、第 4 次封第 1 档（回归）（`TestRecordAuthFailureKnownUserKeepsGrace`）。 + - [x] `LoginExists` 邮箱/裸用户名/不存在/空输入矩阵（`TestLoginExists`）。 +- 决策记录：**方案 A**（按失败性质区分宽限：真实用户防误封，枚举爆破即时封禁）——在不改变正常用户体验的前提下，让针对不存在账号的字典爆破首次达到阈值即被封，兼顾误封防护与爆破门槛。 + +## 部署侧建议（非代码项） + +- Caddy 加固（可选，应用层已加安全头）、8080 端口保持仅本机可达。 +- GitHub 仓库中 3 个 50MB+ 的 exe 文件（mailgo.exe / mail_go.exe / mailgo_qa.exe）建议改用 Git LFS 或从历史中删除。 +- 线上验证：部署新版后检查登录/收件箱/管理页、协议认证封禁、邮件远程图片加载（CSP 影响）。