1
0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-22 17:34:18 +03:00

secret: rework handling of private secrets

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: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This commit is contained in:
Daniel P. Berrangé 2020-09-16 15:47:13 +01:00
parent 9bcbdbd579
commit 8f390ae310
6 changed files with 32 additions and 27 deletions

View File

@ -24,12 +24,6 @@
# error "Don't include this file directly, only use driver.h" # error "Don't include this file directly, only use driver.h"
#endif #endif
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 = 1 << 0,
};
typedef virSecretPtr typedef virSecretPtr
(*virDrvSecretLookupByUUID)(virConnectPtr conn, (*virDrvSecretLookupByUUID)(virConnectPtr conn,
const unsigned char *uuid); const unsigned char *uuid);
@ -57,8 +51,7 @@ typedef int
typedef unsigned char * typedef unsigned char *
(*virDrvSecretGetValue)(virSecretPtr secret, (*virDrvSecretGetValue)(virSecretPtr secret,
size_t *value_size, size_t *value_size,
unsigned int flags, unsigned int flags);
unsigned int internalFlags);
typedef int typedef int
(*virDrvSecretUndefine)(virSecretPtr secret); (*virDrvSecretUndefine)(virSecretPtr secret);

View File

@ -585,7 +585,7 @@ virSecretGetValue(virSecretPtr secret, size_t *value_size, unsigned int flags)
if (conn->secretDriver != NULL && conn->secretDriver->secretGetValue != NULL) { if (conn->secretDriver != NULL && conn->secretDriver->secretGetValue != NULL) {
unsigned char *ret; unsigned char *ret;
ret = conn->secretDriver->secretGetValue(secret, value_size, flags, 0); ret = conn->secretDriver->secretGetValue(secret, value_size, flags);
if (ret == NULL) if (ret == NULL)
goto error; goto error;
return ret; return ret;

View File

@ -5382,7 +5382,7 @@ remoteDomainBuildQemuMonitorEvent(virNetClientProgram *prog G_GNUC_UNUSED,
static unsigned char * static unsigned char *
remoteSecretGetValue(virSecretPtr secret, size_t *value_size, remoteSecretGetValue(virSecretPtr secret, size_t *value_size,
unsigned int flags, unsigned int internalFlags) unsigned int flags)
{ {
unsigned char *rv = NULL; unsigned char *rv = NULL;
remote_secret_get_value_args args; remote_secret_get_value_args args;
@ -5391,12 +5391,6 @@ remoteSecretGetValue(virSecretPtr secret, size_t *value_size,
remoteDriverLock(priv); remoteDriverLock(priv);
/* internalFlags intentionally do not go over the wire */
if (internalFlags) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("no internalFlags support"));
goto done;
}
make_nonnull_secret(&args.secret, secret); make_nonnull_secret(&args.secret, secret);
args.flags = flags; args.flags = flags;

View File

@ -36,6 +36,7 @@
#include "viruuid.h" #include "viruuid.h"
#include "virerror.h" #include "virerror.h"
#include "virfile.h" #include "virfile.h"
#include "viridentity.h"
#include "virpidfile.h" #include "virpidfile.h"
#include "configmake.h" #include "configmake.h"
#include "virstring.h" #include "virstring.h"
@ -352,8 +353,7 @@ secretSetValue(virSecretPtr secret,
static unsigned char * static unsigned char *
secretGetValue(virSecretPtr secret, secretGetValue(virSecretPtr secret,
size_t *value_size, size_t *value_size,
unsigned int flags, unsigned int flags)
unsigned int internalFlags)
{ {
unsigned char *ret = NULL; unsigned char *ret = NULL;
virSecretObj *obj; virSecretObj *obj;
@ -368,11 +368,31 @@ secretGetValue(virSecretPtr secret,
if (virSecretGetValueEnsureACL(secret->conn, def) < 0) if (virSecretGetValueEnsureACL(secret->conn, def) < 0)
goto cleanup; goto cleanup;
if ((internalFlags & VIR_SECRET_GET_VALUE_INTERNAL_CALL) == 0 && /*
def->isprivate) { * For historical compat we want to deny access to
virReportError(VIR_ERR_INVALID_SECRET, "%s", * private secrets, even if no ACL driver is
_("secret is private")); * present.
goto cleanup; *
* 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 = virIdentityIsCurrentElevated();
if (rv < 0)
goto cleanup;
if (rv == 0) {
virReportError(VIR_ERR_INVALID_SECRET, "%s",
_("secret is private"));
goto cleanup;
}
} }
if (!(ret = virSecretObjGetValue(obj))) if (!(ret = virSecretObjGetValue(obj)))

View File

@ -174,8 +174,7 @@ virSecretGetSecretString(virConnectPtr conn,
goto cleanup; goto cleanup;
} }
*secret = conn->secretDriver->secretGetValue(sec, secret_size, 0, *secret = conn->secretDriver->secretGetValue(sec, secret_size, 0);
VIR_SECRET_GET_VALUE_INTERNAL_CALL);
if (!*secret) if (!*secret)
goto cleanup; goto cleanup;

View File

@ -43,8 +43,7 @@ static virQEMUDriver driver;
static unsigned char * static unsigned char *
fakeSecretGetValue(virSecretPtr obj G_GNUC_UNUSED, fakeSecretGetValue(virSecretPtr obj G_GNUC_UNUSED,
size_t *value_size, size_t *value_size,
unsigned int fakeflags G_GNUC_UNUSED, unsigned int fakeflags G_GNUC_UNUSED)
unsigned int internalFlags G_GNUC_UNUSED)
{ {
char *secret; char *secret;
secret = g_strdup("AQCVn5hO6HzFAhAAq0NCv8jtJcIcE+HOBlMQ1A"); secret = g_strdup("AQCVn5hO6HzFAhAAq0NCv8jtJcIcE+HOBlMQ1A");