[systemd-devel] Supervisory Watchdog notification not working when using SmackProcessLabel

[Martin Townsend]

Any comments from systemd devs on this issue? I'm particulary keen to
know if using the very permissive values on the /run/systemd/notify is
advisable or whether this would cause any security issues.

On Wed, Aug 1, 2018 at 6:46 PM Martin Townsend <mtownsend1973 at gmail.com> wrote:
>
> Hi Casey,
>
> Thanks you for you prompt response.
> On Wed, Aug 1, 2018 at 5:32 PM Casey Schaufler <casey at schaufler-ca.com> wrote:
> >
> > On 8/1/2018 3:18 AM, Martin Townsend wrote:
> > > Hi,
> > >
> > > I have a service running with a SmackProcessLabel that uses the
> > > supervisory watchdog feature, ie calls sd_notify().  The Watchdog
> > > keeps resetting the service and I get the following in the journal
> > >
> > > Jul 27 11:36:11 kernel: audit: type=1400 audit(1532691371.270:34):
> > > lsm=SMACK fn=smack_unix_may_send action=denied subject="apphealthd"
> > > object="_" requested=w pid=466 comm="apphealthd"
> > > path="/run/systemd/notify"
> > >
> > > /run/systemd/notify is a socket so I'm guessing sd_notify kicks the
> > > watchdog by writing to this socket.  The problem seems to be that the
> > > socket is labelled with the floor label.
> >
> > What does "attr -S -g SMACK64 /run/systemd/notify" report?
> > It should say that the attribute value is "*". If it doesn't,
> > what kernel and systemd versions are you running? What is your
> > base system? (Tizen, Fedora, ...)
>
> It did report "*" for this socket file descriptor for the
> security.SMACK64 attribute.
> I'm running a 4.9 Kernel built using Yocto for the i.MX6UL so it's the
> NXP 4.9 Kernel.  Systemd is 234 which is the version included in the
> Rocko release of Yocto.
>
> >
> > UDS socket access controls are done based on the processes
> > involved, not the file system object. Do you have any services
> > running with the floor ("_") label?
>
> Most services run with their own label but there are system services
> running with the floor label.
>
> >
> >
> > > After looking through the code that sets up the notify socket I
> > > quickly patched in some code to set SMACK64IPIN and IPOUT (not sure if
> > > this one is required).
> > >
> > > @@ -728,7 +729,12 @@ static int manager_setup_notify(Manager *m) {
> > >
> > >                  m->notify_fd = fd;
> > >                  fd = -1;
> > > -
> > > +                r = mac_smack_apply_fd(m->notify_fd, SMACK_ATTR_IPIN, "*");
> > > +                if (r < 0)
> > > +                        log_error_errno(r, "mac_smack_apply_ip_in_fd: %m");
> > > +                r = mac_smack_apply_fd(m->notify_fd, SMACK_ATTR_IPOUT, "@");
> > > +                if (r < 0)
> > > +                        log_error_errno(r, "mac_smack_apply_ip_out_fd: %m");
> > >                  log_debug("Using notification socket %s", m->notify_socket);
> > >          }
> > >
> > > And the audit message has gone.
> >
> > Your code above is the best way to ensure your service can
> > talk to everyone. You have to be sure talking to everyone is
> > really what you want to do.
>
> This applies the SMACK64_IPIN and IPOUT on the notify socket
> (/run/systemd/notify) so I'm hoping it means that any
> application/service can write to the notify socket, as far as I can
> tell this socket is used to kick the systemd supervisory watchdog for
> a specific application.  If I used a specific label then I take it I
> would have to use something like
>
> r = mac_smack_apply_fd(m->notify_fd, SMACK_ATTR_IPIN, "systemd_notify");
>
> and have a rule like:
>
> service_label systemd_notify rwx
>
> for every service that runs with a SMACK process label and uses the
> systemd supervisory watchdog.  I don't think there would be any harm
> using '*' and '@' for /run/systemd/notify though to avoid this,
> hopefully the systemd devs can confirm this?
>
> >
> > > Is there a better way of ensuring /run/systemd/notify can be accessed
> > > by a service with a User defined SMACK label? or is this patch to
> > > manager_setup_notify sufficient?
> >
> > You can always add access rules to allow communications.
> > In this case it would be
> >
> >         apphealthd _ w
> >         _ apphealthd w
> >
> > Generally you don't want to add rules for the floor ("_")
> > label, but that's where you're seeing an issue. If you compile
> > with SECURITY_SMACK_BRINGUP and use
> >
> >         apphealthd _ wb
> >         _ apphealthd wb
> >
> > accesses permitted by these rules will be logged so that you
> > can track down who you're talking to.
>
> Yes, trying to avoid rules with floor in :)  Thanks for the tip on
> SECURITY_SMACK_BRINGUP though, I'm sure it will come in useful in the
> future.
>
> >
> > >
> > > Many Thanks in Advance,
> > > Martin.
> > >
> >