Merge "shell: Run firejail inside limit.sh, make NO_EXECVE work"

diff --git a/includes/shell/Command.php b/includes/shell/Command.php
index 998b3ed..2f0ea42 100644 (file)
--- a/includes/shell/Command.php
+++ b/includes/shell/Command.php
@@ -36,7 +36,7 @@ class Command {
        use LoggerAwareTrait;
 
        /** @var string */
-       private $command = '';
+       protected $command = '';
 
        /** @var array */
        private $limits = [
@@ -269,9 +269,10 @@ class Command {
         * String together all the options and build the final command
         * to execute
         *
+        * @param string $command Already-escaped command to run
         * @return array [ command, whether to use log pipe ]
         */
-       protected function buildFinalCommand() {
+       protected function buildFinalCommand( $command ) {
                $envcmd = '';
                foreach ( $this->env as $k => $v ) {
                        if ( wfIsWindows() ) {
@@ -291,7 +292,7 @@ class Command {
                }
 
                $useLogPipe = false;
-               $cmd = $envcmd . trim( $this->command );
+               $cmd = $envcmd . trim( $command );
 
                if ( is_executable( '/bin/bash' ) ) {
                        $time = intval( $this->limits['time'] );
@@ -335,7 +336,7 @@ class Command {
 
                $profileMethod = $this->method ?: wfGetCaller();
 
-               list( $cmd, $useLogPipe ) = $this->buildFinalCommand();
+               list( $cmd, $useLogPipe ) = $this->buildFinalCommand( $this->command );
 
                $this->logger->debug( __METHOD__ . ": $cmd" );
 

diff --git a/includes/shell/FirejailCommand.php b/includes/shell/FirejailCommand.php
index 79f679d..0338b53 100644 (file)
--- a/includes/shell/FirejailCommand.php
+++ b/includes/shell/FirejailCommand.php
@@ -59,10 +59,10 @@ class FirejailCommand extends Command {
        /**
         * @inheritDoc
         */
-       protected function buildFinalCommand() {
+       protected function buildFinalCommand( $command ) {
                // If there are no restrictions, don't use firejail
                if ( $this->restrictions === 0 ) {
-                       return parent::buildFinalCommand();
+                       return parent::buildFinalCommand( $command );
                }
 
                if ( $this->firejail === false ) {
@@ -122,6 +122,10 @@ class FirejailCommand extends Command {
 
                if ( $this->hasRestriction( Shell::NO_EXECVE ) ) {
                        $seccomp[] = 'execve';
+                       // Normally firejail will run commands in a bash shell,
+                       // but that won't work if we ban the execve syscall, so
+                       // run the command without a shell.
+                       $cmd[] = '--shell=none';
                }
 
                if ( $seccomp ) {
@@ -136,11 +140,10 @@ class FirejailCommand extends Command {
                        $cmd[] = '--net=none';
                }
 
-               list( $fullCommand, $useLogPipe ) = parent::buildFinalCommand();
-
                $builtCmd = implode( ' ', $cmd );
 
-               return [ "$builtCmd -- $fullCommand", $useLogPipe ];
+               // Prefix the firejail command in front of the wanted command
+               return parent::buildFinalCommand( "$builtCmd -- {$command}" );
        }
 
 }

diff --git a/tests/phpunit/includes/shell/FirejailCommandTest.php b/tests/phpunit/includes/shell/FirejailCommandTest.php
index c9db74f..fab14ca 100644 (file)
--- a/tests/phpunit/includes/shell/FirejailCommandTest.php
+++ b/tests/phpunit/includes/shell/FirejailCommandTest.php
@@ -29,38 +29,38 @@ class FirejailCommandTest extends PHPUnit_Framework_TestCase {
                // @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";
+               $limit = "/bin/bash '$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"
+                               'ls', 0, "$limit ''\''ls'\''' $env"
                        ],
                        [
                                'default restriction',
                                'ls', Shell::RESTRICT_DEFAULT,
-                               "firejail --quiet $profile $default -- /bin/bash '$limit' ''\''ls'\''' $env"
+                               "$limit 'firejail --quiet $profile $default -- '\''ls'\''' $env"
                        ],
                        [
                                'no network',
                                'ls', Shell::NO_NETWORK,
-                               "firejail --quiet $profile --net=none -- /bin/bash '$limit' ''\''ls'\''' $env"
+                               "$limit 'firejail --quiet $profile --net=none -- '\''ls'\''' $env"
                        ],
                        [
                                'default restriction & no network',
                                'ls', Shell::RESTRICT_DEFAULT | Shell::NO_NETWORK,
-                               "firejail --quiet $profile $default --net=none -- /bin/bash '$limit' ''\''ls'\''' $env"
+                               "$limit 'firejail --quiet $profile $default --net=none -- '\''ls'\''' $env"
                        ],
                        [
                                'seccomp',
                                'ls', Shell::SECCOMP,
-                               "firejail --quiet $profile --seccomp=@default -- /bin/bash '$limit' ''\''ls'\''' $env"
+                               "$limit 'firejail --quiet $profile --seccomp=@default -- '\''ls'\''' $env"
                        ],
                        [
                                'seccomp & no execve',
                                'ls', Shell::SECCOMP | Shell::NO_EXECVE,
-                               "firejail --quiet $profile --seccomp=@default,execve -- /bin/bash '$limit' ''\''ls'\''' $env"
+                               "$limit 'firejail --quiet $profile --shell=none --seccomp=@default,execve -- '\''ls'\''' $env"
                        ],
                ];
        }
@@ -75,7 +75,7 @@ class FirejailCommandTest extends PHPUnit_Framework_TestCase {
                        ->params( $params )
                        ->restrict( $flags );
                $wrapper = TestingAccessWrapper::newFromObject( $command );
-               $output = $wrapper->buildFinalCommand();
+               $output = $wrapper->buildFinalCommand( $wrapper->command );
                $this->assertEquals( $expected, $output[0], $desc );
        }