Comment 7 for bug 1649302

Revision history for this message
Adam Plaice (aplaice) wrote :

AFAICT there are possibly two issues with aa-logprof (not apparmor itself!) here. I'm confident that the second is a bug, I'm not sure that the first is.

1. aa-logprof _possibly_ unnecessarily† escapes the special characters (`*{[` etc.) with backslashes in the profile name in the "peer" statement. (e.g. The profile name is `xxx*xxx`, and the suggested peer statement becomes `ptrace trace peer=xxx\*xxx`)

† I haven't extensively checked apparmor's behaviour with and without escaping all of the special characters.

This is caused by `convert_expression_to_aare` (in `aare.py`) which escapes all such characters in expressions taken from logs.

To reproduce:

For instance take a profile in file `usr.bin.foo`

```
/usr/bin/foo* {
  # Arbitrary rule, so that the profile is non-empty.
  /** rw,
}
```

and a log:

```
Dec 8 18:42:06 adam kernel: [320732.937499] audit: type=1400 audit(1638985326.875:800): apparmor="DENIED" operation="ptrace" profile="/usr/bin/foo*" pid=293238 comm="foobar" requested_mask="trace" denied_mask="trace" peer="/usr/bin/foo*"
```

and call `aa-logprof -f log -d path/to/profile`.

Observed behaviour:

aa-logprof suggests this new rule:

```
  ptrace trace peer=/usr/bin/foo\*,
```

with the asterisk escaped.

2. aa-logprof doesn't consider backslashes in peer rules when processing the profiles.

`convert_regexp` (in `common.py`) converts glob characters like `*`, `**`, `{}`, `,` into appropriate regex patterns, according to special rules (e.g. `*` becomes `(((?<=/)[^/\000]+)|((?<!/)[^/\000]*))` ). This happens even if the glob character is escaped with a backslash[0]. Hence, for instance `/usr/bin/foo\*` becomes the python string

    '^/usr/bin/foo\\(((?<=/)[^/\x00]+)|((?<!/)[^/\x00]*))$'

(note that since this is a python string the `\\` represents a single backslash). The interesting part of the pattern is:

    '\\(...)'

The cause of the aa-logprof crash is that the opening parenthesis is escaped (with the `\\` backslash), while the closing one isn't (resulting in the error message "unbalanced parenthesis").

[0] https://gitlab.com/apparmor/apparmor/-/wikis/AppArmor_Core_Policy_Reference#apparmor-globbing-syntax

To reproduce:

For instance take a profile in file `usr.bin.foo` (with the peer pattern generated by aa-logprof):

```
/usr/bin/foo* {
  ptrace trace peer=/usr/bin/foo\*,
}
```

and a log:

```
Dec 8 18:42:06 adam kernel: [320732.937499] audit: type=1400 audit(1638985326.875:800): apparmor="DENIED" operation="ptrace" profile="/usr/bin/foo*" pid=293238 comm="foobar" requested_mask="trace" denied_mask="trace" peer="/usr/bin/foo*"
```

and call `aa-logprof -f log -d path/to/profile`.

Solving this issue correctly would result in a very tricky set of regular expressions, though.