This is a partial fix for CVE-2016-1585, it address the frontend rule encoding problems particularly
- Permissions being given that shouldn't happen
- Multiple option conditionals in a single rule resulting in wider permission instead of multiple rules
- optional flags not being handled correctly
- multiple backend rules being created out of one frontend rule when they shouldn't be
it does not address the backend issue of short cut permissions not being correctly updated when deny rules carve out permissions on an allow rule that has a short cut permission in the encoding.
Thanks to the additional work by Alexander Mikhalitsyn for beating this MR into shape so we can land it
Alexander Changelog:
- rebased to an actual tree
- addressed review comments from @wbumiller and @setharnold
- fixed compiler warnings about class_mount_hdr is uninitialized
- infinite loop fix
- MS_MAKE_CMDS bitmask value fixed
- fixed condition in `gen_flag_rules` to cover cases like `mount options in (bind) /d -> /4,` when flags are empty and only opt_flags are present
- marked some tests as a FAIL case behavior was changed after `parser: add conflicting flags check for options= conditionals` commit
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/333
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
These tests contains incompatible mount options and broken
after ("parser: add conflicting flags check for options= conditionals")
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
gen_flag_rules has a boolean vs bit and case where parenthesis are
helpful to express the intended order of operations.
It also doesn't handle the case where there are no matches. Fix this
by causing that case to fail.
also improve the debug of option extraction.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Now that flag processing for mount rules with single option
conditionals are fixed e-enable multiple mount conditionals on a
single mount rule. The mount conditionals are equivalent to specifying
multiple rules.
mount options=(a,b,c) options=(c,d),
is the same as
mount options=(a,b,c),
mount options=(c,d),
and
mount options in (a,b,c) options in (c,d),
is the same as
mount options in (a,b,c),
mount options in (c,d),
when multiple options= and options in are combined in a single rule
it is the same as the cross product of the options.
where
mount options=(a,b,c) options in (d,e),
is a single rule.
mount options=(a,b,c) options=(d,e) options in (f),
is equivalent to
mount options=(a,b,c) options in (f),
mount options=(d,e) options in (f),
and while it is not recommended that multiple options= and options in
conditions be used in a single rule.
mount options=(a,b,c) options=(d,e) options in (f) options in (g),
is equivalent to
mount options=(a,b,c) options in (f),
mount options=(a,b,c) options in (g),
mount options=(d,e) options in (f),
mount options=(d,e) options in (g),
Bug Link: https://bugs.launchpad.net/apparmor/+bug/1597017
Signed-off-by: John Johansen <john.johansen@canonical.com>
- rebased to bba1a023bf
- fixed infinite loop in mnt_rule::gen_policy_re
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
- rebased to bba1a023bf
- after this commit tests mount/ok_[16-19].sd are failing,
but it's correct
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The combined optional flag and exact match flag processing is problematic
separate out the optional flag processing so it is only combined during
match string generation.
While doing so we fix the flag output so that multiple rules are
not output when they shouldn't be.
In addition we temporarily break multiple options= and 'options in'
conditionals in a single rule, which we will fix in a separate patch.
Bug Link: https://bugs.launchpad.net/apparmor/+bug/1597017
Signed-off-by: John Johansen <john.johansen@canonical.com>
- rebased to bba1a023bf
- made tests happy by changing condition in gen_policy_re()
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Simplify flag masking and fix the MS_MAKE_CMDS flag set. This is a
step in fixing
Bug Link: https://bugs.launchpad.net/apparmor/+bug/1597017
Signed-off-by: John Johansen <john.johansen@canonical.com>
- rebased to bba1a023bf
- fixed MS_MAKE_CMDS definition to the correct one.
We shouldn't add (MS_ALL_FLAGS & ~(MNT_FLAGS)) to this bitmask.
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
A step in cleaning up mount rule generation, split out the handling
of mount rules that imply multiple rules to make it easier to
see what is going on.
Bug Link: https://bugs.launchpad.net/apparmor/+bug/1597017
Signed-off-by: John Johansen <john.johansen@canonical.com>
Do a minimal code refactoring (ie. no functional changes, just moving
code,adding boiler plate and glue) in preparation to fix
bug https://bugs.launchpad.net/apparmor/+bug/1597017
Bug Link: https://bugs.launchpad.net/apparmor/+bug/1597017
Signed-off-by: John Johansen <john.johansen@canonical.com>
- rebased to bba1a023bf
- fixed compiler warnings:
<built-in>: In member function ‘int mnt_rule::gen_policy_new_mount(Profile&, int&, unsigned int, unsigned int)’:
<built-in>: note: by argument 1 of type ‘const char*’ to ‘long unsigned int __builtin_strlen(const char*)’ declared here
mount.cc:880:14: note: ‘class_mount_hdr’ declared here
880 | char class_mount_hdr[64];
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
dbus 1.14.4 changed the behavior of unix:tmpdir to be equivalent to
unix:dir, which cases dbus-daemon to generate path based sockets,
instead of the previous abstract sockets. [1]
In this change we force dbus-daemon to generate an abstract socket by
specifying the abstract socket address in the command.
[1] https://gitlab.freedesktop.org/dbus/dbus/-/blob/dbus-1.14/NEWS#L64
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/999
Approved-by: John Johansen <john@jjmx.net>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
dbus 1.14.4 changed the behavior of unix:tmpdir to be equivalent to
unix:dir, which cases dbus-daemon to generate path based sockets,
instead of the previous abstract sockets. [1]
In this change we force dbus-daemon to generate an abstract socket by
specifying the abstract socket address in the command.
[1] https://gitlab.freedesktop.org/dbus/dbus/-/blob/dbus-1.14/NEWS#L64
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
See the individual commits for details.
Signed-off-by: Noel Power <noel.power@suse.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/989
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
For example winbindd when configured on a samba system using
sssd can trigger
apparmor="DENIED" operation="file_mmap" profile="/usr/sbin/winbindd" name="/usr/lib64/krb5/plugins/authdata /sssd_pac_plugin.so" pid=2798 comm="winbindd" requested_mask="m" denied_mask="m" fsuid=52311 ouid=0
Signed-off-by: Noel Power <noel.power@suse.com>
# profiles/apparmor.d/samba*: allow access to pid files directly in /run/
On Arch Linux, `samba-dcerpcd.pid` is in `/run/`, not `/run/samba/`.
apparmor="DENIED" operation="mknod" profile="samba-dcerpcd" name="/run/samba-dcerpcd.pid" pid=80920 comm="samba-dcerpcd" requested_mask="c" denied_mask="c" fsuid=0 ouid=0
The same is true for `nmbd.pid`, `smbd.pid` and probably others too.
# samba-dcerpcd requires access to `/var/cache/samba/names.tdb`.
audit: type=1400 audit(1676835286.187:62): apparmor="DENIED" operation="open" profile="samba-dcerpcd" name="/var/cache/samba/names.tdb" pid=6948 comm="samba-dcerpcd" requested_mask="wrc" denied_mask="wrc" fsuid=0 ouid=0
See also https://bbs.archlinux.org/viewtopic.php?id=281411
Since `usr.sbin.winbindd` already has a rule for it, and `usr.sbin.nmbd`
has similar ones, simply add `/var/cache/samba/*.tdb rwk` to
`abstractions/samba`.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/987
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
On Arch Linux, `samba-dcerpcd.pid` is in `/run/`, not `/run/samba/`.
apparmor="DENIED" operation="mknod" profile="samba-dcerpcd" name="/run/samba-dcerpcd.pid" pid=80920 comm="samba-dcerpcd" requested_mask="c" denied_mask="c" fsuid=0 ouid=0
The same is true for `nmbd.pid`, `smbd.pid` and probably others too.
samba-dcerpcd requires access to `/var/cache/samba/names.tdb`.
audit: type=1400 audit(1676835286.187:62): apparmor="DENIED" operation="open" profile="samba-dcerpcd" name="/var/cache/samba/names.tdb" pid=6948 comm="samba-dcerpcd" requested_mask="wrc" denied_mask="wrc" fsuid=0 ouid=0
See also https://bbs.archlinux.org/viewtopic.php?id=281411
Since `usr.sbin.winbindd` already has a rule for it, and `usr.sbin.nmbd`
has similar ones, simply add `/var/cache/samba/*.tdb rwk` to
`abstractions/samba`.
This MR continues #280 work by changing string formatting in Python tests. `str.format()` is now used in places where bracket escapes are not necessary. In places where escapes would be necessary, I've ensured that format arguments are tuple-guarded.
I also fixed some potential future bugs that I noticed in the process:
- mutable default arguments that were luckily not being mutated
- a tuple missing a comma that conveniently functioned properly because `iter('_')` and `iter(('_',))` are effectively the same
Additionally, I fixed one current bug in the process:
- `dict.keys()` no longer returns a list in Python 3, so you can no longer call `.sort()` on its return value
I figured that the bug fixes are small enough that they don't deserve separate MRs, but if you disagree then I will happily split this up.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/985
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
... now that they all got it added.
Also remove a superfluous backslash in another make target.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/980
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
... to just the basename and profile name.
This also means to make the firefox and firefox.sh profiles named profiles.
Note: in the `firefox.sh` profile, the local include was added just today, therefore I think changing it (instead of adding) should be fine.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/975
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
contributed by Werner Fink via
https://bugzilla.opensuse.org/show_bug.cgi?id=1065388 comment 25
Note that - compared to the file in bugzilla - I removed the `rix` rules
for /usr/bin/groff and /usr/bin/nroff so that people can choose to ix,
Px or Cx groff/nroff as they wish, and then include the abstraction
inside the target profile to allow executing all the helpers.
I also added `include if exists <abstractions/groff.d>`
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/973
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
This MR addresses step 1 of 6 in #280, i.e. merging Launchpad translations into `master`. Tests are currently failing in this branch due to newly-introduced hotkey conflicts. I therefore prefixed this MR's title with "Draft: " to prevent premature merging. How would you like these hotkey conflicts resolved?
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/966
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
... for var/lib/nscd/passwd and var/lib/nscd/group
I propose this patch for all branches.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/960
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
This was due to the values being defined in both af_unix and af_rule leaving the latter values unset.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/979
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
