Commit graph

5402 commits

Author SHA1 Message Date
John Johansen
730db17607 policy: tag policy with the AppArmor 3.0 abi
Tag profiles and abstractions with abi information.

Tagging abstractions is not strictly necessary but allows the parser
to detect when their is a mismatch and that policy will need an
update for abi.

We do not currently tag the tunables because variable declarations
are not currently affected by abi.

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/491
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
2020-05-29 00:23:17 -07:00
John Johansen
162da1ba48 parser: add basic support for feature abis
Add basic support for policy to specify a feature abi. Under the
current implementation the first feature abi specified will be
used as the policy abi for the entire profile.

If no feature abi is defined before rules are processed then the
default policy abi will be used.

If multiple feature abi rules are encountered and the specified
abi is different then a warning will be issued, and the initial abi
will continue to be used. The ability to support multiple policy
feature abis during a compile will be added in a future patch.

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/491
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
2020-05-29 00:23:37 -07:00
John Johansen
a29e232831 parser: feature abi: setup parser to intersect policy and kernel features
The features abi adds the ability to track the policy abi separate
from the kernel. This allow the compiler to determine whether policy
was developed with a certain feature in mind, eg. unix rules.

This allows the compiler to know whether it should tell the kernel to
enforce the feature if the kernel supports the rule but the policy
doesn't use it.

To find if a feature is supported we take the intersection of what is
supported by the policy and what is supported by the kernel.

Policy encoding features like whether to diff_encode policy are not
influenced by policy so these remain kernel only features.

In addition to adding the above intersection of policy rename
--compile-features to --policy-features as better represents what it
represents. --compile-features is left as a hidden item for backwards
compatibility.

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/491
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
2020-05-29 00:23:17 -07:00
John Johansen
f951f1de28 Merge Add AliasRule and AliasRuleset, and switch alias handling to them
As usual, the classes come with some tests.

A side effect of the change is a fix for the "last one wins" bug if a profile has two alias rules with the same path on the left side.

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/550
Acked-by: John Johansen <john.johansen@canonical.com>
2020-05-29 07:10:34 +00:00
Christian Boltz
e3af898ca4
Switch alias handling to AliasRule and AliasRuleset
This fixes cases when two aliases with the same left side were
configured - instead of "last one wins" in the dict, AliasRuleset now
keeps both.

ProfileList add_alias() changes its parameters and now expects an
AliasRule object. Adjust all callers to that.

Drop the no longer needed write_alias().

Also adjust the tests to use AliasRule and add a dedup test promised in
an earlier patch series.
2020-05-28 20:08:57 +02:00
Steve Beattie
d29bf02d7b
libapparmor/policy_cache.c: asprintf returns a signed int
asprintf(3) returns a signed int, so storing the result in a size_t is
and then comparing that stored value against -1 is not such a good idea.

Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/549
2020-05-28 09:27:12 -07:00
Steve Beattie
5250ca079d
C Makefiles: make C warning flag usage consistent
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/549
2020-05-28 09:24:56 -07:00
Christian Boltz
5bf9b51d4d
Add AliasRule and AliasRuleset classes
Also add some tests for them.
2020-05-27 14:05:38 +02:00
Christian Boltz
f0fd410fd1
Extend RE_PROFILE_ALIAS to provide named matches 2020-05-27 14:05:35 +02:00
Christian Boltz
09896bd5f1 Merge branch 'cboltz-variable' into 'master'
Add VariableRule and VariableRuleset and use it for variable handling

Besides the usual advantages of switching to classes, we finally get rid of the `filelist` hasher.

While on it, also fix some bugs around variable handling, including https://bugs.launchpad.net/apparmor/+bug/1331856 and some that maybe nobody noticed before.

As usual, see the individual commits for details.

See merge request apparmor/apparmor!544

Acked-by: Steve Beattie <steve.beattie@canonical.com>
2020-05-27 11:43:29 +00:00
Christian Boltz
d19735340a
Detect invalid trailing commas in variable definitions
Trailing commas in variable values are not allowed (unless they are
quoted). Fix the regex to avoid "eating" the comma, and add a check to
detect invalid commas.

