fix: use constant time in user checks (#1004)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

.github/workflows/nightly.yml

@@ -23,7 +23,7 @@ jobs:
           REPO: ${{ github.event.repository.name }}
 
       - name: Create release
-        uses: softprops/action-gh-release@718ea10b132b3b2eba29c1007bb80653f286566b # v3
+        uses: softprops/action-gh-release@3d0d9888cb7fd7b750713d6e236d1fcb99157228 # v3
         with:
           prerelease: true
           tag_name: nightly
@@ -476,7 +476,7 @@ jobs:
           merge-multiple: true
 
       - name: Release
-        uses: softprops/action-gh-release@718ea10b132b3b2eba29c1007bb80653f286566b # v3
+        uses: softprops/action-gh-release@3d0d9888cb7fd7b750713d6e236d1fcb99157228 # v3
         with:
           files: binaries/*
           tag_name: nightly

.github/workflows/release.yml

@@ -449,6 +449,6 @@ jobs:
           merge-multiple: true
 
       - name: Release
-        uses: softprops/action-gh-release@718ea10b132b3b2eba29c1007bb80653f286566b # v3
+        uses: softprops/action-gh-release@3d0d9888cb7fd7b750713d6e236d1fcb99157228 # v3
         with:
           files: binaries/*

internal/controller/oidc_controller.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"net/url"
 	"slices"
 	"strconv"
 	"strings"
@@ -343,27 +344,31 @@ func (controller *OIDCController) authorizeComplete(c *gin.Context) {
 	// Create the authorization code
 	code := controller.oidc.CreateCode(*authorizeReq, *userContext)
 
-	queries, err := query.Values(AuthorizeCallback{
+	cu, err := url.Parse(authorizeReq.RedirectURI)
-		Code:  code,
-		State: authorizeReq.State,
-	})
 
 	if err != nil {
 		controller.authorizeError(c, authorizeErrorParams{
-			err:           err,
+			err:          err,
-			reason:        "Failed to build query",
+			reason:       "Failed to parse redirect URI",
-			reasonPublic:  "Failed to build query",
+			reasonPublic: "Failed to parse redirect URI",
-			callback:      authorizeReq.RedirectURI,
+			json:         true,
-			callbackError: "server_error",
-			state:         authorizeReq.State,
-			json:          true,
 		})
 		return
 	}
 
+	q := cu.Query()
+
+	q.Set("code", code)
+
+	if authorizeReq.State != "" {
+		q.Set("state", authorizeReq.State)
+	}
+
+	cu.RawQuery = q.Encode()
+
 	c.JSON(200, gin.H{
 		"status":       200,
-		"redirect_uri": fmt.Sprintf("%s?%s", authorizeReq.RedirectURI, queries.Encode()),
+		"redirect_uri": cu.String(),
 	})
 }
 
@@ -639,37 +644,37 @@ func (controller *OIDCController) authorizeError(c *gin.Context, params authoriz
 	controller.log.App.Error().Err(params.err).Str("reason", params.reason).Msg("Authorization error")
 
 	if params.callback != "" {
-		errorQueries := CallbackError{
+		cu, err := url.Parse(params.callback)
-			Error: params.callbackError,
-		}
-
-		if params.reasonPublic != "" {
-			errorQueries.ErrorDescription = params.reasonPublic
-		}
-
-		if params.state != "" {
-			errorQueries.State = params.state
-		}
-
-		queries, err := query.Values(errorQueries)
 
 		if err != nil {
-			controller.log.App.Error().Err(err).Msg("Failed to build callback error query")
+			controller.log.App.Error().Err(err).Msg("Failed to parse callback URL")
 			c.AbortWithStatus(http.StatusInternalServerError)
 			return
 		}
 
-		redirectUrl := fmt.Sprintf("%s?%s", params.callback, queries.Encode())
+		q := cu.Query()
+
+		q.Set("error", params.callbackError)
+
+		if params.reasonPublic != "" {
+			q.Set("error_description", params.reasonPublic)
+		}
+
+		if params.state != "" {
+			q.Set("state", params.state)
+		}
+
+		cu.RawQuery = q.Encode()
 
 		if params.json {
 			c.JSON(200, gin.H{
 				"status":       200,
-				"redirect_uri": redirectUrl,
+				"redirect_uri": cu.String(),
 			})
 			return
 		}
 
-		c.Redirect(http.StatusFound, redirectUrl)
+		c.Redirect(http.StatusFound, cu.String())
 		return
 	}
 

internal/controller/proxy_controller_test.go

@@ -708,7 +708,7 @@ func TestProxyController(t *testing.T) {
 		Log: log,
 	})
 
-	authService := service.NewAuthService(service.AuthServiceInput{
+	authService, err := service.NewAuthService(service.AuthServiceInput{
 		Log:          log,
 		Config:       &cfg,
 		Runtime:      &runtime,
@@ -721,6 +721,8 @@ func TestProxyController(t *testing.T) {
 		PolicyEngine: policyEngine,
 	})
 
+	require.NoError(t, err)
+
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
 			router := gin.Default()

internal/controller/user_controller.go

@@ -89,30 +89,22 @@ func (controller *UserController) loginHandler(c *gin.Context) {
 	search, err := controller.auth.SearchUser(req.Username)
 
 	if err != nil {
-		controller.constantTime(func() constantTimeRes {
+		if errors.Is(err, service.ErrUserNotFound) {
-			if errors.Is(err, service.ErrUserNotFound) {
+			controller.auth.DummyPasswordCheck(req.Password)
-				controller.log.App.Warn().Str("username", req.Username).Msg("User not found during login attempt")
+			controller.log.App.Warn().Str("username", req.Username).Msg("User not found during login attempt")
-				controller.auth.RecordLoginAttempt(req.Username, false)
+			controller.auth.RecordLoginAttempt(req.Username, false)
-				controller.log.AuditLoginFailure(req.Username, "unknown", c.ClientIP(), "user not found")
+			controller.log.AuditLoginFailure(req.Username, "unknown", c.ClientIP(), "user not found")
-				return constantTimeRes{
+			c.JSON(401, gin.H{
-					Code: 401,
+				"status":  401,
-					Res: gin.H{
+				"message": "Unauthorized",
-						"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{
-			controller.log.App.Error().Err(err).Str("username", req.Username).Msg("Error searching for user during login attempt")
+			"status":  500,
-			return constantTimeRes{
+			"message": "Internal Server Error",
-				Code: 500,
+		})
-				Res: gin.H{
-					"status":  500,
-					"message": "Internal Server Error",
-				},
-			}
-		}, func(res constantTimeRes) {
-			c.JSON(res.Code, res.Res)
-		}, time.Millisecond*45)
 		return
 	}
 
@@ -475,18 +467,3 @@ 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)
-}

internal/controller/user_controller_test.go

@@ -542,7 +542,8 @@ func TestUserController(t *testing.T) {
 		Runtime: &runtime,
 		Ctx:     ctx,
 	})
-	authService := service.NewAuthService(service.AuthServiceInput{
+
+	authService, err := service.NewAuthService(service.AuthServiceInput{
 		Log:          log,
 		Config:       &cfg,
 		Runtime:      &runtime,
@@ -555,6 +556,8 @@ func TestUserController(t *testing.T) {
 		PolicyEngine: policyEngine,
 	})
 
+	require.NoError(t, err)
+
 	beforeEach := func() {
 		// Clear failed login attempts before each test
 		authService.ClearLoginAttempts()

internal/middleware/context_middleware.go

@@ -2,6 +2,7 @@ package middleware
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"strings"
@@ -244,6 +245,9 @@ func (m *ContextMiddleware) basicAuth(username string, password string) (*model.
 	search, err := m.auth.SearchUser(username)
 
 	if err != nil {
+		if errors.Is(err, service.ErrUserNotFound) {
+			m.auth.DummyPasswordCheck(password)
+		}
 		return nil, nil, fmt.Errorf("error searching for user: %w", err)
 	}
 

internal/middleware/context_middleware_test.go

@@ -264,7 +264,8 @@ func TestContextMiddleware(t *testing.T) {
 		Runtime: &runtime,
 		Ctx:     ctx,
 	})
-	authService := service.NewAuthService(service.AuthServiceInput{
+
+	authService, err := service.NewAuthService(service.AuthServiceInput{
 		Log:          log,
 		Config:       &cfg,
 		Runtime:      &runtime,
@@ -277,6 +278,8 @@ func TestContextMiddleware(t *testing.T) {
 		PolicyEngine: policyEngine,
 	})
 
+	require.NoError(t, err)
+
 	contextMiddleware := NewContextMiddleware(ContextMiddlewareInput{
 		Log:              log,
 		RuntimeConfig:    &runtime,

internal/service/auth_service.go

@@ -69,6 +69,8 @@ type AuthService struct {
 	tailscale    *TailscaleService
 	policyEngine *PolicyEngine
 
+	dummyHash string
+
 	lockdown struct {
 		active     bool
 		until      time.Time
@@ -101,7 +103,7 @@ type AuthServiceInput struct {
 	PolicyEngine *PolicyEngine
 }
 
-func NewAuthService(i AuthServiceInput) *AuthService {
+func NewAuthService(i AuthServiceInput) (*AuthService, error) {
 	service := &AuthService{
 		log:          i.Log,
 		runtime:      i.Runtime,
@@ -123,6 +125,15 @@ func NewAuthService(i AuthServiceInput) *AuthService {
 		loginCacheSize = service.maxLoginLimits
 	}
 
+	// dummy hash
+	dummyHash, err := bcrypt.GenerateFromPassword([]byte(utils.GenerateString(8)), bcrypt.DefaultCost)
+
+	if err != nil {
+		return nil, fmt.Errorf("failed to generate dummy hash: %w", err)
+	}
+
+	service.dummyHash = string(dummyHash)
+
 	// caches setup
 	oauthCache := NewCacheStore[OAuthPendingSession](256)
 	loginCache := NewCacheStore[LoginAttempt](loginCacheSize)
@@ -148,7 +159,11 @@ func NewAuthService(i AuthServiceInput) *AuthService {
 		}
 	}, ding.RingMinor)
 
-	return service
+	return service, nil
+}
+
+func (auth *AuthService) DummyPasswordCheck(password string) {
+	bcrypt.CompareHashAndPassword([]byte(auth.dummyHash), []byte(password))
 }
 
 func (auth *AuthService) SearchUser(username string) (*model.UserSearch, error) {