From: Kunal Mehta <legoktm@member.fsf.org>
Date: Sat, 9 Dec 2017 10:11:02 +0000 (-0800)
Subject: shell: Run firejail inside limit.sh, make NO_EXECVE work
X-Git-Tag: 1.31.0-rc.0~1136^2
X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=416975c3ac1bf838369846c2ba2e3217edcde2cb

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 998b3ed905..2f0ea42238 100644
--- 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 79f679d87b..0338b5325e 100644
--- 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 c9db74f5f9..fab14cad7a 100644
--- 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 );
 	}