-
Notifications
You must be signed in to change notification settings - Fork 582
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/b: allow ro when allowing rw for mount-control #14543
i/b: allow ro when allowing rw for mount-control #14543
Conversation
AppArmor has fixed CVE-2016-1585 which made mount rules more strict. This has caused a regression where snaps relying on the old behavior stopped working. One failure mode we are aware of is when the snap declares "rw" mounts in the mount-control plug but uses "ro" mount in practice. In this case we can reasonably emit an equivalent "ro" permission when "rw" permissions are already granted. Jira: https://warthogs.atlassian.net/browse/SNAPDENG-32513 Signed-off-by: Zygmunt Krynicki <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #14543 +/- ##
==========================================
+ Coverage 78.85% 78.87% +0.01%
==========================================
Files 1079 1080 +1
Lines 145615 145837 +222
==========================================
+ Hits 114828 115025 +197
- Misses 23601 23618 +17
- Partials 7186 7194 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Deploying a new |
We don't quite know the extent of snaps that are affected and we'd have to communicate to all the users. At some scale deploying a fix on the side of snapd might be easier. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed privately, I am in two minds about this change - I kind of think if we want this behaviour (rw
implicitly grants use of ro
) then either we should expect this to be done in AppArmor itself - OR we go ask snap publishers to update their snaps. I feel like trying to do this in snapd and grant extra permissions that are not explicitly requested is a bit odd and has the potential for confusion etc, so whilst I understand introducing a regression is not ideal, I think it is cleaner to not introduce such a workaround/hack here. @jrjohansen I'd be keen to know what you think about this too from the AppArmor side.
I understand the sentiment. I'll wait for JJ's opinion as I have no strong preference either way. CC @ernestl for scheduling awareness. |
I'm closing this in favour of a different approach that will effectively allow everyone to request both ro and rw permissions without a conflict. |
AppArmor has fixed CVE-2016-1585 which made mount rules more strict. This has caused a regression where snaps relying on the old behavior stopped working.
One failure mode we are aware of is when the snap declares "rw" mounts in the mount-control plug but uses "ro" mount in practice. In this case we can reasonably emit an equivalent "ro" permission when "rw" permissions are already granted.
Jira: https://warthogs.atlassian.net/browse/SNAPDENG-32513