As usual, add some tests, and remove some testcases from the
exception_not_raised list.
2020-05-27 13:32:44 +02:00
Christian Boltz
0629215f2a
Check for variable names not ending with }
... and add a test to ensure that everything works as expected.

Note that broken variable names like '@{foo' match the (quite
permissive) regex, but are invalid nevertheless.
2020-05-27 13:32:44 +02:00
Christian Boltz
e9b8139cee
Check variable errors when parsing simple_tests
... by calling active_profiles.get_all_merged_variables()

Also remove vars/vars_bad_add_assignment_1.sd from the
exception_not_raised list again - now it raises an exception as
expected.
2020-05-27 13:32:44 +02:00
Christian Boltz
7d86bf9fe2
severity: replace load_variables() with set_variables()
Add set_variables() to severity.py to set the variables for severity
rating. It typically gets the data from the get_all_merged_variables()
result.

This replaces the slightly broken load_variables() that parsed profile
files for variables. (For example, parsing "@{foo} = /bar" resulted
in a variable name "@{foo} " with trailing space.)

Also adjust aa.py and the severity tests to use set_variables() (with
get_all_merged_variables()) instead of load_variables().

This also re-adds the checks that were removed in the "Store variables
in active_profiles (ProfileList)" commit earlier, while still fixing
lp:1331856.

With this change, unload_variables() becomes useless (the variables get
overwritten in set_variables() anyway), drop it and its calls.

Note that load_variables() silently ignored non-existing files while the
get_all_merged_variables() call only works for existing files that are
known to active_profiles. Since the input of ask_the_questions() and
ask_exec() comes from log_dict (= audit.log or a profile to merge), add
a check if that profile actually exists in the set of active profiles.

Also adjust the severity tests to use set_variables().

Finally, drop the tests that check for handling non-existing include
files, redefining and adding to non-existing variables - all these
things get now handled in include_list_recursive() and
get_all_merged_variables() and their tests.

Fixes: https://bugs.launchpad.net/apparmor/+bug/1331856
2020-05-27 13:32:44 +02:00
Christian Boltz
7451c341a6
add get_all_merged_variables()
This function returns a dict with all variables and their values for a
given profile file. It also checks for redefined variables, and adding
to non-existing variables, and errors out in both cases. Note that it
does not check the include order and is therefore more forgiving than
apparmor_parser.

Also add some tests for get_all_merged_variables().

Note: the tests are based on reading "real" profiles, therefore we need
to initialize apparmor.aa and call some of its functions.

The alternative would be to construct ProfileList objects for some
profiles and includes manually, but doing that in a way that can replace
the tests with "real" profiles would be quite some work, and I'm not
bored enough for that ;-)
2020-05-27 13:32:44 +02:00
Christian Boltz
b13037e36d
Move reset_aa() from aa-mergeprof to aa.py
Some tests in the next commit also need it.
2020-05-27 13:32:44 +02:00
Christian Boltz
6155b356b3
VariableRuleset: add get_merged_variables()
This function returns a dict with variables, both definitions (=) and
value additions (+=).

Also add some tests.
2020-05-27 13:32:44 +02:00
Christian Boltz
9d2a10dec7
ProfileList: merge get_clean_first() into get_clean()
Now that ProfileList handles the whole preamble, there's no need to keep
two half functions.
2020-05-27 13:32:44 +02:00
Christian Boltz
fe3c0b0ef8
Get rid of the 'filelist' hasher()
Everything handled in 'filelist' gets handled in active_profiles now.

Note: the 'elif' branch in delete_all_duplicates() was probably never
hit because `if include.get(...)` always matched. The only possible
exception might be non-existing include files, but those cause a 'file
not found' error anyway.
2020-05-27 13:32:44 +02:00
Christian Boltz
430120879c
Drop now unused store_list_var() and its tests 2020-05-27 13:32:44 +02:00
Christian Boltz
abe0e2b246
ProfileStorage: Drop 'lvar' and write_list_vars()
Since the previous commit, variables never get stored in ProfileStorage,
and write_list_vars() is unused.
2020-05-27 13:32:44 +02:00
Christian Boltz
e5d38807df
Store variables in active_profiles (ProfileList)
... instead of filelist[file]['lvar'], and also write them from there.

