Investigate why firejails break PdfHandler
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Reedy
	Apr 29 2017, 9:54 AM

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

In T164045#3222412, @Tgr wrote:

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

Subject	Repo	Branch	Lines +/-
mediawiki-firejail: explicitly signal end of options	operations/puppet	production	+5 -5
Re-instate "Run Pdf Processors in firejails"	operations/mediawiki-config	master	+2 -0
Make mediawiki-firejail-ghostscript quiet	operations/puppet	production	+1 -1

Customize query in gerrit

Related Objects

Mentioned In: T188122: Specific PDF file only displays completely white page previews
T164000: ghostscript dSafer bypass
Mentioned Here: T164000: ghostscript dSafer bypass
T164045: PDF thumbnails fail to render on newly-uploaded PDF files

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

Paladox subscribed.Apr 29 2017, 10:49 AM

Reedy updated the task description. (Show Details)Apr 29 2017, 1:55 PM

MoritzMuehlenhoff subscribed.Apr 29 2017, 2:06 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.

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.

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

gerritbot added a project: Patch-For-Review.Apr 30 2017, 8:02 PM

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

In T164145#3224445, @Tgr wrote:

...
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)

Stashbot mentioned this in T164000: ghostscript dSafer bypass.Jun 5 2017, 9:01 PM

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

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