cmd/snap-confine: allow content interface mounts #2462
Conversation
Signed-off-by: Zygmunt Krynicki <[email protected]>
Signed-off-by: Zygmunt Krynicki <[email protected]>
|
Nice! |
Signed-off-by: Zygmunt Krynicki <[email protected]>
Signed-off-by: Zygmunt Krynicki <[email protected]>
| mount options=(ro bind) /snap/*/** -> /snap/*/*/**, | ||
| mount options=(ro bind) /snap/*/** -> /var/snap/*/*/**, |
There was a problem hiding this comment.
The comment above doesn't describe this new rule (only the other two). Please adjust and LGTM
Signed-off-by: Zygmunt Krynicki <[email protected]>
Signed-off-by: Zygmunt Krynicki <[email protected]>
Signed-off-by: Zygmunt Krynicki <[email protected]>
Signed-off-by: Zygmunt Krynicki <[email protected]>
Signed-off-by: Zygmunt Krynicki <[email protected]>
Signed-off-by: Zygmunt Krynicki <[email protected]>
Signed-off-by: Zygmunt Krynicki <[email protected]>
| # snaps may directly access the slot implementation's files | ||
| # read-only. | ||
| /var/snap/%s/%s/** mrkix, | ||
| `, slot.Snap.Name, slot.Snap.Revision) |
There was a problem hiding this comment.
These rules are fine since they only grant the same access to the same files that the bind SecurityMount backend provides, except it allows named sockets to work predictably in the same way.
The 'write' is only needed for named sockets, but thank you for adding the 'read' since it will avoid an inconsistency that may trip people up down the line if people decide to use the slot implementation's path directly.
All that said, the /var/snap/%s/%s used in the AppArmor rule is exactly the same as what mountEntry gives for source, with '/** ...' appended. Please update/use resolveSpecialVariable() so that we have AppArmor rules that precisely match the src mount.
Signed-off-by: Zygmunt Krynicki <[email protected]>
| # snaps may directly access the slot implementation's files. Due | ||
| # to a limitation in the kernel's LSM hooks for AF_UNIX, this rule | ||
| # is needed for using named sockets within the exported | ||
| # directory. |
There was a problem hiding this comment.
Since this was reorganized the comment isn't quite right for policy auditors. Please adjust to:
# In addition to the bind mount, add any AppArmor rules so that
# snaps may directly access the slot implementation's files. Due
# to a limitation in the kernel's LSM hooks for AF_UNIX, these
# are needed for using named sockets within the exported
# directory.
| fmt.Fprintf(contentSnippet, ` | ||
| # In addition to the bind mount, add an AppArmor rule so that | ||
| # snaps may directly access the slot implementation's files | ||
| # read-only. |
There was a problem hiding this comment.
Same here:
# In addition to the bind mount, add any AppArmor rules so that
# snaps may directly access the slot implementation's files
# read-only.
Signed-off-by: Zygmunt Krynicki <[email protected]>
|
👍 |
|
It would be nice to have a spread test for this to ensure the actual use case covered by this (sharing a unix domain socket) works well and doesn't break. |
This branch fixes writable content sharing, there are two parts to this:
The confinement profile for
snap-confineis changed to allow to perform more of the actionsthat that the content interface allows to express. This specifically allows $SNAP_DATA,
$SNAP_COMMON and $SNAP to be shared as long as both source and destination share
permissions (can be both read or both written). In addition, a snap developer can choose to share
a naturally writable location in a read only way (e.g. a read-only view of $SNAP_DATA)
The generated apparmor profile is changed to work around Linux Security Modules design where
bind mounting a socket doesn't change how apparmor interprets the path (apparmor still sees
the original path).
Signed-off-by: Zygmunt Krynicki [email protected]