It doesn't seem to need a lot of rules, and I've tried running upstream test suite with this profile and it passed.
Signed-off-by: Allen Huang <allen.huang@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1660
Approved-by: Maxime Bélair <maxime.belair@canonical.com>
Merged-by: Maxime Bélair <maxime.belair@canonical.com>
For now, also use a complain mode flag like with Xorg. However, it may be
possible for complain mode to be dropped from both in the future,
tightening confinement (especially since Xorg.wrap is setuid). A
complain-mode profile can still be useful for Xorg.wrap by giving it a
separate label.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
Add AA profile for ghostscript. This profile has been tested on the latest plucky gs version 10.05.0dfsg1-0ubuntu1 while the latest upstream version is 10.05.0. This profile limits file access (read and write) to specific file extensions, printer devices in /dev and directories in /tmp.
The profile has been tested against the regression test suite we use in Ubuntu and manually. Testing against devices has been performed in a limited fashion as I only have access to one usb printer.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1590
Approved-by: Ryan Lee <rlee287@yahoo.com>
Merged-by: Maxime Bélair <maxime.belair@canonical.com>
Add AA profile for `nslookup`. This profile has been tested on the latest plucky `nslookup` version `9.20.4-3ubuntu1` (ultimately part of `dnsutils`). Functionality has been exercised as much as possible, including basic record lookups, querying specific DNS servers, performing reverse DNS lookups, querying a CNAME, querying an MX record, querying a txt record, querying a DNSSEC-related record, performing IPv4 & IPv6 lookups, and overriding to use a custom resolver. These tests were performed through command parsing and the interactive terminal mode. AFAIK, upstream does not have a test suite available for `nslookup`
Signed-off-by: john-breton <john.breton@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1619
Approved-by: Ryan Lee <rlee287@yahoo.com>
Approved-by: Maxime Bélair <maxime.belair@canonical.com>
Merged-by: Maxime Bélair <maxime.belair@canonical.com>
- Tested with different flags manually
- apparmor.d also have a profile for `hostname` which includes `<abstractions/consoles>` but was not needed while testing for plucky
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1650
Approved-by: Maxime Bélair <maxime.belair@canonical.com>
Approved-by: Ryan Lee <rlee287@yahoo.com>
Merged-by: Maxime Bélair <maxime.belair@canonical.com>
Add AA profile for /usr/bin/locale. This profile has been tested on the latest plucky version of locale (Ubuntu GLIBC 2.41-6-ubuntu1). This profile prevents write access to any file, limits read access to all files necessary for locale to work and limits execution of any other file other than the compressors (gzip/bzip2), which are also limited by a specific subprofile..
The profile has been tested manually.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1646
Approved-by: Ryan Lee <rlee287@yahoo.com>
Merged-by: Maxime Bélair <maxime.belair@canonical.com>
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>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1683
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Follow up to MR !1637
`make check-parser` in profiles now verifies that all profiles allow at
least a read access to their attachment path.
This is done with test_profile.py, more robust and therefore replacing
test_profile.sh.
Additionally, fix the permission of 3 profiles, that were not detected by
!1637 due to a bug in a regex
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1657
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: John Johansen <john@jjmx.net>
`make check-parser` in profiles now verifies that all profiles allow at
least a read access to their attachment path.
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
test_profile.sh contained some bash-specific code and a bug in a regex
that failed to flag some profiles where read access to their attachment
path was not allowed.
Replace it with a Python script, more robust and maintenable.
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
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.
