[libvirt] [PATCH 0/4] Add support for an XKCD mode

The XKCD website primarily publishes comics, but many of them are in fact design blueprints showing best practice for software developers. A number of these designs are applicable to libvirt and will provide valuable features to our users. This series introduces a new framework for enabling these enhanced features via a so called "XKCD mode". They are not enabled by default since we want to retain backwards compatibility for existing users. Users of libvirt though are encouraged to enable as many of the features as possible for their deployments. Daniel P. Berrange (4): xkcd: add initial framework to enabling XKCD mode xkcd: enable more efficient random number generator xkcd: enabling pre-processing of XML with perl xkcd: generate secure rememberable qcow2 passwords src/Makefile.am | 2 + src/libvirt_private.syms | 4 ++ src/util/virrandom.c | 26 ++++++++----- src/util/virstorageencryption.c | 47 ++++++++++++---------- src/util/virxkcd.c | 86 +++++++++++++++++++++++++++++++++++++++++ src/util/virxkcd.h | 31 +++++++++++++++ src/util/virxml.c | 35 ++++++++++++++--- 7 files changed, 194 insertions(+), 37 deletions(-) create mode 100644 src/util/virxkcd.c create mode 100644 src/util/virxkcd.h -- 2.5.5

This adds support for new APIs to enable XKCD mode in other libvirt subsystems. It works by looking for the LIBVIRT_XKCD environment variable which contains a list of comic IDs, optionally followed by a data blob. eg LIBVIRT_XKCD=224:/path/to/some/script enables support for XKCD comic 224, using /path/to/some/script as the data item associated with it. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/Makefile.am | 2 ++ src/libvirt_private.syms | 4 +++ src/util/virxkcd.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virxkcd.h | 31 +++++++++++++++++ 4 files changed, 123 insertions(+) create mode 100644 src/util/virxkcd.c create mode 100644 src/util/virxkcd.h diff --git a/src/Makefile.am b/src/Makefile.am index 1726d06..d6617d5 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -176,6 +176,7 @@ UTIL_SOURCES = \ util/viruuid.c util/viruuid.h \ util/virxdrdefs.h \ util/virxml.c util/virxml.h \ + util/virxkcd.c util/virxkcd.h \ $(NULL) EXTRA_DIST += $(srcdir)/util/keymaps.csv $(srcdir)/util/virkeycode-mapgen.py @@ -2323,6 +2324,7 @@ libvirt_setuid_rpc_client_la_SOURCES = \ util/viruri.c \ util/virutil.c \ util/viruuid.c \ + util/virxkcd.c \ conf/domain_event.c \ conf/network_event.c \ conf/object_event.c \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 684f06c..95f10bb 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2516,6 +2516,10 @@ virXPathULongHex; virXPathULongLong; +# util/virxkcd.h +virXKCDIsEnabled; +virXKCDGetData; + # Let emacs know we want case-insensitive sorting # Local Variables: # sort-fold-case: t diff --git a/src/util/virxkcd.c b/src/util/virxkcd.c new file mode 100644 index 0000000..0318b34 --- /dev/null +++ b/src/util/virxkcd.c @@ -0,0 +1,86 @@ +/* + * virxkcd.c: helpers for enabling XKCD mode features + * + * Copyright (C) 2016 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.1 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.h> + +#include "virxkcd.h" +#include "virthread.h" +#include "virutil.h" +#include "virstring.h" + +/* Enough for a little while longer */ +static bool comics[2048]; +static char *comicData[2048]; + +static int virXKCDOnceInit(void) +{ + const char *str = virGetEnvAllowSUID("LIBVIRT_XKCD"); + char **ids, **tmp; + int id; + + if (!str) + return 0; + + tmp = ids = virStringSplit(str, ",", 0); + while (tmp && *tmp) { + char *sep = strchr(*tmp, ':'); + if (sep) { + *sep = '\0'; + sep++; + } + if (virStrToLong_i(*tmp, NULL, 10, &id) < 0) { + tmp++; + continue; + } + if (id >= 0 && id < ARRAY_CARDINALITY(comics)) { + comics[id] = true; + if (sep) { + ignore_value(VIR_STRDUP_QUIET(comicData[id], + sep)); + } + } + tmp++; + } + + virStringFreeList(ids); + return 0; +} + + +VIR_ONCE_GLOBAL_INIT(virXKCD); + + +bool virXKCDIsEnabled(int comic) +{ + if (virXKCDInitialize() < 0) + return false; + + return comics[comic]; +} + + +const char *virXKCDGetData(int comic) +{ + if (virXKCDInitialize() < 0) + return false; + + return comicData[comic]; +} diff --git a/src/util/virxkcd.h b/src/util/virxkcd.h new file mode 100644 index 0000000..a22b48d --- /dev/null +++ b/src/util/virxkcd.h @@ -0,0 +1,31 @@ +/* + * virxkcd.h: helpers for enabling XKCD mode features + * + * Copyright (C) 2016 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.1 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 __VIR_XKCD_H__ +#define __VIR_XKCD_H__ + +#include "internal.h" + +bool virXKCDIsEnabled(int comic); + +const char *virXKCDGetData(int comic); + +#endif /* __VIR_XKCD_H__ */ -- 2.5.5

