The previous code would concatenate all of them together without spacing.
While dump_flags and the corresponding operator<< function aren't currently used,
this will help for when dump_flags is used to debug parser problems.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1465
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
TL;DR: Replace `aa[profile][hat]` with `active_profiles['profile//hat']` as a preparation to get rid of `aa`'s limits, especially to enable handling nested childs.
Since this is an extremely shortened summary, I recommend to check the individual commits for a readable and understandable diff and more details.
Note that this MR is "just" a preparation - nested childs are not supported yet. Also, `include` still uses the old structure. Both will be separate MRs - this one is already big enough ;-)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1360
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
Note that the old code assigned dummy_prof to aa[profile][hat] and
active_profiles[profile] (= the main/parent profile) - which is
diffferent when testing a log for a child profile.
aa[profile][hat] was the wrong place - but since we used exactly that
again when checking for added exec rules, this error was hidden.
Now that the test is switched to using active_profiles, only check the
main profile for exec rules added by ask_exec(). (This will need to be
adjusted when we add a test for exec rules/events in nested childs, but
not earlier ;-)
This is mostly a search-and-replace patch.
In most cases, that means replacing `aa[profile][hat]` with
`active_profiles[full_profile]`.
In cases where the main/parent profile is meant, switch from
`aa[profile][profile]` to `active_profiles[profile]`.
Checks like `p in apparmor.aa` that check if a (main) profile exists
become `active_profiles.profile_exists(p)`.
write_profile() gets changed to loop over
`active_profiles.get_profile_and_childs()` which makes the code simpler.
`split_to_merged(aa)` becomes just `active_profiles`.
The only change that is not search-and-replace style is in
write_piece(). It expects a dict (not a ProfileList), therefore adjust
serialize_profile() so that it always hands over a dict.
This also changes the internal structure - instead of the nested dict
original_aa[profile][hat], we now have a ProfileList original_profiles[profile//hat].
Drop `comment.replace('\\n', '\n')` because that doesn't make sense and
doesn't change anything - not even a comment that contains the literal
string '\n' (backslash + letter n).
Besides that, get rid of the 'string' variable and store everything in
'data'.
... including just-created child profiles and hats.
Also ensure that serialize_profile() doesn't print them out as child
profiles AND external hats.
This commit includes a bugfix for a rare corner case:
Since create_new_profile() can return more than one profile if the
program has required_hats, add all of them to active_profiles.
(aa only got the expected profile added, but not the required_hats.)
... and make it non-optional
Note that read_profile() in aa.py skips child profiles and hats,
therefore active_profiles for now only contains the main profiles.
While the mount syscall documentation disallows this, the kernel silently
ignores make-* flags when doing a remount, and real applications were
passing this conflicting set of flags. Because changing the kernel to
reject this combination would break userspace, we should allow them
instead.
For an example: see https://bugs.launchpad.net/apparmor/+bug/2091424.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
The previous code would concatenate all of them together without spacing.
While dump_flags and the corresponding operator<< function aren't currently used,
this will help for when dump_flags is used to debug parser problems.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
From LP: #2085377, when using ip netns to torrent traffic through a
VPN, attach_disconnected is needed by the policy because ip netns sets
up a mount namespace.
Fixes: https://bugs.launchpad.net/bugs/2085377
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Changes to Python SWIG bindings that are breaking changes but that fix bindings that were previously unusable.
This MR also depends on !1334 and !1337 being merged first, though ~~I can rebase this one if necesssary~~ this MR has now been rebased after those two were merged.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1338
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
As per !1462 it turns out that the swap regression test on btrfs also needs special casing in order to work properly. This is an analogous patch to check for btrfs.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1463
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
Swap on ZFS is *weird*. Getting it working needs some special casing, see e.g. https://askubuntu.com/questions/1198903/can-not-use-swap-file-on-zfs-files-with-holes
Currently, the swap regression test fails on my system (with /tmp in zfs):
```bash
tests/regression/apparmor ❯ ./swap.sh
Error: swap failed. Test 'SWAPON (unconfined)' was expected to 'pass'. Reason for failure 'FAIL: swapon /tmp/sdtest.872368-19048-kN4FN2/swapfile failed - Invalid argument'
Error: swap failed. Test 'SWAPOFF (unconfined)' was expected to 'pass'. Reason for failure 'FAIL: swapoff /tmp/sdtest.872368-19048-kN4FN2/swapfile failed - Invalid argument'
swapon: /tmp/sdtest.872368-19048-kN4FN2/swapfile: skipping - it appears to have holes.
Fatal Error (swap): Unexpected shell error. Run with -x to debug
```
However, just doing a file mount does make the test work on zfs, similar to how it is done with tmpfs. This means we don't need any special-casing for zfs beyond what is already there for working around (similar) tmpfs limitations.
Also, while researching this, it is possible a similar patch is needed for btrfs, but i currently don't have an easy way to test that.
This is non-breaking for anyone *not* using zfs, and it is currently broken with zfs anyways.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1462
Approved-by: Ryan Lee <rlee287@yahoo.com>
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
previously, this check would fail if the setuptools version would contain non-integers.
On my system, that is the case: `setuptools.__version__` is `'75.1.0.post0'`
I believe it is entirely fair to just check the relevant bits and refuse to continue if those can not be checked properly.
Having some extra slug on the version should not immediately cause issues (e.g. the `post0` here, or slugs like `beta`, `alpha` and the likes).
Probably only very few systems are running setuptools with weird version info, but supporting this is a simple one-line change i figured i might as well MR.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1460
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
previously, this check would fail if the setuptools version would contain non-integers.
On my system, that is the case: `setuptools.__version__` is `'75.1.0.post0'`
I believe it is entirely fair to just check the relevant bits and refuse to continue if those can not be checked properly.
But haviong something extra on the version should not immediately cause issues (e.g. the `post0` here, or slugs like `beta`, `alpha` and the likes).
Probably only very few systems are running setuptools with weird version info, but supporting this doesn't cost much, i believe.
This is neeed for "inherit owner = yes" in smb.conf.
From man smb.conf:
inherit owner (S)
The ownership of new files and directories is normally governed by
effective uid of the connected user. This option allows the Samba
administrator to specify that the ownership for new files and
directories should be controlled by the ownership of the parent
directory.
Fixes: https://bugzilla.suse.com/show_bug.cgi?id=1234327
I propose this fix for 3.x, 4.x and master.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1456
Approved-by: Ryan Lee <rlee287@yahoo.com>
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
MS_SYNC is a flag for msync(2) while MS_SYNCHRONOUS is a flag for mount(2).
The header used to define MS_SYNC but IMO this is confusing since that's an
unrelated flag.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1458
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
MS_SYNC is a flag for msync(2) while MS_SYNCHRONOUS is a flag for mount(2).
The header used to define MS_SYNC but IMO this is confusing since that's an
unrelated flag.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
