Important is that the CUPS Snap can be installed on any Snap-supporting operating system, not only Ubuntu. So we must be careful with allowing the Snap to install files out of the sandbox (*.policy) or require certain files outside the sandbox (*.rules/*.pkla) this can restrict the Snap to Ubuntu and it also can be a similar situation as the (now deprecated) printer drivers being PPD files and filters which need to be in certain file system locations, a situation we want to avoid with the new printing and scanning architecture.
Perhaps we need to go the PulseAudio way.
I’ve often said (outside of this thread) that polkit integration would be a welcome addition to cupsd. It can then listen on its socket like normal and apply polkit policies to its APIs. Presumably, all things related to printing would be allowed and all things related to admin would require auth_admin/auth_admin_keep. This would be a great addition to cupsd irrespective of snapd.
With respect to snapd:
- it would be easy enough to allow the cupsd providing (slot) to talk to polkit via its PermanentSlotAppArmor policy
- there is no polkit daemon on Ubuntu Core devices
- the devil is in the details wrt the polkit policy. Today, access to admin functionality is limited to group membership. I strongly suspect that for a similar UX, distros (including Ubuntu) would recreate this under polkit and say “admin APIs are allowed if user is in the lpadmin group” to avoid bugs from users complaining about needing a password to configure some aspect of a printer when they never used to. Since single user desktop installs will default to this, we are back to where we started with snap applications running in the desktop environment are usually able to access admin APIs via the socket
- the polkit policy will likely need to allow non-console root processes without a password so snap daemons that plugs cups-control would have access to the admin APIs
- snapd doesn’t currently have a polkit backend for the snap to ship policy and put in place for the host’s polkit to use
- the patch to cupsd would be rather extensive in comparison to the ‘pulseaudio approach’ (even considering what @jamesh reminded us about that there is no API for querying snapd so cupsd would have to implement that)
Due to ‘2’ on Ubuntu Core devices and ‘4’ in general, cupsd would need the ‘pulseaudio approach’ anyway to mediate admin access for snaps
‘3’ is a real problem when accessing cupsd coming from a deb when that deb implements polkit policy that isn’t what we would want for snaps. The “pulseaudio approach” is likely needed for this as well. While snap-confine is in a position to drop “lpadmin” from the list of supplementary groups as part of startup, this is unlikely to scale out cross-distro since it is possible that different distros will use a different group for cupsd.
‘5’ requires not insignificant work to snapd (it would be generally useful though), but is inhibited by ‘2’ (we would want to consider retroactively adding polkit to UC16, UC18 and UC20 for this). As @till.kamppeter mentioned, that implementation would need to be careful to not step on existing host policy (though, in theory we could utilize the vendor or site policy mechanisms to override deb/rpm/ubuntu/distro policy…).
(It is also theoretically possible for the printing stack snap to ship polkitd, listening on something printing stack-specific in terms of DBus and have the snap have internal polkit policy that this snap-specific-polkitd would use and cupsd would query that. This still wouldn’t address ‘4’ and for systems with only the printing stack snap installed, UX regresses due to ‘3’. Not to mention, this is pretty terrible implementation-wise. 
)
In short, I would welcome polkit integration for cupsd, but it is unlikely to be sufficient to address all mediation points for snaps accessing the admin APIs. We’ll still need a “pulseaudio approach” IME, but just like @till.kamppeter said for polkit, this would be an optional build feature.
@jdstrand, thanks, I think that the PulseAudio way is the better one then.
@jamesh, could you do the PoC you suggsted for patching CUPS with the PulseAudio-like solution, like suggested in @jdstrand’s meeting summary and your answer? Thanks.
I’m not at all convinced that this is true. Either implementation is going to be doing roughly the same thing:
- do an
SO_PEERCREDlookup on the socket to determine the client uid and pid. - gather more information about that client from
/proc/$pid - for each request the client makes on the connection, check to see if it is privileged.
- for privileged operations, issue a request to a second daemon to check the client’s authorisation.
- when the second daemon’s response comes back, either serve the client’s request or return an error.
I’ve got a pretty full plate at the moment, and you’ve got a lot more experience with the CUPS source code than me. I’m happy to give you some pointers though.
That part of the code, sure, but the problem is with polkit, the APIs need to be carved up into polkit chunks that may or may not align with the current group checks. Even if they are, there is generating the xml and deciding on how the default policy should be written. This could be shortened to reimplement the group membership, but then the patch didn’t help our snap mediation needs.
@jamesh, @jdstrand, I have started the CUPS patch now (see below, is there really no way to add attachments in this forum?). I have found out that all authentication is done at a central place in cupsd, in the cupsdIsAuthorized() function in the file scheduler/auth.c.
To define which of the many IPP operations which CUPS supports are administrative operations (the ones which should only be allowed through the “cups-control” interface and not through the “cups” interface) I have taken the ones which are only allowed by the pseudo-group @SYSTEM in the policies in /etc/cups/cupsd.conf. This way I can simply check in the cupsdIsAuthorized() function whether the authorization is done through @SYSTEM and if so, call an extra function, which I have called cupsdCheckAdminTask() and only if this one tests positive, allow the operation.
In cupsdCheckAdminTask() I check whether the client connects via domain socket (network address family AF_LOCAL) and if not, the function simply passes. If the client connects through a domain socket I poll its peer credentials and so get the PID of the client process. After the line
/* Examine client process here */
one only needs to insert the checking for whether this process is from a Snap and whether this Snap plugs with “cups-control”.
Till
diff --git a/scheduler/auth.c b/scheduler/auth.c
index 4fbad6e24..466a0e529 100644
--- a/scheduler/auth.c
+++ b/scheduler/auth.c
@@ -43,6 +43,7 @@
# include <sys/ucred.h>
typedef struct xucred cupsd_ucred_t;
# define CUPSD_UCRED_UID(c) (c).cr_uid
+# define CUPSD_UCRED_PID(c) (c).cr_pid
#else
# ifndef __OpenBSD__
typedef struct ucred cupsd_ucred_t;
@@ -50,6 +51,7 @@ typedef struct ucred cupsd_ucred_t;
typedef struct sockpeercred cupsd_ucred_t;
# endif
# define CUPSD_UCRED_UID(c) (c).uid
+# define CUPSD_UCRED_PID(c) (c).pid
#endif /* HAVE_SYS_UCRED_H */
@@ -1520,6 +1522,61 @@ cupsdFreeLocation(cupsd_location_t *loc)/* I - Location to free */
}
+/*
+ * 'cupsdCheckAdminTask()' - Do additional checks on administrative tasks
+ */
+
+int /* O - 1 if admin task authorized */
+cupsdCheckAdminTask(cupsd_client_t *con) /* I - Connection */
+{
+ cupsdLogMessage(CUPSD_LOG_DEBUG,
+ "cupsdCheckAdminTask: ADMINISTRATIVE TASK!!");
+
+#if defined(SO_PEERCRED) && defined(AF_LOCAL)
+ /*
+ * Get the client's PID if it accesses locally via domain socket
+ */
+
+ if (httpAddrFamily(con->http->hostaddr) == AF_LOCAL)
+ {
+ cupsd_ucred_t peercred; /* Peer credentials */
+ socklen_t peersize; /* Size of peer credentials */
+ int client_pid; /* PID of client */
+
+ peersize = sizeof(peercred);
+
+# ifdef __APPLE__
+ if (getsockopt(httpGetFd(con->http), 0, LOCAL_PEERCRED, &peercred,
+ &peersize))
+# else
+ if (getsockopt(httpGetFd(con->http), SOL_SOCKET, SO_PEERCRED, &peercred,
+ &peersize))
+# endif /* __APPLE__ */
+ {
+ cupsdLogMessage(CUPSD_LOG_ERROR,
+ "cupsdCheckAdminTask: Unable to get peer credentials - %s",
+ strerror(errno));
+ }
+ else
+ {
+ cupsdLogMessage(CUPSD_LOG_DEBUG,
+ "cupsdCheckAdminTask: Client UID %d PID %d",
+ CUPSD_UCRED_UID(peercred),
+ CUPSD_UCRED_PID(peercred));
+ client_pid = CUPSD_UCRED_PID(peercred);
+
+ /* Examine client process here */
+ cupsdLogMessage(CUPSD_LOG_DEBUG,
+ "cupsdCheckAdminTask: Examining process %d ...",
+ client_pid);
+ }
+ }
+#endif /* SO_PEERCRED && AF_LOCAL */
+
+ return 1;
+}
+
+
/*
* 'cupsdIsAuthorized()' - Check to see if the user is authorized...
*/
@@ -1714,12 +1771,9 @@ cupsdIsAuthorized(cupsd_client_t *con, /* I - Connection */
/*
* OK, got a username. See if we need normal user access, or group
- * access... (root always matches)
+ * access...
*/
- if (!strcmp(username, "root"))
- return (HTTP_OK);
-
/*
* Strip any @domain or @KDC from the username and owner...
*/
@@ -1749,6 +1803,21 @@ cupsdIsAuthorized(cupsd_client_t *con, /* I - Connection */
else
pw = NULL;
+ /*
+ * For matching user and group memberships below we will first go
+ * through all names except @SYSTEM to authorize the task as
+ * non-administrative, like printing or deleting one's own job, if this
+ * fails we will check whether we can authorize via the special name
+ * @SYSTEM, as an administrative task, like creating a print queue or
+ * deleting someone else's job.
+ * Note that tasks are considered as administrative by the policies
+ * in cupsd.conf, when they require the user or group @SYSTEM.
+ * We do this separation because if the client is a Snap connecting via
+ * domain socket, we need to additionally check whether it plugs to us
+ * through the "cups-control" interface which allows administration and
+ * not through the "cups" interface which allows only printing.
+ */
+
if (best->level == CUPSD_AUTH_USER)
{
/*
@@ -1779,8 +1848,15 @@ cupsdIsAuthorized(cupsd_client_t *con, /* I - Connection */
{
if (!_cups_strncasecmp(name, "@AUTHKEY(", 9) && check_authref(con, name + 9))
return (HTTP_OK);
- else if (!_cups_strcasecmp(name, "@SYSTEM") && SystemGroupAuthKey &&
- check_authref(con, SystemGroupAuthKey))
+ }
+
+ for (name = (char *)cupsArrayFirst(best->names);
+ name;
+ name = (char *)cupsArrayNext(best->names))
+ {
+ if (!_cups_strcasecmp(name, "@SYSTEM") && SystemGroupAuthKey &&
+ check_authref(con, SystemGroupAuthKey) &&
+ cupsdCheckAdminTask(con))
return (HTTP_OK);
}
@@ -1797,9 +1873,8 @@ cupsdIsAuthorized(cupsd_client_t *con, /* I - Connection */
return (HTTP_OK);
else if (!_cups_strcasecmp(name, "@SYSTEM"))
{
- for (i = 0; i < NumSystemGroups; i ++)
- if (cupsdCheckGroup(username, pw, SystemGroups[i]))
- return (HTTP_OK);
+ /* Do @SYSTEM later, when every other entry fails */
+ continue;
}
else if (name[0] == '@')
{
@@ -1810,6 +1885,19 @@ cupsdIsAuthorized(cupsd_client_t *con, /* I - Connection */
return (HTTP_OK);
}
+ for (name = (char *)cupsArrayFirst(best->names);
+ name;
+ name = (char *)cupsArrayNext(best->names))
+ {
+ if (!_cups_strcasecmp(name, "@SYSTEM"))
+ {
+ for (i = 0; i < NumSystemGroups; i ++)
+ if (cupsdCheckGroup(username, pw, SystemGroups[i]) &&
+ cupsdCheckAdminTask(con))
+ return (HTTP_OK);
+ }
+ }
+
return (con->username[0] ? HTTP_FORBIDDEN : HTTP_UNAUTHORIZED);
}
@@ -1827,16 +1915,31 @@ cupsdIsAuthorized(cupsd_client_t *con, /* I - Connection */
name;
name = (char *)cupsArrayNext(best->names))
{
+ if (!_cups_strcasecmp(name, "@SYSTEM"))
+ {
+ /* Do @SYSTEM later, when every other entry fails */
+ continue;
+ }
+
cupsdLogMessage(CUPSD_LOG_DEBUG2, "cupsdIsAuthorized: Checking group \"%s\" membership...", name);
+ if (cupsdCheckGroup(username, pw, name))
+ return (HTTP_OK);
+ }
+
+ for (name = (char *)cupsArrayFirst(best->names);
+ name;
+ name = (char *)cupsArrayNext(best->names))
+ {
if (!_cups_strcasecmp(name, "@SYSTEM"))
{
+ cupsdLogMessage(CUPSD_LOG_DEBUG2, "cupsdIsAuthorized: Checking group \"%s\" membership...", name);
+
for (i = 0; i < NumSystemGroups; i ++)
- if (cupsdCheckGroup(username, pw, SystemGroups[i]))
+ if (cupsdCheckGroup(username, pw, SystemGroups[i]) &&
+ cupsdCheckAdminTask(con))
return (HTTP_OK);
}
- else if (cupsdCheckGroup(username, pw, name))
- return (HTTP_OK);
}
/*
Here is the CUPS patch development in progress:
The patch is the sum of the newest commits, currently only this commit which lets administrative operation requests from Snaps simply get denied.
1 Like
And here is a mini Snap for testing. Create a directory and put this snapcraft.yaml inside:
name: cups-admin-test
base: core18 # The base snap is the execution environment for this snap
version: 0.1.0
summary: CUPS admin and non-admin tasks out of a Snap
description: Testing interfaces for the CUPS Snap
grade: devel
confinement: strict
apps:
lpinfo:
command: lpinfo
plugs: [network, avahi-control, raw-usb, cups-control]
lpadmin:
command: lpadmin
plugs: [network, avahi-control, home, cups-control]
lpstat:
command: lpstat
plugs: [network, avahi-control, cups-control]
lpoptions:
command: lpoptions
plugs: [network, avahi-control, home, cups-control]
lp:
command: lp
plugs: [network, avahi-control, home, cups-control]
cancel:
command: cancel
plugs: [network, avahi-control, cups-control]
lpmove:
command: lpmove
plugs: [network, avahi-control, cups-control]
cupsenable:
command: cupsenable
plugs: [network, avahi-control, cups-control]
cupsdisable:
command: cupsdisable
plugs: [network, avahi-control, cups-control]
cupsaccept:
command: cupsaccept
plugs: [network, avahi-control, cups-control]
cupsreject:
command: cupsreject
plugs: [network, avahi-control, cups-control]
accept:
command: accept
plugs: [network, avahi-control, cups-control]
reject:
command: reject
plugs: [network, avahi-control, cups-control]
cupsctl:
command: cupsctl
plugs: [network, avahi-control, cups-control]
parts:
cups-client:
plugin: dump
source: .
stage-packages:
- cups-client
- libcups2
prime:
- usr/bin/*
- usr/sbin/*
- usr/lib/*
In this directory run:
snapcraft snap
sudo snap install --dangerous cups-admin-test_0.1.0_amd64.snap
sudo snap connect cups-admin-test:cups-control
This snap provides the command line tools of CUPS in a Snap, so you can run administrative tasks as follows:
cups-admin-test.lpadmin -p authtest -E -v /dev/null &
cups-admin-test.lpadmin -x authtest &
The & at the end makes the PID of the commands appear on the screen, so that you can compare in the CUPS error_log.
Non-administrative tasks are:
cups-admin-test.lpstat -v &
cups-admin-test.lp -d authtest ~/.bashrc &
If you try the patch on the system’s non-Snapped CUPS, run
sudo aa-complain cupsd
as libapparmor checks for the AppArmor profile reading /proc/PID/attr/current.
Also set LogLevel debug2 in cupsd.conf and search for lines containing cupsdCheckAdminTask: in /var/log/cups/error_log.
Quick way to test the patch with the system’s CUPS installation:
git clone git@github.com:tillkamppeter/cups.git
cd cups
autoconf
./configure --prefix=/usr --sysconfdir=/etc --localstatedir=/var
make
sudo aa-complain cupsd
sudo systemctl stop cups
sudo cp scheduler/cupsd /usr/sbin/
sudo systemctl start cups
Then run test commands, out of the Snap and from the system and see what works and what not.
Till
3 Likes
Now I have done the second part of the patch, actually granting permission based on the fact whether the client connects via “cups-control”, meaning that the we have now functionality equivalent to PulseAudio.
In contrary to PulseAudio we use synchronous functions of the snapd-glib library to make the code simpler, and in CUPS it seems to work. According to error_log the whole check happens in well less than a second, and when entering client commands one gets instant answers. Probably PulseAudio uses the asynchronous versions to assure continuity of sound playing (am I right @jamesh?).
The work still needed is improving the error handling, as any error on
the way makes the operation be allowed, destroying the connection to
snapd when done, to avoid memory leaks, and some code polishing.
@jdstrand, @jamesh, could you have a look into this, whether it is what we need and does not have some security problems which I perhaps have not seen?
Till
1 Like
Pulse Audio uses a single threaded event driven daemon, so direct calls to the snapd-glib synchronous API would have been a problem. Complicating matters, it doesn’t use the GLib main loop, so the async API couldn’t be called directly either. I could have performed synchronous calls in a thread pool, but opted to use the async API in another thread running the GLib main loop.
If cupsd is built on a “one connection per thread” model, then snapd-glib’s synchronous API is a reasonable choice.
In snapcraft.yaml I can define for each contained app individually, which interfaces it uses, so in my little test Snap I could let the lpadmin app use “cups-control”, but the lpstat app only use “cups” (assuming that our new CUPS interfaces are implemented in snapd), as the latter is supposed to never do administrative tasks.
Now when cupsd knows the PID of a client it can find out that it is from a Snap and from which Snap via libapparmor, and it could also find out the executable name from /proc/PID/comm. So with the executable name one can find out which app from the Snap was used.
But what I did not find out is can I poll vis snapd-glib which interfaces this app is plugging?
For now I can only determine which interfaces the whole Snap is plugging and so my patch grants admin access to inquiries from any app of the Snap, even if lpstat is used and it is only plugging “cups” and not “cups-control”.
@jdstrand, @ijohnson, @jamesh, does the snapd-glib API support this?
Correction: The name the app within the client Snap cupsd could also determine from the AppArmor profile name. The last part after the second dot at the end of the Snap name is the app name.
Completed the CUPS patch with two more commits: One to free data structures from snapd-glib after use and another to improve the error handling and the log messages.
Now errors in communication with snapd lead to a denial of access.
Note that I did not find a solution for my question in the previous posts here, so the patch only checks whether the client Snap plugs “cups-control” and not whether the individual app of the Snap plugs it.
My next step is to apply this patch to the CUPS snap. Then I will also rename the CUPS Snap.
@ijohnson, @jdstrand, so I think that now you could do the changes on the CUPS interfaces, to have “cups” for user tasks and “cups-control” for administrative tasks.
@till.kamppeter - I’ve added this to the list for 2.45 and I’m going to try to get to it this week. If you find yourself wanting to demo it before the snapd changes are committed, you can always install the snap with --devmode; it should still mediate clients connecting to it.
@jdstrand, thanks, is there some documentation about the specification of the new “cups” and “cups-control” interfaces? Can you also post a link to the commits/changes you have done on the snapd code to implement the new interfaces? Does installing snapd with “–devmode” automatically select the development snapshot with these interfaces?
I think Jamie was suggesting you install your ipp-printing snap in devmode, not the snapd snap. I don’t think anything has been added to snapd yet, and so in the meantime installing your snap in devmode will get around any denials your snap might run into, and allow you to test the patches to cups mediation.
1 Like
@jdstrand, @ijohnson, libapparmor used in the cupsd patch needs to access /proc/PID/attr/current. Is “system-observe” the correct interface for that?
@jamesh, how is PulseAudio doing that?
This and other accesses will need to be in the updates to the cups-control interface that I will make. For now, please use --devmode when installing your snap. I’ll post something here when there is something to test.
As for pulseaudio, the pulseaudio snap does not have the mediation patches and so the existing pulseaudio slot policy is sufficient. That policy will need updating if/when the pulseaudio snap is updated with the mediation patches.