All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v4 0/5] Add framework for passing secrets to QEMU
@ 2015-12-09 14:08 Daniel P. Berrange
  2015-12-09 14:08 ` [Qemu-devel] [PATCH v4 1/5] util: add base64 decoding function Daniel P. Berrange
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Daniel P. Berrange @ 2015-12-09 14:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: Markus Armbruster

This small series contains the basic patches for adding a secrets
object previously posted:

 WIP: https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg04748.html
  v2: https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg05297.html
  v3: https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg06184.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 v4:

 - Changed base64 "bad data" tests to be clearer about what they
   are testing (Eric)
 - Change comment about @errp variable in qbase64_decode (Eric)
 - Document need to g_free result of qbase64_decode (Eric)
 - Remove dead code NULL terminating result of g_file_get_contents
   (Eric)
 - Replace g_new+memcpy with g_strdup (Eric)
 - Allow zero-length file for secrets (Eric)

Changed in v3:

 - s/adhoc/ad hoc/ (Eric)
 - Fix mistake in unit test sample data (Eric)
 - Permit inclusion of newline in base64 and test it (Eric)
 - Switch to g_file_get_contents() instead of hand written
   equivalent (Eric)
 - Add missing conversion to qbase64_decode (Eric)

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               | 513 ++++++++++++++++++++++++++++++++++++++++++
 crypto/tlscredsx509.c         |  48 ++++
 include/crypto/secret.h       | 148 ++++++++++++
 include/crypto/tlscredsx509.h |   1 +
 include/qemu/base64.h         |  58 +++++
 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           | 109 +++++++++
 tests/test-crypto-secret.c    | 452 +++++++++++++++++++++++++++++++++++++
 util/Makefile.objs            |   1 +
 util/base64.c                 |  60 +++++
 20 files changed, 1533 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] 9+ messages in thread

* [Qemu-devel] [PATCH v4 1/5] util: add base64 decoding function
  2015-12-09 14:08 [Qemu-devel] [PATCH v4 0/5] Add framework for passing secrets to QEMU Daniel P. Berrange
@ 2015-12-09 14:08 ` Daniel P. Berrange
  2015-12-09 15:26   ` Eric Blake
  2015-12-09 19:13   ` John Snow
  2015-12-09 14:08 ` [Qemu-devel] [PATCH v4 2/5] qemu-char: convert to use error checked base64 decode Daniel P. Berrange
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 9+ messages in thread
From: Daniel P. Berrange @ 2015-12-09 14:08 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 |  58 +++++++++++++++++++++++++++
 tests/.gitignore      |   1 +
 tests/Makefile        |   3 ++
 tests/test-base64.c   | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++
 util/Makefile.objs    |   1 +
 util/base64.c         |  60 +++++++++++++++++++++++++++
 6 files changed, 232 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..793708d
--- /dev/null
+++ b/include/qemu/base64.h
@@ -0,0 +1,58 @@
+/*
+ * 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
+ * @errp: pointer to a NULL-initialized 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. The caller
+ * must use g_free() to free the returned data when
+ * it is no longer required.
+ *
+ * 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 a1d03b4..7dc8437 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
 
@@ -416,6 +417,8 @@ tests/test-vmstate$(EXESUF): tests/test-vmstate.o \
 	$(test-qom-obj-y)
 tests/test-timed-average$(EXESUF): tests/test-timed-average.o qemu-timer.o \
 	$(test-util-obj-y)
+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..f40eb94
--- /dev/null
+++ b/tests/test-base64.c
@@ -0,0 +1,109 @@
+/*
+ * 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[] =
+        "QmVjYXVzZSB3ZSBmb2N1c2VkIG9uIHRoZSBzbmFrZSwgd2UgbW\n"
+        "lzc2VkIHRoZSBzY29ycGlvbi4=";
+    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)
+{
+    /* We put a NUL character in the middle of the base64
+     * text which is invalid data, given the expected length */
+    const char input[] =
+        "QmVjYXVzZSB3ZSBmb2N1c2VkIG9uIHRoZSBzbmFrZSwgd2UgbW\0"
+        "lzc2VkIHRoZSBzY29ycGlvbi4=";
+
+    test_base64_bad(input, G_N_ELEMENTS(input) - 1);
+}
+
+
+static void test_base64_not_nul_terminated(void)
+{
+    const char input[] =
+        "QmVjYXVzZSB3ZSBmb2N1c2VkIG9uIHRoZSBzbmFrZSwgd2UgbW\n"
+        "lzc2VkIHRoZSBzY29ycGlvbi4=";
+
+    /* Using '-2' to make us drop the trailing NUL, thus
+     * creating an invalid base64 sequence for decoding */
+    test_base64_bad(input, G_N_ELEMENTS(input) - 2);
+}
+
+
+static void test_base64_invalid_chars(void)
+{
+    /* We put a single quote character in the middle
+     * of the base64 text which is invalid data */
+    const char input[] =
+        "QmVjYXVzZSB3ZSBmb2N1c2VkIG9uIHRoZSBzbmFrZSwgd2UgbW'"
+        "lzc2VkIHRoZSBzY29ycGlvbi4=";
+
+    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..f82caa7
--- /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+/=\n";
+
+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] 9+ messages in thread

* [Qemu-devel] [PATCH v4 2/5] qemu-char: convert to use error checked base64 decode
  2015-12-09 14:08 [Qemu-devel] [PATCH v4 0/5] Add framework for passing secrets to QEMU Daniel P. Berrange
  2015-12-09 14:08 ` [Qemu-devel] [PATCH v4 1/5] util: add base64 decoding function Daniel P. Berrange
