Disable git, svn, http protocols for VCS downloaders, fixes #4968

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

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];

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}
      */

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)) {

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)) {

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);

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;

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
      */

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())

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);