Commit graph

1681 commits

Author SHA1 Message Date
Ryan Lee
b43f1c4073 Make parser_include push_include_stack take const char because it doesn't actually modify it
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
2024-10-28 12:35:26 +01:00
Ryan Lee
2e841655cf Add a tst_binaries target to the parser to build tst binaries
This allows building the tst_* binaries in parallel independently of running the parser test suite

Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
2024-10-18 11:46:40 -07:00
Ryan Lee
776c56bd7e Fix compiler warnings about format specifiers with DEBUG 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>
2024-10-16 12:26:54 -07:00
Ryan Lee
ed0b539c94 Replace sigalarm-based subprocess timeout with the built-in one
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
2024-10-15 16:51:17 -07:00
John Johansen
9fe5a6d853 Merge update translations pot file for C code
The parser and binutils pot file have not been recently refreshed. Update them to current code and add missing pot files for aa_load and aa_status. Also give aa_status base support for translations to populate its pot file.

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1318
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
2024-09-17 09:18:10 +00:00
John Johansen
4628f8e880 Merge Add parser_sanity-no-gen target to run simple_tests
... without all the profiles generated by the gen-*.py scripts.

This target is meant for local manual testing, especially when working
on additional simple_tests profiles.

It makes local testing much faster (15 seconds for ~2k profiles vs.
several minutes for the additional ~70k profiles generated by gen-*.py)

Needless to say that the CI should continue to use the parser_sanity
target that includes all the generated profiles.

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1325
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
2024-09-11 07:50:01 +00:00
Christian Boltz
762df7e753
Add more tests for network port range 2024-09-10 23:10:32 +02:00
Christian Boltz
0d5ae170d4
Add parser_sanity-no-gen target to run simple_tests
... without all the profiles generated by the gen-*.py scripts.

This target is meant for local manual testing, especially when working
on additional simple_tests profiles.

It makes local testing much faster (15 seconds for ~2k profiles vs.
several minutes for the additional ~70k profiles generated by gen-*.py)

Needless to say that the CI should continue to use the parser_sanity
target that includes all the generated profiles.
2024-09-10 22:31:11 +02:00
Georgia Garcia
93f3a0fa99 parser: add equality tests for network port range
To run the network port range equality tests, we need to check if the
kernel supports the network_v8/af_inet feature. Also, a new file
features.af_inet is needed containing the af_inet feature.

Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
2024-09-06 09:49:59 -03:00
Georgia Garcia
f9621054d7 parser: add port range support on network policy
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
2024-09-05 17:01:46 -03:00
John Johansen
ef95c96f45 parser: update translations pot file to current code
The parser pot file should have been updated before beta. Make
sure it is up to date with the current code.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-09-03 03:39:16 -07:00
Ryan Lee
8cb2e4ca9f Merge Replace 'scrub the environment' and similar wordings
The wording of "scrub the environment" with respect to execution modes is misleading, because a quick read of it could imply that it removes all environment variables. However, it actually enables ld.so's secure-execution mode, which removes a very limited subset of them. This MR rewords the relevant documentation and prompts. If proper environment variable filtering is added later, the documentation can be updated again then.

