Bug 1150338 - Drop Ghostscript apparmor profile as it is useless
Drop Ghostscript apparmor profile as it is useless
Status: RESOLVED MOVED
Classification: openSUSE
Product: openSUSE Tumbleweed
Classification: openSUSE
Component: Security
Current
All openSUSE Factory
: P2 - High : Major (vote)
: ---
Assigned To: Johannes Segitz
E-mail List
https://rudin.suse.de:8894/request/sh...
:
Depends on: 1134327
Blocks:
  Show dependency treegraph
 
Reported: 2019-09-11 12:32 UTC by Dr. Werner Fink
Modified: 2019-10-02 10:05 UTC (History)
7 users (show)

See Also:
Found By: Community User
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---


Attachments
ghostscript-wrapper.patch (24.32 KB, text/plain)
2019-09-13 08:01 UTC, Dr. Werner Fink
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Dr. Werner Fink 2019-09-11 12:32:28 UTC
+++ This bug was initially created as a clone of Bug #1134327 +++
Comment 1 Christian Boltz 2019-09-11 13:08:33 UTC
I could easily troll you by changing the bug title, but maybe you can be a bit more verbose about your reasoning? ;-)

BTW: https://rudin.suse.de:8894/request/show/701304 is non-public - can you please explain what it contains?
Comment 2 Dr. Werner Fink 2019-09-11 13:23:02 UTC
(In reply to Christian Boltz from comment #1)
> I could easily troll you by changing the bug title, but maybe you can be a
> bit more verbose about your reasoning? ;-)
> 
> BTW: https://rudin.suse.de:8894/request/show/701304 is non-public - can you
> please explain what it contains?

As I'm now maintainer for the time where Johannes is regeneration you have to note that I will speak plaintext :P  And in this case I say that the apparmor profile is useless as I know how ghostscript or better libgs works.

As long as only scripts which are using /usr/bin/gs (or now /usr/bin/gs.bin) which its self is using libgs as a few more programs and libraries out there I call this profile useless.  I do not see any advantage to break helper scripts of the package ghostscript by apparmor if the real work is done by the binary gs and its library ... somehow snake oil here.

For ghostscript using libgs I've written a small wrapper script using bwrap from package bubblewrap (compare with https://github.com/bitstreamout/ghostscriptwrap and package gswrap).  This works at least for the binary and its library.
Comment 3 Dr. Werner Fink 2019-09-11 13:26:29 UTC
AFAICS the request is about fixing the broken profile for the ps2epsi helper script.
Comment 4 Johannes Segitz 2019-09-12 06:31:47 UTC
Since the new sandboxing mechanism is better anyway the AA can be dropped. Apart from the helper script issues the main problem is here that AA doesn't have any clue about what GS wants to do. So the profile needs to be overly broad to prevents legitimate use cases from failing. I'll submit the change
Comment 5 Dr. Werner Fink 2019-09-12 07:43:25 UTC
(In reply to Johannes Segitz from comment #4)
> Since the new sandboxing mechanism is better anyway the AA can be dropped.
> Apart from the helper script issues the main problem is here that AA doesn't
> have any clue about what GS wants to do. So the profile needs to be overly
> broad to prevents legitimate use cases from failing. I'll submit the change

Indeed as AA seems to be designed to catch bufferoverlow and attacks based on that but for an attack with an interpreter provided by a library (the gs it self is a very small program) it seems that AA has some shortcomings.

Beside this my sanding approach also does not catch libnrary calls from other programs ... here something like a library mapper which redirects API calls to open(2)/fopen(3), and rename(2)/unlink(2)/remove(3) or the simple poor approach I've used for older ghostscript version might help
Comment 6 Johannes Segitz 2019-09-12 08:47:39 UTC
(In reply to Dr. Werner Fink from comment #5)
I think the approach you chose for the older ghostscripts would be worthwile.

I'll submit soonish to remove the AA profile (still building)
Comment 7 Christian Boltz 2019-09-12 12:02:43 UTC
(In reply to Dr. Werner Fink from comment #2)
> As I'm now maintainer for the time where Johannes is regeneration you have
> to note that I will speak plaintext :P 

No problem, I prefer that over having to read between the lines ;-)

> As long as only scripts which are using /usr/bin/gs (or now /usr/bin/gs.bin)
> which its self is using libgs as a few more programs and libraries out there
> I call this profile useless. 

Note that _everything_ calling /usr/bin/gs (not only the helper scripts) will get the AppArmor confinement. (Well, as soon as someone adjusts the profile to cover /usr/bin/gs.bin - I can do that if you decide to keep it ;-)

> I do not see any advantage to break helper
> scripts of the package ghostscript by apparmor if the real work is done by
> the binary gs and its library ... somehow snake oil here.

I've noticed a few minor issues in the helper scripts (see bug 1134722), bot nothing terrible. An AppArmor profile of course never hurts, but dropping it for the helper scripts doesn't sound too scary ;-)

(It might make sense to have separate profiles for a) gs.bin and b) the helper scripts, but that's another topic.)

Oh, and just for the records - if /usr/bin/gs uses the ghostscript library, the library usage is of course confined by AppArmor.

What AppArmor can't cover is if another binary (for example a document viewer) is linked against the ghostscript library. In this case, you'll need a profile for the document viewer.


