diff --git a/http/http_service.go b/http/http_service.go index 25c9032f..63d0ffcc 100644 --- a/http/http_service.go +++ b/http/http_service.go @@ -97,12 +97,22 @@ func (httpSvc *HttpService) RegisterSharedRoutes(e *echo.Echo) { e.POST("/api/setup", httpSvc.setupHandler) e.POST("/api/restore", httpSvc.restoreBackupHandler) - // allow one unlock request per second - unlockRateLimiter := middleware.RateLimiter(middleware.NewRateLimiterMemoryStore(1)) + // A single global rate limiter (one bucket for all callers, not per-IP) + // shared by every endpoint that verifies the unlock password, to bound how + // fast the password can be guessed. + unlockRateLimiter := middleware.RateLimiterWithConfig(middleware.RateLimiterConfig{ + Store: middleware.NewRateLimiterMemoryStoreWithConfig( + // burst of 2 so unlocking and then immediately acting is not blocked + middleware.RateLimiterMemoryStoreConfig{Rate: 1, Burst: 2}, + ), + IdentifierExtractor: func(c echo.Context) (string, error) { + return "", nil + }, + }) e.POST("/api/start", httpSvc.startHandler, unlockRateLimiter) e.POST("/api/unlock", httpSvc.unlockHandler, unlockRateLimiter) e.POST("/api/backup", httpSvc.createBackupHandler, unlockRateLimiter) - e.GET("/logout", httpSvc.logoutHandler, unlockRateLimiter) + e.GET("/logout", httpSvc.logoutHandler) frontend.RegisterHandlers(e) @@ -157,17 +167,17 @@ func (httpSvc *HttpService) RegisterSharedRoutes(e *echo.Echo) { fullAccessApiGroup.Use(httpSvc.requireFullAccess) fullAccessApiGroup.POST("/event", httpSvc.eventHandler) - fullAccessApiGroup.PATCH("/unlock-password", httpSvc.changeUnlockPasswordHandler) - fullAccessApiGroup.PATCH("/auto-unlock", httpSvc.autoUnlockHandler) + fullAccessApiGroup.PATCH("/unlock-password", httpSvc.changeUnlockPasswordHandler, unlockRateLimiter) + fullAccessApiGroup.PATCH("/auto-unlock", httpSvc.autoUnlockHandler, unlockRateLimiter) fullAccessApiGroup.PATCH("/settings", httpSvc.updateSettingsHandler) fullAccessApiGroup.PATCH("/apps/:pubkey", httpSvc.appsUpdateHandler) fullAccessApiGroup.PATCH("/transactions/:id/labels", httpSvc.setTransactionUserLabelsHandler) fullAccessApiGroup.DELETE("/apps/:pubkey", httpSvc.appsDeleteHandler) fullAccessApiGroup.POST("/transfers", httpSvc.transfersHandler) - fullAccessApiGroup.POST("/apps", httpSvc.appsCreateHandler) + fullAccessApiGroup.POST("/apps", httpSvc.appsCreateHandler, unlockRateLimiter) fullAccessApiGroup.POST("/lightning-addresses", httpSvc.lightningAddressesCreateHandler) fullAccessApiGroup.DELETE("/lightning-addresses/:appId", httpSvc.lightningAddressesDeleteHandler) - fullAccessApiGroup.POST("/mnemonic", httpSvc.mnemonicHandler) + fullAccessApiGroup.POST("/mnemonic", httpSvc.mnemonicHandler, unlockRateLimiter) fullAccessApiGroup.PATCH("/backup-reminder", httpSvc.backupReminderHandler) fullAccessApiGroup.POST("/channels", httpSvc.openChannelHandler) fullAccessApiGroup.POST("/channels/rebalance", httpSvc.rebalanceChannelHandler) @@ -192,7 +202,7 @@ func (httpSvc *HttpService) RegisterSharedRoutes(e *echo.Echo) { fullAccessApiGroup.POST("/swaps/refund", httpSvc.refundSwapHandler) fullAccessApiGroup.GET("/swaps/mnemonic", httpSvc.swapMnemonicHandler) fullAccessApiGroup.GET("/log/:type", httpSvc.getLogOutputHandler) - fullAccessApiGroup.POST("/autoswap", httpSvc.enableAutoSwapOutHandler) + fullAccessApiGroup.POST("/autoswap", httpSvc.enableAutoSwapOutHandler, unlockRateLimiter) fullAccessApiGroup.DELETE("/autoswap", httpSvc.disableAutoSwapOutHandler) fullAccessApiGroup.POST("/node/alias", httpSvc.setNodeAliasHandler) diff --git a/http/http_service_test.go b/http/http_service_test.go index 0d452677..f5cce7f9 100644 --- a/http/http_service_test.go +++ b/http/http_service_test.go @@ -92,6 +92,95 @@ func TestUnlock_UnknownPermission(t *testing.T) { mockConfig.AssertNotCalled(t, "GetJWTSecret") } +// TestUnlock_RateLimited verifies that repeated requests to an unlock-password +// endpoint are throttled with HTTP 429 once the limit is exceeded. +func TestUnlock_RateLimited(t *testing.T) { + e := echo.New() + logger.Init(strconv.Itoa(int(logrus.DebugLevel))) + mockSvc := mocks.NewMockService(t) + gormDb, err := db.NewDB(t) + require.NoError(t, err) + defer db.CloseDB(gormDb) + + mockEventPublisher := events.NewEventPublisher() + + mockConfig := mocks.NewMockConfig(t) + mockConfig.On("GetEnv").Return(&config.AppConfig{}) + mockConfig.On("CheckUnlockPassword", "wrong").Return(false) + + mockSvc.On("GetDB").Return(gormDb) + mockSvc.On("GetConfig").Return(mockConfig) + mockSvc.On("GetKeys").Return(mocks.NewMockKeys(t)) + mockSvc.On("GetAlbySvc").Return(mocks.NewMockAlbyService(t)) + mockSvc.On("GetAlbyOAuthSvc").Return(mocks.NewMockAlbyOAuthService(t)) + + httpSvc := NewHttpService(mockSvc, mockEventPublisher) + httpSvc.RegisterSharedRoutes(e) + + jsonBody, _ := json.Marshal(api.UnlockRequest{UnlockPassword: "wrong", Permission: "full"}) + send := func() int { + req := httptest.NewRequest(http.MethodPost, "/api/unlock", bytes.NewBuffer(jsonBody)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + return rec.Code + } + + // the burst of 2 is served (wrong password, so unauthorized) + assert.Equal(t, http.StatusUnauthorized, send()) + assert.Equal(t, http.StatusUnauthorized, send()) + // the next request exceeds the limit and is rejected with 429 + assert.Equal(t, http.StatusTooManyRequests, send()) +} + +// TestUnlock_RateLimitNotBypassedBySpoofedIP verifies that the unlock rate +// limiter is global rather than per-IP: varying the X-Forwarded-For header per +// request does not grant each request a fresh bucket. +func TestUnlock_RateLimitNotBypassedBySpoofedIP(t *testing.T) { + e := echo.New() + logger.Init(strconv.Itoa(int(logrus.DebugLevel))) + mockSvc := mocks.NewMockService(t) + gormDb, err := db.NewDB(t) + require.NoError(t, err) + defer db.CloseDB(gormDb) + + mockEventPublisher := events.NewEventPublisher() + + mockConfig := mocks.NewMockConfig(t) + mockConfig.On("GetEnv").Return(&config.AppConfig{}) + mockConfig.On("CheckUnlockPassword", "wrong").Return(false) + + mockSvc.On("GetDB").Return(gormDb) + mockSvc.On("GetConfig").Return(mockConfig) + mockSvc.On("GetKeys").Return(mocks.NewMockKeys(t)) + mockSvc.On("GetAlbySvc").Return(mocks.NewMockAlbyService(t)) + mockSvc.On("GetAlbyOAuthSvc").Return(mocks.NewMockAlbyOAuthService(t)) + + httpSvc := NewHttpService(mockSvc, mockEventPublisher) + httpSvc.RegisterSharedRoutes(e) + + jsonBody, _ := json.Marshal(api.UnlockRequest{UnlockPassword: "wrong", Permission: "full"}) + + send := func(forwardedFor string) int { + req := httptest.NewRequest(http.MethodPost, "/api/unlock", bytes.NewBuffer(jsonBody)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-Forwarded-For", forwardedFor) + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + return rec.Code + } + + rateLimited := 0 + for i := 0; i < 12; i++ { + // each request presents a distinct client address + if send("10.0.0."+strconv.Itoa(i)) == http.StatusTooManyRequests { + rateLimited++ + } + } + + assert.Positive(t, rateLimited, "spoofing X-Forwarded-For must not grant a fresh rate-limit bucket") +} + func TestGetApps_NoToken(t *testing.T) { e := echo.New() logger.Init(strconv.Itoa(int(logrus.DebugLevel)))