From 4ea205e2629455d908a30367d5f42f9d07c1fd45 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 16 Apr 2015 22:39:44 +0200 Subject: [PATCH 1/5] Block old legacy clients This Pull Request introduces a SabreDAV plugin that will block all older clients than 1.6.1 to connect and sync with the ownCloud instance. This has multiple reasons: 1. Old ownCloud client versions before 1.6.0 are not properly working with sticky cookies for load balancers and thus generating sessions en masse 2. Old ownCloud client versions tend to be horrible buggy In some cases we had in 80minutes about 10'000 sessions created by a single user. While this change set does not really "fix" the problem as 3rdparty legacy clients are affected as well, it is a good work-around and hopefully should force users to update their client --- apps/files/appinfo/remote.php | 1 + config/config.sample.php | 8 ++ .../sabre/blocklegacyclientplugin.php | 76 ++++++++++++ .../sabre/BlockLegacyClientPluginTest.php | 117 ++++++++++++++++++ 4 files changed, 202 insertions(+) create mode 100644 lib/private/connector/sabre/blocklegacyclientplugin.php create mode 100644 tests/lib/connector/sabre/BlockLegacyClientPluginTest.php diff --git a/apps/files/appinfo/remote.php b/apps/files/appinfo/remote.php index 1e54fc10ef..b8dc68f1f8 100644 --- a/apps/files/appinfo/remote.php +++ b/apps/files/appinfo/remote.php @@ -43,6 +43,7 @@ $server->setBaseUri($baseuri); // Load plugins $defaults = new OC_Defaults(); +$server->addPlugin(new \OC\Connector\Sabre\BlockLegacyClientPlugin(\OC::$server->getConfig())); $server->addPlugin(new \Sabre\DAV\Auth\Plugin($authBackend, $defaults->getName())); // FIXME: The following line is a workaround for legacy components relying on being able to send a GET to / $server->addPlugin(new \OC\Connector\Sabre\DummyGetResponsePlugin()); diff --git a/config/config.sample.php b/config/config.sample.php index 61ae59542d..e0e4a3cae9 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -799,6 +799,14 @@ $CONFIG = array( */ 'cipher' => 'AES-256-CFB', +/** + * The minimum ownCloud desktop client version that is required to sync with + * this instance. Defaults to the official supported releases by ownCloud. + * + * When adjusting this please be aware of the fact that older versions may + * be buggy and for best user experience we recommend setting + */ +'minimum.supported.desktop.version' => '1.7.0', /** * Memory caching backend configuration diff --git a/lib/private/connector/sabre/blocklegacyclientplugin.php b/lib/private/connector/sabre/blocklegacyclientplugin.php new file mode 100644 index 0000000000..4d595b5f6d --- /dev/null +++ b/lib/private/connector/sabre/blocklegacyclientplugin.php @@ -0,0 +1,76 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program 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, version 3, + * along with this program. If not, see + * + */ + +namespace OC\Connector\Sabre; + +use OC\ServiceUnavailableException; +use OCP\IConfig; +use Sabre\HTTP\RequestInterface; +use Sabre\DAV\ServerPlugin; +use Sabre\DAV\Server; + +/** + * Class BlockLegacyClientPlugin is used to detect old legacy sync clients and + * returns a 503 status to those clients. + * + * @package OC\Connector\Sabre + */ +class BlockLegacyClientPlugin extends ServerPlugin { + /** @var Server */ + protected $server; + /** @var IConfig */ + protected $config; + + /** + * @param IConfig $config + */ + public function __construct(IConfig $config) { + $this->config = $config; + } + + /** + * @param Server $server + * @return void + */ + public function initialize(Server $server) { + $this->server = $server; + $this->server->on('beforeMethod', [$this, 'beforeHandler'], 200); + } + + /** + * Detects all unsupported clients and throws a ServiceUnavailableException + * which will result in a 503 to them. + * @param RequestInterface $request + * @throws ServiceUnavailableException If the client version is not supported + */ + public function beforeHandler(RequestInterface $request) { + $userAgent = $request->getHeader('User-Agent'); + $minimumSupportedDesktopVersion = $this->config->getSystemValue('minimum.supported.desktop.version', '1.7.0'); + + // Match on the mirall version which is in scheme "Mozilla/5.0 (%1) mirall/%2" or + // "mirall/%1" for older releases + preg_match("/(?:mirall\\/)([\d.]+)/i", $userAgent, $versionMatches); + if(isset($versionMatches[1]) && + version_compare($versionMatches[1], $minimumSupportedDesktopVersion) === -1) { + throw new ServiceUnavailableException('Unsupported client version.'); + } + } +} diff --git a/tests/lib/connector/sabre/BlockLegacyClientPluginTest.php b/tests/lib/connector/sabre/BlockLegacyClientPluginTest.php new file mode 100644 index 0000000000..ed735f0684 --- /dev/null +++ b/tests/lib/connector/sabre/BlockLegacyClientPluginTest.php @@ -0,0 +1,117 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program 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, version 3, + * along with this program. If not, see + * + */ +namespace Test\Connector\Sabre; + +use OC\Connector\Sabre\BlockLegacyClientPlugin; +use Test\TestCase; +use OCP\IConfig; + +/** + * Class BlockLegacyClientPluginTest + * + * @package Test\Connector\Sabre + */ +class BlockLegacyClientPluginTest extends TestCase { + /** @var IConfig */ + private $config; + /** @var BlockLegacyClientPlugin */ + private $blockLegacyClientVersionPlugin; + + public function setUp() { + parent::setUp(); + + $this->config = $this->getMock('\OCP\IConfig'); + $this->blockLegacyClientVersionPlugin = new BlockLegacyClientPlugin($this->config); + } + + /** + * @return array + */ + public function oldDesktopClientProvider() { + return [ + ['Mozilla/5.0 (1.5.0) mirall/1.5.0'], + ['mirall/1.5.0'], + ['mirall/1.5.4'], + ['mirall/1.6.0'], + ['Mozilla/5.0 (Bogus Text) mirall/1.6.9'], + ]; + } + + /** + * @dataProvider oldDesktopClientProvider + * @param string $userAgent + * @expectedException \OC\ServiceUnavailableException + * @expectedExceptionMessage Unsupported client version. + */ + public function testBeforeHandlerException($userAgent) { + /** @var \Sabre\HTTP\RequestInterface $request */ + $request = $this->getMock('\Sabre\HTTP\RequestInterface'); + $request + ->expects($this->once()) + ->method('getHeader') + ->with('User-Agent') + ->will($this->returnValue($userAgent)); + + $this->config + ->expects($this->once()) + ->method('getSystemValue') + ->with('minimum.supported.desktop.version', '1.7.0') + ->will($this->returnValue('1.7.0')); + + $this->blockLegacyClientVersionPlugin->beforeHandler($request); + } + + /** + * @return array + */ + public function newAndAlternateDesktopClientProvider() { + return [ + ['Mozilla/5.0 (1.7.0) mirall/1.7.0'], + ['mirall/1.8.3'], + ['mirall/1.7.2'], + ['mirall/1.7.0'], + ['Mozilla/5.0 (Bogus Text) mirall/1.9.3'], + ]; + } + + /** + * @dataProvider newAndAlternateDesktopClientProvider + * @param string $userAgent + */ + public function testBeforeHandlerSucess($userAgent) { + /** @var \Sabre\HTTP\RequestInterface $request */ + $request = $this->getMock('\Sabre\HTTP\RequestInterface'); + $request + ->expects($this->once()) + ->method('getHeader') + ->with('User-Agent') + ->will($this->returnValue($userAgent)); + + $this->config + ->expects($this->once()) + ->method('getSystemValue') + ->with('minimum.supported.desktop.version', '1.7.0') + ->will($this->returnValue('1.7.0')); + + $this->blockLegacyClientVersionPlugin->beforeHandler($request); + } + +} From 6b31d325d6bfaf8251e4f78556cd23b9c087072f Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 20 Apr 2015 11:12:58 +0200 Subject: [PATCH 2/5] Wording --- config/config.sample.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/config.sample.php b/config/config.sample.php index e0e4a3cae9..a62438a20b 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -804,7 +804,7 @@ $CONFIG = array( * this instance. Defaults to the official supported releases by ownCloud. * * When adjusting this please be aware of the fact that older versions may - * be buggy and for best user experience we recommend setting + * be buggy and for best user experience we recommend to not change this value. */ 'minimum.supported.desktop.version' => '1.7.0', From ed0b465cf9c2d4f257d8f1e2fabcd443f133ca38 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 20 Apr 2015 12:53:40 +0200 Subject: [PATCH 3/5] Use 403 instead a 50x response --- .../sabre/blocklegacyclientplugin.php | 19 +++++++++---------- .../sabre/BlockLegacyClientPluginTest.php | 3 ++- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/private/connector/sabre/blocklegacyclientplugin.php b/lib/private/connector/sabre/blocklegacyclientplugin.php index 4d595b5f6d..7da6ff563d 100644 --- a/lib/private/connector/sabre/blocklegacyclientplugin.php +++ b/lib/private/connector/sabre/blocklegacyclientplugin.php @@ -21,20 +21,19 @@ namespace OC\Connector\Sabre; -use OC\ServiceUnavailableException; use OCP\IConfig; use Sabre\HTTP\RequestInterface; use Sabre\DAV\ServerPlugin; -use Sabre\DAV\Server; +use Sabre\DAV\Exception; /** * Class BlockLegacyClientPlugin is used to detect old legacy sync clients and - * returns a 503 status to those clients. + * returns a 403 status to those clients * * @package OC\Connector\Sabre */ class BlockLegacyClientPlugin extends ServerPlugin { - /** @var Server */ + /** @var \Sabre\DAV\Server */ protected $server; /** @var IConfig */ protected $config; @@ -47,19 +46,19 @@ class BlockLegacyClientPlugin extends ServerPlugin { } /** - * @param Server $server + * @param \Sabre\DAV\ $server * @return void */ - public function initialize(Server $server) { + public function initialize(\Sabre\DAV\Server $server) { $this->server = $server; $this->server->on('beforeMethod', [$this, 'beforeHandler'], 200); } /** - * Detects all unsupported clients and throws a ServiceUnavailableException - * which will result in a 503 to them. + * Detects all unsupported clients and throws a \Sabre\DAV\Exception\Forbidden + * exception which will result in a 403 to them. * @param RequestInterface $request - * @throws ServiceUnavailableException If the client version is not supported + * @throws \Sabre\DAV\Exception\Forbidden If the client version is not supported */ public function beforeHandler(RequestInterface $request) { $userAgent = $request->getHeader('User-Agent'); @@ -70,7 +69,7 @@ class BlockLegacyClientPlugin extends ServerPlugin { preg_match("/(?:mirall\\/)([\d.]+)/i", $userAgent, $versionMatches); if(isset($versionMatches[1]) && version_compare($versionMatches[1], $minimumSupportedDesktopVersion) === -1) { - throw new ServiceUnavailableException('Unsupported client version.'); + throw new \Sabre\DAV\Exception\Forbidden('Unsupported client version.'); } } } diff --git a/tests/lib/connector/sabre/BlockLegacyClientPluginTest.php b/tests/lib/connector/sabre/BlockLegacyClientPluginTest.php index ed735f0684..2c7835dd73 100644 --- a/tests/lib/connector/sabre/BlockLegacyClientPluginTest.php +++ b/tests/lib/connector/sabre/BlockLegacyClientPluginTest.php @@ -18,6 +18,7 @@ * along with this program. If not, see * */ + namespace Test\Connector\Sabre; use OC\Connector\Sabre\BlockLegacyClientPlugin; @@ -58,7 +59,7 @@ class BlockLegacyClientPluginTest extends TestCase { /** * @dataProvider oldDesktopClientProvider * @param string $userAgent - * @expectedException \OC\ServiceUnavailableException + * @expectedException \Sabre\DAV\Exception\Forbidden * @expectedExceptionMessage Unsupported client version. */ public function testBeforeHandlerException($userAgent) { From 21ad4400afdc6806268d1d8c07dd97dacbe99df6 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 20 Apr 2015 21:08:45 +0200 Subject: [PATCH 4/5] Reword configuration text --- config/config.sample.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/config/config.sample.php b/config/config.sample.php index a62438a20b..45aaf6a107 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -800,11 +800,14 @@ $CONFIG = array( 'cipher' => 'AES-256-CFB', /** - * The minimum ownCloud desktop client version that is required to sync with - * this instance. Defaults to the official supported releases by ownCloud. + * The minimum ownCloud desktop client version that will be allowed to sync with + * this server instance. All connections made from earlier clients will be denied + * by the server. Defaults to the minimum officially supported ownCloud version at + * the time of release of this server version. * - * When adjusting this please be aware of the fact that older versions may - * be buggy and for best user experience we recommend to not change this value. + * When changing this, note that older unsupported versions of the ownCloud desktop + * client may not function as expected, and could lead to permanent data loss for + * clients or other unexpected results. */ 'minimum.supported.desktop.version' => '1.7.0', From ab9ea97d3af62fd1c9c1f813b84e5c45a585a8d8 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Thu, 23 Apr 2015 16:33:51 +0200 Subject: [PATCH 5/5] Catch not existing User-Agent header In case of an not sent UA header consider the client as valid --- .../connector/sabre/blocklegacyclientplugin.php | 6 +++++- .../connector/sabre/BlockLegacyClientPluginTest.php | 13 ++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/lib/private/connector/sabre/blocklegacyclientplugin.php b/lib/private/connector/sabre/blocklegacyclientplugin.php index 7da6ff563d..9480cd1f06 100644 --- a/lib/private/connector/sabre/blocklegacyclientplugin.php +++ b/lib/private/connector/sabre/blocklegacyclientplugin.php @@ -46,7 +46,7 @@ class BlockLegacyClientPlugin extends ServerPlugin { } /** - * @param \Sabre\DAV\ $server + * @param \Sabre\DAV\Server $server * @return void */ public function initialize(\Sabre\DAV\Server $server) { @@ -62,6 +62,10 @@ class BlockLegacyClientPlugin extends ServerPlugin { */ public function beforeHandler(RequestInterface $request) { $userAgent = $request->getHeader('User-Agent'); + if($userAgent === null) { + return; + } + $minimumSupportedDesktopVersion = $this->config->getSystemValue('minimum.supported.desktop.version', '1.7.0'); // Match on the mirall version which is in scheme "Mozilla/5.0 (%1) mirall/%2" or diff --git a/tests/lib/connector/sabre/BlockLegacyClientPluginTest.php b/tests/lib/connector/sabre/BlockLegacyClientPluginTest.php index 2c7835dd73..05488d9716 100644 --- a/tests/lib/connector/sabre/BlockLegacyClientPluginTest.php +++ b/tests/lib/connector/sabre/BlockLegacyClientPluginTest.php @@ -97,7 +97,7 @@ class BlockLegacyClientPluginTest extends TestCase { * @dataProvider newAndAlternateDesktopClientProvider * @param string $userAgent */ - public function testBeforeHandlerSucess($userAgent) { + public function testBeforeHandlerSuccess($userAgent) { /** @var \Sabre\HTTP\RequestInterface $request */ $request = $this->getMock('\Sabre\HTTP\RequestInterface'); $request @@ -115,4 +115,15 @@ class BlockLegacyClientPluginTest extends TestCase { $this->blockLegacyClientVersionPlugin->beforeHandler($request); } + public function testBeforeHandlerNoUserAgent() { + /** @var \Sabre\HTTP\RequestInterface $request */ + $request = $this->getMock('\Sabre\HTTP\RequestInterface'); + $request + ->expects($this->once()) + ->method('getHeader') + ->with('User-Agent') + ->will($this->returnValue(null)); + $this->blockLegacyClientVersionPlugin->beforeHandler($request); + } + }