[apparmor] [patch] dnsmasq profile update

[Christian Boltz]

Hello,

Am Freitag, 18. September 2015 schrieb Christian Boltz:
> Am Mittwoch, 16. September 2015 schrieb Seth Arnold:
> > I don't like the /dev/tty; that deserves more investigation. The
> > fscanf() on 70 is reading a file specified in a configuration
> > option,
> > so that's not likely to be it.

Are you sure about that? Line 45 does
         leasestream = popen(daemon->dhcp_buff, "r");
and in line 70 leasestream is used:
    while (fscanf(leasestream, "%255s %255s", daemon->dhcp_buff3, daemon->dhcp_buff2) == 2)

I'm quite sure fscanf is working on the popen stream from line 45 here.

In other words: either my ability to read C is worse than I thought, or
you misread the code ;-)

(There's also an else branch that fopen's the lease file as leasestream 
in line 58 - maybe that's what you noticed.)

> I asked in the bugreport and will send the /dev/tty patch again if
> someone gives me a good reason to do that. Otherwise, well, it's just
> SLE and I "randomly" found that patch [1], so... ;-)

Cédric Bosdonnat answered:

---------------------------------------------------------------------
> However, upstream didn't like the "/dev/tty rw" permissions, [...]

When commenting that rule, I'm getting this in the logs, thought it 
doesn't prevent dnsmasq from running:

[  469.229207] type=1400 audit(1442822084.589:117): apparmor="DENIED" operation="open" parent=2250 profile="/usr/sbin/dnsmasq" name="/dev/tty" pid=2251 comm="sh" requested_mask="rw" denied_mask="rw" fsuid=0 ouid=0

I'll investigate a bit more to understand why dnsmasq needs this.

[... a comment later ...]

In fact this is surely for the process to write to stdout / stderr. 
Don't forget that the lease helper is started with popen (aka sh -c).
---------------------------------------------------------------------

I'd say that makes sense, therefore I propose the remaining part of the
patch again:


[patch] dnsmasq profile update: allow /dev/tty

This patch is based on a SLE12 patch to allow executing the
--dhcp-script. We already have most parts of that patch since r2841,
except /dev/tty rw which is needed for the shell's stdout and stderr.

>From looking at the source (link from the SLE bugreport)
http://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=blob;f=src/lease.c;h=8adb60588671324d9ddf00d7dab40474d40d4393;hb=HEAD#l45
I'd guess that fscanf() (line 70) should explain it - it works on the
leasestream that was popen'ed in line 45.

References: https://bugzilla.opensuse.org/show_bug.cgi?id=940749 (non-public)


I propose this patch for trunk and 2.9.


[ dnsmasq-profile-boo940749-dev-tty.diff ]

=== modified file 'profiles/apparmor.d/usr.sbin.dnsmasq'
--- profiles/apparmor.d/usr.sbin.dnsmasq        2015-07-24 18:56:27 +0000
+++ profiles/apparmor.d/usr.sbin.dnsmasq        2015-09-16 12:03:40 +0000
@@ -29,6 +29,8 @@
   signal (receive) peer=/usr/sbin/libvirtd,
   ptrace (readby) peer=/usr/sbin/libvirtd,
 
+  /dev/tty rw,
+
   /etc/dnsmasq.conf r,
   /etc/dnsmasq.d/ r,
   /etc/dnsmasq.d/* r,






Regards,

Christian Boltz
-- 
After perilous fights with terrific monsters which live in our
dark dusty store rooms I have now an Epson LQ 500 printer!
[Johannes Meixner in https://bugzilla.novell.com/show_bug.cgi?id=173782]