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
