Commit graph

5468 commits

Author SHA1 Message Date
Christian Boltz
7629d18e04
convert RE_PROFILE_VARIABLE to named match groups
(no behaviour change, just easier to use)
2020-05-22 00:51:18 +02:00
John Johansen
f891dad070 Merge vulkan: fix device and driver enumeration
vkcube test application produces these DENIED messages on current Debian Sid:

```
type=AVC msg=audit(1589815066.509:868): apparmor="DENIED"
operation="open" profile="/usr/bin/vkcube"
name="/sys/devices/pci0000:00/0000:00:02.0/drm/card0/gt_min_freq_mhz"
pid=32553 comm="vkcube" requested_mask="r" denied_mask="r" fsuid=1000
ouid=0
```

```
type=AVC msg=audit(1590082079.377:1155): apparmor="DENIED"
operation="open" profile="/usr/bin/vkcube"
name="/sys/devices/pci0000:00/0000:00:02.0/drm/card0/gt_max_freq_mhz"
pid=51348 comm="vkcube" requested_mask="r" denied_mask="r" fsuid=1000
ouid=0
```

```
type=AVC msg=audit(1590082107.057:1249): apparmor="DENIED"
operation="open" profile="/usr/bin/vkcube"
name="/sys/devices/pci0000:00/0000:00:02.0/drm/card0/metrics/" pid=51459
comm="vkcube" requested_mask="r" denied_mask="r" fsuid=1000 ouid=0
```

```
type=AVC msg=audit(1590082519.937:1408): apparmor="DENIED"
operation="open" profile="/usr/bin/vkcube"
name="/sys/devices/pci0000:00/0000:00:02.0/drm/card0/metrics/403d8832-1a27-4aa6-a64e-f5389ce7b212/id"
pid=51960 comm="vkcube" requested_mask="r" denied_mask="r" fsuid=1000
ouid=0
```

```
type=AVC msg=audit(1590082842.193:1829): apparmor="DENIED"
operation="open" profile="/usr/bin/vkcube"
name="/etc/glvnd/egl_vendor.d/" pid=53000 comm="vkcube"
requested_mask="r" denied_mask="r" fsuid=1000 ouid=0
```

```
type=AVC msg=audit(1590082842.193:1830): apparmor="DENIED"
operation="open" profile="/usr/bin/vkcube"
name="/usr/share/glvnd/egl_vendor.d/" pid=53000 comm="vkcube"
requested_mask="r" denied_mask="r" fsuid=1000 ouid=0
```

Add appropriate rules to fix these enumerations.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/543
Acked-by: John Johansen <john.johansen@canonical.com>
2020-05-21 19:53:20 +00:00
John Johansen
51c5251d8c Merge binutils: add -Wformat to fix compiler warning
The warning is/was:

cc1: warning: '-Wformat-security' ignored without '-Wformat' \[-Wformat-security\]

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/542
Acked-by: John Johansen <john.johansen@canonical.com>
2020-05-21 19:50:18 +00:00
John Johansen
1fccf0458f Merge aa-status: append \n to --json and --pretty-json output
... so that - if someone calls aa-status from the commandline - the next prompt doesn't get displayed in the same line as the json output.

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/541
Acked-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
2020-05-21 19:48:14 +00:00
John Johansen
7daed8eb79 Merge Error out on conflicting aa-exec parameters
If multiple -p/--profile or -n/--namespace parameters are given to aa-exec, they are handled on a "last one wins" base.

This probably isn't expected behaviour, and erroring out in case of conflicting parameters probably makes more sense.

PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/540
Acked-by: John Johansen <john.johansen@canonical.com>
2020-05-21 19:11:00 +00:00
Vincas Dargis
21bbfc0652 vulkan: fix device and driver enumeration
vkcube test application produces these DENIED messages on current Debian
Sid:
```
type=AVC msg=audit(1589815066.509:868): apparmor="DENIED"
operation="open" profile="/usr/bin/vkcube"
name="/sys/devices/pci0000:00/0000:00:02.0/drm/card0/gt_min_freq_mhz"
pid=32553 comm="vkcube" requested_mask="r" denied_mask="r" fsuid=1000
ouid=0
```

