apparmor denial to several paths to binaries

[Christian Ehrhardt ]

[Impact]

 * Apparmor denies access to bin directories which the option parsing code
   of ntp touches.

[Test Case]

 1. get a container of target release
 2. install ntp
    apt install ntp
 3. watch dmesg on container-host
    dmesg -w
 4. restart ntp in container
    systemctl restart ntp
 => see (or no more after fix) apparmor denie:
 apparmor="DENIED" operation="open" profile="/usr/sbin/ntpd" name="/usr/local/sbin/" pid=23933 comm="ntpd" requested_mask="r" denied_mask="r"
 apparmor="DENIED" operation="open" profile="/usr/sbin/ntpd" name="/usr/local/bin/" pid=23933 comm="ntpd" requested_mask="r" denied_mask="r"

[Regression Potential]

 * we are only slightly opening up the apparmor profile, but none of the
   changes poses a security risk so regression potential on it's own
   should be close to zero.

 * we discussed if this would be a security risk but came to the
   conclusion that r-only should be ok (the same content anyone can grab
   from the archive by installing the packages)

[Other Info]

 * n/a

Issue shows up (non fatal) as:
 apparmor="DENIED" operation="open" profile="/usr/sbin/ntpd" name="/usr/local/sbin/" pid=23933 comm="ntpd" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
 apparmor="DENIED" operation="open" profile="/usr/sbin/ntpd" name="/usr/local/bin/" pid=23933 comm="ntpd" requested_mask="r" denied_mask="r" fsuid=0 ouid=0

Since non crit this is mostyl about many of us being curious why it actually does do it :-)

[Christian Ehrhardt  (paelzer)]

I found a setup that triggers it again (often but not always happening).

Once reproducible I took backtraces of the sys open to see what in ntp does that.

Both opens are from the same place.
Thread 1 "ntpd" hit Catchpoint 1 (returned from syscall open), 0x00007f8afd8fcd53 in __opendir (name=0x7ffc9999df90 "/usr/local/sbin")
    at ../sysdeps/posix/opendir.c:191
191 in ../sysdeps/posix/opendir.c
(gdb) bt
#0 0x00007f8afd8fcd53 in __opendir (name=0x7ffc9999df90 "/usr/local/sbin") at ../sysdeps/posix/opendir.c:191
#1 0x00007f8afe5c6ba7 in ?? () from /usr/lib/x86_64-linux-gnu/libopts.so.25
#2 0x00007f8afe5cdde6 in ?? () from /usr/lib/x86_64-linux-gnu/libopts.so.25
#3 0x00007f8afe5cfa4f in optionProcess () from /usr/lib/x86_64-linux-gnu/libopts.so.25
#4 0x000055c12781ae7f in parse_cmdline_opts (pargc=0x7ffc9999f0dc, pargv=0x7ffc9999f0d0) at ntpd.c:359
#5 0x000055c12781af8d in ntpdmain (argc=<optimized out>, argv=<optimized out>) at ntpd.c:575
#6 0x00007f8afd84a1c1 in __libc_start_main (main=0x55c12780b0e0 <main>, argc=8, argv=0x7ffc9999f6b8, init=<optimized out>,
    fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7ffc9999f6a8) at ../csu/libc-start.c:308
#7 0x000055c12780b11a in _start ()

[Christian Ehrhardt  (paelzer)]

libopts is of autogen
See
autogen-5.18.12/autoopts/autoopts.c

Note for gdb on affected systems:
(gdb) catch syscall open
(gdb) run -p /var/run/ntpd.pid -g -c /run/ntp.conf.dhcp -u 112:116

dbg on libopts is odd
get from https://launchpad.net/ubuntu/bionic/amd64/libopts25-dbgsym/1:5.18.12-3

Now Details are better:
#0 0x00007fd4f489fd53 in __opendir (name=name@entry=0x7fff724da8d0 "/usr/local/sbin") at ../sysdeps/posix/opendir.c:191
#1 0x00007fd4f5569ba7 in option_pathfind (path=0x7fff724ddf82 "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin",
    fname=fname@entry=0x7fff724dd836 "/usr/sbin/ntpd", mode=<synthetic pointer>) at ../compat/pathfind.c:61
#2 0x00007fd4f5570de6 in validate_struct (opts=opts@entry=0x55a84db841e0 <ntpdOptions>, pname=0x7fff724dd836 "/usr/sbin/ntpd") at init.c:108
#3 0x00007fd4f5572a4f in optionProcess (opts=0x55a84db841e0 <ntpdOptions>, a_ct=8, a_v=0x7fff724dbff8) at autoopts.c:335
#4 0x000055a84d8e8e7f in parse_cmdline_opts (pargc=0x7fff724dba1c, pargv=0x7fff724dba10) at ntpd.c:359
#5 0x000055a84d8e8f8d in ntpdmain (argc=<optimized out>, argv=<optimized out>) at ntpd.c:575
#6 0x00007fd4f47ed1c1 in __libc_start_main (main=0x55a84d8d90e0 <main>, argc=8, argv=0x7fff724dbff8, init=<optimized out>,
    fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fff724dbfe8) at ../csu/libc-start.c:308

[Christian Ehrhardt  (paelzer)]

So what happens is this:
1. ntp verifies its options
2. the binary name is always included, so we get a verify in libopts like
   validate_struct (opts=opts@entry=0x55a84db841e0 <ntpdOptions>, pname=0x7fff724dd836 "/usr/sbin/ntpd")
