On Tue, Feb 13, 2024 at 05:16:05PM +0100, Michal Privoznik wrote:
> The systemd-cred offers a convenient way to talk to host's TPM.
> While its original intent is to be used in systemd unit files, it
> offers two additional commands: encrypt and decrypt that can be
> used independent of the rest of systemd. And these are the ones
> we need. They offer a convenient way to encrypt/decrypt plaintext
> using a key that's stored in host's TPM (when --with-key=tpm is
> passed).
This is one way we could do it, but what I don't like about
this is that it ties the secret driver to system. I think
it would be better to have an intermediate key, that is
protected by credentials and use that for encryption of
the individual secrets.
Roughly speaking....
* Make virtsecretd accept a '--encryption-key /some/path' option
* In ExecPreStart in virtsecretd.service, if no key already
exists, we dd from /dev/random enough bytes for a AES256 key,
and then use systemd-creds encrypt to store it
* In ExecStart we set the --encryption-key arg to point to
the credentials that systemd will decrypt
* The secret driver can now use this master key to
AES encrypt/decrypt individual secrets
The benefit of this is that only encryption of the intermediate
key is tied to the TPM.
If someone needs to move everything to a new host, then can copy
over all the secret data files, and then just re-seal the intermediate
key against the new machine's TPM.
The also lets people setup the --encryption-key for virtsecretd
with some arbitrary non-systemd mechanism. eg could use clevis/tang,
or $whatever.
The secret driver does not need to know about systemd at all,
all the credentials logic is done with a minimal tweak to the
.service file.
> In addition, there's 'has-tpm2' command which allows us to detect
> whether host and the OS have everything needed to utilize the
> TPM.
FWIW, systemd-creds does not require the use of a TPM2, it is
quite happy running without one, so libvirt should not really
describe this as TPM based, nor require a TPM. Without a TPM
systemd just generates a random key and stores it in /var.
That is less secure by default of course, but still reasonable,
as in theory you could way to inject the master key to /var in
some manner.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/libvirt_private.syms | 3 +
> src/util/virsecret.c | 170 +++++++++++++++++++++++++++++++++++++++
> src/util/virsecret.h | 10 +++
> 3 files changed, 183 insertions(+)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 396bd80844..1d1bba915a 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -3362,6 +3362,9 @@ virSecretLookupDefClear;
> virSecretLookupDefCopy;
> virSecretLookupFormatSecret;
> virSecretLookupParseSecret;
> +virSecretTPMAvailable;
> +virSecretTPMDecrypt;
> +virSecretTPMEncrypt;
>
>
> # util/virsecureerase.h
> diff --git a/src/util/virsecret.c b/src/util/virsecret.c
> index 8a220a37ec..11aa146b6d 100644
> --- a/src/util/virsecret.c
> +++ b/src/util/virsecret.c
> @@ -23,10 +23,13 @@
>
> #include "datatypes.h"
> #include "viralloc.h"
> +#include "vircommand.h"
> #include "virerror.h"
> #include "virlog.h"
> #include "virsecret.h"
> +#include "virutil.h"
> #include "viruuid.h"
> +#include "virfile.h"
>
> #define VIR_FROM_THIS VIR_FROM_NONE
>
> @@ -176,3 +179,170 @@ virSecretGetSecretString(virConnectPtr conn,
>
> return 0;
> }
> +
> +
> +/**
> + * virSecretTPMAvailable:
> + *
> + * Checks whether systemd-creds is available and whether host supports
> + * TPM. Use this prior calling other virSecretTPM*() APIs.
> + *
> + * Returns: 1 in case TPM is available,
> + * 0 in case TPM is NOT available,
> + * -1 otherwise.
> + */
> +int
> +virSecretTPMAvailable(void)
> +{
> + g_autoptr(virCommand) cmd = NULL;
> + g_autofree char *out = NULL;
> + int exitstatus;
> + int rc;
> +
> + cmd = virCommandNewArgList("systemd-creds", "has-tpm2", NULL);
> +
> + virCommandSetOutputBuffer(cmd, &out);
> +
> + rc = virCommandRun(cmd, &exitstatus);
> +
> + if (rc < 0) {
> + /* systemd-creds not available */
> + return -1;
> + }
> +
> + if (!out || !*out) {
> + /* systemd-creds reported nothing. Ouch. */
> + return 0;
> + }
> +
> + /* systemd-creds can report one of these:
> + *
> + * yes - TPM is available and recognized by FW, kernel, etc.
> + * no - TPM is not available or not recognized
> + * partial - otherwise
> + */
> +
> + if (STRPREFIX(out, "yes\n"))
> + return 1;
> +
> + return 0;
> +}
> +
> +
> +/**
> + * virSecretTPMEncrypt:
> + * @name: credential name
> + * @value: credential value
> + * @value_size: size of @value
> + * @base64: encrypted @value, base64 encoded
> + *
> + * Encrypts given plaintext (@value) with a secret key automatically
> + * derived from the system's TPM2 chip. Ciphertext is base64 encoded and
> + * stored into @base64. Pass the same @name to virSecretTPMDecrypt().
> + *
> + * Returns: 0 on success,
> + * -1 otherwise (with error reported).
> + */
> +int
> +virSecretTPMEncrypt(const char *name,
> + unsigned const char *value,
> + size_t value_size,
> + char **base64)
> +{
> + g_autoptr(virCommand) cmd = NULL;
> + g_autofree char *errBuf = NULL;
> + int pipeFD[2] = { -1, -1 };
> + int ret = -1;
> +
> + if (virPipe(pipeFD) < 0)
> + return -1;
> +
> + if (virSetCloseExec(pipeFD[1]) < 0) {
> + virReportSystemError(errno, "%s",
> + _("Unable to set cloexec flag"));
> + goto cleanup;
> + }
> +
> + cmd = virCommandNewArgList("systemd-creds", "encrypt",
> + "--with-key=tpm2", NULL);
> + virCommandAddArgFormat(cmd, "--name=%s", name);
> + virCommandAddArgList(cmd, "-", "-", NULL);
> +
> + virCommandSetInputFD(cmd, pipeFD[0]);
> + virCommandSetOutputBuffer(cmd, base64);
> + virCommandSetErrorBuffer(cmd, &errBuf);
> + virCommandDoAsyncIO(cmd);
> +
> + if (virCommandRunAsync(cmd, NULL) < 0)
> + goto cleanup;
> +
> + if (safewrite(pipeFD[1], value, value_size) != value_size) {
> + virReportSystemError(errno, "%s",
> + _("Unable to pass secret value to systemd-cred"));
> + goto cleanup;
> + }
> +
> + if (VIR_CLOSE(pipeFD[1]) < 0) {
> + virReportSystemError(errno, "%s", _("unable to close pipe"));
> + goto cleanup;
> + }
> +
> + if (virCommandWait(cmd, NULL) < 0) {
> + if (errBuf && *errBuf) {
> + VIR_WARN("systemd-creds reported: %s", errBuf);
> + }
> + goto cleanup;
> + }
> +
> + ret = 0;
> + cleanup:
> + virCommandAbort(cmd);
> + VIR_FORCE_CLOSE(pipeFD[0]);
> + VIR_FORCE_CLOSE(pipeFD[1]);
> + return ret;
> +}
> +
> +
> +/**
> + * virSecretTPMDecrypt:
> + * @name: credential name
> + * @base64: encrypted @value, base64 encoded
> + * @value: credential value,
> + * @value_size: size of @value
> + *
> + * Decrypts given ciphertext, base64 encoded (@base64) and stores
> + * plaintext into @value and its size into @value_size.
> + *
> + * Returns: 0 on success,
> + * -1 otherwise (with error reported).
> + */
> +int
> +virSecretTPMDecrypt(const char *name,
> + const char *base64,
> + unsigned char **value,
> + size_t *value_size)
> +{
> + g_autoptr(virCommand) cmd = NULL;
> + g_autofree char *out = NULL;
> + g_autofree char *errBuf = NULL;
> +
> + cmd = virCommandNewArgList("systemd-creds", "decrypt",
> + "--transcode=base64", NULL);
> + virCommandAddArgFormat(cmd, "--name=%s", name);
> + virCommandAddArgList(cmd, "-", "-", NULL);
> +
> + virCommandSetInputBuffer(cmd, base64);
> + virCommandSetOutputBuffer(cmd, &out);
> + virCommandSetErrorBuffer(cmd, &errBuf);
> +
> + if (virCommandRun(cmd, NULL) < 0) {
> + if (errBuf && *errBuf) {
> + VIR_WARN("systemd-creds reported: %s", errBuf);
> + }
> + return -1;
> + }
> +
> + *value = g_base64_decode(out, value_size);
> +
> + return 0;
> +}
> diff --git a/src/util/virsecret.h b/src/util/virsecret.h
> index c803f0fe33..a5ca764bb7 100644
> --- a/src/util/virsecret.h
> +++ b/src/util/virsecret.h
> @@ -62,3 +62,13 @@ int virSecretGetSecretString(virConnectPtr conn,
> size_t *ret_secret_size)
> ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4)
> ATTRIBUTE_NONNULL(5) G_GNUC_WARN_UNUSED_RESULT;
> +
> +int virSecretTPMAvailable(void);
> +int virSecretTPMEncrypt(const char *name,
> + unsigned const char *value,
> + size_t value_size,
> + char **base64);
> +int virSecretTPMDecrypt(const char *name,
> + const char *base64,
> + unsigned char **value,
> + size_t *value_size);
> --
> 2.43.0
> _______________________________________________
> Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
> To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
With 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 :|
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
