Code review comment for lp:~sdeziel/apparmor-profiles/unbound-refresh

Revision history for this message
Simon Déziel (sdeziel) wrote :

Upstream wants the PID file to be tentatively deleted when the daemon stops. Since this is not always possible failing to do so isn't fatal:

        /* this unlink may not work if the pidfile is located outside
         * of the chroot/workdir or we no longer have permissions */
        if(daemon->pidfile) {
                int fd;
                /* truncate pidfile */
                fd = open(daemon->pidfile, O_WRONLY | O_TRUNC, 0644);
                if(fd != -1)
                        close(fd);
                /* delete pidfile */
                unlink(daemon->pidfile);
        }

In fact, on Debian/Ubuntu, even with the needed caps to make the PID writeable by the low priv user, it cannot delete it because it's outside of the chroot. I just tested with/without the caps and the PID is never deleted.

One drawback of denying chown/dac_overide is that unbound complains:

  unbound: [21915:0] error: cannot chown 112.121 /run/unbound.pid: Operation not permitted

The error could be turned into a debug message and then we could do without the additional caps. I'll check with upstream if they would accept such change. Thanks for the feedback Seth!

« Back to merge proposal