```
type=AVC msg=audit(1590082079.377:1155): apparmor="DENIED"
operation="open" profile="/usr/bin/vkcube"
name="/sys/devices/pci0000:00/0000:00:02.0/drm/card0/gt_max_freq_mhz"
pid=51348 comm="vkcube" requested_mask="r" denied_mask="r" fsuid=1000
ouid=0
```

```
type=AVC msg=audit(1590082107.057:1249): apparmor="DENIED"
operation="open" profile="/usr/bin/vkcube"
name="/sys/devices/pci0000:00/0000:00:02.0/drm/card0/metrics/" pid=51459
comm="vkcube" requested_mask="r" denied_mask="r" fsuid=1000 ouid=0
```

```
type=AVC msg=audit(1590082519.937:1408): apparmor="DENIED"
operation="open" profile="/usr/bin/vkcube"
name="/sys/devices/pci0000:00/0000:00:02.0/drm/card0/metrics/403d8832-1a27-4aa6-a64e-f5389ce7b212/id"
pid=51960 comm="vkcube" requested_mask="r" denied_mask="r" fsuid=1000
ouid=0
```

```
type=AVC msg=audit(1590082842.193:1829): apparmor="DENIED"
operation="open" profile="/usr/bin/vkcube"
name="/etc/glvnd/egl_vendor.d/" pid=53000 comm="vkcube"
requested_mask="r" denied_mask="r" fsuid=1000 ouid=0
```

```
type=AVC msg=audit(1590082842.193:1830): apparmor="DENIED"
operation="open" profile="/usr/bin/vkcube"
name="/usr/share/glvnd/egl_vendor.d/" pid=53000 comm="vkcube"
requested_mask="r" denied_mask="r" fsuid=1000 ouid=0

```

Add appropriate rules to fix these enumerations.
2020-05-21 21:00:13 +03:00
Christian Boltz
91781aecde
binutils: add -Wformat to fix compiler warning
The warning is/was:

cc1: warning: '-Wformat-security' ignored without '-Wformat' [-Wformat-security]
2020-05-21 17:54:24 +02:00
Christian Boltz
0b8839932a
aa-status: append \n to --json and --pretty-json output
... so that - if someone calls aa-status from the commandline - the next
prompt doesn't get displayed in the same line as the json output.
2020-05-21 17:52:23 +02:00
Christian Boltz
02df146f31
Error out on conflicting aa-exec parameters
If multiple -p/--profile or -n/--namespace parameters are given to
aa-exec, they are handled on a "last one wins" base.

This probably isn't expected behaviour, and erroring out in case of
conflicting parameters probably makes more sense.
2020-05-21 17:04:40 +02:00
John Johansen
4b79a58c25 Merge Switch handling of include rules from filelist to IncludeRule
This patch series moves include rule handling away from the `filelist` hasher to using IncludeRule and IncludeRuleset. This means that only variable handling is left in `filelist`.

As usual, check the individual commits for details.

PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/537
Acked-by: John Johansen <john.johansen@canonical.com>
2020-05-21 08:52:49 +00:00
Christian Boltz
1d2e710b26
parse_profile_data(): drop old code for parsing include rules
With this, all include rules (with and without 'if exists') get handled
by the code that handles IncludeRule.
2020-05-20 18:57:54 +02:00
Christian Boltz
cefc64522d
Drop now unused 'include' from ProfileStorage and ProfileList 2020-05-20 18:57:54 +02:00
Christian Boltz
60e8c1ff41
No longer write to filelist['include] or profile['include']
All code is migrated to IncludeRuleset / 'inc_ie'], so there's no point
in filling 'include' anymore.
2020-05-20 18:57:54 +02:00
Christian Boltz
5cfab9b81b
serialize_profile(): write preamble without checking filelist
filelist is getting emptier and emptier, and checking against a
nearly-empty filelist would result in not writing the preamble.
2020-05-20 18:57:54 +02:00
Christian Boltz
f0fa69a010
Convert compare_profiles() to use inc_ie / IncludeRule 2020-05-20 18:57:54 +02:00
Christian Boltz
706138ed67
Convert remove_duplicate_rules() to use inc_ie / IncludeRule
Besides switching to 'inc_ie', also drop the old code that removed
duplicate 'include' rules.
2020-05-20 18:57:54 +02:00
Christian Boltz
c8fa3dec0a
aa-cleanprof: don't check profile includes against preamble includes
remove_duplicate_rules() composed the 'includes' list from a) the
include rules in the profile and b) the file_includes (preamble
includes), and then checked the profile includes against this
combination of profile and preamble includes.

Checking profile includes against preamble includes is wrong, and the
only reason why this went unnoticed for years is that preamble include
files (like tunables) won't work inside a profile and therefore never
appear there.

In theory this might be a backport candidate, even if this shouldn't
cause a real-world bug.
2020-05-20 18:57:54 +02:00
Christian Boltz
6161641bcf
Convert match_includes() to use inc_ie / IncludeRule 2020-05-20 18:57:54 +02:00
Christian Boltz
cfdbcb20cc
add include_list_recursive()
This function returns a list of all includes in a profile and its
included files.

It's split off from is_known_rule() and get_file_perms() which shared
lots of common code to recursively get the include list. These functions
now use include_list_recursive().
2020-05-20 18:57:54 +02:00
Christian Boltz
6f1245ede9
Convert is_known_rule() to use inc_ie / IncludeRule
While on it, also add a missing check to avoid checking the same file
twice.
2020-05-20 18:57:54 +02:00
Christian Boltz
9f1b2f4014
collapse_log(): avoid accidently initializing aa[profile]
... or calling is_known_rule() on events for non-existing hats.

It's the usual hasher() "fun" again - accessing a non-existing element
will create its parent.

In theory this commit might be worth a backport. In practise, it doesn't cause
any visible problem.

However, starting with the next commit, it will cause lots of test errors.

Also add a missing is_known_rule() call for dbus rules, which might have
caused similar hasher() "fun".
2020-05-20 18:57:54 +02:00
Christian Boltz
fa67815698
Convert get_file_perms() to use inc_ie / IncludeRule 2020-05-20 18:54:53 +02:00
Christian Boltz
0cfef21713
IncludeRuleset: add get_all_full_paths()
This function returns a list of full paths of all includes.

Also add some tests.
2020-05-20 18:54:53 +02:00
Christian Boltz
057c916032
parse_profile_data(): call load_include() for IncludeRule
rule_obj.get_full_paths() handles directories and non-existing files
(depending on 'if exists'), so we can simply feed the resulting list
into load_include()
2020-05-20 18:54:53 +02:00
Christian Boltz
f3f597ff0b
IncludeRule: add get_full_paths()
This function returns a list of paths for an include rule. This can be
- a single path if the include file is a file and exists
- a single path if the include file doesn't exist, but doesn't have
  'if exists' (this will cause a 'file not found' error when used)
- a list of paths if the include is a directory
- an empty list if the rule has 'if exists' and the file doesn't exist

Also add some tests for get_full_paths()
2020-05-20 18:54:53 +02:00
Christian Boltz
d799195e0a
move is_skippable_file() from aa.py to common.py
IncludeRuleset needs to import it (next commit), which is impossible
when it lives in aa.py (would cause recursive import loop because aa.py
imports IncludeRuleset)
2020-05-20 18:54:53 +02:00
Christian Boltz
3f0f7154f7
Let aa-cleanprof remove duplicate preamble rules
Technically, this is done in the new function
delete_preamble_duplicates() in ProfileList.

