On Mon, 2018-01-22 at 22:55 +0100, Vincent Blut wrote: > Hi Jamie, > > On Mon, Jan 22, 2018 at 02:17:26PM -0600, Jamie Strandboge wrote: > > Package: chrony > > Version: 3.2-1 > > Severity: wishlist > > Tags: patch > > User: [email protected] > > Usertags: origin-ubuntu bionic ubuntu-patch > > > > Dear Maintainer, > > > > In Ubuntu, the attached patch was applied to achieve the following: > > > > * add AppArmor profile for /usr/sbin/chronyd: > > - add debian/usr.sbin.chronyd AppArmor profile > > - debian/control: Build-Depends on dh-apparmor > > - debian/dirs: create etc/apparmor.d/force-complain > > - debian/install: install debian/usr.sbin.chronyd > > - debian/preinst: force-complain on upgrade before this version > > - debian/rules: install apparmor profile with dh_apparmor > > > > Thanks for considering the patch. For Debian, you would need to do > > is update > > the version in preinst to the version which ships the AppArmor > > profile. > > Awesome! > Note that I was working on a chronyd Apparmor profile for Debian, so > please see my review below.
Cool
> > diff -Nru chrony-3.2/debian/usr.sbin.chronyd
> > chrony-3.2/debian/usr.sbin.chronyd
> > --- chrony-3.2/debian/usr.sbin.chronyd 1969-12-31
> > 18:00:00.000000000
> > -0600
> > +++ chrony-3.2/debian/usr.sbin.chronyd 2018-01-20
> > 03:20:00.000000000 -0600
> > @@ -0,0 +1,39 @@
> > +# Last Modified: Sat Jan 20 10:45:05 2018
> > +#include <tunables/global>
>
> +#include <tunables/sys>
>
> We will need this until #871441¹ and #1728551² are fixed to support
> the
> “tempcomp” directive. See the attached diff for details.
>
Your changes look fine to me.
> > +
> > +/usr/sbin/chronyd (attach_disconnected) {
> > + #include <abstractions/base>
> > + #include <abstractions/nameservice>
> > +
> > + capability sys_time,
> > + capability net_bind_service,
> > + capability setuid,
> > + capability setgid,
>
> + capability sys_nice,
> + capability sys_resource,
>
> The first one is needed to support chronyd's “-P” option; The second
> one
> is needed to have the ability to lock chronyd into RAM.
Makes sense. +1
> > +
> > + /usr/sbin/chronyd mr,
> > +
> > + /etc/chrony/{,**} r,
> > + /run/chronyd.pid w,
> > + /run/chrony/{,*} rw,
>
> I think we should prefix /run/* paths by /{,var/} to make our
> profile
> easier to port to other distros as some of them have yet to migrate
> from
> /var/run to /run.
This sounds fine too. Note that your profile has this:
/{,var/}/run/chronyd.pid w,
/{,var/}/run/chrony/{,*} rw,
but you want this (to avoid /var//run/...):
/{,var/}run/chronyd.pid w,
/{,var/}run/chrony/{,*} rw,
I'll update our upload to bionic and the upstream PR for your changes.
Thanks!
--
Jamie Strandboge | http://www.canonical.com
signature.asc
Description: This is a digitally signed message part
