Skip to content

Commit

Permalink
Merge pull request #29444 from nextcloud/bugfix/noid/public-api-for-t…
Browse files Browse the repository at this point in the history
…rusted-domain-helpers

Add an OCP for trusted domain helper
  • Loading branch information
nickvergessen authored Oct 28, 2021
2 parents 9b7258f + c42f5bc commit 74cfe73
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 23 deletions.
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,7 @@
'OCP\\Security\\ICrypto' => $baseDir . '/lib/public/Security/ICrypto.php',
'OCP\\Security\\IHasher' => $baseDir . '/lib/public/Security/IHasher.php',
'OCP\\Security\\ISecureRandom' => $baseDir . '/lib/public/Security/ISecureRandom.php',
'OCP\\Security\\ITrustedDomainHelper' => $baseDir . '/lib/public/Security/ITrustedDomainHelper.php',
'OCP\\Security\\VerificationToken\\IVerificationToken' => $baseDir . '/lib/public/Security/VerificationToken/IVerificationToken.php',
'OCP\\Security\\VerificationToken\\InvalidTokenException' => $baseDir . '/lib/public/Security/VerificationToken/InvalidTokenException.php',
'OCP\\Session\\Exceptions\\SessionNotAvailableException' => $baseDir . '/lib/public/Session/Exceptions/SessionNotAvailableException.php',
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
'OCP\\Security\\ICrypto' => __DIR__ . '/../../..' . '/lib/public/Security/ICrypto.php',
'OCP\\Security\\IHasher' => __DIR__ . '/../../..' . '/lib/public/Security/IHasher.php',
'OCP\\Security\\ISecureRandom' => __DIR__ . '/../../..' . '/lib/public/Security/ISecureRandom.php',
'OCP\\Security\\ITrustedDomainHelper' => __DIR__ . '/../../..' . '/lib/public/Security/ITrustedDomainHelper.php',
'OCP\\Security\\VerificationToken\\IVerificationToken' => __DIR__ . '/../../..' . '/lib/public/Security/VerificationToken/IVerificationToken.php',
'OCP\\Security\\VerificationToken\\InvalidTokenException' => __DIR__ . '/../../..' . '/lib/public/Security/VerificationToken/InvalidTokenException.php',
'OCP\\Session\\Exceptions\\SessionNotAvailableException' => __DIR__ . '/../../..' . '/lib/public/Session/Exceptions/SessionNotAvailableException.php',
Expand Down
32 changes: 19 additions & 13 deletions lib/private/Security/TrustedDomainHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,9 @@

use OC\AppFramework\Http\Request;
use OCP\IConfig;
use OCP\Security\ITrustedDomainHelper;

/**
* Class TrustedDomain
*
* @package OC\Security
*/
class TrustedDomainHelper {
class TrustedDomainHelper implements ITrustedDomainHelper {
/** @var IConfig */
private $config;

Expand Down Expand Up @@ -65,13 +61,23 @@ private function getDomainWithoutPort(string $host): string {
}

/**
* Checks whether a domain is considered as trusted from the list
* of trusted domains. If no trusted domains have been configured, returns
* true.
* This is used to prevent Host Header Poisoning.
* @param string $domainWithPort
* @return bool true if the given domain is trusted or if no trusted domains
* have been configured
* {@inheritDoc}
*/
public function isTrustedUrl(string $url): bool {
$parsedUrl = parse_url($url);
if (empty($parsedUrl['host'])) {
return false;
}

if (isset($parsedUrl['port']) && $parsedUrl['port']) {
return $this->isTrustedDomain($parsedUrl['host'] . ':' . $parsedUrl['port']);
}

return $this->isTrustedDomain($parsedUrl['host']);
}

/**
* {@inheritDoc}
*/
public function isTrustedDomain(string $domainWithPort): bool {
// overwritehost is always trusted
Expand Down
56 changes: 56 additions & 0 deletions lib/public/Security/ITrustedDomainHelper.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<?php

declare(strict_types=1);
/**
* @copyright Copyright (c) 2021 Joas Schilling <coding@schilljs.com>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* 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
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

namespace OCP\Security;

/**
* Allows checking domains and full URLs against the list of trusted domains for
* this server in the config file.
*
* @package OCP\Security
* @since 23.0.0
*/
interface ITrustedDomainHelper {
/**
* Checks whether a given URL is considered as trusted from the list
* of trusted domains in the server's config file. If no trusted domains
* have been configured and the url is valid, returns true.
*
* @param string $url
* @return bool
* @since 23.0.0
*/
public function isTrustedUrl(string $url): bool;

/**
* Checks whether a given domain is considered as trusted from the list
* of trusted domains in the server's config file. If no trusted domains
* have been configured, returns true.
* This is used to prevent Host Header Poisoning.
*
* @param string $domainWithPort
* @return bool
* @since 23.0.0
*/
public function isTrustedDomain(string $domainWithPort): bool;
}
33 changes: 23 additions & 10 deletions tests/lib/Security/TrustedDomainHelperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,35 @@ protected function setUp(): void {
$this->config = $this->getMockBuilder(IConfig::class)->getMock();
}

/**
* @dataProvider trustedDomainDataProvider
* @param string $trustedDomains
* @param string $testDomain
* @param bool $result
*/
public function testIsTrustedUrl($trustedDomains, $testDomain, $result) {
$this->config->method('getSystemValue')
->willReturnMap([
['overwritehost', '', ''],
['trusted_domains', [], $trustedDomains],
]);

$trustedDomainHelper = new TrustedDomainHelper($this->config);
$this->assertEquals($result, $trustedDomainHelper->isTrustedUrl('https://' . $testDomain . '/index.php/something'));
}

/**
* @dataProvider trustedDomainDataProvider
* @param string $trustedDomains
* @param string $testDomain
* @param bool $result
*/
public function testIsTrustedDomain($trustedDomains, $testDomain, $result) {
$this->config->expects($this->at(0))
->method('getSystemValue')
->with('overwritehost')
->willReturn('');
$this->config->expects($this->at(1))
->method('getSystemValue')
->with('trusted_domains')
->willReturn($trustedDomains);
$this->config->method('getSystemValue')
->willReturnMap([
['overwritehost', '', ''],
['trusted_domains', [], $trustedDomains],
]);

$trustedDomainHelper = new TrustedDomainHelper($this->config);
$this->assertEquals($result, $trustedDomainHelper->isTrustedDomain($testDomain));
Expand Down Expand Up @@ -122,8 +136,7 @@ public function trustedDomainDataProvider() {
}

public function testIsTrustedDomainOverwriteHost() {
$this->config->expects($this->at(0))
->method('getSystemValue')
$this->config->method('getSystemValue')
->with('overwritehost')
->willReturn('myproxyhost');

Expand Down

0 comments on commit 74cfe73

Please sign in to comment.