Browse Source

FIX #yogosha17128

Laurent Destailleur 1 year ago
parent
commit
e41f449f95

+ 1 - 1
SECURITY.md

@@ -59,7 +59,7 @@ ONLY vulnerabilities discovered, when the following setup on test platform is us
 * CSRF attacks are accepted but double check that you have set MAIN_SECURITY_CSRF_WITH_TOKEN to value 3.
 * The modules DebugBar and ModuleBuilder must NOT be enabled. (by default, these modules are not enabled. They are developer tools)
 * Ability for a high level user to edit web site pages into the CMS by including HTML or Javascript is an expected feature. Vulnerabilities into the website module are validated only if HTML or Javascript injection can be done by a non allowed user.
-* Fail2ban rules for rate limit on the login page,password forgotten page and all public pages (/public/*) must be installed as recommendend into the section "About - Admin tools - Section Access limits and mitigation".
+* Fail2ban rules for rate limit on the login page,password forgotten page, api calls and all public pages (/public/*) must be installed as recommendend into the section "About - Admin tools - Section Access limits and mitigation".
 
 Scope is the web application (back office) and the APIs.
 

+ 1 - 1
dev/setup/fail2ban/filter.d/web-dolibarr-rulesbruteforce.conf

@@ -15,5 +15,5 @@
 # To test rule file on a existing log file
 # fail2ban-regex /mypath/documents/dolibarr.log /etc/fail2ban/filter.d/web-dolibarr-rulesbruteforce.conf
 
-failregex = ^ [A-Z\s]+ <HOST>\s+functions_dolibarr::check_user_password_.* Authentication KO
+failregex = ^ [A-Z\s]+ <HOST>\s+functions_.*::check_user_.* Authentication KO
 ignoreregex =

+ 19 - 13
htdocs/api/class/api_access.class.php

@@ -113,7 +113,7 @@ class DolibarrApiAccess implements iAuthenticate
 		if ($api_key) {
 			$userentity = 0;
 
-			$sql = "SELECT u.login, u.datec, u.api_key, ";
+			$sql = "SELECT u.login, u.datec, u.api_key,";
 			$sql .= " u.tms as date_modification, u.entity";
 			$sql .= " FROM ".MAIN_DB_PREFIX."user as u";
 			$sql .= " WHERE u.api_key = '".$this->db->escape($api_key)."' OR u.api_key = '".$this->db->escape(dolEncrypt($api_key, '', '', 'dolibarr'))."'";
@@ -140,43 +140,49 @@ class DolibarrApiAccess implements iAuthenticate
 				throw new RestException(503, 'Error when fetching user api_key :'.$this->db->error_msg);
 			}
 
-			if ($stored_key != $api_key) {		// This should not happen since we did a search on api_key
+			if ($login && $stored_key != $api_key) {		// This should not happen since we did a search on api_key
 				$userClass::setCacheIdentifier($api_key);
 				return false;
 			}
 
+			$genericmessageerroruser = 'Error user not valid (not found with api key or bad status or bad validity dates) (conf->entity='.$conf->entity.')';
+
 			if (!$login) {
-				throw new RestException(503, 'Error when searching login user from api key');
+				dol_syslog("functions_isallowed::check_user_api_key Authentication KO for api key: Error when searching login user from api key", LOG_NOTICE);
+				sleep(1); // Anti brut force protection. Must be same delay when user and password are not valid.
+				throw new RestException(401, $genericmessageerroruser);
 			}
 
-
-			$genericmessageerroruser = 'Error user not valid (not found or bad status or bad validity dates) (conf->entity='.$conf->entity.')';
-
 			$fuser = new User($this->db);
 			$result = $fuser->fetch('', $login, '', 0, (empty($userentity) ? -1 : $conf->entity)); // If user is not entity 0, we search in working entity $conf->entity  (that may have been forced to a different value than user entity)
 			if ($result <= 0) {
-				throw new RestException(503, $genericmessageerroruser);
+				dol_syslog("functions_isallowed::check_user_api_key Authentication KO for '".$login."': Failed to fetch on entity", LOG_NOTICE);
+				sleep(1); // Anti brut force protection. Must be same delay when user and password are not valid.
+				throw new RestException(401, $genericmessageerroruser);
 			}
 
 			// Check if user status is enabled
 			if ($fuser->statut != $fuser::STATUS_ENABLED) {
 				// Status is disabled
-				dol_syslog("The user has been disabled");
-				throw new RestException(503, $genericmessageerroruser);
+				dol_syslog("functions_isallowed::check_user_api_key Authentication KO for '".$login."': The user has been disabled", LOG_NOTICE);
+				sleep(1); // Anti brut force protection. Must be same delay when user and password are not valid.
+				throw new RestException(401, $genericmessageerroruser);
 			}
 
 			// Check if session was unvalidated by a password change
 			if (($fuser->flagdelsessionsbefore && !empty($_SESSION["dol_logindate"]) && $fuser->flagdelsessionsbefore > $_SESSION["dol_logindate"])) {
 				// Session is no more valid
-				dol_syslog("The user has a date for session invalidation = ".$fuser->flagdelsessionsbefore." and a session date = ".$_SESSION["dol_logindate"].". We must invalidate its sessions.");
-				throw new RestException(503, $genericmessageerroruser);
+				dol_syslog("functions_isallowed::check_user_api_key Authentication KO for '".$login."': The user has a date for session invalidation = ".$fuser->flagdelsessionsbefore." and a session date = ".$_SESSION["dol_logindate"].". We must invalidate its sessions.");
+				sleep(1); // Anti brut force protection. Must be same delay when user and password are not valid.
+				throw new RestException(401, $genericmessageerroruser);
 			}
 
 			// Check date validity
 			if ($fuser->isNotIntoValidityDateRange()) {
 				// User validity dates are no more valid
-				dol_syslog("The user login has a validity between [".$fuser->datestartvalidity." and ".$fuser->dateendvalidity."], curren date is ".dol_now());
-				throw new RestException(503, $genericmessageerroruser);
+				dol_syslog("functions_isallowed::check_user_api_key Authentication KO for '".$login."': The user login has a validity between [".$fuser->datestartvalidity." and ".$fuser->dateendvalidity."], curren date is ".dol_now());
+				sleep(1); // Anti brut force protection. Must be same delay when user and password are not valid.
+				throw new RestException(401, $genericmessageerroruser);
 			}
 
 			// User seems valid

+ 3 - 1
htdocs/api/index.php

@@ -48,7 +48,9 @@ if (!defined("NOLOGIN")) {
 if (!defined("NOSESSION")) {
 	define("NOSESSION", '1');
 }
-
+if (!defined("NODEFAULTVALUES")) {
+	define("NODEFAULTVALUES", '1');
+}
 
 // Force entity if a value is provided into HTTP header. Otherwise, will use the entity of user of token used.
 if (!empty($_SERVER['HTTP_DOLAPIENTITY'])) {

+ 1 - 1
htdocs/main.inc.php

@@ -1226,7 +1226,7 @@ if (!defined('NOLOGIN')) {
 	}
 } else {
 	// We may have NOLOGIN set, but NOREQUIREUSER not
-	if (!empty($user) && method_exists($user, 'loadDefaultValues')) {
+	if (!empty($user) && method_exists($user, 'loadDefaultValues') && !defined('NODEFAULTVALUES')) {
 		$user->loadDefaultValues();		// Load default values for everybody (works even if $user->id = 0
 	}
 }

+ 2 - 2
htdocs/user/card.php

@@ -1097,7 +1097,7 @@ if ($action == 'create' || $action == 'adduserldap') {
 		//$generated_password = getRandomPassword(false);
 		print '<tr><td>'.$langs->trans("ApiKey").'</td>';
 		print '<td>';
-		print '<input class="minwidth300 maxwidth400 widthcentpercentminusx" maxlength="128" type="text" id="api_key" name="api_key" value="'.GETPOST('api_key', 'alphanohtml').'" autocomplete="off">';
+		print '<input class="minwidth300 maxwidth400 widthcentpercentminusx" minlength="16" maxlength="128" type="text" id="api_key" name="api_key" value="'.GETPOST('api_key', 'alphanohtml').'" autocomplete="off">';
 		if (!empty($conf->use_javascript_ajax)) {
 			print '&nbsp;'.img_picto($langs->trans('Generate'), 'refresh', 'id="generate_api_key" class="linkobject"');
 		}
@@ -2472,7 +2472,7 @@ if ($action == 'create' || $action == 'adduserldap') {
 				print '<tr><td>'.$langs->trans("ApiKey").'</td>';
 				print '<td>';
 				if ($caneditpassword || $user->hasRight("api", "apikey", "generate")) {
-					print '<input class="minwidth300" maxsize="32" type="text" id="api_key" name="api_key" value="'.$object->api_key.'" autocomplete="off">';
+					print '<input class="minwidth300" minlength="16" maxlength="128" type="text" id="api_key" name="api_key" value="'.$object->api_key.'" autocomplete="off">';
 					if (!empty($conf->use_javascript_ajax)) {
 						print '&nbsp;'.img_picto($langs->trans('Generate'), 'refresh', 'id="generate_api_key" class="linkobject"');
 					}