-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat: add configurable sign-in attempts limit and IP allow/deny lists #2776
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 7 commits
ae83a3b
532fbdb
ed4f8cf
d1bdd74
1252c5f
8adaab1
cc16024
cd63e3b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,8 @@ import ( | |
| "encoding/binary" | ||
| "encoding/json" | ||
| "fmt" | ||
| "net" | ||
| "strings" | ||
| "time" | ||
|
|
||
| "github.com/OpenListTeam/OpenList/v4/internal/errs" | ||
|
|
@@ -32,10 +34,91 @@ const ( | |
|
|
||
| var LoginCache = cache.NewMemCache[int]() | ||
|
|
||
| var ( | ||
| DefaultLockDuration = time.Minute * 5 | ||
| DefaultMaxAuthRetries = 5 | ||
| ) | ||
| const DefaultLockDurationMinutes = 5 | ||
| const DefaultMaxAuthRetries = 5 | ||
|
|
||
| // CheckLoginLocked checks if the IP has exceeded the max retry limit. | ||
| // If locked, it refreshes the lock expiry and returns true. | ||
| // maxRetries <= 0 means the lock feature is disabled. | ||
| func CheckLoginLocked(ip string, maxRetries int, lockDuration time.Duration) bool { | ||
| if maxRetries <= 0 { | ||
| return false | ||
| } | ||
| count, ok := LoginCache.Get(ip) | ||
| if ok && count >= maxRetries { | ||
| LoginCache.Expire(ip, lockDuration) | ||
| return true | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| // RecordLoginAttempt records a failed login attempt for the IP and returns the new count. | ||
| func RecordLoginAttempt(ip string) int { | ||
| count, _ := LoginCache.Get(ip) | ||
| LoginCache.Set(ip, count+1) | ||
| return count + 1 | ||
|
Comment on lines
+55
to
+59
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Useful? React with 👍 / 👎. |
||
| } | ||
|
|
||
| // ClearLoginAttempts clears the login attempt counter for the given IP. | ||
| func ClearLoginAttempts(ip string) { | ||
| LoginCache.Del(ip) | ||
| } | ||
|
|
||
| // IsIPWhitelisted returns true if the given IP matches any entry in the whitelist. | ||
| // The whitelist supports single IPs and CIDR notation (e.g., 192.168.1.0/24). | ||
| func IsIPWhitelisted(ip string, whitelist []string) bool { | ||
| return matchIPList(ip, whitelist) | ||
| } | ||
|
|
||
| // IsIPBlacklisted returns true if the given IP matches any entry in the blacklist. | ||
| // The blacklist supports single IPs and CIDR notation (e.g., 10.0.0.0/8). | ||
| func IsIPBlacklisted(ip string, blacklist []string) bool { | ||
| return matchIPList(ip, blacklist) | ||
| } | ||
|
|
||
| // matchIPList checks if the given IP matches any entry in the list. | ||
| func matchIPList(ip string, list []string) bool { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 这里直接用字符串比较可能不太合适:前面解析 建议将 |
||
| if len(list) == 0 { | ||
| return false | ||
| } | ||
| parsedIP := net.ParseIP(strings.TrimSpace(ip)) | ||
| if parsedIP == nil { | ||
| return false | ||
| } | ||
| for _, entry := range list { | ||
| entry = strings.TrimSpace(entry) | ||
| if entry == "" { | ||
| continue | ||
| } | ||
| if strings.Contains(entry, "/") { | ||
| _, cidr, err := net.ParseCIDR(entry) | ||
| if err == nil && cidr.Contains(parsedIP) { | ||
| return true | ||
| } | ||
| } else { | ||
| if entry == ip { | ||
| return true | ||
| } | ||
| } | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| // ParseIPList parses a newline-separated string into a slice of IP/CIDR entries. | ||
| func ParseIPList(raw string) []string { | ||
| if raw == "" { | ||
| return nil | ||
| } | ||
| lines := strings.Split(raw, "\n") | ||
| var result []string | ||
| for _, line := range lines { | ||
| line = strings.TrimSpace(line) | ||
| if line != "" { | ||
| result = append(result, line) | ||
| } | ||
| } | ||
| return result | ||
| } | ||
|
|
||
| type User struct { | ||
| ID uint `json:"id" gorm:"primaryKey"` // unique key | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ import ( | |
| "strconv" | ||
| "strings" | ||
| "sync" | ||
| "time" | ||
|
|
||
| "github.com/OpenListTeam/OpenList/v4/drivers/base" | ||
| "github.com/OpenListTeam/OpenList/v4/internal/conf" | ||
|
|
@@ -114,11 +115,20 @@ func (d *FtpMainDriver) ClientDisconnected(cc ftpserver.ClientContext) { | |
|
|
||
| func (d *FtpMainDriver) AuthUser(cc ftpserver.ClientContext, user, pass string) (ftpserver.ClientDriver, error) { | ||
| ip := cc.RemoteAddr().String() | ||
| count, ok := model.LoginCache.Get(ip) | ||
| if ok && count >= model.DefaultMaxAuthRetries { | ||
| model.LoginCache.Expire(ip, model.DefaultLockDuration) | ||
| return nil, errors.New("Too many unsuccessful sign-in attempts have been made using an incorrect username or password, Try again later.") | ||
| maxRetries := setting.GetInt(conf.LoginMaxRetries, model.DefaultMaxAuthRetries) | ||
| lockDuration := time.Duration(setting.GetInt(conf.LoginLockDuration, model.DefaultLockDurationMinutes)) * time.Minute | ||
|
|
||
| // check IP blacklist | ||
| if model.IsIPBlacklisted(ip, model.ParseIPList(setting.GetStr(conf.LoginIPBlacklist))) { | ||
| return nil, errors.New(model.TooManyAttempts) | ||
|
Comment on lines
+121
to
+123
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For FTP clients, Useful? React with 👍 / 👎. |
||
| } | ||
|
|
||
| // check IP whitelist (bypasses lock) | ||
| whitelist := model.ParseIPList(setting.GetStr(conf.LoginIPWhitelist)) | ||
| if !model.IsIPWhitelisted(ip, whitelist) && model.CheckLoginLocked(ip, maxRetries, lockDuration) { | ||
| return nil, errors.New(model.TooManyAttempts) | ||
| } | ||
|
|
||
| var userObj *model.User | ||
| var err error | ||
| if user == "anonymous" || user == "guest" { | ||
|
|
@@ -137,15 +147,19 @@ func (d *FtpMainDriver) AuthUser(cc ftpserver.ClientContext, user, pass string) | |
| userObj, err = tryLdapLoginAndRegister(user, pass) | ||
| } | ||
| if err != nil { | ||
| model.LoginCache.Set(ip, count+1) | ||
| if !model.IsIPWhitelisted(ip, whitelist) { | ||
| model.RecordLoginAttempt(ip) | ||
| } | ||
| return nil, err | ||
| } | ||
| } | ||
| if userObj.Disabled || !userObj.CanFTPAccess() { | ||
| model.LoginCache.Set(ip, count+1) | ||
| if !model.IsIPWhitelisted(ip, whitelist) { | ||
| model.RecordLoginAttempt(ip) | ||
| } | ||
| return nil, errors.New("user is not allowed to access via FTP") | ||
| } | ||
| model.LoginCache.Del(ip) | ||
| model.ClearLoginAttempts(ip) | ||
|
|
||
| ctx := context.Background() | ||
| ctx = context.WithValue(ctx, conf.UserKey, userObj) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
为了保证兼容性应该设置-1 但是给真正默认值确实更安全,这个需要团队内其他开发看下考虑下