This sounds pretty cool to me. We probably want to land @MaxSem 's refactor of wfShellExec() into a class first before adding new functionality: https://gerrit.wikimedia.org/r/#/c/319505/

Change 384930 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/core@master] [POC] Optionally restrict commands with firejail

https://gerrit.wikimedia.org/r/384930

Change 384931 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/core@master] Allow shell commands to restrict network access

https://gerrit.wikimedia.org/r/384931

Change 384931 abandoned by Legoktm:
Allow shell commands to restrict network access

Reason:
squashed into parent

https://gerrit.wikimedia.org/r/384931

Change 384930 merged by jenkins-bot:
[mediawiki/core@master] shell: Optionally restrict commands' access with firejail

https://gerrit.wikimedia.org/r/384930

The core framework has been merged, I'll file subtasks to convert Wikimedia's manual firejailing over to use it and then I think we can close this.

Change 393825 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[operations/mediawiki-config@master] Set $wgRestrictionMethod = 'firejail'; everywhere

https://gerrit.wikimedia.org/r/393825

Documented at https://www.mediawiki.org/wiki/Manual:Shell_framework#Restrictions and https://www.mediawiki.org/wiki/Manual:$wgShellRestrictionMethod

Change 393825 merged by jenkins-bot:
[operations/mediawiki-config@master] Set $wgRestrictionMethod = 'firejail'; everywhere

https://gerrit.wikimedia.org/r/393825

Mentioned in SAL (#wikimedia-operations) [2017-12-04T19:17:12Z] <legoktm@tin> Synchronized wmf-config/InitialiseSettings.php: Set $wgRestrictionMethod = 'firejail'; everywhere (T173370) (duration: 00m 43s)

This looks pretty cool, however this requires every client to manually set up restrictions. Which is pretty easy to forget or to be completely unaware such feature even exists. I wonder if we shouldn't have default settings, that apply to every shell invocation, unless the client specifically invokes different one? Or maybe even stronger, a configured mandatory mask that applies to every shell invocation in general (e.g., nobody ever gets root or /dev access, no matter how much they ask, but if they ask explicitly, they can get network). The idea is to provide some default level so extension writer doesn't have to be aware of the whole thing unless they need to, and wiki admin knowing if the extension doesn't call specific methods (which is easy to check for) it will have uniform security policy with the rest of the wiki. What do you think?

In T173370#3811353, @Smalyshev wrote:

This looks pretty cool, however this requires every client to manually set up restrictions. Which is pretty easy to forget or to be completely unaware such feature even exists. I wonder if we shouldn't have default settings, that apply to every shell invocation, unless the client specifically invokes different one? Or maybe even stronger, a configured mandatory mask that applies to every shell invocation in general (e.g., nobody ever gets root or /dev access, no matter how much they ask, but if they ask explicitly, they can get network). The idea is to provide some default level so extension writer doesn't have to be aware of the whole thing unless they need to, and wiki admin knowing if the extension doesn't call specific methods (which is easy to check for) it will have uniform security policy with the rest of the wiki. What do you think?

Agreed in full that this should be our eventual goal. However, simply flipping it on by default would be a huge change and basically impossible to test properly. So our current implementation is opt-in, which lets us review each command for these kinds of restrictions. Once we have most things reviewed/restricted, we can re-evaluate the value of RESTRICT_DEFAULT (e.g. maybe NO_NETWORK should go in it) and make some restrictions opt-out. Ideally we can do this before the 1.31 release.

The main follow-up work on the patch itself were Tim's suggestions for more specific features that I didn't address:

• Deny read access to LocalSettings.php. We could even call get_included_files() and thereby automatically deny read access to PrivateSettings.php.

T182484: Add shell restriction to deny access to LocalSettings.php

• Require a private temporary directory. The common pattern of writing an input file to a temporary directory then running a command would need to be improved. Perhaps wfTempDir() could return a private subdirectory.

T179901: Create a tmp directory just for MediaWiki

• Deny execve. This would prevent a range of vulnerabilities, and is used to great effect in apparmor.

This was implemented, but it doesn't actually work: T182489: The NO_EXECVE shell restriction doesn't work with firejail because of limit.sh

• Some sort of pre-configured netfilter option which will deny access to memcached and similar services, for the benefit of commands that can't use NO_NETWORK.

T182490: Implement shell restriction alternative to NO_NETWORK that only allows HTTP(S) traffic

@Smalyshev I filed T182741: Determine whether firejail shell restrictions can be enabled by default to figure out how we can enable this by default.

With that I think everything now has follow-up tasks, so we can close this as resolved.