Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dump default AppArmor profile #7599

Closed

Conversation

flavio
Copy link
Contributor

@flavio flavio commented Sep 11, 2020

Lately I've been struggling a lot to debug an AppArmor related issue: the default profile was too tight for my application (buildah BTW 馃槃) and I had to find how to relax it.

The default AppArmor profile is dynamically generated by podman at runtime and, once loaded into the kernel, there's no way to print it in a human-readable form. The only way to read the default profile is to start a spelunking session inside of the vendored source code, not fun/doable for the average user 馃槩

This "issue" is common also with docker, where apparently things are going to get better thanks to this PR.

This draft PR provides a way to let the average user dump the default AppArmor profile. Right now I just added this information to the podman info output. OFC this could be moved/changed/dropped... I'm basically looking for your feedback 馃槃

This is an example of the feature in action:

$ ./bin/podman info
AppArmor: |2


  #include <tunables/global>


  profile default flags=(attach_disconnected,mediate_deleted) {

    #include <abstractions/base>


    network,
    capability,
    file,
    umount,


    # Allow signals from privileged profiles and from within the same profile
    signal (receive) peer=unconfined,
    signal (send,receive) peer=default,


    deny @{PROC}/* w,   # deny write for all files directly in /proc (not in a subdir)
    # deny write to files not in /proc/<number>/** or /proc/sys/**
    deny @{PROC}/{[^1-9],[^1-9][^0-9],[^1-9s][^0-9y][^0-9s],[^1-9][^0-9][^0-9][^0-9]*}/** w,
    deny @{PROC}/sys/[^k]** w,  # deny /proc/sys except /proc/sys/k* (effectively /proc/sys/kernel)
    deny @{PROC}/sys/kernel/{?,??,[^s][^h][^m]**} w,  # deny everything except shm* in /proc/sys/kernel/
    deny @{PROC}/sysrq-trigger rwklx,
    deny @{PROC}/kcore rwklx,

    deny mount,

    deny /sys/[^f]*/** wklx,
    deny /sys/f[^s]*/** wklx,
    deny /sys/fs/[^c]*/** wklx,
    deny /sys/fs/c[^g]*/** wklx,
    deny /sys/fs/cg[^r]*/** wklx,
    deny /sys/firmware/** rwklx,
    deny /sys/kernel/security/** rwklx,


    # suppress ptrace denials when using using 'ps' inside a container
    ptrace (trace,read) peer=default,

  }
host:
  arch: amd64
  buildahVersion: 1.16.0-dev
  cgroupVersion: v1
  conmon:
    package: conmon-2.0.20-2.1.x86_64
    path: /usr/bin/conmon
    version: 'conmon version 2.0.20, commit: unknown'
  cpus: 8
  distribution:
    distribution: '"opensuse-tumbleweed"'
    version: "20200904"
  eventLogger: journald
[ ... goes on ... ]

Extend the `info` command to dump the default AppArmor profile used by
podman.

This can be useful to debug AppArmor related issues when the default
profile is too tight for the containerized application.

The default AppArmor profile is dynamically generated by podman at
runtime and, once loaded into the kernel, there's no way to print it
in a human-readable form.

Signed-off-by: Flavio Castelli <fcastelli@suse.com>
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 11, 2020
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Sep 11, 2020

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: flavio
To complete the pull request process, please assign mheon
You can assign the PR to them by writing /assign @mheon in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@flavio
Copy link
Contributor Author

flavio commented Sep 11, 2020

/assign @mheon

@vrothberg
Copy link
Member

vrothberg commented Sep 11, 2020

I think that podman info is the right place for it. But I prefer to not move it into the struct but hide it behind a flag (e.g., --apparmor) where podman info would print the profile and nothing else.

@vrothberg
Copy link
Member

vrothberg commented Sep 11, 2020

@AkihiroSuda @giuseppe @rhatdan WDYT?

@rhatdan
Copy link
Member

rhatdan commented Sep 11, 2020

Why don't we ship this as a separate file and not have a built in. Like we do with seccomp.json?

@vrothberg
Copy link
Member

vrothberg commented Sep 11, 2020

Why don't we ship this as a separate file and not have a built in. Like we do with seccomp.json?

I think that the profile may have to many runtime deps on the host so it may look slightly different among different hosts.

@flavio
Copy link
Contributor Author

flavio commented Sep 11, 2020

The reason nobody ships that as a dedicated file is complicated. This comment provides some background, however I think @cyphar could add more to the discussion as he's the main expert on the topic.

@cyphar
Copy link

cyphar commented Sep 11, 2020

The apparmor.d thing is a separate issue -- in principle we could (and should!) ship the apparmor profile template as a separate file (in /etc/containers or /etc/docker) which is loaded by podman/docker/etc (just like seccomp.json is today as @rhatdan mentioned). To be honest, having this feature would've made it easier to give customers workarounds for bugs in the default AppArmor profile on several occasions -- so I'm definitely 馃憤 on having this.

I looked into adding something like this to Docker some time ago (moby/moby#33060 (comment) -- this is the comment Flavio linked but I did discuss it elsewhere at the time) but at the time I didn't want to make the internal template public and so tried to implement it without the templating code. Unfortunately this doesn't really work, and we will have to expose the template to users (you need to have a way to detect apparmor_parser versions as well as several other things).

I can look at implementing this again -- this will also likely tie in with the whole "unification" of AppArmor profiles between Docker/podman/cri-o, so it's as good a chance as any to implement the damn thing.

@vrothberg

I think that the profile may have to many runtime deps on the host so it may look slightly different among different hosts.

Yeah that's why we'd need to just save the Go template to disk. It's a bit unfortunate since it's slightly more magical than just a standard AppArmor profile but this is simpler than having a special case for the internal profile. And ultimately most users probably won't need the templating support, but distributions will probably want to make use of it -- if we ever need to give workarounds to customers, templating support would be necessary to be able to 1-for-1 swap out the default profile.

@cyphar
Copy link

cyphar commented Sep 11, 2020

(I also think adding a feature to print out the profile is a somewhat less elegant workaround for it not being user-configurable in the first place.)

@rhatdan
Copy link
Member

rhatdan commented Sep 11, 2020

Well you people will have to handle this, since I am woefully unqualified to work on AppArmor. I can only handle one MAC at a time.

@cyphar
Copy link

cyphar commented Sep 13, 2020

I've implemented a basic version of the custom AppArmor profile configuration for Docker here. A similar idea could be implemented for podman/cri-o and containerd.

@flavio
Copy link
Contributor Author

flavio commented Sep 14, 2020

I think Aleksa's solution is far more elegant. I will close the PR then.

@rhatdan
Copy link
Member

rhatdan commented Jan 12, 2021

@cyphar Could you submit a default apparmor profile for Podman/containers-common to ship rather then use the builtin one, making it easier for users to customize?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants