From 1012d317e320468c0e704a3d93514334f4557a63 Mon Sep 17 00:00:00 2001 From: Bart Visscher Date: Tue, 4 Sep 2012 18:07:38 +0200 Subject: [PATCH 01/15] Add support for multiple login cookie tokens --- lib/base.php | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/lib/base.php b/lib/base.php index 51f8f4efc5b..3b873118f4f 100644 --- a/lib/base.php +++ b/lib/base.php @@ -449,6 +449,7 @@ class OC{ OC_App::loadApps(); OC_User::setupBackends(); if(isset($_GET["logout"]) and ($_GET["logout"])) { + OC_Preferences::deleteKey(OC_User::getUser(), 'login_token', $_COOKIE['oc_token']); OC_User::logout(); header("Location: ".OC::$WEBROOT.'/'); }else{ @@ -523,15 +524,17 @@ class OC{ OC_Log::write('core', 'Trying to login from cookie', OC_Log::DEBUG); } // confirm credentials in cookie - if(isset($_COOKIE['oc_token']) && OC_User::userExists($_COOKIE['oc_username']) && - OC_Preferences::getValue($_COOKIE['oc_username'], "login", "token") === $_COOKIE['oc_token']) - { - OC_User::setUserId($_COOKIE['oc_username']); - OC_Util::redirectToDefaultPage(); - } - else { - OC_User::unsetMagicInCookie(); + if(isset($_COOKIE['oc_token']) && OC_User::userExists($_COOKIE['oc_username'])) { + $tokens = OC_Preferences::getKeys($_COOKIE['oc_username'], 'login_token'); + $tokens[] = OC_Preferences::getValue($_COOKIE['oc_username'], 'login', 'token'); + if (in_array($_COOKIE['oc_token'], $tokens, true)) { + OC_User::setUserId($_COOKIE['oc_username']); + OC_Util::redirectToDefaultPage(); + // doesn't return + } + OC_Preferences::deleteKey($_POST['user'], 'login_token', $_COOKIE['oc_token']); } + OC_User::unsetMagicInCookie(); return true; } @@ -551,7 +554,7 @@ class OC{ OC_Log::write('core', 'Setting remember login to cookie', OC_Log::DEBUG); } $token = md5($_POST["user"].time().$_POST['password']); - OC_Preferences::setValue($_POST['user'], 'login', 'token', $token); + OC_Preferences::setValue($_POST['user'], 'login_token', $token, time()); OC_User::setMagicInCookie($_POST["user"], $token); } else { From 7f3e0b5566b8c3e54cb97d186da6d398f58f8b15 Mon Sep 17 00:00:00 2001 From: Bart Visscher Date: Tue, 4 Sep 2012 20:36:26 +0200 Subject: [PATCH 02/15] Cleanup login tokens on login success --- lib/base.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lib/base.php b/lib/base.php index 3b873118f4f..78f1f85f745 100644 --- a/lib/base.php +++ b/lib/base.php @@ -511,6 +511,17 @@ class OC{ OC_Util::displayLoginPage($error); } + protected static function cleanupLoginTokens($user) { + $cutoff = time() - 60*60*24*15; + $tokens = OC_Preferences::getKeys($_COOKIE['oc_username'], 'login_token'); + foreach($tokens as $token) { + $time = OC_Preferences::getValue($user, 'login_token', $token); + if ($time < $cutoff) { + OC_Preferences::deleteKey($user, 'login_token', $token); + } + } + } + protected static function tryRememberLogin() { if(!isset($_COOKIE["oc_remember_login"]) || !isset($_COOKIE["oc_token"]) @@ -528,6 +539,7 @@ class OC{ $tokens = OC_Preferences::getKeys($_COOKIE['oc_username'], 'login_token'); $tokens[] = OC_Preferences::getValue($_COOKIE['oc_username'], 'login', 'token'); if (in_array($_COOKIE['oc_token'], $tokens, true)) { + self::cleanupLoginTokens($_COOKIE['oc_username']); OC_User::setUserId($_COOKIE['oc_username']); OC_Util::redirectToDefaultPage(); // doesn't return @@ -549,6 +561,7 @@ class OC{ OC_User::setupBackends(); if(OC_User::login($_POST["user"], $_POST["password"])) { + self::cleanupLoginTokens($_POST['user']); if(!empty($_POST["remember_login"])) { if(defined("DEBUG") && DEBUG) { OC_Log::write('core', 'Setting remember login to cookie', OC_Log::DEBUG); From 4b799a69824f9f4a2ddb7df382b305b304b7d754 Mon Sep 17 00:00:00 2001 From: Bart Visscher Date: Wed, 5 Sep 2012 17:33:15 +0200 Subject: [PATCH 03/15] Make the lifetime of the remember login cookie --- config/config.sample.php | 5 ++++- lib/base.php | 2 +- lib/user.php | 7 ++++--- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/config/config.sample.php b/config/config.sample.php index 09eb6053c24..0c685945904 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -86,6 +86,9 @@ $CONFIG = array( /* Loglevel to start logging at. 0=DEBUG, 1=INFO, 2=WARN, 3=ERROR (default is WARN) */ "loglevel" => "", +/* Lifetime of the remember login cookie, default is 15 days */ +"remember_login_cookie_lifetime" => 60*60*24*15, + /* The directory where the user data is stored, default to data in the owncloud * directory. The sqlite database is also stored here, when sqlite is used. */ @@ -104,4 +107,4 @@ $CONFIG = array( 'writable' => true, ), ), -); \ No newline at end of file +); diff --git a/lib/base.php b/lib/base.php index 78f1f85f745..be93cb40e7c 100644 --- a/lib/base.php +++ b/lib/base.php @@ -512,7 +512,7 @@ class OC{ } protected static function cleanupLoginTokens($user) { - $cutoff = time() - 60*60*24*15; + $cutoff = time() - OC_Config::getValue('remember_login_cookie_lifetime', 60*60*24*15); $tokens = OC_Preferences::getKeys($_COOKIE['oc_username'], 'login_token'); foreach($tokens as $token) { $time = OC_Preferences::getValue($user, 'login_token', $token); diff --git a/lib/user.php b/lib/user.php index 7de2a4b7fe6..be8ddce88bb 100644 --- a/lib/user.php +++ b/lib/user.php @@ -472,9 +472,10 @@ class OC_User { */ public static function setMagicInCookie($username, $token) { $secure_cookie = OC_Config::getValue("forcessl", false); - setcookie("oc_username", $username, time()+60*60*24*15, '', '', $secure_cookie); - setcookie("oc_token", $token, time()+60*60*24*15, '', '', $secure_cookie); - setcookie("oc_remember_login", true, time()+60*60*24*15, '', '', $secure_cookie); + $expires = time() + OC_Config::getValue('remember_login_cookie_lifetime', 60*60*24*15); + setcookie("oc_username", $username, $expires, '', '', $secure_cookie); + setcookie("oc_token", $token, $expires, '', '', $secure_cookie); + setcookie("oc_remember_login", true, $expires, '', '', $secure_cookie); } /** From ee5d0f328fcaaabee00f3a3fda22c49f6ab84f58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20G=C3=B6hler?= Date: Thu, 11 Oct 2012 10:50:17 +0200 Subject: [PATCH 04/15] improve token security switched from time() to internal method OC_Util::generate_random_bytes() --- lib/base.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/base.php b/lib/base.php index be93cb40e7c..4dd69f3cc3e 100644 --- a/lib/base.php +++ b/lib/base.php @@ -566,7 +566,7 @@ class OC{ if(defined("DEBUG") && DEBUG) { OC_Log::write('core', 'Setting remember login to cookie', OC_Log::DEBUG); } - $token = md5($_POST["user"].time().$_POST['password']); + $token = md5($_POST["user"].OC_Util::generate_random_bytes(10).$_POST['password']); OC_Preferences::setValue($_POST['user'], 'login_token', $token, time()); OC_User::setMagicInCookie($_POST["user"], $token); } From 45f1c3f120e459a48ccb54b74cc97facb1946042 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20G=C3=B6hler?= Date: Thu, 11 Oct 2012 11:38:42 +0200 Subject: [PATCH 05/15] further improvements on multiple login token support outdated tokens are deleted before checking against cookies if an invalid token is used we delete all stored tokens for saveness used token will be replaced by a new one after successful authentication --- lib/base.php | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/base.php b/lib/base.php index 4dd69f3cc3e..cac416003e0 100644 --- a/lib/base.php +++ b/lib/base.php @@ -536,15 +536,25 @@ class OC{ } // confirm credentials in cookie if(isset($_COOKIE['oc_token']) && OC_User::userExists($_COOKIE['oc_username'])) { + // delete outdated cookies + cleanupLoginTokens($_COOKIE['oc_username']); + // get new tokens $tokens = OC_Preferences::getKeys($_COOKIE['oc_username'], 'login_token'); - $tokens[] = OC_Preferences::getValue($_COOKIE['oc_username'], 'login', 'token'); + // test cookies token against stored tokens if (in_array($_COOKIE['oc_token'], $tokens, true)) { - self::cleanupLoginTokens($_COOKIE['oc_username']); + // replace successfully used token with a new one + OC_Preferences::deleteKey($_POST['user'], 'login_token', $_COOKIE['oc_token']); + $token = md5($_POST["user"].OC_Util::generate_random_bytes(10).$_COOKIE['oc_token']); + OC_Preferences::setValue($_POST['user'], 'login_token', $token, time()); + OC_User::setMagicInCookie($_POST['user'], $token); + // login OC_User::setUserId($_COOKIE['oc_username']); OC_Util::redirectToDefaultPage(); // doesn't return } - OC_Preferences::deleteKey($_POST['user'], 'login_token', $_COOKIE['oc_token']); + // if you reach this point you are an attacker + // we remove all tokens to be save + OC_Preferences::deleteApp($_POST['user'], 'login_token'); } OC_User::unsetMagicInCookie(); return true; From 2ea06f67bd8bf8293afdff91fbbc42d021d2c211 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20G=C3=B6hler?= Date: Thu, 11 Oct 2012 11:54:40 +0200 Subject: [PATCH 06/15] delete all tokens on password change --- lib/base.php | 8 +++++--- lib/user.php | 2 ++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/base.php b/lib/base.php index cac416003e0..b02db4d05f5 100644 --- a/lib/base.php +++ b/lib/base.php @@ -552,9 +552,11 @@ class OC{ OC_Util::redirectToDefaultPage(); // doesn't return } - // if you reach this point you are an attacker - // we remove all tokens to be save - OC_Preferences::deleteApp($_POST['user'], 'login_token'); + // if you reach this point you have changed your password + // or you are an attacker + // we can not delete tokens here because users will reach + // this point multible times after a password change + //OC_Preferences::deleteApp($_POST['user'], 'login_token'); } OC_User::unsetMagicInCookie(); return true; diff --git a/lib/user.php b/lib/user.php index be8ddce88bb..11373a74014 100644 --- a/lib/user.php +++ b/lib/user.php @@ -329,6 +329,8 @@ class OC_User { } } } + // invalidate all login cookies + OC_Preferences::deleteApp($uid, 'login_token'); OC_Hook::emit( "OC_User", "post_setPassword", array( "uid" => $uid, "password" => $password )); return $success; } From eb79ccafe3c0fcb20282d56e6ab04e0c051f929a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20G=C3=B6hler?= Date: Thu, 11 Oct 2012 12:30:43 +0200 Subject: [PATCH 07/15] forgot a class name --- lib/base.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/base.php b/lib/base.php index b02db4d05f5..9cd2ee55b03 100644 --- a/lib/base.php +++ b/lib/base.php @@ -537,8 +537,8 @@ class OC{ // confirm credentials in cookie if(isset($_COOKIE['oc_token']) && OC_User::userExists($_COOKIE['oc_username'])) { // delete outdated cookies - cleanupLoginTokens($_COOKIE['oc_username']); - // get new tokens + self::cleanupLoginTokens($_COOKIE['oc_username']); + // get stored tokens $tokens = OC_Preferences::getKeys($_COOKIE['oc_username'], 'login_token'); // test cookies token against stored tokens if (in_array($_COOKIE['oc_token'], $tokens, true)) { From 38b9bffaeaaa6154c903f87100f20e07afe8261d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20G=C3=B6hler?= Date: Thu, 11 Oct 2012 12:53:34 +0200 Subject: [PATCH 08/15] call unsetMagicInCookie if token is invalid --- lib/base.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/base.php b/lib/base.php index 9cd2ee55b03..6105f525845 100644 --- a/lib/base.php +++ b/lib/base.php @@ -554,9 +554,9 @@ class OC{ } // if you reach this point you have changed your password // or you are an attacker - // we can not delete tokens here because users will reach + // we can not delete tokens here because users may reach // this point multible times after a password change - //OC_Preferences::deleteApp($_POST['user'], 'login_token'); + OC_User::unsetMagicInCookie(); } OC_User::unsetMagicInCookie(); return true; From 382f8d060cad9da01ff7c24fc937b3f0d1b46990 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20G=C3=B6hler?= Date: Thu, 11 Oct 2012 14:11:30 +0200 Subject: [PATCH 09/15] fixed wrong variable usage --- lib/base.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/base.php b/lib/base.php index 6105f525845..4007d241dee 100644 --- a/lib/base.php +++ b/lib/base.php @@ -513,7 +513,7 @@ class OC{ protected static function cleanupLoginTokens($user) { $cutoff = time() - OC_Config::getValue('remember_login_cookie_lifetime', 60*60*24*15); - $tokens = OC_Preferences::getKeys($_COOKIE['oc_username'], 'login_token'); + $tokens = OC_Preferences::getKeys($user, 'login_token'); foreach($tokens as $token) { $time = OC_Preferences::getValue($user, 'login_token', $token); if ($time < $cutoff) { @@ -543,10 +543,10 @@ class OC{ // test cookies token against stored tokens if (in_array($_COOKIE['oc_token'], $tokens, true)) { // replace successfully used token with a new one - OC_Preferences::deleteKey($_POST['user'], 'login_token', $_COOKIE['oc_token']); - $token = md5($_POST["user"].OC_Util::generate_random_bytes(10).$_COOKIE['oc_token']); - OC_Preferences::setValue($_POST['user'], 'login_token', $token, time()); - OC_User::setMagicInCookie($_POST['user'], $token); + OC_Preferences::deleteKey($_COOKIE['oc_username'], 'login_token', $_COOKIE['oc_token']); + $token = md5($_COOKIE['oc_username'].OC_Util::generate_random_bytes(10).$_COOKIE['oc_token']); + OC_Preferences::setValue($_COOKIE['oc_username'], 'login_token', $token, time()); + OC_User::setMagicInCookie($_COOKIE['oc_username'], $token); // login OC_User::setUserId($_COOKIE['oc_username']); OC_Util::redirectToDefaultPage(); From d8fe6fbb400e80e51655b78883925efe6b3af937 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20G=C3=B6hler?= Date: Thu, 11 Oct 2012 14:12:19 +0200 Subject: [PATCH 10/15] added a warning message to the log when a cookie is rejected --- lib/base.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/base.php b/lib/base.php index 4007d241dee..b030b38b5ee 100644 --- a/lib/base.php +++ b/lib/base.php @@ -556,6 +556,7 @@ class OC{ // or you are an attacker // we can not delete tokens here because users may reach // this point multible times after a password change + OC_Log::write('core', 'Authentication cookie rejected for user '.$_COOKIE['oc_username'], OC_Log::WARN); OC_User::unsetMagicInCookie(); } OC_User::unsetMagicInCookie(); From a6c4046f48752b957d591a9c9574caf6f18e6d40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20G=C3=B6hler?= Date: Fri, 12 Oct 2012 11:12:31 +0200 Subject: [PATCH 11/15] fixed typo and redundant method call --- lib/base.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/base.php b/lib/base.php index b030b38b5ee..bf2c2199ff5 100644 --- a/lib/base.php +++ b/lib/base.php @@ -555,9 +555,8 @@ class OC{ // if you reach this point you have changed your password // or you are an attacker // we can not delete tokens here because users may reach - // this point multible times after a password change + // this point multiple times after a password change OC_Log::write('core', 'Authentication cookie rejected for user '.$_COOKIE['oc_username'], OC_Log::WARN); - OC_User::unsetMagicInCookie(); } OC_User::unsetMagicInCookie(); return true; From b92fd984aa7f9281144b410ff703ca1796c10d41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20G=C3=B6hler?= Date: Sat, 13 Oct 2012 16:02:45 +0200 Subject: [PATCH 12/15] removed username and password from token generation --- lib/base.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/base.php b/lib/base.php index bf2c2199ff5..ebeec22088a 100644 --- a/lib/base.php +++ b/lib/base.php @@ -544,7 +544,7 @@ class OC{ if (in_array($_COOKIE['oc_token'], $tokens, true)) { // replace successfully used token with a new one OC_Preferences::deleteKey($_COOKIE['oc_username'], 'login_token', $_COOKIE['oc_token']); - $token = md5($_COOKIE['oc_username'].OC_Util::generate_random_bytes(10).$_COOKIE['oc_token']); + $token = OC_Util::generate_random_bytes(128); OC_Preferences::setValue($_COOKIE['oc_username'], 'login_token', $token, time()); OC_User::setMagicInCookie($_COOKIE['oc_username'], $token); // login @@ -578,7 +578,7 @@ class OC{ if(defined("DEBUG") && DEBUG) { OC_Log::write('core', 'Setting remember login to cookie', OC_Log::DEBUG); } - $token = md5($_POST["user"].OC_Util::generate_random_bytes(10).$_POST['password']); + $token = OC_Util::generate_random_bytes(128); OC_Preferences::setValue($_POST['user'], 'login_token', $token, time()); OC_User::setMagicInCookie($_POST["user"], $token); } From ae1f33db5453052a1b267b00b0c6fd7b6b70ff82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20G=C3=B6hler?= Date: Sun, 14 Oct 2012 20:47:31 +0200 Subject: [PATCH 13/15] implement fixed php session timeout and session id regeneration --- lib/base.php | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/lib/base.php b/lib/base.php index ebeec22088a..0ba028a68d2 100644 --- a/lib/base.php +++ b/lib/base.php @@ -264,8 +264,30 @@ class OC{ } public static function initSession() { + // prevents javascript from accessing php session cookies ini_set('session.cookie_httponly', '1;'); + + // (re)-initialize session session_start(); + + // regenerate session id periodically to avoid session fixation + if (!isset($_SESSION['SID_CREATED'])) { + $_SESSION['SID_CREATED'] = time(); + } else if (time() - $_SESSION['SID_CREATED'] > 900) { + session_regenerate_id(true); + $_SESSION['SID_CREATED'] = time(); + } + + // session timeout + if (isset($_SESSION['LAST_ACTIVITY']) && (time() - $_SESSION['LAST_ACTIVITY'] > 3600)) { + if (isset($_COOKIE[session_name()])) { + setcookie(session_name(), '', time() - 42000, '/'); + } + session_unset(); + session_destroy(); + session_start(); + } + $_SESSION['LAST_ACTIVITY'] = time(); } public static function init() { From 22fa23b4da06eef0cb2f22db25339838fc58a994 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20G=C3=B6hler?= Date: Sun, 14 Oct 2012 22:25:05 +0200 Subject: [PATCH 14/15] extend configkey column to hold 128bit values --- db_structure.xml | 2 +- lib/util.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/db_structure.xml b/db_structure.xml index 99a30cb6137..a17ab90b8a1 100644 --- a/db_structure.xml +++ b/db_structure.xml @@ -395,7 +395,7 @@ text true - 64 + 128 diff --git a/lib/util.php b/lib/util.php index 68c4920258f..707100a9bcc 100755 --- a/lib/util.php +++ b/lib/util.php @@ -83,7 +83,7 @@ class OC_Util { */ public static function getVersion() { // hint: We only can count up. So the internal version number of ownCloud 4.5 will be 4.90.0. This is not visible to the user - return array(4,91,00); + return array(4,91,01); } /** From 8be9c04a3a6f84c8673e0b6db3305cf0f427a43b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20G=C3=B6hler?= Date: Mon, 15 Oct 2012 20:00:33 +0200 Subject: [PATCH 15/15] 128byte is not 128bit - now we realy use 256bit (same as PHPSESSID) --- db_structure.xml | 2 +- lib/base.php | 4 ++-- lib/util.php | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/db_structure.xml b/db_structure.xml index a17ab90b8a1..99a30cb6137 100644 --- a/db_structure.xml +++ b/db_structure.xml @@ -395,7 +395,7 @@ text true - 128 + 64 diff --git a/lib/base.php b/lib/base.php index 0ba028a68d2..c9dcac3cbb9 100644 --- a/lib/base.php +++ b/lib/base.php @@ -566,7 +566,7 @@ class OC{ if (in_array($_COOKIE['oc_token'], $tokens, true)) { // replace successfully used token with a new one OC_Preferences::deleteKey($_COOKIE['oc_username'], 'login_token', $_COOKIE['oc_token']); - $token = OC_Util::generate_random_bytes(128); + $token = OC_Util::generate_random_bytes(32); OC_Preferences::setValue($_COOKIE['oc_username'], 'login_token', $token, time()); OC_User::setMagicInCookie($_COOKIE['oc_username'], $token); // login @@ -600,7 +600,7 @@ class OC{ if(defined("DEBUG") && DEBUG) { OC_Log::write('core', 'Setting remember login to cookie', OC_Log::DEBUG); } - $token = OC_Util::generate_random_bytes(128); + $token = OC_Util::generate_random_bytes(32); OC_Preferences::setValue($_POST['user'], 'login_token', $token, time()); OC_User::setMagicInCookie($_POST["user"], $token); } diff --git a/lib/util.php b/lib/util.php index 707100a9bcc..68c4920258f 100755 --- a/lib/util.php +++ b/lib/util.php @@ -83,7 +83,7 @@ class OC_Util { */ public static function getVersion() { // hint: We only can count up. So the internal version number of ownCloud 4.5 will be 4.90.0. This is not visible to the user - return array(4,91,01); + return array(4,91,00); } /**