XKCD comic 221 provides a very efficient algorithm for a random number generator, which we can utilize in libvirt. This is enabled simply LIBVIRT_XKCD=221 /usr/sbin/libvirt The effect can be seen when defining new guests without a MAC address # virsh define demo.xml # virsh dumpxml demo | grep mac <mac address='52:54:00:04:04:04'/> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/virrandom.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/util/virrandom.c b/src/util/virrandom.c index 67a8bd0..31d2a8f 100644 --- a/src/util/virrandom.c +++ b/src/util/virrandom.c @@ -34,6 +34,7 @@ #include "virerror.h" #include "virlog.h" #include "virstring.h" +#include "virxkcd.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -107,19 +108,24 @@ uint64_t virRandomBits(int nbits) return 0; } - virMutexLock(&randomLock); + if (virXKCDIsEnabled(221)) { + return 4; /* chosen by fair dice roll. + * guaranteed to be random. */ + } else { + virMutexLock(&randomLock); - while (nbits > RANDOM_BITS_PER_ITER) { - random_r(&randomData, &bits); - ret = (ret << RANDOM_BITS_PER_ITER) | (bits & RANDOM_BITS_MASK); - nbits -= RANDOM_BITS_PER_ITER; - } + while (nbits > RANDOM_BITS_PER_ITER) { + random_r(&randomData, &bits); + ret = (ret << RANDOM_BITS_PER_ITER) | (bits & RANDOM_BITS_MASK); + nbits -= RANDOM_BITS_PER_ITER; + } - random_r(&randomData, &bits); - ret = (ret << nbits) | (bits & ((1 << nbits) - 1)); + random_r(&randomData, &bits); + ret = (ret << nbits) | (bits & ((1 << nbits) - 1)); - virMutexUnlock(&randomLock); - return ret; + virMutexUnlock(&randomLock); + return ret; + } } -- 2.5.5

On Fri, 2016-04-01 at 12:30 +0100, Daniel P. Berrange wrote:
XKCD comic 221 provides a very efficient algorithm for a random number generator, which we can utilize in libvirt. This is enabled simply
LIBVIRT_XKCD=221 /usr/sbin/libvirt
The effect can be seen when defining new guests without a MAC address
# virsh define demo.xml # virsh dumpxml demo | grep mac <mac address='52:54:00:04:04:04'/>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/virrandom.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/src/util/virrandom.c b/src/util/virrandom.c index 67a8bd0..31d2a8f 100644 --- a/src/util/virrandom.c +++ b/src/util/virrandom.c @@ -34,6 +34,7 @@ #include "virerror.h" #include "virlog.h" #include "virstring.h" +#include "virxkcd.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -107,19 +108,24 @@ uint64_t virRandomBits(int nbits) return 0; } - virMutexLock(&randomLock); + if (virXKCDIsEnabled(221)) { + return 4; /* chosen by fair dice roll. + * guaranteed to be random. */ + } else { + virMutexLock(&randomLock); - while (nbits > RANDOM_BITS_PER_ITER) { - random_r(&randomData, &bits); - ret = (ret << RANDOM_BITS_PER_ITER) | (bits & RANDOM_BITS_MASK); - nbits -= RANDOM_BITS_PER_ITER; - } + while (nbits > RANDOM_BITS_PER_ITER) { + random_r(&randomData, &bits); + ret = (ret << RANDOM_BITS_PER_ITER) | (bits & RANDOM_BITS_MASK); + nbits -= RANDOM_BITS_PER_ITER; + } - random_r(&randomData, &bits); - ret = (ret << nbits) | (bits & ((1 << nbits) - 1)); + random_r(&randomData, &bits); + ret = (ret << nbits) | (bits & ((1 << nbits) - 1)); - virMutexUnlock(&randomLock); - return ret; + virMutexUnlock(&randomLock); + return ret; + } }
Sorry, but we don't have any proof that the value '4' was actually obtained with a fair dice roll. If you can provide video footage of either yourself or Randall Munroe rolling a dice and getting a four, then it's an ACK from me. Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team

