This is a set of small tweaks to the merged rule interface window
- don't specify, font or size when setting bold
- improve message around unknown profiles
- add a custom message for snap profile
- output tktheme missing message, to help users identify they can improve the interface when started manually
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1529
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Snap policy is a special case of the unknown profile. Give the user
a slightly better message for these messages.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The current notification can be confusing, in that it can present a
profile followed by a list of rules that can't be selected.
Explictly state that the Unknown profile can't be modified so the user
has some indication that not being able to select the shown rules is
expected.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Instead of specifying the font type and size, which will not work for
all display configuration, use the the default BOLD font that tkinter
supplies.
Signed-off-by: John Johansen <john.johansen@canonical.com>
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>
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.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Some classes don't support modifiers like audit and deny. Only rlimit
has been checking for the allow keyword, but the others shouldn't
support it as well. Since they all do the same check, refactor them
into a method from BaseRule in case more modifiers are added.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
- set filetype, instead of syntax, in vim modelines
- replace filetype of subdomain with apparmor
- move modelines in the first or last five lines of each file so that
vim can recognize them
Unconfined delegates access to open file descriptors. Therefore when running a confined binary from unconfined, it will work even when the attachment path is not read-allowed.
However, as soon as these confined binaries are run from another confined process, this delegation is not permitted anymore and the program breaks.
This has been the cause of several bugs such as https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/2107455 or https://github.com/canonical/snapd/pull/15181 .
This MR makes sure every confining AppArmor profiles explicitly allow (at least) read access to their attachment path.
This Merge request:
- Introduce `test_profile.sh`, a helper script that ensures confining AppArmor profiles explicitly allow (at least) read access to their attachment path.
- Modifies a lot of profiles so that all profiles have r/mr access to their attachment path
- Extends `make check` to automatically ensure all AppArmor profile grant explicit read access to their attachment path, preventing future omissions.
- Modifies apparmor_parser to show attachment in --debug output
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1637
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
This enables us to exercise the front perms parse logic in the utils rule parsing through the simple tests as well
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
... and adjust the tools to raise an exception if such a rule is found.
While this is not nice, it's better than the previous behaviour where
only the last 'fstype' was kept, and the others were lost when writing
the rule.
... and adjust the tools to raise an exception if such a rule is found.
While this is not nice, it's better than the previous behaviour where
only the last 'options' was kept, and the others were lost when writing
the rule.
Note: If multiple fstype= or options= are given, this is not detected as
an error (to keep the regex simpler). When writing back such a rule,
only one fstype and options will "survive".
Adjust the exclude list in test-parser-simple-tests.py accordingly:
- several valid mount rules no longer fail
- two invalid mount rules which so far accidentally raised an exception
because of the fstype/options order no longer raise this exception
(conflicting mount options, which are the real reason why these rules
are invalid, are not detected in the tools)
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/501
The tools are wrong in parsing the detached mount test.
Until that can be fixed, mark the tools as wrong.
Signed-off-by: John Johansen <john.johansen@canonical.com>
--configdir is meant for testing and should override all other configs,
instead of being combined with them. Config combination causes aa-notify
test failures if e.g. the user-local config sets filtering options.
Also supply a notify.conf file for exclusive use during testing.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1610
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: John Johansen <john@jjmx.net>
--configdir is meant for testing and should override all other configs,
instead of being combined with them. Config combination causes aa-notify
test failures if e.g. the user-local config sets filtering options.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
Both the unconfined profile and unprivileged_userns are part of the
default notify.conf, so the default fallback when no configurations are
present should also match this default.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
Fixes https://bugs.launchpad.net/apparmor/+bug/2106033
Improve the validation of AARE file paths by introducing a new regex
that supports paths starting with '{' (e.g. '{/,/org/freedesktop/DBus}').
These paths are notably used in snap.lxd.* profiles.
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
If the following scenario was present in a profile, cleanprof
would fail with a RecursionError exception (maximum recursion
depth exceeded)
/parent { }
/parent///child { }
This occured because in aa.py, in the write_piece function, the
wrong depth was being passed, along with a wrong hat. The
formatting of the spaces was also incorrect.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1605
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
If the following scenario was present in a profile, cleanprof
would fail with a RecursionError exception (maximum recursion
depth exceeded)
/parent { }
/parent///child { }
This occured because in aa.py, in the write_piece function, the
wrong depth was being passed, along with a wrong hat. The
formatting of the spaces was also incorrect.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Profiles that are defined like below did not have the parent attribute
set in profile storage:
/parent///child {}
The condition on which child profiles were written was also changed so
they are not removed from the profile if /parent does not exist.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Commit c9d41a3ebb introduced a regression on profile header
generation.
This commit removes the name parameter from the get_header function
since the ProfileStorage should already contain all the information
required to generate the header for profiles and hats. The tests
needed to be updated as well to make sure the ProfileStorage object
contained the information needed by the get_header method.
Fixes: c9d41a3ebb ("utils: fix profile and hat header generation")
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
The header was being generated incorrectly in 2 cases:
When the profile/hat contained the parent profile in its name, as in
profile firefox//dash {
hat ^firefox//dash {
and in the unit tests, the child profile or hat was being named as the
parent profile. This was not caught by the general case because the
code has not yet been fully adapted to handle multiple nested child
profiles.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/493
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
When doing testing via LXD VMs and in particular when using "lxc exec" to run
commands in the VM, there is no controlling tty and so the output of last is
missing this column of data. Instead try even harder to parse the timestamp from
the output of "last".
Signed-off-by: Alex Murray <alex.murray@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1582
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
When doing testing via LXD VMs and in particular when using "lxc exec" to run
commands in the VM, there is no controlling tty and so the output of last is
missing this column of data. Instead try even harder to parse the timestamp from
the output of "last".
Signed-off-by: Alex Murray <alex.murray@canonical.com>
* Make glob_pattern more readable
- replace filename and variable regex parts with RE_PROFILE_PATH_OR_VAR
- split to multiline string
* Move `[\w-]+` into inner match group by removing/moving the ')' after the empty source.
* Prepare source_fileglob_pattern and dest_fileglob_pattern to be customizable by moving adding the closing ')))' into each of them.
* Allow empty source and any word only in mount source
See the individual commits for details.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1574
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
When running aa-disable and then aa-enforce passing the binary path as
the argument, aa-enforce fails to enforce the profile with the error:
$ sudo aa-disable /home/foo/test
skipping disabled profile test
Profile for /home/foo/test not found, skipping
According to the man page for aa-enforce, it should work for disabled
profiles.
Note that this does not happen when passing the profile directly to
the tools, so there's a workaround for this issue:
$ sudo /aa-enforce /etc/apparmor.d/test
Setting /etc/apparmor.d/test to enforce mode.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
... by removing/moving the ')' after the empty source.
Also prepare source_fileglob_pattern and dest_fileglob_pattern to be
customizable by moving adding the closing ')))' into each of them.
Add quotes if a mount source or mountpoint includes whitespace.
Also explicitely handle empty mount source (known from
1f33fc9b29)
As usual, some tests can't hurt ;-)
First expand nested `(...)` in glob_pattern. This duplicates a few bytes, but makes the regex easier to read.
With that done, allow `-` in glob_pattern.
One of the possible matches in glob_pattern was `\w+` which matched for example `none`.
However, it doesn't match `revokefs-fuse` because of the `-`. Therefore change `\w+` to [\w-]+.
While on it, add two more tests - one for `none` with some options, and one with `revokefs-fuse`.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1565
Approved-by: Maxime Bélair <maxime.belair@canonical.com>
Approved-by: Ryan Lee <rlee287@yahoo.com>
Merged-by: Christian Boltz <apparmor@cboltz.de>
