Have the parser extract the attachment path from the profile declaration
and make it available as a local variable within the profile. This allows
profile rules to use the executable attachment path in rules.
eg.
```
profile ex /bin/** {
@{attach_path} r,
# ...
}
profile /path/to/bin {
@{attach_path} r,
# ...
}
```
if a profile does not define an attachment like
```
profile noattach {
@{attach_path} r,
}
```
the apparmor_parser will fail the compile with the error.
```
Found reference to variable attach_path, but is never declared
```
While not recommended for rules directly in a profile the above
the undeclared variable error can be avoided in in abstractions
by wrapping the variable in a conditional.
```
if defined @{attach_path} {
@{attach_path r,
}
```
The attachment xattr/label conditionals are not made available at
this time as regular file path rules can not use them.
Similarly a @{exec_path} variable is made available. It is different
than @{attach_path} in that it is intended to be a kernel variable
that represents the specific executable that was matched at run
time. However to support policy on kernels that don't define the
kernel variable it has a fallback value that is the same as
@{attach_path}.
This patch is a follow on to MR:1637 (https://gitlab.com/apparmor/apparmor/-/me\
rge_requests/1637)
and is similar to how the apparmor.d project uses the manually setup
@{exec_path} variable.
We can bike shed over the variable name. @{attach_path} was chosen
here because this is the attachment conditional path for the
executable, not the executable's actual path. While @{exec_path} is
intended to be the applications actual executable path.
support the @{exec_path} kernel variable (all of them atm).
Notes:
The minimize.sh tests are changed because this patch causes path based
profile names to create an attachment. This could be done by doing the
attach_variable expansion in the alternate location marked by the
patch, but since the kernel is going to start doing this for all
profiles that don't have an attachment it is better for the parser to
do it, as it can optimize better.
This patch series may cause breakage if policy declares either
@{attach_path} or @{exec_path} by shadowing those previously declared
variables in the profile block. The previously declared variable
is available in the attachment specification so uses like the
apparmor.d project won't break as it with transfer its variable
value to the attachment which will the transfer that value into
the automatic local var.
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1643
Approved-by: Maxime Bélair <maxime.belair@canonical.com>
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
See the comment for an explanation of why CAP_SYS_ADMIN was being checked and why it isn't actually necessary for setting ionice values for processes
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
Due to how labeling is implemented, during the creation it is not yet
defined, so we need to grant create permissions without attaching the
label yet. Also, adjust tests to pass when label support is
implemented in the kernel.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1623
Approved-by: Maxime Bélair <maxime.belair@canonical.com>
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Make it so the @{exec_path} and @{attach_path} variables behavior
completely as local variables, overriding global variables of the
same name, instead of conflicting with them.
The exec var is only validate for the profile block after the attachment
is defined so the pattern
@{exec_path}=/path
profile test @{exec_path} {
@{exec_path} rw,
}
is valid with the global var defining the attachent which then sets
the local auto @{exec_path} and @{attach_path} variables.
Signed-off-by: John Johansen <john.johansen@canonical.com>
- the autovars not being defined because the profile doesn't have an
attachment
- the autovar conflicting with a user defined var of the same name
Signed-off-by: John Johansen <john.johansen@canonical.com>
This patch updates the set of profiles updated by MR:1637, this is split
off from the rest of the profile updates because that set is explicity
recently set apart.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Have the parser extract the attachment path from the profile declaration
and make it available as a variable within the profile. This allows
profile rules to use the executable attachment path in rules.
eg.
```
profile ex /bin/** {
@{attach_path} r,
# ...
}
profile /path/to/bin {
@{attach_path} r,
# ...
}
```
if a profile does not define an attachment like
```
profile noattach {
@{attach_path} r,
}
```
the apparmor_parser will fail the compile with the error.
```
Found reference to variable attach_path, but is never declared
```
The attachment xattr/label conditionals are not made available at
this time as regular file path rules can not use them.
Similarly a @{exec_path} variable is made available. It is different
than @{attach_path} in that it is intended to be a kernel variable
that represents the specific executable that was matched at run
time. However to support policy on kernels that don't define the
kernel variable it has a fallback value that is the same as
@{attach_path}.
This patch is a follow on to MR:1637 (https://gitlab.com/apparmor/apparmor/-/merge_requests/1637)
and is similar to how the apparmor.d project uses the manually setup
@{exec_path} variable.
We can bike shed over the variable name. @{attach_path} was chosen
here because this is the attachment conditional path for the
executable, not the executable's actual path. While @{exec_path} is
intended to be the applications actual executable path.
support the @{exec_path} kernel variable (all of them atm).
Notes:
The minimize.sh tests are changed because this patch causes path based
profile names to create an attachment. This could be done by doing the
attach_variable expansion in the alternate location marked by the
patch, but since the kernel is going to start doing this for all
profiles that don't have an attachment it is better for the parser to
do it, as it can optimize better.
This patch may cause breakage if policy declares either @{attach_path}
or @{exec_path} this will not be dealt with here, but in a subsequent
patch that allows variables to have a local scope so that the compiler
defined vars will just get declared locally.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The spread pipeline was failing due to missing tests
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1682
Approved-by: Maxime Bélair <maxime.belair@canonical.com>
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
Due to how the debug information shows up when something fails in
spread the information is hard to figure out.
See this example when the allow_all test was missing
https://gitlab.com/apparmor/apparmor/-/jobs/9958642493
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
fuse_overlayfs requires noatime, but we should also allow more flags than
just that to preempt future breakage from flags not included in the rules.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1673
Approved-by: Maxime Bélair <maxime.belair@canonical.com>
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
ipa_verify is a simple libcamera tool that does not use the portion of
libcamera that creates user namespaces. This simple profile should be
enough to replace the previous unconfined profile.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1624
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
This includes testing for options in (list) by itself, along with a rudimentary test for the combination of options=(list) and options in (list).
In particular, the test for the combination confirms that the `apparmor.d` man page was wrong about what happens when these options are combined.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1672
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Specifying norelatime should set the corresponding MS_RELATIME flag clear
bit. Instead, it ORed in MS_NORELATIME, which expands to 0. Properly set
the clear bit by using MS_RELATIME.
Fixes: c9e31b7f "Add mount rules"
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1679
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
In particular, the dbus rules were completely rebuilt based on reading through wpa_supplicant's dbus source code.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1630
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
... and drop rules that are part of abstractions/gtk
Note that abstractions/gtk contains more than the rules dropped here,
which means it effectively extends the permissions granted by
abstractions/gnome.
Idea by darix.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1678
Approved-by: Ryan Lee <rlee287@yahoo.com>
Merged-by: John Johansen <john@jjmx.net>
reported by darix
The initial radv_builtin_shaders rule was added in 4.1, therefore I propose this patch for at least 4.1 and master.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1677
Approved-by: Ryan Lee <rlee287@yahoo.com>
Merged-by: John Johansen <john@jjmx.net>
Reported by darix, seen with comm="sshd-session"
I propose this for master and 4.x (optionally also 3.x even if it's less likely that systems using these branches already use lastlog2)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1676
Approved-by: Ryan Lee <rlee287@yahoo.com>
Merged-by: John Johansen <john@jjmx.net>
- iotop-c fails with permission errors in nl_init without network netlink
raw.
- iotop-c also needs access to the iotop config directory instead of just
the iotoprc file within.
- iotop-c uses CAP_SYS_NICE to set ionice values. For some reason, no
audit log is generated without the capability present, but include it
anyways in case this allowance is due to a parser or kernel bug that
needs to be squashed later.
Fixes: https://bugs.launchpad.net/bugs/2107727
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1675
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
disconnected_mount_complain only contains xpass tests, which should
not be included in the spread XFAIL tests.
Fixes: 1aca4a1d ("tests: regression: mark disconnected-complain-mode tests as xpass")
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1681
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Remove virtual from non-base class fns, as this can hide/make it hard to discover some bugs.
Add override to virtual fns that should be overriding, which helps catch certain class of bugs at compile time
fix(non-virtual-dtor): add missed virtual destructor
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1669
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
For NAME_MAX
Fixes 322a98c8 ("Fix incorrect strnlen length in aa_load.c load_policy_dir")
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1666
Approved-by: Maxime Bélair <maxime.belair@canonical.com>
Approved-by: Ryan Lee <rlee287@yahoo.com>
Merged-by: John Johansen <john@jjmx.net>
disconnected_mount_complain only contains xpass tests, which should
not be included in the spread XFAIL tests.
Fixes: 1aca4a1d ("tests: regression: mark disconnected-complain-mode tests as xpass")
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
- iotop-c fails with permission errors in nl_init without network netlink
raw.
- iotop-c also needs access to the iotop config directory instead of just
the iotoprc file within.
- iotop-c uses CAP_SYS_NICE to set ionice values. For some reason, no
audit log is generated without the capability present, but include it
anyways in case this allowance is due to a parser or kernel bug that
needs to be squashed later.
Fixes: https://bugs.launchpad.net/bugs/2107727
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
The internal permission accumulation is currently broken in that
the ordering of rules matter to whether deny is clearing accumulated
perms.
If a deny node comes before an allow node the deny bits will get set
but the following allow bits won't get cleared by the deny node.
This isn't currently an actual issue for mediation as the deny
bit will be applied at one of
1. apply_and_clear_deny
2. permission remapping
3. run time mediation
but it does result in the internal state having sometimes having both
allow and deny bits set, dependent on order of computation, resulting
in state machines with different sizes because minimization
partitioning is based on the internal permissions.
This means that dfa minimization may not result in a truly minimal
state machine, and even worse can cause inconsistenty and failure in
tests that rely on internal state like the equality and minimization
test, as seen in https://gitlab.com/apparmor/apparmor/-/issues/513
The failure was due to musl stl sets implementation producing a
different ordering of the nodes than glibc. So when the permissions
where accumulated the internal set of permissions were different.
Fix this by giving the different node classes their own internal priority.
This will ensure the bits are properly cleared for that priority before
accumulating.
Note: other ways of fixing.
1. Fixup internal accumulation to use accumulating perms of "higher"
priority as part of the mask (deny and allow mask prompt).
2. Do a hard masking apply at the end after all bits have been accumulated
(ie, in accept_perms after the for loop).
the priority route was chosen because it is a little smaller and
scales better if we get new Node types we have to deal with
(eg. planned complain node).
BugLink: https://gitlab.com/apparmor/apparmor/-/issues/513
Fixes: 1ebd99115 ("parser: change priority so that it accumulates based on permissions")
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1655
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
The internal permission accumulation is currently broken in that
the ordering of rules matter to whether deny is clearing accumulated
perms.
If a deny node comes before an allow node the deny bits will get set
but the following allow bits won't get cleared by the deny node.
This isn't currently an actual issue for mediation as the deny
bit will be applied at one of
1. apply_and_clear_deny
2. permission remapping
3. run time mediation
but it does result in the internal state having sometimes having both
allow and deny bits set, dependent on order of computation, resulting
in state machines with different sizes because minimization
partitioning is based on the internal permissions.
This means that dfa minimization may not result in a truly minimal
state machine, and even worse can cause inconsistenty and failure in
tests that rely on internal state like the equality and minimization
test, as seen in https://gitlab.com/apparmor/apparmor/-/issues/513
The failure was due to musl stl sets implementation producing a
different ordering of the nodes than glibc. So when the permissions
where accumulated the internal set of permissions were different.
Fix this by giving the different node classes their own internal priority.
This will ensure the bits are properly cleared for that priority before
accumulating.
Note: other ways of fixing.
1. Fixup internal accumulation to use accumulating perms of "higher"
priority as part of the mask (deny and allow mask prompt).
2. Do a hard masking apply at the end after all bits have been accumulated
(ie, in accept_perms after the for loop).
the priority route was chosen because it is a little smaller and
scales better if we get new Node types we have to deal with
(eg. planned complain node).
BugLink: https://gitlab.com/apparmor/apparmor/-/issues/513
Fixes: 1ebd99115 ("parser: change priority so that it accumulates based on permissions")
Signed-off-by: John Johansen <john.johansen@canonical.com>
Specifying norelatime should set the corresponding MS_RELATIME flag clear
bit. Instead, it ORed in MS_NORELATIME, which expands to 0. Properly set
the clear bit by using MS_RELATIME.
Fixes: c9e31b7f "Add mount rules"
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
... and drop rules that are part of abstractions/gtk
Note that abstractions/gtk contains more than the rules dropped here,
which means it effectively extends the permissions granted by
abstractions/gnome.
Idea by darix.
Add AA profile for ssh-keyscan. This profile has been tested on the latest plucky `ssh-agent` version `1:9.9p1-3ubuntu3`. In particular, this has been tested using the tests upstream and the archive in:
* `openssh-tests`
This is linked to the discussion in https://gitlab.com/apparmor/apparmor/-/merge_requests/1502, to add the profile deps of ssh-agent in as well.
Signed-off-by: Evan Caville <evan.caville@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1597
Approved-by: Ryan Lee <rlee287@yahoo.com>
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
fuse_overlayfs requires noatime, but we should also allow more flags than
just that to preempt future breakage from flags not included in the rules.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
This helps to limit the amount of rules skipped in the utils tests
(because the utils don't support the `unsafe` keyword)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1671
Approved-by: Maxime Bélair <maxime.belair@canonical.com>
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Approved-by: Ryan Lee <rlee287@yahoo.com>
Merged-by: Ryan Lee <rlee287@yahoo.com>
The documentation was wrong about how options=(list) and options in (list) are combined
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1674
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
The lack of a space after $testtype is a syntax error and was causing the
equality tests on Ubuntu Xenial to be silently skipped and marked PASS.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1670
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
Add basic support for the priority rules prefix. This patch does not\
allow the utils to set or suggest priorities. It allows parsing and\
retaining of the priority prefix if it already exists on rules and\
checking if it's in the supported range.
Note that this MR is supposed to replace WIP MR https://gitlab.com/apparmor/apparmor/-/merge_requests/1531
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1636
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
