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/apparmor: do not skip ABI 4.0 from host parser #14167

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

zyga
Copy link
Contributor

@zyga zyga commented Jul 9, 2024

The state of AppArmor 4 in Ubuntu 24.04 and 24.10 is now sufficient for
using in snapd. The recently-added version-aware feature check guarantee
that features such as mqueue are not used when the parser is not
sufficiently up-to-date.

@zyga zyga added ⛔ Blocked Needs security review Can only be merged once security gave a :+1: labels Jul 9, 2024
@zyga zyga force-pushed the feature/use-host-apparmor-4-abi branch from 6853098 to ddfcb2d Compare July 9, 2024 08:47
@ernestl ernestl self-requested a review July 9, 2024 11:43
@zyga zyga force-pushed the feature/use-host-apparmor-4-abi branch from ddfcb2d to 82d73c9 Compare July 12, 2024 09:47
@zyga zyga changed the title sandbox/apparmor: do not ignore ABI 4.0 from the host sandbox/apparmor: do not skip ABI 4.0 from host parser Jul 12, 2024
@zyga zyga removed the ⛔ Blocked label Jul 12, 2024
@zyga zyga marked this pull request as ready for review July 12, 2024 09:48
Copy link
Collaborator

@ernestl ernestl left a comment

Choose a reason for hiding this comment

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

Thanks

@ernestl ernestl added this to the 2.64 milestone Jul 17, 2024
@ernestl
Copy link
Collaborator

ernestl commented Jul 17, 2024

Consider for 2.64

Copy link
Contributor

@jslarraz jslarraz left a comment

Choose a reason for hiding this comment

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

PR #14150 already introduced support for ABI 4.0 when using the internal copy of the apparmor_parser. It looks appropriate to the same when using the host parser.
LGTM, thanks!

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 - thanks for this @zyga!

@alexmurray alexmurray removed the Needs security review Can only be merged once security gave a :+1: label Jul 18, 2024
The state of AppArmor 4 in Ubuntu 24.04 and 24.10 is now sufficient for
using in snapd. The recently-added version-aware feature check guarantee
that features such as mqueue are not used when the parser is not
sufficiently up-to-date.

Signed-off-by: Zygmunt Krynicki <[email protected]>
@zyga zyga force-pushed the feature/use-host-apparmor-4-abi branch from 82d73c9 to 0cf5d26 Compare July 19, 2024 06:46
@zyga
Copy link
Contributor Author

zyga commented Jul 19, 2024

I've rebased on master to give tests a chance to pass.

@ernestl ernestl merged commit fa03549 into canonical:master Jul 19, 2024
44 of 53 checks passed
ernestl pushed a commit to ernestl/snapd that referenced this pull request Jul 19, 2024
The state of AppArmor 4 in Ubuntu 24.04 and 24.10 is now sufficient for
using in snapd. The recently-added version-aware feature check guarantee
that features such as mqueue are not used when the parser is not
sufficiently up-to-date.

Signed-off-by: Zygmunt Krynicki <[email protected]>
zyga added a commit to zyga/snapd that referenced this pull request Jul 22, 2024
…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 anything related to
mqueue mediation class, 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 added a commit to zyga/snapd that referenced this pull request Jul 22, 2024
…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]>
@ernestl
Copy link
Collaborator

ernestl commented Jul 22, 2024

This was reverted, there are issues: #14223
Removed this from release snapd 2.64

ernestl pushed a commit that referenced this pull request Jul 23, 2024
…" (#14223)

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]>
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.

4 participants