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@xxxxxxxxxx>
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> ---
> 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
