Re: [RFC PATCH] crypto/secret: support fetching secrets from Linux keyring
On Sat, Mar 28, 2020 at 02:40:14PM +0300, Alexey Krasikov wrote: > Add the ability for the secret object to obtain secret data from the > Linux in-kernel key managment and retention facility, as an extra option > to the existing ones: reading from a file or passing directly as a > string. > > The secret is identified by the key serial number. The upper layers > need to instantiate the key and make sure the QEMU process has access > rights to read it. This looks like a generally useful concept to me, however, I think it highlights a mistake in the original secrets design. We should have had TYPE_QCRYPTO_SECRET be an abstract interface, and then had subclasses TYPE_QCRYPTO_SECRET_FILE, TYPE_QCRYPTO_SECRET_INLINE Then we could add TYPE_QCRYPTO_SECRET_LINUX. We can still mostly fix that mistake now though. We can introduce a TYPE_QCRYPTO_SECRET_INTERFACE which defines the generic interface. This interface just needs to define one API entry point "char *get_data(QCryptoSecretInterface *secret)" The current TYPE_QCRYPTO_SECRET can be the current impl of that interface that returns the rawdata field. Then we can add the new TYPE_QCRYPTO_SECRET_LINUX for the keyring implementation that does the Linux specific magic. This will let us easily compile out the Linux impl on platforms where it is not available too. > > Signed-off-by: Alexey Krasikov > --- > crypto/secret.c | 88 +++-- > include/crypto/secret.h | 3 ++ > 2 files changed, 88 insertions(+), 3 deletions(-) > > diff --git a/crypto/secret.c b/crypto/secret.c > index 1cf0ad0ce8..2e8be6241c 100644 > --- a/crypto/secret.c > +++ b/crypto/secret.c > @@ -19,6 +19,8 @@ > */ > > #include "qemu/osdep.h" > +#include > +#include > #include "crypto/secret.h" > #include "crypto/cipher.h" > #include "qapi/error.h" > @@ -28,6 +30,40 @@ > #include "trace.h" > > > +static inline > +long keyctl_read(key_serial_t key, uint8_t *buffer, size_t buflen) > +{ > +#ifdef __NR_keyctl > +return syscall(__NR_keyctl, KEYCTL_READ, key, buffer, buflen, 0); > +#else > +errno = ENOSYS; > +return -1; > +#endif > +} > + > +static > +long keyctl_read_alloc(key_serial_t key, uint8_t **buffer) > +{ > +uint8_t *loc_buf; > +long retcode = keyctl_read(key, NULL, 0); > +if (retcode < 0) { > +return retcode; > +} > +loc_buf = g_malloc(retcode + 1); > +retcode = keyctl_read(key, loc_buf, retcode + 1); > + /* > +* We don't have key operations locks between syscalls. > +* For example, the key could have been removed or expired. > +*/ > +if (retcode >= 0) { > +loc_buf[retcode] = '\0'; > +*buffer = loc_buf; > +} else { > +g_free(loc_buf); > +} > +return retcode; > +} > + > static void > qcrypto_secret_load_data(QCryptoSecret *secret, > uint8_t **output, > @@ -41,10 +77,28 @@ qcrypto_secret_load_data(QCryptoSecret *secret, > *output = NULL; > *outputlen = 0; > > -if (secret->file) { > +if (secret->syskey) { > +uint8_t *buffer = NULL; > +long retcode; > +if (secret->data || secret->file) { > +error_setg(errp, > + "'syskey', 'file' and 'data' are mutually exclusive"); > +return; > +} > +retcode = keyctl_read_alloc(secret->syskey, &buffer); > +if (retcode < 0) { > +error_setg_errno(errp, errno, > + "Unable to read serial key %08x", > + secret->syskey); > +return; > +} else { > +*outputlen = retcode; > +*output = buffer; > +} > +} else if (secret->file) { > if (secret->data) { > error_setg(errp, > - "'file' and 'data' are mutually exclusive"); > + "'syskey', 'file' and 'data' are mutually exclusive"); > return; > } > if (!g_file_get_contents(secret->file, &data, &length, &gerr)) { > @@ -60,7 +114,8 @@ qcrypto_secret_load_data(QCryptoSecret *secret, > *outputlen = strlen(secret->data); > *output = (uint8_t *)g_strdup(secret->data); > } else { > -error_setg(errp, "Either 'file' or 'data' must be provided"); > +error_setg(errp, > + "Either 'syskey' or 'file' or 'data' must be provided"); > } > } > > @@ -298,6 +353,29 @@ qcrypto_secret_prop_get_file(Object *obj, > } > > > +static void > +qcrypto_secret_prop_set_syskey(Object *obj, Visitor *v, > + const char *name, void *opaque, > + Error **errp) > +{ > +QCryptoSecret *secret = QCRYPTO_SECRET(obj); > +int32_t value; > +visit_type_int32(v, name, &value, errp); > +secret->syskey = value; > +} > + > + > +static void > +qcrypto_secret_prop_get_syskey(Object *obj, Visitor *v, > +
Re: [RFC PATCH] crypto/secret: support fetching secrets from Linux keyring
Patchew URL: https://patchew.org/QEMU/20200328114014.6362-1-alex-krasi...@yandex-team.ru/ Hi, This series failed the docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #! /bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-mingw@fedora J=14 NETWORK=1 === TEST SCRIPT END === CC chardev/char-ringbuf.o CC chardev/char-serial.o CC chardev/char-socket.o /tmp/qemu-test/src/crypto/secret.c:22:10: fatal error: asm/unistd.h: No such file or directory #include ^~ compilation terminated. make: *** [/tmp/qemu-test/src/rules.mak:69: crypto/secret.o] Error 1 make: *** Waiting for unfinished jobs Traceback (most recent call last): File "./tests/docker/docker.py", line 664, in --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=4961116ada114c5d93e518eb1e9d8685', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-rgq6jw7h/src/docker-src.2020-03-28-11.34.05.19902:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=4961116ada114c5d93e518eb1e9d8685 make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-rgq6jw7h/src' make: *** [docker-run-test-mingw@fedora] Error 2 real1m49.176s user0m7.677s The full log is available at http://patchew.org/logs/20200328114014.6362-1-alex-krasi...@yandex-team.ru/testing.docker-mingw@fedora/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
[RFC PATCH] crypto/secret: support fetching secrets from Linux keyring
Add the ability for the secret object to obtain secret data from the Linux in-kernel key managment and retention facility, as an extra option to the existing ones: reading from a file or passing directly as a string. The secret is identified by the key serial number. The upper layers need to instantiate the key and make sure the QEMU process has access rights to read it. Signed-off-by: Alexey Krasikov --- crypto/secret.c | 88 +++-- include/crypto/secret.h | 3 ++ 2 files changed, 88 insertions(+), 3 deletions(-) diff --git a/crypto/secret.c b/crypto/secret.c index 1cf0ad0ce8..2e8be6241c 100644 --- a/crypto/secret.c +++ b/crypto/secret.c @@ -19,6 +19,8 @@ */ #include "qemu/osdep.h" +#include +#include #include "crypto/secret.h" #include "crypto/cipher.h" #include "qapi/error.h" @@ -28,6 +30,40 @@ #include "trace.h" +static inline +long keyctl_read(key_serial_t key, uint8_t *buffer, size_t buflen) +{ +#ifdef __NR_keyctl +return syscall(__NR_keyctl, KEYCTL_READ, key, buffer, buflen, 0); +#else +errno = ENOSYS; +return -1; +#endif +} + +static +long keyctl_read_alloc(key_serial_t key, uint8_t **buffer) +{ +uint8_t *loc_buf; +long retcode = keyctl_read(key, NULL, 0); +if (retcode < 0) { +return retcode; +} +loc_buf = g_malloc(retcode + 1); +retcode = keyctl_read(key, loc_buf, retcode + 1); + /* +* We don't have key operations locks between syscalls. +* For example, the key could have been removed or expired. +*/ +if (retcode >= 0) { +loc_buf[retcode] = '\0'; +*buffer = loc_buf; +} else { +g_free(loc_buf); +} +return retcode; +} + static void qcrypto_secret_load_data(QCryptoSecret *secret, uint8_t **output, @@ -41,10 +77,28 @@ qcrypto_secret_load_data(QCryptoSecret *secret, *output = NULL; *outputlen = 0; -if (secret->file) { +if (secret->syskey) { +uint8_t *buffer = NULL; +long retcode; +if (secret->data || secret->file) { +error_setg(errp, + "'syskey', 'file' and 'data' are mutually exclusive"); +return; +} +retcode = keyctl_read_alloc(secret->syskey, &buffer); +if (retcode < 0) { +error_setg_errno(errp, errno, + "Unable to read serial key %08x", + secret->syskey); +return; +} else { +*outputlen = retcode; +*output = buffer; +} +} else if (secret->file) { if (secret->data) { error_setg(errp, - "'file' and 'data' are mutually exclusive"); + "'syskey', 'file' and 'data' are mutually exclusive"); return; } if (!g_file_get_contents(secret->file, &data, &length, &gerr)) { @@ -60,7 +114,8 @@ qcrypto_secret_load_data(QCryptoSecret *secret, *outputlen = strlen(secret->data); *output = (uint8_t *)g_strdup(secret->data); } else { -error_setg(errp, "Either 'file' or 'data' must be provided"); +error_setg(errp, + "Either 'syskey' or 'file' or 'data' must be provided"); } } @@ -298,6 +353,29 @@ qcrypto_secret_prop_get_file(Object *obj, } +static void +qcrypto_secret_prop_set_syskey(Object *obj, Visitor *v, + const char *name, void *opaque, + Error **errp) +{ +QCryptoSecret *secret = QCRYPTO_SECRET(obj); +int32_t value; +visit_type_int32(v, name, &value, errp); +secret->syskey = value; +} + + +static void +qcrypto_secret_prop_get_syskey(Object *obj, Visitor *v, + const char *name, void *opaque, + Error **errp) +{ +QCryptoSecret *secret = QCRYPTO_SECRET(obj); +int32_t value = secret->syskey; +visit_type_int32(v, name, &value, errp); +} + + static void qcrypto_secret_prop_set_iv(Object *obj, const char *value, @@ -384,6 +462,10 @@ qcrypto_secret_class_init(ObjectClass *oc, void *data) qcrypto_secret_prop_get_file, qcrypto_secret_prop_set_file, NULL); +object_class_property_add(oc, "syskey", "key_serial_t", + qcrypto_secret_prop_get_syskey, + qcrypto_secret_prop_set_syskey, + NULL, NULL, NULL); object_class_property_add_str(oc, "keyid", qcrypto_secret_prop_get_keyid, qcrypto_secret_prop_set_keyid, diff --git a/include/crypto/secret.h b/include/crypto/secret.h index 5e07e29bae..9d350e35ed 100644 --- a/include/crypto/secret.h +++ b/include/crypto/secret.h @@ -31,6 +31,8 @@ typedef struct QCryptoSecret