From 49524bc4baa41ea7b7464cfd5820c4526300a020 Mon Sep 17 00:00:00 2001 From: Niels Keurentjes Date: Thu, 10 Mar 2016 00:19:52 +0100 Subject: [PATCH] Centralize secure-http checking --- src/Composer/Config.php | 25 ++++++++++ src/Composer/Downloader/HgDownloader.php | 13 ++--- src/Composer/Repository/Vcs/HgDriver.php | 5 +- src/Composer/Util/Git.php | 5 +- src/Composer/Util/RemoteFilesystem.php | 17 ++----- src/Composer/Util/Svn.php | 5 +- tests/Composer/Test/ConfigTest.php | 61 ++++++++++++++++++++++++ 7 files changed, 99 insertions(+), 32 deletions(-) diff --git a/src/Composer/Config.php b/src/Composer/Config.php index e326cda2e..39c539b7e 100644 --- a/src/Composer/Config.php +++ b/src/Composer/Config.php @@ -13,6 +13,7 @@ namespace Composer; use Composer\Config\ConfigSourceInterface; +use Composer\Downloader\TransportException; /** * @author Jordi Boggiano @@ -395,4 +396,28 @@ class Config return false; } + + /** + * Validates that the passed URL is allowed to be used by current config, or throws an exception. + * + * @param string $url + */ + public function prohibitUrlByConfig($url) + { + if (!$this->get('secure-http')) { + return; + } + + // Parse the URL into its separate parts + $parsed = parse_url($url); + if (false === $parsed || !isset($parsed['scheme'])) { + // If the URL is malformed or does not contain a usable scheme it's not going to work anyway + return; + } + + // Throw exception on known insecure protocols + if (in_array($parsed['scheme'], array('http', 'git', 'ftp', 'svn'))) { + throw new TransportException("Your configuration does not allow connections to $url. See https://getcomposer.org/doc/06-config.md#secure-http for details."); + } + } } diff --git a/src/Composer/Downloader/HgDownloader.php b/src/Composer/Downloader/HgDownloader.php index 7732c1b62..6041d1dc6 100644 --- a/src/Composer/Downloader/HgDownloader.php +++ b/src/Composer/Downloader/HgDownloader.php @@ -25,7 +25,8 @@ class HgDownloader extends VcsDownloader */ public function doDownload(PackageInterface $package, $path, $url) { - $this->checkSecureHttp($url); + // Ensure we are allowed to use this URL by config + $this->config->prohibitUrlByConfig($url); $url = ProcessExecutor::escape($url); $ref = ProcessExecutor::escape($package->getSourceReference()); @@ -45,7 +46,8 @@ class HgDownloader extends VcsDownloader */ public function doUpdate(PackageInterface $initial, PackageInterface $target, $path, $url) { - $this->checkSecureHttp($url); + // Ensure we are allowed to use this URL by config + $this->config->prohibitUrlByConfig($url); $url = ProcessExecutor::escape($url); $ref = ProcessExecutor::escape($target->getSourceReference()); @@ -89,13 +91,6 @@ class HgDownloader extends VcsDownloader return $output; } - protected function checkSecureHttp($url) - { - if (preg_match('{^http:}i', $url) && $this->config->get('secure-http')) { - throw new TransportException("Your configuration does not allow connection to $url. See https://getcomposer.org/doc/06-config.md#secure-http for details."); - } - } - /** * {@inheritDoc} */ diff --git a/src/Composer/Repository/Vcs/HgDriver.php b/src/Composer/Repository/Vcs/HgDriver.php index c400342ab..caabc5f60 100644 --- a/src/Composer/Repository/Vcs/HgDriver.php +++ b/src/Composer/Repository/Vcs/HgDriver.php @@ -48,9 +48,8 @@ class HgDriver extends VcsDriver throw new \RuntimeException('Can not clone '.$this->url.' to access package information. The "'.$cacheDir.'" directory is not writable by the current user.'); } - if (preg_match('{^http:}i', $this->url) && $this->config->get('secure-http')) { - throw new TransportException("Your configuration does not allow connection to $url. See https://getcomposer.org/doc/06-config.md#secure-http for details."); - } + // Ensure we are allowed to use this URL by config + $this->config->prohibitUrlByConfig($this->url); // update the repo if it is a valid hg repository if (is_dir($this->repoDir) && 0 === $this->process->execute('hg summary', $output, $this->repoDir)) { diff --git a/src/Composer/Util/Git.php b/src/Composer/Util/Git.php index 13ee9aecf..57c6b4122 100644 --- a/src/Composer/Util/Git.php +++ b/src/Composer/Util/Git.php @@ -40,9 +40,8 @@ class Git public function runCommand($commandCallable, $url, $cwd, $initialClone = false) { - if (preg_match('{^(http|git):}i', $url) && $this->config->get('secure-http')) { - throw new TransportException("Your configuration does not allow connection to $url. See https://getcomposer.org/doc/06-config.md#secure-http for details."); - } + // Ensure we are allowed to use this URL by config + $this->config->prohibitUrlByConfig($url); if ($initialClone) { $origCwd = $cwd; diff --git a/src/Composer/Util/RemoteFilesystem.php b/src/Composer/Util/RemoteFilesystem.php index 16d97fcf2..a5e83b2c8 100644 --- a/src/Composer/Util/RemoteFilesystem.php +++ b/src/Composer/Util/RemoteFilesystem.php @@ -260,20 +260,9 @@ class RemoteFilesystem $this->io->writeError(" Downloading: Connecting...", false); } - // Check for secure HTTP - if ( - ($this->scheme === 'http' || substr($fileUrl, 0, 5) === 'http:') - && $this->config && $this->config->get('secure-http') - ) { - // Passthru unsecure Packagist calls to $hashed providers as file integrity is verified with sha256 - if (substr($fileUrl, 0, 23) !== 'http://packagist.org/p/' || (false === strpos($fileUrl, '$') && false === strpos($fileUrl, '%24'))) { - // other URLs must fail hard - throw new TransportException(sprintf( - 'Your configuration does not allow connection to %s://%s. See https://getcomposer.org/doc/06-config.md#secure-http for details.', - $this->scheme, - $originUrl - )); - } + // Check for secure HTTP, but allow insecure Packagist calls to $hashed providers as file integrity is verified with sha256 + if ((substr($fileUrl, 0, 23) !== 'http://packagist.org/p/' || (false === strpos($fileUrl, '$') && false === strpos($fileUrl, '%24'))) && $this->config) { + $this->config->prohibitUrlByConfig($fileUrl); } $errorMessage = ''; diff --git a/src/Composer/Util/Svn.php b/src/Composer/Util/Svn.php index b13cb3c11..100325f93 100644 --- a/src/Composer/Util/Svn.php +++ b/src/Composer/Util/Svn.php @@ -100,9 +100,8 @@ class Svn */ public function execute($command, $url, $cwd = null, $path = null, $verbose = false) { - if (preg_match('{^(http|svn):}i', $url) && $this->config->get('secure-http')) { - throw new TransportException("Your configuration does not allow connection to $url. See https://getcomposer.org/doc/06-config.md#secure-http for details."); - } + // Ensure we are allowed to use this URL by config + $this->config->prohibitUrlByConfig($url); $svnCommand = $this->getCommand($command, $url, $path); $output = null; diff --git a/tests/Composer/Test/ConfigTest.php b/tests/Composer/Test/ConfigTest.php index 07550c7a5..9fd693122 100644 --- a/tests/Composer/Test/ConfigTest.php +++ b/tests/Composer/Test/ConfigTest.php @@ -13,6 +13,7 @@ namespace Composer\Test; use Composer\Config; +use Composer\Downloader\TransportException; class ConfigTest extends \PHPUnit_Framework_TestCase { @@ -212,6 +213,66 @@ class ConfigTest extends \PHPUnit_Framework_TestCase $this->assertEquals(array('https', 'git'), $config->get('github-protocols')); } + /** + * @dataProvider allowedUrlProvider + * + * @param string $url + */ + public function testAllowedUrlsPass($url) + { + $config = new Config(false); + $config->prohibitUrlByConfig($url); + } + + /** + * @dataProvider prohibitedUrlProvider + * + * @param string $url + */ + public function testProhibitedUrlsThrowException($url) + { + $this->setExpectedException( + 'Composer\Downloader\TransportException', + 'Your configuration does not allow connections to ' . $url + ); + $config = new Config(false); + $config->prohibitUrlByConfig($url); + } + + /** + * @return array List of test URLs that should pass strict security + */ + public function allowedUrlProvider() + { + $urls = array( + 'https://packagist.org', + 'git@github.com:composer/composer.git', + 'hg://user:pass@my.satis/satis', + '\\myserver\myplace.git', + 'file://myserver.localhost/mygit.git', + 'file://example.org/mygit.git', + ); + return array_combine($urls, array_map(function($e) { return array($e); }, $urls)); + } + + /** + * @return array List of test URLs that should not pass strict security + */ + public function prohibitedUrlProvider() + { + $urls = array( + 'http://packagist.org', + 'http://10.1.0.1/satis', + 'http://127.0.0.1/satis', + 'svn://localhost/trunk', + 'svn://will.not.resolve/trunk', + 'svn://192.168.0.1/trunk', + 'svn://1.2.3.4/trunk', + 'git://5.6.7.8/git.git', + ); + return array_combine($urls, array_map(function($e) { return array($e); }, $urls)); + } + /** * @group TLS */