> For ghostscript using libgs I've written a small wrapper script using bwrap
> from package bubblewrap (compare with
> https://github.com/bitstreamout/ghostscriptwrap and package gswrap).  This
> works at least for the binary and its library.

Correct me if I'm wrong, but IIRC the plan was to let people choose between the "traditional" gs (now gs.bin) and the bubblewrap solution via update-alternatives. If this is still correct, I'd strongly recommend to keep the AppArmor profile for gs.bin for people who don't want to use bubblewrap for whatever reason. (For example, my current tumbleweed has /etc/alternatives/gs -> /usr/bin/gs.bin - I never did any manual changes to it, so it seems to be the current default.)


(In reply to Johannes Segitz from comment #4)
> Since the new sandboxing mechanism is better anyway the AA can be dropped.
> Apart from the helper script issues the main problem is here that AA doesn't
> have any clue about what GS wants to do. So the profile needs to be overly
> broad to prevents legitimate use cases from failing. I'll submit the change

I know that the profile needs to be quite permissive, but at least ;-) it prevents executing other binaries not listed in the profile, and IIRC preventing remote code execution was one of the reasons the profile was introduced.

IMHO the AppArmor profile should be kept as long as openSUSE ships /usr/bin/gs.bin.
Comment 8 Martin Wilck 2019-09-13 07:24:36 UTC
(In reply to Christian Boltz from comment #7)
> (For example, my current tumbleweed has
> /etc/alternatives/gs -> /usr/bin/gs.bin - I never did any manual changes to
> it, so it seems to be the current default.)

The default changes to /usr/bin/gs.wrap if you install the gswrap package.

gs - auto mode
  link best version is /usr/bin/gs.wrap
  link currently points to /usr/bin/gs.wrap
  link gs is /usr/bin/gs
/usr/bin/gs.bin - priority 15
/usr/bin/gs.wrap - priority 100

Anyway, as I've been asked to review https://build.opensuse.org/request/show/730528 - is there consensus about the AA profile?
Comment 9 Dr. Werner Fink 2019-09-13 08:01:08 UTC
Created attachment 818106 [details]
ghostscript-wrapper.patch

(In reply to Johannes Segitz from comment #6)
> (In reply to Dr. Werner Fink from comment #5)
> I think the approach you chose for the older ghostscripts would be worthwile.
> 

This patch compiles but is untested
Comment 10 Christian Boltz 2019-09-15 21:49:57 UTC
(In reply to Martin Wilck from comment #8)
> (In reply to Christian Boltz from comment #7)
> > (For example, my current tumbleweed has
> > /etc/alternatives/gs -> /usr/bin/gs.bin - I never did any manual changes to
> > it, so it seems to be the current default.)
> 
> The default changes to /usr/bin/gs.wrap if you install the gswrap package.

Well, _if_. On the latest Tumbleweed (dup'ed since years), gswrap doesn't get installed automatically, therefore I'm quite sure that 99% of the Tumbleweed users still use gs.bin.

Even with the quite broad profile we have now, I don't see the point in removing the AppArmor profile because
- removing it makes things less secure (even the very broad profile can for 
  exammple prevent executing binaries)
- most people (still?) use gs.bin because it's (still?) the default on a
  regularly dup'ed Tumbleweed
- even if at some point in the future most people use gs.wrap, we shouldn't 
  reduce security for those using gs.bin

> Anyway, as I've been asked to review
> https://build.opensuse.org/request/show/730528 - is there consensus about
> the AA profile?

I strongly recommend to keep the profile (and to extend it to also attach to gs.bin, not only gs)
Comment 11 Dr. Werner Fink 2019-09-16 06:02:32 UTC
(In reply to Christian Boltz from comment #10)
> (In reply to Martin Wilck from comment #8)
> 
> > Anyway, as I've been asked to review
> > https://build.opensuse.org/request/show/730528 - is there consensus about
> > the AA profile?
> 
> I strongly recommend to keep the profile (and to extend it to also attach to
> gs.bin, not only gs)

If AA profile then yes for gs.bin as this is the main binary now ... and no for the shell scripts using gs(.bin) as this does not increase security at all but cause more complicated rules not to break scripts.
Comment 12 Johannes Segitz 2019-09-17 08:41:17 UTC
I don't really see much use for the profile as it is and would prefer to go for a solution that uses knowledge about the actual task that gs should carry out to sandbox it. But OTOH I also don't have strong feelings that we should remove it. I'll drop the helpers and resubmit later today
Comment 13 Johannes Segitz 2019-09-23 11:59:32 UTC
(In reply to Johannes Segitz from comment #12)
not really today, but now it's there: sr#732679
Comment 14 Dr. Werner Fink 2019-09-23 13:17:53 UTC
(In reply to Johannes Segitz from comment #13)
> (In reply to Johannes Segitz from comment #12)
> not really today, but now it's there: sr#732679

Thanks,  I've only one question: does this work with the /usr/bin/gs.wrap script linked to /usr/bin/gs  which calls the programs bash, id, ldd, mktemp, mkfifo, sed, and bwrap?
Comment 15 Martin Wilck 2019-09-24 15:35:21 UTC
sr#732679 has been accepted and is on the way to factory (sr#732862).

I'd like to see the answer to comment 14 though.
Comment 16 Johannes Segitz 2019-09-26 14:33:56 UTC
(In reply to Dr. Werner Fink from comment #14)
yes, according to my testing this works (but this doesn't mean that for different use cases it might bail)
Comment 17 Dr. Werner Fink 2019-10-02 10:05:29 UTC
Then close this a moved