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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable rootless AppArmor #19303

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

kernelmethod
Copy link

Related issues:

As previously noted in containers/common#958 and #15874, it is perfectly possible for rootless containers to run with an AppArmor profile. However, the current architecture of Podman requires it to load the default AppArmor profile at runtime before running the container.

With this PR, we no longer attempt to load the profile at runtime. Instead (as discussed in #15874), default profiles should be installed to /etc/apparmor.d/ at the point where Podman is installed, and then specified in a default containers.conf.

This is the sister PR to containers/common#1572; they should be merged in together to avoid breakage.

Does this PR introduce a user-facing change?

None

These changes make it possible to specify an AppArmor profile to run a
container under via the --security-opt apparmor=... flag when rootless.
Podman previously required root privileges to specify an AppArmor in
order to check /sys/kernel/security/apparmor/profiles to see whether or
not the provided AppArmor profile was loaded into the kernel. After
these changes, Podman will instead error out when it attempts to write
/proc/thread-self/attr/exec with a non-existent profile.

Note that Podman can't use apparmor_parser to load a new profile into
the kernel without root. There isn't a convenient way to allow
generateSpec to run CheckProfileAndLoadDefault rootlessly from
containers/common in its current implementation, so I've removed all
uses of that function from generateSpec (even when we're running as
root). In theory this would mean that under these changes we won't be
able to load a default profile for containers. However, it turns out
that in practice there was never any path through the code where Podman
would load the default profile anyways, so this is a non-breaking
change.

Fixes an issue previously described in:
containers/common#958

Signed-off-by: Will Shand <[email protected]>
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 20, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: kernelmethod
Once this PR has been reviewed and has the lgtm label, please assign edsantiago for approval. For more information see the Kubernetes Code Review Process.

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

- Import github.com/opencontainers/runc/libcontainer/apparmor as
  apparmor rather than runcaa for clarity.
- Don't skip the 'podman run no apparmor --privileged' if we're running
  rootlessly.

Signed-off-by: Will Shand <[email protected]>
Skip("Apparmor is not enabled")
}
}
func skipIfRootless() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is already SkipIfRootless

@github-actions
Copy link

A friendly reminder that this PR had no activity for 30 days.

@rhatdan
Copy link
Member

rhatdan commented Aug 22, 2023

@kernelmethod Still working on this one?

@rhatdan rhatdan removed the stale-pr label Aug 22, 2023
@kernelmethod
Copy link
Author

I am, I've just fallen behind on getting it finished. If you'd prefer that I close this PR and open a new one when it's ready to merge, that's fine with me.

@rhatdan
Copy link
Member

rhatdan commented Aug 22, 2023

No problem, I would prefer you update this PR with fixes when you get a chance.

@github-actions
Copy link

A friendly reminder that this PR had no activity for 30 days.

@rhatdan
Copy link
Member

rhatdan commented Sep 29, 2023

@kernelmethod any update?

@kernelmethod
Copy link
Author

Not at the moment; I'm planning on returning to this in mid- / late-October. If somebody else wants to pick this up then they're free to do so, but it's currently low-priority for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants