Page MenuHomePhabricator

Investigate why firejails break PdfHandler
Closed, ResolvedPublic

Description

For T164000 we made $wgPdfProcessor and $wgPdfPostProcessor use /usr/local/bin/mediawiki-firejail-ghostscript and /usr/local/bin/mediawiki-firejail-convert

But it was noted in T164045 that it caused a breakage

thumbnail failed on mw2150: error 0 "Reading profile /etc/firejail/mediawiki-converters.profile
Reading profile /etc/firejail/mediawiki-converters.profile
]0;firejail /usr/bin/convert -depth 8 -quality 95 -resize 180 - /tmp/transform_75a822c4e8a2.jpg convert: no decode delegate for this image format `' @ error/constitute.c/ReadImage/501.
convert: no images defined `/tmp/transform_75a822c4e8a2.jpg' @ error/convert.c/ConvertImageCommand/3210.
Parent pid 28613, child pid 28615
Parent is shutting down, bye..." from "('/usr/local/bin/mediawiki-firejail-ghostscript' '-sDEVICE=jpeg' '-sOutputFile=-' '-dFirstPage=2' '-dLastPage=2' '-dSAFER' '-r150' '-dBATCH' '-dNOPAUSE' '-q' '/tmp/localcopy_ee85b9c71615.pdf' | '/usr/local/bin/mediawiki-firejail-convert' '-depth' '8' '-quality' '95' '-resize' '180' '-' '/tmp/transform_75a822c4e8a2.jpg')"

It probably makes sense longer term to find out why it's broken, and ideally fix it, and then reapply https://gerrit.wikimedia.org/r/#/c/350643/ / revert the revert in https://gerrit.wikimedia.org/r/#/c/350981/ and run it in production again

https://github.com/wikimedia/puppet/blob/production/modules/mediawiki/files/mediawiki-firejail-ghostscript
https://github.com/wikimedia/puppet/blob/production/modules/mediawiki/files/mediawiki-firejail-convert
https://github.com/wikimedia/puppet/blob/production/modules/mediawiki/files/mediawiki-converters.profile

Details

Related Gerrit Patches:
operations/puppet : productionmediawiki-firejail: explicitly signal end of options
operations/mediawiki-config : masterRe-instate "Run Pdf Processors in firejails"
operations/puppet : productionMake mediawiki-firejail-ghostscript quiet

Event Timeline

Reedy created this task.Apr 29 2017, 9:54 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 29 2017, 9:54 AM
Reedy updated the task description. (Show Details)Apr 29 2017, 10:10 AM
Reedy updated the task description. (Show Details)Apr 29 2017, 1:55 PM
Tgr added a comment.Apr 30 2017, 6:48 PM
tgr@wasat:~$ /usr/bin/gs -sDEVICE=jpeg -sOutputFile=/home/tgr/Special_301_Report_2014.jpeg -dFirstPage=2 -dLastPage=2 -dSAFER -r150 -dBATCH -dNOPAUSE -q /home/tgr/Special_301_Report_2014.pdf
<works>

tgr@wasat:~$ /usr/bin/firejail --profile=/etc/firejail/mediawiki-converters.profile /usr/bin/gs -sDEVICE=jpeg -sOutputFile=/home/tgr/Special_301_Report_2014.jpeg -dFirstPage=2 -dLastPage=2 -dSAFER -r150 -dBATCH -dNOPAUSE -q /home/tgr/Special_301_Report_2014.pdf 
Reading profile /etc/firejail/mediawiki-converters.profile
Parent pid 17344, child pid 17345
Child process initialized
Error: /undefinedfilename in (/home/tgr/Special_301_Report_2014.pdf)
Operand stack:

Execution stack:
   %interp_exit   .runexec2   --nostringval--   --nostringval--   --nostringval--   2   %stopped_push   --nostringval--   --nostringval--   --nostringval--   false   1   %stopped_push
Dictionary stack:
   --dict:1171/1684(ro)(G)--   --dict:0/20(G)--   --dict:77/200(L)--
Current allocation mode is local
Last OS error: Permission denied
GPL Ghostscript 9.06: Unrecoverable error, exit code 1

Parent is shutting down, bye...

So it seems firejail is messing up parameter parsing somehow which results in gs not finding the file. It does work without the --profile flag though.

Tgr added a comment.Apr 30 2017, 7:07 PM

Never mind that. That's expected behavior from the profile blacklisting /home.

The actual error is that ghostscript outputs some junk: when I redirect the standard output to a file, it starts with

