From 1a6642c3664057b5101148dd9039a0c7d10a2bf2 Mon Sep 17 00:00:00 2001 From: Hivert Quentin Date: Tue, 10 Nov 2026 14:06:00 +0100 Subject: [PATCH] #3124: limit number of attempt for password recovery --- .gitignore | 4 +- Documentation/SOGoInstallationGuide.asciidoc | 11 ++ SoObjects/SOGo/SOGoCache.h | 5 + SoObjects/SOGo/SOGoCache.m | 71 +++++++-- SoObjects/SOGo/SOGoDefaults.plist | 1 + SoObjects/SOGo/SOGoSystemDefaults.h | 1 + SoObjects/SOGo/SOGoSystemDefaults.m | 11 ++ SoObjects/SOGo/SOGoUserManager.h | 5 +- SoObjects/SOGo/SOGoUserManager.m | 149 +++++++++++++++++- UI/MainUI/SOGoRootPage.m | 18 ++- .../js/Common/Authentication.service.js | 2 + 11 files changed, 249 insertions(+), 29 deletions(-) diff --git a/.gitignore b/.gitignore index d5e060c7b..2927d3426 100644 --- a/.gitignore +++ b/.gitignore @@ -59,4 +59,6 @@ Tests/Unit/SoObjects # SAML2 autogenerated files SoObjects/SOGo/SOGoSAML2Exceptions.* -node_modules \ No newline at end of file +node_modules +CLAUDE.md +.claude/ \ No newline at end of file diff --git a/Documentation/SOGoInstallationGuide.asciidoc b/Documentation/SOGoInstallationGuide.asciidoc index 07fcb475d..3f17c4ee8 100644 --- a/Documentation/SOGoInstallationGuide.asciidoc +++ b/Documentation/SOGoInstallationGuide.asciidoc @@ -782,6 +782,17 @@ Default value is `0`, or disabled. _SOGoCacheCleanupInterval_ must be set to a value equal or higher than _SOGoFailedLoginBlockInterval_. +|S |SOGoMaximumFailedPasswordRecoveryCount +|Parameter used to control the number of wrong secret question answers +allowed when recovering a password. When reached, password recovery +by secret question is blocked for the account for +_SOGoFailedLoginBlockInterval_ seconds since the first wrong answer. +This counter is independent of _SOGoMaximumFailedLoginCount_: it does +not block logins, and it is enforced even when failed login +"rate-limiting" is disabled. + +Default value is `5`. Set to `0` to disable. + |S |SOGoMaximumMessageSubmissionCount |Parameter used to control the number of email messages a user can send from SOGo's webmail interface, to _SOGoMaximumRecipientCount_, in diff --git a/SoObjects/SOGo/SOGoCache.h b/SoObjects/SOGo/SOGoCache.h index 3e3bae9b4..c547cb414 100644 --- a/SoObjects/SOGo/SOGoCache.h +++ b/SoObjects/SOGo/SOGoCache.h @@ -92,6 +92,11 @@ - (NSDictionary *) failedCountForLogin: (NSString *) login; +- (void) setFailedPasswordRecoveryCount: (int) theCount + forLogin: (NSString *) theLogin; + +- (NSDictionary *) failedPasswordRecoveryCountForLogin: (NSString *) login; + - (void) setMessageSubmissionsCount: (int) theCount recipientsCount: (int) theRecipientsCount forLogin: (NSString *) theLogin; diff --git a/SoObjects/SOGo/SOGoCache.m b/SoObjects/SOGo/SOGoCache.m index 01e6790ab..12e3f1b43 100644 --- a/SoObjects/SOGo/SOGoCache.m +++ b/SoObjects/SOGo/SOGoCache.m @@ -479,8 +479,26 @@ static memcached_st *handle = NULL; // // SOGo password failed counts // -- (void) setFailedCount: (int) theCount - forLogin: (NSString *) theLogin +- (NSDictionary *) _failedCountForLogin: (NSString *) theLogin + ofType: (NSString *) theType +{ + NSDictionary *d; + NSString *s; + + s = [self _valuesOfType: theType forKey: theLogin]; + d = nil; + + if (s) + { + d = [s objectFromJSONString]; + } + + return d; +} + +- (void) _setFailedCount: (int) theCount + forLogin: (NSString *) theLogin + ofType: (NSString *) theType { NSMutableDictionary *d; NSNumber *count; @@ -489,7 +507,8 @@ static memcached_st *handle = NULL; { count = [NSNumber numberWithInt: theCount]; - d = [NSMutableDictionary dictionaryWithDictionary: [self failedCountForLogin: theLogin]]; + d = [NSMutableDictionary dictionaryWithDictionary: [self _failedCountForLogin: theLogin + ofType: theType]]; if (![d objectForKey: @"InitialDate"]) { @@ -500,35 +519,53 @@ static memcached_st *handle = NULL; [d setObject: count forKey: @"FailedCount"]; [self _cacheValues: [d jsonRepresentation] - ofType: @"failedlogins" + ofType: theType forKey: theLogin]; } else { - [self removeValueForKey: [NSString stringWithFormat: @"%@+failedlogins", theLogin]]; + [self removeValueForKey: [NSString stringWithFormat: @"%@+%@", theLogin, theType]]; } } +- (void) setFailedCount: (int) theCount + forLogin: (NSString *) theLogin +{ + [self _setFailedCount: theCount + forLogin: theLogin + ofType: @"failedlogins"]; +} + // -// Returns a dictionary with two keys/values +// Returns a dictionary with three keys/values // // FailedCount -> // InitialDate -> +// LastRequestDate -> // - (NSDictionary *) failedCountForLogin: (NSString *) theLogin { - NSDictionary *d; - NSString *s; - - s = [self _valuesOfType: @"failedlogins" forKey: theLogin]; - d = nil; + return [self _failedCountForLogin: theLogin + ofType: @"failedlogins"]; +} - if (s) - { - d = [s objectFromJSONString]; - } - - return d; +// +// SOGo password recovery (secret question) failed counts. +// Kept separate from failed logins so that a recovery lockout +// does not prevent the user from logging in, and vice versa. +// +- (void) setFailedPasswordRecoveryCount: (int) theCount + forLogin: (NSString *) theLogin +{ + [self _setFailedCount: theCount + forLogin: theLogin + ofType: @"failedrecovery"]; +} + +- (NSDictionary *) failedPasswordRecoveryCountForLogin: (NSString *) theLogin +{ + return [self _failedCountForLogin: theLogin + ofType: @"failedrecovery"]; } // diff --git a/SoObjects/SOGo/SOGoDefaults.plist b/SoObjects/SOGo/SOGoDefaults.plist index b31372620..00f5a567b 100644 --- a/SoObjects/SOGo/SOGoDefaults.plist +++ b/SoObjects/SOGo/SOGoDefaults.plist @@ -208,4 +208,5 @@ SOGoPasswordRecoveryMode = "Disabled"; SOGoPasswordRecoveryQuestion = "SecretQuestion1"; + SOGoMaximumFailedPasswordRecoveryCount = 5; } diff --git a/SoObjects/SOGo/SOGoSystemDefaults.h b/SoObjects/SOGo/SOGoSystemDefaults.h index 7e5de83a2..65cf55bc7 100644 --- a/SoObjects/SOGo/SOGoSystemDefaults.h +++ b/SoObjects/SOGo/SOGoSystemDefaults.h @@ -127,6 +127,7 @@ NSComparisonResult languageSort(id el1, id el2, void *context); - (int) maximumFailedLoginCount; - (int) maximumFailedLoginInterval; - (int) failedLoginBlockInterval; +- (int) maximumFailedPasswordRecoveryCount; - (int) maximumMessageSizeLimit; diff --git a/SoObjects/SOGo/SOGoSystemDefaults.m b/SoObjects/SOGo/SOGoSystemDefaults.m index 2d4d825b1..000ce66b4 100644 --- a/SoObjects/SOGo/SOGoSystemDefaults.m +++ b/SoObjects/SOGo/SOGoSystemDefaults.m @@ -916,6 +916,17 @@ NSComparisonResult languageSort(id el1, id el2, void *context) return v; } +// +// Number of wrong secret question answers allowed during +// SOGoFailedLoginBlockInterval seconds before password recovery +// is blocked for the account. Defaults to 5 (see SOGoDefaults.plist), +// 0 disables the protection. +// +- (int) maximumFailedPasswordRecoveryCount +{ + return [self integerForKey: @"SOGoMaximumFailedPasswordRecoveryCount"]; +} + // // // diff --git a/SoObjects/SOGo/SOGoUserManager.h b/SoObjects/SOGo/SOGoUserManager.h index 8feb79fe8..6db033180 100644 --- a/SoObjects/SOGo/SOGoUserManager.h +++ b/SoObjects/SOGo/SOGoUserManager.h @@ -106,7 +106,10 @@ perr: (SOGoPasswordPolicyError *) perr; - (NSDictionary *) getPasswordRecoveryInfosForUsername: (NSString *) username domain:(NSString *) domain; - (NSString *) generateAndSavePasswordRecoveryTokenWithUid: (NSString *) uid username: (NSString *) username domain:(NSString *) domain; -- (NSString *) getTokenAndCheckPasswordRecoveryDataForUsername: (NSString *) username domain:(NSString *) domain withData:(NSDictionary *) passwordRecoveryData; +- (NSString *) getTokenAndCheckPasswordRecoveryDataForUsername: (NSString *) username + domain: (NSString *) domain + withData: (NSDictionary *) passwordRecoveryData + perr: (SOGoPasswordPolicyError *) perr; - (NSString *) getPasswordRecoveryTokenFor: (NSString *) username domain:(NSString *) domain; - (BOOL) isPasswordRecoveryTokenValidFor: (NSString *) token uid: (NSString *) uid; @end diff --git a/SoObjects/SOGo/SOGoUserManager.m b/SoObjects/SOGo/SOGoUserManager.m index ba53568d6..97913045e 100644 --- a/SoObjects/SOGo/SOGoUserManager.m +++ b/SoObjects/SOGo/SOGoUserManager.m @@ -802,6 +802,45 @@ static const NSString *kObfuscatedSecondaryEmailKey = @"obfuscatedSecondaryEmail } // +// +// Same failed-login accounting as -checkLogin:..., used when the old +// password is verified while changing it (e.g. ppolicy change upon login, +// which is reachable without a session). This prevents using +// changePassword to bypass SOGoMaximumFailedLoginCount. +// +- (BOOL) _isFailedLoginBlockedForLogin: (NSString *) username + failedCount: (int *) failedCount +{ + NSDictionary *failedLogin; + SOGoSystemDefaults *sd; + unsigned int currentTime, startTime, lastRequestTime; + + *failedCount = 0; + sd = [SOGoSystemDefaults sharedSystemDefaults]; + if ([sd maximumFailedLoginCount] <= 0) + return NO; + + failedLogin = [[SOGoCache sharedCache] failedCountForLogin: username]; + if (!failedLogin) + return NO; + + currentTime = [[NSCalendarDate date] timeIntervalSince1970]; + startTime = [[failedLogin objectForKey: @"InitialDate"] unsignedIntValue]; + lastRequestTime = [[failedLogin objectForKey: @"LastRequestDate"] unsignedIntValue]; + + if (currentTime - startTime > (unsigned int) [sd failedLoginBlockInterval]) + { + [[SOGoCache sharedCache] setFailedCount: 0 + forLogin: username]; + return NO; + } + + *failedCount = [[failedLogin objectForKey: @"FailedCount"] intValue]; + + return (*failedCount >= [sd maximumFailedLoginCount] + && currentTime - lastRequestTime < (unsigned int) [sd maximumFailedLoginInterval]); +} + // // - (BOOL) changePasswordForLogin: (NSString *) login @@ -812,9 +851,10 @@ static const NSString *kObfuscatedSecondaryEmailKey = @"obfuscatedSecondaryEmail token: (NSString *) token perr: (SOGoPasswordPolicyError *) perr { - NSString *jsonUser, *userLogin, *uid, *userToken; + NSString *jsonUser, *userLogin, *uid, *userToken, *failedLoginKey; NSMutableDictionary *currentUser; BOOL didChange; + int failedCount; SOGoSystemDefaults *sd; NSDictionary *info; SOGoUserSettings *us; @@ -830,9 +870,22 @@ static const NSString *kObfuscatedSecondaryEmailKey = @"obfuscatedSecondaryEmail info = [self contactInfosForUserWithUIDorEmail: login]; uid = [info objectForKey: @"c_uid"]; + failedCount = 0; + + // Key used by -checkLogin:... for the failed login count + if ([domain isNotNull] && [domain length] + && [login rangeOfString: @"@"].location == NSNotFound) + failedLoginKey = [NSString stringWithFormat: @"%@@%@", login, domain]; + else + failedLoginKey = login; + if (passwordRecovery && (![userToken isEqualToString: token] || ![self isPasswordRecoveryTokenValidFor: userToken uid: uid])) { didChange = NO; *perr = PolicyPasswordRecoveryInvalidToken; + } else if (!passwordRecovery + && [self _isFailedLoginBlockedForLogin: failedLoginKey failedCount: &failedCount]) { + didChange = NO; + *perr = PolicyAccountLocked; } else if ([self _sourceChangePasswordForLogin: login inDomain: domain oldPassword: oldPassword @@ -868,7 +921,20 @@ static const NSString *kObfuscatedSecondaryEmailKey = @"obfuscatedSecondaryEmail forLogin: userLogin]; } else - didChange = NO; + { + didChange = NO; + + // A rejected new password (quality, history, age) means the old + // password was accepted; anything else counts as a failed login. + if (!passwordRecovery + && [[SOGoSystemDefaults sharedSystemDefaults] maximumFailedLoginCount] > 0 + && *perr != PolicyInsufficientPasswordQuality + && *perr != PolicyPasswordTooShort + && *perr != PolicyPasswordTooYoung + && *perr != PolicyPasswordInHistory) + [[SOGoCache sharedCache] setFailedCount: failedCount + 1 + forLogin: failedLoginKey]; + } return didChange; } @@ -1479,28 +1545,82 @@ static const NSString *kObfuscatedSecondaryEmailKey = @"obfuscatedSecondaryEmail return jwtToken; } +/** + * (Secret Question) Check whether password recovery is temporarily blocked for uid + * because of too many wrong answers. Resets the counter once the block interval is over. + * @param uid User id + * @param failedCount Current number of failures (out) + * @return True if blocked + */ +- (BOOL) _isPasswordRecoveryBlockedForUid: (NSString *) uid + failedCount: (int *) failedCount +{ + NSDictionary *failedRecovery; + SOGoSystemDefaults *sd; + unsigned int currentTime, startTime; + int maxFailed; + + *failedCount = 0; + sd = [SOGoSystemDefaults sharedSystemDefaults]; + maxFailed = [sd maximumFailedPasswordRecoveryCount]; + if (maxFailed <= 0) + return NO; + + failedRecovery = [[SOGoCache sharedCache] failedPasswordRecoveryCountForLogin: uid]; + if (!failedRecovery) + return NO; + + currentTime = [[NSCalendarDate date] timeIntervalSince1970]; + startTime = [[failedRecovery objectForKey: @"InitialDate"] unsignedIntValue]; + if (currentTime - startTime > (unsigned int) [sd failedLoginBlockInterval]) + { + [[SOGoCache sharedCache] setFailedPasswordRecoveryCount: 0 + forLogin: uid]; + return NO; + } + + *failedCount = [[failedRecovery objectForKey: @"FailedCount"] intValue]; + + return (*failedCount >= maxFailed); +} + /** * (Secret Question) Check secret question answer, generates and save in user pref the JWT token + * Wrong answers are counted per uid; after SOGoMaximumFailedPasswordRecoveryCount failures, + * recovery is blocked for SOGoFailedLoginBlockInterval seconds. * @param username Username * @param domain Domain * @param passwordRecoveryData Password recovery data (answer, ...) + * @param perr Set to PolicyAccountLocked if recovery is blocked * @return JWT Token */ -- (NSString *) getTokenAndCheckPasswordRecoveryDataForUsername: (NSString *) username domain:(NSString *) domain withData:(NSDictionary *) passwordRecoveryData +- (NSString *) getTokenAndCheckPasswordRecoveryDataForUsername: (NSString *) username + domain: (NSString *) domain + withData: (NSDictionary *) passwordRecoveryData + perr: (SOGoPasswordPolicyError *) perr { - NSString *mode, *questionKey, *answer, *uid, *suffix, *token; + NSString *mode, *questionKey, *answer, *storedAnswer, *uid, *suffix, *token; SOGoUserDefaults *userDefaults; SOGoSystemDefaults *sd; NSDictionary *info; + id value; + int failedCount; + *perr = PolicyNoError; mode = [passwordRecoveryData objectForKey:@"mode"]; questionKey = [passwordRecoveryData objectForKey:@"question"]; - answer = [[[passwordRecoveryData objectForKey:@"answer"] lowercaseString] stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceAndNewlineCharacterSet]]; + value = [passwordRecoveryData objectForKey:@"answer"]; + answer = nil; + if ([value isKindOfClass: [NSString class]]) + answer = [[value lowercaseString] stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceAndNewlineCharacterSet]]; token = nil; userDefaults = nil; info = [self contactInfosForUserWithUIDorEmail: username]; uid = [info objectForKey: @"c_uid"]; + if (![uid length]) + return nil; + sd = [SOGoSystemDefaults sharedSystemDefaults]; if ([sd enableDomainBasedUID] && ![[info objectForKey: @"DomainLessLogin"] boolValue]) @@ -1519,9 +1639,24 @@ static const NSString *kObfuscatedSecondaryEmailKey = @"obfuscatedSecondaryEmail } if ([sd isPasswordRecoveryEnabled] && nil != userDefaults && [[userDefaults passwordRecoveryMode] isEqualToString: mode]) { - if ([[userDefaults passwordRecoveryQuestion] isEqualToString: questionKey] - && [[[[userDefaults passwordRecoveryQuestionAnswer] lowercaseString] stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceAndNewlineCharacterSet]] isEqualToString: answer]) { + // The counter is keyed on the resolved uid so that it can't be bypassed + // by using another form of the username (email, case, etc.) + if ([self _isPasswordRecoveryBlockedForUid: uid failedCount: &failedCount]) { + *perr = PolicyAccountLocked; + return nil; + } + + storedAnswer = [[[userDefaults passwordRecoveryQuestionAnswer] lowercaseString] stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceAndNewlineCharacterSet]]; + if ([[userDefaults passwordRecoveryQuestion] isEqualToString: questionKey] + && [storedAnswer length] > 0 + && [answer length] > 0 + && [storedAnswer isEqualToString: answer]) { token = [self generateAndSavePasswordRecoveryTokenWithUid: uid username: username domain: domain]; + [[SOGoCache sharedCache] setFailedPasswordRecoveryCount: 0 + forLogin: uid]; + } else if ([sd maximumFailedPasswordRecoveryCount] > 0) { + [[SOGoCache sharedCache] setFailedPasswordRecoveryCount: failedCount + 1 + forLogin: uid]; } } diff --git a/UI/MainUI/SOGoRootPage.m b/UI/MainUI/SOGoRootPage.m index 27b654d84..3fcb667d5 100644 --- a/UI/MainUI/SOGoRootPage.m +++ b/UI/MainUI/SOGoRootPage.m @@ -1441,11 +1441,12 @@ static const NSString *kJwtKey = @"jwt"; */ - (WOResponse *) passwordRecoveryCheckAction { - NSString *username, *domain, *token, *mode; + NSString *username, *domain, *token, *mode, *remoteHost; NSDictionary *message; WORequest *request; SOGoUserManager *um; WOResponse *response; + SOGoPasswordPolicyError perr; username = nil; domain = nil; @@ -1454,12 +1455,23 @@ static const NSString *kJwtKey = @"jwt"; username = [message objectForKey: @"userName"]; domain = [message objectForKey: @"domain"]; mode = [message objectForKey: @"mode"]; + remoteHost = [request headerForKey: @"x-webobjects-remote-host"]; um = [SOGoUserManager sharedUserManager]; if ([mode isEqualToString: SOGoPasswordRecoveryQuestion]) { // Secret question recovery - token = [um getTokenAndCheckPasswordRecoveryDataForUsername: username domain: domain withData: message]; - if (!token) { + token = [um getTokenAndCheckPasswordRecoveryDataForUsername: username + domain: domain + withData: message + perr: &perr]; + if (!token && perr == PolicyAccountLocked) { + [self logWithFormat: @"Password recovery from '%@' for user '%@' blocked - too many failed attempts", + remoteHost, username]; + response = [self responseWithStatus: 403 + andString: @"Your account was locked due to too many failed attempts."]; + } else if (!token) { + [self logWithFormat: @"Password recovery from '%@' for user '%@' failed - invalid secret question answer", + remoteHost, username]; response = [self responseWithStatus: 403 andString: @"Invalid secret question answer"]; } else { diff --git a/UI/WebServerResources/js/Common/Authentication.service.js b/UI/WebServerResources/js/Common/Authentication.service.js index e94452978..56ae3f014 100644 --- a/UI/WebServerResources/js/Common/Authentication.service.js +++ b/UI/WebServerResources/js/Common/Authentication.service.js @@ -260,6 +260,8 @@ error = l("Password change failed - Password is in history"); } else if (perr == passwordPolicyConfig.PolicyPasswordRecoveryInvalidToken) { error = l("Invalid token. Could not change password"); + } else if (perr == passwordPolicyConfig.PolicyAccountLocked) { + error = l("Your account was locked due to too many failed attempts."); } else { error = l("Unhandled policy error: %{0}").formatted(perr); perr = passwordPolicyConfig.PolicyPasswordUnknown;