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

sandbox: Revert "sandbox/apparmor: do not skip ABI 4.0 from host parser (#14167)" #14223

Merged
merged 1 commit into from
Jul 23, 2024

Conversation

zyga
Copy link
Contributor

@zyga zyga commented Jul 22, 2024

This reverts commit fa03549.

We cannot use host AppArmor with 4.0 ABI as there's no control mechanism to shield us from broken implementation of mqueue mediation class.

We look for the right version of apparmor parser and correctly not emit the mqueue permission but since the host parser (4.0.0~beta3) looks at host's ABI file which contains:

ipc {posix_mqueue {create read write open delete setattr getattr}

And similarly the kernel supports posix_mqueue, then the parser (with
the bug or without the bug) will correctly not emit any permissions
related to mqueue mediation class, while emitting the mediation class
root element, causing the kernel to rightfully deny operations:

[Mon Jul 22 12:43:40 2024] audit: type=1400 audit(1721652220.385:212):
apparmor="DENIED" operation="unlink" class="posix_mqueue"
profile="snap.docker.dockerd" name="/" pid=35290 comm="runc:[2:INIT]"
requested="getattr" denied="getattr"class="posix_mqueue" fsuid=0 ouid=0

As such we need to do one of two things to allow host apparmor to be used in a world with re-executing snapd:

  • Create our own ABI feature files that understand broken features and mask them, so that from the point of view of the kernel mqueue is not mediated by the binary profile.
  • Detect presence of 4.0 ABI but ignore it on known-broken parser versions, effectively doing the same thing as the earlier approach but without creating a new ABI file that only snapd uses (possibly experiencing fewer bugs).

@zyga
Copy link
Contributor Author

zyga commented Jul 22, 2024

And similarly the kernel supports posix_mqueue, then the parser (with the bug or without the bug) will correctly not emit anything related to mqueue mediation class, causing the kernel to rightfully deny operations:

I meant that the parser emits mqueue mediation class DFA elements but they contain no permissions.

EDIT: This has been re-worded to:

And similarly the kernel supports posix_mqueue, then the parser (with
the bug or without the bug) will correctly not emit any permissions
related to mqueue mediation class, while emitting the mediation class
root element, causing the kernel to rightfully deny operations:

…ical#14167)"

This reverts commit fa03549.

We cannot use host AppArmor with 4.0 ABI as there's no control mechanism
to shield us from broken implementation of mqueue mediation class.

We look for the right version of apparmor parser and correctly not emit
the mqueue permission but since the host parser (4.0.0~beta3) looks at
host's ABI file which contains:

    ipc {posix_mqueue {create read write open delete setattr getattr}

And similarly the kernel supports posix_mqueue, then the parser (with
the bug or without the bug) will correctly not emit any permissions
related to mqueue mediation class, while emitting the mediation class
root element, causing the kernel to rightfully deny operations:

    [Mon Jul 22 12:43:40 2024] audit: type=1400 audit(1721652220.385:212):
    apparmor="DENIED" operation="unlink" class="posix_mqueue"
    profile="snap.docker.dockerd" name="/" pid=35290 comm="runc:[2:INIT]"
    requested="getattr" denied="getattr"class="posix_mqueue" fsuid=0 ouid=0

As such we need to do one of two things to allow host apparmor to be
used in a world with re-executing snapd:

 - Create our own ABI feature files that understand broken features and
   mask them, so that from the point of view of the kernel mqueue
   is _not_ mediated by the binary profile.
 - Detect presence of 4.0 ABI but ignore it on known-broken parser
   versions, effectively doing the same thing as the earlier approach
   but without creating a new ABI file that only snapd uses (possibly
   experiencing fewer bugs).

Signed-off-by: Zygmunt Krynicki <[email protected]>
@zyga zyga changed the title Revert "sandbox/apparmor: do not skip ABI 4.0 from host parser (#14167)" sandbox: Revert "sandbox/apparmor: do not skip ABI 4.0 from host parser (#14167)" Jul 22, 2024
Copy link
Contributor

@alexmurray alexmurray left a comment

Choose a reason for hiding this comment

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

LGTM - fwiw I prefer the second option of detecting parser version etc rather than trying to do our own abi files plus masking.

@ernestl ernestl added this to the 2.64 milestone Jul 22, 2024
@zyga zyga closed this Jul 22, 2024
@zyga zyga reopened this Jul 22, 2024
@ernestl ernestl merged commit ae4884d into canonical:master Jul 23, 2024
60 of 70 checks passed
@zyga zyga deleted the fix/revert-host-aa-4 branch July 23, 2024 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants