[Pkg-chromium-maint] Bug#877391: Please add apparmor profile

[Guido Günther]

Hi Daniel,
On Sun, Oct 01, 2017 at 04:09:16PM -0400, Daniel Richard G. wrote:
> Hi Guido,
> 
> On Sun, 2017 Oct  1 12:28+0200, Guido Günther wrote:
> > Package: chromium
> > Version: 61.0.3163.100-2
> > Severity: wishlist
> > Tags: patch
> > 
> > Hi,
> >
> > I'd be great if Debian would ship an apparmor profile for chromium. The
> > attached profile was mostly prepared by Daniel Richard and is based on
> > the one in Ubuntu so I assume it has seen quiet some exposure to real
> > world usage. It works here nicely here. I'm sure there will be tweaks
> > needed over time so feel free to cc' me and Richard on apparmor related
> > issues. If this shouldn't work out we can always disable it again.
> 
> I had a look at your additions to the profile. Some comments:
> 
> * As mentioned in the earlier bug report, we should add the abstractions
>   file to Debian as well (though not necessarily the same file as Ubuntu
>   has). I'd like to move the aliases into an include file, eventually,
>   and that one would probably make the most sense.

Maintaining a single file is IMHO simpler than splitting this up with no
other users but I'm not tied to this.

> 
> * This line gave me pause:
> 
>     + @{PROC}/@{pid}/task/@{tid}/status rw,
> 
>   I've seen denials from the lack of this line, but have hesitated to
>   add this. I'm quite suspicious of Chromium wanting write access to
>   this proc file of unrelated processes, and would want more information
>   as to why this is needed before allowing this.
> 
>   (@{pid} and @{tid} will one day represent actual kernel variables, but
>   for now they remain basically equivalent to "[0-9]*".)

(yeah, it's a pitty these are currently patterns and not real variables
i apparmor).

> 
>   I've found no issues with this access being denied, and would have in
>   fact added this line with a "deny" qualifier if that didn't also
>   disallow such access to Chromium's own processes.

I'm o.k. using a deny rule instead to silence the denials.

> * The new lines for "tr" and "head": As much as possible, I try to keep
>   lists of similar items in alphabetical order, because it's more work
>   to maintain lists when there isn't a well-defined ordering.
> 
> * The rest looks reasonable, the sort of AppArmor footprint increment
>   that Chromium usually follows.

Thanks for your feedback - makes sense. Feel free to update the patch
accordingly. That said I think it's o.k. to be applied and patched in a
followup.

Cheers,
 -- Guido