Also fix detection of variable definitions inside a profile, which is
not allowed.

Note that ProfileList has a different write order than the old code -
first includes, then variable definitions. This makes more sense because
typical profiles first include tunables/global, and then define
additonal variables (that might use variables from tunables/global) or
extend variables defined in tunables/global.

This change also fixes some problems with the simple_test test profiles.
The "adding to non-existing variable" check currently doesn't exist,
which "fixes" lp:1331856.

OTOH this also means that such cases are not detected, therefore add
vars_bad_add_assignment_1.sd to the exception_not_raised list.

The check will be re-added in a later commit
in get_all_merged_variables().
2020-05-27 13:32:44 +02:00
Christian Boltz
2a58e0ada2
Extend ProfileList to handle (store/write) variables
... and also add some tests.
2020-05-27 13:32:43 +02:00
Christian Boltz
61db5595aa
VariableRuleset: Prevent re-defining variables
When adding a variable with a name that is already known to the
VariableRuleset, raise an exception.

Also add a test for this.
2020-05-27 13:32:43 +02:00
Christian Boltz
39eb1939ba
Split variables
... and enable tests related to this
2020-05-27 13:32:43 +02:00
Christian Boltz
1eb9791ed7
move separate_vars() from aa.py to VariableRule
... and also move its tests to test-variable.py
2020-05-27 13:32:43 +02:00
Christian Boltz
215ec38ae3
Add VariableRule and VariableRuleset
... and a set of tests for them.

Note that the tests include some TODOs, these will be handled in the
following commits.
2020-05-27 13:32:43 +02:00
Steve Beattie
c5a7bcd50e
utils/tests: test-include.py sometimes fails due to ordering
With the includes rule class landing, this particular test failed
in a test vm due to the sort ordering being different. It's not
clear that there should be an expectation of ordering returned from
get_full_paths(), so sort the result.

Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/548
2020-05-27 01:03:56 -07:00
John Johansen
7fa74521fb Merge Fix strip_quotes() to handle empty strings
strip_quotes() assumed its parameter is at least one character long, and errored out on an empty string.

It also converted a string consisting of a single quote to an empty string because that single quote had a quote as first and last char.

This commit fixes these two bugs.

Also rewrite TestStripQuotes to use tests\[\], and add some test for an empty string, a one-char path (just a slash) and a single quote.

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/545
Acked-by: John Johansen <john.johansen@canonical.com>
2020-05-26 07:40:56 +00:00
John Johansen
1aa80fd14d Merge Allow to read icu *.dat files in postfix-related profiles
This is needed for postdrop, postmap, postqueue and sendmail

Seen on openSUSE Tumbleweed.

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/546
Acked-by: John Johansen <john.johansen@canonical.com>
2020-05-26 07:38:41 +00:00
Christian Boltz
5d04922f00 Merge branch 'dnsmasq-trust-anchors' into 'master'
dnsmasq: allow reading DNSSEC trust anchors

See merge request apparmor/apparmor!547

Acked-by: Christian Boltz <apparmor@cboltz.de
2020-05-25 17:05:03 +00:00
Christian Boltz
b23aa00b81
Allow to read icu *.dat files in postfix-related profiles
This is needed for postdrop, postmap, postqueue and sendmail

Seen on openSUSE Tumbleweed.
2020-05-25 13:33:07 +02:00
intrigeri
1b9469eb31 dnsmasq: allow reading DNSSEC trust anchors
At least on Debian, when DNSSEC validation is enabled, dnsmasq needs to read
/usr/share/dnsmasq-base/trust-anchors.conf

Bug-Debian: https://bugs.debian.org/934869
2020-05-25 10:42:26 +00:00
Christian Boltz
373e8e23b1
Fix strip_quotes() to handle empty strings
strip_quotes() assumed its parameter is at least one character long, and
errored out on an empty string.

It also converted a string consisting of a single quote to an empty
string because that single quote had a quote as first and last char.

This commit fixes these two bugs.

Also rewrite TestStripQuotes to use tests[], and add some test for an empty
string, a one-char path (just a slash) and a single quote.
2020-05-24 13:41:13 +02:00
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