From 6f42b9c8651a591663f29a743acc67a8d298ca9b Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Tue, 1 Mar 2016 13:19:44 +0000 Subject: [PATCH] Disable git, svn, http protocols for VCS downloaders, fixes #4968 --- CHANGELOG.md | 1 + src/Composer/Config.php | 10 +++++--- src/Composer/Downloader/HgDownloader.php | 11 ++++++++ src/Composer/Downloader/VcsDownloader.php | 4 +++ src/Composer/Repository/Vcs/HgDriver.php | 4 +++ src/Composer/Util/Git.php | 21 +++++++++------- src/Composer/Util/Svn.php | 4 +++ tests/Composer/Test/ConfigTest.php | 12 ++++++++- .../Test/Downloader/GitDownloaderTest.php | 25 +++++++++++-------- .../Test/Repository/Vcs/SvnDriverTest.php | 6 ++--- 10 files changed, 72 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c4728e190..69b870216 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ### [1.0.0-beta1] - 2016-XX-XX + * Break: By default we now disable any non-secure protocols (http, git, svn). This may lead to issues if you rely on those. See `secure-http` config option. * Added VCS repo support for the GitLab API, see also `gitlab-oauth` and `gitlab-domains` config options * Added `prohibits` / `why-not` command to show what blocks an upgrade to a given package:version pair * Added --tree / -t to the `show` command to see all your installed packages in a tree view diff --git a/src/Composer/Config.php b/src/Composer/Config.php index bbabac95a..e326cda2e 100644 --- a/src/Composer/Config.php +++ b/src/Composer/Config.php @@ -26,7 +26,7 @@ class Config 'use-include-path' => false, 'preferred-install' => 'auto', 'notify-on-install' => true, - 'github-protocols' => array('git', 'https', 'ssh'), + 'github-protocols' => array('https', 'ssh', 'git'), 'vendor-dir' => 'vendor', 'bin-dir' => '{$vendor-dir}/bin', 'cache-dir' => '{$home}/cache', @@ -285,11 +285,15 @@ class Config return $this->config[$key]; case 'github-protocols': - if (reset($this->config['github-protocols']) === 'http') { + $protos = $this->config['github-protocols']; + if ($this->config['secure-http'] && false !== ($index = array_search('git', $protos))) { + unset($protos[$index]); + } + if (reset($protos) === 'http') { throw new \RuntimeException('The http protocol for github is not available anymore, update your config\'s github-protocols to use "https", "git" or "ssh"'); } - return $this->config[$key]; + return $protos; case 'disable-tls': return $this->config[$key] !== 'false' && (bool) $this->config[$key]; diff --git a/src/Composer/Downloader/HgDownloader.php b/src/Composer/Downloader/HgDownloader.php index 419ce92cb..7732c1b62 100644 --- a/src/Composer/Downloader/HgDownloader.php +++ b/src/Composer/Downloader/HgDownloader.php @@ -25,6 +25,8 @@ class HgDownloader extends VcsDownloader */ public function doDownload(PackageInterface $package, $path, $url) { + $this->checkSecureHttp($url); + $url = ProcessExecutor::escape($url); $ref = ProcessExecutor::escape($package->getSourceReference()); $this->io->writeError(" Cloning ".$package->getSourceReference()); @@ -43,6 +45,8 @@ class HgDownloader extends VcsDownloader */ public function doUpdate(PackageInterface $initial, PackageInterface $target, $path, $url) { + $this->checkSecureHttp($url); + $url = ProcessExecutor::escape($url); $ref = ProcessExecutor::escape($target->getSourceReference()); $this->io->writeError(" Updating to ".$target->getSourceReference()); @@ -85,6 +89,13 @@ 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/Downloader/VcsDownloader.php b/src/Composer/Downloader/VcsDownloader.php index 377286726..242abb938 100644 --- a/src/Composer/Downloader/VcsDownloader.php +++ b/src/Composer/Downloader/VcsDownloader.php @@ -69,6 +69,10 @@ abstract class VcsDownloader implements DownloaderInterface, ChangeReportInterfa $this->doDownload($package, $path, $url); break; } catch (\Exception $e) { + // rethrow phpunit exceptions to avoid hard to debug bug failures + if ($e instanceof \PHPUnit_Framework_Exception) { + throw $e; + } if ($this->io->isDebug()) { $this->io->writeError('Failed: ['.get_class($e).'] '.$e->getMessage()); } elseif (count($urls)) { diff --git a/src/Composer/Repository/Vcs/HgDriver.php b/src/Composer/Repository/Vcs/HgDriver.php index 06382f039..c14e07d98 100644 --- a/src/Composer/Repository/Vcs/HgDriver.php +++ b/src/Composer/Repository/Vcs/HgDriver.php @@ -47,6 +47,10 @@ 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."); + } + // update the repo if it is a valid hg repository if (is_dir($this->repoDir) && 0 === $this->process->execute('hg summary', $output, $this->repoDir)) { if (0 !== $this->process->execute('hg pull', $output, $this->repoDir)) { diff --git a/src/Composer/Util/Git.php b/src/Composer/Util/Git.php index a6d87e1bb..d78d3be06 100644 --- a/src/Composer/Util/Git.php +++ b/src/Composer/Util/Git.php @@ -39,6 +39,10 @@ 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."); + } + if ($initialClone) { $origCwd = $cwd; $cwd = null; @@ -60,21 +64,20 @@ class Git if (!is_array($protocols)) { throw new \RuntimeException('Config value "github-protocols" must be an array, got '.gettype($protocols)); } - // public github, autoswitch protocols if (preg_match('{^(?:https?|git)://'.self::getGitHubDomainsRegex($this->config).'/(.*)}', $url, $match)) { $messages = array(); foreach ($protocols as $protocol) { if ('ssh' === $protocol) { - $url = "git@" . $match[1] . ":" . $match[2]; + $protoUrl = "git@" . $match[1] . ":" . $match[2]; } else { - $url = $protocol ."://" . $match[1] . "/" . $match[2]; + $protoUrl = $protocol ."://" . $match[1] . "/" . $match[2]; } - if (0 === $this->process->execute(call_user_func($commandCallable, $url), $ignoredOutput, $cwd)) { + if (0 === $this->process->execute(call_user_func($commandCallable, $protoUrl), $ignoredOutput, $cwd)) { return; } - $messages[] = '- ' . $url . "\n" . preg_replace('#^#m', ' ', $this->process->getErrorOutput()); + $messages[] = '- ' . $protoUrl . "\n" . preg_replace('#^#m', ' ', $this->process->getErrorOutput()); if ($initialClone) { $this->filesystem->removeDirectory($origCwd); } @@ -104,8 +107,8 @@ class Git if ($this->io->hasAuthentication($match[1])) { $auth = $this->io->getAuthentication($match[1]); - $url = 'https://'.rawurlencode($auth['username']) . ':' . rawurlencode($auth['password']) . '@'.$match[1].'/'.$match[2].'.git'; - $command = call_user_func($commandCallable, $url); + $authUrl = 'https://'.rawurlencode($auth['username']) . ':' . rawurlencode($auth['password']) . '@'.$match[1].'/'.$match[2].'.git'; + $command = call_user_func($commandCallable, $authUrl); if (0 === $this->process->execute($command, $ignoredOutput, $cwd)) { return; } @@ -137,9 +140,9 @@ class Git } if ($auth) { - $url = $match[1].rawurlencode($auth['username']).':'.rawurlencode($auth['password']).'@'.$match[2].$match[3]; + $authUrl = $match[1].rawurlencode($auth['username']).':'.rawurlencode($auth['password']).'@'.$match[2].$match[3]; - $command = call_user_func($commandCallable, $url); + $command = call_user_func($commandCallable, $authUrl); if (0 === $this->process->execute($command, $ignoredOutput, $cwd)) { $this->io->setAuthentication($match[2], $auth['username'], $auth['password']); $authHelper = new AuthHelper($this->io, $this->config); diff --git a/src/Composer/Util/Svn.php b/src/Composer/Util/Svn.php index 4117499c8..9a3c65fd5 100644 --- a/src/Composer/Util/Svn.php +++ b/src/Composer/Util/Svn.php @@ -99,6 +99,10 @@ 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."); + } + $svnCommand = $this->getCommand($command, $url, $path); $output = null; $io = $this->io; diff --git a/tests/Composer/Test/ConfigTest.php b/tests/Composer/Test/ConfigTest.php index abce384c2..07550c7a5 100644 --- a/tests/Composer/Test/ConfigTest.php +++ b/tests/Composer/Test/ConfigTest.php @@ -196,12 +196,22 @@ class ConfigTest extends \PHPUnit_Framework_TestCase public function testOverrideGithubProtocols() { $config = new Config(false); - $config->merge(array('config' => array('github-protocols' => array('https', 'git')))); + $config->merge(array('config' => array('github-protocols' => array('https', 'ssh')))); $config->merge(array('config' => array('github-protocols' => array('https')))); $this->assertEquals(array('https'), $config->get('github-protocols')); } + public function testGitDisabledByDefaultInGithubProtocols() + { + $config = new Config(false); + $config->merge(array('config' => array('github-protocols' => array('https', 'git')))); + $this->assertEquals(array('https'), $config->get('github-protocols')); + + $config->merge(array('config' => array('secure-http' => false))); + $this->assertEquals(array('https', 'git'), $config->get('github-protocols')); + } + /** * @group TLS */ diff --git a/tests/Composer/Test/Downloader/GitDownloaderTest.php b/tests/Composer/Test/Downloader/GitDownloaderTest.php index 1cd7b076f..90ddeda32 100644 --- a/tests/Composer/Test/Downloader/GitDownloaderTest.php +++ b/tests/Composer/Test/Downloader/GitDownloaderTest.php @@ -123,13 +123,18 @@ class GitDownloaderTest extends TestCase ->will($this->returnValue('1.0.0')); $processExecutor = $this->getMock('Composer\Util\ProcessExecutor'); - $expectedGitCommand = $this->winCompat("git clone --no-checkout 'git://github.com/mirrors/composer' 'composerPath' && cd 'composerPath' && git remote add composer 'git://github.com/mirrors/composer' && git fetch composer"); + $expectedGitCommand = $this->winCompat("git clone --no-checkout 'https://github.com/mirrors/composer' 'composerPath' && cd 'composerPath' && git remote add composer 'https://github.com/mirrors/composer' && git fetch composer"); $processExecutor->expects($this->at(0)) ->method('execute') ->with($this->equalTo($expectedGitCommand)) ->will($this->returnValue(1)); - $expectedGitCommand = $this->winCompat("git clone --no-checkout 'https://github.com/mirrors/composer' 'composerPath' && cd 'composerPath' && git remote add composer 'https://github.com/mirrors/composer' && git fetch composer"); + $processExecutor->expects($this->at(1)) + ->method('getErrorOutput') + ->with() + ->will($this->returnValue('Error1')); + + $expectedGitCommand = $this->winCompat("git clone --no-checkout 'git@github.com:mirrors/composer' 'composerPath' && cd 'composerPath' && git remote add composer 'git@github.com:mirrors/composer' && git fetch composer"); $processExecutor->expects($this->at(2)) ->method('execute') ->with($this->equalTo($expectedGitCommand)) @@ -141,7 +146,7 @@ class GitDownloaderTest extends TestCase ->with($this->equalTo($expectedGitCommand), $this->equalTo(null), $this->equalTo($this->winCompat('composerPath'))) ->will($this->returnValue(0)); - $expectedGitCommand = $this->winCompat("git remote set-url --push origin 'git@github.com:composer/composer.git'"); + $expectedGitCommand = $this->winCompat("git remote set-url --push origin 'https://github.com/composer/composer.git'"); $processExecutor->expects($this->at(4)) ->method('execute') ->with($this->equalTo($expectedGitCommand), $this->equalTo(null), $this->equalTo($this->winCompat('composerPath'))) @@ -164,15 +169,15 @@ class GitDownloaderTest extends TestCase public function pushUrlProvider() { return array( - array('git', 'git@github.com:composer/composer.git'), - array('https', 'https://github.com/composer/composer.git'), + array('ssh', 'git@github.com:composer/composer', 'https://github.com/composer/composer.git'), + array('https', 'https://github.com/composer/composer', 'https://github.com/composer/composer.git'), ); } /** * @dataProvider pushUrlProvider */ - public function testDownloadAndSetPushUrlUseCustomVariousProtocolsForGithub($protocol, $pushUrl) + public function testDownloadAndSetPushUrlUseCustomVariousProtocolsForGithub($protocol, $url, $pushUrl) { $packageMock = $this->getMock('Composer\Package\PackageInterface'); $packageMock->expects($this->any()) @@ -189,7 +194,7 @@ class GitDownloaderTest extends TestCase ->will($this->returnValue('1.0.0')); $processExecutor = $this->getMock('Composer\Util\ProcessExecutor'); - $expectedGitCommand = $this->winCompat("git clone --no-checkout '{$protocol}://github.com/composer/composer' 'composerPath' && cd 'composerPath' && git remote add composer '{$protocol}://github.com/composer/composer' && git fetch composer"); + $expectedGitCommand = $this->winCompat("git clone --no-checkout '{$url}' 'composerPath' && cd 'composerPath' && git remote add composer '{$url}' && git fetch composer"); $processExecutor->expects($this->at(0)) ->method('execute') ->with($this->equalTo($expectedGitCommand)) @@ -252,7 +257,7 @@ class GitDownloaderTest extends TestCase public function testUpdate() { - $expectedGitUpdateCommand = $this->winCompat("git remote set-url composer 'git://github.com/composer/composer' && git fetch composer && git fetch --tags composer"); + $expectedGitUpdateCommand = $this->winCompat("git remote set-url composer 'https://github.com/composer/composer' && git fetch composer && git fetch --tags composer"); $packageMock = $this->getMock('Composer\Package\PackageInterface'); $packageMock->expects($this->any()) @@ -309,7 +314,7 @@ class GitDownloaderTest extends TestCase */ public function testUpdateThrowsRuntimeExceptionIfGitCommandFails() { - $expectedGitUpdateCommand = $this->winCompat("git remote set-url composer 'git://github.com/composer/composer' && git fetch composer && git fetch --tags composer"); + $expectedGitUpdateCommand = $this->winCompat("git remote set-url composer 'https://github.com/composer/composer' && git fetch composer && git fetch --tags composer"); $packageMock = $this->getMock('Composer\Package\PackageInterface'); $packageMock->expects($this->any()) @@ -354,7 +359,7 @@ class GitDownloaderTest extends TestCase $this->markTestIncomplete('This test is disabled until https://github.com/composer/composer/issues/4973 is resolved'); $expectedFirstGitUpdateCommand = $this->winCompat("git remote set-url composer '' && git fetch composer && git fetch --tags composer"); - $expectedSecondGitUpdateCommand = $this->winCompat("git remote set-url composer 'git://github.com/composer/composer' && git fetch composer && git fetch --tags composer"); + $expectedSecondGitUpdateCommand = $this->winCompat("git remote set-url composer 'https://github.com/composer/composer' && git fetch composer && git fetch --tags composer"); $packageMock = $this->getMock('Composer\Package\PackageInterface'); $packageMock->expects($this->any()) diff --git a/tests/Composer/Test/Repository/Vcs/SvnDriverTest.php b/tests/Composer/Test/Repository/Vcs/SvnDriverTest.php index 881b86ea2..7cd90b6aa 100644 --- a/tests/Composer/Test/Repository/Vcs/SvnDriverTest.php +++ b/tests/Composer/Test/Repository/Vcs/SvnDriverTest.php @@ -47,9 +47,9 @@ class SvnDriverTest extends TestCase { $console = $this->getMock('Composer\IO\IOInterface'); - $output = "svn: OPTIONS of 'http://corp.svn.local/repo':"; + $output = "svn: OPTIONS of 'https://corp.svn.local/repo':"; $output .= " authorization failed: Could not authenticate to server:"; - $output .= " rejected Basic challenge (http://corp.svn.local/)"; + $output .= " rejected Basic challenge (https://corp.svn.local/)"; $process = $this->getMock('Composer\Util\ProcessExecutor'); $process->expects($this->at(1)) @@ -63,7 +63,7 @@ class SvnDriverTest extends TestCase ->will($this->returnValue(0)); $repoConfig = array( - 'url' => 'http://till:secret@corp.svn.local/repo', + 'url' => 'https://till:secret@corp.svn.local/repo', ); $svn = new SvnDriver($repoConfig, $console, $this->config, $process);