From nobody Sun Feb 8 18:48:54 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 216.205.24.124 as permitted sender) client-ip=216.205.24.124; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-124.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 216.205.24.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1620150260; cv=none; d=zohomail.com; s=zohoarc; b=Ei4uVkqTbjPTm8puyjXJSmKNfIqxuSHosZOMyeq9uqS3xYlkyfcHoHR1csbpQnDq05rQMJivo1ETVMg0LV5SfeXOu+WkHA+eMxD/QbwUulD+5jyP66L/BWew/CZbHZyUbIPY7+bvv3p5nYMiRzaDIIIGGGbVG5vfV1cy4AuwInM= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1620150260; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=jYj3mJ2k6tU3l+vm3ZYWUYtagymlEi+wRhE8ZQxgZ2s=; b=d+H/nJQYSG9f3J3rNAMttfaz0Hd8FLpQIYUDGI9Bht5rMjiC72qnPd+yTn94w6RrXOLRT70nkZC8+Re+NEKIsrGrSpwVcX+hBRlbIm3KbOWABnb8okuyrYvDvGjRMH2pzj1/nBeW9HBurwPG2Z6CWQeiInR4VGwgBkEVGARz1/I= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 216.205.24.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.zohomail.com with SMTPS id 1620150260705439.4742497879489; Tue, 4 May 2021 10:44:20 -0700 (PDT) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-97-oNGNfxCPMzqZ6wre-sG8Eg-1; Tue, 04 May 2021 13:44:16 -0400 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id E3F99824FA9; Tue, 4 May 2021 17:44:09 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C1DF5BA6F; Tue, 4 May 2021 17:44:09 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 87DEA55352; Tue, 4 May 2021 17:44:09 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 144Hi57E003950 for ; Tue, 4 May 2021 13:44:05 -0400 Received: by smtp.corp.redhat.com (Postfix) id 025975B4A6; Tue, 4 May 2021 17:44:05 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-113-37.ams2.redhat.com [10.36.113.37]) by smtp.corp.redhat.com (Postfix) with ESMTP id 104BF1964B; Tue, 4 May 2021 17:44:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1620150259; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=jYj3mJ2k6tU3l+vm3ZYWUYtagymlEi+wRhE8ZQxgZ2s=; b=LBtazquMNWNqv7oLq2zrstoTes5zLyog3z9Tm0norqFYSeloE9Dp90wYylpL/hNjdXG8Ak uMZkomS1d2VpKgNBKm3kcqsi8sj7suUvkEDde4qwPi1aGrCKuBQtXl/ognep5hIKr/wVKN 9ju553Zbor4P0Z1WFCng61iTO0aoBew= X-MC-Unique: oNGNfxCPMzqZ6wre-sG8Eg-1 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= To: libvir-list@redhat.com Subject: [libvirt PATCH 8/9] secret: rework handling of private secrets Date: Tue, 4 May 2021 18:43:49 +0100 Message-Id: <20210504174350.488942-9-berrange@redhat.com> In-Reply-To: <20210504174350.488942-1-berrange@redhat.com> References: <20210504174350.488942-1-berrange@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-loop: libvir-list@redhat.com X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) A secret can be marked with the "private" attribute. The intent was that it is not possible for any libvirt client to be able to read the secret value, it would only be accesible from within libvirtd. eg the QEMU driver can read the value to launch a guest. With the modular daemons, the QEMU, storage and secret drivers are all running in separate daemons. The QEMU and storage drivers thus appear to be normal libvirt client's from the POV of the secret driver, and thus they are not able to read a private secret. This is unhelpful. With the previous patches that introduced a "system token" to the identity object, we can now distinguish APIs invoked by libvirt daemons from those invoked by client applications. Reviewed-by: Daniel P. Berrang=C3=A9 --- src/driver-secret.h | 9 +-------- src/libvirt-secret.c | 2 +- src/remote/remote_driver.c | 8 +------- src/secret/secret_driver.c | 34 +++++++++++++++++++++++++++------- src/util/virsecret.c | 3 +-- tests/qemuxml2argvtest.c | 3 +-- 6 files changed, 32 insertions(+), 27 deletions(-) diff --git a/src/driver-secret.h b/src/driver-secret.h index eb6e82478c..1d21f62bb3 100644 --- a/src/driver-secret.h +++ b/src/driver-secret.h @@ -24,12 +24,6 @@ # error "Don't include this file directly, only use driver.h" #endif =20 -enum { - /* This getValue call is inside libvirt, override the "private" flag. - This flag cannot be set by outside callers. */ - VIR_SECRET_GET_VALUE_INTERNAL_CALL =3D 1 << 0, -}; - typedef virSecretPtr (*virDrvSecretLookupByUUID)(virConnectPtr conn, const unsigned char *uuid); @@ -57,8 +51,7 @@ typedef int typedef unsigned char * (*virDrvSecretGetValue)(virSecretPtr secret, size_t *value_size, - unsigned int flags, - unsigned int internalFlags); + unsigned int flags); =20 typedef int (*virDrvSecretUndefine)(virSecretPtr secret); diff --git a/src/libvirt-secret.c b/src/libvirt-secret.c index 75d40f53dc..a427805c7a 100644 --- a/src/libvirt-secret.c +++ b/src/libvirt-secret.c @@ -585,7 +585,7 @@ virSecretGetValue(virSecretPtr secret, size_t *value_si= ze, unsigned int flags) if (conn->secretDriver !=3D NULL && conn->secretDriver->secretGetValue= !=3D NULL) { unsigned char *ret; =20 - ret =3D conn->secretDriver->secretGetValue(secret, value_size, fla= gs, 0); + ret =3D conn->secretDriver->secretGetValue(secret, value_size, fla= gs); if (ret =3D=3D NULL) goto error; return ret; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 0c72d69933..eed99af127 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -5382,7 +5382,7 @@ remoteDomainBuildQemuMonitorEvent(virNetClientProgram= *prog G_GNUC_UNUSED, =20 static unsigned char * remoteSecretGetValue(virSecretPtr secret, size_t *value_size, - unsigned int flags, unsigned int internalFlags) + unsigned int flags) { unsigned char *rv =3D NULL; remote_secret_get_value_args args; @@ -5391,12 +5391,6 @@ remoteSecretGetValue(virSecretPtr secret, size_t *va= lue_size, =20 remoteDriverLock(priv); =20 - /* internalFlags intentionally do not go over the wire */ - if (internalFlags) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("no internalFlags s= upport")); - goto done; - } - make_nonnull_secret(&args.secret, secret); args.flags =3D flags; =20 diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index 6ea8cc8ce9..d2175de8ed 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -36,6 +36,7 @@ #include "viruuid.h" #include "virerror.h" #include "virfile.h" +#include "viridentity.h" #include "virpidfile.h" #include "configmake.h" #include "virstring.h" @@ -352,8 +353,7 @@ secretSetValue(virSecretPtr secret, static unsigned char * secretGetValue(virSecretPtr secret, size_t *value_size, - unsigned int flags, - unsigned int internalFlags) + unsigned int flags) { unsigned char *ret =3D NULL; virSecretObj *obj; @@ -368,11 +368,31 @@ secretGetValue(virSecretPtr secret, if (virSecretGetValueEnsureACL(secret->conn, def) < 0) goto cleanup; =20 - if ((internalFlags & VIR_SECRET_GET_VALUE_INTERNAL_CALL) =3D=3D 0 && - def->isprivate) { - virReportError(VIR_ERR_INVALID_SECRET, "%s", - _("secret is private")); - goto cleanup; + /* + * For historical compat we want to deny access to + * private secrets, even if no ACL driver is + * present. + * + * We need to validate the identity requesting + * the secret value is running as the same user + * credentials as this driver. + * + * ie a non-root libvirt client should not be + * able to request the value from privileged + * libvirt driver. + * + * To apply restrictions to processes running under + * the same user account is out of scope. + */ + if (def->isprivate) { + int rv =3D virIdentityIsCurrentElevated(); + if (rv < 0) + goto cleanup; + if (rv =3D=3D 0) { + virReportError(VIR_ERR_INVALID_SECRET, "%s", + _("secret is private")); + goto cleanup; + } } =20 if (!(ret =3D virSecretObjGetValue(obj))) diff --git a/src/util/virsecret.c b/src/util/virsecret.c index 0695288229..604d900f77 100644 --- a/src/util/virsecret.c +++ b/src/util/virsecret.c @@ -174,8 +174,7 @@ virSecretGetSecretString(virConnectPtr conn, goto cleanup; } =20 - *secret =3D conn->secretDriver->secretGetValue(sec, secret_size, 0, - VIR_SECRET_GET_VALUE_INTE= RNAL_CALL); + *secret =3D conn->secretDriver->secretGetValue(sec, secret_size, 0); =20 if (!*secret) goto cleanup; diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index f0efe98d7e..2cd8534b47 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -42,8 +42,7 @@ static virQEMUDriver driver; static unsigned char * fakeSecretGetValue(virSecretPtr obj G_GNUC_UNUSED, size_t *value_size, - unsigned int fakeflags G_GNUC_UNUSED, - unsigned int internalFlags G_GNUC_UNUSED) + unsigned int fakeflags G_GNUC_UNUSED) { char *secret; secret =3D g_strdup("AQCVn5hO6HzFAhAAq0NCv8jtJcIcE+HOBlMQ1A"); --=20 2.31.1