12:43 p.m.
If you define a secret with private="yes", then libvirt won't let any
client query the secret value after it is set. Only other libvirt
drivers inside the daemon can query it by passing a special internal
only flag to the virSecretGetValue API. The remote driver/daemon
refuses to let this internal flag go over the wire preventing normal
clients from using it
This doesn't work with the split daemons because the virSecretGetValue
API done by virqemud / virtstoraged has to go over the wire to reach
the virsecretd.
We need to come up with an alternative way to "prove" that the caller
of virSecretGetValue is a libvirt daemon, as opposed to a general
libvirt client.
Note with if only traditional POSIX DAC permissions are in effect
then we could consider it pointless trying to restrict access to
clients running the same user/group as the libvirt daemon. We ought
to take into account that the client might be confined by SELinux
though, so the "private secret" concept isn't entirely pointless.
Thus doing a simple uid of client == uid of daemon check is a bit
too weak. The UID check might also not fly if the modular daemons
are run inside containers with user namespaces, as the container
for virtsecretd and virtqemud might have different user mappings
in theory.
This series adds a concept of a "token" which is known only to the
libvirt daemons. The first daemon to use it writes a random hex
string to /var/run/libvirt/common/system.token. Other daemons can
read and compare this. Unless a MAC system is present this is still
largely security theatre, but that's not really worse than the
historical behaviour.
When an API call is made the virIdentity by default reflects the
identity of the UNIX process that initiated it.
When connecting to virtproxyd, the client apps' identity is forwarded
to the next virtNNNNd daemon.
When libvirt drivers, however, initiate an API call we never set any
identity. With monolithic libvirtd, they'd inherit the current client
identity automagically since it was all in the same thread local. With
modular daemons the othe driver would see the identity of the other
libvirt daemon which is bad as this gives elevated privileges in the
ACL check.
Thus we fix the code which drivers use to open a connection to other
daemons, such that it applies the current caller's identity. It does
this using an "elevated" identity though, which means, we have added
in the system token. Thus the virtsecretd daemon getting the call
virSecretGetValue sees the virIdentity reflecting the client
application which originally called the virDomainCreate() API, but
with the system token set. Thus virsecretd can see that the
virSecretGetValue was invoked by another daemon, not a libvirt
client app.
Daniel P. Berrangé (9):
util: add virRandomToken API
util: introduce concept of a system token into identities
util: generate a persistent system token
src: set system token for system identity
util: add API for copying identity objects
util: add method for getting the current identity with system token
src: add API to determine if current identity is a system identity
secret: rework handling of private secrets
src: set identity when opening secondary drivers
src/driver-secret.h | 9 +-
src/driver.c | 27 +++++
src/libvirt-secret.c | 2 +-
src/libvirt_private.syms | 5 +
src/remote/remote_driver.c | 8 +-
src/secret/secret_driver.c | 34 ++++--
src/util/viridentity.c | 230 +++++++++++++++++++++++++++++++++++++
src/util/viridentity.h | 7 ++
src/util/virrandom.c | 18 +++
src/util/virrandom.h | 1 +
src/util/virsecret.c | 3 +-
tests/qemuxml2argvtest.c | 3 +-
12 files changed, 320 insertions(+), 27 deletions(-)
--
2.31.1
12:43 p.m.
New subject: [libvirt PATCH 1/9] util: add virRandomToken API
A random token is simply a string of random bytes formatted in
hexidecimal.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/virrandom.c | 18 ++++++++++++++++++
src/util/virrandom.h | 1 +
3 files changed, 20 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9f761c2c00..418688a4fa 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3081,6 +3081,7 @@ virRandomBits;
virRandomBytes;
virRandomGenerateWWN;
virRandomInt;
+virRandomToken;
# util/virresctrl.h
diff --git a/src/util/virrandom.c b/src/util/virrandom.c
index 3ae1297e6b..c3f3aa1fa6 100644
--- a/src/util/virrandom.c
+++ b/src/util/virrandom.c
@@ -161,3 +161,21 @@ virRandomGenerateWWN(char **wwn,
(unsigned long long)virRandomBits(36));
return 0;
}
+
+char *virRandomToken(size_t len)
+{
+ g_autofree unsigned char *data = g_new0(unsigned char, len);
+ g_autofree char *token = g_new0(char, (len * 2) + 1);
+ static const char hex[] = "0123456789abcdef";
+ size_t i;
+
+ if (virRandomBytes(data, len) < 0)
+ return NULL;
+
+ for (i = 0; i < len; i++) {
+ token[(i*2)] = hex[data[i] & 0xf];
+ token[(i*2)+1] = hex[(data[i] >> 4) & 0xf];
+ }
+
+ return g_steal_pointer(&token);
+}
diff --git a/src/util/virrandom.h b/src/util/virrandom.h
index 297721f912..aac684ada9 100644
--- a/src/util/virrandom.h
+++ b/src/util/virrandom.h
@@ -26,3 +26,4 @@ uint32_t virRandomInt(uint32_t max);
int virRandomBytes(unsigned char *buf, size_t buflen)
ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT G_GNUC_NO_INLINE;
int virRandomGenerateWWN(char **wwn, const char *virt_type) G_GNUC_NO_INLINE;
+char *virRandomToken(size_t len);
--
2.31.1
12:43 p.m.
New subject: [libvirt PATCH 2/9] util: introduce concept of a system token into identities
We want a way to distinguish between calls from a libvirt daemon, and a
regular client application when both are running as the same user
account. This is not possible with the current set of attributes
recorded against an identity, as there is nothing that is common to all
of the modular libvirt daemons, while distinct to all other processes.
We thus introduce the idea of a system token, which is simply a random
hex string that is only known by the libvirt daemons, to be recored
against the system identity.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/libvirt_private.syms | 2 ++
src/util/viridentity.c | 34 ++++++++++++++++++++++++++++++++++
src/util/viridentity.h | 4 ++++
3 files changed, 40 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 418688a4fa..c5f6c90365 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2404,6 +2404,7 @@ virIdentityGetProcessTime;
virIdentityGetSASLUserName;
virIdentityGetSELinuxContext;
virIdentityGetSystem;
+virIdentityGetSystemToken;
virIdentityGetUNIXGroupID;
virIdentityGetUNIXUserID;
virIdentityGetUserName;
@@ -2416,6 +2417,7 @@ virIdentitySetProcessID;
virIdentitySetProcessTime;
virIdentitySetSASLUserName;
virIdentitySetSELinuxContext;
+virIdentitySetSystemToken;
virIdentitySetUNIXGroupID;
virIdentitySetUNIXUserID;
virIdentitySetUserName;
diff --git a/src/util/viridentity.c b/src/util/viridentity.c
index 7edb6a171a..7da4ea12f5 100644
--- a/src/util/viridentity.c
+++ b/src/util/viridentity.c
@@ -40,6 +40,8 @@
#define VIR_FROM_THIS VIR_FROM_IDENTITY
+#define VIR_CONNECT_IDENTITY_SYSTEM_TOKEN "system.token"
+
VIR_LOG_INIT("util.identity");
struct _virIdentity {
@@ -382,6 +384,17 @@ int virIdentityGetSELinuxContext(virIdentity *ident,
}
+int virIdentityGetSystemToken(virIdentity *ident,
+ const char **token)
+{
+ *token = NULL;
+ return virTypedParamsGetString(ident->params,
+ ident->nparams,
+ VIR_CONNECT_IDENTITY_SYSTEM_TOKEN,
+ token);
+}
+
+
int virIdentitySetUserName(virIdentity *ident,
const char *username)
{
@@ -554,6 +567,25 @@ int virIdentitySetSELinuxContext(virIdentity *ident,
}
+int virIdentitySetSystemToken(virIdentity *ident,
+ const char *token)
+{
+ if (virTypedParamsGet(ident->params,
+ ident->nparams,
+ VIR_CONNECT_IDENTITY_SYSTEM_TOKEN)) {
+ virReportError(VIR_ERR_OPERATION_DENIED, "%s",
+ _("Identity attribute is already set"));
+ return -1;
+ }
+
+ return virTypedParamsAddString(&ident->params,
+ &ident->nparams,
+ &ident->maxparams,
+ VIR_CONNECT_IDENTITY_SYSTEM_TOKEN,
+ token);
+}
+
+
int virIdentitySetParameters(virIdentity *ident,
virTypedParameterPtr params,
int nparams)
@@ -577,6 +609,8 @@ int virIdentitySetParameters(virIdentity *ident,
VIR_TYPED_PARAM_STRING,
VIR_CONNECT_IDENTITY_SELINUX_CONTEXT,
VIR_TYPED_PARAM_STRING,
+ VIR_CONNECT_IDENTITY_SYSTEM_TOKEN,
+ VIR_TYPED_PARAM_STRING,
NULL) < 0)
return -1;
diff --git a/src/util/viridentity.h b/src/util/viridentity.h
index fa3f46788c..640a7ba2e4 100644
--- a/src/util/viridentity.h
+++ b/src/util/viridentity.h
@@ -52,6 +52,8 @@ int virIdentityGetX509DName(virIdentity *ident,
const char **dname);
int virIdentityGetSELinuxContext(virIdentity *ident,
const char **context);
+int virIdentityGetSystemToken(virIdentity *ident,
+ const char **token);
int virIdentitySetUserName(virIdentity *ident,
@@ -72,6 +74,8 @@ int virIdentitySetX509DName(virIdentity *ident,
const char *dname);
int virIdentitySetSELinuxContext(virIdentity *ident,
const char *context);
+int virIdentitySetSystemToken(virIdentity *ident,
+ const char *token);
int virIdentitySetParameters(virIdentity *ident,
virTypedParameterPtr params,
--
2.31.1
12:43 p.m.
New subject: [libvirt PATCH 3/9] util: generate a persistent system token
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(a)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
5:13 a.m.
New subject: [libvirt PATCH 3/9] util: generate a persistent system token
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(a)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
5:16 a.m.
New subject: [libvirt PATCH 3/9] util: generate a persistent system token
On Thu, May 06, 2021 at 12:13:01PM +0200, Michal Prívozník wrote:
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(a)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.
O_CLOEXEC didn't exist on older Linux and non-Linux OS.
It has been quite a while now though, and so I'm not sure
if perhaps all our desired platforms do indeed now support
it. We should investigate, as it would obviously make life
simpler if it was possible to rely on it existing.
> + virReportSystemError(errno,
> + _("Failed to set close-on-exec flag
'%s'"),
> + tokenfile);
> + goto error;
> + }
Michal
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
12:43 p.m.
New subject: [libvirt PATCH 4/9] src: set system token for system identity
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/util/viridentity.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/util/viridentity.c b/src/util/viridentity.c
index 065db06e49..83044a3de1 100644
--- a/src/util/viridentity.c
+++ b/src/util/viridentity.c
@@ -268,6 +268,7 @@ virIdentity *virIdentityGetSystem(void)
#if WITH_SELINUX
char *con;
#endif
+ g_autofree char *token = virIdentityEnsureSystemToken();
if (!(ret = virIdentityNew()))
return NULL;
@@ -310,6 +311,9 @@ virIdentity *virIdentityGetSystem(void)
}
#endif
+ if (virIdentitySetSystemToken(ret, token) < 0)
+ return NULL;
+
return g_steal_pointer(&ret);
}
--
2.31.1
5:13 a.m.
New subject: [libvirt PATCH 4/9] src: set system token for system identity
On 5/4/21 7:43 PM, Daniel P. Berrangé wrote:
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/util/viridentity.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/util/viridentity.c b/src/util/viridentity.c
index 065db06e49..83044a3de1 100644
--- a/src/util/viridentity.c
+++ b/src/util/viridentity.c
@@ -268,6 +268,7 @@ virIdentity *virIdentityGetSystem(void)
#if WITH_SELINUX
char *con;
#endif
+ g_autofree char *token = virIdentityEnsureSystemToken();
It should be a hard error if EnsureSystemToken() fails.
if (!(ret = virIdentityNew()))
return NULL;
@@ -310,6 +311,9 @@ virIdentity *virIdentityGetSystem(void)
}
#endif
+ if (virIdentitySetSystemToken(ret, token) < 0)
+ return NULL;
+
return g_steal_pointer(&ret);
}
Michal
12:43 p.m.
New subject: [libvirt PATCH 5/9] util: add API for copying identity objects
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/viridentity.c | 21 +++++++++++++++++++++
src/util/viridentity.h | 1 +
3 files changed, 23 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c5f6c90365..90ca52c95c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2410,6 +2410,7 @@ virIdentityGetUNIXUserID;
virIdentityGetUserName;
virIdentityGetX509DName;
virIdentityNew;
+virIdentityNewCopy;
virIdentitySetCurrent;
virIdentitySetGroupName;
virIdentitySetParameters;
diff --git a/src/util/viridentity.c b/src/util/viridentity.c
index 83044a3de1..3b523d7a2d 100644
--- a/src/util/viridentity.c
+++ b/src/util/viridentity.c
@@ -332,6 +332,27 @@ virIdentity *virIdentityNew(void)
}
+/**
+ * virIdentityNewCopy:
+ *
+ * Creates a new identity object that is a deep copy of an
+ * existing identity.
+ *
+ * Returns: a copy of the source identity
+ */
+virIdentity *virIdentityNewCopy(virIdentity *src)
+{
+ g_autoptr(virIdentity) ident = virIdentityNew();
+
+ if (virTypedParamsCopy(&ident->params, src->params, src->nparams) <
0)
+ return NULL;
+ ident->nparams = src->nparams;
+ ident->maxparams = src->nparams;
+
+ return g_steal_pointer(&ident);
+}
+
+
static void virIdentityFinalize(GObject *object)
{
virIdentity *ident = VIR_IDENTITY(object);
diff --git a/src/util/viridentity.h b/src/util/viridentity.h
index 640a7ba2e4..512bca286d 100644
--- a/src/util/viridentity.h
+++ b/src/util/viridentity.h
@@ -33,6 +33,7 @@ int virIdentitySetCurrent(virIdentity *ident);
virIdentity *virIdentityGetSystem(void);
virIdentity *virIdentityNew(void);
+virIdentity *virIdentityNewCopy(virIdentity *src);
int virIdentityGetUserName(virIdentity *ident,
const char **username);
--
2.31.1
12:43 p.m.
New subject: [libvirt PATCH 6/9] util: add method for getting the current identity with system token
The current identity object represents the identity of the application
which initiated the currently executing public API operation. Normally
this is the libvirt client application identity.
There are times when the libvirt daemon has to make extra public API
calls on behalf of the client application. We want these API calls to
still use the client appication's identity for ACL checking. At the
same time we need to be able to show that the API call is coming from
the daemon.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/util/viridentity.c | 36 ++++++++++++++++++++++++++++++++++++
src/util/viridentity.h | 1 +
2 files changed, 37 insertions(+)
diff --git a/src/util/viridentity.c b/src/util/viridentity.c
index 3b523d7a2d..9fa6ab0dd0 100644
--- a/src/util/viridentity.c
+++ b/src/util/viridentity.c
@@ -123,6 +123,42 @@ virIdentity *virIdentityGetCurrent(void)
}
+/**
+ * virIdentityGetCurrentElevated:
+ *
+ * Get a copy of the current identity associated with this thread,
+ * with elevated privileges to allow it to identity a system
+ * initiated operation. The caller will own a reference to the
+ * returned identity, but must not modify the object in any way,
+ * other than to release the reference when done with g_object_unref
+ *
+ * Returns: a reference to the current identity, or NULL
+ */
+virIdentity *virIdentityGetCurrentElevated(void)
+{
+ g_autoptr(virIdentity) ident = virIdentityGetCurrent();
+ const char *token;
+ int rc;
+
+ if (!ident) {
+ return NULL;
+ }
+
+ if ((rc = virIdentityGetSystemToken(ident, &token)) < 0)
+ return NULL;
+
+ if (rc == 0) {
+ g_autoptr(virIdentity) identel = virIdentityNewCopy(ident);
+
+ if (virIdentitySetSystemToken(identel, systemToken) < 0)
+ return NULL;
+
+ return g_steal_pointer(&identel);
+ }
+
+ return g_steal_pointer(&ident);
+}
+
/**
* virIdentitySetCurrent:
*
diff --git a/src/util/viridentity.h b/src/util/viridentity.h
index 512bca286d..420cd82854 100644
--- a/src/util/viridentity.h
+++ b/src/util/viridentity.h
@@ -28,6 +28,7 @@
G_DECLARE_FINAL_TYPE(virIdentity, vir_identity, VIR, IDENTITY, GObject);
virIdentity *virIdentityGetCurrent(void);
+virIdentity *virIdentityGetCurrentElevated(void);
int virIdentitySetCurrent(virIdentity *ident);
virIdentity *virIdentityGetSystem(void);
--
2.31.1
5:13 a.m.
New subject: [libvirt PATCH 6/9] util: add method for getting the current identity with system token
On 5/4/21 7:43 PM, Daniel P. Berrangé wrote:
The current identity object represents the identity of the
application
which initiated the currently executing public API operation. Normally
this is the libvirt client application identity.
There are times when the libvirt daemon has to make extra public API
calls on behalf of the client application. We want these API calls to
still use the client appication's identity for ACL checking. At the
same time we need to be able to show that the API call is coming from
the daemon.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/util/viridentity.c | 36 ++++++++++++++++++++++++++++++++++++
src/util/viridentity.h | 1 +
2 files changed, 37 insertions(+)
Don't forget to expose the symbol in libvirt_private.syms.
Michal
12:43 p.m.
New subject: [libvirt PATCH 7/9] src: add API to determine if current identity is a system identity
This is essentially a way to determine if the current identity
is that of another libvirt daemon.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/viridentity.c | 28 ++++++++++++++++++++++++++++
src/util/viridentity.h | 1 +
3 files changed, 30 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 90ca52c95c..698ba50d6b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2409,6 +2409,7 @@ virIdentityGetUNIXGroupID;
virIdentityGetUNIXUserID;
virIdentityGetUserName;
virIdentityGetX509DName;
+virIdentityIsCurrentElevated;
virIdentityNew;
virIdentityNewCopy;
virIdentitySetCurrent;
diff --git a/src/util/viridentity.c b/src/util/viridentity.c
index 9fa6ab0dd0..424de513d9 100644
--- a/src/util/viridentity.c
+++ b/src/util/viridentity.c
@@ -354,6 +354,34 @@ virIdentity *virIdentityGetSystem(void)
}
+/**
+ * virIdentityIsCurrentElevated:
+ *
+ * Determine if the current identity has elevated privileges.
+ * This indicates that it was invoked on behalf of the
+ * user by a libvirt daemon.
+ *
+ * Returns: true if elevated
+ */
+int virIdentityIsCurrentElevated(void)
+{
+ g_autoptr(virIdentity) current = virIdentityGetCurrent();
+ const char *currentToken = NULL;
+ int rv;
+
+ if (!current) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("No current identity"));
+ return -1;
+ }
+
+ rv = virIdentityGetSystemToken(current, ¤tToken);
+ if (rv <= 0)
+ return rv;
+
+ return STREQ_NULLABLE(currentToken, systemToken);
+}
+
/**
* virIdentityNew:
*
diff --git a/src/util/viridentity.h b/src/util/viridentity.h
index 420cd82854..37a0c1ad4c 100644
--- a/src/util/viridentity.h
+++ b/src/util/viridentity.h
@@ -31,6 +31,7 @@ virIdentity *virIdentityGetCurrent(void);
virIdentity *virIdentityGetCurrentElevated(void);
int virIdentitySetCurrent(virIdentity *ident);
+int virIdentityIsCurrentElevated(void);
virIdentity *virIdentityGetSystem(void);
virIdentity *virIdentityNew(void);
--
2.31.1
12:43 p.m.
New subject: [libvirt PATCH 8/9] 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: Daniel P. Berrangé <berrange(a)redhat.com>
---
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
-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
(*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);
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_size, unsigned
int flags)
if (conn->secretDriver != NULL && conn->secretDriver->secretGetValue
!= NULL) {
unsigned char *ret;
- ret = conn->secretDriver->secretGetValue(secret, value_size, flags, 0);
+ ret = conn->secretDriver->secretGetValue(secret, value_size, flags);
if (ret == 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,
static unsigned char *
remoteSecretGetValue(virSecretPtr secret, size_t *value_size,
- unsigned int flags, unsigned int internalFlags)
+ unsigned int flags)
{
unsigned char *rv = NULL;
remote_secret_get_value_args args;
@@ -5391,12 +5391,6 @@ remoteSecretGetValue(virSecretPtr secret, size_t *value_size,
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);
args.flags = flags;
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 = NULL;
virSecretObj *obj;
@@ -368,11 +368,31 @@ secretGetValue(virSecretPtr secret,
if (virSecretGetValueEnsureACL(secret->conn, def) < 0)
goto cleanup;
- if ((internalFlags & VIR_SECRET_GET_VALUE_INTERNAL_CALL) == 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 = virIdentityIsCurrentElevated();
+ if (rv < 0)
+ goto cleanup;
+ if (rv == 0) {
+ virReportError(VIR_ERR_INVALID_SECRET, "%s",
+ _("secret is private"));
+ goto cleanup;
+ }
}
if (!(ret = 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;
}
- *secret = conn->secretDriver->secretGetValue(sec, secret_size, 0,
- VIR_SECRET_GET_VALUE_INTERNAL_CALL);
+ *secret = conn->secretDriver->secretGetValue(sec, secret_size, 0);
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 = g_strdup("AQCVn5hO6HzFAhAAq0NCv8jtJcIcE+HOBlMQ1A");
--
2.31.1
12:43 p.m.
New subject: [libvirt PATCH 9/9] src: set identity when opening secondary drivers
The drivers can all call virGetConnectXXX to open a connection to a
secondary driver. For example, when creating a encrypted storage volume,
the storage driver has to open a secret driver connection, or when
starting a guest, the QEMU driver has to open the network driver to
lookup a virtual network.
When using monolithic libvirtd, the connection has the same effective
identity as the client, since everything is still in the same process.
When using the modular daemons, however, the remote daemon sees the
identity of the calling daemon. This is a mistake as it results in
the modular daemons seeing the client with elevated privileges.
We need to pass on the current identity explicitly when opening the
secondary drivers. This is the same thing that is done by daemon RPC
dispatcher code when it is directly forwarding top level API calls
from virtproxyd and other daemons.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/driver.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/src/driver.c b/src/driver.c
index f8022d2522..dc409c48fc 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -33,6 +33,8 @@
#include "virstring.h"
#include "virthread.h"
#include "virutil.h"
+#include "viridentity.h"
+#include "datatypes.h"
#include "configmake.h"
VIR_LOG_INIT("driver");
@@ -136,6 +138,7 @@ static virConnectPtr
virGetConnectGeneric(virThreadLocal *threadPtr, const char *name)
{
virConnectPtr conn;
+ virErrorPtr saved;
if (virConnectCacheInitialize() < 0)
return NULL;
@@ -153,8 +156,32 @@ virGetConnectGeneric(virThreadLocal *threadPtr, const char *name)
conn = virConnectOpen(uri);
VIR_DEBUG("Opened new %s connection %p", name, conn);
+ if (!conn)
+ return NULL;
+
+ if (conn->driver->connectSetIdentity != NULL) {
+ g_autoptr(virIdentity) ident = NULL;
+ virTypedParameterPtr identparams = NULL;
+ int nidentparams = 0;
+
+ VIR_DEBUG("Attempting to delegate current identity");
+ if (!(ident = virIdentityGetCurrentElevated()))
+ goto error;
+
+ if (virIdentityGetParameters(ident, &identparams, &nidentparams) <
0)
+ goto error;
+
+ if (virConnectSetIdentity(conn, identparams, nidentparams, 0) < 0)
+ goto error;
+ }
}
return conn;
+
+ error:
+ saved = virSaveLastError();
+ virConnectClose(conn);
+ virSetError(saved);
+ return NULL;
}
--
2.31.1
5:13 a.m.
On 5/4/21 7:43 PM, Daniel P. Berrangé wrote:
If you define a secret with private="yes", then libvirt
won't let any
client query the secret value after it is set. Only other libvirt
drivers inside the daemon can query it by passing a special internal
only flag to the virSecretGetValue API. The remote driver/daemon
refuses to let this internal flag go over the wire preventing normal
clients from using it
This doesn't work with the split daemons because the virSecretGetValue
API done by virqemud / virtstoraged has to go over the wire to reach
the virsecretd.
We need to come up with an alternative way to "prove" that the caller
of virSecretGetValue is a libvirt daemon, as opposed to a general
libvirt client.
Note with if only traditional POSIX DAC permissions are in effect
then we could consider it pointless trying to restrict access to
clients running the same user/group as the libvirt daemon. We ought
to take into account that the client might be confined by SELinux
though, so the "private secret" concept isn't entirely pointless.
Thus doing a simple uid of client == uid of daemon check is a bit
too weak. The UID check might also not fly if the modular daemons
are run inside containers with user namespaces, as the container
for virtsecretd and virtqemud might have different user mappings
in theory.
This series adds a concept of a "token" which is known only to the
libvirt daemons. The first daemon to use it writes a random hex
string to /var/run/libvirt/common/system.token. Other daemons can
read and compare this. Unless a MAC system is present this is still
largely security theatre, but that's not really worse than the
historical behaviour.
When an API call is made the virIdentity by default reflects the
identity of the UNIX process that initiated it.
When connecting to virtproxyd, the client apps' identity is forwarded
to the next virtNNNNd daemon.
When libvirt drivers, however, initiate an API call we never set any
identity. With monolithic libvirtd, they'd inherit the current client
identity automagically since it was all in the same thread local. With
modular daemons the othe driver would see the identity of the other
libvirt daemon which is bad as this gives elevated privileges in the
ACL check.
Thus we fix the code which drivers use to open a connection to other
daemons, such that it applies the current caller's identity. It does
this using an "elevated" identity though, which means, we have added
in the system token. Thus the virtsecretd daemon getting the call
virSecretGetValue sees the virIdentity reflecting the client
application which originally called the virDomainCreate() API, but
with the system token set. Thus virsecretd can see that the
virSecretGetValue was invoked by another daemon, not a libvirt
client app.
Daniel P. Berrangé (9):
util: add virRandomToken API
util: introduce concept of a system token into identities
util: generate a persistent system token
src: set system token for system identity
util: add API for copying identity objects
util: add method for getting the current identity with system token
src: add API to determine if current identity is a system identity
secret: rework handling of private secrets
src: set identity when opening secondary drivers
src/driver-secret.h | 9 +-
src/driver.c | 27 +++++
src/libvirt-secret.c | 2 +-
src/libvirt_private.syms | 5 +
src/remote/remote_driver.c | 8 +-
src/secret/secret_driver.c | 34 ++++--
src/util/viridentity.c | 230 +++++++++++++++++++++++++++++++++++++
src/util/viridentity.h | 7 ++
src/util/virrandom.c | 18 +++
src/util/virrandom.h | 1 +
src/util/virsecret.c | 3 +-
tests/qemuxml2argvtest.c | 3 +-
12 files changed, 320 insertions(+), 27 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
7:33 a.m.
I'm not going to apply this yet as I realized that I have broken
internal secrets with monolithic libvirtd instead :-(
On Tue, May 04, 2021 at 06:43:41PM +0100, Daniel P. Berrangé wrote:
If you define a secret with private="yes", then libvirt
won't let any
client query the secret value after it is set. Only other libvirt
drivers inside the daemon can query it by passing a special internal
only flag to the virSecretGetValue API. The remote driver/daemon
refuses to let this internal flag go over the wire preventing normal
clients from using it
This doesn't work with the split daemons because the virSecretGetValue
API done by virqemud / virtstoraged has to go over the wire to reach
the virsecretd.
We need to come up with an alternative way to "prove" that the caller
of virSecretGetValue is a libvirt daemon, as opposed to a general
libvirt client.
Note with if only traditional POSIX DAC permissions are in effect
then we could consider it pointless trying to restrict access to
clients running the same user/group as the libvirt daemon. We ought
to take into account that the client might be confined by SELinux
though, so the "private secret" concept isn't entirely pointless.
Thus doing a simple uid of client == uid of daemon check is a bit
too weak. The UID check might also not fly if the modular daemons
are run inside containers with user namespaces, as the container
for virtsecretd and virtqemud might have different user mappings
in theory.
This series adds a concept of a "token" which is known only to the
libvirt daemons. The first daemon to use it writes a random hex
string to /var/run/libvirt/common/system.token. Other daemons can
read and compare this. Unless a MAC system is present this is still
largely security theatre, but that's not really worse than the
historical behaviour.
When an API call is made the virIdentity by default reflects the
identity of the UNIX process that initiated it.
When connecting to virtproxyd, the client apps' identity is forwarded
to the next virtNNNNd daemon.
When libvirt drivers, however, initiate an API call we never set any
identity. With monolithic libvirtd, they'd inherit the current client
identity automagically since it was all in the same thread local. With
modular daemons the othe driver would see the identity of the other
libvirt daemon which is bad as this gives elevated privileges in the
ACL check.
Thus we fix the code which drivers use to open a connection to other
daemons, such that it applies the current caller's identity. It does
this using an "elevated" identity though, which means, we have added
in the system token. Thus the virtsecretd daemon getting the call
virSecretGetValue sees the virIdentity reflecting the client
application which originally called the virDomainCreate() API, but
with the system token set. Thus virsecretd can see that the
virSecretGetValue was invoked by another daemon, not a libvirt
client app.
Daniel P. Berrangé (9):
util: add virRandomToken API
util: introduce concept of a system token into identities
util: generate a persistent system token
src: set system token for system identity
util: add API for copying identity objects
util: add method for getting the current identity with system token
src: add API to determine if current identity is a system identity
secret: rework handling of private secrets
src: set identity when opening secondary drivers
src/driver-secret.h | 9 +-
src/driver.c | 27 +++++
src/libvirt-secret.c | 2 +-
src/libvirt_private.syms | 5 +
src/remote/remote_driver.c | 8 +-
src/secret/secret_driver.c | 34 ++++--
src/util/viridentity.c | 230 +++++++++++++++++++++++++++++++++++++
src/util/viridentity.h | 7 ++
src/util/virrandom.c | 18 +++
src/util/virrandom.h | 1 +
src/util/virsecret.c | 3 +-
tests/qemuxml2argvtest.c | 3 +-
12 files changed, 320 insertions(+), 27 deletions(-)
--
2.31.1
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
8:46 a.m.
On Fri, May 07, 2021 at 03:43:32PM +0200, Michal Prívozník wrote:
On 5/7/21 2:33 PM, Daniel P. Berrangé wrote:
> I'm not going to apply this yet as I realized that I have broken
> internal secrets with monolithic libvirtd instead :-(
You did? I've tested it (by starting a pool with a secret) and it worked.
Secrets aren't used when starting a pool with merely encrypted vols present.
They're only needed to perform I/O, so would have to test starting a guest,
or cloning a encrypted vol. Alternatively starting a RBD/iSCSI pool that
requires auth.
I'm testing with starting a QEMU guest with an encrypted vol
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
8:48 a.m.
On 5/7/21 3:46 PM, Daniel P. Berrangé wrote:
On Fri, May 07, 2021 at 03:43:32PM +0200, Michal Prívozník wrote:
> On 5/7/21 2:33 PM, Daniel P. Berrangé wrote:
>> I'm not going to apply this yet as I realized that I have broken
>> internal secrets with monolithic libvirtd instead :-(
>
> You did? I've tested it (by starting a pool with a secret) and it worked.
Secrets aren't used when starting a pool with merely encrypted vols present.
They're only needed to perform I/O, so would have to test starting a guest,
or cloning a encrypted vol. Alternatively starting a RBD/iSCSI pool that
requires auth.
Yeah, it's an iSCSI pool that requires auth. But okay, I'll try
I'm testing with starting a QEMU guest with an encrypted vol
this for v2.
Michal
9:08 a.m.
On Fri, May 07, 2021 at 03:48:42PM +0200, Michal Prívozník wrote:
On 5/7/21 3:46 PM, Daniel P. Berrangé wrote:
> On Fri, May 07, 2021 at 03:43:32PM +0200, Michal Prívozník wrote:
>> On 5/7/21 2:33 PM, Daniel P. Berrangé wrote:
>>> I'm not going to apply this yet as I realized that I have broken
>>> internal secrets with monolithic libvirtd instead :-(
>>
>> You did? I've tested it (by starting a pool with a secret) and it worked.
>
> Secrets aren't used when starting a pool with merely encrypted vols present.
>
> They're only needed to perform I/O, so would have to test starting a guest,
> or cloning a encrypted vol. Alternatively starting a RBD/iSCSI pool that
> requires auth.
Yeah, it's an iSCSI pool that requires auth. But okay, I'll try
You did have the secret marked with private="yes" ?
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
11:02 a.m.
On 5/7/21 4:08 PM, Daniel P. Berrangé wrote:
On Fri, May 07, 2021 at 03:48:42PM +0200, Michal Prívozník wrote:
> On 5/7/21 3:46 PM, Daniel P. Berrangé wrote:
>> On Fri, May 07, 2021 at 03:43:32PM +0200, Michal Prívozník wrote:
>>> On 5/7/21 2:33 PM, Daniel P. Berrangé wrote:
>>>> I'm not going to apply this yet as I realized that I have broken
>>>> internal secrets with monolithic libvirtd instead :-(
>>>
>>> You did? I've tested it (by starting a pool with a secret) and it
worked.
>>
>> Secrets aren't used when starting a pool with merely encrypted vols present.
>>
>> They're only needed to perform I/O, so would have to test starting a guest,
>> or cloning a encrypted vol. Alternatively starting a RBD/iSCSI pool that
>> requires auth.
>
> Yeah, it's an iSCSI pool that requires auth. But okay, I'll try
You did have the secret marked with private="yes" ?
Yes.
Michal
1331
days inactive
1334
days old
20 comments
2 participants
participants (2)
-
Daniel P. Berrangé -
Michal Prívozník