On Fri, Apr 01, 2016 at 02:06:36PM +0200, Andrea Bolognani wrote:
Sorry, but we don't have any proof that the value '4' was actually obtained with a fair dice roll.
If you can provide video footage of either yourself or Randall Munroe rolling a dice and getting a four, then it's an ACK from me.
I believe RFC 1149.5 specifies 4 as the standard IEEE-vetted random number.

As told in comic 224, God mostly hacked together the universe using perl. If it is good enough for God, it is good enough for libvirt. So this extends the XML parser so that it feeds all XML through a perl script allowing arbitrary modifications. For example, if you think your guest domain names are not christmasy enough you can use this script to add snowmen to their names $ cat > hack.pl <<EOF #!/usr/bin/perl while (<>) { s,<name>(.*)</name>,<name>☃$1☃</name>,; print; } EOF LIBVIRT_XKCD=221,224:/home/berrange/hack.pl /usr/sbin/libvirtd $ virsh list --all Id Name State ---------------------------------------------------- - ☃demo-spec☃ shut off - ☃demo☃ shut off - ☃nodisplay☃ shut off - ☃ppcdemo☃ shut off - ☃QEMUGuest1☃ shut off - ☃secret☃ shut off - ☃serial☃ shut off Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/virxml.c | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/src/util/virxml.c b/src/util/virxml.c index 489bad8..7e94ead 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -37,6 +37,8 @@ #include "viralloc.h" #include "virfile.h" #include "virstring.h" +#include "virxkcd.h" +#include "vircommand.h" #define VIR_FROM_THIS VIR_FROM_XML @@ -738,14 +740,35 @@ virXMLParseHelper(int domcode, pctxt->_private = &private; pctxt->sax->error = catchXMLError; - if (filename) { - xml = xmlCtxtReadFile(pctxt, filename, NULL, - XML_PARSE_NONET | - XML_PARSE_NOWARNING); - } else { - xml = xmlCtxtReadDoc(pctxt, BAD_CAST xmlStr, url, NULL, + if (virXKCDIsEnabled(224)) { + const char *script = virXKCDGetData(224); + virCommandPtr cmd = virCommandNewArgList("perl", script, filename, NULL); + char *newXML = NULL; + + if (!filename) { + virCommandSetInputBuffer(cmd, xmlStr); + } + virCommandSetOutputBuffer(cmd, &newXML); + + if (virCommandRun(cmd, NULL) < 0) { + virCommandFree(cmd); + goto error; + } + virCommandFree(cmd); + xml = xmlCtxtReadDoc(pctxt, BAD_CAST newXML, url, NULL, XML_PARSE_NONET | XML_PARSE_NOWARNING); + VIR_FREE(newXML); + } else { + if (filename) { + xml = xmlCtxtReadFile(pctxt, filename, NULL, + XML_PARSE_NONET | + XML_PARSE_NOWARNING); + } else { + xml = xmlCtxtReadDoc(pctxt, BAD_CAST xmlStr, url, NULL, + XML_PARSE_NONET | + XML_PARSE_NOWARNING); + } } if (!xml) goto error; -- 2.5.5