Also add some tests to ensure this works as expected.
2020-05-20 18:54:53 +02:00
Christian Boltz
0cf15d54b6
aa-mergeprof: drop separate code asking for include rules
... because this is now done via IncludeRule, and keeping the separate
code would mean asking twice.

Note that the user interface changes slightly.

The old workflow was

    1 - #include <foo>
    2 - #include <bar>
    3 - #include <baz>
    [select 2 and (A)dd, then get the next question for the remainder]

    1 - #include <foo>
    2 - #include <baz>
    [(A)dd another one, or (I)gnore the remainder]

The new workflow will ask separately for each abstraction, and you can
(A)dd or (I)gnore it. This is probably easier to understand because it's
basically a yes/no question.
2020-05-20 18:52:16 +02:00
Christian Boltz
23af115fa5
store include rules (also) in 'inc_ie' (IncludeRueset)
... and write them (only) from 'inc_ie' (IncludeRuleset), which can
handle both "include" and "include if exists" rules.

This duplicates storage of include rules because 'include' is still used
and needed at various places that work on/with the include rules.

With this, we get removal of duplicate include lines insinde a profile
in aa-cleanprof "for free" - extend cleanprof_test.in to confirm this.
2020-05-20 18:50:20 +02:00
Christian Boltz
7144bd7181 Merge branch 'drop-includes' into 'master'
aa-notify: don't load policy includes

See merge request apparmor/apparmor!538

Acked-by: Christian Boltz <apparmor@cboltz.de>
2020-05-19 17:12:03 +00:00
John Johansen
cc869c901e aa-notify: don't load policy includes
aa-notify does not need to load the policy includes for its current
features, so drop the unneeded overhead.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2020-05-19 02:47:21 -07:00
John Johansen
a7a727b1b0 Merge Make save_profiles() usable for aa-mergeprof
... and replace the aa-mergeprof code that asks about saving the profile with a call to save_profiles().

PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/536
Acked-by: John Johansen <john.johansen@canonical.com>
2020-05-18 08:27:10 +00:00
John Johansen
69eeca80dd Merge aa-mergeprof: simplify internal parameter handover
Instead of stuffing two variables into a list and split them again, hand them over as plain separate variables.

PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/535
Acked-by: John Johansen <john.johansen@canonical.com>
2020-05-18 08:22:26 +00:00
John Johansen
500acf7119 Merge valid_include(): move out unrelated check
alid_include() checks if the given include file exists or is whitelisted in cfg\['settings'\]\['custom_includes'\].

The check if that include file is already part of the profile is unrelated to that. Move it to match_includes() where it fits much better (and drop the now superfluous profile parameter from valid_include())

In theory is_known_rule() should prevent that case from ever happening, but let's restrict this commit to moving the code around and keep this check just to be sure.

While on it, add some documentation to both functions.

PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/534
Acked-by: John Johansen <john.johansen@canonical.com>
2020-05-18 08:19:54 +00:00
John Johansen
c4d1cfa487 Merge split off ask_rule_questions() from ask_the_questions()
Also rename aa.py delete_duplicates() and make ruletypes a parameter.

See the commit messages for details.

This reduces usage of global variables.

The final change is that aa-genprof now asks about preamble rules that are \*Rule classes (currently `abi` and `include if exists`).

PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/532
Acked-by: John Johansen <john.johansen@canonical.com>
2020-05-18 08:16:48 +00:00
Christian Boltz
34d4d9f5af
Make save_profiles() usable for aa-mergeprof
... and replace the aa-mergeprof code that asks about saving the profile
with a call to save_profiles().
2020-05-17 16:50:01 +02:00
Christian Boltz
ed0f2fe505
aa-mergeprof: simplify internal parameter handover
Instead of stuffing two variables into a list and split them again, hand
them over as plain separate variables.
2020-05-14 23:29:31 +02:00
Christian Boltz
7f7fc973f8
valid_include(): move out unrelated check
valid_include() checks if the given include file exists or is
whitelisted in cfg['settings']['custom_includes'].