3. if opts->pzProgName is not set validate_struct will check for the binary through paths
4. it calls pathfind which looks through all of PATH
5. there is uses opendir and wants to enumerate things (to find the prog)

If path does not include forbidden dir's the error is non existing.

So the denie is really low severity - although it partially is stupid programming as it is not really needed.

I wonder if we should add an allow or even a deny rule to just silence it?

Since this only happens in later ntp versions an upstream change might have dropped opts->pzProgName somehow to now trigger.

[Martin Pitt (pitti)]

The most plausible explanation for enumerating /usr/local/bin/ is that ntpd has some hooks.d/ mechanism which gets called after syncing the time, and that runs a shell in between. So IMHO this should be allowed.

[Christian Ehrhardt  (paelzer)]

@Martin - hi, thanks to chime in.
There are no hooks of that kind, but really just option parsing on (re-)start.

But still if it is happy with "r" only I might add a rule for it.

[Christian Ehrhardt  (paelzer)]

And btw - we had such rules for /bin and /usr/bin already - maybe even for the same reasons.
Chances are that the actual change is that /usr/local/bin and /usr/local/sbin are now in PATH of the service.

[Christian Ehrhardt  (paelzer)]

Yep - testing shows that read is enough and since we have such rules for similar paths I think it is fine to add them.
Prepping a change to the packaging.

[Seth Arnold (seth-arnold)]

Thanks for investigating!

[Christian Ehrhardt  (paelzer)]

FYI - Fix in bionic upload queue

[Launchpad Janitor (janitor)]

This bug was fixed in the package ntp - 1:4.2.8p10+dfsg-5ubuntu5

---------------
ntp (1:4.2.8p10+dfsg-5ubuntu5) bionic; urgency=medium

  * debian/apparmor-profile: avoid denies to to arg checks (LP: #1741227)

 -- Christian Ehrhardt <email address hidden> Thu, 04 Jan 2018 14:20:53 +0100

[Tony (tony-sarajarvi)]

Still happens with MAAS deployed Ubuntu 17.10.
And with update I get "ntp is already the newest version (1:4.2.8p10+dfsg-5ubuntu3.1)." How do I get the last part that's "3.1" into that "5" if that matters?

[Christian Ehrhardt  (paelzer)]

Hi Tony,
yeah this is so far only fixed in Bionic (18.04) onwards.
It it not really an issue other than a log message itself (it is not needed for the opt parsing).
It occurs "only" on NTP start/restart (since it is arg parsing) so it is not that frequent that it would fill up the log or similar secondary issues.

All that makes it a hard case for the SRU policy [1] to make this change in releases.
Until then everybody who bothers about the log message can add:
   /usr/local/{,s}bin/ r,
to
   /etc/apparmor.d/usr.sbin.ntpd

I'm adding X/A bug tasks and set won't fix to mark that more explicitly.

I'm happy to discuss if one thinks this case would qualify fot the SRU policy - the change itself is easy enough to be done.

[1]: https://wiki.ubuntu.com/StableReleaseUpdates

[Christian Ehrhardt  (paelzer)]

I looked a bit around and found another apparmor change which I think will open up a reasonable SRU for ntp on apparmor. And while we do it anyway we can also add this rule.

So this now depends on bug 1749389 completing in Bionic and then can be made one SRU together.

[Christian Ehrhardt  (paelzer)]

Bionic - ok
SRU Template - ok
Debdiff for X/T checked - ok
Tested A upload from ppa - ok.
(This issue in particular doesn't apply to Xenial, so dropping this task)

[Christian Ehrhardt  (paelzer)]

fix in SRU queue (Artful) for review by the SRU Team

[Chris J Arges (arges)]

Hello ChristianEhrhardt, or anyone else affected,

Accepted ntp into artful-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/ntp/1:4.2.8p10+dfsg-5ubuntu3.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-artful to verification-done-artful. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-artful. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Christian Ehrhardt  (paelzer)]

Verification of Proposed:
[2020342.769272] audit: type=1400 audit(1518622578.674:4871): apparmor="DENIED" operation="open" namespace="root//lxd-artful-test_<var-snap-lxd-common-lxd>" profile="/usr/sbin/ntpd" name="/usr/local/sbin/" pid=16638 comm="ntpd" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
[2020342.769282] audit: type=1400 audit(1518622578.674:4872): apparmor="DENIED" operation="open" namespace="root//lxd-artful-test_<var-snap-lxd-common-lxd>" profile="/usr/sbin/ntpd" name="/usr/local/bin/" pid=16638 comm="ntpd" requested_mask="r" denied_mask="r" fsuid=0 ouid=0

After upgrade from proposed:
- 1:4.2.8p10+dfsg-5ubuntu3.2

The messages above are gone - so verified

[Łukasz Zemczak (sil2100)]

The verification of the Stable Release Update for ntp has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package ntp - 1:4.2.8p10+dfsg-5ubuntu3.2

---------------
ntp (1:4.2.8p10+dfsg-5ubuntu3.2) artful; urgency=medium

  * d/apparmor-profile: avoid denies on argument checks (LP: #1741227)
  * d/apparmor-profile: fix denial checking for running ntpdate (LP: #1749389)

 -- Christian Ehrhardt <email address hidden> Wed, 14 Feb 2018 13:14:24 +0100

[Christian Ehrhardt  (paelzer)]

FYI - accepted by Debian in debian/1%4.2.8p11+dfsg-1