Bug 1150338

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?

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

AFAICS the request is about fixing the broken profile for the ps2epsi helper script.

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

(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

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

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

(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?

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

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

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

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

(In reply to Johannes Segitz from comment #12)
not really today, but now it's there: sr#732679

(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?

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.

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

Then close this a moved