* [Qemu-devel] [PATCH v2 0/5] Add framework for passing secrets to QEMU
@ 2015-11-24 15:02 Daniel P. Berrange
2015-11-24 15:02 ` [Qemu-devel] [PATCH v2 1/5] util: add base64 decoding function Daniel P. Berrange
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Daniel P. Berrange @ 2015-11-24 15:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster
This small series contains just the first two patches for
adding the secrets object to QEMU previously shown here:
https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg04748.html
The QCryptoSecret object provides a QOM object that allows
passing secrets (passwords, encryption keys, etc) to QEMU
in a secure manner, via an external file, or on the CLI
or monitor with encryption. It also uses this to allow
use of encrypted x509 keys with the TLS handling code.
Changed in v2:
- Fix version tag in QAPI schema to be 2.6 (Eric)
- Changed "echo -n" to "printf" (Eric)
- Misc typo fixes (Eric)
- Added a genmeric qbase64_decode() wrapper around
g_base64_decode() that does error checking (Markus)
- Convert callers of g_base64_decode() to qbase64_decode()
to get error checking (Markus)
Daniel P. Berrange (5):
util: add base64 decoding function
qemu-char: convert to use error checked base64 decode
qga: convert to use error checked base64 decode
crypto: add QCryptoSecret object class for password/key handling
crypto: add support for loading encrypted x509 keys
crypto/Makefile.objs | 1 +
crypto/secret.c | 540 ++++++++++++++++++++++++++++++++++++++++++
crypto/tlscredsx509.c | 47 ++++
include/crypto/secret.h | 148 ++++++++++++
include/crypto/tlscredsx509.h | 1 +
include/qemu/base64.h | 56 +++++
qapi-schema.json | 2 -
qapi/crypto.json | 14 ++
qemu-char.c | 8 +-
qemu-options.hx | 85 ++++++-
qga/commands-posix.c | 11 +-
qga/commands-win32.c | 11 +-
qga/commands.c | 13 +-
qmp-commands.hx | 2 -
tests/.gitignore | 2 +
tests/Makefile | 5 +
tests/test-base64.c | 97 ++++++++
tests/test-crypto-secret.c | 446 ++++++++++++++++++++++++++++++++++
util/Makefile.objs | 1 +
util/base64.c | 60 +++++
20 files changed, 1539 insertions(+), 11 deletions(-)
create mode 100644 crypto/secret.c
create mode 100644 include/crypto/secret.h
create mode 100644 include/qemu/base64.h
create mode 100644 tests/test-base64.c
create mode 100644 tests/test-crypto-secret.c
create mode 100644 util/base64.c
--
2.5.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 1/5] util: add base64 decoding function
2015-11-24 15:02 [Qemu-devel] [PATCH v2 0/5] Add framework for passing secrets to QEMU Daniel P. Berrange
@ 2015-11-24 15:02 ` Daniel P. Berrange
2015-11-24 15:54 ` Eric Blake
2015-11-24 15:02 ` [Qemu-devel] [PATCH v2 2/5] qemu-char: convert to use error checked base64 decode Daniel P. Berrange
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Daniel P. Berrange @ 2015-11-24 15:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster
The standard glib provided g_base64_decode doesn't provide any
kind of sensible error checking on its input. Add a QEMU custom
wrapper qbase64_decode which can be used with untrustworthy
input that can contain invalid base64 characters, embedded
NUL characters, or not be NUL terminated at all.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
include/qemu/base64.h | 56 +++++++++++++++++++++++++++++
tests/.gitignore | 1 +
tests/Makefile | 3 ++
tests/test-base64.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++
util/Makefile.objs | 1 +
util/base64.c | 60 +++++++++++++++++++++++++++++++
6 files changed, 218 insertions(+)
create mode 100644 include/qemu/base64.h
create mode 100644 tests/test-base64.c
create mode 100644 util/base64.c
diff --git a/include/qemu/base64.h b/include/qemu/base64.h
new file mode 100644
index 0000000..34f44d9
--- /dev/null
+++ b/include/qemu/base64.h
@@ -0,0 +1,56 @@
+/*
+ * QEMU base64 helpers
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef QEMU_BASE64_H__
+#define QEMU_BASE64_H__
+
+#include "qemu-common.h"
+
+
+/**
+ * qbase64_decode:
+ * @input: the (possibly) base64 encoded text
+ * @in_len: length of @input or -1 if NUL terminated
+ * @out_len: filled with length of decoded data
+ * @errpr: pointer to uninitialized error object
+ *
+ * Attempt to decode the (possibly) base64 encoded
+ * text provided in @input. If the @input text may
+ * contain embedded NUL characters, or may not be
+ * NUL terminated, then @in_len must be set to the
+ * known size of the @input buffer.
+ *
+ * Note that embedded NULs, or lack of a NUL terminator
+ * are considered invalid base64 data and errors
+ * will be reported to this effect.
+ *
+ * If decoding is successful, the decoded data will
+ * be returned and @out_len set to indicate the
+ * number of bytes in the decoded data.
+ *
+ * Returns: the decoded data or NULL
+ */
+uint8_t *qbase64_decode(const char *input,
+ size_t in_len,
+ size_t *out_len,
+ Error **errp);
+
+
+#endif /* QEMU_BUFFER_H__ */
diff --git a/tests/.gitignore b/tests/.gitignore
index 1e55722..00a7fed 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -8,6 +8,7 @@ check-qom-interface
check-qom-proplist
rcutorture
test-aio
+test-base64
test-bitops
test-blockjob-txn
test-coroutine
diff --git a/tests/Makefile b/tests/Makefile
index b937984..0e1289f 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -84,6 +84,7 @@ check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlscredsx509$(EXESUF)
check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlssession$(EXESUF)
check-unit-$(CONFIG_LINUX) += tests/test-qga$(EXESUF)
check-unit-y += tests/test-timed-average$(EXESUF)
+check-unit-y += tests/test-base64$(EXESUF)
check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
@@ -417,6 +418,8 @@ tests/test-vmstate$(EXESUF): tests/test-vmstate.o \
tests/test-timed-average$(EXESUF): tests/test-timed-average.o qemu-timer.o \
libqemuutil.a stubs/clock-warp.o stubs/cpu-get-icount.o \
stubs/notify-event.o stubs/replay.o
+tests/test-base64$(EXESUF): tests/test-base64.o \
+ libqemuutil.a libqemustub.a
tests/test-qapi-types.c tests/test-qapi-types.h :\
$(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
diff --git a/tests/test-base64.c b/tests/test-base64.c
new file mode 100644
index 0000000..3db809f
--- /dev/null
+++ b/tests/test-base64.c
@@ -0,0 +1,97 @@
+/*
+ * QEMU base64 heper test
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include <glib.h>
+
+#include "qemu/base64.h"
+
+
+static void test_base64_good(void)
+{
+ const char *input = "QmVjYXVzZSB3ZSBmb2N1c2VkIG9uIHRoZS"
+ "BzbmFrZSwgd2UgbWlzc2VkIHRoZSBzY29ycGlvbi4=";
+ const char *expect = "Because we focused on the snake, "
+ "we missed the scorpion.";
+
+ size_t len;
+ uint8_t *actual = qbase64_decode(input,
+ -1,
+ &len,
+ &error_abort);
+
+ g_assert(actual != NULL);
+ g_assert_cmpint(len, ==, strlen(expect));
+ g_assert_cmpstr((char *)actual, ==, expect);
+ g_free(actual);
+}
+
+
+static void test_base64_bad(const char *input,
+ size_t input_len)
+{
+ size_t len;
+ Error *err = NULL;
+ uint8_t *actual = qbase64_decode(input,
+ input_len,
+ &len,
+ &err);
+
+ g_assert(err != NULL);
+ g_assert(actual == NULL);
+ g_assert_cmpint(len, ==, 0);
+ error_free(err);
+}
+
+
+static void test_base64_embedded_nul(void)
+{
+ const char input[] = "There's no such\0thing as a free lunch.";
+
+ test_base64_bad(input, G_N_ELEMENTS(input) - 1);
+}
+
+
+static void test_base64_not_nul_terminated(void)
+{
+ char input[] = "There's no such\0thing as a free lunch.";
+ input[G_N_ELEMENTS(input) - 1] = '!';
+
+ test_base64_bad(input, G_N_ELEMENTS(input) - 1);
+}
+
+
+static void test_base64_invalid_chars(void)
+{
+ const char *input = "There's no such thing as a free lunch.";
+
+ test_base64_bad(input, strlen(input));
+}
+
+
+int main(int argc, char **argv)
+{
+ g_test_init(&argc, &argv, NULL);
+ g_test_add_func("/util/base64/good", test_base64_good);
+ g_test_add_func("/util/base64/embedded-nul", test_base64_embedded_nul);
+ g_test_add_func("/util/base64/not-nul-terminated",
+ test_base64_not_nul_terminated);
+ g_test_add_func("/util/base64/invalid-chars", test_base64_invalid_chars);
+ return g_test_run();
+}
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 89dd80e..8620a80 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -30,3 +30,4 @@ util-obj-y += qemu-coroutine-sleep.o
util-obj-y += coroutine-$(CONFIG_COROUTINE_BACKEND).o
util-obj-y += buffer.o
util-obj-y += timed-average.o
+util-obj-y += base64.o
diff --git a/util/base64.c b/util/base64.c
new file mode 100644
index 0000000..2e8a2eb
--- /dev/null
+++ b/util/base64.c
@@ -0,0 +1,60 @@
+/*
+ * QEMU base64 helpers
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include <config-host.h>
+
+#include "qemu/base64.h"
+
+static const char *base64_valid_chars =
+ "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=";
+
+uint8_t *qbase64_decode(const char *input,
+ size_t in_len,
+ size_t *out_len,
+ Error **errp)
+{
+ *out_len = 0;
+
+ if (in_len != -1) {
+ /* Lack of NUL terminator is an error */
+ if (input[in_len] != '\0') {
+ error_setg(errp, "Base64 data is not NUL terminated");
+ return NULL;
+ }
+ /* Check there's no NULs embedded since we expect
+ * this to be valid base64 data */
+ if (memchr(input, '\0', in_len) != NULL) {
+ error_setg(errp, "Base64 data contains embedded NUL characters");
+ return NULL;
+ }
+
+ /* Now we know its a valid nul terminated string
+ * strspn is safe to use... */
+ } else {
+ in_len = strlen(input);
+ }
+
+ if (strspn(input, base64_valid_chars) != in_len) {
+ error_setg(errp, "Base64 data contains invalid characters");
+ return NULL;
+ }
+
+ return g_base64_decode(input, out_len);
+}
--
2.5.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 2/5] qemu-char: convert to use error checked base64 decode
2015-11-24 15:02 [Qemu-devel] [PATCH v2 0/5] Add framework for passing secrets to QEMU Daniel P. Berrange
2015-11-24 15:02 ` [Qemu-devel] [PATCH v2 1/5] util: add base64 decoding function Daniel P. Berrange
@ 2015-11-24 15:02 ` Daniel P. Berrange
2015-11-24 15:56 ` Eric Blake
2015-11-24 15:02 ` [Qemu-devel] [PATCH v2 3/5] qga: " Daniel P. Berrange
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Daniel P. Berrange @ 2015-11-24 15:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster
Switch from using g_base64_decode over to qbase64_decode
in order to get error checking of the base64 input data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
qapi-schema.json | 2 --
qemu-char.c | 8 +++++++-
qmp-commands.hx | 2 --
3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 8b1a423..7d92c04 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -295,8 +295,6 @@
# @format: #optional data encoding (default 'utf8').
# - base64: data must be base64 encoded text. Its binary
# decoding gets written.
-# Bug: invalid base64 is currently not rejected.
-# Whitespace *is* invalid.
# - utf8: data's UTF-8 encoding is written
# - data itself is always Unicode regardless of format, like
# any other string.
diff --git a/qemu-char.c b/qemu-char.c
index 5448b0f..ac01d73 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -32,6 +32,7 @@
#include "qapi/qmp-input-visitor.h"
#include "qapi/qmp-output-visitor.h"
#include "qapi-visit.h"
+#include "qemu/base64.h"
#include <unistd.h>
#include <fcntl.h>
@@ -3259,7 +3260,12 @@ void qmp_ringbuf_write(const char *device, const char *data,
}
if (has_format && (format == DATA_FORMAT_BASE64)) {
- write_data = g_base64_decode(data, &write_count);
+ write_data = qbase64_decode(data, -1,
+ &write_count,
+ errp);
+ if (!write_data) {
+ return;
+ }
} else {
write_data = (uint8_t *)data;
write_count = strlen(data);
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 9d8b42f..1b47cdd 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -512,8 +512,6 @@ Arguments:
- "data": data to write (json-string)
- "format": data format (json-string, optional)
- Possible values: "utf8" (default), "base64"
- Bug: invalid base64 is currently not rejected.
- Whitespace *is* invalid.
Example:
--
2.5.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 3/5] qga: convert to use error checked base64 decode
2015-11-24 15:02 [Qemu-devel] [PATCH v2 0/5] Add framework for passing secrets to QEMU Daniel P. Berrange
2015-11-24 15:02 ` [Qemu-devel] [PATCH v2 1/5] util: add base64 decoding function Daniel P. Berrange
2015-11-24 15:02 ` [Qemu-devel] [PATCH v2 2/5] qemu-char: convert to use error checked base64 decode Daniel P. Berrange
@ 2015-11-24 15:02 ` Daniel P. Berrange
2015-11-24 16:39 ` Eric Blake
2015-11-24 15:02 ` [Qemu-devel] [PATCH v2 4/5] crypto: add QCryptoSecret object class for password/key handling Daniel P. Berrange
2015-11-24 15:02 ` [Qemu-devel] [PATCH v2 5/5] crypto: add support for loading encrypted x509 keys Daniel P. Berrange
4 siblings, 1 reply; 13+ messages in thread
From: Daniel P. Berrange @ 2015-11-24 15:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster
Switch from using g_base64_decode over to qbase64_decode
in order to get error checking of the base64 input data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
qga/commands-posix.c | 11 +++++++++--
qga/commands-win32.c | 11 +++++++++--
qga/commands.c | 13 ++++++++++++-
3 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 0ebd473..e495cc2 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -29,6 +29,7 @@
#include "qemu/queue.h"
#include "qemu/host-utils.h"
#include "qemu/sockets.h"
+#include "qemu/base64.h"
#ifndef CONFIG_HAS_ENVIRON
#ifdef __APPLE__
@@ -496,7 +497,10 @@ GuestFileWrite *qmp_guest_file_write(int64_t handle, const char *buf_b64,
}
fh = gfh->fh;
- buf = g_base64_decode(buf_b64, &buf_len);
+ buf = qbase64_decode(buf_b64, -1, &buf_len, errp);
+ if (!buf) {
+ return NULL;
+ }
if (!has_count) {
count = buf_len;
@@ -1909,7 +1913,10 @@ void qmp_guest_set_user_password(const char *username,
char *chpasswddata = NULL;
size_t chpasswdlen;
- rawpasswddata = (char *)g_base64_decode(password, &rawpasswdlen);
+ rawpasswddata = (char *)qbase64_decode(password, -1, &rawpasswdlen, errp);
+ if (!rawpasswddata) {
+ return;
+ }
rawpasswddata = g_renew(char, rawpasswddata, rawpasswdlen + 1);
rawpasswddata[rawpasswdlen] = '\0';
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 41f6dd9..c9e1773 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -34,6 +34,7 @@
#include "qapi/qmp/qerror.h"
#include "qemu/queue.h"
#include "qemu/host-utils.h"
+#include "qemu/base64.h"
#ifndef SHTDN_REASON_FLAG_PLANNED
#define SHTDN_REASON_FLAG_PLANNED 0x80000000
@@ -357,7 +358,10 @@ GuestFileWrite *qmp_guest_file_write(int64_t handle, const char *buf_b64,
return NULL;
}
fh = gfh->fh;
- buf = g_base64_decode(buf_b64, &buf_len);
+ buf = qbase64_decode(buf_b64, -1, &buf_len, errp);
+ if (!buf) {
+ return NULL;
+ }
if (!has_count) {
count = buf_len;
@@ -1276,7 +1280,10 @@ void qmp_guest_set_user_password(const char *username,
return;
}
- rawpasswddata = (char *)g_base64_decode(password, &rawpasswdlen);
+ rawpasswddata = (char *)qbase64_decode(password, -1, &rawpasswdlen, errp);
+ if (!rawpasswddata) {
+ return;
+ }
rawpasswddata = g_renew(char, rawpasswddata, rawpasswdlen + 1);
rawpasswddata[rawpasswdlen] = '\0';
diff --git a/qga/commands.c b/qga/commands.c
index bb73e7d..58568d8 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -14,6 +14,7 @@
#include "qga/guest-agent-core.h"
#include "qga-qmp-commands.h"
#include "qapi/qmp/qerror.h"
+#include "qemu/base64.h"
/* Maximum captured guest-exec out_data/err_data - 16MB */
#define GUEST_EXEC_MAX_OUTPUT (16*1024*1024)
@@ -393,10 +394,19 @@ GuestExec *qmp_guest_exec(const char *path,
GIOChannel *in_ch, *out_ch, *err_ch;
GSpawnFlags flags;
bool has_output = (has_capture_output && capture_output);
+ uint8_t *input = NULL;
+ size_t ninput = 0;
arglist.value = (char *)path;
arglist.next = has_arg ? arg : NULL;
+ if (has_input_data) {
+ input = qbase64_decode(input_data, -1, &ninput, err);
+ if (!input) {
+ return NULL;
+ }
+ }
+
argv = guest_exec_get_args(&arglist, true);
envp = has_env ? guest_exec_get_args(env, false) : NULL;
@@ -425,7 +435,8 @@ GuestExec *qmp_guest_exec(const char *path,
g_child_watch_add(pid, guest_exec_child_watch, gei);
if (has_input_data) {
- gei->in.data = g_base64_decode(input_data, &gei->in.size);
+ gei->in.data = input;
+ gei->in.size = ninput;
#ifdef G_OS_WIN32
in_ch = g_io_channel_win32_new_fd(in_fd);
#else
--
2.5.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 4/5] crypto: add QCryptoSecret object class for password/key handling
2015-11-24 15:02 [Qemu-devel] [PATCH v2 0/5] Add framework for passing secrets to QEMU Daniel P. Berrange
` (2 preceding siblings ...)
2015-11-24 15:02 ` [Qemu-devel] [PATCH v2 3/5] qga: " Daniel P. Berrange
@ 2015-11-24 15:02 ` Daniel P. Berrange
2015-11-24 18:29 ` Eric Blake
2015-11-24 15:02 ` [Qemu-devel] [PATCH v2 5/5] crypto: add support for loading encrypted x509 keys Daniel P. Berrange
4 siblings, 1 reply; 13+ messages in thread
From: Daniel P. Berrange @ 2015-11-24 15:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster
Introduce a new QCryptoSecret object class which will be used
for providing passwords and keys to other objects which need
sensitive credentials.
The new object can provide secret values directly as properties,
or indirectly via a file. The latter includes support for file
descriptor passing syntax on UNIX platforms. Ordinarily passing
secret values directly as properties is insecure, since they
are visible in process listings, or in log files showing the
CLI args / QMP commands. It is possible to use AES-256-CBC to
encrypt the secret values though, in which case all that is
visible is the ciphertext. For adhoc developer testing though,
it is fine to provide the secrets directly without encryption
so this is not explicitly forbidden.
The anticipated scenario is that libvirtd will create a random
master key per QEMU instance (eg /var/run/libvirt/qemu/$VMNAME.key)
and will use that key to encrypt all passwords it provides to
QEMU via '-object secret,....'. This avoids the need for libvirt
(or other mgmt apps) to worry about file descriptor passing.
It also makes life easier for people who are scripting the
management of QEMU, for whom FD passing is significantly more
complex.
Providing data inline (insecure, only for adhoc dev testing)
$QEMU -object secret,id=sec0,data=letmein
Providing data indirectly in raw format
printf "letmein" > mypasswd.txt
$QEMU -object secret,id=sec0,file=mypasswd.txt
Providing data indirectly in base64 format
$QEMU -object secret,id=sec0,file=mykey.b64,format=base64
Providing data with encryption
$QEMU -object secret,id=master0,file=mykey.b64,format=base64 \
-object secret,id=sec0,data=[base64 ciphertext],\
keyid=master0,iv=[base64 IV],format=base64
Note that 'format' here refers to the format of the ciphertext
data. The decrypted data must always be in raw byte format.
More examples are shown in the updated docs.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
crypto/Makefile.objs | 1 +
crypto/secret.c | 540 +++++++++++++++++++++++++++++++++++++++++++++
include/crypto/secret.h | 148 +++++++++++++
qapi/crypto.json | 14 ++
qemu-options.hx | 77 +++++++
tests/.gitignore | 1 +
tests/Makefile | 2 +
tests/test-crypto-secret.c | 446 +++++++++++++++++++++++++++++++++++++
8 files changed, 1229 insertions(+)
create mode 100644 crypto/secret.c
create mode 100644 include/crypto/secret.h
create mode 100644 tests/test-crypto-secret.c
diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index b2a0e0b..a3135f1 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -7,6 +7,7 @@ crypto-obj-y += tlscreds.o
crypto-obj-y += tlscredsanon.o
crypto-obj-y += tlscredsx509.o
crypto-obj-y += tlssession.o
+crypto-obj-y += secret.o
# Let the userspace emulators avoid linking gnutls/etc
crypto-aes-obj-y = aes.o
diff --git a/crypto/secret.c b/crypto/secret.c
new file mode 100644
index 0000000..fc880f1
--- /dev/null
+++ b/crypto/secret.c
@@ -0,0 +1,540 @@
+/*
+ * QEMU crypto secret support
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "crypto/secret.h"
+#include "crypto/cipher.h"
+#include "qom/object_interfaces.h"
+#include "qemu/base64.h"
+#include "trace.h"
+
+
+static void
+qcrypto_secret_load_data(QCryptoSecret *secret,
+ uint8_t **output,
+ size_t *outputlen,
+ Error **errp)
+{
+ int fd;
+ char *data = NULL;
+ size_t offset = 0;
+ size_t length = 0;
+
+ *output = NULL;
+ *outputlen = 0;
+
+ if (secret->file) {
+ if (secret->data) {
+ error_setg(errp,
+ "'file' and 'data' are mutually exclusive");
+ return;
+ }
+ fd = qemu_open(secret->file, O_RDONLY);
+ if (fd < 0) {
+ error_setg_errno(errp, errno,
+ "Unable to open %s", secret->file);
+ return;
+ }
+ while (length < (1024 * 1024)) { /* Limit secrets to 1 MB */
+ if ((length - offset) < 1024) {
+ length += 1024;
+ data = g_renew(char, data, length);
+ }
+ ssize_t ret = read(fd, data + offset, length - offset);
+ if (ret == 0) {
+ break;
+ }
+ if (ret < 0) {
+ error_setg_errno(errp, errno,
+ "Unable to read from %s", secret->file);
+ close(fd);
+ g_free(data);
+ return;
+ }
+ offset += ret;
+ }
+ close(fd);
+ if (offset) {
+ /* Even though data is raw 8-bit, so may contain
+ * arbitrary NULs, ensure it is explicitly NUL
+ * terminated */
+ *output = g_renew(uint8_t, data, offset + 1);
+ (*output)[offset] = '\0';
+ *outputlen = offset;
+ } else {
+ error_setg(errp, "Secret file %s is empty",
+ secret->file);
+ g_free(data);
+ }
+ } else if (secret->data) {
+ *outputlen = strlen(secret->data);
+ *output = g_new(uint8_t, *outputlen + 1);
+ memcpy(*output, secret->data, *outputlen + 1);
+ } else {
+ error_setg(errp, "Either 'file' or 'data' must be provided");
+ }
+}
+
+
+static void qcrypto_secret_decrypt(QCryptoSecret *secret,
+ const uint8_t *input,
+ size_t inputlen,
+ uint8_t **output,
+ size_t *outputlen,
+ Error **errp)
+{
+ uint8_t *key = NULL, *ciphertext = NULL, *iv = NULL;
+ size_t keylen, ciphertextlen, ivlen;
+ QCryptoCipher *aes = NULL;
+ uint8_t *plaintext = NULL;
+
+ *output = NULL;
+ *outputlen = 0;
+
+ if (qcrypto_secret_lookup(secret->keyid,
+ &key, &keylen,
+ errp) < 0) {
+ goto cleanup;
+ }
+
+ if (keylen != 32) {
+ error_setg(errp, "Key should be 32 bytes in length");
+ goto cleanup;
+ }
+
+ if (!secret->iv) {
+ error_setg(errp, "IV is required to decrypt secret");
+ goto cleanup;
+ }
+
+ iv = (uint8_t *)g_base64_decode(secret->iv, &ivlen);
+ if (ivlen != 16) {
+ error_setg(errp, "IV should be 16 bytes in length not %zu",
+ ivlen);
+ goto cleanup;
+ }
+
+ aes = qcrypto_cipher_new(QCRYPTO_CIPHER_ALG_AES_256,
+ QCRYPTO_CIPHER_MODE_CBC,
+ key, keylen,
+ errp);
+ if (!aes) {
+ goto cleanup;
+ }
+
+ if (qcrypto_cipher_setiv(aes, iv, ivlen, errp) < 0) {
+ goto cleanup;
+ }
+
+ if (secret->format == QCRYPTO_SECRET_FORMAT_BASE64) {
+ ciphertext = qbase64_decode((const gchar*)input,
+ inputlen,
+ &ciphertextlen,
+ errp);
+ if (!ciphertext) {
+ goto cleanup;
+ }
+ plaintext = g_new0(uint8_t, ciphertextlen + 1);
+ } else {
+ ciphertextlen = inputlen;
+ plaintext = g_new0(uint8_t, inputlen + 1);
+ }
+ if (qcrypto_cipher_decrypt(aes,
+ ciphertext ? ciphertext : input,
+ plaintext,
+ ciphertextlen,
+ errp) < 0) {
+ plaintext = NULL;
+ goto cleanup;
+ }
+
+ if (plaintext[ciphertextlen - 1] > 16 ||
+ plaintext[ciphertextlen - 1] > ciphertextlen) {
+ error_setg(errp, "Incorrect number of padding bytes (%d) "
+ "found on decrypted data",
+ (int)plaintext[ciphertextlen - 1]);
+ g_free(plaintext);
+ plaintext = NULL;
+ goto cleanup;
+ }
+
+ /* Even though plaintext may contain arbitrary NUL
+ * ensure it is explicitly NUL terminated.
+ */
+ ciphertextlen -= plaintext[ciphertextlen - 1];
+ plaintext[ciphertextlen] = '\0';
+
+ *output = plaintext;
+ *outputlen = ciphertextlen;
+
+ cleanup:
+ g_free(ciphertext);
+ g_free(iv);
+ g_free(key);
+ qcrypto_cipher_free(aes);
+}
+
+
+static void qcrypto_secret_decode(const uint8_t *input,
+ size_t inputlen,
+ uint8_t **output,
+ size_t *outputlen,
+ Error **errp)
+{
+ *output = qbase64_decode((const gchar*)input,
+ inputlen,
+ outputlen,
+ errp);
+}
+
+
+static void
+qcrypto_secret_prop_set_loaded(Object *obj,
+ bool value,
+ Error **errp)
+{
+ QCryptoSecret *secret = QCRYPTO_SECRET(obj);
+
+ if (value) {
+ Error *local_err = NULL;
+ uint8_t *input = NULL;
+ size_t inputlen = 0;
+ uint8_t *output = NULL;
+ size_t outputlen = 0;
+
+ qcrypto_secret_load_data(secret, &input, &inputlen, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ if (secret->keyid) {
+ qcrypto_secret_decrypt(secret, input, inputlen,
+ &output, &outputlen, &local_err);
+ g_free(input);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ input = output;
+ inputlen = outputlen;
+ } else {
+ if (secret->format != QCRYPTO_SECRET_FORMAT_RAW) {
+ qcrypto_secret_decode(input, inputlen,
+ &output, &outputlen, &local_err);
+ g_free(input);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ input = output;
+ inputlen = outputlen;
+ }
+ }
+
+ secret->rawdata = input;
+ secret->rawlen = inputlen;
+ } else {
+ g_free(secret->rawdata);
+ secret->rawlen = 0;
+ }
+}
+
+
+static bool
+qcrypto_secret_prop_get_loaded(Object *obj,
+ Error **errp G_GNUC_UNUSED)
+{
+ QCryptoSecret *secret = QCRYPTO_SECRET(obj);
+ return secret->data != NULL;
+}
+
+
+static void
+qcrypto_secret_prop_set_format(Object *obj,
+ int value,
+ Error **errp G_GNUC_UNUSED)
+{
+ QCryptoSecret *creds = QCRYPTO_SECRET(obj);
+
+ creds->format = value;
+}
+
+
+static int
+qcrypto_secret_prop_get_format(Object *obj,
+ Error **errp G_GNUC_UNUSED)
+{
+ QCryptoSecret *creds = QCRYPTO_SECRET(obj);
+
+ return creds->format;
+}
+
+
+static void
+qcrypto_secret_prop_set_data(Object *obj,
+ const char *value,
+ Error **errp)
+{
+ QCryptoSecret *secret = QCRYPTO_SECRET(obj);
+
+ g_free(secret->data);
+ secret->data = g_strdup(value);
+}
+
+
+static char *
+qcrypto_secret_prop_get_data(Object *obj,
+ Error **errp)
+{
+ QCryptoSecret *secret = QCRYPTO_SECRET(obj);
+ return g_strdup(secret->data);
+}
+
+
+static void
+qcrypto_secret_prop_set_file(Object *obj,
+ const char *value,
+ Error **errp)
+{
+ QCryptoSecret *secret = QCRYPTO_SECRET(obj);
+
+ g_free(secret->file);
+ secret->file = g_strdup(value);
+}
+
+
+static char *
+qcrypto_secret_prop_get_file(Object *obj,
+ Error **errp)
+{
+ QCryptoSecret *secret = QCRYPTO_SECRET(obj);
+ return g_strdup(secret->file);
+}
+
+
+static void
+qcrypto_secret_prop_set_iv(Object *obj,
+ const char *value,
+ Error **errp)
+{
+ QCryptoSecret *secret = QCRYPTO_SECRET(obj);
+
+ g_free(secret->iv);
+ secret->iv = g_strdup(value);
+}
+
+
+static char *
+qcrypto_secret_prop_get_iv(Object *obj,
+ Error **errp)
+{
+ QCryptoSecret *secret = QCRYPTO_SECRET(obj);
+ return g_strdup(secret->iv);
+}
+
+
+static void
+qcrypto_secret_prop_set_keyid(Object *obj,
+ const char *value,
+ Error **errp)
+{
+ QCryptoSecret *secret = QCRYPTO_SECRET(obj);
+
+ g_free(secret->keyid);
+ secret->keyid = g_strdup(value);
+}
+
+
+static char *
+qcrypto_secret_prop_get_keyid(Object *obj,
+ Error **errp)
+{
+ QCryptoSecret *secret = QCRYPTO_SECRET(obj);
+ return g_strdup(secret->keyid);
+}
+
+
+static void
+qcrypto_secret_complete(UserCreatable *uc, Error **errp)
+{
+ object_property_set_bool(OBJECT(uc), true, "loaded", errp);
+}
+
+
+static void
+qcrypto_secret_init(Object *obj)
+{
+ object_property_add_bool(obj, "loaded",
+ qcrypto_secret_prop_get_loaded,
+ qcrypto_secret_prop_set_loaded,
+ NULL);
+ object_property_add_enum(obj, "format",
+ "QCryptoSecretFormat",
+ QCryptoSecretFormat_lookup,
+ qcrypto_secret_prop_get_format,
+ qcrypto_secret_prop_set_format,
+ NULL);
+ object_property_add_str(obj, "data",
+ qcrypto_secret_prop_get_data,
+ qcrypto_secret_prop_set_data,
+ NULL);
+ object_property_add_str(obj, "file",
+ qcrypto_secret_prop_get_file,
+ qcrypto_secret_prop_set_file,
+ NULL);
+ object_property_add_str(obj, "keyid",
+ qcrypto_secret_prop_get_keyid,
+ qcrypto_secret_prop_set_keyid,
+ NULL);
+ object_property_add_str(obj, "iv",
+ qcrypto_secret_prop_get_iv,
+ qcrypto_secret_prop_set_iv,
+ NULL);
+}
+
+
+static void
+qcrypto_secret_finalize(Object *obj)
+{
+ QCryptoSecret *secret = QCRYPTO_SECRET(obj);
+
+ g_free(secret->iv);
+ g_free(secret->file);
+ g_free(secret->keyid);
+ g_free(secret->rawdata);
+ g_free(secret->data);
+}
+
+static void
+qcrypto_secret_class_init(ObjectClass *oc, void *data)
+{
+ UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
+
+ ucc->complete = qcrypto_secret_complete;
+}
+
+
+int qcrypto_secret_lookup(const char *secretid,
+ uint8_t **data,
+ size_t *datalen,
+ Error **errp)
+{
+ Object *obj;
+ QCryptoSecret *secret;
+
+ obj = object_resolve_path_component(
+ object_get_objects_root(), secretid);
+ if (!obj) {
+ error_setg(errp, "No secret with id '%s'", secretid);
+ return -1;
+ }
+
+ secret = (QCryptoSecret *)
+ object_dynamic_cast(obj,
+ TYPE_QCRYPTO_SECRET);
+ if (!secret) {
+ error_setg(errp, "Object with id '%s' is not a secret",
+ secretid);
+ return -1;
+ }
+
+ if (!secret->rawdata) {
+ error_setg(errp, "Secret with id '%s' has no data",
+ secretid);
+ return -1;
+ }
+
+ *data = g_new0(uint8, secret->rawlen + 1);
+ memcpy(*data, secret->rawdata, secret->rawlen);
+ (*data)[secret->rawlen] = '\0';
+ *datalen = secret->rawlen;
+
+ return 0;
+}
+
+
+char *qcrypto_secret_lookup_as_utf8(const char *secretid,
+ Error **errp)
+{
+ uint8_t *data;
+ size_t datalen;
+
+ if (qcrypto_secret_lookup(secretid,
+ &data,
+ &datalen,
+ errp) < 0) {
+ return NULL;
+ }
+
+ if (!g_utf8_validate((const gchar*)data, datalen, NULL)) {
+ error_setg(errp,
+ "Data from secret %s is not valid UTF-8",
+ secretid);
+ g_free(data);
+ return NULL;
+ }
+
+ return (char *)data;
+}
+
+
+char *qcrypto_secret_lookup_as_base64(const char *secretid,
+ Error **errp)
+{
+ uint8_t *data;
+ size_t datalen;
+ char *ret;
+
+ if (qcrypto_secret_lookup(secretid,
+ &data,
+ &datalen,
+ errp) < 0) {
+ return NULL;
+ }
+
+ ret = g_base64_encode(data, datalen);
+ g_free(data);
+ return ret;
+}
+
+
+static const TypeInfo qcrypto_secret_info = {
+ .parent = TYPE_OBJECT,
+ .name = TYPE_QCRYPTO_SECRET,
+ .instance_size = sizeof(QCryptoSecret),
+ .instance_init = qcrypto_secret_init,
+ .instance_finalize = qcrypto_secret_finalize,
+ .class_size = sizeof(QCryptoSecretClass),
+ .class_init = qcrypto_secret_class_init,
+ .interfaces = (InterfaceInfo[]) {
+ { TYPE_USER_CREATABLE },
+ { }
+ }
+};
+
+
+static void
+qcrypto_secret_register_types(void)
+{
+ type_register_static(&qcrypto_secret_info);
+}
+
+
+type_init(qcrypto_secret_register_types);
diff --git a/include/crypto/secret.h b/include/crypto/secret.h
new file mode 100644
index 0000000..38abc50
--- /dev/null
+++ b/include/crypto/secret.h
@@ -0,0 +1,148 @@
+/*
+ * QEMU crypto secret support
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef QCRYPTO_SECRET_H__
+#define QCRYPTO_SECRET_H__
+
+#include "qemu-common.h"
+#include "qapi/error.h"
+#include "qom/object.h"
+
+#define TYPE_QCRYPTO_SECRET "secret"
+#define QCRYPTO_SECRET(obj) \
+ OBJECT_CHECK(QCryptoSecret, (obj), TYPE_QCRYPTO_SECRET)
+
+typedef struct QCryptoSecret QCryptoSecret;
+typedef struct QCryptoSecretClass QCryptoSecretClass;
+
+/**
+ * QCryptoSecret:
+ *
+ * The QCryptoSecret object provides storage of secrets,
+ * which may be user passwords, encryption keys or any
+ * other kind of sensitive data that is represented as
+ * a sequence of bytes.
+ *
+ * The sensitive data associated with the secret can
+ * be provided directly via the 'data' property, or
+ * indirectly via the 'file' property. In the latter
+ * case there is support for file descriptor passing
+ * via the usual /dev/fdset/NN syntax that QEMU uses.
+ *
+ * The data for a secret can be provided in two formats,
+ * either as a UTF-8 string (the default), or as base64
+ * encoded 8-bit binary data. The latter is appropriate
+ * for raw encryption keys, while the former is appropriate
+ * for user entered passwords.
+ *
+ * The data may be optionally encrypted with AES-256-CBC,
+ * and the decryption key provided by another
+ * QCryptoSecret instance identified by the 'keyid'
+ * property. When passing sensitive data directly
+ * via the 'data' property it is strongly recommended
+ * to use the AES encryption facility to prevent the
+ * sensitive data being exposed in the process listing
+ * or system log files.
+ *
+ * Providing data directly, insecurely (suitable for
+ * adhoc developer testing only)
+ *
+ * $QEMU -object secret,id=sec0,data=letmein
+ *
+ * Providing data indirectly:
+ *
+ * # printf "letmein" > password.txt
+ * # $QEMU \
+ * -object secret,id=sec0,file=password.txt
+ *
+ * Using a master encryption key with data.
+ *
+ * The master key needs to be created as 32 secure
+ * random bytes (optionally base64 encoded)
+ *
+ * # openssl rand -base64 32 > key.b64
+ * # KEY=$(base64 -d key.b64 | hexdump -v -e '/1 "%02X"')
+ *
+ * Each secret to be encrypted needs to have a random
+ * initialization vector generated. These do not need
+ * to be kept secret
+ *
+ * # openssl rand -base64 16 > iv.b64
+ * # IV=$(base64 -d iv.b64 | hexdump -v -e '/1 "%02X"')
+ *
+ * A secret to be defined can now be encrypted
+ *
+ * # SECRET=$(printf "letmein" |
+ * openssl enc -aes-256-cbc -a -K $KEY -iv $IV)
+ *
+ * When launching QEMU, create a master secret pointing
+ * to key.b64 and specify that to be used to decrypt
+ * the user password
+ *
+ * # $QEMU \
+ * -object secret,id=secmaster0,format=base64,file=key.b64 \
+ * -object secret,id=sec0,keyid=secmaster0,format=base64,\
+ * data=$SECRET,iv=$(<iv.b64)
+ *
+ * When encrypting, the data can still be provided via an
+ * external file, in which case it is possible to use either
+ * raw binary data, or base64 encoded. This example uses
+ * raw format
+ *
+ * # printf "letmein" |
+ * openssl enc -aes-256-cbc -K $KEY -iv $IV -o pw.aes
+ * # $QEMU \
+ * -object secret,id=secmaster0,format=base64,file=key.b64 \
+ * -object secret,id=sec0,keyid=secmaster0,\
+ * file=pw.aes,iv=$(<iv.b64)
+ *
+ * Note that the ciphertext can be in either raw or base64
+ * format, as indicated by the 'format' parameter, but the
+ * plaintext resulting from decryption is expected to always
+ * be in raw format.
+ */
+
+struct QCryptoSecret {
+ Object parent_obj;
+ uint8_t *rawdata;
+ size_t rawlen;
+ QCryptoSecretFormat format;
+ char *data;
+ char *file;
+ char *keyid;
+ char *iv;
+};
+
+
+struct QCryptoSecretClass {
+ ObjectClass parent_class;
+};
+
+
+extern int qcrypto_secret_lookup(const char *secretid,
+ uint8_t **data,
+ size_t *datalen,
+ Error **errp);
+extern char *qcrypto_secret_lookup_as_utf8(const char *secretid,
+ Error **errp);
+extern char *qcrypto_secret_lookup_as_base64(const char *secretid,
+ Error **errp);
+
+#endif /* QCRYPTO_SECRET_H__ */
diff --git a/qapi/crypto.json b/qapi/crypto.json
index b058b14..4012659 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -19,3 +19,17 @@
{ 'enum': 'QCryptoTLSCredsEndpoint',
'prefix': 'QCRYPTO_TLS_CREDS_ENDPOINT',
'data': ['client', 'server']}
+
+
+##
+# QCryptoSecretFormat:
+#
+# The data format that the secret is provided in
+#
+# @raw: raw bytes. When encoded in JSON only valid UTF-8 sequences can be used
+# @base64: arbitrary base64 encoded binary data
+# Since: 2.6
+##
+{ 'enum': 'QCryptoSecretFormat',
+ 'prefix': 'QCRYPTO_SECRET_FORMAT',
+ 'data': ['raw', 'base64']}
diff --git a/qemu-options.hx b/qemu-options.hx
index 0eea4ee..41c0e59 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3677,6 +3677,83 @@ Dump the network traffic on netdev @var{dev} to the file specified by
The file format is libpcap, so it can be analyzed with tools such as tcpdump
or Wireshark.
+@item -object secret,id=@var{id},data=@var{string},format=@var{raw|base64}[,keyid=@var{secretid},iv=@var{string}]
+@item -object secret,id=@var{id},file=@var{filename},format=@var{raw|base64}[,keyid=@var{secretid},iv=@var{string}]
+
+Defines a secret to store a password, encryption key, or some other sensitive
+data. The senstive data can either be passed directly via the @var{data}
+parameter, or indirectly via the @var{file} parameter. Using the @var{data}
+parameter is insecure unless the sensitive data is encrypted.
+
+The sensitive data can be provided in raw format (the default), or base64.
+When encoded as JSON, the raw format only supports valid UTF-8 characters,
+so base64 is recommended for sending binary data. QEMU will convert from
+which ever format is provided to the format it needs internally. eg, an
+RBD password can be provided in raw format, even though it will be base64
+encoded when passed onto the RBD sever.
+
+For added protection, it is possible to encrypt the data associated with
+a secret using the AES-256-CBC cipher. Use of encryption is indicated
+by providing the @var{keyid} and @var{iv} parameters. The @var{keyid}
+parameter provides the ID of a previously defined secret that contains
+the AES-256 decryption key. This key should be 32-bytes long and be
+base64 encoded. The @var{iv} parameter provides the random initialization
+vector used for encryption of this particular secret and should be a
+base64 encrypted string of the 32-byte IV.
+
+The simplest (insecure) usage is to provide the secret inline
+
+@example
+
+ # $QEMU -object secret,id=sec0,data=letmein,format=raw
+
+@end example
+
+The simplest secure usage is to provide the secret via a file
+
+ # echo -n "letmein" > mypasswd.txt
+ # $QEMU -object secret,id=sec0,file=mypasswd.txt,format=raw
+
+For greater security, AES-256-CBC should be used. To illustrate usage,
+consider the openssl command line tool which can encrypt the data. Note
+that when encrypting, the plaintext must be padded to the cipher block
+size (32 bytes) using the standard PKCS#5/6 compatible padding algorithm.
+
+First a master key needs to be created in base64 encoding:
+
+@example
+ # openssl rand -base64 32 > key.b64
+ # KEY=$(base64 -d key.b64 | hexdump -v -e '/1 "%02X"')
+@end example
+
+Each secret to be encrypted needs to have a random initialization vector
+generated. These do not need to be kept secret
+
+@example
+ # openssl rand -base64 16 > iv.b64
+ # IV=$(base64 -d iv.b64 | hexdump -v -e '/1 "%02X"')
+@end example
+
+The secret to be defined can now be encrypted, in this case we're
+telling openssl to base64 encode the result, but it could be left
+as raw bytes if desired.
+
+@example
+ # SECRET=$(echo -n "letmein" |
+ openssl enc -aes-256-cbc -a -K $KEY -iv $IV)
+@end example
+
+When launching QEMU, create a master secret pointing to @code{key.b64}
+and specify that to be used to decrypt the user password. Pass the
+contents of @code{iv.b64} to the second secret
+
+@example
+ # $QEMU \
+ -object secret,id=secmaster0,format=base64,file=key.b64 \
+ -object secret,id=sec0,keyid=secmaster0,format=base64,\
+ data=$SECRET,iv=$(<iv.b64)
+@end example
+
@end table
ETEXI
diff --git a/tests/.gitignore b/tests/.gitignore
index 00a7fed..dbe7c06 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -14,6 +14,7 @@ test-blockjob-txn
test-coroutine
test-crypto-cipher
test-crypto-hash
+test-crypto-secret
test-crypto-tlscredsx509
test-crypto-tlscredsx509-work/
test-crypto-tlscredsx509-certs/
diff --git a/tests/Makefile b/tests/Makefile
index 0e1289f..3f22c68 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -80,6 +80,7 @@ check-unit-y += tests/test-write-threshold$(EXESUF)
gcov-files-test-write-threshold-y = block/write-threshold.c
check-unit-$(CONFIG_GNUTLS_HASH) += tests/test-crypto-hash$(EXESUF)
check-unit-y += tests/test-crypto-cipher$(EXESUF)
+check-unit-y += tests/test-crypto-secret$(EXESUF)
check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlscredsx509$(EXESUF)
check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlssession$(EXESUF)
check-unit-$(CONFIG_LINUX) += tests/test-qga$(EXESUF)
@@ -461,6 +462,7 @@ tests/test-mul64$(EXESUF): tests/test-mul64.o $(test-util-obj-y)
tests/test-bitops$(EXESUF): tests/test-bitops.o $(test-util-obj-y)
tests/test-crypto-hash$(EXESUF): tests/test-crypto-hash.o $(test-crypto-obj-y)
tests/test-crypto-cipher$(EXESUF): tests/test-crypto-cipher.o $(test-crypto-obj-y)
+tests/test-crypto-secret$(EXESUF): tests/test-crypto-secret.o $(test-crypto-obj-y)
tests/crypto-tls-x509-helpers.o-cflags := $(TASN1_CFLAGS)
tests/crypto-tls-x509-helpers.o-libs := $(TASN1_LIBS)
diff --git a/tests/test-crypto-secret.c b/tests/test-crypto-secret.c
new file mode 100644
index 0000000..6743608
--- /dev/null
+++ b/tests/test-crypto-secret.c
@@ -0,0 +1,446 @@
+/*
+ * QEMU Crypto secret handling
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include <glib.h>
+
+#include "crypto/init.h"
+#include "crypto/secret.h"
+
+static void test_secret_direct(void)
+{
+ Object *sec = object_new_with_props(
+ TYPE_QCRYPTO_SECRET,
+ object_get_objects_root(),
+ "sec0",
+ &error_abort,
+ "data", "123456",
+ NULL);
+
+ char *pw = qcrypto_secret_lookup_as_utf8("sec0",
+ &error_abort);
+
+ g_assert_cmpstr(pw, ==, "123456");
+
+ object_unparent(sec);
+ g_free(pw);
+}
+
+
+static void test_secret_indirect_good(void)
+{
+ Object *sec;
+ char *fname = NULL;
+ int fd = g_file_open_tmp("secretXXXXXX",
+ &fname,
+ NULL);
+
+ g_assert(fd >= 0);
+ g_assert_nonnull(fname);
+
+ g_assert(write(fd, "123456", 6) == 6);
+
+ sec = object_new_with_props(
+ TYPE_QCRYPTO_SECRET,
+ object_get_objects_root(),
+ "sec0",
+ &error_abort,
+ "file", fname,
+ NULL);
+
+ char *pw = qcrypto_secret_lookup_as_utf8("sec0",
+ &error_abort);
+
+ g_assert_cmpstr(pw, ==, "123456");
+
+ object_unparent(sec);
+ g_free(pw);
+ close(fd);
+ g_free(fname);
+}
+
+
+static void test_secret_indirect_badfile(void)
+{
+ Object *sec = object_new_with_props(
+ TYPE_QCRYPTO_SECRET,
+ object_get_objects_root(),
+ "sec0",
+ NULL,
+ "file", "does-not-exist",
+ NULL);
+
+ g_assert(sec == NULL);
+}
+
+
+static void test_secret_indirect_emptyfile(void)
+{
+ Object *sec;
+ char *fname = NULL;
+ int fd = g_file_open_tmp("secretXXXXXX",
+ &fname,
+ NULL);
+
+ g_assert(fd >= 0);
+ g_assert_nonnull(fname);
+
+ sec = object_new_with_props(
+ TYPE_QCRYPTO_SECRET,
+ object_get_objects_root(),
+ "sec0",
+ NULL,
+ "file", fname,
+ NULL);
+
+ close(fd);
+ g_free(fname);
+ g_assert(sec == NULL);
+}
+
+
+static void test_secret_noconv_base64_good(void)
+{
+ Object *sec = object_new_with_props(
+ TYPE_QCRYPTO_SECRET,
+ object_get_objects_root(),
+ "sec0",
+ &error_abort,
+ "data", "MTIzNDU2",
+ "format", "base64",
+ NULL);
+
+ char *pw = qcrypto_secret_lookup_as_base64("sec0",
+ &error_abort);
+
+ g_assert_cmpstr(pw, ==, "MTIzNDU2");
+
+ object_unparent(sec);
+ g_free(pw);
+}
+
+
+static void test_secret_noconv_base64_bad(void)
+{
+ Object *sec = object_new_with_props(
+ TYPE_QCRYPTO_SECRET,
+ object_get_objects_root(),
+ "sec0",
+ NULL,
+ "data", "MTI$NDU2",
+ "format", "base64",
+ NULL);
+
+ g_assert(sec == NULL);
+}
+
+
+static void test_secret_noconv_utf8(void)
+{
+ Object *sec = object_new_with_props(
+ TYPE_QCRYPTO_SECRET,
+ object_get_objects_root(),
+ "sec0",
+ &error_abort,
+ "data", "123456",
+ "format", "raw",
+ NULL);
+
+ char *pw = qcrypto_secret_lookup_as_utf8("sec0",
+ &error_abort);
+
+ g_assert_cmpstr(pw, ==, "123456");
+
+ object_unparent(sec);
+ g_free(pw);
+}
+
+
+static void test_secret_conv_base64_utf8valid(void)
+{
+ Object *sec = object_new_with_props(
+ TYPE_QCRYPTO_SECRET,
+ object_get_objects_root(),
+ "sec0",
+ &error_abort,
+ "data", "MTIzNDU2",
+ "format", "base64",
+ NULL);
+
+ char *pw = qcrypto_secret_lookup_as_utf8("sec0",
+ &error_abort);
+
+ g_assert_cmpstr(pw, ==, "123456");
+
+ object_unparent(sec);
+ g_free(pw);
+}
+
+
+static void test_secret_conv_base64_utf8invalid(void)
+{
+ Object *sec = object_new_with_props(
+ TYPE_QCRYPTO_SECRET,
+ object_get_objects_root(),
+ "sec0",
+ &error_abort,
+ "data", "f0VMRgIBAQAAAA==",
+ "format", "base64",
+ NULL);
+
+ char *pw = qcrypto_secret_lookup_as_utf8("sec0",
+ NULL);
+ g_assert(pw == NULL);
+
+ object_unparent(sec);
+}
+
+
+static void test_secret_conv_utf8_base64(void)
+{
+ Object *sec = object_new_with_props(
+ TYPE_QCRYPTO_SECRET,
+ object_get_objects_root(),
+ "sec0",
+ &error_abort,
+ "data", "123456",
+ NULL);
+
+ char *pw = qcrypto_secret_lookup_as_base64("sec0",
+ &error_abort);
+
+ g_assert_cmpstr(pw, ==, "MTIzNDU2");
+
+ object_unparent(sec);
+ g_free(pw);
+}
+
+
+static void test_secret_crypt_raw(void)
+{
+ Object *master = object_new_with_props(
+ TYPE_QCRYPTO_SECRET,
+ object_get_objects_root(),
+ "master",
+ &error_abort,
+ "data", "9miloPQCzGy+TL6aonfzVcptibCmCIhKzrnlfwiWivk=",
+ "format", "base64",
+ NULL);
+ Object *sec = object_new_with_props(
+ TYPE_QCRYPTO_SECRET,
+ object_get_objects_root(),
+ "sec0",
+ &error_abort,
+ "data",
+ "\xCC\xBF\xF7\x09\x46\x19\x0B\x52\x2A\x3A\xB4\x6B\xCD\x7A\xB0\xB0",
+ "format", "raw",
+ "keyid", "master",
+ "iv", "0I7Gw/TKuA+Old2W2apQ3g==",
+ NULL);
+
+ char *pw = qcrypto_secret_lookup_as_utf8("sec0",
+ &error_abort);
+
+ g_assert_cmpstr(pw, ==, "123456");
+
+ object_unparent(sec);
+ object_unparent(master);
+ g_free(pw);
+}
+
+
+static void test_secret_crypt_base64(void)
+{
+ Object *master = object_new_with_props(
+ TYPE_QCRYPTO_SECRET,
+ object_get_objects_root(),
+ "master",
+ &error_abort,
+ "data", "9miloPQCzGy+TL6aonfzVcptibCmCIhKzrnlfwiWivk=",
+ "format", "base64",
+ NULL);
+ Object *sec = object_new_with_props(
+ TYPE_QCRYPTO_SECRET,
+ object_get_objects_root(),
+ "sec0",
+ &error_abort,
+ "data", "zL/3CUYZC1IqOrRrzXqwsA==",
+ "format", "base64",
+ "keyid", "master",
+ "iv", "0I7Gw/TKuA+Old2W2apQ3g==",
+ NULL);
+
+ char *pw = qcrypto_secret_lookup_as_utf8("sec0",
+ &error_abort);
+
+ g_assert_cmpstr(pw, ==, "123456");
+
+ object_unparent(sec);
+ object_unparent(master);
+ g_free(pw);
+}
+
+
+static void test_secret_crypt_short_key(void)
+{
+ Object *master = object_new_with_props(
+ TYPE_QCRYPTO_SECRET,
+ object_get_objects_root(),
+ "master",
+ &error_abort,
+ "data", "9miloPQCzGy+TL6aonfzVc",
+ "format", "base64",
+ NULL);
+ Object *sec = object_new_with_props(
+ TYPE_QCRYPTO_SECRET,
+ object_get_objects_root(),
+ "sec0",
+ NULL,
+ "data", "zL/3CUYZC1IqOrRrzXqwsA==",
+ "format", "raw",
+ "keyid", "master",
+ "iv", "0I7Gw/TKuA+Old2W2apQ3g==",
+ NULL);
+
+ g_assert(sec == NULL);
+ object_unparent(master);
+}
+
+
+static void test_secret_crypt_short_iv(void)
+{
+ Object *master = object_new_with_props(
+ TYPE_QCRYPTO_SECRET,
+ object_get_objects_root(),
+ "master",
+ &error_abort,
+ "data", "9miloPQCzGy+TL6aonfzVcptibCmCIhKzrnlfwiWivk=",
+ "format", "base64",
+ NULL);
+ Object *sec = object_new_with_props(
+ TYPE_QCRYPTO_SECRET,
+ object_get_objects_root(),
+ "sec0",
+ NULL,
+ "data", "zL/3CUYZC1IqOrRrzXqwsA==",
+ "format", "raw",
+ "keyid", "master",
+ "iv", "0I7Gw/TKuA+Old2W2a",
+ NULL);
+
+ g_assert(sec == NULL);
+ object_unparent(master);
+}
+
+
+static void test_secret_crypt_missing_iv(void)
+{
+ Object *master = object_new_with_props(
+ TYPE_QCRYPTO_SECRET,
+ object_get_objects_root(),
+ "master",
+ &error_abort,
+ "data", "9miloPQCzGy+TL6aonfzVcptibCmCIhKzrnlfwiWivk=",
+ "format", "base64",
+ NULL);
+ Object *sec = object_new_with_props(
+ TYPE_QCRYPTO_SECRET,
+ object_get_objects_root(),
+ "sec0",
+ NULL,
+ "data", "zL/3CUYZC1IqOrRrzXqwsA==",
+ "format", "raw",
+ "keyid", "master",
+ NULL);
+
+ g_assert(sec == NULL);
+ object_unparent(master);
+}
+
+
+static void test_secret_crypt_bad_iv(void)
+{
+ Object *master = object_new_with_props(
+ TYPE_QCRYPTO_SECRET,
+ object_get_objects_root(),
+ "master",
+ &error_abort,
+ "data", "9miloPQCzGy+TL6aonfzVcptibCmCIhKzrnlfwiWivk=",
+ "format", "base64",
+ NULL);
+ Object *sec = object_new_with_props(
+ TYPE_QCRYPTO_SECRET,
+ object_get_objects_root(),
+ "sec0",
+ NULL,
+ "data", "zL/3CUYZC1IqOrRrzXqwsA==",
+ "format", "raw",
+ "keyid", "master",
+ "iv", "0I7Gw/TK$$uA+Old2W2a",
+ NULL);
+
+ g_assert(sec == NULL);
+ object_unparent(master);
+}
+
+
+int main(int argc, char **argv)
+{
+ module_call_init(MODULE_INIT_QOM);
+ g_test_init(&argc, &argv, NULL);
+
+ g_assert(qcrypto_init(NULL) == 0);
+
+ g_test_add_func("/crypto/secret/direct",
+ test_secret_direct);
+ g_test_add_func("/crypto/secret/indirect/good",
+ test_secret_indirect_good);
+ g_test_add_func("/crypto/secret/indirect/badfile",
+ test_secret_indirect_badfile);
+ g_test_add_func("/crypto/secret/indirect/emptyfile",
+ test_secret_indirect_emptyfile);
+
+ g_test_add_func("/crypto/secret/noconv/base64/good",
+ test_secret_noconv_base64_good);
+ g_test_add_func("/crypto/secret/noconv/base64/bad",
+ test_secret_noconv_base64_bad);
+ g_test_add_func("/crypto/secret/noconv/utf8",
+ test_secret_noconv_utf8);
+ g_test_add_func("/crypto/secret/conv/base64/utf8valid",
+ test_secret_conv_base64_utf8valid);
+ g_test_add_func("/crypto/secret/conv/base64/utf8invalid",
+ test_secret_conv_base64_utf8invalid);
+ g_test_add_func("/crypto/secret/conv/utf8/base64",
+ test_secret_conv_utf8_base64);
+
+ g_test_add_func("/crypto/secret/crypt/raw",
+ test_secret_crypt_raw);
+ g_test_add_func("/crypto/secret/crypt/base64",
+ test_secret_crypt_base64);
+ g_test_add_func("/crypto/secret/crypt/shortkey",
+ test_secret_crypt_short_key);
+ g_test_add_func("/crypto/secret/crypt/shortiv",
+ test_secret_crypt_short_iv);
+ g_test_add_func("/crypto/secret/crypt/missingiv",
+ test_secret_crypt_missing_iv);
+ g_test_add_func("/crypto/secret/crypt/badiv",
+ test_secret_crypt_bad_iv);
+
+ return g_test_run();
+}
--
2.5.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 5/5] crypto: add support for loading encrypted x509 keys
2015-11-24 15:02 [Qemu-devel] [PATCH v2 0/5] Add framework for passing secrets to QEMU Daniel P. Berrange
` (3 preceding siblings ...)
2015-11-24 15:02 ` [Qemu-devel] [PATCH v2 4/5] crypto: add QCryptoSecret object class for password/key handling Daniel P. Berrange
@ 2015-11-24 15:02 ` Daniel P. Berrange
2015-11-24 18:33 ` Eric Blake
4 siblings, 1 reply; 13+ messages in thread
From: Daniel P. Berrange @ 2015-11-24 15:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster
Make use of the QCryptoSecret object to support loading of
encrypted x509 keys. The optional 'passwordid' parameter
to the tls-creds-x509 object type, provides the ID of a
secret object instance that holds the decryption password
for the PEM file.
# printf "123456" > mypasswd.txt
# $QEMU \
-object secret,id=sec0,filename=mypasswd.txt \
-object tls-creds-x509,passwordid=sec0,id=creds0,\
dir=/home/berrange/.pki/qemu,endpoint=server \
-vnc :1,tls-creds=creds0
This requires QEMU to be linked to GNUTLS >= 3.1.11. If
GNUTLS is too old an error will be reported if an attempt
is made to pass a decryption password.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
crypto/tlscredsx509.c | 47 +++++++++++++++++++++++++++++++++++++++++++
include/crypto/tlscredsx509.h | 1 +
qemu-options.hx | 8 +++++++-
3 files changed, 55 insertions(+), 1 deletion(-)
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index d080deb..00bb299 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -20,6 +20,7 @@
#include "crypto/tlscredsx509.h"
#include "crypto/tlscredspriv.h"
+#include "crypto/secret.h"
#include "qom/object_interfaces.h"
#include "trace.h"
@@ -606,9 +607,29 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
}
if (cert != NULL && key != NULL) {
+#if GNUTLS_VERSION_NUMBER >= 0x030111
+ char *password = NULL;
+ if (creds->passwordid) {
+ password = qcrypto_secret_lookup_as_utf8(creds->passwordid,
+ errp);
+ if (!password) {
+ goto cleanup;
+ }
+ }
+ ret = gnutls_certificate_set_x509_key_file2(creds->data,
+ cert, key,
+ GNUTLS_X509_FMT_PEM,
+ password,
+ 0);
+#else /* GNUTLS_VERSION_NUMBER < 0x030111 */
+ if (creds->passwordid) {
+ error_setg(errp, "PKCS8 decryption requires GNUTLS >= 3.1.11");
+ goto cleanup;
+ }
ret = gnutls_certificate_set_x509_key_file(creds->data,
cert, key,
GNUTLS_X509_FMT_PEM);
+#endif /* GNUTLS_VERSION_NUMBER < 0x030111 */
if (ret < 0) {
error_setg(errp, "Cannot load certificate '%s' & key '%s': %s",
cert, key, gnutls_strerror(ret));
@@ -736,6 +757,27 @@ qcrypto_tls_creds_x509_prop_set_sanity(Object *obj,
}
+static void
+qcrypto_tls_creds_x509_prop_set_passwordid(Object *obj,
+ const char *value,
+ Error **errp G_GNUC_UNUSED)
+{
+ QCryptoTLSCredsX509 *creds = QCRYPTO_TLS_CREDS_X509(obj);
+
+ creds->passwordid = g_strdup(value);
+}
+
+
+static char *
+qcrypto_tls_creds_x509_prop_get_passwordid(Object *obj,
+ Error **errp G_GNUC_UNUSED)
+{
+ QCryptoTLSCredsX509 *creds = QCRYPTO_TLS_CREDS_X509(obj);
+
+ return g_strdup(creds->passwordid);
+}
+
+
static bool
qcrypto_tls_creds_x509_prop_get_sanity(Object *obj,
Error **errp G_GNUC_UNUSED)
@@ -768,6 +810,10 @@ qcrypto_tls_creds_x509_init(Object *obj)
qcrypto_tls_creds_x509_prop_get_sanity,
qcrypto_tls_creds_x509_prop_set_sanity,
NULL);
+ object_property_add_str(obj, "passwordid",
+ qcrypto_tls_creds_x509_prop_get_passwordid,
+ qcrypto_tls_creds_x509_prop_set_passwordid,
+ NULL);
}
@@ -776,6 +822,7 @@ qcrypto_tls_creds_x509_finalize(Object *obj)
{
QCryptoTLSCredsX509 *creds = QCRYPTO_TLS_CREDS_X509(obj);
+ g_free(creds->passwordid);
qcrypto_tls_creds_x509_unload(creds);
}
diff --git a/include/crypto/tlscredsx509.h b/include/crypto/tlscredsx509.h
index b9785fd..25796d7 100644
--- a/include/crypto/tlscredsx509.h
+++ b/include/crypto/tlscredsx509.h
@@ -101,6 +101,7 @@ struct QCryptoTLSCredsX509 {
gnutls_certificate_credentials_t data;
#endif
bool sanityCheck;
+ char *passwordid;
};
diff --git a/qemu-options.hx b/qemu-options.hx
index 41c0e59..38fd8a8 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3626,7 +3626,7 @@ expensive operation that consumes random pool entropy, so it is
recommended that a persistent set of parameters be generated
upfront and saved.
-@item -object tls-creds-x509,id=@var{id},endpoint=@var{endpoint},dir=@var{/path/to/cred/dir},verify-peer=@var{on|off}
+@item -object tls-creds-x509,id=@var{id},endpoint=@var{endpoint},dir=@var{/path/to/cred/dir},verify-peer=@var{on|off},passwordid=@var{id}
Creates a TLS anonymous credentials object, which can be used to provide
TLS support on network backends. The @option{id} parameter is a unique
@@ -3653,6 +3653,12 @@ in PEM format, in filenames @var{ca-cert.pem}, @var{ca-crl.pem} (optional),
@var{server-cert.pem} (only servers), @var{server-key.pem} (only servers),
@var{client-cert.pem} (only clients), and @var{client-key.pem} (only clients).
+For the @var{server-key.pem} and @var{client-key.pem} files which
+contain sensitive private keys, it is possible to use an encrypted
+version by providing the @var{passwordid} parameter. This provides
+the ID of a previously created @code{secret} object containing the
+password for decryption.
+
@item -object filter-buffer,id=@var{id},netdev=@var{netdevid},interval=@var{t}[,queue=@var{all|rx|tx}]
Interval @var{t} can't be 0, this filter batches the packet delivery: all
--
2.5.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/5] util: add base64 decoding function
2015-11-24 15:02 ` [Qemu-devel] [PATCH v2 1/5] util: add base64 decoding function Daniel P. Berrange
@ 2015-11-24 15:54 ` Eric Blake
2015-11-24 16:02 ` Daniel P. Berrange
0 siblings, 1 reply; 13+ messages in thread
From: Eric Blake @ 2015-11-24 15:54 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel; +Cc: Markus Armbruster
[-- Attachment #1: Type: text/plain, Size: 3310 bytes --]
On 11/24/2015 08:02 AM, Daniel P. Berrange wrote:
> The standard glib provided g_base64_decode doesn't provide any
> kind of sensible error checking on its input. Add a QEMU custom
> wrapper qbase64_decode which can be used with untrustworthy
> input that can contain invalid base64 characters, embedded
> NUL characters, or not be NUL terminated at all.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> +/**
> + * qbase64_decode:
> + * @input: the (possibly) base64 encoded text
> + * @in_len: length of @input or -1 if NUL terminated
> + * @out_len: filled with length of decoded data
> + * @errpr: pointer to uninitialized error object
s/errpr/errp/
> + * Returns: the decoded data or NULL
> + */
> +uint8_t *qbase64_decode(const char *input,
> + size_t in_len,
> + size_t *out_len,
> + Error **errp);
> +
Is char* any easier to work with than uint8_t* as the return type? I'm
fine with either, and actually like that uint8_t doesn't cause
unintentional sign-extension on 8-bit input, but just want to make sure
we aren't forcing the majority of our callers to cast back to a more
convenient type.
> +static void test_base64_good(void)
> +{
> + const char *input = "QmVjYXVzZSB3ZSBmb2N1c2VkIG9uIHRoZS"
> + "BzbmFrZSwgd2UgbWlzc2VkIHRoZSBzY29ycGlvbi4=";
> + const char *expect = "Because we focused on the snake, "
> + "we missed the scorpion.";
> +
> + size_t len;
> + uint8_t *actual = qbase64_decode(input,
> + -1,
> + &len,
> + &error_abort);
> +
> + g_assert(actual != NULL);
> + g_assert_cmpint(len, ==, strlen(expect));
> + g_assert_cmpstr((char *)actual, ==, expect);
> + g_free(actual);
For example, this demonstrates a caller having to cast. But it doesn't
show whether more callers want char* or uint8_t*.
> +
> +static void test_base64_embedded_nul(void)
> +{
> + const char input[] = "There's no such\0thing as a free lunch.";
> +
> + test_base64_bad(input, G_N_ELEMENTS(input) - 1);
> +}
> +
> +
> +static void test_base64_not_nul_terminated(void)
> +{
> + char input[] = "There's no such\0thing as a free lunch.";
> + input[G_N_ELEMENTS(input) - 1] = '!';
> +
> + test_base64_bad(input, G_N_ELEMENTS(input) - 1);
> +}
> +
Did you mean to have an embedded NUL in the second test, or should you
change that \0 to space?
> +++ b/util/base64.c
> +#include "qemu/base64.h"
> +
> +static const char *base64_valid_chars =
> + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=";
Do we want to allow newlines (perhaps by adding a bool parameter)?
After all, although newline is not valid in base64, it is the one
character that GNU coreutils special-cases (produce on output every
--wrap columns, ignore on input without needing --ignore-garbage) to
make base64 blocks easier to read by breaking into lines rather than one
long string - and which may be relevant if someone is pasting output
from base64(1) into QMP.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/5] qemu-char: convert to use error checked base64 decode
2015-11-24 15:02 ` [Qemu-devel] [PATCH v2 2/5] qemu-char: convert to use error checked base64 decode Daniel P. Berrange
@ 2015-11-24 15:56 ` Eric Blake
0 siblings, 0 replies; 13+ messages in thread
From: Eric Blake @ 2015-11-24 15:56 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel; +Cc: Markus Armbruster
[-- Attachment #1: Type: text/plain, Size: 546 bytes --]
On 11/24/2015 08:02 AM, Daniel P. Berrange wrote:
> Switch from using g_base64_decode over to qbase64_decode
> in order to get error checking of the base64 input data.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> qapi-schema.json | 2 --
> qemu-char.c | 8 +++++++-
> qmp-commands.hx | 2 --
> 3 files changed, 7 insertions(+), 5 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/5] util: add base64 decoding function
2015-11-24 15:54 ` Eric Blake
@ 2015-11-24 16:02 ` Daniel P. Berrange
0 siblings, 0 replies; 13+ messages in thread
From: Daniel P. Berrange @ 2015-11-24 16:02 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Markus Armbruster
On Tue, Nov 24, 2015 at 08:54:24AM -0700, Eric Blake wrote:
> On 11/24/2015 08:02 AM, Daniel P. Berrange wrote:
> > The standard glib provided g_base64_decode doesn't provide any
> > kind of sensible error checking on its input. Add a QEMU custom
> > wrapper qbase64_decode which can be used with untrustworthy
> > input that can contain invalid base64 characters, embedded
> > NUL characters, or not be NUL terminated at all.
> >
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
>
> > +/**
> > + * qbase64_decode:
> > + * @input: the (possibly) base64 encoded text
> > + * @in_len: length of @input or -1 if NUL terminated
> > + * @out_len: filled with length of decoded data
> > + * @errpr: pointer to uninitialized error object
>
> s/errpr/errp/
>
> > + * Returns: the decoded data or NULL
> > + */
> > +uint8_t *qbase64_decode(const char *input,
> > + size_t in_len,
> > + size_t *out_len,
> > + Error **errp);
> > +
>
> Is char* any easier to work with than uint8_t* as the return type? I'm
> fine with either, and actually like that uint8_t doesn't cause
> unintentional sign-extension on 8-bit input, but just want to make sure
> we aren't forcing the majority of our callers to cast back to a more
> convenient type.
I think using 'char *' encourages dangerous behaviour as you should
not assume that the decoded data is a NUL terminated string without
embedded NULs. It is arbirary binary data, which should be sanitized
before using as a regular C 'char *' string. So uint8 forces the
callers to thing about this. Fortunately all callers do currently
handle this correctly, since g_base64_decode also returns a uint8_t
equivalent type.
> > +
> > +static void test_base64_embedded_nul(void)
> > +{
> > + const char input[] = "There's no such\0thing as a free lunch.";
> > +
> > + test_base64_bad(input, G_N_ELEMENTS(input) - 1);
> > +}
> > +
> > +
> > +static void test_base64_not_nul_terminated(void)
> > +{
> > + char input[] = "There's no such\0thing as a free lunch.";
> > + input[G_N_ELEMENTS(input) - 1] = '!';
> > +
> > + test_base64_bad(input, G_N_ELEMENTS(input) - 1);
> > +}
> > +
>
> Did you mean to have an embedded NUL in the second test, or should you
> change that \0 to space?
Sigh yes, there shouldn't be the embedded NUL in the second test.
> > +#include "qemu/base64.h"
> > +
> > +static const char *base64_valid_chars =
> > + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=";
>
> Do we want to allow newlines (perhaps by adding a bool parameter)?
> After all, although newline is not valid in base64, it is the one
> character that GNU coreutils special-cases (produce on output every
> --wrap columns, ignore on input without needing --ignore-garbage) to
> make base64 blocks easier to read by breaking into lines rather than one
> long string - and which may be relevant if someone is pasting output
> from base64(1) into QMP.
I'll test that glib correctly decodes base64 with newlines. Assuming
it does then we should definitely allow it and extend the unit test
to cover it too.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/5] qga: convert to use error checked base64 decode
2015-11-24 15:02 ` [Qemu-devel] [PATCH v2 3/5] qga: " Daniel P. Berrange
@ 2015-11-24 16:39 ` Eric Blake
0 siblings, 0 replies; 13+ messages in thread
From: Eric Blake @ 2015-11-24 16:39 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel; +Cc: Markus Armbruster
[-- Attachment #1: Type: text/plain, Size: 585 bytes --]
On 11/24/2015 08:02 AM, Daniel P. Berrange wrote:
> Switch from using g_base64_decode over to qbase64_decode
> in order to get error checking of the base64 input data.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> qga/commands-posix.c | 11 +++++++++--
> qga/commands-win32.c | 11 +++++++++--
> qga/commands.c | 13 ++++++++++++-
> 3 files changed, 30 insertions(+), 5 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/5] crypto: add QCryptoSecret object class for password/key handling
2015-11-24 15:02 ` [Qemu-devel] [PATCH v2 4/5] crypto: add QCryptoSecret object class for password/key handling Daniel P. Berrange
@ 2015-11-24 18:29 ` Eric Blake
2015-11-24 18:52 ` Daniel P. Berrange
0 siblings, 1 reply; 13+ messages in thread
From: Eric Blake @ 2015-11-24 18:29 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel; +Cc: Markus Armbruster
[-- Attachment #1: Type: text/plain, Size: 8484 bytes --]
On 11/24/2015 08:02 AM, Daniel P. Berrange wrote:
> Introduce a new QCryptoSecret object class which will be used
> for providing passwords and keys to other objects which need
> sensitive credentials.
>
> The new object can provide secret values directly as properties,
> or indirectly via a file. The latter includes support for file
> descriptor passing syntax on UNIX platforms. Ordinarily passing
> secret values directly as properties is insecure, since they
> are visible in process listings, or in log files showing the
> CLI args / QMP commands. It is possible to use AES-256-CBC to
> encrypt the secret values though, in which case all that is
> visible is the ciphertext. For adhoc developer testing though,
dictionary.com says it's always 'ad hoc' (since it is literally two
Latin words), even though the two are always spoken together in English.
And although 'ad-hoc' is starting to gain some traction in modern
usage, squashing it to 'adhoc' just feels like a typo.
> it is fine to provide the secrets directly without encryption
> so this is not explicitly forbidden.
>
> The anticipated scenario is that libvirtd will create a random
> master key per QEMU instance (eg /var/run/libvirt/qemu/$VMNAME.key)
> and will use that key to encrypt all passwords it provides to
> QEMU via '-object secret,....'. This avoids the need for libvirt
> (or other mgmt apps) to worry about file descriptor passing.
>
> It also makes life easier for people who are scripting the
> management of QEMU, for whom FD passing is significantly more
> complex.
>
> Providing data inline (insecure, only for adhoc dev testing)
Of course, if we touch it up in one place, you should use consistent
spelling throughout :)
>
> $QEMU -object secret,id=sec0,data=letmein
>
> Providing data indirectly in raw format
>
> printf "letmein" > mypasswd.txt
> $QEMU -object secret,id=sec0,file=mypasswd.txt
>
> Providing data indirectly in base64 format
>
> $QEMU -object secret,id=sec0,file=mykey.b64,format=base64
>
> Providing data with encryption
>
> $QEMU -object secret,id=master0,file=mykey.b64,format=base64 \
> -object secret,id=sec0,data=[base64 ciphertext],\
> keyid=master0,iv=[base64 IV],format=base64
>
> Note that 'format' here refers to the format of the ciphertext
> data. The decrypted data must always be in raw byte format.
>
> More examples are shown in the updated docs.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> +static void
> +qcrypto_secret_load_data(QCryptoSecret *secret,
> + uint8_t **output,
> + size_t *outputlen,
> + Error **errp)
> +{
> + int fd;
> + char *data = NULL;
> + size_t offset = 0;
> + size_t length = 0;
> +
> + *output = NULL;
> + *outputlen = 0;
> +
> + if (secret->file) {
> + if (secret->data) {
> + error_setg(errp,
> + "'file' and 'data' are mutually exclusive");
Is it worth trying to use a qapi flat union to make the mutual exclusion
inherent in the type definition, rather than something we have to
enforce manually? (I've got more experience with qapi than with Object,
so my question may be nonsensical)
> + return;
> + }
> + fd = qemu_open(secret->file, O_RDONLY);
> + if (fd < 0) {
> + error_setg_errno(errp, errno,
> + "Unable to open %s", secret->file);
Using error_setg_file_open() makes for a consistent message on open()
failure.
> + return;
> + }
> + while (length < (1024 * 1024)) { /* Limit secrets to 1 MB */
> + if ((length - offset) < 1024) {
> + length += 1024;
> + data = g_renew(char, data, length);
> + }
> + ssize_t ret = read(fd, data + offset, length - offset);
> + if (ret == 0) {
> + break;
> + }
> + if (ret < 0) {
> + error_setg_errno(errp, errno,
> + "Unable to read from %s", secret->file);
Does glib have a convenience function for reading contents of a file?
> + close(fd);
> + g_free(data);
> + return;
> + }
> + offset += ret;
> + }
> + close(fd);
> + if (offset) {
> + /* Even though data is raw 8-bit, so may contain
> + * arbitrary NULs, ensure it is explicitly NUL
> + * terminated */
> + *output = g_renew(uint8_t, data, offset + 1);
> + (*output)[offset] = '\0';
> + *outputlen = offset;
> + } else {
> + error_setg(errp, "Secret file %s is empty",
> + secret->file);
> + g_free(data);
> + }
> + } else if (secret->data) {
> + *outputlen = strlen(secret->data);
> + *output = g_new(uint8_t, *outputlen + 1);
> + memcpy(*output, secret->data, *outputlen + 1);
> + } else {
> + error_setg(errp, "Either 'file' or 'data' must be provided");
> + }
> +}
> +
> +
> +static void qcrypto_secret_decrypt(QCryptoSecret *secret,
> + const uint8_t *input,
> + size_t inputlen,
> + uint8_t **output,
> + size_t *outputlen,
> + Error **errp)
> +{
> + uint8_t *key = NULL, *ciphertext = NULL, *iv = NULL;
> + size_t keylen, ciphertextlen, ivlen;
> + QCryptoCipher *aes = NULL;
> + uint8_t *plaintext = NULL;
> +
> + *output = NULL;
> + *outputlen = 0;
> +
> + if (qcrypto_secret_lookup(secret->keyid,
> + &key, &keylen,
> + errp) < 0) {
> + goto cleanup;
> + }
> +
> + if (keylen != 32) {
> + error_setg(errp, "Key should be 32 bytes in length");
> + goto cleanup;
> + }
> +
> + if (!secret->iv) {
> + error_setg(errp, "IV is required to decrypt secret");
> + goto cleanup;
> + }
> +
> + iv = (uint8_t *)g_base64_decode(secret->iv, &ivlen);
Shouldn't this be using qbase64_decode()?
> +++ b/include/crypto/secret.h
> +/**
> + * QCryptoSecret:
> + *
> + * The QCryptoSecret object provides storage of secrets,
> + * which may be user passwords, encryption keys or any
> + * other kind of sensitive data that is represented as
> + * a sequence of bytes.
> + *
> + * Providing data directly, insecurely (suitable for
> + * adhoc developer testing only)
Another instance.
> +++ b/qapi/crypto.json
> @@ -19,3 +19,17 @@
> { 'enum': 'QCryptoTLSCredsEndpoint',
> 'prefix': 'QCRYPTO_TLS_CREDS_ENDPOINT',
> 'data': ['client', 'server']}
> +
> +
> +##
> +# QCryptoSecretFormat:
> +#
> +# The data format that the secret is provided in
> +#
> +# @raw: raw bytes. When encoded in JSON only valid UTF-8 sequences can be used
> +# @base64: arbitrary base64 encoded binary data
> +# Since: 2.6
> +##
> +{ 'enum': 'QCryptoSecretFormat',
> + 'prefix': 'QCRYPTO_SECRET_FORMAT',
> + 'data': ['raw', 'base64']}
This part is fine.
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 0eea4ee..41c0e59 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3677,6 +3677,83 @@ Dump the network traffic on netdev @var{dev} to the file specified by
> The file format is libpcap, so it can be analyzed with tools such as tcpdump
> or Wireshark.
>
> +@item -object secret,id=@var{id},data=@var{string},format=@var{raw|base64}[,keyid=@var{secretid},iv=@var{string}]
> +@item -object secret,id=@var{id},file=@var{filename},format=@var{raw|base64}[,keyid=@var{secretid},iv=@var{string}]
> +
> +Defines a secret to store a password, encryption key, or some other sensitive
> +data. The senstive data can either be passed directly via the @var{data}
s/senstive/sensitive/
> +parameter, or indirectly via the @var{file} parameter. Using the @var{data}
> +parameter is insecure unless the sensitive data is encrypted.
> +
Overall looks fairly reasonable.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 5/5] crypto: add support for loading encrypted x509 keys
2015-11-24 15:02 ` [Qemu-devel] [PATCH v2 5/5] crypto: add support for loading encrypted x509 keys Daniel P. Berrange
@ 2015-11-24 18:33 ` Eric Blake
0 siblings, 0 replies; 13+ messages in thread
From: Eric Blake @ 2015-11-24 18:33 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel; +Cc: Markus Armbruster
[-- Attachment #1: Type: text/plain, Size: 1194 bytes --]
On 11/24/2015 08:02 AM, Daniel P. Berrange wrote:
> Make use of the QCryptoSecret object to support loading of
> encrypted x509 keys. The optional 'passwordid' parameter
> to the tls-creds-x509 object type, provides the ID of a
> secret object instance that holds the decryption password
> for the PEM file.
>
> # printf "123456" > mypasswd.txt
> # $QEMU \
> -object secret,id=sec0,filename=mypasswd.txt \
> -object tls-creds-x509,passwordid=sec0,id=creds0,\
> dir=/home/berrange/.pki/qemu,endpoint=server \
> -vnc :1,tls-creds=creds0
>
> This requires QEMU to be linked to GNUTLS >= 3.1.11. If
> GNUTLS is too old an error will be reported if an attempt
> is made to pass a decryption password.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> crypto/tlscredsx509.c | 47 +++++++++++++++++++++++++++++++++++++++++++
> include/crypto/tlscredsx509.h | 1 +
> qemu-options.hx | 8 +++++++-
> 3 files changed, 55 insertions(+), 1 deletion(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/5] crypto: add QCryptoSecret object class for password/key handling
2015-11-24 18:29 ` Eric Blake
@ 2015-11-24 18:52 ` Daniel P. Berrange
0 siblings, 0 replies; 13+ messages in thread
From: Daniel P. Berrange @ 2015-11-24 18:52 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Markus Armbruster
On Tue, Nov 24, 2015 at 11:29:55AM -0700, Eric Blake wrote:
> > +static void
> > +qcrypto_secret_load_data(QCryptoSecret *secret,
> > + uint8_t **output,
> > + size_t *outputlen,
> > + Error **errp)
> > +{
> > + int fd;
> > + char *data = NULL;
> > + size_t offset = 0;
> > + size_t length = 0;
> > +
> > + *output = NULL;
> > + *outputlen = 0;
> > +
> > + if (secret->file) {
> > + if (secret->data) {
> > + error_setg(errp,
> > + "'file' and 'data' are mutually exclusive");
>
> Is it worth trying to use a qapi flat union to make the mutual exclusion
> inherent in the type definition, rather than something we have to
> enforce manually? (I've got more experience with qapi than with Object,
> so my question may be nonsensical)
Not ensure sure how you'd wire up a qapi type to a QOM property. It
is probably possible in some manner, but I not sure its particularly
compelling to do it for this.
>
> > + return;
> > + }
> > + fd = qemu_open(secret->file, O_RDONLY);
> > + if (fd < 0) {
> > + error_setg_errno(errp, errno,
> > + "Unable to open %s", secret->file);
>
> Using error_setg_file_open() makes for a consistent message on open()
> failure.
Yep
>
> > + return;
> > + }
> > + while (length < (1024 * 1024)) { /* Limit secrets to 1 MB */
> > + if ((length - offset) < 1024) {
> > + length += 1024;
> > + data = g_renew(char, data, length);
> > + }
> > + ssize_t ret = read(fd, data + offset, length - offset);
> > + if (ret == 0) {
> > + break;
> > + }
> > + if (ret < 0) {
> > + error_setg_errno(errp, errno,
> > + "Unable to read from %s", secret->file);
>
> Does glib have a convenience function for reading contents of a file?
Of course, I completely forgot about g_file_get_contents() which
works just fine.
> > +static void qcrypto_secret_decrypt(QCryptoSecret *secret,
> > + const uint8_t *input,
> > + size_t inputlen,
> > + uint8_t **output,
> > + size_t *outputlen,
> > + Error **errp)
> > +{
> > + uint8_t *key = NULL, *ciphertext = NULL, *iv = NULL;
> > + size_t keylen, ciphertextlen, ivlen;
> > + QCryptoCipher *aes = NULL;
> > + uint8_t *plaintext = NULL;
> > +
> > + *output = NULL;
> > + *outputlen = 0;
> > +
> > + if (qcrypto_secret_lookup(secret->keyid,
> > + &key, &keylen,
> > + errp) < 0) {
> > + goto cleanup;
> > + }
> > +
> > + if (keylen != 32) {
> > + error_setg(errp, "Key should be 32 bytes in length");
> > + goto cleanup;
> > + }
> > +
> > + if (!secret->iv) {
> > + error_setg(errp, "IV is required to decrypt secret");
> > + goto cleanup;
> > + }
> > +
> > + iv = (uint8_t *)g_base64_decode(secret->iv, &ivlen);
>
> Shouldn't this be using qbase64_decode()?
Yeah, it really should
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2015-11-24 18:52 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-24 15:02 [Qemu-devel] [PATCH v2 0/5] Add framework for passing secrets to QEMU Daniel P. Berrange
2015-11-24 15:02 ` [Qemu-devel] [PATCH v2 1/5] util: add base64 decoding function Daniel P. Berrange
2015-11-24 15:54 ` Eric Blake
2015-11-24 16:02 ` Daniel P. Berrange
2015-11-24 15:02 ` [Qemu-devel] [PATCH v2 2/5] qemu-char: convert to use error checked base64 decode Daniel P. Berrange
2015-11-24 15:56 ` Eric Blake
2015-11-24 15:02 ` [Qemu-devel] [PATCH v2 3/5] qga: " Daniel P. Berrange
2015-11-24 16:39 ` Eric Blake
2015-11-24 15:02 ` [Qemu-devel] [PATCH v2 4/5] crypto: add QCryptoSecret object class for password/key handling Daniel P. Berrange
2015-11-24 18:29 ` Eric Blake
2015-11-24 18:52 ` Daniel P. Berrange
2015-11-24 15:02 ` [Qemu-devel] [PATCH v2 5/5] crypto: add support for loading encrypted x509 keys Daniel P. Berrange
2015-11-24 18:33 ` Eric Blake
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.