interfaces/cups: add cups-socket-directory attr, use to specify mount rules in backend #10427
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,17 @@ | |
|
|
||
| package builtin | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "strings" | ||
|
|
||
| "github.com/snapcore/snapd/interfaces" | ||
| "github.com/snapcore/snapd/interfaces/apparmor" | ||
| "github.com/snapcore/snapd/interfaces/mount" | ||
| "github.com/snapcore/snapd/osutil" | ||
| "github.com/snapcore/snapd/snap" | ||
| ) | ||
|
|
||
| // On systems where the slot is provided by an app snap, the cups interface is | ||
| // the companion interface to the cups-control interface. The design of these | ||
| // interfaces is based on the idea that the slot implementation (eg cupsd) is | ||
|
|
@@ -46,17 +57,162 @@ const cupsBaseDeclarationSlots = ` | |
| const cupsConnectedPlugAppArmor = ` | ||
| # Allow communicating with the cups server | ||
|
|
||
| # Do not allow reading the user or global client.conf for this snap, as this may | ||
| # allow a user to point an application at an unconfined cupsd which could be | ||
| # used to load printer drivers etc. We only want client snaps with the cups | ||
| # interface plug connected to be able to talk to a version of cupsd which is | ||
| # strictly confined and performs mediation. This means only allowing to talk to | ||
| # /var/cups/cups.sock and not /run/cups/cups.sock since snapd has no way to know | ||
| # if the latter cupsd is confined and performs mediation, but the upstream | ||
| # maintained cups snap providing a cups slot will always perform mediation. | ||
| # As such, do not use the <abstractions/cups-client> include file here. | ||
|
|
||
| # Allow reading the personal settings for cups like default printer, etc. | ||
| owner @{HOME}/.cups/lpoptions r, | ||
|
|
||
| /{,var/}run/cups/printcap r, | ||
|
|
||
| # Allow talking to the snap version of cupsd socket that we expose via bind | ||
| # mounts from a snap providing the cups slot to this snap. | ||
| /var/cups/cups.sock rw, | ||
| ` | ||
|
|
||
| type cupsInterface struct { | ||
| commonInterface | ||
| } | ||
|
|
||
| func (iface *cupsInterface) AppArmorConnectedSlot(spec *apparmor.Specification, plug *interfaces.ConnectedPlug, slot *interfaces.ConnectedSlot) error { | ||
| return nil | ||
| } | ||
|
|
||
| func validateCupsSocketDirAttr(a interfaces.Attrer, snapInfo *snap.Info) (string, error) { | ||
anonymouse64 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // Allow an empty specification for the slot, in which case we don't perform | ||
| // any mounts, etc. This is mainly to prevent errors in systems which still | ||
| // have the old cups snap installed that haven't been updated to use the new | ||
| // snap with the new slot declaration | ||
| if _, ok := a.Lookup("cups-socket-dir"); !ok { | ||
anonymouse64 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return "", nil | ||
| } | ||
|
|
||
| var cupsdSocketSourceDir string | ||
| if err := a.Attr("cups-socket-dir", &cupsdSocketSourceDir); err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| // make sure that the cups socket dir is not an AppArmor Regular expression | ||
| if err := apparmor.ValidateNoAppArmorRegexp(cupsdSocketSourceDir); err != nil { | ||
| return "", fmt.Errorf("cups-socket-dir is not usable: %v", err) | ||
| } | ||
|
|
||
| if !cleanSubPath(cupsdSocketSourceDir) { | ||
anonymouse64 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return "", fmt.Errorf("cups-socket-dir is not clean: %q", cupsdSocketSourceDir) | ||
anonymouse64 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| // validate that the setting for cups-socket-dir is in $SNAP_DATA or | ||
| // $SNAP_COMMON, we don't allow any other directories for the slot socket | ||
| // dir | ||
| // TODO: should we also allow /run/$SNAP_INSTANCE_NAME/ too ? | ||
| if !strings.HasPrefix(cupsdSocketSourceDir, "$SNAP_COMMON") && !strings.HasPrefix(cupsdSocketSourceDir, "$SNAP_DATA") { | ||
| return "", fmt.Errorf("cups-socket-dir must be a directory of $SNAP_COMMON or $SNAP_DATA") | ||
| } | ||
| // otherwise it must have a prefix of either SNAP_COMMON or SNAP_DATA, | ||
| // validate that it has no other variables in it | ||
| err := snap.ValidatePathVariables(cupsdSocketSourceDir) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| // The path starts with $ and ValidatePathVariables() ensures | ||
| // path contains only $SNAP, $SNAP_DATA, $SNAP_COMMON, and no | ||
| // other $VARs are present. It is ok to use | ||
| // ExpandSnapVariables() since it only expands $SNAP, $SNAP_DATA | ||
| // and $SNAP_COMMON | ||
| return snapInfo.ExpandSnapVariables(cupsdSocketSourceDir), nil | ||
| } | ||
|
|
||
| func (iface *cupsInterface) BeforePrepareSlot(slot *snap.SlotInfo) error { | ||
| // verify that the snap has a cups-socket-dir interface attribute, which is | ||
| // needed to identify where to find the cups socket is located in the snap | ||
| // providing the cups socket | ||
| _, err := validateCupsSocketDirAttr(slot, slot.Snap) | ||
| return err | ||
| } | ||
|
|
||
| func (iface *cupsInterface) AppArmorConnectedPlug(spec *apparmor.Specification, plug *interfaces.ConnectedPlug, slot *interfaces.ConnectedSlot) error { | ||
| cupsdSocketSourceDir, err := validateCupsSocketDirAttr(slot, slot.Snap()) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // add the base snippet | ||
| spec.AddSnippet(cupsConnectedPlugAppArmor) | ||
|
|
||
| if cupsdSocketSourceDir == "" { | ||
| // no other rules, this is the legacy slot without the additional | ||
| // attribute | ||
| return nil | ||
| } | ||
|
|
||
| // add rules to access the socket dir from the slot location directly | ||
| // this is necessary otherwise clients get denials like this: | ||
| // apparmor="DENIED" operation="connect" | ||
| // profile="snap.test-snapd-cups-consumer.bin" | ||
| // name="/var/snap/test-snapd-cups-provider/common/cups.sock" | ||
| // pid=3195747 comm="nc" requested_mask="wr" denied_mask="wr" fsuid=0 ouid=0 | ||
| // this denial is the same that would happen for the content interface, so | ||
| // we employ the same workaround from the content interface here too | ||
| spec.AddSnippet(fmt.Sprintf(` | ||
| # 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. | ||
| "%s/**" mrwklix,`, cupsdSocketSourceDir)) | ||
|
|
||
| // setup the snap-update-ns rules for bind mounting for the plugging snap | ||
| emit := spec.AddUpdateNSf | ||
|
|
||
| emit(" # Mount cupsd socket from cups snap to client snap\n") | ||
| // note the trailing "/" is needed - we ensured that cupsdSocketSourceDir is | ||
| // clean when we validated it, so it will not have a trailing "/" so we are | ||
| // safe to add this here | ||
| emit(" mount options=(rw bind) \"%s/\" -> /var/cups/,\n", cupsdSocketSourceDir) | ||
| emit(" umount /var/cups/,\n") | ||
|
|
||
| apparmor.GenWritableProfile(emit, cupsdSocketSourceDir, 1) | ||
| apparmor.GenWritableProfile(emit, "/var/cups", 1) | ||
anonymouse64 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (iface *cupsInterface) MountConnectedPlug(spec *mount.Specification, plug *interfaces.ConnectedPlug, slot *interfaces.ConnectedSlot) error { | ||
anonymouse64 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| cupsdSocketSourceDir, err := validateCupsSocketDirAttr(slot, slot.Snap()) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if cupsdSocketSourceDir == "" { | ||
| // no other rules, this is the legacy slot without the additional | ||
| // attribute | ||
| return nil | ||
| } | ||
|
|
||
| // add a bind mount of the cups-socket-dir to /var/cups of the plugging snap | ||
| return spec.AddMountEntry(osutil.MountEntry{ | ||
| Name: cupsdSocketSourceDir, | ||
| Dir: "/var/cups/", | ||
|
There was a problem hiding this comment. this requires a mimic for /var, not sure what are the consequences of that in terms namespace updating bugs. is there a different place we could put this that doesn't have this sort of side-effect? There was a problem hiding this comment. From the CUPS point of view the socket can be in any writable space on the system. A client uses what There was a problem hiding this comment. @pedronis if we want the directory to be named "cups" with any prefix or suffix, then we cannot avoid creating a mimic entirely. If we wanted to avoid creating the mimic on /var/, we could go with /var/lib/cups instead, which would just create a mimic on /var/lib/, but that does involve /var/lib/snapd/ now being on a mimic (which I suppose is fine, snap apps from inside the namespace shouldn't need to care about anything in /var/lib/snapd), otherwise we could do something like /var/lib/misc/cups which would create a mimic over /var/lib/misc, but there is nothing inside that dir with core20 at least so the impact should be minimal. In terms of avoiding the namespace updating bugs, I will look into that situation, I don't remember off the top of my head if it is better or worse for this to be in a deeper directory or not, I think it depends on ordering to make sure that directory mimics higher up get processed before deeper ones which my other PR would take care of I think. There was a problem hiding this comment. We could also update There was a problem hiding this comment. Personally I think the suggestion to add an empty /var/cups directory to base snaps would be best, I'm not sure there is a better directory that avoids creating a mimic entirely and still allows us to have I don't think it's better personally to go down the route of opening up There was a problem hiding this comment. For me it is not important whether the socket directory is actually in So depends on file system conventions and also on convenience where it is easier to place the socket directory. There was a problem hiding this comment. do we have a plan now to add those empty dirs to the base snaps? There was a problem hiding this comment. Yes if there are no objections I will propose PR's to the base snaps to add /var/cups to the base snaps as empty dirs which will avoid unnecessary writable mimics, but IMHO that's an internal optimization that doesn't need to be done before this PR has landed |
||
| Options: []string{"bind", "rw"}, | ||
| }) | ||
anonymouse64 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| func init() { | ||
| registerIface(&cupsInterface{ | ||
| commonInterface: commonInterface{ | ||
| name: "cups", | ||
| summary: cupsSummary, | ||
| implicitOnCore: false, | ||
| implicitOnClassic: false, | ||
| baseDeclarationSlots: cupsBaseDeclarationSlots, | ||
| }, | ||
| }) | ||
| } | ||
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.
It's a shame we couldn't get this working with
/run/cupsas the directory exposed to the snap. Although it is not clear it'd be better to special case the trespassing code to allow creating/run/cupswithout a mimic if it doesn't exist.