@ 2015-12-09 14:08 ` Daniel P. Berrange
  2015-12-09 14:08 ` [Qemu-devel] [PATCH v4 3/5] qga: " Daniel P. Berrange
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Daniel P. Berrange @ 2015-12-09 14:08 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.

Reviewed-by: Eric Blake <eblake@redhat.com>
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 2969c44..c7f5330 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>
@@ -3264,7 +3265,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] 9+ messages in thread

* [Qemu-devel] [PATCH v4 3/5] qga: convert to use error checked base64 decode
  2015-12-09 14:08 [Qemu-devel] [PATCH v4 0/5] Add framework for passing secrets to QEMU Daniel P. Berrange
  2015-12-09 14:08 ` [Qemu-devel] [PATCH v4 1/5] util: add base64 decoding function Daniel P. Berrange
  2015-12-09 14:08 ` [Qemu-devel] [PATCH v4 2/5] qemu-char: convert to use error checked base64 decode Daniel P. Berrange
@ 2015-12-09 14:08 ` Daniel P. Berrange
  2015-12-09 14:08 ` [Qemu-devel] [PATCH v4 4/5] crypto: add QCryptoSecret object class for password/key handling Daniel P. Berrange
  2015-12-09 14:08 ` [Qemu-devel] [PATCH v4 5/5] crypto: add support for loading encrypted x509 keys Daniel P. Berrange
  4 siblings, 0 replies; 9+ messages in thread
From: Daniel P. Berrange @ 2015-12-09 14:08 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.

Reviewed-by: Eric Blake <eblake@redhat.com>
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 c2ff970..8fe708f 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__
@@ -525,7 +526,10 @@ GuestFileWrite *qmp_guest_file_write(int64_t handle, const char *buf_b64,
         gfh->state = RW_STATE_NEW;
     }
 
-    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;
@@ -1963,7 +1967,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 0654fe4..61ffbdf 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;
@@ -1294,7 +1298,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] 9+ messages in thread

* [Qemu-devel] [PATCH v4 4/5] crypto: add QCryptoSecret object class for password/key handling
  2015-12-09 14:08 [Qemu-devel] [PATCH v4 0/5] Add framework for passing secrets to QEMU Daniel P. Berrange
                   ` (2 preceding siblings ...)
  2015-12-09 14:08 ` [Qemu-devel] [PATCH v4 3/5] qga: " Daniel P. Berrange
@ 2015-12-09 14:08 ` Daniel P. Berrange
  2015-12-09 15:30   ` Eric Blake
  2015-12-09 14:08 ` [Qemu-devel] [PATCH v4 5/5] crypto: add support for loading encrypted x509 keys Daniel P. Berrange
  4 siblings, 1 reply; 9+ messages in thread
From: Daniel P. Berrange @ 2015-12-09 14:08 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 ad hoc 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 ad hoc 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            | 513 +++++++++++++++++++++++++++++++++++++++++++++
 include/crypto/secret.h    | 148 +++++++++++++
 qapi/crypto.json           |  14 ++
 qemu-options.hx            |  77 +++++++
 tests/.gitignore           |   1 +
 tests/Makefile             |   2 +
 tests/test-crypto-secret.c | 452 +++++++++++++++++++++++++++++++++++++++
 8 files changed, 1208 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..9a9257a
