snapd icon indicating copy to clipboard operation
snapd copied to clipboard

many: add experimental.apparmor-prompting

Open olivercalder opened this issue 1 year ago • 5 comments

Add experimental.apparmor-prompting feature. Enabling this feature regenerates AppArmor profiles for relevant interfaces to add the prompt prefix to AppArmor rules. AppArmor then knows to send a prompt request via the notify socket, which will be received by the prompting listener (defined in sandbox/apparmor/notify/listener).

For now, this PR explicitly marks notify support as unavailable, so it will have no effect on AppArmor rules yet. Once the other prompting components are merged into snapd master, the commit which marks notify unsupported should be reverted.

The PR configures the home interface to use the prompt in its rules if experimental.apparmor-prompting is enabled. In the future, this may be extended to other interfaces.

olivercalder avatar Mar 06 '24 21:03 olivercalder

Codecov Report

Attention: Patch coverage is 42.62295% with 35 lines in your changes are missing coverage. Please review.

Project coverage is 78.86%. Comparing base (32c0489) to head (4315839). Report is 9 commits behind head on master.

Files Patch % Lines
overlord/configstate/configcore/prompting.go 35.29% 20 Missing and 2 partials :warning:
overlord/ifacestate/handlers.go 0.00% 10 Missing :warning:
interfaces/builtin/home.go 66.66% 2 Missing and 1 partial :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #13670      +/-   ##
==========================================
- Coverage   78.88%   78.86%   -0.03%     
==========================================
  Files        1038     1040       +2     
  Lines      133499   133661     +162     
==========================================
+ Hits       105312   105410      +98     
- Misses      21614    21664      +50     
- Partials     6573     6587      +14     
Flag Coverage Δ
unittests 78.86% <42.62%> (-0.03%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Mar 06 '24 21:03 codecov-commenter

Rebased on master to pull in recent changes, notably those from https://github.com/snapcore/snapd/pull/13693

olivercalder avatar Apr 04 '24 21:04 olivercalder

@olivercalder I have a feeling that enabling the experimental feature should require a restart of snapd. Enabling the value is easy. Dynamically doing this adds a lot of complexity that seems unwarranted. I'm not sure how you see this but I would focus on quickly getting to the point where we can see if the core is right, not if the interactions around it are nice.

We can always add this before this is non-experimental.

zyga avatar Apr 05 '24 13:04 zyga

If flag is disabled on startup and enabled later: System key will be set with prompting disabled, so listener will not start, so profile generation should never include prompt prefix Importantly, we never want to include prompt prefix unless the listener is running, since otherwise application syscalls may block waiting for a prompt reply which will never arrive

@olivercalder what will restart snapd though when the flag is enabled in the UI?

pedronis avatar Apr 11 '24 18:04 pedronis

To be clear, my most recent comment chain is a proposal for an alternate approach, rather than a description of this PR. I am working on implementing that alternative approach in a separate, parallel PR, so these approaches can be compared.

Additionally, in the meantime, I've opened #13821 to implement the AppArmor kernel and parser support check, as that is common between both approaches and orthogonal to the open questions addressed by this and the forthcoming PR.

olivercalder avatar Apr 11 '24 22:04 olivercalder

Closing in favor of #13822

olivercalder avatar May 28 '24 15:05 olivercalder