[pkg-apparmor] Bug#883682: don't install features-file as conffile for easier overriding

[Fabian Grünbichler]

On Thu, Dec 07, 2017 at 08:47:52AM +0100, intrigeri wrote:
> Control: tag -1 - patch
> Control: severity -1 normal
> 
> Hi,

hi (no need to CC me, I subscribe to all bugs I submit ;))

> 
> Fabian Grünbichler:
> > see attached patch.
> 
> Thanks!
> 
> > I didn't find a branch for the s-p-u upload (but
> > that might just be my non-existing bzr skills failing me),
> 
> https://alioth.debian.org/scm/loggerhead/collab-maint/apparmor-stretch/changes
> https://anonscm.debian.org/bzr/collab-maint/apparmor-stretch/
> 
> I'll update the Vcs-* fields on that branch if we ever do another
> stretch-pu.

thanks for the pointer!

> 
> > for applying to master/unstable, an additional rm_conffile is probably a
> > good idea to decruft /etc/apparmor.
> 
> Yes, absolutely ⇒ please update the patch (so I'm removing the "patch"
> tag as I cannot apply it as-is).
> 
> > I decided to move the features file into /usr/share/apparmor-features
> > because /usr/share/apparmor is already the default include search path.
> 
> Sounds good to me. No need to create that directory in the packaging?

updating/extending the patch with the above points is no problem, but
see below for further questions before I proceed.

> 
> > it would be possible to extend this further and e.g. provide multiple
> > features files for different (Debian) kernel versions, and symlink the
> > default one which is referenced in /etc/apparmor/parser.conf ?
> 
> Good idea for the future, let's not bother for now.
> 
> > it would be rather important for us to know whether you plan on applying
> > and including this for the point release on Sunday (or postpone the
> > feature pinning apparmor update until the next point release)
> 
> 2.11.0-3+deb9u1 won't be part of the next point-release due to the
> kernel bug you helped identify, that breaks mount operations when
> pinning a feature set that supposedly disables mount mediation.
> 
> > I am not sure whether we are the only derivative/downstream/.. affected
> > by this change, but it has the potential to break a lot of setups using
> > their own (more recent / patched to support more of AA) kernel and AA
> > profiles on top of Stretch..
> 
> With my AppArmor in Debian maintainer hat, I've never heard of people
> running this kind of things in production until you reached out to me.
> So these people might exist, but they're not talking to me. Thanks to
> you I'm now aware of this use case and we can work together to support
> it better :)

I guess we do run a bit of an unusual setup here:

- base Debian
- kernel based on Ubuntu's, but with some custom modifications
- more recent and modified LXC/LXCFS/Qemu
- more recent and sometimes patched storage related packages (Ceph, ZFS)
- custom clustering and management stack (AGPL)
- some glue inbetween to workaround incompatibilities ;)

we only use AA for LXC at the moment, although there are vague plans of
extending it to VM guests as well some day. one of the reasons for
choosing the Ubuntu kernel as base was and is the good integration
between upstream LXC, AA and the kernel.

> 
> >> > intrigeri:
> >> >> Understood. Ideally parser.conf would be complemented by
> >> >> /etc/apparmor/parser.conf.d/*.conf, which could be sourced at the end
> >> >> of parser.conf somehow. And then we can ship the default parser.conf
> >> >> in /usr. TTBOMK we have no way to source such additional config
> >> >> drop-in snippets though. I suspect upstream would be happy to consider
> >> >> patches that add this feature :)
> >> 
> >> > yes, that would have been nice. alas, there is no such thing now, and
> >> > getting one in time for the upcoming point release is not really an
> >> > option.. maybe in time for buster?
> >> 
> >> >> If we had this drop-in snippet support for complementing the default
> >> >> parser.conf, then both your use case and that one would be supported
> >> >> nicely, right?
> >> 
> >> > yes.
> >> 
> >> Would you be willing to work on such a feature upstream so downstreams
> >> & derivatives have a cleaner (than diversion) way to address
> >> this problem?
> 
> > that should be do-able, but I won't have time to take a closer look this
> > week.
> 
> Sure, no hurry. It would be good to have an upstream bug on Launchpad
> about this, so we don't forget about it and we have a place to discuss
> this with other upstream people :)

https://bugs.launchpad.net/apparmor/+bug/1736896

if such a feature becomes available soon, do you think it would be
viable to move the feature pinning directive into such a snippet file,
instead of or in addition to moving the feature file to /usr/share/foo?
if so, would that be something that is in scope to be cherry-picked for
Stretch in a point release?

if the answer is yes, I would try to avoid moving stuff twice and focus
on this snippet mechanism for now, and only falling back to moving the
features file (and diverting it downstream) as a last resort. otherwise,
moving the features file to a non-conffile location should be done
sooner rather than later, and the snippet mechanism can just be
introduced as part of the next AA upstream release (whenever that
happens) and used to override the features-file directive downstream
whenever it becomes available, replacing the divert.