Skip to content

fix: use constant time in user lookups #1002

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.

Sign up for GitHub

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

Closed
steveiliop56 wants to merge 1 commit into from
Closed

fix: use constant time in user lookups #1002

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 39 additions & 15 deletions internal/controller/user_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,21 +89,30 @@ func (controller *UserController) loginHandler(c *gin.Context) {
search, err := controller.auth.SearchUser(req.Username)

if err != nil {
if errors.Is(err, service.ErrUserNotFound) {
controller.log.App.Warn().Str("username", req.Username).Msg("User not found during login attempt")
controller.auth.RecordLoginAttempt(req.Username, false)
controller.log.AuditLoginFailure(req.Username, "unknown", c.ClientIP(), "user not found")
c.JSON(401, gin.H{
"status": 401,
"message": "Unauthorized",
})
return
}
controller.log.App.Error().Err(err).Str("username", req.Username).Msg("Error searching for user during login attempt")
c.JSON(500, gin.H{
"status": 500,
"message": "Internal Server Error",
})
controller.constantTime(func() constantTimeRes {
if errors.Is(err, service.ErrUserNotFound) {
controller.log.App.Warn().Str("username", req.Username).Msg("User not found during login attempt")
controller.auth.RecordLoginAttempt(req.Username, false)
controller.log.AuditLoginFailure(req.Username, "unknown", c.ClientIP(), "user not found")
return constantTimeRes{
Code: 401,
Res: gin.H{
"status": 401,
"message": "Unauthorized",
},
}
}
controller.log.App.Error().Err(err).Str("username", req.Username).Msg("Error searching for user during login attempt")
return constantTimeRes{
Code: 500,
Res: gin.H{
"status": 500,
"message": "Internal Server Error",
},
}
}, func(res constantTimeRes) {
c.JSON(res.Code, res.Res)
}, time.Millisecond*45)
Comment on lines +113 to +115

@coderabbitai coderabbitai Bot Jul 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Hardcoded timing delay is brittle against user enumeration.

While this PR aims to prevent user enumeration via a constant-time response, hardcoding 45ms is brittle. The actual time taken by CheckUserPassword (e.g., bcrypt hashing or LDAP binding) will vary significantly across different environments, hardware specifications, and server loads, or if password work-factors are updated in the future. Consequently, an attacker can still enumerate users by distinguishing between this fixed 45ms sleep and the actual, variable password verification time on the server.

To reliably mask the timing difference, consider performing a dummy password verification (e.g., computing a dummy hash with the same work factor or making a dummy LDAP bind) for the ErrUserNotFound case instead of using a hardcoded time.Sleep. If a sleep must be used, consider calibrating it dynamically based on the moving average of actual verification times.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/controller/user_controller.go` around lines 113 - 115, Replace the
hardcoded time.Millisecond*45 delay in the constant-time response callback with
dummy password verification using the same work factor, or an equivalent dummy
LDAP bind, when CheckUserPassword reports ErrUserNotFound. Preserve the existing
c.JSON response behavior and ensure the missing-user path performs comparable
verification work without relying on a fixed sleep.

return
}

Expand Down Expand Up @@ -466,3 +475,18 @@ func (controller *UserController) tailscaleHandler(c *gin.Context) {
"message": "Login successful",
})
}

type constantTimeRes struct {
Code int
Res any
}

func (controller *UserController) constantTime(f func() constantTimeRes, rf func(res constantTimeRes), targetTime time.Duration) {
tStart := time.Now()
res := f()
tEnd := time.Now()
if tEnd.Sub(tStart) < targetTime {
time.Sleep(targetTime - tEnd.Sub(tStart))
}
rf(res)
}