From 743826bbf34b82b92371cf7e9b0478897188c046 Mon Sep 17 00:00:00 2001 From: Christian Reiner Date: Fri, 28 Sep 2012 13:30:44 +0200 Subject: [PATCH 01/10] Reimplementation of CSRF protection including autorefresh --- core/ajax/requesttoken.php | 41 +++++++++++++++++++++++++ core/js/eventsource.js | 2 +- core/js/requesttoken.js | 55 ++++++++++++++++++++++++++++++++++ core/templates/layout.user.php | 9 ++---- lib/base.php | 2 ++ lib/template.php | 6 ---- lib/util.php | 36 +++++++++++++++------- 7 files changed, 127 insertions(+), 24 deletions(-) create mode 100644 core/ajax/requesttoken.php create mode 100644 core/js/requesttoken.js diff --git a/core/ajax/requesttoken.php b/core/ajax/requesttoken.php new file mode 100644 index 0000000000..96d5402e62 --- /dev/null +++ b/core/ajax/requesttoken.php @@ -0,0 +1,41 @@ + +* +* This library is free software; you can redistribute it and/or +* modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE +* License as published by the Free Software Foundation; either +* version 3 of the license, or any later version. +* +* This library is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU AFFERO GENERAL PUBLIC LICENSE for more details. +* +* You should have received a copy of the GNU Affero General Public +* License along with this library. +* If not, see . +* +*/ + +/** + * @file core/ajax/requesttoken.php + * @brief Ajax method to retrieve a fresh request protection token for ajax calls + * @return json: success/error state indicator including a fresh request token + * @author Christian Reiner + */ +require_once '../../lib/base.php'; + +// don't load apps or filesystem for this task +$RUNTIME_NOAPPS = TRUE; +$RUNTIME_NOSETUPFS = TRUE; + +// Sanity checks +// using OCP\JSON::callCheck() below protects the token refreshing itself. +//OCP\JSON::callCheck ( ); +OCP\JSON::checkLoggedIn ( ); +// hand out a fresh token +OCP\JSON::success ( array ( 'token' => OCP\Util::callRegister() ) ); +?> diff --git a/core/js/eventsource.js b/core/js/eventsource.js index e3ad7e3a67..45c63715a7 100644 --- a/core/js/eventsource.js +++ b/core/js/eventsource.js @@ -40,7 +40,7 @@ OC.EventSource=function(src,data){ dataStr+=name+'='+encodeURIComponent(data[name])+'&'; } } - dataStr+='requesttoken='+OC.EventSource.requesttoken; + dataStr+='requesttoken='+OC.Request.Token; if(!this.useFallBack && typeof EventSource !='undefined'){ this.source=new EventSource(src+'?'+dataStr); this.source.onmessage=function(e){ diff --git a/core/js/requesttoken.js b/core/js/requesttoken.js new file mode 100644 index 0000000000..0d78cd7e93 --- /dev/null +++ b/core/js/requesttoken.js @@ -0,0 +1,55 @@ +/** + * ownCloud + * + * @file core/js/requesttoken.js + * @brief Routine to refresh the Request protection request token periodically + * @author Christian Reiner (arkascha) + * @copyright 2011-2012 Christian Reiner + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE + * License as published by the Free Software Foundation; either + * version 3 of the license, or any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU AFFERO GENERAL PUBLIC LICENSE for more details. + * + * You should have received a copy of the GNU Affero General Public + * License along with this library. + * If not, see . + * + */ + +OC.Request = { + // the request token + Token: {}, + // the lifespan span (in secs) + Lifespan: {}, + // method to refresh the local request token periodically + Refresh: function(){ + // just a client side console log to preserve efficiency + console.log("refreshing request token (lifebeat)"); + var dfd=new $.Deferred(); + $.ajax({ + type: 'POST', + url: OC.filePath('core','ajax','requesttoken.php'), + cache: false, + data: { }, + dataType: 'json' + }).done(function(response){ + // store refreshed token inside this class + OC.Request.Token=response.token; + dfd.resolve(); + }).fail(dfd.reject); + return dfd; + } +} +// accept requesttoken and lifespan into the OC namespace +OC.Request.Token = oc_requesttoken; +OC.Request.Lifespan = oc_requestlifespan; +// refresh the request token periodically shortly before it becomes invalid on the server side +setInterval(OC.Request.Refresh,Math.floor(1000*OC.Request.Lifespan*0.93)), // 93% of lifespan value, close to when the token expires +// early bind token as additional ajax argument for every single request +$(document).bind('ajaxSend', function(elm, xhr, s){xhr.setRequestHeader('requesttoken', OC.Request.Token);}); diff --git a/core/templates/layout.user.php b/core/templates/layout.user.php index 679be2657d..25af64c8d5 100644 --- a/core/templates/layout.user.php +++ b/core/templates/layout.user.php @@ -11,6 +11,8 @@ var oc_webroot = ''; var oc_appswebroots = ; var oc_current_user = ''; + var oc_requesttoken = ''; + var oc_requestlifespan = ''; @@ -24,13 +26,6 @@ echo '/>'; ?> - diff --git a/lib/base.php b/lib/base.php index f6afc8fe2f..5a2decc6f6 100644 --- a/lib/base.php +++ b/lib/base.php @@ -240,6 +240,8 @@ class OC{ OC_Util::addScript( "jquery-tipsy" ); OC_Util::addScript( "oc-dialogs" ); OC_Util::addScript( "js" ); + // request protection token MUST be defined after the jquery library but before any $('document').ready() + OC_Util::addScript( "requesttoken" ); OC_Util::addScript( "eventsource" ); OC_Util::addScript( "config" ); //OC_Util::addScript( "multiselect" ); diff --git a/lib/template.php b/lib/template.php index 0987d6f0d8..0033683b66 100644 --- a/lib/template.php +++ b/lib/template.php @@ -155,9 +155,6 @@ class OC_Template{ $this->renderas = $renderas; $this->application = $app; $this->vars = array(); - if($renderas == 'user') { - $this->vars['requesttoken'] = OC_Util::callRegister(); - } $parts = explode('/', $app); // fix translation when app is something like core/lostpassword $this->l10n = OC_L10N::get($parts[0]); header('X-Frame-Options: Sameorigin'); @@ -372,9 +369,6 @@ class OC_Template{ if( $this->renderas ) { $page = new OC_TemplateLayout($this->renderas); - if($this->renderas == 'user') { - $page->assign('requesttoken', $this->vars['requesttoken']); - } // Add custom headers $page->assign('headers',$this->headers, false); diff --git a/lib/util.php b/lib/util.php index 0841246425..b14664c9d1 100755 --- a/lib/util.php +++ b/lib/util.php @@ -416,14 +416,29 @@ class OC_Util { } /** - * @brief Register an get/post call. This is important to prevent CSRF attacks - * Todo: Write howto + * @brief Static lifespan (in seconds) when a request token expires. + * @see OC_Util::callRegister() + * @see OC_Util::isCallRegistered() + * @description + * Also required for the client side to compute the piont in time when to + * request a fresh token. The client will do so when nearly 97% of the + * timespan coded here has expired. + */ + public static $callLifespan = 3600; // 3600 secs = 1 hour + + /** + * @brief Register an get/post call. Important to prevent CSRF attacks. + * @todo Write howto: CSRF protection guide * @return $token Generated token. + * @description + * Creates a 'request token' (random) and stores it inside the session. + * Ever subsequent (ajax) request must use such a valid token to succeed, + * otherwise the request will be denied as a protection against CSRF. + * The tokens expire after a fixed lifespan. + * @see OC_Util::$callLifespan + * @see OC_Util::isCallRegistered() */ public static function callRegister() { - //mamimum time before token exires - $maxtime=(60*60); // 1 hour - // generate a random token. $token=mt_rand(1000,9000).mt_rand(1000,9000).mt_rand(1000,9000); @@ -436,7 +451,8 @@ class OC_Util { foreach($_SESSION as $key=>$value) { // search all tokens in the session if(substr($key,0,12)=='requesttoken') { - if($value+$maxtime Date: Fri, 28 Sep 2012 16:38:25 +0200 Subject: [PATCH 02/10] Added name to AUTHORS file, since mentioned in file headers. --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index b701c768d3..2404174e32 100644 --- a/AUTHORS +++ b/AUTHORS @@ -17,6 +17,7 @@ ownCloud is written by: Sam Tuke Simon Birnbach Lukas Reschke + Christian Reiner … With help from many libraries and frameworks including: From f8f73e267550a46dc4cee5b025e83d786cddb1bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Schie=C3=9Fle?= Date: Fri, 28 Sep 2012 17:44:46 +0200 Subject: [PATCH 03/10] move back to "lastmodified" property since "getlastmodified" is protected by webdav --- lib/connector/sabre/node.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/connector/sabre/node.php b/lib/connector/sabre/node.php index 55fa0dfde6..bdedc030c8 100644 --- a/lib/connector/sabre/node.php +++ b/lib/connector/sabre/node.php @@ -23,8 +23,7 @@ abstract class OC_Connector_Sabre_Node implements Sabre_DAV_INode, Sabre_DAV_IProperties { const GETETAG_PROPERTYNAME = '{DAV:}getetag'; - const LASTMODIFIED_PROPERTYNAME_DEPRECIATED = '{DAV:}lastmodified'; // FIXME: keept for the transition period, can be removed for OC 4.5.1 if the sync client update too - const GETLASTMODIFIED_PROPERTYNAME = '{DAV:}getlastmodified'; + const LASTMODIFIED_PROPERTYNAME = '{DAV:}lastmodified'; /** * The path to the current node @@ -151,9 +150,8 @@ abstract class OC_Connector_Sabre_Node implements Sabre_DAV_INode, Sabre_DAV_IPr $query->execute( array( OC_User::getUser(), $this->path, $propertyName )); } } - else { //FIXME: first part of if statement can be removed together with the LASTMODIFIED_PROPERTYNAME_DEPRECIATED const for oc4.5.1 if the sync client was updated too - if( strcmp( $propertyName, self::LASTMODIFIED_PROPERTYNAME_DEPRECIATED) === 0 || - strcmp( $propertyName, self::GETLASTMODIFIED_PROPERTYNAME) === 0 ) { + else { + if( strcmp( $propertyName, self::LASTMODIFIED_PROPERTYNAME) === 0 ) { $this->touch($propertyValue); } else { if(!array_key_exists( $propertyName, $existing )) { From 35357f3afb90dec1d0a9755ab0e0504a916c5e5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Schie=C3=9Fle?= Date: Fri, 28 Sep 2012 18:47:54 +0200 Subject: [PATCH 04/10] etag has to be removed after version rollback to enable the sync client to detect the changes (bug #1829) --- lib/filesystem.php | 9 ++++++++- lib/filesystemview.php | 3 ++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/filesystem.php b/lib/filesystem.php index f5c10923b3..5516aae397 100644 --- a/lib/filesystem.php +++ b/lib/filesystem.php @@ -521,12 +521,19 @@ class OC_Filesystem{ return self::$defaultInstance->hasUpdated($path,$time); } - static public function removeETagHook($params) { + static public function removeETagHook($params, $root = false) { if (isset($params['path'])) { $path=$params['path']; } else { $path=$params['oldpath']; } + + if (root) { // reduce path to the required part of it (no 'username/files') + $fakeRootView = new OC_FilesystemView($root); + $count = 1; + $path=str_replace(OC_App::getStorage("files")->getAbsolutePath(), "", $fakeRootView->getAbsolutePath($path), $count); + } + $path = self::normalizePath($path); OC_Connector_Sabre_Node::removeETagPropertyForPath($path); } diff --git a/lib/filesystemview.php b/lib/filesystemview.php index 02a0b52105..2950ced5f9 100644 --- a/lib/filesystemview.php +++ b/lib/filesystemview.php @@ -451,8 +451,9 @@ class OC_FilesystemView { OC_Filesystem::signal_post_write, array( OC_Filesystem::signal_param_path => $path2) ); - } else { // no real copy, file comes from somewhere else, e.g. version rollback -> just update the file cache without all the other post_write actions + } else { // no real copy, file comes from somewhere else, e.g. version rollback -> just update the file cache and the webdav properties without all the other post_write actions OC_FileCache_Update::update($path2, $this->fakeRoot); + OC_Filesystem::removeETagHook(array("path" => $path2), $this->fakeRoot); } return $result; } From 71454b1bca0accd1ab5d7628169d4714bb682030 Mon Sep 17 00:00:00 2001 From: Christian Reiner Date: Fri, 28 Sep 2012 18:57:20 +0200 Subject: [PATCH 05/10] Fix to preserve backward compatibility for apps creating static links containing the request token (currently the contacts app and maybe some 3rd party implementations) --- core/templates/layout.user.php | 4 ++-- lib/template.php | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/core/templates/layout.user.php b/core/templates/layout.user.php index 25af64c8d5..b6d8a7604a 100644 --- a/core/templates/layout.user.php +++ b/core/templates/layout.user.php @@ -11,8 +11,8 @@ var oc_webroot = ''; var oc_appswebroots = ; var oc_current_user = ''; - var oc_requesttoken = ''; - var oc_requestlifespan = ''; + var oc_requesttoken = ''; + var oc_requestlifespan = ''; diff --git a/lib/template.php b/lib/template.php index 0033683b66..681b3f0b14 100644 --- a/lib/template.php +++ b/lib/template.php @@ -155,6 +155,10 @@ class OC_Template{ $this->renderas = $renderas; $this->application = $app; $this->vars = array(); + if($renderas == 'user') { + $this->vars['requesttoken'] = OC_Util::callRegister(); + $this->vars['requestlifespan'] = OC_Util::$callLifespan; + } $parts = explode('/', $app); // fix translation when app is something like core/lostpassword $this->l10n = OC_L10N::get($parts[0]); header('X-Frame-Options: Sameorigin'); @@ -369,6 +373,10 @@ class OC_Template{ if( $this->renderas ) { $page = new OC_TemplateLayout($this->renderas); + if($this->renderas == 'user') { + $page->assign('requesttoken', $this->vars['requesttoken']); + $page->assign('requestlifespan', $this->vars['requestlifespan']); + } // Add custom headers $page->assign('headers',$this->headers, false); From 4e2f575938f565f8589919bece6f6da3c313eb4d Mon Sep 17 00:00:00 2001 From: Bart Visscher Date: Fri, 28 Sep 2012 17:03:00 +0200 Subject: [PATCH 06/10] Correctly fix oc-1016 and fix downloading of files --- apps/files/js/fileactions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js index f579d8530e..e184cbfa91 100644 --- a/apps/files/js/fileactions.js +++ b/apps/files/js/fileactions.js @@ -157,7 +157,7 @@ $(document).ready(function(){ var downloadScope = 'file'; } FileActions.register(downloadScope,'Download', OC.PERMISSION_READ, function(){return OC.imagePath('core','actions/download');},function(filename){ - window.location=OC.filePath('files', 'ajax', 'download.php') + encodeURIComponent('?files='+encodeURIComponent(filename)+'&dir='+encodeURIComponent($('#dir').val())); + window.location=OC.filePath('files', 'ajax', 'download.php') + '&files='+encodeURIComponent(filename)+'&dir='+encodeURIComponent($('#dir').val()); }); }); From 366ae6661d1491a7f3db9396e05fd24034a1102f Mon Sep 17 00:00:00 2001 From: Bart Visscher Date: Fri, 28 Sep 2012 21:14:59 +0200 Subject: [PATCH 07/10] Simplify generating file search results --- lib/search/provider/file.php | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/lib/search/provider/file.php b/lib/search/provider/file.php index 135e40667b..e21278f391 100644 --- a/lib/search/provider/file.php +++ b/lib/search/provider/file.php @@ -5,29 +5,35 @@ class OC_Search_Provider_File extends OC_Search_Provider{ $files=OC_FileCache::search($query,true); $results=array(); foreach($files as $fileData) { - $file=$fileData['path']; - $mime=$fileData['mimetype']; + $path = $fileData['path']; + $mime = $fileData['mimetype']; + + $name = basename($path); + $text = ''; if($mime=='httpd/unix-directory') { - $results[]=new OC_Search_Result(basename($file),'',OC_Helper::linkTo( 'files', 'index.php', array('dir' => $file)),'Files'); + $link = OC_Helper::linkTo( 'files', 'index.php', array('dir' => $path)); + $type = 'Files'; }else{ - $mimeBase=$fileData['mimepart']; + $link = OC_Helper::linkTo( 'files', 'download.php', array('file' => $path)); + $mimeBase = $fileData['mimepart']; switch($mimeBase) { case 'audio': break; case 'text': - $results[]=new OC_Search_Result(basename($file),'',OC_Helper::linkTo( 'files', 'download.php', array('file' => $file) ),'Text'); + $type = 'Text'; break; case 'image': - $results[]=new OC_Search_Result(basename($file),'',OC_Helper::linkTo( 'files', 'download.php', array('file' => $file) ),'Images'); + $type = 'Images'; break; default: if($mime=='application/xml') { - $results[]=new OC_Search_Result(basename($file),'',OC_Helper::linkTo( 'files', 'download.php', array('file' => $file) ),'Text'); + $type = 'Text'; }else{ - $results[]=new OC_Search_Result(basename($file),'',OC_Helper::linkTo( 'files', 'download.php', array('file' => $file) ),'Files'); + $type = 'Files'; } } } + $results[] = new OC_Search_Result($name, $text, $link, $type); } return $results; } From 24bb7d16b7f72b4438cb79ff55cdcd4a5e66062c Mon Sep 17 00:00:00 2001 From: Bart Visscher Date: Fri, 28 Sep 2012 21:15:48 +0200 Subject: [PATCH 08/10] urlencode filename in search result, fixes problems with & in name --- lib/search/provider/file.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/search/provider/file.php b/lib/search/provider/file.php index e21278f391..21fae0c1ce 100644 --- a/lib/search/provider/file.php +++ b/lib/search/provider/file.php @@ -10,6 +10,7 @@ class OC_Search_Provider_File extends OC_Search_Provider{ $name = basename($path); $text = ''; + $path = urlencode($path); if($mime=='httpd/unix-directory') { $link = OC_Helper::linkTo( 'files', 'index.php', array('dir' => $path)); $type = 'Files'; From 9c4c79346c9ccedc72998566911d1cd2a2e57b11 Mon Sep 17 00:00:00 2001 From: Bart Visscher Date: Fri, 28 Sep 2012 21:18:11 +0200 Subject: [PATCH 09/10] After selecting a search result, hide the results --- search/js/result.js | 1 + 1 file changed, 1 insertion(+) diff --git a/search/js/result.js b/search/js/result.js index aaecde08c6..cadb0d0aab 100644 --- a/search/js/result.js +++ b/search/js/result.js @@ -25,6 +25,7 @@ OC.search.showResults=function(results){ parent.load(OC.filePath('search','templates','part.results.php'),function(){ OC.search.showResults.loaded=true; $('#searchresults').click(function(event){ + OC.search.hide(); event.stopPropagation(); }); $(window).click(function(event){ From fed34aecfa5b36c031ed754ed245549aa4a4194b Mon Sep 17 00:00:00 2001 From: Bart Visscher Date: Fri, 28 Sep 2012 21:30:06 +0200 Subject: [PATCH 10/10] Fix syntax error in removeETagHook --- lib/filesystem.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/filesystem.php b/lib/filesystem.php index 5516aae397..c6da826a33 100644 --- a/lib/filesystem.php +++ b/lib/filesystem.php @@ -527,13 +527,13 @@ class OC_Filesystem{ } else { $path=$params['oldpath']; } - - if (root) { // reduce path to the required part of it (no 'username/files') - $fakeRootView = new OC_FilesystemView($root); - $count = 1; + + if ($root) { // reduce path to the required part of it (no 'username/files') + $fakeRootView = new OC_FilesystemView($root); + $count = 1; $path=str_replace(OC_App::getStorage("files")->getAbsolutePath(), "", $fakeRootView->getAbsolutePath($path), $count); } - + $path = self::normalizePath($path); OC_Connector_Sabre_Node::removeETagPropertyForPath($path); }