[apparmor] Some apparmor profile statements are not honored for an application

Wed Sep 2 17:53:24 UTC 2020

On 9/2/20 2:30 AM, John Ernberg wrote:
> Hi John.
> 
> (I am leaving most of my original reply in here in case others run into 
> the same problem, I was missing the --features-file because I missed 
> this part of the docs)
> 
> On 9/2/20 9:47 AM, John Johansen wrote:
>> On 8/31/20 7:54 AM, John Ernberg wrote:
>>> Hi,
>>>
>>> I seem to have a bit of an odd problem in that some of the profile
>>> statements do not appear honored and the library loads they statements
>>> allow are thus denied. There is the only program in the system so far
>>> that is running confined. The program in question only handles files and
>>> library loads.
>>>
>>> I double checked the syntax of the failing rules, they are aligned with
>>> functional rules. I re-did them by copy-pasting the file name in the
>>> audit message and then copy-pasted the flags from a functional rule,
>>> just to be sure. No change.
>>>
>> example rules and profiles would help. If you don't want them on the
>> public list you can send them to me directly
>>
>>> I have checked the statemachine produced with apparmor_parse -D
>>> dfa-states and it looks correct, however, when I dump the statemachine
>>> transitions in the kernel they are a little off compared to the
>>> statemachine generated by apparmor_parse -D dfa-states command when
>>> logging in aa_dfa_match on the kernel side, it really looks like the
>>> machine makes incorrect lookups.
>>>
>> this is weird. What are you using to dump the kernel state machine?
> 
> (email client mangling risk)
> @@ -464,8 +466,10 @@ unsigned int aa_dfa_match(struct aa_dfa *dfa, 
> unsigned int start,
>                                     equiv[(u8) *str++]);
>          } else {
>                  /* default is direct to next state */
> -               while (*str)
> +               while (*str) {
> +                       pr_err("apparmor: dfa: [%u] testing perm for 
> %s\n", state, str);
>                          match_char(state, def, base, next, check, (u8) 
> *str++);
> +               }
>          }
> 
>          return state;
> 
> Then I match the [xxx] number with the statemachine from dfa-states 
> output. A bit into the string the transitions stop matching, and after 
> another bit into the string segments might get skipped or read past end 
> of string.
> 
ah, got it.

>>
>> you can compare the binary in userspace and the equivalent kernel
>> blobs to make sure they are the same.
>>
>> Eg. for the ping profile on my system
>>
>>    $ cat /sys/kernel/security/apparmor/policy/profiles/ping.3/raw_sha1
>>    704b8eafff4dc11ede6b1843384fb7379f13028b
>>    $ sudo sha1sum /etc/apparmor.d/cache/bin.ping
>>    704b8eafff4dc11ede6b1843384fb7379f13028b  /etc/apparmor.d/cache/bin.ping
>>
>> and diff of binaries
>>
>>    $ sudo diff -q /sys/kernel/security/apparmor/policy/profiles/ping.3/raw_data /etc/apparmor.d/cache/bin.ping
>>    $
>>
>> You need to be aware that the sha1 and raw_sha1 are different, you want
>> the raw_sha1 and raw_data when comparing to the userspace file.
>>
>> Those two will correspond to the compiled blob in the file that is loaded
>> into the kernel. If that blob contains multiple profiles each profile
>> in the blob will point to the same raw_data, raw_sha1, but their
>> individual sha1 file will differ (its taken from a subsection).
>>
>> The raw_data and raw_sha1 files reported by the profiles are the same
>> that can be found in /sys/kernel/security/apparmor/policy/raw_data
>> going through the profile just makes it easier to correlate with the
>> policy on disk.
>>
>> Doing this check does not guarantee that what the kernel is using is
>> the same as what you see in userspace, but it does show what the
>> kernel should be using. This check will help us determine where we
>> should be focusing the search
>>
>> The kernel unpacks the loaded blob into a native format so there
>> is some data shuffling of what is actually used by the kernel. So what
>> the kernel is using could be off for 3 reasons
>> - the unpack is broken
>> - there is kernel memory corrupts
>> - there was a previous version of the profile loaded into the kernel
>>    and it is in use. Profile replacement is not atomic, when a mediation
>>    hook is entered the current policy what ever it may be at that
>>    point will be used, even if profile replacement happens while the
>>    hook is running. The new profile should be used in the next call to
>>    a mediation hook but there have in the past been bugs, causing the
>>    old profile to be used long past when it should.
>>
>>
>>> I have noticed transitions to IDs other than those expected when
>>> comparing to the dfa-states, and it seems to start skipping characters
>>> in the paths or going beyond the end of the path string for some paths.
>>> These paths are the paths for which statements are not honored.
>>>
>> how are you checking this? This should not be possible as it is the path
>> that drives the state machine. That is the path is walked linearly one
>> character at a time and we step to the next state in the state machine.
> 
> See diff chunk earlier in the mail.
>>
>> The only time we walk a path more than once per character is when doing
>> profile matching, and the overlapping express match allows for a limited
>> nfa stack of N states for backing up. iirc in 5.4 it is limited to 8
>> states worth of backup.
>>
>>> Trying to add debug to the match_char macro on the kernel side seems to
>>> break the statemachine completely, so I wasn't able to debug this route
>>> further.
>>>
>> you can add stuff to the macro but you do need to be careful it is
>> easy to break it
> @@ -376,6 +376,7 @@ struct aa_dfa *aa_dfa_unpack(void *blob, size_t 
> size, int flags)
>   do {                                                   \
>          u32 b = (base)[(state)];                        \
>          unsigned int pos = base_idx(b) + (C);           \
> +       pr_err("apparmor: dfa: current char %c\n", (C));\
>          if ((check)[pos] != (state)) {                  \
>                  (state) = (def)[(state)];               \
>                  if (b & MATCH_FLAG_DIFF_ENCODE)         \
> 
> That chunk is all that I added, after that no statement in the profile 
> applies and (C) is mostly > 127
> 
weird, that should work

>>
>>> I'm running version 2.13.3 of the userspace tools, and kernel 5.4.24
>>> (vendor kernel, can't upgrade, can't try mainline due to missing support
>>> for the SoC I'm using), with the latest 5.4 stable patches for apparmor
>>> applied on top.
>>>
>> can you run a debug kernel with custom patches if needed?
> Slightly limited with my debug options due to working from home with 
> limited equipment, but I can definitely try patches as needed.
> 
>>
>>> The rules is compiled on my machine (x86_64) and embedded in my target
>>> (aarch64) readonly rootfs. Target has a readonly filesystem.
>>>
>> this should be fine the compiled policy tries to arch independent but it
>> is always possible we have an bug in the native mappings. It is something
>> to keep in mind, while trying to figure out what is going on
>>
>>> The rule is loaded on target with the following command:
>>>       /sbin/apparmor_parser -K -B /etc/apparmor.d/myprogram
>>> Where myprogram is the profile for my program
>>>
>> what audit message/messages are logged when you do this
> 
> [    4.478643] audit: type=1400 audit(1603443648.037:3): 
> apparmor="STATUS" operation="profile_load" profile="unconfined
> " name="/usr/bin/myprogram" pid=533 comm="apparmor_parser"
> 
> The program is started later
> 

Ack, just wanted to make sure the load was reported as succeeding

>>
>>> So far I have not been able to create a shareable reproducer which,
>>> unfortunately, makes this all the more harder.
>>>
>>> I would appreciate any suggestions in how to proceed or what kind of
>>> info I should be looking at in order to find out what is going wrong.
>>>
>> First verify that the user space binary file and what the kernel expects
>> are the same.
> 
> (from target)
> # sha1sum /etc/apparmor.d/usr.bin.myprogram
> 90ca35aa2f57b3ab562c14a6927cef11fbc24d0c  /etc/apparmor.d/usr.bin.myprogram
> # cat 
> /sys/kernel/security/apparmor/policy/profiles/usr.bin.myprogram.1/raw_sha1
> 90ca35aa2f57b3ab562c14a6927cef11fbc24d0c

okay, so this would then be pointing to
kernel memory corruption, stale profile, or abi issues

>>
>> Second, are you pinning the policy feature abi or using the kernel feature
>> abi, and if you are using the kernel feature abi, what is the feature
>> abi of the machine you are compiling on vs. the kernel you are loading
>> policy on.
> 
> I wasn't, I had completely missed this feature. I extracted the 
> .features file from target kernel and provided that to the compiler with 
> --features-file
> 
> Everything now looks like I expect it to look. Thank you very much for 
> this hint.
> 
> Very curious results when this is skipped!
> 

So apparmor supports multiple abis. This allows for profiles developed
under different abis to be used at the same time. This is useful when
you are using a profile that is maintained separately from the main
profile set, so that it doesn't just break on newer kernels that
have features the profile wasn't developed under (apparmor being default
deny will deny the new features that profile doesn't have rules for).

It is also used for containers so a Ubuntu 20.04 can load its policy
but also set up a policy namespace for an 16.04 container (LXD supports
this). The container can load its policy and it will work as expected
for 16.04

You can find a little more detail on the feature abi
https://gitlab.com/apparmor/apparmor/-/wikis/apparmorpolicyfeaturesABI 

when pre-compiling policy you need to specify either --features-file
or in AppArmor 3 --kernel-features to make sure the abi is something
properly supported on the target.

>>
>> Extracting the feature abi in apparmor 2.13 is a bit of a pita. For the
>> machine you are compiling on look for the .features file in the cache
>> directory. If you are not writing the cache, make a temporary cache
>> in tmp
>>    mkdir /tmp/cache
>>
>> and add
>>
>>    --cache-loc=/tmp/cache --write-cache
>>
>> to your compile.
> 
> My local machine doesn't have apparmor enabled, but compiled in.
>>
>> On the machine you are loading the policy on do the same thing. Make
>> a temporary cache and compile a simple profile
>>
>>    profile test { }
>>
>> will do.
>>
>> Compare the two feature abi files. >
>> Also can you be more specific about the rules that are having problems?
>> File rules, signal, and even which components. The more detail the
>> better, feel free to substitue example values instead of real ones if
>> you need to.
> 
> Only file rules are used at this time.
>>
>>
>>> Thank you in advance.
>>>
>>> Best regards // John Ernberg
>>>
>>> (not subscribed to the mailing list)
>>>
>> thats fine just expect some moderation delay
>>
> 
> Best regards // John Ernberg
> 