From bdb5b592f444f4670070808048afcb0bb1adcc5a Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Tue, 17 Oct 2017 23:54:19 -0700 Subject: [PATCH] shell: Optionally restrict commands' access with firejail 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 | 1 + includes/DefaultSettings.php | 16 ++ includes/GitInfo.php | 2 + includes/ServiceWiring.php | 3 +- includes/shell/Command.php | 46 ++++++ includes/shell/CommandFactory.php | 38 ++++- includes/shell/FirejailCommand.php | 146 ++++++++++++++++++ includes/shell/Shell.php | 51 ++++++ includes/shell/firejail.profile | 7 + .../includes/shell/CommandFactoryTest.php | 14 +- .../includes/shell/FirejailCommandTest.php | 82 ++++++++++ 11 files changed, 402 insertions(+), 4 deletions(-) create mode 100644 includes/shell/FirejailCommand.php create mode 100644 includes/shell/firejail.profile create mode 100644 tests/phpunit/includes/shell/FirejailCommandTest.php diff --git a/autoload.php b/autoload.php index 5a2156ac92..2231a3feab 100644 --- a/autoload.php +++ b/autoload.php @@ -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', diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 2d344fd8d9..25be60c975 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -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 + * - 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 } /************************************************************************//** diff --git a/includes/GitInfo.php b/includes/GitInfo.php index 8095fd7308..f170a025f7 100644 --- a/includes/GitInfo.php +++ b/includes/GitInfo.php @@ -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 ) { diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index ae88d375cc..dad0630edf 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -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(); diff --git a/includes/shell/Command.php b/includes/shell/Command.php index 9f080d5141..264c3b48cc 100644 --- a/includes/shell/Command.php +++ b/includes/shell/Command.php @@ -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 diff --git a/includes/shell/CommandFactory.php b/includes/shell/CommandFactory.php index 84dd50f71f..78f1d8008f 100644 --- a/includes/shell/CommandFactory.php +++ b/includes/shell/CommandFactory.php @@ -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 index 0000000000..79f679d87b --- /dev/null +++ b/includes/shell/FirejailCommand.php @@ -0,0 +1,146 @@ + + * + * 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 ]; + } + +} diff --git a/includes/shell/Shell.php b/includes/shell/Shell.php index 604c96a6ad..6e4fd02a13 100644 --- a/includes/shell/Shell.php +++ b/includes/shell/Shell.php @@ -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 + * + * @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 + * + * @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 index 0000000000..07f059bad0 --- /dev/null +++ b/includes/shell/firejail.profile @@ -0,0 +1,7 @@ +# Firejail profile used by MediaWiki when shelling out +# See for +# syntax documentation +# Persistent local customizations +include /etc/firejail/mediawiki.local +# Persistent global definitions +include /etc/firejail/globals.local diff --git a/tests/phpunit/includes/shell/CommandFactoryTest.php b/tests/phpunit/includes/shell/CommandFactoryTest.php index f90e8377ca..bf0f65b5e9 100644 --- a/tests/phpunit/includes/shell/CommandFactoryTest.php +++ b/tests/phpunit/includes/shell/CommandFactoryTest.php @@ -1,6 +1,8 @@ 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 index 0000000000..c9db74f5f9 --- /dev/null +++ b/tests/phpunit/includes/shell/FirejailCommandTest.php @@ -0,0 +1,82 @@ + + * + * 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 ); + } + +} -- 2.20.1