Re: [libvirt] [PATCH v3 43/48] api: introduce virConnectSetIdentity for pasing uid, gid, selinux info
On Mon, 2019-07-29 at 18:11 +0100, Daniel P. Berrangé wrote:
api: introduce virConnectSetIdentity for pasing uid, gid, selinux
info
s/pasing/passing/
When using the fine grained access control mechanism for APIs, when
a
client connects to libvirtd, it will fetch the uid, gid, selinux
info of the remote client on the UNIX domain socket. This is then used
as the identity when checking ACLs.
s/it will/the latter will/
With the new split daemons things are a bit more complicated. The
user
can connect to virtproxyd, which in turn connects to virtqemud. When
virtqemud requests the identity over the UNIX domain socket, it will
get the identity that the virtproxyd is running as, not the identity of
the real end user/application.
s/the virtproxyd/virtproxyd/
virproxyd knows what the real identity is, and needs to be able to
forward this information to virtqemud. The virConnectSetIdentity API
provides a mechanism for doing this. Obviously virtqemud should not
accept such identity overrides from any client, it must only honour it
from a trusted client, aka one running as the same uid/gid as itself.
I've been trying to figure out where the very reasonable check you
describe is performed, either here or later in the series, but I have
to admit that I haven't been able to find it. Can you please point me
in the right direction?
The typed parameters exposed in the API are the same as those
currently
supported by the internal virIdentity class.
... although with slightly different names...
+++ b/include/libvirt/libvirt-host.h
+/**
+ * VIR_CONNECT_IDENTITY_OS_USER_NAME:
+ *
+ * The operating system user name as VIR_TYPED_PARAM_STRING
+ */
+# define VIR_CONNECT_IDENTITY_OS_USER_NAME "os-user-name"
The documentation should end with a period. Same for all following
values.
+/**
+ * VIR_CONNECT_IDENTITY_OS_PROCESS_ID:
+ *
+ * The operating system user ID as VIR_TYPED_PARAM_LLONG
+ */
+# define VIR_CONNECT_IDENTITY_OS_PROCESS_ID "os-process-id"
Welp, looks like you've copy-pasted the same documentation over
and over again! Please fix that :)
Anyway, shouldn't this be VIR_TYPED_PARAM_ULLONG as well? Can pids
be negative?
Looking at the code that you're replacing with patch 46, it uses
virStrToLong_i() to parse uid and gid and virStrToLong_ull() to
parse pid, so if anything the VIR_TYPED_PARAM_* should be the other
way around? But it seems to me like we really want all of these to
be VIR_TYPED_PARAM_ULLONGs.
+/**
+ * VIR_CONNECT_IDENTITY_SELINUX_CONTEXT:
+ *
+ * The application's SELinux context as VIR_TYPED_PARAM_STRING
+ *
+ */
+# define VIR_CONNECT_IDENTITY_SELINUX_CONTEXT "selinux-context"
Spurious empty line in the documentation.
+++ b/src/libvirt_public.syms
LIBVIRT_5.6.0 {
global:
+ virConnectSetIdentity;
virDomainCheckpointCreateXML;
virDomainCheckpointDelete;
virDomainCheckpointFree;
Yeah, about that...
Overall the patch looks good.
--
Andrea Bolognani / Red Hat / Virtualization