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

NO_EXECVE doesn't work because limit.sh needs to execute the main
command, and does so through the execve syscall. Eventually we should be
able to replace limit.sh with firejail functionality entirely (T179021),
but in the meantime we can run firejail inside limit.sh.

We also need to stop firejail from running the command in a bash shell
via --shell=none, since that shell would also use the execve syscall.

Bug: T182489
Change-Id: I3fc8ad2f9e5eb5bf13b49d0bccd6094668a5ec55

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