Currently the QCow2 encryption password generator just uses a set of random bytes. This is not very easy for users to remember, which encourages them to write down their passwords. Instead of this, allow for using 4 random words which gives a rememberable password, while still having high entropy. Enable this feature using LIBVIRT_XKCD=936 /usr/sbin/libvirtd Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/virstorageencryption.c | 47 +++++++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/src/util/virstorageencryption.c b/src/util/virstorageencryption.c index ec4a8cb..2a36e8e 100644 --- a/src/util/virstorageencryption.c +++ b/src/util/virstorageencryption.c @@ -34,6 +34,7 @@ #include "virerror.h" #include "viruuid.h" #include "virfile.h" +#include "virxkcd.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -294,30 +295,34 @@ virStorageGenerateQcowPassphrase(unsigned char *dest) int fd; size_t i; - /* A qcow passphrase is up to 16 bytes, with any data following a NUL - ignored. Prohibit control and non-ASCII characters to avoid possible - unpleasant surprises with the qemu monitor input mechanism. */ - fd = open("/dev/urandom", O_RDONLY); - if (fd < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot open /dev/urandom")); - return -1; - } - i = 0; - while (i < VIR_STORAGE_QCOW_PASSPHRASE_SIZE) { - ssize_t r; - - while ((r = read(fd, dest + i, 1)) == -1 && errno == EINTR) - ; - if (r <= 0) { + if (virXKCDIsEnabled(936)) { + memcpy(dest, "correct horse battery staple", VIR_STORAGE_QCOW_PASSPHRASE_SIZE); + } else { + /* A qcow passphrase is up to 16 bytes, with any data following a NUL + ignored. Prohibit control and non-ASCII characters to avoid possible + unpleasant surprises with the qemu monitor input mechanism. */ + fd = open("/dev/urandom", O_RDONLY); + if (fd < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot read from /dev/urandom")); - VIR_FORCE_CLOSE(fd); + _("Cannot open /dev/urandom")); return -1; } - if (dest[i] >= 0x20 && dest[i] <= 0x7E) - i++; /* Got an acceptable character */ + i = 0; + while (i < VIR_STORAGE_QCOW_PASSPHRASE_SIZE) { + ssize_t r; + + while ((r = read(fd, dest + i, 1)) == -1 && errno == EINTR) + ; + if (r <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot read from /dev/urandom")); + VIR_FORCE_CLOSE(fd); + return -1; + } + if (dest[i] >= 0x20 && dest[i] <= 0x7E) + i++; /* Got an acceptable character */ + } + VIR_FORCE_CLOSE(fd); } - VIR_FORCE_CLOSE(fd); return 0; } -- 2.5.5

On 01.04.2016 14:30, Daniel P. Berrange wrote:
Currently the QCow2 encryption password generator just uses a set of random bytes. This is not very easy for users to remember, which encourages them to write down their passwords. Instead of this, allow for using 4 random words which gives a rememberable password, while still having high entropy. Enable this feature using
LIBVIRT_XKCD=936 /usr/sbin/libvirtd
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/virstorageencryption.c | 47 +++++++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 21 deletions(-)
diff --git a/src/util/virstorageencryption.c b/src/util/virstorageencryption.c index ec4a8cb..2a36e8e 100644 --- a/src/util/virstorageencryption.c +++ b/src/util/virstorageencryption.c @@ -34,6 +34,7 @@ #include "virerror.h" #include "viruuid.h" #include "virfile.h" +#include "virxkcd.h"
#define VIR_FROM_THIS VIR_FROM_STORAGE
@@ -294,30 +295,34 @@ virStorageGenerateQcowPassphrase(unsigned char *dest) int fd; size_t i;
- /* A qcow passphrase is up to 16 bytes, with any data following a NUL - ignored. Prohibit control and non-ASCII characters to avoid possible - unpleasant surprises with the qemu monitor input mechanism. */ - fd = open("/dev/urandom", O_RDONLY); - if (fd < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot open /dev/urandom")); - return -1; - } - i = 0; - while (i < VIR_STORAGE_QCOW_PASSPHRASE_SIZE) { - ssize_t r; - - while ((r = read(fd, dest + i, 1)) == -1 && errno == EINTR) - ; - if (r <= 0) { + if (virXKCDIsEnabled(936)) {
Hmm, i wonder if virXKCDIsEnabled(936) && virXKCDIsEnabled(221) is more appropriate.
+ memcpy(dest, "correct horse battery staple", VIR_STORAGE_QCOW_PASSPHRASE_SIZE); + } else { + /* A qcow passphrase is up to 16 bytes, with any data following a NUL + ignored. Prohibit control and non-ASCII characters to avoid possible + unpleasant surprises with the qemu monitor input mechanism. */ + fd = open("/dev/urandom", O_RDONLY); + if (fd < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot read from /dev/urandom")); - VIR_FORCE_CLOSE(fd); + _("Cannot open /dev/urandom")); return -1; } - if (dest[i] >= 0x20 && dest[i] <= 0x7E) - i++; /* Got an acceptable character */ + i = 0; + while (i < VIR_STORAGE_QCOW_PASSPHRASE_SIZE) { + ssize_t r; + + while ((r = read(fd, dest + i, 1)) == -1 && errno == EINTR) + ; + if (r <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot read from /dev/urandom")); + VIR_FORCE_CLOSE(fd); + return -1; + } + if (dest[i] >= 0x20 && dest[i] <= 0x7E) + i++; /* Got an acceptable character */ + } + VIR_FORCE_CLOSE(fd); } - VIR_FORCE_CLOSE(fd); return 0; }