Synchronizes-with:
- Wiki page update, which I can do after this MR is approved
- Kernel patch to update wording of debug logs (patch submitted to the Apparmor mailing list [here](https://lists.ubuntu.com/archives/apparmor/2024-August/013339.html))

Things that may need updating first:

- Translations: attempting to update `utils/po/apparmor-utils.pot` resulted in a bunch of unrelated changes, so I'd like to ask about translation statuses before making a commit that updates that file properly.
- Adding info on which libc's actually behave differently based on AT_SECURE: glibc and musl libc both do, but they may do subtly different things. I don't know about other libc's.

Signed-off-by: Ryan Lee <ryan.lee@canonical.com>

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1315
Approved-by: John Johansen <john@jjmx.net>
Merged-by: Ryan Lee <rlee287@yahoo.com>
2024-08-30 16:14:13 +00:00
John Johansen
b6e9df3495 Merge parser: fix rule priority destroying rule permissions for some classes
io_uring and userns mediation are encoding permissions on the class
byte. This is a mistake that should never have been allowed.

With the addition of rule priorities the class byte mediates rule,
that ensure the kernel can determine a class is being mediated is
given the highest priority possible, to ensure class mediation can not
be removed by a deny rule. See
  61b7568e1 ("parser: bug fix mediates_X stub rules.")
for details.

Unfortunately this breaks rule classes that encode permissions on the
class byte, because those rules will always have a lower priority and
the class mediates rule will always be selected over them resulting in
only the class mediates permission being on the rule class state.

Fix this by adding the mediaties class rules for these rule classes
with the lowest priority possible. This means that any rule mediating
the class will wipe out the mediates class rule. So add a new mediates
class rule at the same priority, as the rule being added.

This is a naive implementation and does result in more mediates rules
being added than necessary. The rule class could keep track of the
highest priority rule that had been added, and use that to reduce the
number of mediates rules it adds for the class.

Technically we could also get away with not adding the rules for allow
rules, as the kernel doesn't actually check the encoded permission but
whether the class state is not the trap state. But it is required with
deny rules to ensure the deny rule doesn't result in permissions being
removed from the class, resulting in the kernel thinking it is
unmediated. We also want to ensure that mediation is encoded for other
rule types like prompt, and in the future the kernel could check the
permission so we do want to guarantee that the class state has the
MAY_READ permission on it.

Note: there is another set of classes (file, mqueue, dbus, ...) which
encodes a default rule permission as

  class .* <perm>

this encoding is unfortunate in that it will also add the permission
to the class byte, but also sets up following states with the permission.
thankfully, this accespt anything, including nothing generally isn't
valid in the nothing case (eg. a file without any absolute name). For
this set of classes, the high priority mediates rule just ensures
that the null match case does not have permission.

Fixes: 61b7568e1 parser: bug fix mediates_X stub rules.
Signed-off-by: John Johansen <john.johansen@canonical.com>

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1307
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: John Johansen <john@jjmx.net>
2024-08-29 18:41:51 +00:00
Ryan Lee
65c84071bb Replace 'scrub the environment' wording in man pages with something more accurate
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
2024-08-28 11:22:08 -07:00
Jörg Sommer
8195500a1e apparmor.d.pod: Fix writing of aa_change_profile
Signed-off-by: Jörg Sommer <joerg@jo-so.de>
2024-08-17 14:13:08 +02:00
John Johansen
204c0c5a3a parser: fix rule priority destroying rule permissions for some classes
io_uring and userns mediation are encoding permissions on the class
byte. This is a mistake that should never have been allowed.

With the addition of rule priorities the class byte mediates rule,
that ensure the kernel can determine a class is being mediated is
given the highest priority possible, to ensure class mediation can not
be removed by a deny rule. See
  61b7568e1 ("parser: bug fix mediates_X stub rules.")
for details.

Unfortunately this breaks rule classes that encode permissions on the
class byte, because those rules will always have a lower priority and
the class mediates rule will always be selected over them resulting in
only the class mediates permission being on the rule class state.

Fix this by adding the mediaties class rules for these rule classes
with the lowest priority possible. This means that any rule mediating
the class will wipe out the mediates class rule. So add a new mediates
class rule at the same priority, as the rule being added.

This is a naive implementation and does result in more mediates rules
being added than necessary. The rule class could keep track of the
highest priority rule that had been added, and use that to reduce the
number of mediates rules it adds for the class.

Technically we could also get away with not adding the rules for allow
rules, as the kernel doesn't actually check the encoded permission but
whether the class state is not the trap state. But it is required with
deny rules to ensure the deny rule doesn't result in permissions being
removed from the class, resulting in the kernel thinking it is
unmediated. We also want to ensure that mediation is encoded for other
rule types like prompt, and in the future the kernel could check the
permission so we do want to guarantee that the class state has the
MAY_READ permission on it.

Note: there is another set of classes (file, mqueue, dbus, ...) which
encodes a default rule permission as

  class .* <perm>

this encoding is unfortunate in that it will also add the permission
to the class byte, but also sets up following states with the permission.
thankfully, this accespt anything, including nothing generally isn't
valid in the nothing case (eg. a file without any absolute name). For
this set of classes, the high priority mediates rule just ensures
that the null match case does not have permission.

Fixes: 61b7568e1 parser: bug fix mediates_X stub rules.
Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-15 03:51:20 -07:00
John Johansen
903a1b5689 parser: make ix of file, rule have lower priority so it can be overridden
the ix portion of file, causes x conflicts in regular priority. The
long term goal is to fix this by using dominance for x rules. But in
the mean time we can fix by giving the ix portion of the rule a
reduced priority.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 18:21:26 -07:00
John Johansen
e3fca60d11 parser: add the ability to specify a priority prefix to rules
This enables adding a priority to a rules in policy, finishing out the
priority work done to plumb priority support through the internals in
the previous patch.

Rules have a default priority of 0. The priority prefix can be added
before the other currently support rule prefixes, ie.

  [priority prefix][audit qualifier][rule mode][owner]

If present a numerical priority can be assigned to the rule, where the
greater the number the higher the priority. Eg.

    priority=1 audit file r /etc/passwd,

    priority=-1 deny file w /etc/**,

Rule priority allows the rule with the highest priority to completely
override lower priority rules where they overlap. Within a given
priority level rules will accumulate in standard apparmor fashion.

    Eg. given
        priority=1 w   /*c,
        priority=0 r   /a*,
        priority=-1 k  /*b*,

    /abc, /bc, /ac   .. will have permissions of w
    /ab, /abb, /aaa, .. will have permissions of r
    /b, /bcb, /bab,  .. will have permissions of k

User specified rule priorities are currently capped at the arbitrary
values of 1000, and -1000.

Notes:
* not all rule types support the priority prefix. Rukes like
  - network
  - capability
  - rlimits need to be reworked
  need to be reworked to properly preserve the policy rule structure.
* this patch does not support priority on rule blocks
* this patch does not support using a variable in the priority value.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 17:15:24 -07:00
John Johansen
61b7568e19 parser: bug fix mediates_X stub rules.
Currently mediates_X stub rules are added to the dfa to ensure a valid
transition state will exist if X should be mediated. The kernel uses
this to test whether the dfa supports certain mediation classes.

Unfortunately the mediates stub rules can be removed by other rules,
combined with minimization. In the allow case this is not a problem,
as if the stub rule is removed it will be due to state merging and the
test will still be valid. Unfortunately the deny case can wipe out the
stub rule in a couple of cases, meaning the when the kernel tests that
its in a valid state for mediation it will fail and treat the dfa as
not mediating the rule type, which results in allowing instead of
denying.

Fix this by making sure mediated stub rules can't be overridden by a
deny rule by giving them maximum priority.

Note: there is another issue with stub rule elimination in the allow
      case. It will can cause equality tests to fail when combined
      with priority rules, because the stub rules where added at
      priority 0 and an actual rule of higher priority could
      completely override it removing the permission on the stub rule.
      This issue will be caught by the equality.sh tests in the
      following patch that exposes priority to rules in policy.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 17:15:24 -07:00
John Johansen
abc18e45a4 parser: simplify prefix comparison code
The prefix comparison doesn't need to do as many operations as it is
doing, and the operator< can be based on the cmp() fn further reducing
the chance that the code will get out of sync if prefixes are changed.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 17:15:24 -07:00
John Johansen
cd95d46397 parser: split parse boolean into boolean and integer
the parser front end boolean is used for both boolean and integer
values. This is confusing when integer values different than 1 or 0
are being assigned to and from boolean.

Split its uses into the correct semantic boolean and integer cases.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 17:15:24 -07:00
John Johansen
ee1a5e6e18 parser: enable extended perms if supported by the kernel
Currently use of extended perms are dependent on prompt rules being present
in policy. Switch to using extended perms if they are supported.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 17:15:24 -07:00
John Johansen
2737cb2c2b parser: minimization - remove unnecessary second minimization pass
Moving apply_and_clear_deny() before the first minimization pass, which
was necessary to propperly support building accept information for
older none extended permission dfas, allows us to also get rid of doing a
second minimization pass if we want to force clearing explicit deny
info from extended permission tables.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 17:15:24 -07:00
John Johansen
1fa45b7c1f parser: dfa minimization prepare for extended permissions
Instead of compressing the permission set into 128 bit and using that
as the index in the permission map, just use the permissions directly
as the index into the permission map.

Note: this will break equality and minimization tests. Because deny
is not being cleared it will result in more partitions in the initial
setup. This will be addressed and the tests will be fixed in a follow
on patch.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 17:15:24 -07:00
John Johansen
5ff00bba3a parser: drop unused hash minimization remnants
Hash minimization was removed in
  f0b154528 Fix dfa minimization

however some remnants of minimization remained. A comment and the use
of the hash but only as a 0 value. Drop this dead code and comment.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 17:15:24 -07:00
John Johansen
012dcb6489 parser: only use 32 bit next/check tables if required
If the state machine does not requires more than 2^16 states use the
dfa16 encoding for next/check tables to keep the dfa size small.

Bug: https://gitlab.com/apparmor/apparmor/-/issues/419
Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 17:01:32 -07:00
John Johansen
f86fda02f5 parser: fix 16 bit state limitation
The hfa stores next/check transitions in 16 bit fields to reduce memory
usage. However this means the state machine can on contain 2^16
states.

Allow the next/check tables to be 32 bit. This theoretically could allow
for 2^32 states however the base table uses the top 8 bits as flags
giving us only 2^24 bits to index into the next/check tables. With
most states having at least 1 transition this effectively caps the
number of states at 2^24.

To obtain 2^32 possible states a flags table needs to be added. Add
a skeleton around supporting a flags table, so we can note the remaining
work that needs to be done. This patch will only allow for 2^24 states.

Bug: https://gitlab.com/apparmor/apparmor/-/issues/419

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 17:01:30 -07:00
John Johansen
22e1863e20 Merge parser: add support for prompt rules
This adds support for prompt rules and the beginning of support for extended permissions. Currently extended permissions are only used if a prompt rule is used in policy.

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1305
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
2024-08-14 23:34:29 +00:00
John Johansen
d0062b6d4a parser: fix protocol error on older kernels caused by additional xtable
Older kernels do not support an xtable grouped with the policy dfa.
The presence of a policy.dfa does not indicate whether we should create
an xtable with the policy dfa.

Instead the check should be if the kernel supports the extended
permstable32 format.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 15:47:13 -07:00
John Johansen
b72cae79cb parser: support uin128_t key as a pair of uint64_t numbers
__uint128 is not supported by gcc on 32 bit architectures so rework
the 128 bit map key to be a pair of 64bit numbers.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 15:47:13 -07:00
John Johansen
373c095b3e parser: switch backend to perm32_t for permission bits
switch permission bits to use perm32_t type. This is just annotating
the code as it is no different than uint32_t at this time.

We do not convert the accept values as they may be mapped permission
bits or they may be and index value.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 15:47:13 -07:00
John Johansen
4264338bed convert owner to an enum
provide better type checking and semantics to the owner conditional

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 15:47:13 -07:00
John Johansen
07155e8e83 parser: add note of what perms.h is
perms.h contains policy uapi for extended perms v2/v3 add a note
about what it is

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 15:47:13 -07:00
John Johansen
89673d0c5e parser: don't set xbits when using permstable32_v1
The use of xbits can not pass verification so we need to leave them
off this makes the profile a leaf profile.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 15:47:13 -07:00
John Johansen
5bd2271189 pass prompt info down into the backend for mapping
mapping for PROMPT_DEV needs to know that we should prompt
2024-08-14 15:47:13 -07:00
John Johansen
2510698f63 parser: make minimization sets take prompt into account
Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 15:47:13 -07:00
John Johansen
b4384d53e1 parser: Add prompt dev compat support
Support mapping rule prompt via the audit bits in pre permtable32
kernels.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 15:47:13 -07:00
John Johansen
1d0d1fd0c2 parser: and prompt-compat control flag
Allow contronling which prompt compat mode fallback is used.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 15:47:13 -07:00
John Johansen
5c2bd20720 parser: pass rule mode prompt through to backend
Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 15:47:13 -07:00
John Johansen
48b727b88a parser: frontend carry use of prompt rules flag on profile
add a flag to make it easy to check if a profile uses prompt rules.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 15:45:58 -07:00
John Johansen
e4890e6ba1 parser: Add work around for buggy permstable32
v1 of permstable32 has some broken verification checks. By using two
copies of a merged dfa and an xtable the same size of the permstable
we can work around the issue.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 15:45:58 -07:00
John Johansen
2e18cb9aed parser: rename rules.h perms_t to perm32_t
There are two distinct declarations of perms_t.
  rule.h: typedef uint32_t perms_t
  hfa.h: class perms_t

these definitions clash when the front end and backend share more info.
To avoid this rename rule.h to perm32_t, and move the definition into
perms.h and use it in struct aa_perms.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 14:39:18 -07:00
John Johansen
e29f5ce5f3 parser: if extended perms are supported by the kernel build a permstable
If extended permissions are supported use them. We need to build a
permission table and set the accept state of the chfa up as an index
into the table.

For now map the front end permission layout into the old format and
then convert that to the perms table just as the kernel does.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 14:39:18 -07:00
John Johansen
c86f8f06dd parser: add non-functional prompt parsing
Add the ability to parse the prompt qualifier but do not provide
functionality because the backend does not currently support prompt
permissions.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 14:39:18 -07:00
Georgia Garcia
15ee7ac92c parser: refactor conditional logic into its own class
Remove conditional logic from the parser and move it to its own class,
that way any improvements or conditional features will make cleaner
changes.

Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
2024-08-14 17:22:48 -03:00
John Johansen
db66b36064 parser: move perm to accept mapping into State
Let the state deal with permission mappings and what to do if outputting
an index.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-08-14 12:37:02 -07:00
John Johansen
b1a35e6cbd Merge Revert "parser: fix potential padding bug." and fix code to for correct padding
This reverts commit 78ae956087.

And the add the correct padding fix, so that the header size and what is written match.

Signed-off-by: John Johansen <john.johansen@canonical.com>

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1274
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: John Johansen <john@jjmx.net>
2024-08-01 18:46:28 +00:00
John Johansen
792f23c878 chfa: get not-flextable size and padding correct
The kernel does not expect a name and it is not used even within the
parser so drop it. Correct the padding calculation.

  sizeof(th_version)

includes the trailing \0 in the count so we should not be adding it
explicitly. Doing so made it seem like we were writing an extra byte
and messing things up, because the string write below did not include
the \0 which we had to add explicitly.

Switch to writing the th_version using size_of() bytes as is used in
the pad calculation, to avoid confusion around the header padding.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-07-20 01:58:26 -07:00
John Johansen
30d57a6bf0 parser: ensure mqueue obeys abi
The abi is not being respected by mqueue rules in many cases. If policy
does ot specify an mqueue rule the abi is correctly applied but if
an mqueue rule is specified explicitly or implicitly (eg. allow all).
without setting the mqueue type OR setting the mqueue type to sysv.
The abi will be ignored and mqueue will be enforced for policy regadless.

Known good mqueue rule that respects abi
  mqueue type=posix,

  # and all variations that keep type=posix

Known bad mqueue rules that do not respect abi

  mqueue,
  # and all variants that do not specify the type= option

  mqueue type=sysv,
  # and all variants that specify the type=sysv option

Issue: https://gitlab.com/apparmor/apparmor/-/issues/412
Fixes: d98c5c4cf ("parser: add parser support for message queue mediation")

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-07-19 03:17:08 -07:00
John Johansen
42523bae91 Revert "parser: fix potential padding bug."
This reverts commit 78ae956087.

Commit 78ae956087 causes policy to not
to conform to protocol as determined by the kernel. Technically the
reverted patch is correct and the kernel is wrong but we can not
change 15 years of history.

The reason it breaks the policy in the kernel is because the kernel
does not use the name field, and does not expect it. It just expects
the size with a single trailing 0. This doesn't break because this
section is all padded to 64 bytes so writing the extra 0 doesn't
hurt as it is effectively just manually adding to the padding.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2024-07-18 06:15:05 -07:00