The check if that include file is already part of the profile is
unrelated to that. Move it to match_includes() where it fits much
better (and drop the now superfluous profile parameter from
valid_include())

In theory is_known_rule() should prevent that case from ever happening,
but let's restrict this commit to moving the code around and keep this
check just to be sure.

While on it, add some documentation to both functions.
2020-05-14 22:48:20 +02:00
Christian Boltz
e0f9b7cb07 Merge branch 'cboltz-fail-verbose' into 'master'
read_profile(): don't fail silently

See merge request apparmor/apparmor!530

Acked-by: Steve Beattie <steve.beattie@canonical.com> for 2.11..master
2020-05-12 19:43:44 +00:00
John Johansen
d1dca1f2d4 Merge ProfileList: add __repr__() with list of filenames
Even if incomplete (it doesn't print the whole ProfileList content), this should make debugging easier.

PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/531
Acked-by: Steve Beattie <steve.beattie@canonical.com>
2020-05-12 07:44:12 +00:00
Christian Boltz
85f2cd4ed4
Let aa-mergeprof ask about more preamble (*Rule) rules
For now, that means aa-mergeprof will ask for `abi` and `include if
exists` rules (currently hardcoded).

This needs storing of `active_profiles` in the Prof object - the
preamble `abi` and `include if exists` rules are stored there.

Since several functions expect an `include` dict, add an empty one to
ProfileList to prevent lots of errors and breakage. It can be removed
again when handling of `include` rules gets moved to IncludeRule.
2020-05-11 23:41:25 +02:00
Christian Boltz
3e0f4e1dba
rename aa.py delete_duplicates() and make ruletypes a parameter
Rename delete_duplicates() to delete_all_duplicates() to make the
function easier grep-able - the *rule classes have delete_duplicates()
which might be confused with the old name.

Also hand over 'ruletypes' as parameter to delete_all_duplicates()
instead of using the global variable.
2020-05-11 23:03:23 +02:00
Christian Boltz
ca693db3b0
split off ask_rule_questions() from ask_the_questions()
ask_the_questions() stays the "main" function, loops over all profiles,
and calls ask_rule_questions() for each profile.

ask_rule_questions() asks the questions for all events in a specific
profile or hat.

This reduces the usage of global variables in ask_rule_questions().
2020-05-11 23:01:06 +02:00
Christian Boltz
e841c866d3
ProfileList: add __repr__() with list of filenames
Even if incomplete (it doesn't print the whole ProfileList content),
this should make debugging easier.
2020-05-11 22:46:29 +02:00
Christian Boltz
af8b9dc5bb
read_profile(): don't fail silently
If a profile file can't be read, print a warning so that the user is
aware of the problem.
2020-05-11 22:23:20 +02:00
Steve Beattie
da2ce3ac67 utils: Use ProfileList to handle aliases
Merge branch 'cboltz-alias' into 'master'

Acked-by: Steve Beattie <steve.beattie@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/526
2020-05-11 08:51:42 +00:00
Christian Boltz
cb44e6d47d Merge branch 'cboltz-drop-is-active-profile' into 'master'
Drop is_active_profile()

See merge request apparmor/apparmor!529

Acked-by: Steve Beattie <steve.beattie@canonical.com>
2020-05-11 08:37:16 +00:00
Christian Boltz
a5434e562d Merge branch 'cboltz-drop-write-rules' into 'master'
Drop / inline write_rules()

See merge request apparmor/apparmor!528

Acked-by: Steve Beattie <steve.beattie@canonical.com>
2020-05-10 23:03:47 +00:00
Christian Boltz
ae70ecfbaa Merge branch 'cboltz-vim-alias' into 'master'
apparmor.vim: allow leading whitespace for alias rules

See merge request apparmor/apparmor!527

Acked-by: Steve Beattie <steve.beattie@canonical.com> for 2.11..master
2020-05-10 22:54:34 +00:00
Christian Boltz
710d5ae466
Drop is_active_profile()
... and inline it at the only place that uses it
2020-05-10 23:49:16 +02:00