ESC]0;firejail /usr/bin/gs -sDEVICE=jpeg -sOutputFile=- -dFirstPage=2 -dLastPage=2 -dSAFER -r150 -dBATCH -dNOPAUSE -q /tmp/tgr-T164145-Special_301_Report_2014.pdf ^G<FF><D8><FF><E0>^@^PJFIF^@^A^A^A^@<96>^@<96>^@^@<FF><E2>

and ends with

Parent pid 22910, child pid 22911

Parent is shutting down, bye...

Using a filename with -sOutputFile works fine. This is not happening with plain gs.

So it seems that firejail interferes with the ability of gs to detect that it is being called with non-terminal stdout.

Tgr added a comment.Apr 30 2017, 7:30 PM

Or rather, firejail mixes its own output with the gs output. All that's needed is to make it shut up:

tgr@wasat:~$ '/usr/bin/firejail' '--profile=/etc/firejail/mediawiki-converters.profile' '/usr/bin/gs' '-sDEVICE=jpeg' '-sOutputFile=-' '-dFirstPage=2' '-dLastPage=2' '-dSAFER' '-r150' '-dBATCH' '-dNOPAUSE' '-q' '/tmp/tgr-T164145-Special_301_Report_2014.pdf' | '/usr/local/bin/mediawiki-firejail-convert' '-depth' '8' '-quality' '95' '-resize' '180' '-' '/tmp/tgr-T164145-Special_301_Report_2014-transformed.jpeg'
Reading profile /etc/firejail/mediawiki-converters.profile
Reading profile /etc/firejail/mediawiki-converters.profile
Parent pid 32362, child pid 32363
Child process initialized
convert: no decode delegate for this image format `' @ error/constitute.c/ReadImage/501.
convert: no images defined `/tmp/tgr-T164145-Special_301_Report_2014-transformed.jpeg' @ error/convert.c/ConvertImageCommand/3210.

Parent is shutting down, bye...

tgr@wasat:~$ '/usr/bin/firejail' '--quiet' '--profile=/etc/firejail/mediawiki-converters.profile' '/usr/bin/gs' '-sDEVICE=jpeg' '-sOutputFile=-' '-dFirstPage=2' '-dLastPage=2' '-dSAFER' '-r150' '-dBATCH' '-dNOPAUSE' '-q' '/tmp/tgr-T164145-Special_301_Report_2014.pdf' | '/usr/local/bin/mediawiki-firejail-convert' '-depth' '8' '-quality' '95' '-resize' '180' '-' '/tmp/tgr-T164145-Special_301_Report_2014-transformed.jpeg'
Reading profile /etc/firejail/mediawiki-converters.profile
Parent pid 32401, child pid 32402
Child process initialized

Parent is shutting down, bye...

Change 351123 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[operations/puppet@production] Make mediawiki-firejail-ghostscript quiet

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

Change 351123 merged by Muehlenhoff:
[operations/puppet@production] Make mediawiki-firejail-ghostscript quiet

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

Change 338979 had a related patch set uploaded (by Hashar; owner: Hashar):
[operations/puppet@production] mediawiki-firejail: explicitly signal end of options

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

hashar added a subscriber: hashar.May 2 2017, 7:51 AM

...
So it seems firejail is messing up parameter parsing somehow which results in gs not finding the file. It does work without the --profile flag though.

Yes that is broken, because firejail eats parameters it recognizes and does not pass them to the other command. That is solved by passing separating the arguments for firejail and the ones from the command by using: --

https://gerrit.wikimedia.org/r/338979 mediawiki-firejail: explicitly signal end of options

Change 352572 had a related patch set uploaded (by Reedy; owner: Reedy):
[operations/mediawiki-config@master] Re-instate "Run Pdf Processors in firejails"

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

MoritzMuehlenhoff triaged this task as Medium priority.May 10 2017, 6:43 AM

Change 352572 merged by jenkins-bot:
[operations/mediawiki-config@master] Re-instate "Run Pdf Processors in firejails"

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

Mentioned in SAL (#wikimedia-operations) [2017-06-05T21:01:55Z] <reedy@tin> Synchronized wmf-config/CommonSettings.php: Run Pdf Processors in firejails T164145 T164000 (duration: 00m 40s)

Reedy closed this task as Resolved.Jun 5 2017, 9:03 PM
Reedy claimed this task.

Redeployed, purging PDFs works fine, doesn't break thumbnailing now!

Change 338979 abandoned by Hashar:
mediawiki-firejail: explicitly signal end of options

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