commit 1fa45b7c1 ("parser: dfa minimization prepare for extended
permissions") removed implicit filtering of explicit denies in the
minimization pass (the information was ignored in building the set of
final accept states).
The filtering of explicit denies reduces the size of the produced
dfa. Since we need to be smarter about when explicit denies are
kept (eg. during complain mode), and most dfas are limited to 65k
states we currently need to filter explicit deny perms by default.
To compensate commit 2737cb2c2 ("parser: minimization - remove
unnecessary second minimization pass") moved the
apply_and_clear_deny() to before minimization. However its check to
apply removal denials before minimization is broken. Remove minimization
triggering apply_and_clear_deny() and just set the FILTER_DENY flag
by default, until we have better selection of rules/conditions where
explicit deny information should be carried through to the backend.
Fixes: 2737cb2c2 ("parser: minimization - remove unnecessary second minimization pass")
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1397
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: John Johansen <john@jjmx.net>
There is an integer overflow when comparing priorities when cmp is
used because it uses subtraction to find lessthan, equal, and greater
than in one operation.
But INT_MAX and INT_MIN are being used by priorities and this results
in INT_MAX - INT_MIN and INT_MIN - INT_MAX which are both overflows
causing an incorrect comparison result and selection of the wrong
rule permission.
Closes: https://gitlab.com/apparmor/apparmor/-/issues/452
Fixes: e3fca60d1 ("parser: add the ability to specify a priority prefix to rules")
Signed-off-by: John Johansen <john.johansen@canonical.com>
Closes#452
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1396
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: John Johansen <john@jjmx.net>
There is an integer overflow when comparing priorities when cmp is
used because it uses subtraction to find lessthan, equal, and greater
than in one operation.
But INT_MAX and INT_MIN are being used by priorities and this results
in INT_MAX - INT_MIN and INT_MIN - INT_MAX which are both overflows
causing an incorrect comparison result and selection of the wrong
rule permission.
Closes: https://gitlab.com/apparmor/apparmor/-/issues/452
Fixes: e3fca60d1 ("parser: add the ability to specify a priority prefix to rules")
Signed-off-by: John Johansen <john.johansen@canonical.com>
commit 1fa45b7c1 ("parser: dfa minimization prepare for extended
permissions") removed implicit filtering of explicit denies in the
minimization pass (the information was ignored in building the set of
final accept states).
The filtering of explicit denies reduces the size of the produced
dfa. Since we need to be smarter about when explicit denies are
kept (eg. during complain mode), and most dfas are limited to 65k
states we currently need to filter explicit deny perms by default.
To compensate commit 2737cb2c2 ("parser: minimization - remove
unnecessary second minimization pass") moved the
apply_and_clear_deny() to before minimization. However its check to
apply removal denials before minimization is broken. Remove minimization
triggering apply_and_clear_deny() and just set the FILTER_DENY flag
by default, until we have better selection of rules/conditions where
explicit deny information should be carried through to the backend.
Fixes: 2737cb2c2 ("parser: minimization - remove unnecessary second minimization pass")
Signed-off-by: John Johansen <john.johansen@canonical.com>
As per https://docs.gitlab.com/ee/ci/pipelines/compute_minutes.html#gitlab-hosted-runner-cost-factors, GitLab CI computes minutes as wall clock time per stage * a constant cost factor derived from the runner type, so using parallelism in `make -j $(nproc)` will reduce the time it takes for GitLab CI to complete without increasing usage of GitLab CI minutes.
When investigating this, I also found out that the test stages needlessly rebuilt large parts of the C code base due to mtimes not being preserved when artifacts are restored from the build stage. Adding `make --touch` updates the mtimes so that the subsequent tests do not need to rebuild binaries needlessly.
The combined changes in this MR reduce the CI time from 13 minutes and 57 seconds (cb0f84e101 of `master`, https://gitlab.com/rlee287/apparmor/-/pipelines/1501017669 on my own fork without Coverity) to 12 minutes and 49 seconds (https://gitlab.com/rlee287/apparmor/-/pipelines/1502723883). This comparison omits the `make -j $(nproc)` addition to cov-build since I do not have a way of testing its effectiveness.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1387
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
confirm_and_abort() is unused (note that a function with the same name
exists in ui.py and is used there)
Also delete the now-unused delete_profile() - luckily it was never used,
because it would also have deleted profiles that were "just" modified.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1388
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Christian Boltz <apparmor@cboltz.de>
confirm_and_abort() is unused (note that a function with the same name
exists in ui.py and is used there)
Also delete the now-unused delete_profile() - luckily it was never used,
because it would also have deleted profiles that were "just" modified.
Systemd's PrivateTmp= in transmission service is causing mount namespaces to be used leading to disconnected paths
[395201.414562] audit: type=1400 audit(1727277774.392:573): apparmor="ALLOWED" operation="sendmsg" class="file" info="Failed name lookup - disconnected path" error=-13 profile="transmission-daemon" name="run/systemd/notify" pid=193060 comm="transmission-da" requested_mask="w" denied_mask="w" fsuid=114 ouid=0
Fixes: https://bugs.launchpad.net/bugs/2083548
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1355
Approved-by: Ryan Lee <rlee287@yahoo.com>
Merged-by: Steve Beattie <steve+gitlab@nxnw.org>
... that are generated during `make`
I propose this patch for 3.x..master.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1374
Approved-by: Ryan Lee <rlee287@yahoo.com>
Approved-by: Steve Beattie <steve+gitlab@nxnw.org>
Merged-by: Steve Beattie <steve+gitlab@nxnw.org>
The artifact restoration step does not preserve mtime, resulting in source files newer than built files, resulting in a needless rebuild of everything before actually running the tests.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
When a log like system.journal is passed on to aa-genprof, for
example, the user receives a TypeError exception: in method
'parse_record', argument 1 of type 'char *'
This patch catches that exception and displays a more meaningful
message.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/436
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Closes#436
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1354
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: John Johansen <john@jjmx.net>
This fixes format string specification warnings that are emitted when DEBUG=1 is set. As for %s when the pointer is null: even if gcc prints (null) this is still undefined behavior, so we should do this explicitly.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1382
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Ryan Lee <rlee287@yahoo.com>
When a log like system.journal is passed on to aa-genprof, for
example, the user receives a TypeError exception: in method
'parse_record', argument 1 of type 'char *'
This patch catches that exception and displays a more meaningful
message.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/436
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
As for %s when the pointer is null: even if gcc prints (null) this is still undefined behavior, so we should do this explicitly
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
Limit access to \*.status files located in /var/lib/libvirt/dnsmasq/ as opposed to every file in the same directory.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1379
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: John Johansen <john@jjmx.net>
These are small changes to the man pages, with the most important one being updating some function signatures to be consistent with apparmor.h.
We should put together a man page for aalogparse functions too, but I'm submitting this MR first to get the smaller changes in faster.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1378
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Ryan Lee <rlee287@yahoo.com>
The timeout parameter for subprocess.Popen.communicate has been available since Python 3.3. Given the fragility of SIGALRM based mechanisms, there's no reason to reimplement our own timeout instead of using the built-in one.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1377
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
As I have read multiple MR mentioning the `nameservice-strict`. Therefore, I thought it would make sense to directly import it here.
To give some context, this abstraction is probably the most commonly included abstraction (after `base`). In `apparmor.d`, it is used by over 700 profiles (only counting direct import). Therefore, adding new rules can have an important impact over a lot of profiles.
Note: the abstraction is a direct import from https://gitlab.com/roddhjav/apparmor.d. The license is the same, I obviously kept Morfikov copyright line. However, I am not sure either or not the SPDX identifier can be used here.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1368
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Approved-by: Christian Boltz <apparmor@cboltz.de>
Approved-by: Ryan Lee <rlee287@yahoo.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
Add support for hostname resolution via libnss-libvirt. This change has been tested against the latest oracular version 10.6.0-1ubuntu3.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1362
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
Do an identifier rename combined with preprocessor directives and SWIG directives to allow the header to be included in C++ while keeping backwards compatibility to the extent possible.
Closes: #439
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
Closes#439
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1342
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Systemd's PrivateTmp= in transmission service is causing mount namespaces to be used leading to disconnected paths
[395201.414562] audit: type=1400 audit(1727277774.392:573): apparmor="ALLOWED" operation="sendmsg" class="file" info="Failed name lookup - disconnected path" error=-13 profile="transmission-daemon" name="run/systemd/notify" pid=193060 comm="transmission-da" requested_mask="w" denied_mask="w" fsuid=114 ouid=0
Fixes: https://bugs.launchpad.net/bugs/2083548
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
This requires adding some `.get()` guards at one place, but should
otherwise be a boring change.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1347
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
This is one of those functions that never worked anyways, because it
modified the passed-in label in place. Moreover, it is a low-level
interface that requires its callers to manually construct a binary query.
As such, it would be better not to expose it and to add wrappers like
aa_query_file_path for the other query classes if that functionality is
needed later.
The removal of this function from the bindings was dropped from !1337 because it exposed functionality that was not present in wrappers around aa_query_label. However, upon further discussion, we decided that it'd be better to remove it now and add other wrappers to libapparmor itself if the functionality provided by the existing wrappers became insufficient.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1352
Approved-by: John Johansen <john@jjmx.net>
Merged-by: Ryan Lee <rlee287@yahoo.com>
This is one of those functions that never worked anyways, because it
modified the passed-in label in place. Moreover, it is a low-level
interface that requires its callers to manually construct a binary query.
As such, it would be better not to expose it and to add wrappers like
aa_query_file_path for the other query classes if that functionality is
needed later.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
This pipeline only makes sense to run in the upstream project where
the coverity variables are defined, so they currently fail in forks.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1351
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
This pipeline only makes sense to run in the upstream project where
the coverity variables are defined, so they currently fail in forks.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
