[libvirt PATCH v2 03/10] util: generate a persistent system token

[Michal Prívozník]

On 5/7/21 6:24 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.
> 
> Reviewed-by: Michal Privoznik <mprivozn at redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>  src/util/viridentity.c | 102 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 102 insertions(+)
> 
> diff --git a/src/util/viridentity.c b/src/util/viridentity.c
> index 7da4ea12f5..8c939a507e 100644
> --- a/src/util/viridentity.c
> +++ b/src/util/viridentity.c

> @@ -144,6 +154,98 @@ 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;
> +    VIR_AUTOCLOSE fd = -1;
> +    struct stat st;
> +

Sorry for not spotting this in v1, but @tokenfile can be NULL here, in
which case virIdentityConstructSystemTokenPath() already reported
accurate error. Something like: if (!tokenfile) return NULL;  is sufficient.

> +    fd = open(tokenfile, O_RDWR|O_APPEND|O_CREAT, 0600);
> +    if (fd < 0) {
> +        virReportSystemError(errno,
> +                             _("Unable to open system token %s"),
> +                             tokenfile);
> +        return NULL;
> +    }
> +


Also, I believe you will want to mock this function, because if you
don't then the viridentitytest starts failing after next patch.
Something among these lines:

https://gitlab.com/MichalPrivoznik/libvirt/-/commit/5594631df3b3512adecea0f5a0056611b324fa5d

Michal