Merge "shell: Optionally restrict commands' access with firejail"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Tue, 28 Nov 2017 00:14:32 +0000 (00:14 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Tue, 28 Nov 2017 00:14:32 +0000 (00:14 +0000)
autoload.php
includes/DefaultSettings.php
includes/GitInfo.php
includes/ServiceWiring.php
includes/shell/Command.php
includes/shell/CommandFactory.php
includes/shell/FirejailCommand.php [new file with mode: 0644]
includes/shell/Shell.php
includes/shell/firejail.profile [new file with mode: 0644]
tests/phpunit/includes/shell/CommandFactoryTest.php
tests/phpunit/includes/shell/FirejailCommandTest.php [new file with mode: 0644]

index 5a2156a..2231a3f 100644 (file)
@@ -939,6 +939,7 @@ $wgAutoloadLocalClasses = [
        'MediaWiki\\ShellDisabledError' => __DIR__ . '/includes/exception/ShellDisabledError.php',
        'MediaWiki\\Shell\\Command' => __DIR__ . '/includes/shell/Command.php',
        'MediaWiki\\Shell\\CommandFactory' => __DIR__ . '/includes/shell/CommandFactory.php',
+       'MediaWiki\\Shell\\FirejailCommand' => __DIR__ . '/includes/shell/FirejailCommand.php',
        'MediaWiki\\Shell\\Result' => __DIR__ . '/includes/shell/Result.php',
        'MediaWiki\\Shell\\Shell' => __DIR__ . '/includes/shell/Shell.php',
        'MediaWiki\\Site\\MediaWikiPageNameNormalizer' => __DIR__ . '/includes/site/MediaWikiPageNameNormalizer.php',
index 2d344fd..25be60c 100644 (file)
@@ -8271,6 +8271,22 @@ $wgPhpCli = '/usr/bin/php';
  */
 $wgShellLocale = 'C.UTF-8';
 
+/**
+ * Method to use to restrict shell commands
+ *
+ * Supported options:
+ * - 'autodetect': Autodetect if any restriction methods are available
+ * - 'firejail': Use firejail <https://firejail.wordpress.com/>
+ * - false: Don't use any restrictions
+ *
+ * @note If using firejail with MediaWiki running in a home directory different
+ *  from the webserver user, firejail 0.9.44+ is required.
+ *
+ * @since 1.31
+ * @var string|bool
+ */
+$wgShellRestrictionMethod = false;
+
 /** @} */ # End shell }
 
 /************************************************************************//**
index 8095fd7..f170a02 100644 (file)
@@ -232,6 +232,8 @@ class GitInfo {
                                ];
                                $result = Shell::command( $cmd )
                                        ->environment( [ 'GIT_DIR' => $this->basedir ] )
+                                       ->restrict( Shell::RESTRICT_DEFAULT | Shell::NO_NETWORK )
+                                       ->whitelistPaths( [ $this->basedir ] )
                                        ->execute();
 
                                if ( $result->getExitCode() === 0 ) {
index ae88d37..dad0630 100644 (file)
@@ -439,8 +439,9 @@ return [
                        'filesize' => $config->get( 'MaxShellFileSize' ),
                ];
                $cgroup = $config->get( 'ShellCgroup' );
+               $restrictionMethod = $config->get( 'ShellRestrictionMethod' );
 
-               $factory = new CommandFactory( $limits, $cgroup );
+               $factory = new CommandFactory( $limits, $cgroup, $restrictionMethod );
                $factory->setLogger( LoggerFactory::getInstance( 'exec' ) );
                $factory->logStderr();
 
index 9f080d5..264c3b4 100644 (file)
@@ -68,6 +68,13 @@ class Command {
        /** @var string|false */
        private $cgroup = false;
 
+       /**
+        * bitfield with restrictions
+        *
+        * @var int
+        */
+       protected $restrictions = 0;
+
        /**
         * Constructor. Don't call directly, instead use Shell::command()
         *
@@ -212,6 +219,45 @@ class Command {
                return $this;
        }
 
+       /**
+        * Set additional restrictions for this request
+        *
+        * @since 1.31
+        * @param int $restrictions
+        * @return $this
+        */
+       public function restrict( $restrictions ) {
+               $this->restrictions |= $restrictions;
+
+               return $this;
+       }
+
+       /**
+        * Bitfield helper on whether a specific restriction is enabled
+        *
+        * @param int $restriction
+        *
+        * @return bool
+        */
+       protected function hasRestriction( $restriction ) {
+               return ( $this->restrictions & $restriction ) === $restriction;
+       }
+
+       /**
+        * If called, only the files/directories that are
+        * whitelisted will be available to the shell command.
+        *
+        * limit.sh will always be whitelisted
+        *
+        * @param string[] $paths
+        *
+        * @return $this
+        */
+       public function whitelistPaths( array $paths ) {
+               // Default implementation is a no-op
+               return $this;
+       }
+
        /**
         * String together all the options and build the final command
         * to execute
index 84dd50f..78f1d80 100644 (file)
@@ -20,6 +20,7 @@
 
 namespace MediaWiki\Shell;
 
+use ExecutableFinder;
 use Psr\Log\LoggerAwareTrait;
 use Psr\Log\NullLogger;
 
@@ -40,18 +41,47 @@ class CommandFactory {
        /** @var bool */
        private $doLogStderr = false;
 
+       /**
+        * @var string|bool
+        */
+       private $restrictionMethod;
+
+       /**
+        * @var string|bool
+        */
+       private $firejail;
+
        /**
         * Constructor
         *
         * @param array $limits See {@see Command::limits()}
         * @param string|bool $cgroup See {@see Command::cgroup()}
+        * @param string|bool $restrictionMethod
         */
-       public function __construct( array $limits, $cgroup ) {
+       public function __construct( array $limits, $cgroup, $restrictionMethod ) {
                $this->limits = $limits;
                $this->cgroup = $cgroup;
+               if ( $restrictionMethod === 'autodetect' ) {
+                       // On Linux systems check for firejail
+                       if ( PHP_OS === 'Linux' && $this->findFirejail() !== false ) {
+                               $this->restrictionMethod = 'firejail';
+                       } else {
+                               $this->restrictionMethod = false;
+                       }
+               } else {
+                       $this->restrictionMethod = $restrictionMethod;
+               }
                $this->setLogger( new NullLogger() );
        }
 
+       private function findFirejail() {
+               if ( $this->firejail === null ) {
+                       $this->firejail = ExecutableFinder::findInDefaultPaths( 'firejail' );
+               }
+
+               return $this->firejail;
+       }
+
        /**
         * When enabled, text sent to stderr will be logged with a level of 'error'.
         *
@@ -68,7 +98,11 @@ class CommandFactory {
         * @return Command
         */
        public function create() {
-               $command = new Command();
+               if ( $this->restrictionMethod === 'firejail' ) {
+                       $command = new FirejailCommand( $this->findFirejail() );
+               } else {
+                       $command = new Command();
+               }
                $command->setLogger( $this->logger );
 
                return $command
diff --git a/includes/shell/FirejailCommand.php b/includes/shell/FirejailCommand.php
new file mode 100644 (file)
index 0000000..79f679d
--- /dev/null
@@ -0,0 +1,146 @@
+<?php
+/**
+ * Copyright (C) 2017 Kunal Mehta <legoktm@member.fsf.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ */
+
+namespace MediaWiki\Shell;
+
+use RuntimeException;
+
+/**
+ * Restricts execution of shell commands using firejail
+ *
+ * @see https://firejail.wordpress.com/
+ * @since 1.31
+ */
+class FirejailCommand extends Command {
+
+       /**
+        * @var string Path to firejail
+        */
+       private $firejail;
+
+       /**
+        * @var string[]
+        */
+       private $whitelistedPaths = [];
+
+       /**
+        * @param string $firejail Path to firejail
+        */
+       public function __construct( $firejail ) {
+               parent::__construct();
+               $this->firejail = $firejail;
+       }
+
+       /**
+        * @inheritDoc
+        */
+       public function whitelistPaths( array $paths ) {
+               $this->whitelistedPaths = array_merge( $this->whitelistedPaths, $paths );
+               return $this;
+       }
+
+       /**
+        * @inheritDoc
+        */
+       protected function buildFinalCommand() {
+               // If there are no restrictions, don't use firejail
+               if ( $this->restrictions === 0 ) {
+                       return parent::buildFinalCommand();
+               }
+
+               if ( $this->firejail === false ) {
+                       throw new RuntimeException( 'firejail is enabled, but cannot be found' );
+               }
+               // quiet has to come first to prevent firejail from adding
+               // any output.
+               $cmd = [ $this->firejail, '--quiet' ];
+               // Use a profile that allows people to add local overrides
+               // if their system is setup in an incompatible manner. Also it
+               // prevents any default profiles from running.
+               // FIXME: Doesn't actually override command-line switches?
+               $cmd[] = '--profile=' . __DIR__ . '/firejail.profile';
+
+               // By default firejail hides all other user directories, so if
+               // MediaWiki is inside a home directory (/home) but not the
+               // current user's home directory, pass --allusers to whitelist
+               // the home directories again.
+               static $useAllUsers = null;
+               if ( $useAllUsers === null ) {
+                       global $IP;
+                       // In case people are doing funny things with symlinks
+                       // or relative paths, resolve them all.
+                       $realIP = realpath( $IP );
+                       $currentUser = posix_getpwuid( posix_geteuid() );
+                       $useAllUsers = ( strpos( $realIP, '/home/' ) === 0 )
+                               && ( strpos( $realIP, $currentUser['dir'] ) !== 0 );
+                       if ( $useAllUsers ) {
+                               $this->logger->warning( 'firejail: MediaWiki is located ' .
+                                       'in a home directory that does not belong to the ' .
+                                       'current user, so allowing access to all home ' .
+                                       'directories (--allusers)' );
+                       }
+               }
+
+               if ( $useAllUsers ) {
+                       $cmd[] = '--allusers';
+               }
+
+               if ( $this->whitelistedPaths ) {
+                       // Always whitelist limit.sh
+                       $cmd[] = '--whitelist=' . __DIR__ . '/limit.sh';
+                       foreach ( $this->whitelistedPaths as $whitelistedPath ) {
+                               $cmd[] = "--whitelist={$whitelistedPath}";
+                       }
+               }
+
+               if ( $this->hasRestriction( Shell::NO_ROOT ) ) {
+                       $cmd[] = '--noroot';
+               }
+
+               $seccomp = [];
+
+               if ( $this->hasRestriction( Shell::SECCOMP ) ) {
+                       $seccomp[] = '@default';
+               }
+
+               if ( $this->hasRestriction( Shell::NO_EXECVE ) ) {
+                       $seccomp[] = 'execve';
+               }
+
+               if ( $seccomp ) {
+                       $cmd[] = '--seccomp=' . implode( ',', $seccomp );
+               }
+
+               if ( $this->hasRestriction( Shell::PRIVATE_DEV ) ) {
+                       $cmd[] = '--private-dev';
+               }
+
+               if ( $this->hasRestriction( Shell::NO_NETWORK ) ) {
+                       $cmd[] = '--net=none';
+               }
+
+               list( $fullCommand, $useLogPipe ) = parent::buildFinalCommand();
+
+               $builtCmd = implode( ' ', $cmd );
+
+               return [ "$builtCmd -- $fullCommand", $useLogPipe ];
+       }
+
+}
index 604c96a..6e4fd02 100644 (file)
@@ -41,6 +41,57 @@ use MediaWiki\MediaWikiServices;
  */
 class Shell {
 
+       /**
+        * Apply a default set of restrictions for improved
+        * security out of the box.
+        *
+        * Equal to NO_ROOT | SECCOMP | PRIVATE_DEV
+        *
+        * @note This value will change over time to provide increased security
+        *       by default, and is not guaranteed to be backwards-compatible.
+        * @since 1.31
+        */
+       const RESTRICT_DEFAULT = 7;
+
+       /**
+        * Disallow any root access. Any setuid binaries
+        * will be run without elevated access.
+        *
+        * @since 1.31
+        */
+       const NO_ROOT = 1;
+
+       /**
+        * Use seccomp to block dangerous syscalls
+        * @see <https://en.wikipedia.org/wiki/seccomp>
+        *
+        * @since 1.31
+        */
+       const SECCOMP = 2;
+
+       /**
+        * Create a private /dev
+        *
+        * @since 1.31
+        */
+       const PRIVATE_DEV = 4;
+
+       /**
+        * Restrict the request to have no
+        * network access
+        *
+        * @since 1.31
+        */
+       const NO_NETWORK = 8;
+
+       /**
+        * Deny execve syscall with seccomp
+        * @see <https://en.wikipedia.org/wiki/exec_(system_call)>
+        *
+        * @since 1.31
+        */
+       const NO_EXECVE = 16;
+
        /**
         * Returns a new instance of Command class
         *
diff --git a/includes/shell/firejail.profile b/includes/shell/firejail.profile
new file mode 100644 (file)
index 0000000..07f059b
--- /dev/null
@@ -0,0 +1,7 @@
+# Firejail profile used by MediaWiki when shelling out
+# See <https://firejail.wordpress.com/features-3/man-firejail-profile/> for
+# syntax documentation
+# Persistent local customizations
+include /etc/firejail/mediawiki.local
+# Persistent global definitions
+include /etc/firejail/globals.local
index f90e837..bf0f65b 100644 (file)
@@ -1,6 +1,8 @@
 <?php
 
+use MediaWiki\Shell\Command;
 use MediaWiki\Shell\CommandFactory;
+use MediaWiki\Shell\FirejailCommand;
 use Psr\Log\NullLogger;
 use Wikimedia\TestingAccessWrapper;
 
@@ -21,10 +23,11 @@ class CommandFactoryTest extends PHPUnit_Framework_TestCase {
                        'walltime' => 40,
                ];
 
-               $factory = new CommandFactory( $limits, $cgroup );
+               $factory = new CommandFactory( $limits, $cgroup, false );
                $factory->setLogger( $logger );
                $factory->logStderr();
                $command = $factory->create();
+               $this->assertInstanceOf( Command::class, $command );
 
                $wrapper = TestingAccessWrapper::newFromObject( $command );
                $this->assertSame( $logger, $wrapper->logger );
@@ -32,4 +35,13 @@ class CommandFactoryTest extends PHPUnit_Framework_TestCase {
                $this->assertSame( $limits, $wrapper->limits );
                $this->assertTrue( $wrapper->doLogStderr );
        }
+
+       /**
+        * @covers MediaWiki\Shell\CommandFactory::create
+        */
+       public function testFirejailCreate() {
+               $factory = new CommandFactory( [], false, 'firejail' );
+               $factory->setLogger( new NullLogger() );
+               $this->assertInstanceOf( FirejailCommand::class, $factory->create() );
+       }
 }
diff --git a/tests/phpunit/includes/shell/FirejailCommandTest.php b/tests/phpunit/includes/shell/FirejailCommandTest.php
new file mode 100644 (file)
index 0000000..c9db74f
--- /dev/null
@@ -0,0 +1,82 @@
+<?php
+
+/**
+ * Copyright (C) 2017 Kunal Mehta <legoktm@member.fsf.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ */
+
+use MediaWiki\Shell\FirejailCommand;
+use MediaWiki\Shell\Shell;
+use Wikimedia\TestingAccessWrapper;
+
+class FirejailCommandTest extends PHPUnit_Framework_TestCase {
+       public function provideBuildFinalCommand() {
+               global $IP;
+               // @codingStandardsIgnoreStart
+               $env = "'MW_INCLUDE_STDERR=;MW_CPU_LIMIT=180; MW_CGROUP='\'''\''; MW_MEM_LIMIT=307200; MW_FILE_SIZE_LIMIT=102400; MW_WALL_CLOCK_LIMIT=180; MW_USE_LOG_PIPE=yes'";
+               // @codingStandardsIgnoreEnd
+               $limit = "$IP/includes/shell/limit.sh";
+               $profile = "--profile=$IP/includes/shell/firejail.profile";
+               $default = '--noroot --seccomp=@default --private-dev';
+               return [
+                       [
+                               'No restrictions',
+                               'ls', 0, "/bin/bash '$limit' ''\''ls'\''' $env"
+                       ],
+                       [
+                               'default restriction',
+                               'ls', Shell::RESTRICT_DEFAULT,
+                               "firejail --quiet $profile $default -- /bin/bash '$limit' ''\''ls'\''' $env"
+                       ],
+                       [
+                               'no network',
+                               'ls', Shell::NO_NETWORK,
+                               "firejail --quiet $profile --net=none -- /bin/bash '$limit' ''\''ls'\''' $env"
+                       ],
+                       [
+                               'default restriction & no network',
+                               'ls', Shell::RESTRICT_DEFAULT | Shell::NO_NETWORK,
+                               "firejail --quiet $profile $default --net=none -- /bin/bash '$limit' ''\''ls'\''' $env"
+                       ],
+                       [
+                               'seccomp',
+                               'ls', Shell::SECCOMP,
+                               "firejail --quiet $profile --seccomp=@default -- /bin/bash '$limit' ''\''ls'\''' $env"
+                       ],
+                       [
+                               'seccomp & no execve',
+                               'ls', Shell::SECCOMP | Shell::NO_EXECVE,
+                               "firejail --quiet $profile --seccomp=@default,execve -- /bin/bash '$limit' ''\''ls'\''' $env"
+                       ],
+               ];
+       }
+
+       /**
+        * @covers \MediaWiki\Shell\FirejailCommand::buildFinalCommand()
+        * @dataProvider provideBuildFinalCommand
+        */
+       public function testBuildFinalCommand( $desc, $params, $flags, $expected ) {
+               $command = new FirejailCommand( 'firejail' );
+               $command
+                       ->params( $params )
+                       ->restrict( $flags );
+               $wrapper = TestingAccessWrapper::newFromObject( $command );
+               $output = $wrapper->buildFinalCommand();
+               $this->assertEquals( $expected, $output[0], $desc );
+       }
+
+}