When creating the system identity set the system token. The system
token is currently stored in a local path

   /var/run/libvirt/common/system.token

Obviously with only traditional UNIX DAC in effect, this is largely
security through obscurity, if the client is running at the same
privilege level as the daemon. It does, however, reliably distinguish
an unprivilegd client from the system daemons.

With a MAC system like SELinux though, or possible use of containers,
access can be further restricted.

A possible future improvement for Linux would be to populate the
kernel keyring with a secret for libvirt daemons to share.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/util/viridentity.c | 107 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/src/util/viridentity.c b/src/util/viridentity.c
index 7da4ea12f5..065db06e49 100644
--- a/src/util/viridentity.c
+++ b/src/util/viridentity.c
@@ -22,6 +22,7 @@
 #include <config.h>
 
 #include <unistd.h>
+#include <fcntl.h>
 #if WITH_SELINUX
 # include <selinux/selinux.h>
 #endif
@@ -32,11 +33,14 @@
 #include "viridentity.h"
 #include "virlog.h"
 #include "virobject.h"
+#include "virrandom.h"
 #include "virthread.h"
 #include "virutil.h"
 #include "virstring.h"
 #include "virprocess.h"
 #include "virtypedparam.h"
+#include "virfile.h"
+#include "configmake.h"
 
 #define VIR_FROM_THIS VIR_FROM_IDENTITY
 
@@ -54,7 +58,10 @@ struct _virIdentity {
 
 G_DEFINE_TYPE(virIdentity, vir_identity, G_TYPE_OBJECT)
 
+static char *virIdentityEnsureSystemToken(void);
+
 static virThreadLocal virIdentityCurrent;
+static char *systemToken;
 
 static void virIdentityFinalize(GObject *obj);
 
@@ -73,6 +80,9 @@ static int virIdentityOnceInit(void)
         return -1;
     }
 
+    if (!(systemToken = virIdentityEnsureSystemToken()))
+        return -1;
+
     return 0;
 }
 
@@ -144,6 +154,103 @@ int virIdentitySetCurrent(virIdentity *ident)
 }
 
 
