Commit graph

20 commits

Author SHA1 Message Date
Brad Jorsch
86cfcfdbba Shell: Don't hang on empty stdin
If the write buffer for a file descriptor is empty, don't try to write
to it. Just close it and continue on.

Bug: T188019
Change-Id: Ie5b5ac1ef1aec4ae763cf4d0d58d3a28e42b7d2a
2018-02-22 17:13:28 -05:00
Brad Jorsch
38bf4c1521 Shell: Set pipes to non-blocking
The select(2) system call only guarantees a "sufficiently small write"
can be made without blocking. It doesn't define what that means.

And on Linux the read might block too in certain cases, although I don't
know if any of them can occur here.

Regardless, set all the pipes to non-blocking, which avoids the blocking
that's behind T184171.

And then, since a non-blocking read might validly return empty-string or
a non-blocking write might validly return 0, use feof() to check for EOF
and actually close the write pipe when it runs out of data.

Bug: T184171
Change-Id: I403235a328630112b6920905730f933777e2d453
2018-02-01 16:04:12 -05:00
Gergő Tisza
0e211c4f29 Allow programmatic input in Command
Bug: T182463
Change-Id: Ib68180c7af12558686f4864c24fd85f01201d6fb
2018-01-03 19:53:47 +03:00
Umherirrender
255d76f2a1 build: Updating mediawiki/mediawiki-codesniffer to 15.0.0
Clean up use of @codingStandardsIgnore
- @codingStandardsIgnoreFile -> phpcs:ignoreFile
- @codingStandardsIgnoreLine -> phpcs:ignore
- @codingStandardsIgnoreStart -> phpcs:disable
- @codingStandardsIgnoreEnd -> phpcs:enable

For phpcs:disable always the necessary sniffs are provided.
Some start/end pairs are changed to line ignore

Change-Id: I92ef235849bcc349c69e53504e664a155dd162c8
2018-01-01 14:10:16 +01:00
Kunal Mehta
416975c3ac 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
2017-12-09 04:07:32 -08:00
Max Semenik
36009e3ca7 Shell: skip null parameters
Right now they're treated as empty strings, however
this doesn't allow skipping parameters in the middle like
 $params = [
     'foo',
     $x ? '--bar' : null,
     '--baz',
 ];

In some cases this matters, e.g. `ls` works while `ls ''` doesn't.

Also, fix spacing problems the new tests uncovered:
* Extra space when using params()
* Missing space when combining params() and unsafeParams()

Change-Id: Icb29d4c48ae7f92fb5635e3865346c98f47abb01
2017-11-29 12:38:35 -08:00
Kunal Mehta
bdb5b592f4 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
2017-11-28 00:06:40 +00:00
Gergő Tisza
7d9dbc0040
MediaWiki\Shell: log stderr
Change-Id: I1495fe2aba10102d7e36c3a3e5fdabf97f14546b
2017-10-26 21:06:03 -07:00
Kunal Mehta
c3bbadcc83 Shell\Command: Move code that builds final shell command into separate method
Change-Id: I6aae209fd0b20057b5f7f7129db92c184ec945f8
2017-10-19 19:38:42 -07:00
Max Semenik
32912b8c8d Introduce Shell\CommandFactory
Bug: T177038
Change-Id: Id875e68ea1fa72b44a463f977ab52270fe1e7088
2017-10-17 18:55:11 -07:00
Max Semenik
4d9a95f970 Don't trigger PHP errors for unused Shell\Command
Change-Id: Id29da4f21a44ccb18d8a2ae11348d69ca3233aa5
2017-10-17 18:55:11 -07:00
Kunal Mehta
29e36cc4d8 Command: Avoid using wfDebug()
Change-Id: I92ea55d7a5b6a71a6f6b944f377215c08ea3b096
2017-10-17 18:22:42 -07:00
Max Semenik
945f8870d3 Shell\Command: Better walltime fallback
Previously, it assumed that the only way times could be overridden is to
reduce the limits - which isn't the case for video transcoding.

Bug: T178314
Change-Id: I492a44f280a36ee666e9963788caac2bbc6bc6f3
2017-10-17 03:43:09 +00:00
Fomafix
af72813a12 shell: Deduplicate code in Command.php by combining else paths
Also reduce indenting.

Change-Id: I33f83786c38bba0919372df0d5cdfa806d4361fc
2017-10-12 20:46:15 +00:00
Max Semenik
926c97c69f
Return stderr from Shell\Command
Change-Id: I5551ae4bbe7b539b528a734aa82198b11f103871
2017-10-12 02:12:20 -07:00
jenkins-bot
d4f712c326 Merge "Suppress error in MediaWiki\Shell\Command" 2017-10-10 18:52:53 +00:00
Gergő Tisza
2ebb0ca271 Suppress error in MediaWiki\Shell\Command
Command uses a certain error message to detect and ignore
EINTR in stream_select, and uses trigger_error to clear
the message from get_last_error (clear_last_error is PHP7 only).
This works rather poorly with a system config that does not catch
or ignore most errors; specifically it breaks database tests
on Vagrant with the warnings_as_errors role on.

Change-Id: I9c8f922bc0a8f5ee6b8e7501b22223cce4f98ecb
2017-10-09 00:28:02 +00:00
Max Semenik
7c3d3d54bd Get rid of $IP in Command
Change-Id: Iccfe1b79963462f9cad80ff327ccd574ee1122c5
2017-10-06 19:50:45 -07:00
Max Semenik
1bb9a223d2 Inject dependencies into Shell\Command
This slightly changes how execution time limits fall back on each other.

Change-Id: I7754a9e6be9638eebe90cb953adb8e2a6ee97cef
2017-10-03 20:01:59 -07:00
Max Semenik
77ce3b98a0 Replace wfShellExec() with a class
This function has gotten so unwieldy that a helper was
introduced. Instead, here's this class that makes
shelling out easier and more readable.

Example usage:
  $result = Shell::command( 'shell command' )
       ->environment( [ 'ENVIRONMENT_VARIABLE' => 'VALUE' ] )
       ->limits( [ 'time' => 300 ] )
       ->execute();

  $exitCode = $result->getExitCode();
  $output = $result->getStdout();

This is a minimal change, so lots of stuff remains
unrefactored - I'd rather limit the scope of this commit.
A future improvement could be an ability to get stderr
separately from stdout.

Caveat: execution errors (proc_open is disabled/returned error) now
throw errors instead of returning a status code. wfShellExec() still
emulates this behavior though.

Competing commit: I7dccb2b67a4173a8a89b035e444fbda9102e4d0f
<legoktm> MaxSem: so you should continue working on your patch and I'll
          probably refactor on top of it later after its merged :P

Change-Id: I8ac9858b80d7908cf7e7981d7e19d0fc9c2265c0
2017-09-08 21:49:49 -07:00