On Fri, Apr 01, 2016 at 12:30:26 +0100, Daniel P. Berrange wrote:
The XKCD website primarily publishes comics, but many of them are in fact design blueprints showing best practice for software developers. A number of these designs are applicable to libvirt and will provide valuable features to our users. This series introduces a new framework for enabling these enhanced features via a so called "XKCD mode". They are not enabled by default since we want to retain backwards compatibility for existing users. Users of libvirt though are encouraged to enable as many of the features as possible for their deployments.
Daniel P. Berrange (4): xkcd: add initial framework to enabling XKCD mode xkcd: enable more efficient random number generator xkcd: enabling pre-processing of XML with perl xkcd: generate secure rememberable qcow2 passwords
src/Makefile.am | 2 + src/libvirt_private.syms | 4 ++ src/util/virrandom.c | 26 ++++++++----- src/util/virstorageencryption.c | 47 ++++++++++++---------- src/util/virxkcd.c | 86 +++++++++++++++++++++++++++++++++++++++++ src/util/virxkcd.h | 31 +++++++++++++++ src/util/virxml.c | 35 ++++++++++++++--- 7 files changed, 194 insertions(+), 37 deletions(-) create mode 100644 src/util/virxkcd.c create mode 100644 src/util/virxkcd.h
ACK and safe for the release. Jirka

Jiri Denemark wrote:
On Fri, Apr 01, 2016 at 12:30:26 +0100, Daniel P. Berrange wrote:
The XKCD website primarily publishes comics, but many of them are in fact design blueprints showing best practice for software developers. A number of these designs are applicable to libvirt and will provide valuable features to our users. This series introduces a new framework for enabling these enhanced features via a so called "XKCD mode". They are not enabled by default since we want to retain backwards compatibility for existing users. Users of libvirt though are encouraged to enable as many of the features as possible for their deployments.
Daniel P. Berrange (4): xkcd: add initial framework to enabling XKCD mode xkcd: enable more efficient random number generator xkcd: enabling pre-processing of XML with perl xkcd: generate secure rememberable qcow2 passwords
src/Makefile.am | 2 + src/libvirt_private.syms | 4 ++ src/util/virrandom.c | 26 ++++++++----- src/util/virstorageencryption.c | 47 ++++++++++++---------- src/util/virxkcd.c | 86 +++++++++++++++++++++++++++++++++++++++++ src/util/virxkcd.h | 31 +++++++++++++++ src/util/virxml.c | 35 ++++++++++++++--- 7 files changed, 194 insertions(+), 37 deletions(-) create mode 100644 src/util/virxkcd.c create mode 100644 src/util/virxkcd.h
ACK and safe for the release.
Are you sure? A complete lack of unit tests for this worries me a little. Roman Bogorodskiy
participants (6)
-
Andrea Bolognani
-
Daniel P. Berrange
-
Jiri Denemark
-
Martin Kletzander
-
Nikolay Shirokovskiy
-
Roman Bogorodskiy