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

i/apparmor: add missing expansion for s-u-n template #13853

Merged
merged 1 commit into from Apr 18, 2024

Conversation

zyga
Copy link
Collaborator

@zyga zyga commented Apr 18, 2024

This fixes access to /etc/apparmor.d/tunables when running from snapd snap. When snapd snap re-executes, and uses apparmor_parser from snapd snap (those are separate conditions), then it re-directs the parser away from host /etc/apparmor.d and we have special code to load tunables from the host anyway. Those tunables are themselves conditional on the conditional include syntax that may or may not be supported by apparmor (otherwise the would be explicitly spelled out in the template, and not dynamically expanded with custom logic).

The problem was introduced along with patch
b98e4af (i/apparmor: support for home.d tunables from /etc/ (#13118)), as the case for snap-update-ns was missed, and the default expansion is an empty string.

Regression-testing this requires that we re-package snapd snap, so the test will come in with a separate patch as it requires somewhat more effort to behave correctly.

This issue was identified by Maciej Borzecki.

This fixes access to /etc/apparmor.d/tunables when running from snapd snap.
When snapd snap re-executes, and uses apparmor_parser from snapd snap (those
are separate conditions), then it re-directs the parser away from host
/etc/apparmor.d and we have special code to load tunables from the host anyway.
Those tunables are themselves conditional on the conditional include syntax
that may or may not be supported by apparmor (otherwise the would be explicitly
spelled out in the template, and not dynamically expanded with custom logic).

The problem was introduced along with patch
b98e4af (i/apparmor: support for home.d
tunables from /etc/ (snapcore#13118)), as the case for snap-update-ns was missed, and
the default expansion is an empty string.

Regression-testing this requires that we re-package snapd snap, so the test
will come in with a separate patch as it requires somewhat more effort to
behave correctly.

This issue was identified by Maciej Borzecki.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
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 the Simple 😃 A small PR which can be reviewed quickly label Apr 18, 2024
Copy link
Collaborator

@bboozzoo bboozzoo left a comment

Choose a reason for hiding this comment

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

LGTM

}
return ""
default:
// TODO: Warn that an invalid pattern is being used.
Copy link
Collaborator

Choose a reason for hiding this comment

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

related #13854

@ernestl ernestl merged commit 63a26ef into snapcore:master Apr 18, 2024
34 of 44 checks passed
ernestl pushed a commit to ernestl/snapd that referenced this pull request Apr 18, 2024
This fixes access to /etc/apparmor.d/tunables when running from snapd snap.
When snapd snap re-executes, and uses apparmor_parser from snapd snap (those
are separate conditions), then it re-directs the parser away from host
/etc/apparmor.d and we have special code to load tunables from the host anyway.
Those tunables are themselves conditional on the conditional include syntax
that may or may not be supported by apparmor (otherwise the would be explicitly
spelled out in the template, and not dynamically expanded with custom logic).

The problem was introduced along with patch
b98e4af (i/apparmor: support for home.d
tunables from /etc/ (snapcore#13118)), as the case for snap-update-ns was missed, and
the default expansion is an empty string.

Regression-testing this requires that we re-package snapd snap, so the test
will come in with a separate patch as it requires somewhat more effort to
behave correctly.

This issue was identified by Maciej Borzecki.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
ernestl pushed a commit that referenced this pull request Apr 24, 2024
This fixes access to /etc/apparmor.d/tunables when running from snapd snap.
When snapd snap re-executes, and uses apparmor_parser from snapd snap (those
are separate conditions), then it re-directs the parser away from host
/etc/apparmor.d and we have special code to load tunables from the host anyway.
Those tunables are themselves conditional on the conditional include syntax
that may or may not be supported by apparmor (otherwise the would be explicitly
spelled out in the template, and not dynamically expanded with custom logic).

The problem was introduced along with patch
b98e4af (i/apparmor: support for home.d
tunables from /etc/ (#13118)), as the case for snap-update-ns was missed, and
the default expansion is an empty string.

Regression-testing this requires that we re-package snapd snap, so the test
will come in with a separate patch as it requires somewhat more effort to
behave correctly.

This issue was identified by Maciej Borzecki.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Simple 😃 A small PR which can be reviewed quickly
Projects
None yet
3 participants