--- /dev/null
+++ b/crypto/secret.c
@@ -0,0 +1,513 @@
+/*
+ * 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)
+{
+    char *data = NULL;
+    size_t length = 0;
+    GError *gerr = NULL;
+
+    *output = NULL;
+    *outputlen = 0;
+
+    if (secret->file) {
+        if (secret->data) {
+            error_setg(errp,
+                       "'file' and 'data' are mutually exclusive");
+            return;
+        }
+        if (!g_file_get_contents(secret->file, &data, &length, &gerr)) {
+            error_setg(errp,
+                       "Unable to read %s: %s",
+                       secret->file, gerr->message);
+            g_error_free(gerr);
+            return;
+        }
+        *output = (uint8_t *)data;
+        *outputlen = length;
+    } else if (secret->data) {
+        *outputlen = strlen(secret->data);
+        *output = (uint8_t *)g_strdup(secret->data);
+    } 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 = qbase64_decode(secret->iv, -1, &ivlen, errp);
+    if (!iv) {
+        goto cleanup;
+    }
+    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..913519a
--- /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
+ * ad hoc 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..5aa22fc 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 sensitive 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 7dc8437..34d3c52 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)
@@ -460,6 +461,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..a49c8e3
--- /dev/null
+++ b/tests/test-crypto-secret.c
@@ -0,0 +1,452 @@
+/*
+ * 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",
+        &error_abort,
+        "file", fname,
+        NULL);
+
+    char *pw = qcrypto_secret_lookup_as_utf8("sec0",
+                                             &error_abort);
+
+    g_assert_cmpstr(pw, ==, "");
+
+    object_unparent(sec);
+    g_free(pw);
+    close(fd);
+    g_free(fname);
+}
+
+
+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] 9+ messages in thread

* [Qemu-devel] [PATCH v4 5/5] crypto: add support for loading encrypted x509 keys
  2015-12-09 14:08 [Qemu-devel] [PATCH v4 0/5] Add framework for passing secrets to QEMU Daniel P. Berrange
                   ` (3 preceding siblings ...)
  2015-12-09 14:08 ` [Qemu-devel] [PATCH v4 4/5] crypto: add QCryptoSecret object class for password/key handling Daniel P. Berrange
@ 2015-12-09 14:08 ` Daniel P. Berrange
  4 siblings, 0 replies; 9+ messages in thread
From: Daniel P. Berrange @ 2015-12-09 14:08 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.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 crypto/tlscredsx509.c         | 48 +++++++++++++++++++++++++++++++++++++++++++
 include/crypto/tlscredsx509.h |  1 +
 qemu-options.hx               |  8 +++++++-
 3 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index 26f18cb..d58fdea 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"
 
@@ -607,9 +608,30 @@ 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);
+        g_free(password);
+#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));
@@ -737,6 +759,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)
@@ -769,6 +812,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);
 }
 
 
@@ -777,6 +824,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 5aa22fc..0a8c640 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] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v4 1/5] util: add base64 decoding function
  2015-12-09 14:08 ` [Qemu-devel] [PATCH v4 1/5] util: add base64 decoding function Daniel P. Berrange
@ 2015-12-09 15:26   ` Eric Blake
  2015-12-09 19:13   ` John Snow
  1 sibling, 0 replies; 9+ messages in thread
From: Eric Blake @ 2015-12-09 15:26 UTC (permalink / raw)
  To: Daniel P. Berrange, qemu-devel; +Cc: Markus Armbruster

[-- Attachment #1: Type: text/plain, Size: 1059 bytes --]

On 12/09/2015 07:08 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>
> ---
>  include/qemu/base64.h |  58 +++++++++++++++++++++++++++
>  tests/.gitignore      |   1 +
>  tests/Makefile        |   3 ++
>  tests/test-base64.c   | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  util/Makefile.objs    |   1 +
>  util/base64.c         |  60 +++++++++++++++++++++++++++
>  6 files changed, 232 insertions(+)
>  create mode 100644 include/qemu/base64.h
>  create mode 100644 tests/test-base64.c
>  create mode 100644 util/base64.c

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] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v4 4/5] crypto: add QCryptoSecret object class for password/key handling
  2015-12-09 14:08 ` [Qemu-devel] [PATCH v4 4/5] crypto: add QCryptoSecret object class for password/key handling Daniel P. Berrange
@ 2015-12-09 15:30   ` Eric Blake
  0 siblings, 0 replies; 9+ messages in thread
From: Eric Blake @ 2015-12-09 15:30 UTC (permalink / raw)
  To: Daniel P. Berrange, qemu-devel; +Cc: Markus Armbruster

[-- Attachment #1: Type: text/plain, Size: 518 bytes --]

On 12/09/2015 07:08 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.
> 

> 
> More examples are shown in the updated docs.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---

After comparing to v3,

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] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v4 1/5] util: add base64 decoding function
  2015-12-09 14:08 ` [Qemu-devel] [PATCH v4 1/5] util: add base64 decoding function Daniel P. Berrange
  2015-12-09 15:26   ` Eric Blake
@ 2015-12-09 19:13   ` John Snow
  1 sibling, 0 replies; 9+ messages in thread
From: John Snow @ 2015-12-09 19:13 UTC (permalink / raw)
  To: Daniel P. Berrange, qemu-devel; +Cc: Markus Armbruster



On 12/09/2015 09:08 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>
> ---
>  include/qemu/base64.h |  58 +++++++++++++++++++++++++++
>  tests/.gitignore      |   1 +
>  tests/Makefile        |   3 ++
>  tests/test-base64.c   | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  util/Makefile.objs    |   1 +
>  util/base64.c         |  60 +++++++++++++++++++++++++++
>  6 files changed, 232 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..793708d
> --- /dev/null
> +++ b/include/qemu/base64.h
> @@ -0,0 +1,58 @@
> +/*
> + * 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
> + * @errp: pointer to a NULL-initialized 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. The caller
> + * must use g_free() to free the returned data when
> + * it is no longer required.
> + *
> + * 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 a1d03b4..7dc8437 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
>  
> @@ -416,6 +417,8 @@ tests/test-vmstate$(EXESUF): tests/test-vmstate.o \
>  	$(test-qom-obj-y)
>  tests/test-timed-average$(EXESUF): tests/test-timed-average.o qemu-timer.o \
>  	$(test-util-obj-y)
> +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..f40eb94
> --- /dev/null
> +++ b/tests/test-base64.c
> @@ -0,0 +1,109 @@
> +/*
> + * QEMU base64 heper test
> + *

helper?

> + * 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[] =
> +        "QmVjYXVzZSB3ZSBmb2N1c2VkIG9uIHRoZSBzbmFrZSwgd2UgbW\n"
> +        "lzc2VkIHRoZSBzY29ycGlvbi4=";
> +    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)
> +{
> +    /* We put a NUL character in the middle of the base64
> +     * text which is invalid data, given the expected length */
> +    const char input[] =
> +        "QmVjYXVzZSB3ZSBmb2N1c2VkIG9uIHRoZSBzbmFrZSwgd2UgbW\0"
> +        "lzc2VkIHRoZSBzY29ycGlvbi4=";
> +
> +    test_base64_bad(input, G_N_ELEMENTS(input) - 1);
> +}
> +
> +
> +static void test_base64_not_nul_terminated(void)
> +{
> +    const char input[] =
> +        "QmVjYXVzZSB3ZSBmb2N1c2VkIG9uIHRoZSBzbmFrZSwgd2UgbW\n"
> +        "lzc2VkIHRoZSBzY29ycGlvbi4=";
> +
> +    /* Using '-2' to make us drop the trailing NUL, thus
> +     * creating an invalid base64 sequence for decoding */
> +    test_base64_bad(input, G_N_ELEMENTS(input) - 2);
> +}
> +
> +
> +static void test_base64_invalid_chars(void)
> +{
> +    /* We put a single quote character in the middle
> +     * of the base64 text which is invalid data */
> +    const char input[] =
> +        "QmVjYXVzZSB3ZSBmb2N1c2VkIG9uIHRoZSBzbmFrZSwgd2UgbW'"
> +        "lzc2VkIHRoZSBzY29ycGlvbi4=";
> +
> +    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..f82caa7
> --- /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+/=\n";
> +
> +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);
> +}
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-12-09 19:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-09 14:08 [Qemu-devel] [PATCH v4 0/5] Add framework for passing secrets to QEMU Daniel P. Berrange
2015-12-09 14:08 ` [Qemu-devel] [PATCH v4 1/5] util: add base64 decoding function Daniel P. Berrange
2015-12-09 15:26   ` Eric Blake
2015-12-09 19:13   ` John Snow
2015-12-09 14:08 ` [Qemu-devel] [PATCH v4 2/5] qemu-char: convert to use error checked base64 decode Daniel P. Berrange
2015-12-09 14:08 ` [Qemu-devel] [PATCH v4 3/5] qga: " Daniel P. Berrange
2015-12-09 14:08 ` [Qemu-devel] [PATCH v4 4/5] crypto: add QCryptoSecret object class for password/key handling Daniel P. Berrange
2015-12-09 15:30   ` Eric Blake
2015-12-09 14:08 ` [Qemu-devel] [PATCH v4 5/5] crypto: add support for loading encrypted x509 keys Daniel P. Berrange

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.