+#define TOKEN_BYTES 16
+#define TOKEN_STRLEN (TOKEN_BYTES * 2)
+
+static char *
+virIdentityConstructSystemTokenPath(void)
+{
+    g_autofree char *commondir = NULL;
+    if (geteuid() == 0) {
+        commondir = g_strdup(RUNSTATEDIR "/libvirt/common");
+    } else {
+        g_autofree char *rundir = virGetUserRuntimeDirectory();
+        commondir = g_strdup_printf("%s/common", rundir);
+    }
+
+    if (g_mkdir_with_parents(commondir, 0700) < 0) {
+        virReportSystemError(errno,
+                             _("Cannot create daemon common directory '%s'"),
+                             commondir);
+        return NULL;
+    }
+
+    return g_strdup_printf("%s/system.token", commondir);
+}
+
+
+static char *
+virIdentityEnsureSystemToken(void)
+{
+    g_autofree char *tokenfile = virIdentityConstructSystemTokenPath();
+    g_autofree char *token = NULL;
+    int fd = -1;
+    struct stat st;
+
+    fd = open(tokenfile, O_RDWR|O_APPEND|O_CREAT, 0600);
+    if (fd < 0) {
+        virReportSystemError(errno,
+                             _("Unable to open system token %s"),
+                             tokenfile);
+        goto error;
+    }
+
+    if (virSetCloseExec(fd) < 0) {
+        virReportSystemError(errno,
+                             _("Failed to set close-on-exec flag '%s'"),
+                             tokenfile);
+        goto error;
+    }
+
+    if (virFileLock(fd, false, 0, 1, true) < 0) {
+        virReportSystemError(errno,
+                             _("Failed to lock system token '%s'"),
+                             tokenfile);
+        goto error;
+    }
+
+    if (fstat(fd, &st) < 0) {
+        virReportSystemError(errno,
+                             _("Failed to check system token '%s'"),
+                             tokenfile);
+        goto error;
+    }
+
+    /* Ok, we're the first one here, so we must populate it */
+    if (st.st_size == 0) {
+        if (!(token = virRandomToken(TOKEN_BYTES))) {
+            goto error;
+        }
+        if (safewrite(fd, token, TOKEN_STRLEN) != TOKEN_STRLEN) {
+            virReportSystemError(errno,
+                                 _("Failed to write system token '%s'"),
+                                 tokenfile);
+            goto error;
+        }
+    } else {
+        if (virFileReadLimFD(fd, TOKEN_STRLEN, &token) < 0) {
+            virReportSystemError(errno,
+                                 _("Failed to write system token '%s'"),
+                                 tokenfile);
+            goto error;
+        }
+        if (strlen(token) != TOKEN_STRLEN) {
+            virReportSystemError(errno,
+                                 _("System token in %s was corrupt"),
+                                 tokenfile);
+            goto error;
+        }
+    }
+
+    VIR_FORCE_CLOSE(fd);
+    return g_steal_pointer(&token);
+
+ error:
+    VIR_FORCE_CLOSE(fd);
+    return NULL;
+}
+
+
 /**
  * virIdentityGetSystem:
  *
-- 
2.31.1

On 5/4/21 7:43 PM, Daniel P. Berrangé wrote:
> When creating the system identity set the system token. The system
> token is currently stored in a local path
> 
>    /var/run/libvirt/common/system.token
> 
> Obviously with only traditional UNIX DAC in effect, this is largely
> security through obscurity, if the client is running at the same
> privilege level as the daemon. It does, however, reliably distinguish
> an unprivilegd client from the system daemons.
> 
> With a MAC system like SELinux though, or possible use of containers,
> access can be further restricted.
> 
> A possible future improvement for Linux would be to populate the
> kernel keyring with a secret for libvirt daemons to share.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/util/viridentity.c | 107 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 107 insertions(+)
> 
> diff --git a/src/util/viridentity.c b/src/util/viridentity.c
> index 7da4ea12f5..065db06e49 100644
> --- a/src/util/viridentity.c
> +++ b/src/util/viridentity.c
> @@ -22,6 +22,7 @@
>  #include <config.h>
>  
>  #include <unistd.h>
> +#include <fcntl.h>
>  #if WITH_SELINUX
>  # include <selinux/selinux.h>
>  #endif
> @@ -32,11 +33,14 @@
>  #include "viridentity.h"
>  #include "virlog.h"
>  #include "virobject.h"
> +#include "virrandom.h"
>  #include "virthread.h"
>  #include "virutil.h"
>  #include "virstring.h"
>  #include "virprocess.h"
>  #include "virtypedparam.h"
> +#include "virfile.h"
> +#include "configmake.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_IDENTITY
>  
> @@ -54,7 +58,10 @@ struct _virIdentity {
>  
>  G_DEFINE_TYPE(virIdentity, vir_identity, G_TYPE_OBJECT)
>  
> +static char *virIdentityEnsureSystemToken(void);
> +
>  static virThreadLocal virIdentityCurrent;
> +static char *systemToken;
>  
>  static void virIdentityFinalize(GObject *obj);
>  
> @@ -73,6 +80,9 @@ static int virIdentityOnceInit(void)
>          return -1;
>      }
>  
> +    if (!(systemToken = virIdentityEnsureSystemToken()))
> +        return -1;
> +
>      return 0;
>  }
>  
> @@ -144,6 +154,103 @@ int virIdentitySetCurrent(virIdentity *ident)
>  }
>  
>  
> +#define TOKEN_BYTES 16
> +#define TOKEN_STRLEN (TOKEN_BYTES * 2)
> +
> +static char *
> +virIdentityConstructSystemTokenPath(void)
> +{
> +    g_autofree char *commondir = NULL;
> +    if (geteuid() == 0) {
> +        commondir = g_strdup(RUNSTATEDIR "/libvirt/common");
> +    } else {
> +        g_autofree char *rundir = virGetUserRuntimeDirectory();
> +        commondir = g_strdup_printf("%s/common", rundir);
> +    }
> +
> +    if (g_mkdir_with_parents(commondir, 0700) < 0) {
> +        virReportSystemError(errno,
> +                             _("Cannot create daemon common directory '%s'"),
> +                             commondir);
> +        return NULL;
> +    }
> +
> +    return g_strdup_printf("%s/system.token", commondir);
> +}
> +
> +
> +static char *
> +virIdentityEnsureSystemToken(void)
> +{
> +    g_autofree char *tokenfile = virIdentityConstructSystemTokenPath();
> +    g_autofree char *token = NULL;
> +    int fd = -1;

Or VIR_AUTOCLOSE fd = -1 and drop those explicit close calls at the end.

> +    struct stat st;
> +
> +    fd = open(tokenfile, O_RDWR|O_APPEND|O_CREAT, 0600);
> +    if (fd < 0) {
> +        virReportSystemError(errno,
> +                             _("Unable to open system token %s"),
> +                             tokenfile);
> +        goto error;
> +    }
> +
> +    if (virSetCloseExec(fd) < 0) {

I know we have this pattern in many areas, but it's inherently racy.
What's stopping us from passing O_CLOEXEC to open()? IIUC, O_CLOEXEC
will exist on mingw where it's just an alias to O_NOINHERIT.

> +        virReportSystemError(errno,
> +                             _("Failed to set close-on-exec flag '%s'"),
> +                             tokenfile);
> +        goto error;
> +    }

Michal