shell: Optionally restrict commands' access with firejail
authorKunal Mehta <legoktm@member.fsf.org>
Wed, 18 Oct 2017 06:54:19 +0000 (23:54 -0700)
committerTim Starling <tstarling@wikimedia.org>
Tue, 28 Nov 2017 00:06:40 +0000 (00:06 +0000)
Introduces a FirejailCommand class, which can be used to add additional
restrictions to a command, for increased security. For now, firejail
containment needs to be enabled on a per-command basis.

The following restrictions are implemented:
* NO_ROOT - disallows any root access, including via setuid binaries
* SECCOMP - block dangerous syscalls with seccomp
* PRIVATE_DEV - create a private /dev
* NO_NETWORK - deny all network access
* NO_EXECVE - block the execve syscall

A convenient Shell::RESTRICT_DEFAULT is equivalent to NO_ROOT | SECCOMP
| PRIVATE_DEV, with the expectation that more restrictions may be added
to it in the future.

In addition, specific paths can be whitelisted with
Command::whitelistPaths(). Any file/directory that isn't whitelisted in
that top level directory (e.g. /srv) won't exist inside the firejail.

$wgShellRestrictionMethod can be set to false for no restriction system,
'firejail' to explicitly use it, or 'autodetect' to autodetect whatever
system is available. In the future the default should be changed to
autodetection once firejail is tested more.

Bug: T173370
Change-Id: Id74df0dbba40e1e7c07c4368aacffb6eb06a17c5

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\\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',
        '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';
 
  */
 $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 }
 
 /************************************************************************//**
 /** @} */ # End shell }
 
 /************************************************************************//**
index 8095fd7..f170a02 100644 (file)
@@ -232,6 +232,8 @@ class GitInfo {
                                ];
                                $result = Shell::command( $cmd )
                                        ->environment( [ 'GIT_DIR' => $this->basedir ] )
                                ];
                                $result = Shell::command( $cmd )
                                        ->environment( [ 'GIT_DIR' => $this->basedir ] )
+                                       ->restrict( Shell::RESTRICT_DEFAULT | Shell::NO_NETWORK )
+                                       ->whitelistPaths( [ $this->basedir ] )
                                        ->execute();
 
                                if ( $result->getExitCode() === 0 ) {
                                        ->execute();
 
                                if ( $result->getExitCode() === 0 ) {
index ae88d37..dad0630 100644 (file)
@@ -439,8 +439,9 @@ return [
                        'filesize' => $config->get( 'MaxShellFileSize' ),
                ];
                $cgroup = $config->get( 'ShellCgroup' );
                        '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();
 
                $factory->setLogger( LoggerFactory::getInstance( 'exec' ) );
                $factory->logStderr();
 
index 9f080d5..264c3b4 100644 (file)
@@ -68,6 +68,13 @@ class Command {
        /** @var string|false */
        private $cgroup = false;
 
        /** @var string|false */
        private $cgroup = false;
 
+       /**
+        * bitfield with restrictions
+        *
+        * @var int
+        */
+       protected $restrictions = 0;
+
        /**
         * Constructor. Don't call directly, instead use Shell::command()
         *
        /**
         * Constructor. Don't call directly, instead use Shell::command()
         *
@@ -212,6 +219,45 @@ class Command {
                return $this;
        }
 
                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
        /**
         * String together all the options and build the final command
         * to execute
index 84dd50f..78f1d80 100644 (file)
@@ -20,6 +20,7 @@
 
 namespace MediaWiki\Shell;
 
 
 namespace MediaWiki\Shell;
 
+use ExecutableFinder;
 use Psr\Log\LoggerAwareTrait;
 use Psr\Log\NullLogger;
 
 use Psr\Log\LoggerAwareTrait;
 use Psr\Log\NullLogger;
 
@@ -40,18 +41,47 @@ class CommandFactory {
        /** @var bool */
        private $doLogStderr = false;
 
        /** @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()}
        /**
         * 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;
                $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() );
        }
 
                $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'.
         *
        /**
         * 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() {
         * @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
                $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 {
 
  */
 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
         *
        /**
         * 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
 
 <?php
 
+use MediaWiki\Shell\Command;
 use MediaWiki\Shell\CommandFactory;
 use MediaWiki\Shell\CommandFactory;
+use MediaWiki\Shell\FirejailCommand;
 use Psr\Log\NullLogger;
 use Wikimedia\TestingAccessWrapper;
 
 use Psr\Log\NullLogger;
 use Wikimedia\TestingAccessWrapper;
 
@@ -21,10 +23,11 @@ class CommandFactoryTest extends PHPUnit_Framework_TestCase {
                        'walltime' => 40,
                ];
 
                        'walltime' => 40,
                ];
 
-               $factory = new CommandFactory( $limits, $cgroup );
+               $factory = new CommandFactory( $limits, $cgroup, false );
                $factory->setLogger( $logger );
                $factory->logStderr();
                $command = $factory->create();
                $factory->setLogger( $logger );
                $factory->logStderr();
                $command = $factory->create();
+               $this->assertInstanceOf( Command::class, $command );
 
                $wrapper = TestingAccessWrapper::newFromObject( $command );
                $this->assertSame( $logger, $wrapper->logger );
 
                $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 );
        }
                $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 );
+       }
+
+}