* [Qemu-devel] [PATCH v3 00/10] Provide a QOM-based authorization API
@ 2016-03-10 18:51 Daniel P. Berrange
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 01/10] qdict: implement a qdict_crumple method for un-flattening a dict Daniel P. Berrange
0 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrange @ 2016-03-10 18:51 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Markus Armbruster, Max Reitz, Paolo Bonzini,
Andreas Färber
Many years ago I was responsible for adding the 'qemu_acl' type
and associated HMP commands. Looking back at it now, it is quite
a poor facility with a couple of bad limitations. First, the
responsibility for creating the ACLs was left with the QEMU network
service (VNC server was only thing ever doing it). This meant you
could not share ACLs across multiple services. Second, there was
no way to populate ACLs on the command line, you had no choice but
to use the HMP commands. Third, the API was hardcoded around the
idea of an in-QEMU implementation, leaving no scope for plugging
in alternative implementations backed by, for exmaple, LDAP or PAM.
This series introduces a much better authorization API design
to QEMU that addresses all these problems, and maintains back
compatibility. It of course is based on the QOM framework, so
that immediately gives us ability to create objects via the
CLI, HMP or QMP. There is an abstract base clss "QAuthZ" which
defines the basic API for QEMU network services to use, and a
specific implementation "QAuthZ" simple which replicates the
functionality of 'qemu_acl'. It is thus possible to add other
impls, without changing any other part of QEMU in the future.
Finally, the user is responsible for creating the ACL objects,
so they can have one ACL associated with all their TLS enabled
network services.
There was only one small problem with this, specifically the
-object CLI arg and HMP 'object_add' command had no way to let
the user specify non-scalar properties for objects. eg if an
object had a property which is a list of structs, you are out
of luck if you want to create it without using QMP.
Thus the first three patches do some work around QAPI / QOM
to make it possible to specify non-scalar properties with
the -object CLI arg and HMP 'object_add' command. See the
respective patches for illustration of the syntax used.
The patches 4 and 5 introduce the new base class and specific
implementation.
Patch 6 kills the old qemu_acl code, updating any existing
callers of it to use the QAuthZSimple QOM class instead.
Patches 7-10 add support for associating ACLs with the
network services supporting TLS encryption (NBD, chardev
and VNC).
Aside from the outstanding migration TLS patches, this series
wraps up the feature based work I have for TLS in this release
cycle.
Changed in v3:
- Created separate qdict_list_size method (Max)
- Added unit tests for case of empty dict (Max)
- Fix variable names to use underscore separator (Max)
- Fix potential free of uninitialized variables (Max)
- Use QObject APIs for casts, instead of C type casts (Max)
Changed in v2:
- Adapt to changes in qapi visitor APIs
- Add a 'bool recursive' flag to qdict_crumple (Max)
- Fix memory leaks in qdict_crumple (Max)
- Split out key splitting code from qdict_crumple (Max)
- Use saner variable names in qdict_crumple (Max)
- Added some tests for bad inputs to qdict_crumple
Daniel P. Berrange (10):
qdict: implement a qdict_crumple method for un-flattening a dict
qapi: allow QmpInputVisitor to auto-cast types
qom: support arbitrary non-scalar properties with -object
util: add QAuthZ object as an authorization base class
util: add QAuthZSimple object type for a simple access control list
acl: delete existing ACL implementation
qemu-nbd: add support for ACLs for TLS clients
nbd: allow an ACL to be set with nbd-server-start QMP command
chardev: add support for ACLs for TLS clients
vnc: allow specifying a custom ACL object name
MAINTAINERS | 7 +
Makefile | 9 +-
Makefile.objs | 2 +
Makefile.target | 2 +
blockdev-nbd.c | 10 +-
crypto/tlssession.c | 28 +++-
hmp.c | 20 +--
include/qapi/qmp-input-visitor.h | 3 +
include/qapi/qmp/qdict.h | 1 +
include/qemu/acl.h | 74 ----------
include/qemu/authz-simple.h | 107 ++++++++++++++
include/qemu/authz.h | 81 +++++++++++
monitor.c | 161 +++++++++++++--------
qapi-schema.json | 8 +-
qapi/block.json | 4 +-
qapi/qmp-input-visitor.c | 96 +++++++++++--
qapi/util.json | 31 ++++
qemu-char.c | 11 +-
qemu-nbd.c | 13 +-
qemu-nbd.texi | 4 +
qmp-commands.hx | 2 +-
qobject/qdict.c | 267 +++++++++++++++++++++++++++++++++++
qom/object_interfaces.c | 20 ++-
tests/.gitignore | 1 +
tests/Makefile | 5 +-
tests/check-qdict.c | 143 +++++++++++++++++++
tests/check-qom-proplist.c | 295 ++++++++++++++++++++++++++++++++++++++-
tests/test-authz-simple.c | 156 +++++++++++++++++++++
tests/test-crypto-tlssession.c | 13 +-
tests/test-io-channel-tls.c | 14 +-
tests/test-qmp-input-visitor.c | 115 ++++++++++++++-
ui/vnc-auth-sasl.c | 2 +-
ui/vnc-auth-sasl.h | 4 +-
ui/vnc.c | 76 ++++++++--
util/Makefile.objs | 4 +-
util/acl.c | 188 -------------------------
util/authz-simple.c | 256 +++++++++++++++++++++++++++++++++
util/authz.c | 46 ++++++
38 files changed, 1876 insertions(+), 403 deletions(-)
delete mode 100644 include/qemu/acl.h
create mode 100644 include/qemu/authz-simple.h
create mode 100644 include/qemu/authz.h
create mode 100644 qapi/util.json
create mode 100644 tests/test-authz-simple.c
delete mode 100644 util/acl.c
create mode 100644 util/authz-simple.c
create mode 100644 util/authz.c
--
2.5.0
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v3 01/10] qdict: implement a qdict_crumple method for un-flattening a dict
2016-03-10 18:51 [Qemu-devel] [PATCH v3 00/10] Provide a QOM-based authorization API Daniel P. Berrange
@ 2016-03-10 18:59 ` Daniel P. Berrange
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 02/10] qapi: allow QmpInputVisitor to auto-cast types Daniel P. Berrange
` (9 more replies)
0 siblings, 10 replies; 32+ messages in thread
From: Daniel P. Berrange @ 2016-03-10 18:59 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Markus Armbruster, Max Reitz, Paolo Bonzini,
Andreas Färber
The qdict_flatten() method will take a dict whose elements are
further nested dicts/lists and flatten them by concatenating
keys.
The qdict_crumple() method aims to do the reverse, taking a flat
qdict, and turning it into a set of nested dicts/lists. It will
apply nesting based on the key name, with a '.' indicating a
new level in the hierarchy. If the keys in the nested structure
are all numeric, it will create a list, otherwise it will create
a dict.
If the keys are a mixture of numeric and non-numeric, or the
numeric keys are not in strictly ascending order, an error will
be reported.
As an example, a flat dict containing
{
'foo.0.bar': 'one',
'foo.0.wizz': '1',
'foo.1.bar': 'two',
'foo.1.wizz': '2'
}
will get turned into a dict with one element 'foo' whose
value is a list. The list elements will each in turn be
dicts.
{
'foo' => [
{ 'bar': 'one', 'wizz': '1' }
{ 'bar': 'two', 'wizz': '2' }
],
}
If the key is intended to contain a literal '.', then it must
be escaped as '..'. ie a flat dict
{
'foo..bar': 'wizz',
'bar.foo..bar': 'eek',
'bar.hello': 'world'
}
Will end up as
{
'foo.bar': 'wizz',
'bar': {
'foo.bar': 'eek',
'hello': 'world'
}
}
The intent of this function is that it allows a set of QemuOpts
to be turned into a nested data structure that mirrors the nested
used when the same object is defined over QMP.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
include/qapi/qmp/qdict.h | 1 +
qobject/qdict.c | 267 +++++++++++++++++++++++++++++++++++++++++++++++
tests/check-qdict.c | 143 +++++++++++++++++++++++++
3 files changed, 411 insertions(+)
diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index 71b8eb0..8a3ac13 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -73,6 +73,7 @@ void qdict_flatten(QDict *qdict);
void qdict_extract_subqdict(QDict *src, QDict **dst, const char *start);
void qdict_array_split(QDict *src, QList **dst);
int qdict_array_entries(QDict *src, const char *subqdict);
+QObject *qdict_crumple(QDict *src, bool recursive, Error **errp);
void qdict_join(QDict *dest, QDict *src, bool overwrite);
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 9833bd0..3a01fcc 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -682,6 +682,273 @@ void qdict_array_split(QDict *src, QList **dst)
}
}
+
+/**
+ * qdict_split_flat_key:
+ *
+ * Given a flattened key such as 'foo.0.bar', split it
+ * into two parts at the first '.' separator. Allows
+ * double dot ('..') to escape the normal separator.
+ *
+ * eg
+ * 'foo.0.bar' -> prefix='foo' and suffix='0.bar'
+ * 'foo..0.bar' -> prefix='foo.0' and suffix='bar'
+ *
+ * The '..' sequence will be unescaped in the returned
+ * 'prefix' string. The 'suffix' string will be left
+ * in escaped format, so it can be fed back into the
+ * qdict_split_flat_key() key as the input later.
+ */
+static void qdict_split_flat_key(const char *key, char **prefix, char **suffix)
+{
+ const char *separator;
+ size_t i, j;
+
+ /* Find first '.' separator, but if there is a pair '..'
+ * that acts as an escape, so skip over '..' */
+ separator = NULL;
+ do {
+ if (separator) {
+ separator += 2;
+ } else {
+ separator = key;
+ }
+ separator = strchr(separator, '.');
+ } while (separator && *(separator + 1) == '.');
+
+ if (separator) {
+ *prefix = g_strndup(key,
+ separator - key);
+ *suffix = g_strdup(separator + 1);
+ } else {
+ *prefix = g_strdup(key);
+ *suffix = NULL;
+ }
+
+ /* Unescape the '..' sequence into '.' */
+ for (i = 0, j = 0; (*prefix)[i] != '\0'; i++, j++) {
+ if ((*prefix)[i] == '.' &&
+ (*prefix)[i + 1] == '.') {
+ i++;
+ }
+ (*prefix)[j] = (*prefix)[i];
+ }
+ (*prefix)[j] = '\0';
+}
+
+
+/**
+ * qdict_list_size:
+ * @maybe_List: dict that may be only list elements
+ *
+ * Determine whether all keys in @maybe_list are
+ * valid list elements. They they are all valid,
+ * then this returns the number of elements. If
+ * they all look like non-numeric keys, then returns
+ * zero. If there is a mix of numeric and non-numeric
+ * keys, then an error is set as it is both a list
+ * and a dict at once.
+ *
+ * Returns: number of list elemets, 0 if a dict, -1 on error
+ */
+static ssize_t qdict_list_size(QDict *maybe_list, Error **errp)
+{
+ const QDictEntry *entry, *next;
+ ssize_t len = 0;
+ ssize_t max = -1;
+ int is_list = -1;
+ int64_t val;
+
+ entry = qdict_first(maybe_list);
+ while (entry != NULL) {
+ next = qdict_next(maybe_list, entry);
+
+ if (qemu_strtoll(entry->key, NULL, 10, &val) == 0) {
+ if (is_list == -1) {
+ is_list = 1;
+ } else if (!is_list) {
+ error_setg(errp,
+ "Key '%s' is for a list, but previous key is "
+ "for a dict", entry->key);
+ return -1;
+ }
+ len++;
+ if (val > max) {
+ max = val;
+ }
+ } else {
+ if (is_list == -1) {
+ is_list = 0;
+ } else if (is_list) {
+ error_setg(errp,
+ "Key '%s' is for a dict, but previous key is "
+ "for a list", entry->key);
+ return -1;
+ }
+ }
+
+ entry = next;
+ }
+
+ if (is_list == -1) {
+ is_list = 0;
+ }
+
+ if (len != (max + 1)) {
+ error_setg(errp, "List indexes are not continuous, "
+ "saw %zd elements but %zd largest index",
+ len, max);
+ return -1;
+ }
+
+ return is_list ? len : 0;
+}
+
+/**
+ * qdict_crumple:
+ *
+ * Reverses the flattening done by qdict_flatten by
+ * crumpling the dicts into a nested structure. Similar
+ * qdict_array_split, but copes with arbitrary nesting
+ * of dicts & arrays, not merely one level of arrays
+ *
+ * { 'foo.0.bar': 'one', 'foo.0.wizz': '1',
+ * 'foo.1.bar': 'two', 'foo.1.wizz': '2' }
+ *
+ * =>
+ *
+ * {
+ * 'foo' => [
+ * { 'bar': 'one', 'wizz': '1' }
+ * { 'bar': 'two', 'wizz': '2' }
+ * ],
+ * }
+ *
+ */
+QObject *qdict_crumple(QDict *src, bool recursive, Error **errp)
+{
+ const QDictEntry *entry, *next;
+ QDict *two_level, *multi_level = NULL;
+ QObject *dst = NULL, *child;
+ ssize_t list_len;
+ size_t i;
+ char *prefix = NULL, *suffix = NULL;
+
+ two_level = qdict_new();
+ entry = qdict_first(src);
+
+ /* Step 1: split our totally flat dict into a two level dict */
+ while (entry != NULL) {
+ next = qdict_next(src, entry);
+
+ if (qobject_type(entry->value) == QTYPE_QDICT ||
+ qobject_type(entry->value) == QTYPE_QLIST) {
+ error_setg(errp, "Value %s is not a scalar",
+ entry->key);
+ goto error;
+ }
+
+ qdict_split_flat_key(entry->key, &prefix, &suffix);
+
+ child = qdict_get(two_level, prefix);
+ if (suffix) {
+ if (child) {
+ if (qobject_type(child) != QTYPE_QDICT) {
+ error_setg(errp, "Key %s prefix is already set as a scalar",
+ prefix);
+ goto error;
+ }
+ } else {
+ child = QOBJECT(qdict_new());
+ qdict_put_obj(two_level, prefix, child);
+ }
+ qobject_incref(entry->value);
+ qdict_put_obj(qobject_to_qdict(child), suffix, entry->value);
+ } else {
+ if (child) {
+ error_setg(errp, "Key %s prefix is already set as a dict",
+ prefix);
+ goto error;
+ }
+ qobject_incref(entry->value);
+ qdict_put_obj(two_level, prefix, entry->value);
+ }
+
+ g_free(suffix);
+ g_free(prefix);
+ suffix = prefix = NULL;
+
+ entry = next;
+ }
+
+ /* Step 2: optionally process the two level dict recursively
+ * into a multi-level dict */
+ if (recursive) {
+ multi_level = qdict_new();
+ entry = qdict_first(two_level);
+ while (entry != NULL) {
+ next = qdict_next(two_level, entry);
+
+ if (qobject_type(entry->value) == QTYPE_QDICT) {
+ child = qdict_crumple(qobject_to_qdict(entry->value),
+ recursive, errp);
+ if (!child) {
+ goto error;
+ }
+
+ qdict_put_obj(multi_level, entry->key, child);
+ } else {
+ qobject_incref(entry->value);
+ qdict_put_obj(multi_level, entry->key, entry->value);
+ }
+
+ entry = next;
+ }
+ QDECREF(two_level);
+ } else {
+ multi_level = two_level;
+ }
+ two_level = NULL;
+
+ /* Step 3: detect if we need to turn our dict into list */
+ list_len = qdict_list_size(multi_level, errp);
+ if (list_len < 0) {
+ goto error;
+ }
+
+ if (list_len) {
+ dst = QOBJECT(qlist_new());
+
+ for (i = 0; i < list_len; i++) {
+ char *key = g_strdup_printf("%zu", i);
+
+ child = qdict_get(multi_level, key);
+ g_free(key);
+ if (!child) {
+ error_setg(errp, "Unexpected missing list entry %zu", i);
+ goto error;
+ }
+
+ qobject_incref(child);
+ qlist_append_obj(qobject_to_qlist(dst), child);
+ }
+ QDECREF(multi_level);
+ } else {
+ dst = QOBJECT(multi_level);
+ }
+
+ return dst;
+
+ error:
+ g_free(suffix);
+ g_free(prefix);
+ QDECREF(multi_level);
+ QDECREF(two_level);
+ qobject_decref(dst);
+ return NULL;
+}
+
+
/**
* qdict_array_entries(): Returns the number of direct array entries if the
* sub-QDict of src specified by the prefix in subqdict (or src itself for
diff --git a/tests/check-qdict.c b/tests/check-qdict.c
index a43056c..ea67544 100644
--- a/tests/check-qdict.c
+++ b/tests/check-qdict.c
@@ -596,6 +596,140 @@ static void qdict_join_test(void)
QDECREF(dict2);
}
+
+static void qdict_crumple_test_nonrecursive(void)
+{
+ QDict *src, *dst, *rules;
+ QObject *child, *res;
+
+ src = qdict_new();
+ qdict_put(src, "rule.0.match", qstring_from_str("fred"));
+ qdict_put(src, "rule.0.policy", qstring_from_str("allow"));
+ qdict_put(src, "rule.1.match", qstring_from_str("bob"));
+ qdict_put(src, "rule.1.policy", qstring_from_str("deny"));
+
+ res = qdict_crumple(src, false, &error_abort);
+
+ g_assert_cmpint(qobject_type(res), ==, QTYPE_QDICT);
+
+ dst = qobject_to_qdict(res);
+
+ g_assert_cmpint(qdict_size(dst), ==, 1);
+
+ child = qdict_get(dst, "rule");
+ g_assert_cmpint(qobject_type(child), ==, QTYPE_QDICT);
+
+ rules = qdict_get_qdict(dst, "rule");
+
+ g_assert_cmpint(qdict_size(rules), ==, 4);
+
+ g_assert_cmpstr("fred", ==, qdict_get_str(rules, "0.match"));
+ g_assert_cmpstr("allow", ==, qdict_get_str(rules, "0.policy"));
+ g_assert_cmpstr("bob", ==, qdict_get_str(rules, "1.match"));
+ g_assert_cmpstr("deny", ==, qdict_get_str(rules, "1.policy"));
+
+ QDECREF(src);
+ QDECREF(dst);
+}
+
+
+static void qdict_crumple_test_recursive(void)
+{
+ QDict *src, *dst, *rule;
+ QObject *child, *res;
+ QList *rules;
+
+ src = qdict_new();
+ qdict_put(src, "rule.0.match", qstring_from_str("fred"));
+ qdict_put(src, "rule.0.policy", qstring_from_str("allow"));
+ qdict_put(src, "rule.1.match", qstring_from_str("bob"));
+ qdict_put(src, "rule.1.policy", qstring_from_str("deny"));
+
+ res = qdict_crumple(src, true, &error_abort);
+
+ g_assert_cmpint(qobject_type(res), ==, QTYPE_QDICT);
+
+ dst = qobject_to_qdict(res);
+
+ g_assert_cmpint(qdict_size(dst), ==, 1);
+
+ child = qdict_get(dst, "rule");
+ g_assert_cmpint(qobject_type(child), ==, QTYPE_QLIST);
+
+ rules = qdict_get_qlist(dst, "rule");
+ g_assert_cmpint(qlist_size(rules), ==, 2);
+
+ rule = qobject_to_qdict(qlist_pop(rules));
+ g_assert_cmpint(qdict_size(rule), ==, 2);
+ g_assert_cmpstr("fred", ==, qdict_get_str(rule, "match"));
+ g_assert_cmpstr("allow", ==, qdict_get_str(rule, "policy"));
+ QDECREF(rule);
+
+ rule = qobject_to_qdict(qlist_pop(rules));
+ g_assert_cmpint(qdict_size(rule), ==, 2);
+ g_assert_cmpstr("bob", ==, qdict_get_str(rule, "match"));
+ g_assert_cmpstr("deny", ==, qdict_get_str(rule, "policy"));
+ QDECREF(rule);
+
+ QDECREF(src);
+ QDECREF(dst);
+}
+
+
+static void qdict_crumple_test_empty(void)
+{
+ QDict *src, *dst;
+
+ src = qdict_new();
+
+ dst = (QDict *)qdict_crumple(src, true, &error_abort);
+
+ g_assert_cmpint(qdict_size(dst), ==, 0);
+
+ QDECREF(src);
+ QDECREF(dst);
+}
+
+
+static void qdict_crumple_test_bad_inputs(void)
+{
+ QDict *src;
+ Error *error = NULL;
+
+ src = qdict_new();
+ /* rule.0 can't be both a string and a dict */
+ qdict_put(src, "rule.0", qstring_from_str("fred"));
+ qdict_put(src, "rule.0.policy", qstring_from_str("allow"));
+
+ g_assert(qdict_crumple(src, true, &error) == NULL);
+ g_assert(error != NULL);
+ error_free(error);
+ error = NULL;
+ QDECREF(src);
+
+ src = qdict_new();
+ /* rule can't be both a list and a dict */
+ qdict_put(src, "rule.0", qstring_from_str("fred"));
+ qdict_put(src, "rule.a", qstring_from_str("allow"));
+
+ g_assert(qdict_crumple(src, true, &error) == NULL);
+ g_assert(error != NULL);
+ error_free(error);
+ error = NULL;
+ QDECREF(src);
+
+ src = qdict_new();
+ /* The input should be flat, ie no dicts or lists */
+ qdict_put(src, "rule.0", qdict_new());
+ qdict_put(src, "rule.a", qstring_from_str("allow"));
+
+ g_assert(qdict_crumple(src, true, &error) == NULL);
+ g_assert(error != NULL);
+ error_free(error);
+ error = NULL;
+ QDECREF(src);
+}
+
/*
* Errors test-cases
*/
@@ -743,6 +877,15 @@ int main(int argc, char **argv)
g_test_add_func("/errors/put_exists", qdict_put_exists_test);
g_test_add_func("/errors/get_not_exists", qdict_get_not_exists_test);
+ g_test_add_func("/public/crumple/nonrecursive",
+ qdict_crumple_test_nonrecursive);
+ g_test_add_func("/public/crumple/recursive",
+ qdict_crumple_test_recursive);
+ g_test_add_func("/public/crumple/empty",
+ qdict_crumple_test_empty);
+ g_test_add_func("/public/crumple/bad_inputs",
+ qdict_crumple_test_bad_inputs);
+
/* The Big one */
if (g_test_slow()) {
g_test_add_func("/stress/test", qdict_stress_test);
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v3 02/10] qapi: allow QmpInputVisitor to auto-cast types
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 01/10] qdict: implement a qdict_crumple method for un-flattening a dict Daniel P. Berrange
@ 2016-03-10 18:59 ` Daniel P. Berrange
2016-03-21 23:18 ` Eric Blake
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 03/10] qom: support arbitrary non-scalar properties with -object Daniel P. Berrange
` (8 subsequent siblings)
9 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrange @ 2016-03-10 18:59 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Markus Armbruster, Max Reitz, Paolo Bonzini,
Andreas Färber
Currently the QmpInputVisitor assumes that all scalar
values are directly represented as their final types.
ie it assumes an 'int' is using QInt, and a 'bool' is
using QBool.
This extends it so that QString is optionally permitted
for any of the non-string scalar types. This behaviour
is turned on by requesting the 'autocast' flag in the
constructor.
This makes it possible to use QmpInputVisitor with a
QDict produced from QemuOpts, where everything is in
string format.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
include/qapi/qmp-input-visitor.h | 3 +
qapi/qmp-input-visitor.c | 96 +++++++++++++++++++++++++++-----
tests/test-qmp-input-visitor.c | 115 ++++++++++++++++++++++++++++++++++++++-
3 files changed, 196 insertions(+), 18 deletions(-)
diff --git a/include/qapi/qmp-input-visitor.h b/include/qapi/qmp-input-visitor.h
index 3ed499c..c25cb7c 100644
--- a/include/qapi/qmp-input-visitor.h
+++ b/include/qapi/qmp-input-visitor.h
@@ -21,6 +21,9 @@ typedef struct QmpInputVisitor QmpInputVisitor;
QmpInputVisitor *qmp_input_visitor_new(QObject *obj);
QmpInputVisitor *qmp_input_visitor_new_strict(QObject *obj);
+QmpInputVisitor *qmp_input_visitor_new_full(QObject *obj,
+ bool strict,
+ bool autocast);
void qmp_input_visitor_cleanup(QmpInputVisitor *v);
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index e659832..59d2165 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -35,6 +35,7 @@ struct QmpInputVisitor
StackObject stack[QIV_STACK_SIZE];
int nb_stack;
bool strict;
+ bool autocast;
};
static QmpInputVisitor *to_qiv(Visitor *v)
@@ -217,15 +218,26 @@ static void qmp_input_type_int64(Visitor *v, const char *name, int64_t *obj,
Error **errp)
{
QmpInputVisitor *qiv = to_qiv(v);
- QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true));
+ QObject *qobj = qmp_input_get_object(qiv, name, true);
+ QInt *qint;
+ QString *qstr;
- if (!qint) {
- error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
- "integer");
+ qint = qobject_to_qint(qobj);
+ if (qint) {
+ *obj = qint_get_int(qint);
return;
}
- *obj = qint_get_int(qint);
+ qstr = qobject_to_qstring(qobj);
+ if (qstr && qstr->string && qiv->autocast) {
+ errno = 0;
+ if (qemu_strtoll(qstr->string, NULL, 10, obj) == 0) {
+ return;
+ }
+ }
+
+ error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+ "integer");
}
static void qmp_input_type_uint64(Visitor *v, const char *name, uint64_t *obj,
@@ -233,30 +245,61 @@ static void qmp_input_type_uint64(Visitor *v, const char *name, uint64_t *obj,
{
/* FIXME: qobject_to_qint mishandles values over INT64_MAX */
QmpInputVisitor *qiv = to_qiv(v);
- QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true));
+ QObject *qobj = qmp_input_get_object(qiv, name, true);
+ QInt *qint;
+ QString *qstr;
- if (!qint) {
- error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
- "integer");
+ qint = qobject_to_qint(qobj);
+ if (qint) {
+ *obj = qint_get_int(qint);
return;
}
- *obj = qint_get_int(qint);
+ qstr = qobject_to_qstring(qobj);
+ if (qstr && qstr->string && qiv->autocast) {
+ errno = 0;
+ if (qemu_strtoull(qstr->string, NULL, 10, obj) == 0) {
+ return;
+ }
+ }
+
+ error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+ "integer");
}
static void qmp_input_type_bool(Visitor *v, const char *name, bool *obj,
Error **errp)
{
QmpInputVisitor *qiv = to_qiv(v);
- QBool *qbool = qobject_to_qbool(qmp_input_get_object(qiv, name, true));
+ QObject *qobj = qmp_input_get_object(qiv, name, true);
+ QBool *qbool;
+ QString *qstr;
- if (!qbool) {
- error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
- "boolean");
+ qbool = qobject_to_qbool(qobj);
+ if (qbool) {
+ *obj = qbool_get_bool(qbool);
return;
}
- *obj = qbool_get_bool(qbool);
+
+ qstr = qobject_to_qstring(qobj);
+ if (qstr && qstr->string && qiv->autocast) {
+ if (!strcasecmp(qstr->string, "on") ||
+ !strcasecmp(qstr->string, "yes") ||
+ !strcasecmp(qstr->string, "true")) {
+ *obj = true;
+ return;
+ }
+ if (!strcasecmp(qstr->string, "off") ||
+ !strcasecmp(qstr->string, "no") ||
+ !strcasecmp(qstr->string, "false")) {
+ *obj = false;
+ return;
+ }
+ }
+
+ error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+ "boolean");
}
static void qmp_input_type_str(Visitor *v, const char *name, char **obj,
@@ -281,6 +324,8 @@ static void qmp_input_type_number(Visitor *v, const char *name, double *obj,
QObject *qobj = qmp_input_get_object(qiv, name, true);
QInt *qint;
QFloat *qfloat;
+ QString *qstr;
+ char *endp;
qint = qobject_to_qint(qobj);
if (qint) {
@@ -294,6 +339,15 @@ static void qmp_input_type_number(Visitor *v, const char *name, double *obj,
return;
}
+ qstr = qobject_to_qstring(qobj);
+ if (qstr && qstr->string && qiv->autocast) {
+ errno = 0;
+ *obj = strtod(qstr->string, &endp);
+ if (errno == 0 && endp != qstr->string && *endp == '\0') {
+ return;
+ }
+ }
+
error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
"number");
}
@@ -368,3 +422,15 @@ QmpInputVisitor *qmp_input_visitor_new_strict(QObject *obj)
return v;
}
+
+QmpInputVisitor *qmp_input_visitor_new_full(QObject *obj,
+ bool strict, bool autocast)
+{
+ QmpInputVisitor *v;
+
+ v = qmp_input_visitor_new(obj);
+ v->strict = strict;
+ v->autocast = autocast;
+
+ return v;
+}
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index b05da5b..fd7eb63 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -40,6 +40,7 @@ static void visitor_input_teardown(TestInputVisitorData *data,
function so that the JSON string used by the tests are kept in the test
functions (and not in main()). */
static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data,
+ bool strict, bool autocast,
const char *json_string,
va_list *ap)
{
@@ -50,7 +51,7 @@ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data,
data->obj = qobject_from_jsonv(json_string, ap);
g_assert(data->obj);
- data->qiv = qmp_input_visitor_new(data->obj);
+ data->qiv = qmp_input_visitor_new_full(data->obj, strict, autocast);
g_assert(data->qiv);
v = qmp_input_get_visitor(data->qiv);
@@ -59,6 +60,21 @@ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data,
return v;
}
+static GCC_FMT_ATTR(4, 5)
+Visitor *visitor_input_test_init_full(TestInputVisitorData *data,
+ bool strict, bool autocast,
+ const char *json_string, ...)
+{
+ Visitor *v;
+ va_list ap;
+
+ va_start(ap, json_string);
+ v = visitor_input_test_init_internal(data, strict, autocast,
+ json_string, &ap);
+ va_end(ap);
+ return v;
+}
+
static GCC_FMT_ATTR(2, 3)
Visitor *visitor_input_test_init(TestInputVisitorData *data,
const char *json_string, ...)
@@ -67,7 +83,8 @@ Visitor *visitor_input_test_init(TestInputVisitorData *data,
va_list ap;
va_start(ap, json_string);
- v = visitor_input_test_init_internal(data, json_string, &ap);
+ v = visitor_input_test_init_internal(data, NULL, NULL,
+ json_string, &ap);
va_end(ap);
return v;
}
@@ -82,7 +99,8 @@ Visitor *visitor_input_test_init(TestInputVisitorData *data,
static Visitor *visitor_input_test_init_raw(TestInputVisitorData *data,
const char *json_string)
{
- return visitor_input_test_init_internal(data, json_string, NULL);
+ return visitor_input_test_init_internal(data, NULL, NULL,
+ json_string, NULL);
}
static void test_visitor_in_int(TestInputVisitorData *data,
@@ -114,6 +132,33 @@ static void test_visitor_in_int_overflow(TestInputVisitorData *data,
error_free_or_abort(&err);
}
+static void test_visitor_in_int_autocast(TestInputVisitorData *data,
+ const void *unused)
+{
+ int64_t res = 0, value = -42;
+ Visitor *v;
+
+ v = visitor_input_test_init_full(data, false, true,
+ "\"-42\"");
+
+ visit_type_int(v, NULL, &res, &error_abort);
+ g_assert_cmpint(res, ==, value);
+}
+
+static void test_visitor_in_int_noautocast(TestInputVisitorData *data,
+ const void *unused)
+{
+ int64_t res = 0;
+ Visitor *v;
+ Error *err = NULL;
+
+ v = visitor_input_test_init(data, "\"-42\"");
+
+ visit_type_int(v, NULL, &res, &err);
+ g_assert(err != NULL);
+ error_free(err);
+}
+
static void test_visitor_in_bool(TestInputVisitorData *data,
const void *unused)
{
@@ -126,6 +171,32 @@ static void test_visitor_in_bool(TestInputVisitorData *data,
g_assert_cmpint(res, ==, true);
}
+static void test_visitor_in_bool_autocast(TestInputVisitorData *data,
+ const void *unused)
+{
+ bool res = false;
+ Visitor *v;
+
+ v = visitor_input_test_init_full(data, false, true, "\"true\"");
+
+ visit_type_bool(v, NULL, &res, &error_abort);
+ g_assert_cmpint(res, ==, true);
+}
+
+static void test_visitor_in_bool_noautocast(TestInputVisitorData *data,
+ const void *unused)
+{
+ bool res = false;
+ Visitor *v;
+ Error *err = NULL;
+
+ v = visitor_input_test_init(data, "\"true\"");
+
+ visit_type_bool(v, NULL, &res, &err);
+ g_assert(err != NULL);
+ error_free(err);
+}
+
static void test_visitor_in_number(TestInputVisitorData *data,
const void *unused)
{
@@ -138,6 +209,32 @@ static void test_visitor_in_number(TestInputVisitorData *data,
g_assert_cmpfloat(res, ==, value);
}
+static void test_visitor_in_number_autocast(TestInputVisitorData *data,
+ const void *unused)
+{
+ double res = 0, value = 3.14;
+ Visitor *v;
+
+ v = visitor_input_test_init_full(data, false, true, "\"3.14\"");
+
+ visit_type_number(v, NULL, &res, &error_abort);
+ g_assert_cmpfloat(res, ==, value);
+}
+
+static void test_visitor_in_number_noautocast(TestInputVisitorData *data,
+ const void *unused)
+{
+ double res = 0;
+ Visitor *v;
+ Error *err = NULL;
+
+ v = visitor_input_test_init(data, "\"3.14\"");
+
+ visit_type_number(v, NULL, &res, &err);
+ g_assert(err != NULL);
+ error_free(err);
+}
+
static void test_visitor_in_string(TestInputVisitorData *data,
const void *unused)
{
@@ -811,10 +908,22 @@ int main(int argc, char **argv)
&in_visitor_data, test_visitor_in_int);
input_visitor_test_add("/visitor/input/int_overflow",
&in_visitor_data, test_visitor_in_int_overflow);
+ input_visitor_test_add("/visitor/input/int_autocast",
+ &in_visitor_data, test_visitor_in_int_autocast);
+ input_visitor_test_add("/visitor/input/int_noautocast",
+ &in_visitor_data, test_visitor_in_int_noautocast);
input_visitor_test_add("/visitor/input/bool",
&in_visitor_data, test_visitor_in_bool);
+ input_visitor_test_add("/visitor/input/bool_autocast",
+ &in_visitor_data, test_visitor_in_bool_autocast);
+ input_visitor_test_add("/visitor/input/bool_noautocast",
+ &in_visitor_data, test_visitor_in_bool_noautocast);
input_visitor_test_add("/visitor/input/number",
&in_visitor_data, test_visitor_in_number);
+ input_visitor_test_add("/visitor/input/number_autocast",
+ &in_visitor_data, test_visitor_in_number_autocast);
+ input_visitor_test_add("/visitor/input/number_noautocast",
+ &in_visitor_data, test_visitor_in_number_noautocast);
input_visitor_test_add("/visitor/input/string",
&in_visitor_data, test_visitor_in_string);
input_visitor_test_add("/visitor/input/enum",
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v3 03/10] qom: support arbitrary non-scalar properties with -object
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 01/10] qdict: implement a qdict_crumple method for un-flattening a dict Daniel P. Berrange
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 02/10] qapi: allow QmpInputVisitor to auto-cast types Daniel P. Berrange
@ 2016-03-10 18:59 ` Daniel P. Berrange
2016-03-21 23:27 ` Eric Blake
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 04/10] util: add QAuthZ object as an authorization base class Daniel P. Berrange
` (7 subsequent siblings)
9 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrange @ 2016-03-10 18:59 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Markus Armbruster, Max Reitz, Paolo Bonzini,
Andreas Färber
The current -object command line syntax only allows for
creation of objects with scalar properties, or a list
with a fixed scalar element type. Objects which have
properties that are represented as structs in the QAPI
schema cannot be created using -object.
This is a design limitation of the way the OptsVisitor
is written. It simply iterates over the QemuOpts values
as a flat list. The support for lists is enabled by
allowing the same key to be repeated in the opts string.
It is not practical to extend the OptsVisitor to support
more complex data structures while also maintaining
the existing list handling behaviour that is relied upon
by other areas of QEMU.
Fortunately there is no existing object that implements
the UserCreatable interface that relies on the list
handling behaviour, so it is possible to swap out the
OptsVisitor for a different visitor implementation, so
-object supports non-scalar properties, thus leaving
other users of OptsVisitor unaffected.
The previously added qdict_crumple() method is able to
take a qdict containing a flat set of properties and
turn that into a arbitrarily nested set of dicts and
lists. By combining qemu_opts_to_qdict and qdict_crumple()
together, we can turn the opt string into a data structure
that is practically identical to that passed over QMP
when defining an object. The only difference is that all
the scalar values are represented as strings, rather than
strings, ints and bools. This is sufficient to let us
replace the OptsVisitor with the QMPInputVisitor for
use with -object.
Thus -object can now support non-scalar properties,
for example the QMP object
{
"execute": "object-add",
"arguments": {
"qom-type": "demo",
"id": "demo0",
"parameters": {
"foo": [
{ "bar": "one", "wizz": "1" },
{ "bar": "two", "wizz": "2" }
]
}
}
}
Would be creatable via the CLI now using
$QEMU \
-object demo,id=demo0,\
foo.0.bar=one,foo.0.wizz=1,\
foo.1.bar=two,foo.1.wizz=2
This is also wired up to work for the 'object_add' command
in the HMP monitor with the same syntax.
(hmp) object_add demo,id=demo0,\
foo.0.bar=one,foo.0.wizz=1,\
foo.1.bar=two,foo.1.wizz=2
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
hmp.c | 18 +--
qom/object_interfaces.c | 20 ++-
tests/check-qom-proplist.c | 295 ++++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 313 insertions(+), 20 deletions(-)
diff --git a/hmp.c b/hmp.c
index 5b6084a..7a98726 100644
--- a/hmp.c
+++ b/hmp.c
@@ -25,7 +25,7 @@
#include "qemu/sockets.h"
#include "monitor/monitor.h"
#include "monitor/qdev.h"
-#include "qapi/opts-visitor.h"
+#include "qapi/qmp-input-visitor.h"
#include "qapi/qmp/qerror.h"
#include "qapi/string-output-visitor.h"
#include "qapi/util.h"
@@ -1673,20 +1673,12 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
void hmp_object_add(Monitor *mon, const QDict *qdict)
{
Error *err = NULL;
- QemuOpts *opts;
- OptsVisitor *ov;
+ QmpInputVisitor *qiv;
Object *obj = NULL;
- opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, &err);
- if (err) {
- hmp_handle_error(mon, &err);
- return;
- }
-
- ov = opts_visitor_new(opts);
- obj = user_creatable_add(qdict, opts_get_visitor(ov), &err);
- opts_visitor_cleanup(ov);
- qemu_opts_del(opts);
+ qiv = qmp_input_visitor_new_full((QObject *)qdict, true, true);
+ obj = user_creatable_add(qdict, qmp_input_get_visitor(qiv), &err);
+ qmp_input_visitor_cleanup(qiv);
if (err) {
hmp_handle_error(mon, &err);
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index c2f6e29..9c41730 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -1,9 +1,9 @@
#include "qemu/osdep.h"
#include "qom/object_interfaces.h"
#include "qemu/module.h"
+#include "qemu/option.h"
#include "qapi-visit.h"
-#include "qapi/qmp-output-visitor.h"
-#include "qapi/opts-visitor.h"
+#include "qapi/qmp-input-visitor.h"
void user_creatable_complete(Object *obj, Error **errp)
{
@@ -120,6 +120,7 @@ Object *user_creatable_add_type(const char *type, const char *id,
obj = object_new(type);
if (qdict) {
for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
+
object_property_set(obj, v, e->key, &local_err);
if (local_err) {
goto out;
@@ -151,15 +152,22 @@ out:
Object *user_creatable_add_opts(QemuOpts *opts, Error **errp)
{
- OptsVisitor *ov;
+ QmpInputVisitor *qiv;
QDict *pdict;
+ QObject *pobj;
Object *obj = NULL;
- ov = opts_visitor_new(opts);
pdict = qemu_opts_to_qdict(opts, NULL);
+ pobj = qdict_crumple(pdict, true, errp);
+ if (!pobj) {
+ goto cleanup;
+ }
+ qiv = qmp_input_visitor_new_full(pobj, true, true);
- obj = user_creatable_add(pdict, opts_get_visitor(ov), errp);
- opts_visitor_cleanup(ov);
+ obj = user_creatable_add((QDict *)pobj, qmp_input_get_visitor(qiv), errp);
+ qmp_input_visitor_cleanup(qiv);
+ qobject_decref(pobj);
+ cleanup:
QDECREF(pdict);
return obj;
}
diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
index a2bb556..b19bfb0 100644
--- a/tests/check-qom-proplist.c
+++ b/tests/check-qom-proplist.c
@@ -23,6 +23,14 @@
#include "qom/object.h"
#include "qemu/module.h"
+#include "qapi/visitor.h"
+#include "qom/object_interfaces.h"
+#include "qemu/option.h"
+#include "qemu/config-file.h"
+#include "qapi/qmp/qjson.h"
+#include "qapi/qmp-input-visitor.h"
+#include "qapi-visit.h"
+#include "qapi/dealloc-visitor.h"
#define TYPE_DUMMY "qemu-dummy"
@@ -30,6 +38,11 @@
typedef struct DummyObject DummyObject;
typedef struct DummyObjectClass DummyObjectClass;
+typedef struct DummyPerson DummyPerson;
+typedef struct DummyAddr DummyAddr;
+typedef struct DummyAddrList DummyAddrList;
+typedef struct DummySizeList DummySizeList;
+
#define DUMMY_OBJECT(obj) \
OBJECT_CHECK(DummyObject, (obj), TYPE_DUMMY)
@@ -50,12 +63,35 @@ static const char *const dummy_animal_map[DUMMY_LAST + 1] = {
[DUMMY_LAST] = NULL,
};
+
+struct DummyAddr {
+ char *ip;
+ int64_t prefix;
+ bool ipv6only;
+};
+
+struct DummyAddrList {
+ DummyAddrList *next;
+ struct DummyAddr *value;
+};
+
+struct DummyPerson {
+ char *name;
+ int64_t age;
+};
+
struct DummyObject {
Object parent_obj;
bool bv;
DummyAnimal av;
char *sv;
+
+ intList *sizes;
+
+ DummyPerson *person;
+
+ DummyAddrList *addrs;
};
struct DummyObjectClass {
@@ -117,6 +153,135 @@ static char *dummy_get_sv(Object *obj,
return g_strdup(dobj->sv);
}
+static void visit_type_DummyPerson_fields(Visitor *v, DummyPerson **obj,
+ Error **errp)
+{
+ Error *err = NULL;
+
+ visit_type_str(v, "name", &(*obj)->name, &err);
+ if (err) {
+ goto out;
+ }
+ visit_type_int(v, "age", &(*obj)->age, &err);
+ if (err) {
+ goto out;
+ }
+
+out:
+ error_propagate(errp, err);
+}
+
+static void visit_type_DummyPerson(Visitor *v, const char *name,
+ DummyPerson **obj, Error **errp)
+{
+ Error *err = NULL;
+
+ visit_start_struct(v, name, (void **)obj, sizeof(DummyPerson), &err);
+ if (err) {
+ goto out;
+ }
+ if (!*obj) {
+ goto out_obj;
+ }
+ visit_type_DummyPerson_fields(v, obj, &err);
+ error_propagate(errp, err);
+ err = NULL;
+out_obj:
+ visit_end_struct(v, &err);
+out:
+ error_propagate(errp, err);
+}
+
+static void visit_type_DummyAddr_members(Visitor *v, DummyAddr **obj,
+ Error **errp)
+{
+ Error *err = NULL;
+
+ visit_type_str(v, "ip", &(*obj)->ip, &err);
+ if (err) {
+ goto out;
+ }
+ visit_type_int(v, "prefix", &(*obj)->prefix, &err);
+ if (err) {
+ goto out;
+ }
+ visit_type_bool(v, "ipv6only", &(*obj)->ipv6only, &err);
+ if (err) {
+ goto out;
+ }
+
+out:
+ error_propagate(errp, err);
+}
+
+static void visit_type_DummyAddr(Visitor *v, const char *name,
+ DummyAddr **obj, Error **errp)
+{
+ Error *err = NULL;
+
+ visit_start_struct(v, name, (void **)obj, sizeof(DummyAddr), &err);
+ if (err) {
+ goto out;
+ }
+ if (!*obj) {
+ goto out_obj;
+ }
+ visit_type_DummyAddr_members(v, obj, &err);
+ error_propagate(errp, err);
+ err = NULL;
+out_obj:
+ visit_end_struct(v, &err);
+out:
+ error_propagate(errp, err);
+}
+
+static void visit_type_DummyAddrList(Visitor *v, const char *name,
+ DummyAddrList **obj, Error **errp)
+{
+ Error *err = NULL;
+ GenericList *i, **prev;
+
+ visit_start_list(v, name, &err);
+ if (err) {
+ goto out;
+ }
+
+ for (prev = (GenericList **)obj;
+ !err && (i = visit_next_list(v, prev, sizeof(**obj))) != NULL;
+ prev = &i) {
+ DummyAddrList *native_i = (DummyAddrList *)i;
+ visit_type_DummyAddr(v, NULL, &native_i->value, &err);
+ }
+
+ visit_end_list(v);
+out:
+ error_propagate(errp, err);
+}
+
+
+static void dummy_set_sizes(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ DummyObject *dobj = DUMMY_OBJECT(obj);
+
+ visit_type_intList(v, name, &dobj->sizes, errp);
+}
+
+static void dummy_set_person(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ DummyObject *dobj = DUMMY_OBJECT(obj);
+
+ visit_type_DummyPerson(v, name, &dobj->person, errp);
+}
+
+static void dummy_set_addrs(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ DummyObject *dobj = DUMMY_OBJECT(obj);
+
+ visit_type_DummyAddrList(v, name, &dobj->addrs, errp);
+}
static void dummy_init(Object *obj)
{
@@ -126,9 +291,16 @@ static void dummy_init(Object *obj)
NULL);
}
+static void
+dummy_complete(UserCreatable *uc, Error **errp)
+{
+}
static void dummy_class_init(ObjectClass *cls, void *data)
{
+ UserCreatableClass *ucc = USER_CREATABLE_CLASS(cls);
+ ucc->complete = dummy_complete;
+
object_class_property_add_bool(cls, "bv",
dummy_get_bv,
dummy_set_bv,
@@ -143,12 +315,36 @@ static void dummy_class_init(ObjectClass *cls, void *data)
dummy_get_av,
dummy_set_av,
NULL);
+ object_class_property_add(cls, "sizes",
+ "int[]",
+ NULL,
+ dummy_set_sizes,
+ NULL, NULL, NULL);
+ object_class_property_add(cls, "person",
+ "DummyPerson",
+ NULL,
+ dummy_set_person,
+ NULL, NULL, NULL);
+ object_class_property_add(cls, "addrs",
+ "DummyAddrList",
+ NULL,
+ dummy_set_addrs,
+ NULL, NULL, NULL);
}
static void dummy_finalize(Object *obj)
{
DummyObject *dobj = DUMMY_OBJECT(obj);
+ QapiDeallocVisitor *qdv;
+ Visitor *v;
+
+ qdv = qapi_dealloc_visitor_new();
+ v = qapi_dealloc_get_visitor(qdv);
+ visit_type_intList(v, NULL, &dobj->sizes, NULL);
+ visit_type_DummyAddrList(v, NULL, &dobj->addrs, NULL);
+ visit_type_DummyPerson(v, NULL, &dobj->person, NULL);
+ qapi_dealloc_visitor_cleanup(qdv);
g_free(dobj->sv);
}
@@ -162,6 +358,10 @@ static const TypeInfo dummy_info = {
.instance_finalize = dummy_finalize,
.class_size = sizeof(DummyObjectClass),
.class_init = dummy_class_init,
+ .interfaces = (InterfaceInfo[]) {
+ { TYPE_USER_CREATABLE },
+ { }
+ }
};
@@ -457,7 +657,9 @@ static void test_dummy_iterator(void)
ObjectProperty *prop;
ObjectPropertyIterator iter;
- bool seenbv = false, seensv = false, seenav = false, seentype;
+ bool seenbv = false, seensv = false, seenav = false,
+ seentype = false, seenaddrs = false, seenperson = false,
+ seensizes = false;
object_property_iter_init(&iter, OBJECT(dobj));
while ((prop = object_property_iter_next(&iter))) {
@@ -470,6 +672,12 @@ static void test_dummy_iterator(void)
} else if (g_str_equal(prop->name, "type")) {
/* This prop comes from the base Object class */
seentype = true;
+ } else if (g_str_equal(prop->name, "addrs")) {
+ seenaddrs = true;
+ } else if (g_str_equal(prop->name, "person")) {
+ seenperson = true;
+ } else if (g_str_equal(prop->name, "sizes")) {
+ seensizes = true;
} else {
g_printerr("Found prop '%s'\n", prop->name);
g_assert_not_reached();
@@ -479,6 +687,9 @@ static void test_dummy_iterator(void)
g_assert(seenav);
g_assert(seensv);
g_assert(seentype);
+ g_assert(seenaddrs);
+ g_assert(seenperson);
+ g_assert(seensizes);
object_unparent(OBJECT(dobj));
}
@@ -497,11 +708,91 @@ static void test_dummy_delchild(void)
object_unparent(OBJECT(dev));
}
+
+static QemuOptsList dummy_opts = {
+ .name = "object",
+ .implied_opt_name = "qom-type",
+ .head = QTAILQ_HEAD_INITIALIZER(dummy_opts.head),
+ .desc = {
+ { }
+ },
+};
+
+static void test_dummy_create_complex(DummyObject *dummy)
+{
+ g_assert(dummy->person != NULL);
+ g_assert_cmpstr(dummy->person->name, ==, "fred");
+ g_assert_cmpint(dummy->person->age, ==, 52);
+
+ g_assert(dummy->sizes != NULL);
+ g_assert_cmpint(dummy->sizes->value, ==, 12);
+ g_assert_cmpint(dummy->sizes->next->value, ==, 65);
+ g_assert_cmpint(dummy->sizes->next->next->value, ==, 8139);
+
+ g_assert(dummy->addrs != NULL);
+ g_assert_cmpstr(dummy->addrs->value->ip, ==, "127.0.0.1");
+ g_assert_cmpint(dummy->addrs->value->prefix, ==, 24);
+ g_assert(dummy->addrs->value->ipv6only);
+ g_assert_cmpstr(dummy->addrs->next->value->ip, ==, "0.0.0.0");
+ g_assert_cmpint(dummy->addrs->next->value->prefix, ==, 16);
+ g_assert(!dummy->addrs->next->value->ipv6only);
+}
+
+
+static void test_dummy_createopts(void)
+{
+ const char *optstr = "qemu-dummy,id=dummy0,bv=yes,av=alligator,sv=hiss,"
+ "person.name=fred,person.age=52,sizes.0=12,sizes.1=65,sizes.2=8139,"
+ "addrs.0.ip=127.0.0.1,addrs.0.prefix=24,addrs.0.ipv6only=yes,"
+ "addrs.1.ip=0.0.0.0,addrs.1.prefix=16,addrs.1.ipv6only=no";
+ QemuOpts *opts;
+ DummyObject *dummy;
+
+ opts = qemu_opts_parse_noisily(&dummy_opts,
+ optstr, true);
+ g_assert(opts != NULL);
+
+ dummy = DUMMY_OBJECT(user_creatable_add_opts(opts, &error_abort));
+
+ test_dummy_create_complex(dummy);
+
+ object_unparent(OBJECT(dummy));
+ object_unref(OBJECT(dummy));
+}
+
+
+static void test_dummy_createqmp(void)
+{
+ const char *jsonstr =
+ "{ 'qom-type': 'qemu-dummy', 'id': 'dummy0', "
+ " 'bv': true, 'av': 'alligator', 'sv': 'hiss', "
+ " 'person': { 'name': 'fred', 'age': 52 }, "
+ " 'sizes': [12, 65, 8139], "
+ " 'addrs': [ { 'ip': '127.0.0.1', 'prefix': 24, 'ipv6only': true }, "
+ " { 'ip': '0.0.0.0', 'prefix': 16, 'ipv6only': false } ] }";
+ QObject *obj = qobject_from_json(jsonstr);
+ QmpInputVisitor *qiv = qmp_input_visitor_new_strict(obj);
+ DummyObject *dummy;
+ g_assert(obj);
+ dummy = DUMMY_OBJECT(user_creatable_add(qobject_to_qdict(obj),
+ qmp_input_get_visitor(qiv),
+ &error_abort));
+
+ test_dummy_create_complex(dummy);
+ qmp_input_visitor_cleanup(qiv);
+ object_unparent(OBJECT(dummy));
+ object_unref(OBJECT(dummy));
+ qobject_decref(obj);
+}
+
int main(int argc, char **argv)
{
g_test_init(&argc, &argv, NULL);
module_call_init(MODULE_INIT_QOM);
+
+ qemu_add_opts(&dummy_opts);
+
type_register_static(&dummy_info);
type_register_static(&dummy_dev_info);
type_register_static(&dummy_bus_info);
@@ -513,6 +804,8 @@ int main(int argc, char **argv)
g_test_add_func("/qom/proplist/getenum", test_dummy_getenum);
g_test_add_func("/qom/proplist/iterator", test_dummy_iterator);
g_test_add_func("/qom/proplist/delchild", test_dummy_delchild);
+ g_test_add_func("/qom/proplist/createopts", test_dummy_createopts);
+ g_test_add_func("/qom/proplist/createqmp", test_dummy_createqmp);
return g_test_run();
}
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v3 04/10] util: add QAuthZ object as an authorization base class
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 01/10] qdict: implement a qdict_crumple method for un-flattening a dict Daniel P. Berrange
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 02/10] qapi: allow QmpInputVisitor to auto-cast types Daniel P. Berrange
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 03/10] qom: support arbitrary non-scalar properties with -object Daniel P. Berrange
@ 2016-03-10 18:59 ` Daniel P. Berrange
2016-03-22 16:33 ` Eric Blake
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 05/10] util: add QAuthZSimple object type for a simple access control list Daniel P. Berrange
` (6 subsequent siblings)
9 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrange @ 2016-03-10 18:59 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Markus Armbruster, Max Reitz, Paolo Bonzini,
Andreas Färber
The current qemu_acl module provides a simple access control
list facility inside QEMU, which is used via a set of monitor
commands acl_show, acl_policy, acl_add, acl_remove & acl_reset.
Note there is no ability to create ACLs - the network services
(eg VNC server) were expected to create ACLs that they want to
check.
There is also no way to define ACLs on the command line, nor
potentially integrate with external authorization systems like
polkit, pam, ldap lookup, etc.
The QAuthZ object defines a minimal abstract QOM class that can
be subclassed for creating different authorization providers.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
MAINTAINERS | 7 +++++
Makefile | 1 +
Makefile.objs | 2 ++
Makefile.target | 2 ++
include/qemu/authz.h | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++
util/Makefile.objs | 2 ++
util/authz.c | 46 +++++++++++++++++++++++++++++
7 files changed, 141 insertions(+)
create mode 100644 include/qemu/authz.h
create mode 100644 util/authz.c
diff --git a/MAINTAINERS b/MAINTAINERS
index dc0aa54..73bc431 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1301,6 +1301,13 @@ F: include/qemu/throttle.h
F: util/throttle.c
L: qemu-block@nongnu.org
+Authorization
+M: Daniel P. Berrange <berrange@redhat.com>
+S: Maintained
+F: util/authz*
+F: include/qemu/authz*
+F: tests/test-authz-*
+
Usermode Emulation
------------------
Overall
diff --git a/Makefile b/Makefile
index 70e3ebc..903dc35 100644
--- a/Makefile
+++ b/Makefile
@@ -150,6 +150,7 @@ endif
dummy := $(call unnest-vars,, \
stub-obj-y \
util-obj-y \
+ util-qom-obj-y \
qga-obj-y \
ivshmem-client-obj-y \
ivshmem-server-obj-y \
diff --git a/Makefile.objs b/Makefile.objs
index fbcaa74..8bc9a77 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -4,6 +4,8 @@ stub-obj-y = stubs/
util-obj-y = util/ qobject/ qapi/
util-obj-y += qmp-introspect.o qapi-types.o qapi-visit.o qapi-event.o
+util-qom-obj-y += util/
+
#######################################################################
# block-obj-y is code used by both qemu system emulation and qemu-img
diff --git a/Makefile.target b/Makefile.target
index 34ddb7e..9728f86 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -171,6 +171,7 @@ include $(SRC_PATH)/Makefile.objs
dummy := $(call unnest-vars,,target-obj-y)
target-obj-y-save := $(target-obj-y)
dummy := $(call unnest-vars,.., \
+ util-qom-obj-y \
block-obj-y \
block-obj-m \
crypto-obj-y \
@@ -183,6 +184,7 @@ target-obj-y := $(target-obj-y-save)
all-obj-y += $(common-obj-y)
all-obj-y += $(target-obj-y)
all-obj-y += $(qom-obj-y)
+all-obj-y += $(util-qom-obj-y)
all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y)
all-obj-$(CONFIG_USER_ONLY) += $(crypto-aes-obj-y)
all-obj-$(CONFIG_SOFTMMU) += $(crypto-obj-y)
diff --git a/include/qemu/authz.h b/include/qemu/authz.h
new file mode 100644
index 0000000..89fa6da
--- /dev/null
+++ b/include/qemu/authz.h
@@ -0,0 +1,81 @@
+/*
+ * QEMU authorization framework
+ *
+ * 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 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 QAUTHZ_H__
+#define QAUTHZ_H__
+
+#include "qemu-common.h"
+#include "qapi/error.h"
+#include "qom/object.h"
+
+
+#define TYPE_QAUTHZ "authz"
+
+#define QAUTHZ_CLASS(klass) \
+ OBJECT_CLASS_CHECK(QAuthZClass, (klass), \
+ TYPE_QAUTHZ)
+#define QAUTHZ_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(QAuthZClass, (obj), \
+ TYPE_QAUTHZ)
+#define QAUTHZ(obj) \
+ INTERFACE_CHECK(QAuthZ, (obj), \
+ TYPE_QAUTHZ)
+
+typedef struct QAuthZ QAuthZ;
+typedef struct QAuthZClass QAuthZClass;
+
+/**
+ * QAuthZ:
+ *
+ * The QAuthZ class defines an API contract to be used
+ * for providing an authorization driver for network
+ * services.
+ */
+
+struct QAuthZ {
+ Object parent_obj;
+};
+
+
+struct QAuthZClass {
+ ObjectClass parent_class;
+
+ bool (*is_allowed)(QAuthZ *authz,
+ const char *identity,
+ Error **errp);
+};
+
+
+/**
+ * qauthz_is_allowed:
+ * @authz: the authorization object
+ * @identity: the user identity to authorize
+ * @errp: pointer to a NULL initialized error object
+ *
+ * Check if a user @identity is authorized
+ *
+ * Returns: true if @identity is authorizd, false otherwise
+ */
+bool qauthz_is_allowed(QAuthZ *authz,
+ const char *identity,
+ Error **errp);
+
+#endif /* QAUTHZ_H__ */
+
diff --git a/util/Makefile.objs b/util/Makefile.objs
index a8a777e..b29fb45 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -32,3 +32,5 @@ util-obj-y += buffer.o
util-obj-y += timed-average.o
util-obj-y += base64.o
util-obj-y += log.o
+
+util-qom-obj-y += authz.o
diff --git a/util/authz.c b/util/authz.c
new file mode 100644
index 0000000..fd9f84e
--- /dev/null
+++ b/util/authz.c
@@ -0,0 +1,46 @@
+/*
+ * QEMU authorization framework
+ *
+ * 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 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 "qemu/osdep.h"
+#include "qemu/authz.h"
+
+bool qauthz_is_allowed(QAuthZ *authz,
+ const char *identity,
+ Error **errp)
+{
+ QAuthZClass *cls = QAUTHZ_GET_CLASS(authz);
+
+ return cls->is_allowed(authz, identity, errp);
+}
+
+static const TypeInfo authz_info = {
+ .parent = TYPE_OBJECT,
+ .name = TYPE_QAUTHZ,
+ .instance_size = sizeof(QAuthZ),
+ .class_size = sizeof(QAuthZClass),
+};
+
+static void qauthz_register_types(void)
+{
+ type_register_static(&authz_info);
+}
+
+type_init(qauthz_register_types)
+
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v3 05/10] util: add QAuthZSimple object type for a simple access control list
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 01/10] qdict: implement a qdict_crumple method for un-flattening a dict Daniel P. Berrange
` (2 preceding siblings ...)
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 04/10] util: add QAuthZ object as an authorization base class Daniel P. Berrange
@ 2016-03-10 18:59 ` Daniel P. Berrange
2016-03-22 17:38 ` Eric Blake
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 06/10] acl: delete existing ACL implementation Daniel P. Berrange
` (5 subsequent siblings)
9 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrange @ 2016-03-10 18:59 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Markus Armbruster, Max Reitz, Paolo Bonzini,
Andreas Färber
Add a QAuthZSimple object type that implements the QAuthZ
interface. This simple built-in implementation maintains
a trivial access control list with a sequence of match
rules and a final default policy. This replicates the
functionality currently provided by the qemu_acl module.
To create an instance of this object via the QMP monitor,
the syntax used would be
{
"execute": "object-add",
"arguments": {
"qom-type": "authz-simple",
"id": "auth0",
"parameters": {
"rules": [
{ "match": "fred", "policy": "allow" },
{ "match": "bob", "policy": "allow" },
{ "match": "danb", "policy": "deny" },
{ "match": "dan*", "policy": "allow" }
],
"policy": "deny"
}
}
}
Or via the -object command line
$QEMU \
-object authz-simple,id=acl0,policy=deny,\
match.0.name=fred,match.0.policy=allow, \
match.1.name=bob,match.1.policy=allow, \
match.2.name=danb,match.2.policy=deny, \
match.3.name=dan*,match.3.policy=allow
This sets up an authorization rule that allows 'fred',
'bob' and anyone whose name starts with 'dan', except
for 'danb'. Everyone unmatched is denied.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
Makefile | 2 +-
include/qemu/authz-simple.h | 107 ++++++++++++++++++
qapi-schema.json | 6 +-
qapi/util.json | 31 ++++++
tests/.gitignore | 1 +
tests/Makefile | 3 +
tests/test-authz-simple.c | 156 +++++++++++++++++++++++++++
util/Makefile.objs | 1 +
util/authz-simple.c | 256 ++++++++++++++++++++++++++++++++++++++++++++
9 files changed, 561 insertions(+), 2 deletions(-)
create mode 100644 include/qemu/authz-simple.h
create mode 100644 qapi/util.json
create mode 100644 tests/test-authz-simple.c
create mode 100644 util/authz-simple.c
diff --git a/Makefile b/Makefile
index 903dc35..60ad13e 100644
--- a/Makefile
+++ b/Makefile
@@ -274,7 +274,7 @@ qapi-modules = $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/qapi/common.json \
$(SRC_PATH)/qapi/block.json $(SRC_PATH)/qapi/block-core.json \
$(SRC_PATH)/qapi/event.json $(SRC_PATH)/qapi/introspect.json \
$(SRC_PATH)/qapi/crypto.json $(SRC_PATH)/qapi/rocker.json \
- $(SRC_PATH)/qapi/trace.json
+ $(SRC_PATH)/qapi/trace.json $(SRC_PATH)/qapi/util.json
qapi-types.c qapi-types.h :\
$(qapi-modules) $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
diff --git a/include/qemu/authz-simple.h b/include/qemu/authz-simple.h
new file mode 100644
index 0000000..74c09e3
--- /dev/null
+++ b/include/qemu/authz-simple.h
@@ -0,0 +1,107 @@
+/*
+ * QEMU simple authorization driver
+ *
+ * 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 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 QAUTHZ_SIMPLE_H__
+#define QAUTHZ_SIMPLE_H__
+
+#include "qemu/authz.h"
+
+
+#define TYPE_QAUTHZ_SIMPLE "authz-simple"
+
+#define QAUTHZ_SIMPLE_CLASS(klass) \
+ OBJECT_CLASS_CHECK(QAuthZSimpleClass, (klass), \
+ TYPE_QAUTHZ_SIMPLE)
+#define QAUTHZ_SIMPLE_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(QAuthZSimpleClass, (obj), \
+ TYPE_QAUTHZ_SIMPLE)
+#define QAUTHZ_SIMPLE(obj) \
+ INTERFACE_CHECK(QAuthZSimple, (obj), \
+ TYPE_QAUTHZ_SIMPLE)
+
+typedef struct QAuthZSimple QAuthZSimple;
+typedef struct QAuthZSimpleClass QAuthZSimpleClass;
+
+
+/**
+ * QAuthZSimple:
+ *
+ * This authorization driver provides a simple mechanism
+ * for granting access by matching user names against a
+ * list of globs. Each match rule has an associated policy
+ * and a catch all policy applies if no rule matches
+ *
+ * To create an instace of this class via QMP:
+ *
+ * {
+ * "execute": "object-add",
+ * "arguments": {
+ * "qom-type": "authz-simple",
+ * "id": "auth0",
+ * "parameters": {
+ * "rules": [
+ * { "match": "fred", "policy": "allow" },
+ * { "match": "bob", "policy": "allow" },
+ * { "match": "danb", "policy": "deny" },
+ * { "match": "dan*", "policy": "allow" }
+ * ],
+ * "policy": "deny"
+ * }
+ * }
+ * }
+ *
+ * Or via the CLI:
+ *
+ * $QEMU \
+ * -object authz-simple,id=acl0,policy=deny, \
+ * match.0.name=fred,match.0.policy=allow, \
+ * match.1.name=bob,match.1.policy=allow, \
+ * match.2.name=danb,match.2.policy=deny, \
+ * match.3.name=dan*,match.3.policy=allow
+ *
+ */
+struct QAuthZSimple {
+ QAuthZ parent_obj;
+
+ QAuthZSimplePolicy policy;
+ QAuthZSimpleRuleList *rules;
+};
+
+
+struct QAuthZSimpleClass {
+ QAuthZClass parent_class;
+};
+
+
+QAuthZSimple *qauthz_simple_new(const char *id,
+ QAuthZSimplePolicy policy,
+ Error **errp);
+
+size_t qauthz_simple_append_rule(QAuthZSimple *auth, const char *match,
+ QAuthZSimplePolicy policy);
+
+size_t qauthz_simple_insert_rule(QAuthZSimple *auth, const char *match,
+ QAuthZSimplePolicy policy, size_t index);
+
+ssize_t qauthz_simple_delete_rule(QAuthZSimple *auth, const char *match);
+
+
+#endif /* QAUTHZ_SIMPLE_H__ */
+
diff --git a/qapi-schema.json b/qapi-schema.json
index 362c9d8..b6769de 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -5,6 +5,9 @@
# QAPI common definitions
{ 'include': 'qapi/common.json' }
+# QAPI util definitions
+{ 'include': 'qapi/util.json' }
+
# QAPI crypto definitions
{ 'include': 'qapi/crypto.json' }
@@ -3652,7 +3655,8 @@
# Since 2.5
##
{ 'struct': 'DummyForceArrays',
- 'data': { 'unused': ['X86CPUFeatureWordInfo'] } }
+ 'data': { 'unused': ['X86CPUFeatureWordInfo'],
+ 'iamalsounused': ['QAuthZSimpleRule'] } }
##
diff --git a/qapi/util.json b/qapi/util.json
new file mode 100644
index 0000000..d903497
--- /dev/null
+++ b/qapi/util.json
@@ -0,0 +1,31 @@
+# -*- Mode: Python -*-
+#
+# QAPI util definitions
+
+##
+# QAuthZSimplePolicy:
+#
+# The authorization policy result
+#
+# @deny: deny access
+# @allow: allow access
+#
+# Since: 2.6
+##
+{ 'enum': 'QAuthZSimplePolicy',
+ 'prefix': 'QAUTHZ_SIMPLE_POLICY',
+ 'data': ['deny', 'allow']}
+
+##
+# QAuthZSimpleRule:
+#
+# A single authorization rule.
+#
+# @match: a glob to match against a user identity
+# @policy: the result to return if @match evaluates to true
+#
+# Since: 2.6
+##
+{ 'struct': 'QAuthZSimpleRule',
+ 'data': {'match': 'str',
+ 'policy': 'QAuthZSimplePolicy'}}
diff --git a/tests/.gitignore b/tests/.gitignore
index 787c95c..1e73a2f 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -8,6 +8,7 @@ check-qom-interface
check-qom-proplist
rcutorture
test-aio
+test-authz-simple
test-base64
test-bitops
test-blockjob-txn
diff --git a/tests/Makefile b/tests/Makefile
index cd4bbd4..7a1d959 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -92,6 +92,7 @@ check-unit-$(CONFIG_GNUTLS) += tests/test-io-channel-tls$(EXESUF)
check-unit-y += tests/test-io-channel-command$(EXESUF)
check-unit-y += tests/test-io-channel-buffer$(EXESUF)
check-unit-y += tests/test-base64$(EXESUF)
+check-unit-y += tests/test-authz-simple$(EXESUF)
check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
@@ -430,6 +431,8 @@ 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-authz-simple$(EXESUF): tests/test-authz-simple.o \
+ libqemuutil.a libqemustub.a $(util-qom-obj-y) $(qom-obj-y)
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-authz-simple.c b/tests/test-authz-simple.c
new file mode 100644
index 0000000..4b1dd0c
--- /dev/null
+++ b/tests/test-authz-simple.c
@@ -0,0 +1,156 @@
+/*
+ * QEMU simple authorization object
+ *
+ * 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 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 "qemu/osdep.h"
+#include <glib.h>
+
+#include "qemu/authz-simple.h"
+
+static void test_authz_default_deny(void)
+{
+ QAuthZSimple *auth = qauthz_simple_new("auth0",
+ QAUTHZ_SIMPLE_POLICY_DENY,
+ &error_abort);
+
+ g_assert(!qauthz_is_allowed(QAUTHZ(auth), "fred", &error_abort));
+
+ object_unparent(OBJECT(auth));
+}
+
+static void test_authz_default_allow(void)
+{
+ QAuthZSimple *auth = qauthz_simple_new("auth0",
+ QAUTHZ_SIMPLE_POLICY_ALLOW,
+ &error_abort);
+
+ g_assert(qauthz_is_allowed(QAUTHZ(auth), "fred", &error_abort));
+
+ object_unparent(OBJECT(auth));
+}
+
+static void test_authz_explicit_deny(void)
+{
+ QAuthZSimple *auth = qauthz_simple_new("auth0",
+ QAUTHZ_SIMPLE_POLICY_ALLOW,
+ &error_abort);
+
+ qauthz_simple_append_rule(auth, "fred", QAUTHZ_SIMPLE_POLICY_DENY);
+
+ g_assert(!qauthz_is_allowed(QAUTHZ(auth), "fred", &error_abort));
+
+ object_unparent(OBJECT(auth));
+}
+
+static void test_authz_explicit_allow(void)
+{
+ QAuthZSimple *auth = qauthz_simple_new("auth0",
+ QAUTHZ_SIMPLE_POLICY_DENY,
+ &error_abort);
+
+ qauthz_simple_append_rule(auth, "fred", QAUTHZ_SIMPLE_POLICY_ALLOW);
+
+ g_assert(qauthz_is_allowed(QAUTHZ(auth), "fred", &error_abort));
+
+ object_unparent(OBJECT(auth));
+}
+
+
+#ifdef CONFIG_FNMATCH
+static void test_authz_complex(void)
+{
+ QAuthZSimple *auth = qauthz_simple_new("auth0",
+ QAUTHZ_SIMPLE_POLICY_DENY,
+ &error_abort);
+
+ qauthz_simple_append_rule(auth, "fred", QAUTHZ_SIMPLE_POLICY_ALLOW);
+ qauthz_simple_append_rule(auth, "bob", QAUTHZ_SIMPLE_POLICY_ALLOW);
+ qauthz_simple_append_rule(auth, "dan", QAUTHZ_SIMPLE_POLICY_DENY);
+ qauthz_simple_append_rule(auth, "dan*", QAUTHZ_SIMPLE_POLICY_ALLOW);
+
+ g_assert(qauthz_is_allowed(QAUTHZ(auth), "fred", &error_abort));
+ g_assert(qauthz_is_allowed(QAUTHZ(auth), "bob", &error_abort));
+ g_assert(!qauthz_is_allowed(QAUTHZ(auth), "dan", &error_abort));
+ g_assert(qauthz_is_allowed(QAUTHZ(auth), "danb", &error_abort));
+
+ object_unparent(OBJECT(auth));
+}
+#endif
+
+static void test_authz_add_remove(void)
+{
+ QAuthZSimple *auth = qauthz_simple_new("auth0",
+ QAUTHZ_SIMPLE_POLICY_DENY,
+ &error_abort);
+
+ g_assert_cmpint(qauthz_simple_append_rule(auth, "fred",
+ QAUTHZ_SIMPLE_POLICY_ALLOW),
+ ==, 0);
+ g_assert_cmpint(qauthz_simple_append_rule(auth, "bob",
+ QAUTHZ_SIMPLE_POLICY_ALLOW),
+ ==, 1);
+ g_assert_cmpint(qauthz_simple_append_rule(auth, "dan",
+ QAUTHZ_SIMPLE_POLICY_DENY),
+ ==, 2);
+ g_assert_cmpint(qauthz_simple_append_rule(auth, "dan*",
+ QAUTHZ_SIMPLE_POLICY_ALLOW),
+ ==, 3);
+
+ g_assert(!qauthz_is_allowed(QAUTHZ(auth), "dan", &error_abort));
+
+ g_assert_cmpint(qauthz_simple_delete_rule(auth, "dan"),
+ ==, 2);
+
+ g_assert(qauthz_is_allowed(QAUTHZ(auth), "dan", &error_abort));
+
+ g_assert_cmpint(qauthz_simple_append_rule(auth, "dan",
+ QAUTHZ_SIMPLE_POLICY_DENY),
+ ==, 3);
+
+ g_assert(qauthz_is_allowed(QAUTHZ(auth), "dan", &error_abort));
+
+ g_assert_cmpint(qauthz_simple_delete_rule(auth, "dan"),
+ ==, 3);
+
+ g_assert_cmpint(qauthz_simple_insert_rule(auth, "dan",
+ QAUTHZ_SIMPLE_POLICY_DENY, 2),
+ ==, 2);
+
+ g_assert(!qauthz_is_allowed(QAUTHZ(auth), "dan", &error_abort));
+
+ object_unparent(OBJECT(auth));
+}
+
+int main(int argc, char **argv)
+{
+ g_test_init(&argc, &argv, NULL);
+
+ module_call_init(MODULE_INIT_QOM);
+
+ g_test_add_func("/auth/simple/default/deny", test_authz_default_deny);
+ g_test_add_func("/auth/simple/default/allow", test_authz_default_allow);
+ g_test_add_func("/auth/simple/explicit/deny", test_authz_explicit_deny);
+ g_test_add_func("/auth/simple/explicit/allow", test_authz_explicit_allow);
+#ifdef CONFIG_FNMATCH
+ g_test_add_func("/auth/simple/complex", test_authz_complex);
+#endif
+ g_test_add_func("/auth/simple/add-remove", test_authz_add_remove);
+
+ return g_test_run();
+}
diff --git a/util/Makefile.objs b/util/Makefile.objs
index b29fb45..4870905 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -34,3 +34,4 @@ util-obj-y += base64.o
util-obj-y += log.o
util-qom-obj-y += authz.o
+util-qom-obj-y += authz-simple.o
diff --git a/util/authz-simple.c b/util/authz-simple.c
new file mode 100644
index 0000000..ab82f72
--- /dev/null
+++ b/util/authz-simple.c
@@ -0,0 +1,256 @@
+/*
+ * QEMU simple authorization driver
+ *
+ * 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 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 "qemu/osdep.h"
+#include "qemu/authz-simple.h"
+#include "qom/object_interfaces.h"
+#include "qapi-visit.h"
+
+/* If no fnmatch, fallback to exact string matching
+ * instead of allowing wildcards */
+#ifdef CONFIG_FNMATCH
+#include <fnmatch.h>
+#define qauthz_match(pattern, string) fnmatch(pattern, string, 0)
+#else
+#define qauthz_match(pattern, string) strcmp(pattern, string)
+#endif
+
+static bool qauthz_simple_is_allowed(QAuthZ *authz,
+ const char *identity,
+ Error **errp)
+{
+ QAuthZSimple *sauthz = QAUTHZ_SIMPLE(authz);
+ QAuthZSimpleRuleList *rules = sauthz->rules;
+
+ while (rules) {
+ QAuthZSimpleRule *rule = rules->value;
+ if (qauthz_match(rule->match, identity) == 0) {
+ return rule->policy == QAUTHZ_SIMPLE_POLICY_ALLOW;
+ }
+ rules = rules->next;
+ }
+
+ return sauthz->policy == QAUTHZ_SIMPLE_POLICY_ALLOW;
+}
+
+
+static void
+qauthz_simple_prop_set_policy(Object *obj,
+ int value,
+ Error **errp G_GNUC_UNUSED)
+{
+ QAuthZSimple *sauthz = QAUTHZ_SIMPLE(obj);
+
+ sauthz->policy = value;
+}
+
+
+static int
+qauthz_simple_prop_get_policy(Object *obj,
+ Error **errp G_GNUC_UNUSED)
+{
+ QAuthZSimple *sauthz = QAUTHZ_SIMPLE(obj);
+
+ return sauthz->policy;
+}
+
+
+static void
+qauthz_simple_prop_get_rules(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ QAuthZSimple *sauthz = QAUTHZ_SIMPLE(obj);
+
+ visit_type_QAuthZSimpleRuleList(v, name, &sauthz->rules, errp);
+}
+
+static void
+qauthz_simple_prop_set_rules(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ QAuthZSimple *sauthz = QAUTHZ_SIMPLE(obj);
+
+ qapi_free_QAuthZSimpleRuleList(sauthz->rules);
+ visit_type_QAuthZSimpleRuleList(v, name, &sauthz->rules, errp);
+}
+
+
+static void
+qauthz_simple_complete(UserCreatable *uc, Error **errp)
+{
+}
+
+
+static void
+qauthz_simple_finalize(Object *obj)
+{
+ QAuthZSimple *sauthz = QAUTHZ_SIMPLE(obj);
+
+ qapi_free_QAuthZSimpleRuleList(sauthz->rules);
+}
+
+
+static void
+qauthz_simple_class_init(ObjectClass *oc, void *data)
+{
+ UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
+ QAuthZClass *authz = QAUTHZ_CLASS(oc);
+
+ ucc->complete = qauthz_simple_complete;
+ authz->is_allowed = qauthz_simple_is_allowed;
+
+ object_class_property_add_enum(oc, "policy",
+ "QAuthZSimplePolicy",
+ QAuthZSimplePolicy_lookup,
+ qauthz_simple_prop_get_policy,
+ qauthz_simple_prop_set_policy,
+ NULL);
+
+ object_class_property_add(oc, "rules", "QAuthZSimpleRule",
+ qauthz_simple_prop_get_rules,
+ qauthz_simple_prop_set_rules,
+ NULL, NULL, NULL);
+}
+
+
+QAuthZSimple *qauthz_simple_new(const char *id,
+ QAuthZSimplePolicy policy,
+ Error **errp)
+{
+ return QAUTHZ_SIMPLE(
+ object_new_with_props(TYPE_QAUTHZ_SIMPLE,
+ object_get_objects_root(),
+ id, errp,
+ "policy", QAuthZSimplePolicy_lookup[policy],
+ NULL));
+}
+
+
+size_t qauthz_simple_append_rule(QAuthZSimple *auth, const char *match,
+ QAuthZSimplePolicy policy)
+{
+ QAuthZSimpleRule *rule;
+ QAuthZSimpleRuleList *rules, *tmp;
+ size_t i = 0;
+
+ rule = g_new0(QAuthZSimpleRule, 1);
+ rule->policy = policy;
+ rule->match = g_strdup(match);
+
+ tmp = g_new0(QAuthZSimpleRuleList, 1);
+ tmp->value = rule;
+
+ rules = auth->rules;
+ if (rules) {
+ while (rules->next) {
+ i++;
+ rules = rules->next;
+ }
+ rules->next = tmp;
+ return i + 1;
+ } else {
+ auth->rules = tmp;
+ return 0;
+ }
+}
+
+
+size_t qauthz_simple_insert_rule(QAuthZSimple *auth, const char *match,
+ QAuthZSimplePolicy policy, size_t index)
+{
+ QAuthZSimpleRule *rule;
+ QAuthZSimpleRuleList *rules, *tmp;
+ size_t i = 0;
+
+ rule = g_new0(QAuthZSimpleRule, 1);
+ rule->policy = policy;
+ rule->match = g_strdup(match);
+
+ tmp = g_new0(QAuthZSimpleRuleList, 1);
+ tmp->value = rule;
+
+ rules = auth->rules;
+ if (rules && index > 0) {
+ while (rules->next && i < (index - 1)) {
+ i++;
+ rules = rules->next;
+ }
+ tmp->next = rules->next;
+ rules->next = tmp;
+ return i + 1;
+ } else {
+ tmp->next = auth->rules;
+ auth->rules = tmp;
+ return 0;
+ }
+}
+
+
+ssize_t qauthz_simple_delete_rule(QAuthZSimple *auth, const char *match)
+{
+ QAuthZSimpleRule *rule;
+ QAuthZSimpleRuleList *rules, *prev;
+ size_t i = 0;
+
+ prev = NULL;
+ rules = auth->rules;
+ while (rules) {
+ rule = rules->value;
+ if (g_str_equal(rule->match, match)) {
+ if (prev) {
+ prev->next = rules->next;
+ } else {
+ auth->rules = rules->next;
+ }
+ rules->next = NULL;
+ qapi_free_QAuthZSimpleRuleList(rules);
+ return i;
+ }
+ prev = rules;
+ rules = rules->next;
+ i++;
+ }
+
+ return -1;
+}
+
+
+static const TypeInfo qauthz_simple_info = {
+ .parent = TYPE_QAUTHZ,
+ .name = TYPE_QAUTHZ_SIMPLE,
+ .instance_size = sizeof(QAuthZSimple),
+ .instance_finalize = qauthz_simple_finalize,
+ .class_size = sizeof(QAuthZSimpleClass),
+ .class_init = qauthz_simple_class_init,
+ .interfaces = (InterfaceInfo[]) {
+ { TYPE_USER_CREATABLE },
+ { }
+ }
+};
+
+
+static void
+qauthz_simple_register_types(void)
+{
+ type_register_static(&qauthz_simple_info);
+}
+
+
+type_init(qauthz_simple_register_types);
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v3 06/10] acl: delete existing ACL implementation
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 01/10] qdict: implement a qdict_crumple method for un-flattening a dict Daniel P. Berrange
` (3 preceding siblings ...)
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 05/10] util: add QAuthZSimple object type for a simple access control list Daniel P. Berrange
@ 2016-03-10 18:59 ` Daniel P. Berrange
2016-03-22 17:58 ` Eric Blake
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 07/10] qemu-nbd: add support for ACLs for TLS clients Daniel P. Berrange
` (4 subsequent siblings)
9 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrange @ 2016-03-10 18:59 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Markus Armbruster, Max Reitz, Paolo Bonzini,
Andreas Färber
The 'qemu_acl' type was a previous non-QOM based attempt to
provide an authorization facility in QEMU. Because it is
non-QOM based it cannot be created via the command line and
requires special monitor commands to manipulate it.
The new QAuthZ and QAuthZSimple QOM classes provide a superset
of the functionality in qemu_acl, so the latter can now be
deleted. The HMP 'acl_*' monitor commands are converted to
use the new QAuthZSimple data type instead in order to provide
backwards compatibility, but their use is discouraged.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
Makefile | 6 +-
crypto/tlssession.c | 28 ++++--
include/qemu/acl.h | 74 ----------------
monitor.c | 161 ++++++++++++++++++++++-------------
tests/Makefile | 2 +-
tests/test-crypto-tlssession.c | 13 +--
tests/test-io-channel-tls.c | 14 +--
ui/vnc-auth-sasl.c | 2 +-
ui/vnc-auth-sasl.h | 4 +-
ui/vnc.c | 11 ++-
util/Makefile.objs | 1 -
util/acl.c | 188 -----------------------------------------
12 files changed, 156 insertions(+), 348 deletions(-)
delete mode 100644 include/qemu/acl.h
delete mode 100644 util/acl.c
diff --git a/Makefile b/Makefile
index 60ad13e..8f7ffd3 100644
--- a/Makefile
+++ b/Makefile
@@ -235,9 +235,9 @@ util/module.o-cflags = -D'CONFIG_BLOCK_MODULES=$(block-modules)'
qemu-img.o: qemu-img-cmds.h
-qemu-img$(EXESUF): qemu-img.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) libqemuutil.a libqemustub.a
-qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) libqemuutil.a libqemustub.a
-qemu-io$(EXESUF): qemu-io.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) libqemuutil.a libqemustub.a
+qemu-img$(EXESUF): qemu-img.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) $(util-qom-obj-y) libqemuutil.a libqemustub.a
+qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) $(util-qom-obj-y) libqemuutil.a libqemustub.a
+qemu-io$(EXESUF): qemu-io.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) $(util-qom-obj-y) libqemuutil.a libqemustub.a
qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
diff --git a/crypto/tlssession.c b/crypto/tlssession.c
index e0d9658..26e8097 100644
--- a/crypto/tlssession.c
+++ b/crypto/tlssession.c
@@ -22,7 +22,7 @@
#include "crypto/tlssession.h"
#include "crypto/tlscredsanon.h"
#include "crypto/tlscredsx509.h"
-#include "qemu/acl.h"
+#include "qemu/authz.h"
#include "trace.h"
#ifdef CONFIG_GNUTLS
@@ -207,6 +207,7 @@ qcrypto_tls_session_check_certificate(QCryptoTLSSession *session,
unsigned int nCerts, i;
time_t now;
gnutls_x509_crt_t cert = NULL;
+ Error *err = NULL;
now = time(NULL);
if (now == ((time_t)-1)) {
@@ -295,16 +296,33 @@ qcrypto_tls_session_check_certificate(QCryptoTLSSession *session,
goto error;
}
if (session->aclname) {
- qemu_acl *acl = qemu_acl_find(session->aclname);
- int allow;
- if (!acl) {
+ QAuthZ *acl;
+ Object *obj;
+ Object *container;
+ bool allow;
+
+ container = object_get_objects_root();
+ obj = object_resolve_path_component(container,
+ session->aclname);
+ if (!obj) {
error_setg(errp, "Cannot find ACL %s",
session->aclname);
goto error;
}
- allow = qemu_acl_party_is_allowed(acl, session->peername);
+ if (!object_dynamic_cast(obj, TYPE_QAUTHZ)) {
+ error_setg(errp, "Object '%s' is not a QAuthZ subclass",
+ session->aclname);
+ goto error;
+ }
+ acl = QAUTHZ(obj);
+
+ allow = qauthz_is_allowed(acl, session->peername, &err);
+ if (err) {
+ error_propagate(errp, err);
+ goto error;
+ }
if (!allow) {
error_setg(errp, "TLS x509 ACL check for %s is denied",
session->peername);
diff --git a/include/qemu/acl.h b/include/qemu/acl.h
deleted file mode 100644
index 116487e..0000000
--- a/include/qemu/acl.h
+++ /dev/null
@@ -1,74 +0,0 @@
-/*
- * QEMU access control list management
- *
- * Copyright (C) 2009 Red Hat, Inc
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-
-#ifndef __QEMU_ACL_H__
-#define __QEMU_ACL_H__
-
-#include "qemu/queue.h"
-
-typedef struct qemu_acl_entry qemu_acl_entry;
-typedef struct qemu_acl qemu_acl;
-
-struct qemu_acl_entry {
- char *match;
- int deny;
-
- QTAILQ_ENTRY(qemu_acl_entry) next;
-};
-
-struct qemu_acl {
- char *aclname;
- unsigned int nentries;
- QTAILQ_HEAD(,qemu_acl_entry) entries;
- int defaultDeny;
-};
-
-qemu_acl *qemu_acl_init(const char *aclname);
-
-qemu_acl *qemu_acl_find(const char *aclname);
-
-int qemu_acl_party_is_allowed(qemu_acl *acl,
- const char *party);
-
-void qemu_acl_reset(qemu_acl *acl);
-
-int qemu_acl_append(qemu_acl *acl,
- int deny,
- const char *match);
-int qemu_acl_insert(qemu_acl *acl,
- int deny,
- const char *match,
- int index);
-int qemu_acl_remove(qemu_acl *acl,
- const char *match);
-
-#endif /* __QEMU_ACL_H__ */
-
-/*
- * Local variables:
- * c-indent-level: 4
- * c-basic-offset: 4
- * tab-width: 8
- * End:
- */
diff --git a/monitor.c b/monitor.c
index e99ca8c..74b62a9 100644
--- a/monitor.c
+++ b/monitor.c
@@ -48,7 +48,7 @@
#include "qemu/timer.h"
#include "migration/migration.h"
#include "sysemu/kvm.h"
-#include "qemu/acl.h"
+#include "qemu/authz-simple.h"
#include "sysemu/tpm.h"
#include "qapi/qmp/qerror.h"
#include "qapi/qmp/qint.h"
@@ -59,6 +59,7 @@
#include "qapi/qmp/qjson.h"
#include "qapi/qmp/json-streamer.h"
#include "qapi/qmp/json-parser.h"
+#include "qapi/util.h"
#include <qom/object_interfaces.h>
#include "cpu.h"
#include "trace.h"
@@ -1574,61 +1575,88 @@ static void hmp_wavcapture(Monitor *mon, const QDict *qdict)
QLIST_INSERT_HEAD (&capture_head, s, entries);
}
-static qemu_acl *find_acl(Monitor *mon, const char *name)
+static QAuthZSimple *find_auth(Monitor *mon, const char *name)
{
- qemu_acl *acl = qemu_acl_find(name);
+ Object *obj;
+ Object *container;
- if (!acl) {
+ container = object_get_objects_root();
+ obj = object_resolve_path_component(container, name);
+ if (!obj) {
monitor_printf(mon, "acl: unknown list '%s'\n", name);
+ return NULL;
}
- return acl;
+
+ return QAUTHZ_SIMPLE(obj);
}
static void hmp_acl_show(Monitor *mon, const QDict *qdict)
{
const char *aclname = qdict_get_str(qdict, "aclname");
- qemu_acl *acl = find_acl(mon, aclname);
- qemu_acl_entry *entry;
- int i = 0;
-
- if (acl) {
- monitor_printf(mon, "policy: %s\n",
- acl->defaultDeny ? "deny" : "allow");
- QTAILQ_FOREACH(entry, &acl->entries, next) {
- i++;
- monitor_printf(mon, "%d: %s %s\n", i,
- entry->deny ? "deny" : "allow", entry->match);
- }
+ QAuthZSimple *auth = find_auth(mon, aclname);
+ QAuthZSimpleRuleList *rules;
+ size_t i = 0;
+
+ if (!auth) {
+ return;
+ }
+
+ monitor_printf(mon, "policy: %s\n",
+ QAuthZSimplePolicy_lookup[auth->policy]);
+
+ rules = auth->rules;
+ while (rules) {
+ QAuthZSimpleRule *rule = rules->value;
+ i++;
+ monitor_printf(mon, "%zu: %s %s\n", i,
+ QAuthZSimplePolicy_lookup[rule->policy],
+ rule->match);
+ rules = rules->next;
}
}
static void hmp_acl_reset(Monitor *mon, const QDict *qdict)
{
const char *aclname = qdict_get_str(qdict, "aclname");
- qemu_acl *acl = find_acl(mon, aclname);
+ QAuthZSimple *auth = find_auth(mon, aclname);
- if (acl) {
- qemu_acl_reset(acl);
- monitor_printf(mon, "acl: removed all rules\n");
+ if (!auth) {
+ return;
}
+
+ auth->policy = QAUTHZ_SIMPLE_POLICY_DENY;
+ qapi_free_QAuthZSimpleRuleList(auth->rules);
+ auth->rules = NULL;
+ monitor_printf(mon, "acl: removed all rules\n");
}
static void hmp_acl_policy(Monitor *mon, const QDict *qdict)
{
const char *aclname = qdict_get_str(qdict, "aclname");
const char *policy = qdict_get_str(qdict, "policy");
- qemu_acl *acl = find_acl(mon, aclname);
+ QAuthZSimple *auth = find_auth(mon, aclname);
+ int val;
+ Error *err = NULL;
+
+ if (!auth) {
+ return;
+ }
- if (acl) {
- if (strcmp(policy, "allow") == 0) {
- acl->defaultDeny = 0;
+ val = qapi_enum_parse(QAuthZSimplePolicy_lookup,
+ policy,
+ QAUTHZ_SIMPLE_POLICY__MAX,
+ QAUTHZ_SIMPLE_POLICY_DENY,
+ &err);
+ if (err) {
+ error_free(err);
+ monitor_printf(mon, "acl: unknown policy '%s', "
+ "expected 'deny' or 'allow'\n", policy);
+ } else {
+ auth->policy = val;
+ if (auth->policy == QAUTHZ_SIMPLE_POLICY_ALLOW) {
monitor_printf(mon, "acl: policy set to 'allow'\n");
- } else if (strcmp(policy, "deny") == 0) {
- acl->defaultDeny = 1;
- monitor_printf(mon, "acl: policy set to 'deny'\n");
} else {
- monitor_printf(mon, "acl: unknown policy '%s', "
- "expected 'deny' or 'allow'\n", policy);
+ monitor_printf(mon, "acl: policy set to 'deny'\n");
}
}
}
@@ -1637,46 +1665,59 @@ static void hmp_acl_add(Monitor *mon, const QDict *qdict)
{
const char *aclname = qdict_get_str(qdict, "aclname");
const char *match = qdict_get_str(qdict, "match");
- const char *policy = qdict_get_str(qdict, "policy");
+ const char *policystr = qdict_get_str(qdict, "policy");
int has_index = qdict_haskey(qdict, "index");
int index = qdict_get_try_int(qdict, "index", -1);
- qemu_acl *acl = find_acl(mon, aclname);
- int deny, ret;
-
- if (acl) {
- if (strcmp(policy, "allow") == 0) {
- deny = 0;
- } else if (strcmp(policy, "deny") == 0) {
- deny = 1;
- } else {
- monitor_printf(mon, "acl: unknown policy '%s', "
- "expected 'deny' or 'allow'\n", policy);
- return;
- }
- if (has_index)
- ret = qemu_acl_insert(acl, deny, match, index);
- else
- ret = qemu_acl_append(acl, deny, match);
- if (ret < 0)
- monitor_printf(mon, "acl: unable to add acl entry\n");
- else
- monitor_printf(mon, "acl: added rule at position %d\n", ret);
+ QAuthZSimple *auth = find_auth(mon, aclname);
+ Error *err = NULL;
+ int policy;
+ size_t i = 0;
+
+ if (!auth) {
+ return;
+ }
+
+ policy = qapi_enum_parse(QAuthZSimplePolicy_lookup,
+ policystr,
+ QAUTHZ_SIMPLE_POLICY__MAX,
+ QAUTHZ_SIMPLE_POLICY_DENY,
+ &err);
+ if (err) {
+ error_free(err);
+ monitor_printf(mon, "acl: unknown policy '%s', "
+ "expected 'deny' or 'allow'\n", policystr);
+ return;
+ }
+
+ if (has_index && index == 0) {
+ monitor_printf(mon, "acl: unable to add acl entry\n");
+ return;
+ }
+
+ if (has_index) {
+ i = qauthz_simple_insert_rule(auth, match, policy, index - 1);
+ } else {
+ i = qauthz_simple_append_rule(auth, match, policy);
}
+ monitor_printf(mon, "acl: added rule at position %zu\n", i + 1);
}
static void hmp_acl_remove(Monitor *mon, const QDict *qdict)
{
const char *aclname = qdict_get_str(qdict, "aclname");
const char *match = qdict_get_str(qdict, "match");
- qemu_acl *acl = find_acl(mon, aclname);
- int ret;
+ QAuthZSimple *auth = find_auth(mon, aclname);
+ ssize_t i = 0;
- if (acl) {
- ret = qemu_acl_remove(acl, match);
- if (ret < 0)
- monitor_printf(mon, "acl: no matching acl entry\n");
- else
- monitor_printf(mon, "acl: removed rule at position %d\n", ret);
+ if (!auth) {
+ return;
+ }
+
+ i = qauthz_simple_delete_rule(auth, match);
+ if (i >= 0) {
+ monitor_printf(mon, "acl: removed rule at position %zu\n", i + 1);
+ } else {
+ monitor_printf(mon, "acl: no matching acl entry\n");
}
}
diff --git a/tests/Makefile b/tests/Makefile
index 7a1d959..647f30f 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -391,7 +391,7 @@ test-qom-obj-y = $(qom-obj-y) $(test-util-obj-y)
test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o \
tests/test-qapi-event.o tests/test-qmp-introspect.o \
$(test-qom-obj-y)
-test-crypto-obj-y = $(crypto-obj-y) $(test-qom-obj-y)
+test-crypto-obj-y = $(crypto-obj-y) $(util-qom-obj-y) $(test-qom-obj-y)
test-io-obj-y = $(io-obj-y) $(test-crypto-obj-y)
test-block-obj-y = $(block-obj-y) $(test-io-obj-y)
diff --git a/tests/test-crypto-tlssession.c b/tests/test-crypto-tlssession.c
index 036a86b..e76b249 100644
--- a/tests/test-crypto-tlssession.c
+++ b/tests/test-crypto-tlssession.c
@@ -25,7 +25,7 @@
#include "crypto/tlssession.h"
#include "qom/object_interfaces.h"
#include "qemu/sockets.h"
-#include "qemu/acl.h"
+#include "qemu/authz-simple.h"
#ifdef QCRYPTO_HAVE_TLS_TEST_SUPPORT
@@ -110,7 +110,7 @@ static void test_crypto_tls_session(const void *opaque)
QCryptoTLSCreds *serverCreds;
QCryptoTLSSession *clientSess = NULL;
QCryptoTLSSession *serverSess = NULL;
- qemu_acl *acl;
+ QAuthZSimple *auth;
const char * const *wildcards;
int channel[2];
bool clientShake = false;
@@ -169,11 +169,13 @@ static void test_crypto_tls_session(const void *opaque)
&err);
g_assert(serverCreds != NULL);
- acl = qemu_acl_init("tlssessionacl");
- qemu_acl_reset(acl);
+ auth = qauthz_simple_new("tlssessionacl",
+ QAUTHZ_SIMPLE_POLICY_DENY,
+ &error_abort);
wildcards = data->wildcards;
while (wildcards && *wildcards) {
- qemu_acl_append(acl, 0, *wildcards);
+ qauthz_simple_append_rule(auth, *wildcards,
+ QAUTHZ_SIMPLE_POLICY_ALLOW);
wildcards++;
}
@@ -263,6 +265,7 @@ static void test_crypto_tls_session(const void *opaque)
object_unparent(OBJECT(serverCreds));
object_unparent(OBJECT(clientCreds));
+ object_unparent(OBJECT(auth));
qcrypto_tls_session_free(serverSess);
qcrypto_tls_session_free(clientSess);
diff --git a/tests/test-io-channel-tls.c b/tests/test-io-channel-tls.c
index 3c361a7..390d3e9 100644
--- a/tests/test-io-channel-tls.c
+++ b/tests/test-io-channel-tls.c
@@ -28,7 +28,7 @@
#include "io/channel-socket.h"
#include "io-channel-helpers.h"
#include "crypto/tlscredsx509.h"
-#include "qemu/acl.h"
+#include "qemu/authz-simple.h"
#include "qom/object_interfaces.h"
#ifdef QCRYPTO_HAVE_TLS_TEST_SUPPORT
@@ -115,7 +115,7 @@ static void test_io_channel_tls(const void *opaque)
QIOChannelTLS *serverChanTLS;
QIOChannelSocket *clientChanSock;
QIOChannelSocket *serverChanSock;
- qemu_acl *acl;
+ QAuthZSimple *auth;
const char * const *wildcards;
int channel[2];
struct QIOChannelTLSHandshakeData clientHandshake = { false, false };
@@ -166,11 +166,13 @@ static void test_io_channel_tls(const void *opaque)
&err);
g_assert(serverCreds != NULL);
- acl = qemu_acl_init("channeltlsacl");
- qemu_acl_reset(acl);
+ auth = qauthz_simple_new("channeltlsacl",
+ QAUTHZ_SIMPLE_POLICY_DENY,
+ &error_abort);
wildcards = data->wildcards;
while (wildcards && *wildcards) {
- qemu_acl_append(acl, 0, *wildcards);
+ qauthz_simple_append_rule(auth, *wildcards,
+ QAUTHZ_SIMPLE_POLICY_ALLOW);
wildcards++;
}
@@ -256,6 +258,8 @@ static void test_io_channel_tls(const void *opaque)
object_unref(OBJECT(serverChanSock));
object_unref(OBJECT(clientChanSock));
+ object_unparent(OBJECT(auth));
+
close(channel[0]);
close(channel[1]);
}
diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c
index 13a59f5..53f9a7c 100644
--- a/ui/vnc-auth-sasl.c
+++ b/ui/vnc-auth-sasl.c
@@ -149,7 +149,7 @@ static int vnc_auth_sasl_check_access(VncState *vs)
return 0;
}
- allow = qemu_acl_party_is_allowed(vs->vd->sasl.acl, vs->sasl.username);
+ allow = qauthz_is_allowed(vs->vd->sasl.acl, vs->sasl.username, NULL);
VNC_DEBUG("SASL client %s %s by ACL\n", vs->sasl.username,
allow ? "allowed" : "denied");
diff --git a/ui/vnc-auth-sasl.h b/ui/vnc-auth-sasl.h
index 3f59da6..a6ffb7e 100644
--- a/ui/vnc-auth-sasl.h
+++ b/ui/vnc-auth-sasl.h
@@ -32,7 +32,7 @@
typedef struct VncStateSASL VncStateSASL;
typedef struct VncDisplaySASL VncDisplaySASL;
-#include "qemu/acl.h"
+#include "qemu/authz.h"
#include "qemu/main-loop.h"
struct VncStateSASL {
@@ -61,7 +61,7 @@ struct VncStateSASL {
};
struct VncDisplaySASL {
- qemu_acl *acl;
+ QAuthZ *acl;
};
void vnc_sasl_client_cleanup(VncState *vs);
diff --git a/ui/vnc.c b/ui/vnc.c
index 729f630..324512d 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -33,7 +33,7 @@
#include "qemu/error-report.h"
#include "qemu/sockets.h"
#include "qemu/timer.h"
-#include "qemu/acl.h"
+#include "qemu/authz-simple.h"
#include "qemu/config-file.h"
#include "qapi/qmp/qerror.h"
#include "qapi/qmp/types.h"
@@ -3705,7 +3705,9 @@ void vnc_display_open(const char *id, Error **errp)
} else {
vs->tlsaclname = g_strdup_printf("vnc.%s.x509dname", vs->id);
}
- qemu_acl_init(vs->tlsaclname);
+ qauthz_simple_new(vs->tlsaclname,
+ QAUTHZ_SIMPLE_POLICY_DENY,
+ &error_abort);
}
#ifdef CONFIG_VNC_SASL
if (acl && sasl) {
@@ -3716,7 +3718,10 @@ void vnc_display_open(const char *id, Error **errp)
} else {
aclname = g_strdup_printf("vnc.%s.username", vs->id);
}
- vs->sasl.acl = qemu_acl_init(aclname);
+ vs->sasl.acl =
+ QAUTHZ(qauthz_simple_new(aclname,
+ QAUTHZ_SIMPLE_POLICY_DENY,
+ &error_abort));
g_free(aclname);
}
#endif
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 4870905..44078c1 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -13,7 +13,6 @@ util-obj-y += envlist.o path.o module.o
util-obj-$(call lnot,$(CONFIG_INT128)) += host-utils.o
util-obj-y += bitmap.o bitops.o hbitmap.o
util-obj-y += fifo8.o
-util-obj-y += acl.o
util-obj-y += error.o qemu-error.o
util-obj-y += id.o
util-obj-y += iov.o qemu-config.o qemu-sockets.o uri.o notify.o
diff --git a/util/acl.c b/util/acl.c
deleted file mode 100644
index 723b6a8..0000000
--- a/util/acl.c
+++ /dev/null
@@ -1,188 +0,0 @@
-/*
- * QEMU access control list management
- *
- * Copyright (C) 2009 Red Hat, Inc
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-
-
-#include "qemu/osdep.h"
-#include "qemu-common.h"
-#include "qemu/acl.h"
-
-#ifdef CONFIG_FNMATCH
-#include <fnmatch.h>
-#endif
-
-
-static unsigned int nacls = 0;
-static qemu_acl **acls = NULL;
-
-
-
-qemu_acl *qemu_acl_find(const char *aclname)
-{
- int i;
- for (i = 0 ; i < nacls ; i++) {
- if (strcmp(acls[i]->aclname, aclname) == 0)
- return acls[i];
- }
-
- return NULL;
-}
-
-qemu_acl *qemu_acl_init(const char *aclname)
-{
- qemu_acl *acl;
-
- acl = qemu_acl_find(aclname);
- if (acl)
- return acl;
-
- acl = g_malloc(sizeof(*acl));
- acl->aclname = g_strdup(aclname);
- /* Deny by default, so there is no window of "open
- * access" between QEMU starting, and the user setting
- * up ACLs in the monitor */
- acl->defaultDeny = 1;
-
- acl->nentries = 0;
- QTAILQ_INIT(&acl->entries);
-
- acls = g_realloc(acls, sizeof(*acls) * (nacls +1));
- acls[nacls] = acl;
- nacls++;
-
- return acl;
-}
-
-int qemu_acl_party_is_allowed(qemu_acl *acl,
- const char *party)
-{
- qemu_acl_entry *entry;
-
- QTAILQ_FOREACH(entry, &acl->entries, next) {
-#ifdef CONFIG_FNMATCH
- if (fnmatch(entry->match, party, 0) == 0)
- return entry->deny ? 0 : 1;
-#else
- /* No fnmatch, so fallback to exact string matching
- * instead of allowing wildcards */
- if (strcmp(entry->match, party) == 0)
- return entry->deny ? 0 : 1;
-#endif
- }
-
- return acl->defaultDeny ? 0 : 1;
-}
-
-
-void qemu_acl_reset(qemu_acl *acl)
-{
- qemu_acl_entry *entry, *next_entry;
-
- /* Put back to deny by default, so there is no window
- * of "open access" while the user re-initializes the
- * access control list */
- acl->defaultDeny = 1;
- QTAILQ_FOREACH_SAFE(entry, &acl->entries, next, next_entry) {
- QTAILQ_REMOVE(&acl->entries, entry, next);
- g_free(entry->match);
- g_free(entry);
- }
- acl->nentries = 0;
-}
-
-
-int qemu_acl_append(qemu_acl *acl,
- int deny,
- const char *match)
-{
- qemu_acl_entry *entry;
-
- entry = g_malloc(sizeof(*entry));
- entry->match = g_strdup(match);
- entry->deny = deny;
-
- QTAILQ_INSERT_TAIL(&acl->entries, entry, next);
- acl->nentries++;
-
- return acl->nentries;
-}
-
-
-int qemu_acl_insert(qemu_acl *acl,
- int deny,
- const char *match,
- int index)
-{
- qemu_acl_entry *tmp;
- int i = 0;
-
- if (index <= 0)
- return -1;
- if (index > acl->nentries) {
- return qemu_acl_append(acl, deny, match);
- }
-
- QTAILQ_FOREACH(tmp, &acl->entries, next) {
- i++;
- if (i == index) {
- qemu_acl_entry *entry;
- entry = g_malloc(sizeof(*entry));
- entry->match = g_strdup(match);
- entry->deny = deny;
-
- QTAILQ_INSERT_BEFORE(tmp, entry, next);
- acl->nentries++;
- break;
- }
- }
-
- return i;
-}
-
-int qemu_acl_remove(qemu_acl *acl,
- const char *match)
-{
- qemu_acl_entry *entry;
- int i = 0;
-
- QTAILQ_FOREACH(entry, &acl->entries, next) {
- i++;
- if (strcmp(entry->match, match) == 0) {
- QTAILQ_REMOVE(&acl->entries, entry, next);
- acl->nentries--;
- g_free(entry->match);
- g_free(entry);
- return i;
- }
- }
- return -1;
-}
-
-
-/*
- * Local variables:
- * c-indent-level: 4
- * c-basic-offset: 4
- * tab-width: 8
- * End:
- */
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v3 07/10] qemu-nbd: add support for ACLs for TLS clients
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 01/10] qdict: implement a qdict_crumple method for un-flattening a dict Daniel P. Berrange
` (4 preceding siblings ...)
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 06/10] acl: delete existing ACL implementation Daniel P. Berrange
@ 2016-03-10 18:59 ` Daniel P. Berrange
2016-03-22 18:14 ` Eric Blake
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 08/10] nbd: allow an ACL to be set with nbd-server-start QMP command Daniel P. Berrange
` (3 subsequent siblings)
9 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrange @ 2016-03-10 18:59 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Markus Armbruster, Max Reitz, Paolo Bonzini,
Andreas Färber
Currently any client which can complete the TLS handshake
is able to use the NBD server. The server admin can turn
on the 'verify-peer' option for the x509 creds to require
the client to provide a x509 certificate. This means the
client will have to acquire a certificate from the CA before
they are permitted to use the NBD server. This is still a
fairly weak bar.
This adds a '--tls-acl ACL-ID' option to the qemu-nbd command
which takes the ID of a previously added 'QAuthZ' object
instance. This ACL will be used to validate the client's
x509 distinguished name. Clients failing the ACL will not be
permitted to use the NBD server.
For example to setup an ACL that only allows connection from
a client whose x509 certificate distinguished name contains
'CN=fred', you would use:
qemu-nbd -object tls-creds-x509,id=tls0,dir=/home/berrange/qemutls,\
endpoint=server,verify-peer=yes \
-object authz-simple,id=acl0,policy=deny,\
rules.0.match=*CN=fred,rules.0.policy=allow \
-tls-creds tls0 \
-tls-acl acl0
....other qemu-nbd args...
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
qemu-nbd.c | 13 ++++++++++++-
qemu-nbd.texi | 4 ++++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index a5c1d95..d70960f 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -42,6 +42,7 @@
#define QEMU_NBD_OPT_OBJECT 260
#define QEMU_NBD_OPT_TLSCREDS 261
#define QEMU_NBD_OPT_IMAGE_OPTS 262
+#define QEMU_NBD_OPT_TLSACL 263
static NBDExport *exp;
static bool newproto;
@@ -55,6 +56,7 @@ static int nb_fds;
static QIOChannelSocket *server_ioc;
static int server_watch = -1;
static QCryptoTLSCreds *tlscreds;
+static const char *tlsacl;
static void usage(const char *name)
{
@@ -344,7 +346,7 @@ static gboolean nbd_accept(QIOChannel *ioc, GIOCondition cond, gpointer opaque)
nb_fds++;
nbd_update_server_watch();
nbd_client_new(newproto ? NULL : exp, cioc,
- tlscreds, NULL, nbd_client_closed);
+ tlscreds, tlsacl, nbd_client_closed);
object_unref(OBJECT(cioc));
return TRUE;
@@ -488,6 +490,7 @@ int main(int argc, char **argv)
{ "export-name", required_argument, NULL, 'x' },
{ "tls-creds", required_argument, NULL, QEMU_NBD_OPT_TLSCREDS },
{ "image-opts", no_argument, NULL, QEMU_NBD_OPT_IMAGE_OPTS },
+ { "tls-acl", no_argument, NULL, QEMU_NBD_OPT_TLSACL },
{ NULL, 0, NULL, 0 }
};
int ch;
@@ -689,6 +692,9 @@ int main(int argc, char **argv)
case QEMU_NBD_OPT_IMAGE_OPTS:
imageOpts = true;
break;
+ case QEMU_NBD_OPT_TLSACL:
+ tlsacl = optarg;
+ break;
}
}
@@ -725,6 +731,11 @@ int main(int argc, char **argv)
error_get_pretty(local_err));
exit(EXIT_FAILURE);
}
+ } else {
+ if (tlsacl) {
+ error_report("--tls-acl is not permitted without --tls-creds");
+ exit(EXIT_FAILURE);
+ }
}
if (disconnect) {
diff --git a/qemu-nbd.texi b/qemu-nbd.texi
index 9f23343..69f32cb 100644
--- a/qemu-nbd.texi
+++ b/qemu-nbd.texi
@@ -86,6 +86,10 @@ the new style NBD protocol negotiation
Enable mandatory TLS encryption for the server by setting the ID
of the TLS credentials object previously created with the --object
option.
+@item --tls-acl=ID
+Specify the ID of a qauthz object previously created with the
+--object option. This will be used to authorize users who
+connect against their x509 distinguish name.
@item -v, --verbose
Display extra debugging information
@item -h, --help
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v3 08/10] nbd: allow an ACL to be set with nbd-server-start QMP command
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 01/10] qdict: implement a qdict_crumple method for un-flattening a dict Daniel P. Berrange
` (5 preceding siblings ...)
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 07/10] qemu-nbd: add support for ACLs for TLS clients Daniel P. Berrange
@ 2016-03-10 18:59 ` Daniel P. Berrange
2016-03-22 18:19 ` Eric Blake
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 09/10] chardev: add support for ACLs for TLS clients Daniel P. Berrange
` (2 subsequent siblings)
9 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrange @ 2016-03-10 18:59 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Markus Armbruster, Max Reitz, Paolo Bonzini,
Andreas Färber
As with the previous patch to qemu-nbd, the nbd-server-start
QMP command also needs to be able to specify an ACL when
enabling TLS encryption.
First the client must create a QAuthZ object instance using
the 'object-add' command:
{
'execute': 'object-add',
'arguments': {
'qom-type': 'authz-simple',
'id': 'tls0',
'parameters': {
'policy': 'deny',
'rules': [
{
'match': '*CN=fred',
'policy': 'allow'
}
]
}
}
}
They can then reference this in the new 'tls-acl' parameter
when executing the 'nbd-server-start' command.
{
'execute': 'nbd-server-start',
'arguments': {
'addr': {
'type': 'inet',
'host': '127.0.0.1',
'port': '9000'
},
'tls-creds': 'tls0',
'tls-acl': 'tlsacl0'
}
}
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
blockdev-nbd.c | 10 +++++++++-
hmp.c | 2 +-
qapi/block.json | 4 +++-
qmp-commands.hx | 2 +-
4 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 12cae0e..ae5335e 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -24,6 +24,7 @@ typedef struct NBDServerData {
QIOChannelSocket *listen_ioc;
int watch;
QCryptoTLSCreds *tlscreds;
+ char *tlsacl;
} NBDServerData;
static NBDServerData *nbd_server;
@@ -45,7 +46,8 @@ static gboolean nbd_accept(QIOChannel *ioc, GIOCondition condition,
}
nbd_client_new(NULL, cioc,
- nbd_server->tlscreds, NULL,
+ nbd_server->tlscreds,
+ nbd_server->tlsacl,
nbd_client_put);
object_unref(OBJECT(cioc));
return TRUE;
@@ -65,6 +67,7 @@ static void nbd_server_free(NBDServerData *server)
if (server->tlscreds) {
object_unref(OBJECT(server->tlscreds));
}
+ g_free(server->tlsacl);
g_free(server);
}
@@ -101,6 +104,7 @@ static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, Error **errp)
void qmp_nbd_server_start(SocketAddress *addr,
bool has_tls_creds, const char *tls_creds,
+ bool has_tls_acl, const char *tls_acl,
Error **errp)
{
if (nbd_server) {
@@ -128,6 +132,10 @@ void qmp_nbd_server_start(SocketAddress *addr,
}
}
+ if (has_tls_acl) {
+ nbd_server->tlsacl = g_strdup(tls_acl);
+ }
+
nbd_server->watch = qio_channel_add_watch(
QIO_CHANNEL(nbd_server->listen_ioc),
G_IO_IN,
diff --git a/hmp.c b/hmp.c
index 7a98726..20703fd 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1802,7 +1802,7 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
goto exit;
}
- qmp_nbd_server_start(addr, false, NULL, &local_err);
+ qmp_nbd_server_start(addr, false, NULL, false, NULL, &local_err);
qapi_free_SocketAddress(addr);
if (local_err != NULL) {
goto exit;
diff --git a/qapi/block.json b/qapi/block.json
index 58e6b30..6b209e1 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -147,6 +147,7 @@
#
# @addr: Address on which to listen.
# @tls-creds: (optional) ID of the TLS credentials object. Since 2.6
+# @tls-acl: (optional) ID of the QAuthZ authorization object. Since 2.6
#
# Returns: error if the server is already running.
#
@@ -154,7 +155,8 @@
##
{ 'command': 'nbd-server-start',
'data': { 'addr': 'SocketAddress',
- '*tls-creds': 'str'} }
+ '*tls-creds': 'str',
+ '*tls-acl': 'str'} }
##
# @nbd-server-add:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index b629673..7a3fa26 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3859,7 +3859,7 @@ EQMP
{
.name = "nbd-server-start",
- .args_type = "addr:q,tls-creds:s?",
+ .args_type = "addr:q,tls-creds:s?,tls-acl:s?",
.mhandler.cmd_new = qmp_marshal_nbd_server_start,
},
{
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v3 09/10] chardev: add support for ACLs for TLS clients
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 01/10] qdict: implement a qdict_crumple method for un-flattening a dict Daniel P. Berrange
` (6 preceding siblings ...)
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 08/10] nbd: allow an ACL to be set with nbd-server-start QMP command Daniel P. Berrange
@ 2016-03-10 18:59 ` Daniel P. Berrange
2016-03-22 21:26 ` Eric Blake
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 10/10] vnc: allow specifying a custom ACL object name Daniel P. Berrange
2016-03-21 22:45 ` [Qemu-devel] [PATCH v3 01/10] qdict: implement a qdict_crumple method for un-flattening a dict Eric Blake
9 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrange @ 2016-03-10 18:59 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Markus Armbruster, Max Reitz, Paolo Bonzini,
Andreas Färber
Currently any client which can complete the TLS handshake
is able to use a chardev server. The server admin can turn
on the 'verify-peer' option for the x509 creds to require
the client to provide a x509 certificate. This means the
client will have to acquire a certificate from the CA before
they are permitted to use the chardev server. This is still
a fairly weak bar.
This adds a 'tls-acl=ACL-ID' option to the socket chardev
backend which takes the ID of a previously added 'QAuthZ'
object instance. This ACL will be used to validate the client's
x509 distinguished name. Clients failing the ACL will not be
permitted to use the chardev server.
For example to setup an ACL that only allows connection from
a client whose x509 certificate distinguished name contains
'CN=fred', you would use:
$QEMU -object tls-creds-x509,id=tls0,dir=/home/berrange/qemutls,\
endpoint=server,verify-peer=yes \
-object authz-simple,id=acl0,policy=deny,\
rules.0.match=*CN=fred,rules.0.policy=allow \
-chardev socket,host=127.0.0.1,port=9000,server,\
tls-creds=tls0,tls-acl=acl0 \
...other qemud args...
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
qapi-schema.json | 2 ++
qemu-char.c | 11 ++++++++++-
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index b6769de..a6a7205 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3202,6 +3202,7 @@
# @addr: socket address to listen on (server=true)
# or connect to (server=false)
# @tls-creds: #optional the ID of the TLS credentials object (since 2.6)
+# @tls-acl: #optional the ID of the QAuthZ authorization object (since 2.6)
# @server: #optional create server socket (default: true)
# @wait: #optional wait for incoming connection on server
# sockets (default: false).
@@ -3217,6 +3218,7 @@
##
{ 'struct': 'ChardevSocket', 'data': { 'addr' : 'SocketAddress',
'*tls-creds' : 'str',
+ '*tls-acl' : 'str',
'*server' : 'bool',
'*wait' : 'bool',
'*nodelay' : 'bool',
diff --git a/qemu-char.c b/qemu-char.c
index e0147f3..9533e7e 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2533,6 +2533,7 @@ typedef struct {
QIOChannelSocket *listen_ioc;
guint listen_tag;
QCryptoTLSCreds *tls_creds;
+ char *tls_acl;
int connected;
int max_size;
int do_telnetopt;
@@ -2963,7 +2964,7 @@ static void tcp_chr_tls_init(CharDriverState *chr)
if (s->is_listen) {
tioc = qio_channel_tls_new_server(
s->ioc, s->tls_creds,
- NULL, /* XXX Use an ACL */
+ s->tls_acl,
&err);
} else {
tioc = qio_channel_tls_new_client(
@@ -3084,6 +3085,7 @@ static void tcp_chr_close(CharDriverState *chr)
if (s->tls_creds) {
object_unref(OBJECT(s->tls_creds));
}
+ g_free(s->tls_acl);
if (s->write_msgfds_num) {
g_free(s->write_msgfds);
}
@@ -3623,6 +3625,7 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
const char *host = qemu_opt_get(opts, "host");
const char *port = qemu_opt_get(opts, "port");
const char *tls_creds = qemu_opt_get(opts, "tls-creds");
+ const char *tls_acl = qemu_opt_get(opts, "tls-acl");
SocketAddress *addr;
ChardevSocket *sock;
@@ -3656,6 +3659,7 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
sock->has_reconnect = true;
sock->reconnect = reconnect;
sock->tls_creds = g_strdup(tls_creds);
+ sock->tls_acl = g_strdup(tls_acl);
addr = g_new0(SocketAddress, 1);
if (path) {
@@ -4094,6 +4098,9 @@ QemuOptsList qemu_chardev_opts = {
.name = "tls-creds",
.type = QEMU_OPT_STRING,
},{
+ .name = "tls-acl",
+ .type = QEMU_OPT_STRING,
+ },{
.name = "width",
.type = QEMU_OPT_NUMBER,
},{
@@ -4341,6 +4348,7 @@ static CharDriverState *qmp_chardev_open_socket(const char *id,
}
}
}
+ s->tls_acl = g_strdup(sock->tls_acl);
qapi_copy_SocketAddress(&s->addr, sock->addr);
@@ -4386,6 +4394,7 @@ static CharDriverState *qmp_chardev_open_socket(const char *id,
if (s->tls_creds) {
object_unref(OBJECT(s->tls_creds));
}
+ g_free(s->tls_acl);
g_free(s);
qemu_chr_free_common(chr);
return NULL;
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH v3 10/10] vnc: allow specifying a custom ACL object name
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 01/10] qdict: implement a qdict_crumple method for un-flattening a dict Daniel P. Berrange
` (7 preceding siblings ...)
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 09/10] chardev: add support for ACLs for TLS clients Daniel P. Berrange
@ 2016-03-10 18:59 ` Daniel P. Berrange
2016-03-22 21:38 ` Eric Blake
2016-03-21 22:45 ` [Qemu-devel] [PATCH v3 01/10] qdict: implement a qdict_crumple method for un-flattening a dict Eric Blake
9 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrange @ 2016-03-10 18:59 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Markus Armbruster, Max Reitz, Paolo Bonzini,
Andreas Färber
The VNC server has historically had support for ACLs to check
both the SASL username and the TLS x509 distinguished name.
The VNC server was responsible for creating the initial ACL,
and the client app was then responsible for populating it with
rules using the HMP 'acl_add' command.
This is not satisfactory for a variety of reasons. There is
no way to populate the ACLs from the command line, users are
forced to use the HMP. With multiple network services all
supporting TLS and ACLs now, it is desirable to be able to
define a single ACL that is referenced by all services.
To address these limitations, two new options are added to the
VNC server CLI. The 'tls-acl' option takes the ID of a QAuthZ
object to use for checking TLS x509 distinguished names, and
the 'sasl-acl' option takes the ID of another object to use for
checking SASL usernames.
In this example, we setup two ACLs. The first allows any client
with a certificate issued by the 'RedHat' organization in the
'London' locality. The second ACL allows clients with either
the 'joe@REDHAT.COM' or 'fred@REDHAT.COM' kerberos usernames.
Both ACLs must pass for the user to be allowed.
$QEMU -object tls-creds-x509,id=tls0,dir=/home/berrange/qemutls,\
endpoint=server,verify-peer=yes \
-object authz-simple,id=acl0,policy=deny,\
rules.0.match=O=RedHat,,L=London,rules.0.policy=allow \
-object authz-simple,id=acl0,policy=deny,\
rules.0.match=fred@REDHAT.COM,rules.0.policy=allow \
rules.0.match=joe@REDHAT.COM,rules.0.policy=allow \
-vnc 0.0.0.0:1,tls-creds=tls0,tls-acl=tlsacl0,
sasl,sasl-acl=saslacl0 \
...other QEMU args...
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
ui/vnc.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++------------
1 file changed, 60 insertions(+), 13 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index 324512d..7090f0b 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3261,6 +3261,12 @@ static QemuOptsList qemu_vnc_opts = {
.name = "acl",
.type = QEMU_OPT_BOOL,
},{
+ .name = "tls-acl",
+ .type = QEMU_OPT_STRING,
+ },{
+ .name = "sasl-acl",
+ .type = QEMU_OPT_STRING,
+ },{
.name = "lossy",
.type = QEMU_OPT_BOOL,
},{
@@ -3483,6 +3489,10 @@ void vnc_display_open(const char *id, Error **errp)
int saslErr;
#endif
int acl = 0;
+ const char *tlsacl;
+#ifdef CONFIG_VNC_SASL
+ const char *saslacl;
+#endif
int lock_key_sync = 1;
if (!vs) {
@@ -3670,6 +3680,21 @@ void vnc_display_open(const char *id, Error **errp)
}
}
acl = qemu_opt_get_bool(opts, "acl", false);
+ tlsacl = qemu_opt_get(opts, "tls-acl");
+ if (acl && tlsacl) {
+ error_setg(errp, "'acl' option is mutually exclusive with the "
+ "'tls-acl' options");
+ goto fail;
+ }
+
+#ifdef CONFIG_VNC_SASL
+ saslacl = qemu_opt_get(opts, "sasl-acl");
+ if (acl && saslacl) {
+ error_setg(errp, "'acl' option is mutually exclusive with the "
+ "'sasl-acl' options");
+ goto fail;
+ }
+#endif
share = qemu_opt_get(opts, "share");
if (share) {
@@ -3699,7 +3724,9 @@ void vnc_display_open(const char *id, Error **errp)
vs->non_adaptive = true;
}
- if (acl) {
+ if (tlsacl) {
+ vs->tlsaclname = g_strdup(tlsacl);
+ } else if (acl) {
if (strcmp(vs->id, "default") == 0) {
vs->tlsaclname = g_strdup("vnc.x509dname");
} else {
@@ -3710,19 +3737,39 @@ void vnc_display_open(const char *id, Error **errp)
&error_abort);
}
#ifdef CONFIG_VNC_SASL
- if (acl && sasl) {
- char *aclname;
+ if (sasl) {
+ if (saslacl) {
+ Object *container, *acl;
+ container = object_get_objects_root();
+ acl = object_resolve_path_component(container, saslacl);
+ if (!acl) {
+ error_setg(errp, "Cannot find ACL %s", saslacl);
+ goto fail;
+ }
- if (strcmp(vs->id, "default") == 0) {
- aclname = g_strdup("vnc.username");
- } else {
- aclname = g_strdup_printf("vnc.%s.username", vs->id);
- }
- vs->sasl.acl =
- QAUTHZ(qauthz_simple_new(aclname,
- QAUTHZ_SIMPLE_POLICY_DENY,
- &error_abort));
- g_free(aclname);
+ if (!object_dynamic_cast(acl, TYPE_QAUTHZ)) {
+ error_setg(errp, "Object '%s' is not a QAuthZ subclass",
+ saslacl);
+ goto fail;
+ }
+ vs->sasl.acl = QAUTHZ(acl);
+ } else if (acl) {
+ char *aclname;
+
+ if (strcmp(vs->id, "default") == 0) {
+ aclname = g_strdup("vnc.username");
+ } else {
+ aclname = g_strdup_printf("vnc.%s.username", vs->id);
+ }
+ vs->sasl.acl =
+ QAUTHZ(qauthz_simple_new(aclname,
+ QAUTHZ_SIMPLE_POLICY_DENY,
+ &error_abort));
+ g_free(aclname);
+ }
+ } else if (saslacl) {
+ error_setg(errp, "SASL ACL provided when SASL is disabled");
+ goto fail;
}
#endif
--
2.5.0
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 01/10] qdict: implement a qdict_crumple method for un-flattening a dict
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 01/10] qdict: implement a qdict_crumple method for un-flattening a dict Daniel P. Berrange
` (8 preceding siblings ...)
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 10/10] vnc: allow specifying a custom ACL object name Daniel P. Berrange
@ 2016-03-21 22:45 ` Eric Blake
2016-03-22 15:44 ` Daniel P. Berrange
9 siblings, 1 reply; 32+ messages in thread
From: Eric Blake @ 2016-03-21 22:45 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Paolo Bonzini, qemu-block, Markus Armbruster,
Andreas Färber, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 9478 bytes --]
On 03/10/2016 11:59 AM, Daniel P. Berrange wrote:
> The qdict_flatten() method will take a dict whose elements are
> further nested dicts/lists and flatten them by concatenating
> keys.
>
> The qdict_crumple() method aims to do the reverse, taking a flat
> qdict, and turning it into a set of nested dicts/lists. It will
> apply nesting based on the key name, with a '.' indicating a
> new level in the hierarchy. If the keys in the nested structure
> are all numeric, it will create a list, otherwise it will create
> a dict.
>
>
> will get turned into a dict with one element 'foo' whose
> value is a list. The list elements will each in turn be
> dicts.
>
> {
> 'foo' => [
s/=>/:/
> { 'bar': 'one', 'wizz': '1' }
s/$/,/
> { 'bar': 'two', 'wizz': '2' }
> ],
> }
>
> The intent of this function is that it allows a set of QemuOpts
> to be turned into a nested data structure that mirrors the nested
s/the nested/the nesting/
> used when the same object is defined over QMP.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> include/qapi/qmp/qdict.h | 1 +
> qobject/qdict.c | 267 +++++++++++++++++++++++++++++++++++++++++++++++
> tests/check-qdict.c | 143 +++++++++++++++++++++++++
> 3 files changed, 411 insertions(+)
>
> +
> +/**
> + * qdict_split_flat_key:
> + *
> + * Given a flattened key such as 'foo.0.bar', split it
> + * into two parts at the first '.' separator. Allows
> + * double dot ('..') to escape the normal separator.
> + *
> + * eg
> + * 'foo.0.bar' -> prefix='foo' and suffix='0.bar'
> + * 'foo..0.bar' -> prefix='foo.0' and suffix='bar'
> + *
> + * The '..' sequence will be unescaped in the returned
> + * 'prefix' string. The 'suffix' string will be left
> + * in escaped format, so it can be fed back into the
> + * qdict_split_flat_key() key as the input later.
> + */
Might be worth mentioning that prefix and suffix must both be non-NULL,
and that the caller must g_free() the two resulting strings.
> +static void qdict_split_flat_key(const char *key, char **prefix, char **suffix)
> +{
> + const char *separator;
> + size_t i, j;
> +
> + /* Find first '.' separator, but if there is a pair '..'
> + * that acts as an escape, so skip over '..' */
> + separator = NULL;
> + do {
> + if (separator) {
> + separator += 2;
> + } else {
> + separator = key;
> + }
> + separator = strchr(separator, '.');
> + } while (separator && *(separator + 1) == '.');
I'd probably have written separator[1] == '.', but your approach is
synonymous.
> +
> + if (separator) {
> + *prefix = g_strndup(key,
> + separator - key);
> + *suffix = g_strdup(separator + 1);
> + } else {
> + *prefix = g_strdup(key);
> + *suffix = NULL;
> + }
> +
> + /* Unescape the '..' sequence into '.' */
> + for (i = 0, j = 0; (*prefix)[i] != '\0'; i++, j++) {
> + if ((*prefix)[i] == '.' &&
> + (*prefix)[i + 1] == '.') {
Technically, if (*prefix)[i] == '.', we could assert((*prefix)[i + 1] ==
'.'), since the only way to get a '.' in prefix is via escaping. For
that matter, you could short-circuit (part of) the loop by doing a
strchr for '.' (if not found, the loop is not needed; if found, start
the reduction at that point rather on the bytes leading up to that point).
> + i++;
> + }
> + (*prefix)[j] = (*prefix)[i];
> + }
> + (*prefix)[j] = '\0';
> +}
> +
> +
> +/**
> + * qdict_list_size:
> + * @maybe_List: dict that may be only list elements
s/List/list/
> + *
> + * Determine whether all keys in @maybe_list are
> + * valid list elements. They they are all valid,
s/They they/If they/
> + * then this returns the number of elements. If
> + * they all look like non-numeric keys, then returns
> + * zero. If there is a mix of numeric and non-numeric
> + * keys, then an error is set as it is both a list
> + * and a dict at once.
> + *
> + * Returns: number of list elemets, 0 if a dict, -1 on error
s/elemets/elements/
> + */
> +static ssize_t qdict_list_size(QDict *maybe_list, Error **errp)
> +{
> + const QDictEntry *entry, *next;
> + ssize_t len = 0;
> + ssize_t max = -1;
> + int is_list = -1;
> + int64_t val;
> +
> + entry = qdict_first(maybe_list);
> + while (entry != NULL) {
> + next = qdict_next(maybe_list, entry);
> +
> + if (qemu_strtoll(entry->key, NULL, 10, &val) == 0) {
> + if (is_list == -1) {
> + is_list = 1;
> + } else if (!is_list) {
> + error_setg(errp,
> + "Key '%s' is for a list, but previous key is "
> + "for a dict", entry->key);
Keys are unsorted, so it's a bit hard to call it "previous key". Maybe
a better error message would be along the lines of "cannot crumple
dictionary because of a mix of list and non-list keys"? I dunno...
> + return -1;
> + }
> + len++;
> + if (val > max) {
> + max = val;
> + }
> + } else {
> + if (is_list == -1) {
> + is_list = 0;
> + } else if (is_list) {
> + error_setg(errp,
> + "Key '%s' is for a dict, but previous key is "
> + "for a list", entry->key);
...same argument. If we can wordsmith something that makes sense, it
might work for both places. Otherwise, I can live with your messages.
> +/**
> + * qdict_crumple:
> + *
Worth documenting the 'recursive' parameter?
> + * Reverses the flattening done by qdict_flatten by
> + * crumpling the dicts into a nested structure. Similar
> + * qdict_array_split, but copes with arbitrary nesting
> + * of dicts & arrays, not merely one level of arrays
> + *
> + * { 'foo.0.bar': 'one', 'foo.0.wizz': '1',
> + * 'foo.1.bar': 'two', 'foo.1.wizz': '2' }
> + *
> + * =>
> + *
> + * {
> + * 'foo' => [
s/=>/:/
> + * { 'bar': 'one', 'wizz': '1' }
s/$/,/
> + * { 'bar': 'two', 'wizz': '2' }
> + * ],
> + * }
> + *
Worth mentioning the escaping of '.' in key names?
> + */
> +QObject *qdict_crumple(QDict *src, bool recursive, Error **errp)
> +{
> + const QDictEntry *entry, *next;
> + QDict *two_level, *multi_level = NULL;
> + QObject *dst = NULL, *child;
> + ssize_t list_len;
> + size_t i;
> + char *prefix = NULL, *suffix = NULL;
> +
> + two_level = qdict_new();
> + entry = qdict_first(src);
> +
> + /* Step 1: split our totally flat dict into a two level dict */
> +
> + /* Step 2: optionally process the two level dict recursively
> + * into a multi-level dict */
> + if (recursive) {
> +
> + /* Step 3: detect if we need to turn our dict into list */
> + list_len = qdict_list_size(multi_level, errp);
> + if (list_len < 0) {
> + goto error;
> + }
> +
> + if (list_len) {
> + dst = QOBJECT(qlist_new());
> +
> + for (i = 0; i < list_len; i++) {
> + char *key = g_strdup_printf("%zu", i);
> +
> + child = qdict_get(multi_level, key);
> + g_free(key);
> + if (!child) {
> + error_setg(errp, "Unexpected missing list entry %zu", i);
Couldn't we assert() this, since it is a programming bug if
qdict_list_size() let us get this far but then the key disappeared?
Overall looks like it does the trick.
> +++ b/tests/check-qdict.c
> @@ -596,6 +596,140 @@ static void qdict_join_test(void)
> QDECREF(dict2);
> }
>
> +
> +static void qdict_crumple_test_nonrecursive(void)
> +{
This only covers a single layer of collapse, but not turning a dict into
a list. Is it also worth covering a case where no list indices are
involved, such as the four keys "a.b.d", "a.b.e", "a.c.d", "a.d.e" being
crumpled non-recursively into a single dict "a" with keys "b.d", "b.e",
"c.d", and "d.e"?
> +
> +static void qdict_crumple_test_recursive(void)
> +{
> +
This only covers a list of dict collapse, not a true multi-layer dict
collapse. Is it also worth covering the same four keys as above, but
this time that dict "a" has keys "b" and "c", each of which is a dict in
turn with keys "d" and "e"?
> +static void qdict_crumple_test_empty(void)
> +{
So an empty dict is never crumpled to an empty list. I guess that
shouldn't matter.
> +
> +static void qdict_crumple_test_bad_inputs(void)
> +{
> + QDict *src;
> + Error *error = NULL;
> +
> +
> + src = qdict_new();
> + /* The input should be flat, ie no dicts or lists */
> + qdict_put(src, "rule.0", qdict_new());
> + qdict_put(src, "rule.a", qstring_from_str("allow"));
I'd use "rule.a" and "rule.b" here, so that you aren't confusing this
with the earlier test that you can't mix list and dict.
I'd also add a negative test for "rule.1" without "rule.0" being invalid
(missing a list index).
I'll wait to give R-b until I get further into the series, and/or you
post a v4, but it's mostly there.
--
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] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 02/10] qapi: allow QmpInputVisitor to auto-cast types
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 02/10] qapi: allow QmpInputVisitor to auto-cast types Daniel P. Berrange
@ 2016-03-21 23:18 ` Eric Blake
2016-03-22 15:49 ` Daniel P. Berrange
0 siblings, 1 reply; 32+ messages in thread
From: Eric Blake @ 2016-03-21 23:18 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Paolo Bonzini, qemu-block, Markus Armbruster,
Andreas Färber, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 4975 bytes --]
On 03/10/2016 11:59 AM, Daniel P. Berrange wrote:
> Currently the QmpInputVisitor assumes that all scalar
> values are directly represented as their final types.
> ie it assumes an 'int' is using QInt, and a 'bool' is
> using QBool.
>
> This extends it so that QString is optionally permitted
> for any of the non-string scalar types. This behaviour
> is turned on by requesting the 'autocast' flag in the
> constructor.
>
> This makes it possible to use QmpInputVisitor with a
> QDict produced from QemuOpts, where everything is in
> string format.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> include/qapi/qmp-input-visitor.h | 3 +
> qapi/qmp-input-visitor.c | 96 +++++++++++++++++++++++++++-----
> tests/test-qmp-input-visitor.c | 115 ++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 196 insertions(+), 18 deletions(-)
>
> diff --git a/include/qapi/qmp-input-visitor.h b/include/qapi/qmp-input-visitor.h
> index 3ed499c..c25cb7c 100644
> --- a/include/qapi/qmp-input-visitor.h
> +++ b/include/qapi/qmp-input-visitor.h
> @@ -21,6 +21,9 @@ typedef struct QmpInputVisitor QmpInputVisitor;
>
> QmpInputVisitor *qmp_input_visitor_new(QObject *obj);
> QmpInputVisitor *qmp_input_visitor_new_strict(QObject *obj);
> +QmpInputVisitor *qmp_input_visitor_new_full(QObject *obj,
> + bool strict,
> + bool autocast);
We have so few uses of qmp_input_visitor_new* that it might be worth
just having a single prototype, and maybe using an 'int flags' instead
of a string of bool. But not a show-stopper for this patch (rather, an
idea for a future patch).
> - *obj = qint_get_int(qint);
> + qstr = qobject_to_qstring(qobj);
> + if (qstr && qstr->string && qiv->autocast) {
> + errno = 0;
Dead setting of errno, since...
> + if (qemu_strtoll(qstr->string, NULL, 10, obj) == 0) {
qemu_strtoll() handles it on your behalf, and you aren't using
error_setg_errno().
> @@ -233,30 +245,61 @@ static void qmp_input_type_uint64(Visitor *v, const char *name, uint64_t *obj,
> {
> /* FIXME: qobject_to_qint mishandles values over INT64_MAX */
> QmpInputVisitor *qiv = to_qiv(v);
> - QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true));
> + QObject *qobj = qmp_input_get_object(qiv, name, true);
> + QInt *qint;
> + QString *qstr;
>
> - if (!qint) {
> - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> - "integer");
> + qint = qobject_to_qint(qobj);
> + if (qint) {
> + *obj = qint_get_int(qint);
> return;
> }
>
> - *obj = qint_get_int(qint);
> + qstr = qobject_to_qstring(qobj);
> + if (qstr && qstr->string && qiv->autocast) {
> + errno = 0;
> + if (qemu_strtoull(qstr->string, NULL, 10, obj) == 0) {
And again.
Hmm. Do we need to worry about partial asymmetry? That is,
qint_get_int() returns a signed number, but qemu_strtoull() parses
unsigned; if the original conversion from JSON to qint uses a different
parser, then we could have funny results where we get different results
for things like:
"key1":9223372036854775807, "key2":"9223372036854775807",
even though the same string of digits is being parsed, based on whether
the different parsers handle numbers larger than INT64_MAX differently.
[Ultimately, I'd like QInt to be enhanced to track whether the input was
signed or unsigned, and automatically make the output match the input
when converting back to string; that is, track 65 bits of information
instead of 64; but that's no sooner than 2.7 material]
> static void qmp_input_type_bool(Visitor *v, const char *name, bool *obj,
> Error **errp)
> {
> QmpInputVisitor *qiv = to_qiv(v);
> - QBool *qbool = qobject_to_qbool(qmp_input_get_object(qiv, name, true));
> + QObject *qobj = qmp_input_get_object(qiv, name, true);
> + QBool *qbool;
> + QString *qstr;
>
> - if (!qbool) {
> - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> - "boolean");
> + qbool = qobject_to_qbool(qobj);
> + if (qbool) {
> + *obj = qbool_get_bool(qbool);
> return;
> }
>
> - *obj = qbool_get_bool(qbool);
> +
> + qstr = qobject_to_qstring(qobj);
> + if (qstr && qstr->string && qiv->autocast) {
> + if (!strcasecmp(qstr->string, "on") ||
> + !strcasecmp(qstr->string, "yes") ||
> + !strcasecmp(qstr->string, "true")) {
> + *obj = true;
> + return;
> + }
Do we also want to allow "0"/"1" for true/false?
Overall, I'm a big fan of this patch.
--
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] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 03/10] qom: support arbitrary non-scalar properties with -object
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 03/10] qom: support arbitrary non-scalar properties with -object Daniel P. Berrange
@ 2016-03-21 23:27 ` Eric Blake
2016-03-22 9:07 ` Markus Armbruster
2016-03-22 15:51 ` Daniel P. Berrange
0 siblings, 2 replies; 32+ messages in thread
From: Eric Blake @ 2016-03-21 23:27 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Paolo Bonzini, qemu-block, Markus Armbruster,
Andreas Färber, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 3684 bytes --]
On 03/10/2016 11:59 AM, Daniel P. Berrange wrote:
> The current -object command line syntax only allows for
> creation of objects with scalar properties, or a list
> with a fixed scalar element type. Objects which have
> properties that are represented as structs in the QAPI
> schema cannot be created using -object.
>
> This is a design limitation of the way the OptsVisitor
> is written. It simply iterates over the QemuOpts values
> as a flat list. The support for lists is enabled by
> allowing the same key to be repeated in the opts string.
>
> It is not practical to extend the OptsVisitor to support
> more complex data structures while also maintaining
> the existing list handling behaviour that is relied upon
> by other areas of QEMU.
Zoltán Kővágó tried earlier with his GSoC patches for the audio
subsystem last year, but those got stalled waiting for qapi enhancements
to go in. But I think your approach is indeed a bit nicer (rather than
making the warty OptsVisitor even wartier, just avoid it).
>
> Fortunately there is no existing object that implements
> the UserCreatable interface that relies on the list
> handling behaviour, so it is possible to swap out the
> OptsVisitor for a different visitor implementation, so
> -object supports non-scalar properties, thus leaving
> other users of OptsVisitor unaffected.
>
> The previously added qdict_crumple() method is able to
> take a qdict containing a flat set of properties and
> turn that into a arbitrarily nested set of dicts and
> lists. By combining qemu_opts_to_qdict and qdict_crumple()
> together, we can turn the opt string into a data structure
> that is practically identical to that passed over QMP
> when defining an object. The only difference is that all
> the scalar values are represented as strings, rather than
> strings, ints and bools. This is sufficient to let us
> replace the OptsVisitor with the QMPInputVisitor for
> use with -object.
Indeed, nice replacement.
>
> Thus -object can now support non-scalar properties,
> for example the QMP object
>
> {
> "execute": "object-add",
> "arguments": {
> "qom-type": "demo",
> "id": "demo0",
> "parameters": {
> "foo": [
> { "bar": "one", "wizz": "1" },
> { "bar": "two", "wizz": "2" }
> ]
> }
> }
> }
>
> Would be creatable via the CLI now using
>
> $QEMU \
> -object demo,id=demo0,\
> foo.0.bar=one,foo.0.wizz=1,\
> foo.1.bar=two,foo.1.wizz=2
>
> This is also wired up to work for the 'object_add' command
> in the HMP monitor with the same syntax.
>
> (hmp) object_add demo,id=demo0,\
> foo.0.bar=one,foo.0.wizz=1,\
> foo.1.bar=two,foo.1.wizz=2
Maybe mention that the indentation is not actually present in the real
command lines typed.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> hmp.c | 18 +--
> qom/object_interfaces.c | 20 ++-
> tests/check-qom-proplist.c | 295 ++++++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 313 insertions(+), 20 deletions(-)
>
> @@ -120,6 +120,7 @@ Object *user_creatable_add_type(const char *type, const char *id,
> obj = object_new(type);
> if (qdict) {
> for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
> +
> object_property_set(obj, v, e->key, &local_err);
> if (local_err) {
> goto out;
Spurious hunk?
--
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] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 03/10] qom: support arbitrary non-scalar properties with -object
2016-03-21 23:27 ` Eric Blake
@ 2016-03-22 9:07 ` Markus Armbruster
2016-03-22 10:34 ` Daniel P. Berrange
2016-03-22 15:51 ` Daniel P. Berrange
1 sibling, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2016-03-22 9:07 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-block, qemu-devel, Max Reitz, Paolo Bonzini, Andreas Färber
Eric Blake <eblake@redhat.com> writes:
> On 03/10/2016 11:59 AM, Daniel P. Berrange wrote:
>> The current -object command line syntax only allows for
>> creation of objects with scalar properties, or a list
>> with a fixed scalar element type. Objects which have
>> properties that are represented as structs in the QAPI
>> schema cannot be created using -object.
>>
>> This is a design limitation of the way the OptsVisitor
>> is written. It simply iterates over the QemuOpts values
>> as a flat list. The support for lists is enabled by
>> allowing the same key to be repeated in the opts string.
>>
>> It is not practical to extend the OptsVisitor to support
>> more complex data structures while also maintaining
>> the existing list handling behaviour that is relied upon
>> by other areas of QEMU.
>
> Zoltán Kővágó tried earlier with his GSoC patches for the audio
> subsystem last year, but those got stalled waiting for qapi enhancements
> to go in.
Yet another series stalled on the big QAPI rework. Hitting a GSoC
student that way is really unfortunate.
> But I think your approach is indeed a bit nicer (rather than
> making the warty OptsVisitor even wartier, just avoid it).
QemuOpts defines an important part of the command line language, namely
(most of) the syntax of many option arguments. It parses them into a
set of (name, value) pairs.
"Most of", because additional syntax may hide in the parameter value.
Parameter values are typed, except when they aren't. Types are limited
to string, bool, uint64_t number (accepts negative numbers and casts
them) and uint64_t size (rejects negative numbers, accepts suffixes).
OptsVisitor adds special list syntax. It's used with untyped values.
Bypassing OptsVisitor risks adding different special syntax. Doesn't
mean it's a bad idea, only that we need to keep close watch on what it
does to the language. See below.
I merely scratched the surprising amount of complexity that has accrued
over time. If you doubt me, study how the merge_lists flag works, and
how it interacts with the several ways we do defaults.
The gap between QemuOpts' and QAPI's type system is obvious. But it's
not just a gap, it's also contraditions: QAPI does only *signed*
integers.
Nevertheless, the path from command line to QAPI is through QemuOpts for
now.
Aside: for historical reasons, we have a few QMP commands that detour
through QemuOpts. Mistakes, do not add more.
Ceterum censeo QemuOpts esse delendam. And I say that as author of 35
out of 117 commits touching qemu-option.c.
>> Fortunately there is no existing object that implements
>> the UserCreatable interface that relies on the list
>> handling behaviour, so it is possible to swap out the
>> OptsVisitor for a different visitor implementation, so
>> -object supports non-scalar properties, thus leaving
>> other users of OptsVisitor unaffected.
>>
>> The previously added qdict_crumple() method is able to
>> take a qdict containing a flat set of properties and
>> turn that into a arbitrarily nested set of dicts and
>> lists. By combining qemu_opts_to_qdict and qdict_crumple()
>> together, we can turn the opt string into a data structure
>> that is practically identical to that passed over QMP
>> when defining an object. The only difference is that all
>> the scalar values are represented as strings, rather than
>> strings, ints and bools. This is sufficient to let us
>> replace the OptsVisitor with the QMPInputVisitor for
>> use with -object.
>
> Indeed, nice replacement.
>
>>
>> Thus -object can now support non-scalar properties,
>> for example the QMP object
>>
>> {
>> "execute": "object-add",
>> "arguments": {
>> "qom-type": "demo",
>> "id": "demo0",
>> "parameters": {
>> "foo": [
>> { "bar": "one", "wizz": "1" },
>> { "bar": "two", "wizz": "2" }
>> ]
>> }
>> }
>> }
>>
>> Would be creatable via the CLI now using
>>
>> $QEMU \
>> -object demo,id=demo0,\
>> foo.0.bar=one,foo.0.wizz=1,\
>> foo.1.bar=two,foo.1.wizz=2
Okay, this adds a bare minimum of syntax, and it's not even new: we use
similar syntax for block stuff already.
>> This is also wired up to work for the 'object_add' command
>> in the HMP monitor with the same syntax.
>>
>> (hmp) object_add demo,id=demo0,\
>> foo.0.bar=one,foo.0.wizz=1,\
>> foo.1.bar=two,foo.1.wizz=2
>
> Maybe mention that the indentation is not actually present in the real
> command lines typed.
>
>>
>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>> ---
>> hmp.c | 18 +--
>> qom/object_interfaces.c | 20 ++-
>> tests/check-qom-proplist.c | 295 ++++++++++++++++++++++++++++++++++++++++++++-
>> 3 files changed, 313 insertions(+), 20 deletions(-)
>>
>
>> @@ -120,6 +120,7 @@ Object *user_creatable_add_type(const char *type, const char *id,
>> obj = object_new(type);
>> if (qdict) {
>> for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
>> +
>> object_property_set(obj, v, e->key, &local_err);
>> if (local_err) {
>> goto out;
>
> Spurious hunk?
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 03/10] qom: support arbitrary non-scalar properties with -object
2016-03-22 9:07 ` Markus Armbruster
@ 2016-03-22 10:34 ` Daniel P. Berrange
0 siblings, 0 replies; 32+ messages in thread
From: Daniel P. Berrange @ 2016-03-22 10:34 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-block, qemu-devel, Max Reitz, Paolo Bonzini, Andreas Färber
On Tue, Mar 22, 2016 at 10:07:42AM +0100, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
>
> > On 03/10/2016 11:59 AM, Daniel P. Berrange wrote:
> >> The current -object command line syntax only allows for
> >> creation of objects with scalar properties, or a list
> >> with a fixed scalar element type. Objects which have
> >> properties that are represented as structs in the QAPI
> >> schema cannot be created using -object.
> >>
> >> This is a design limitation of the way the OptsVisitor
> >> is written. It simply iterates over the QemuOpts values
> >> as a flat list. The support for lists is enabled by
> >> allowing the same key to be repeated in the opts string.
> >>
> >> It is not practical to extend the OptsVisitor to support
> >> more complex data structures while also maintaining
> >> the existing list handling behaviour that is relied upon
> >> by other areas of QEMU.
> >
> > Zoltán Kővágó tried earlier with his GSoC patches for the audio
> > subsystem last year, but those got stalled waiting for qapi enhancements
> > to go in.
>
> Yet another series stalled on the big QAPI rework. Hitting a GSoC
> student that way is really unfortunate.
>
> > But I think your approach is indeed a bit nicer (rather than
> > making the warty OptsVisitor even wartier, just avoid it).
>
> QemuOpts defines an important part of the command line language, namely
> (most of) the syntax of many option arguments. It parses them into a
> set of (name, value) pairs.
>
> "Most of", because additional syntax may hide in the parameter value.
>
> Parameter values are typed, except when they aren't. Types are limited
> to string, bool, uint64_t number (accepts negative numbers and casts
> them) and uint64_t size (rejects negative numbers, accepts suffixes).
>
> OptsVisitor adds special list syntax. It's used with untyped values.
>
> Bypassing OptsVisitor risks adding different special syntax. Doesn't
> mean it's a bad idea, only that we need to keep close watch on what it
> does to the language. See below.
FWIW, when I first started attacking this problem I actually went down
the path of extending the OptsVisitor to cope with arbitrarily nested
structs + lists. I quickly discovered that special list syntax supported
by the OptsVisitor. I tried to hack support for nested structs + lists
on top of that, but ultimately the way the special list syntax is designed
makes that an impossible problem without breaking back compat, or having
OptsVisitor support two completely different lists syntaxes at the same
time with ambiguous parsing results. None the less I did implement that
all and it was a huge amount of work. I then took a look at QMPInputVisitor
and realized that if we could converts a QemuOpts into a QDict, we could
just reuse the QMPInputVisitor.
In the specific case of -object the fact that QMPInputVisitor does not
support the special list syntax from OptsVisitor is not a problem because
we don't have any existing user defined object type that uses list props
yet. So if we want to change -object from OptsVisitor to QMPInputVisitor
the sooner we change it the better - it is only a matter of time before
something comes along that depends on the existing special list syntax
and then we'll be locked into OptsVisitor's non-extensible approach.
> >> Would be creatable via the CLI now using
> >>
> >> $QEMU \
> >> -object demo,id=demo0,\
> >> foo.0.bar=one,foo.0.wizz=1,\
> >> foo.1.bar=two,foo.1.wizz=2
>
> Okay, this adds a bare minimum of syntax, and it's not even new: we use
> similar syntax for block stuff already.
Yes, indeed that usage by the block layer is what motivated me to ditch
OptsVisitor and take the approach of converting QemuOpts to QDict and
then using QMPInputVisitor.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 01/10] qdict: implement a qdict_crumple method for un-flattening a dict
2016-03-21 22:45 ` [Qemu-devel] [PATCH v3 01/10] qdict: implement a qdict_crumple method for un-flattening a dict Eric Blake
@ 2016-03-22 15:44 ` Daniel P. Berrange
0 siblings, 0 replies; 32+ messages in thread
From: Daniel P. Berrange @ 2016-03-22 15:44 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-block, Markus Armbruster, qemu-devel, Paolo Bonzini,
Max Reitz, Andreas Färber
On Mon, Mar 21, 2016 at 04:45:39PM -0600, Eric Blake wrote:
> On 03/10/2016 11:59 AM, Daniel P. Berrange wrote:
> > + /* Unescape the '..' sequence into '.' */
> > + for (i = 0, j = 0; (*prefix)[i] != '\0'; i++, j++) {
> > + if ((*prefix)[i] == '.' &&
> > + (*prefix)[i + 1] == '.') {
>
> Technically, if (*prefix)[i] == '.', we could assert((*prefix)[i + 1] ==
> '.'), since the only way to get a '.' in prefix is via escaping. For
> that matter, you could short-circuit (part of) the loop by doing a
> strchr for '.' (if not found, the loop is not needed; if found, start
> the reduction at that point rather on the bytes leading up to that point).
I'm not seeing obvious benefit in trying to short-circuit the loop
using a strchr, as both ways you still end up iterating over all
chars in the string - its just that you're hiding the iteration
in strchr instead.
> > +static ssize_t qdict_list_size(QDict *maybe_list, Error **errp)
> > +{
> > + const QDictEntry *entry, *next;
> > + ssize_t len = 0;
> > + ssize_t max = -1;
> > + int is_list = -1;
> > + int64_t val;
> > +
> > + entry = qdict_first(maybe_list);
> > + while (entry != NULL) {
> > + next = qdict_next(maybe_list, entry);
> > +
> > + if (qemu_strtoll(entry->key, NULL, 10, &val) == 0) {
> > + if (is_list == -1) {
> > + is_list = 1;
> > + } else if (!is_list) {
> > + error_setg(errp,
> > + "Key '%s' is for a list, but previous key is "
> > + "for a dict", entry->key);
>
> Keys are unsorted, so it's a bit hard to call it "previous key". Maybe
> a better error message would be along the lines of "cannot crumple
> dictionary because of a mix of list and non-list keys"? I dunno...
Yeah, I'll use
"Cannot crumple a dictionary with a mix of list and non-list keys"
>
> > + return -1;
> > + }
> > + len++;
> > + if (val > max) {
> > + max = val;
> > + }
> > + } else {
> > + if (is_list == -1) {
> > + is_list = 0;
> > + } else if (is_list) {
> > + error_setg(errp,
> > + "Key '%s' is for a dict, but previous key is "
> > + "for a list", entry->key);
>
> ...same argument. If we can wordsmith something that makes sense, it
> might work for both places. Otherwise, I can live with your messages.
> > +++ b/tests/check-qdict.c
> > @@ -596,6 +596,140 @@ static void qdict_join_test(void)
> > QDECREF(dict2);
> > }
> >
> > +
> > +static void qdict_crumple_test_nonrecursive(void)
> > +{
>
> This only covers a single layer of collapse, but not turning a dict into
> a list. Is it also worth covering a case where no list indices are
> involved, such as the four keys "a.b.d", "a.b.e", "a.c.d", "a.d.e" being
> crumpled non-recursively into a single dict "a" with keys "b.d", "b.e",
> "c.d", and "d.e"?
I'll add an explicit rule to test dict -> list conversion, and some
extra dict items here to cover proper nested dicts.
>
> > +
> > +static void qdict_crumple_test_recursive(void)
> > +{
> > +
>
> This only covers a list of dict collapse, not a true multi-layer dict
> collapse. Is it also worth covering the same four keys as above, but
> this time that dict "a" has keys "b" and "c", each of which is a dict in
> turn with keys "d" and "e"?
I'll add some more dict items to properly cover nested dicts
> > +static void qdict_crumple_test_bad_inputs(void)
> > +{
> > + QDict *src;
> > + Error *error = NULL;
> > +
>
> > +
> > + src = qdict_new();
> > + /* The input should be flat, ie no dicts or lists */
> > + qdict_put(src, "rule.0", qdict_new());
> > + qdict_put(src, "rule.a", qstring_from_str("allow"));
>
> I'd use "rule.a" and "rule.b" here, so that you aren't confusing this
> with the earlier test that you can't mix list and dict.
Good point.
> I'd also add a negative test for "rule.1" without "rule.0" being invalid
> (missing a list index).
Yep, I'll add that.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 02/10] qapi: allow QmpInputVisitor to auto-cast types
2016-03-21 23:18 ` Eric Blake
@ 2016-03-22 15:49 ` Daniel P. Berrange
2016-03-22 16:20 ` Eric Blake
0 siblings, 1 reply; 32+ messages in thread
From: Daniel P. Berrange @ 2016-03-22 15:49 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-block, Markus Armbruster, qemu-devel, Paolo Bonzini,
Max Reitz, Andreas Färber
On Mon, Mar 21, 2016 at 05:18:01PM -0600, Eric Blake wrote:
> On 03/10/2016 11:59 AM, Daniel P. Berrange wrote:
> > Currently the QmpInputVisitor assumes that all scalar
> > values are directly represented as their final types.
> > ie it assumes an 'int' is using QInt, and a 'bool' is
> > using QBool.
> >
> > This extends it so that QString is optionally permitted
> > for any of the non-string scalar types. This behaviour
> > is turned on by requesting the 'autocast' flag in the
> > constructor.
> >
> > This makes it possible to use QmpInputVisitor with a
> > QDict produced from QemuOpts, where everything is in
> > string format.
> >
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> > include/qapi/qmp-input-visitor.h | 3 +
> > qapi/qmp-input-visitor.c | 96 +++++++++++++++++++++++++++-----
> > tests/test-qmp-input-visitor.c | 115 ++++++++++++++++++++++++++++++++++++++-
> > 3 files changed, 196 insertions(+), 18 deletions(-)
> > - *obj = qint_get_int(qint);
> > + qstr = qobject_to_qstring(qobj);
> > + if (qstr && qstr->string && qiv->autocast) {
> > + errno = 0;
> > + if (qemu_strtoull(qstr->string, NULL, 10, obj) == 0) {
>
> And again.
>
> Hmm. Do we need to worry about partial asymmetry? That is,
> qint_get_int() returns a signed number, but qemu_strtoull() parses
> unsigned; if the original conversion from JSON to qint uses a different
> parser, then we could have funny results where we get different results
> for things like:
> "key1":9223372036854775807, "key2":"9223372036854775807",
> even though the same string of digits is being parsed, based on whether
> the different parsers handle numbers larger than INT64_MAX differently.
Is this something you want me to change for re-post, or just a general
point for future ? ie should I be using qemu_strtoll instead of
qemu_strtoull or something else ? qint itself doesn't seem
to concern itself with parsing ints from strintgs, so presumably
this is from json code ?
> [Ultimately, I'd like QInt to be enhanced to track whether the input was
> signed or unsigned, and automatically make the output match the input
> when converting back to string; that is, track 65 bits of information
> instead of 64; but that's no sooner than 2.7 material]
>
>
> > static void qmp_input_type_bool(Visitor *v, const char *name, bool *obj,
> > Error **errp)
> > {
> > QmpInputVisitor *qiv = to_qiv(v);
> > - QBool *qbool = qobject_to_qbool(qmp_input_get_object(qiv, name, true));
> > + QObject *qobj = qmp_input_get_object(qiv, name, true);
> > + QBool *qbool;
> > + QString *qstr;
> >
> > - if (!qbool) {
> > - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> > - "boolean");
> > + qbool = qobject_to_qbool(qobj);
> > + if (qbool) {
> > + *obj = qbool_get_bool(qbool);
> > return;
> > }
> >
> > - *obj = qbool_get_bool(qbool);
> > +
> > + qstr = qobject_to_qstring(qobj);
> > + if (qstr && qstr->string && qiv->autocast) {
> > + if (!strcasecmp(qstr->string, "on") ||
> > + !strcasecmp(qstr->string, "yes") ||
> > + !strcasecmp(qstr->string, "true")) {
> > + *obj = true;
> > + return;
> > + }
>
> Do we also want to allow "0"/"1" for true/false?
These 3 strings I took from opts-visitor.c, so to maintain compat
we probably should not allow 0/1, unless we decide to extend
opts-visitor too
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 03/10] qom: support arbitrary non-scalar properties with -object
2016-03-21 23:27 ` Eric Blake
2016-03-22 9:07 ` Markus Armbruster
@ 2016-03-22 15:51 ` Daniel P. Berrange
1 sibling, 0 replies; 32+ messages in thread
From: Daniel P. Berrange @ 2016-03-22 15:51 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-block, Markus Armbruster, qemu-devel, Paolo Bonzini,
Max Reitz, Andreas Färber
On Mon, Mar 21, 2016 at 05:27:24PM -0600, Eric Blake wrote:
> On 03/10/2016 11:59 AM, Daniel P. Berrange wrote:
> > The current -object command line syntax only allows for
> > creation of objects with scalar properties, or a list
> > with a fixed scalar element type. Objects which have
> > properties that are represented as structs in the QAPI
> > schema cannot be created using -object.
> >
> > This is a design limitation of the way the OptsVisitor
> > is written. It simply iterates over the QemuOpts values
> > as a flat list. The support for lists is enabled by
> > allowing the same key to be repeated in the opts string.
> >
> > It is not practical to extend the OptsVisitor to support
> > more complex data structures while also maintaining
> > the existing list handling behaviour that is relied upon
> > by other areas of QEMU.
>
> Zoltán Kővágó tried earlier with his GSoC patches for the audio
> subsystem last year, but those got stalled waiting for qapi enhancements
> to go in. But I think your approach is indeed a bit nicer (rather than
> making the warty OptsVisitor even wartier, just avoid it).
My first attempt did indeed modify OptsVisitor, but I quickly
abandoned it since it ended up being quite complex code to
make it fit in with the pre-existing hack to supports lists
of scalars in OptsVisitor. The QmpInputVisitor approach is
cleaner and simpler overall
> > Would be creatable via the CLI now using
> >
> > $QEMU \
> > -object demo,id=demo0,\
> > foo.0.bar=one,foo.0.wizz=1,\
> > foo.1.bar=two,foo.1.wizz=2
> >
> > This is also wired up to work for the 'object_add' command
> > in the HMP monitor with the same syntax.
> >
> > (hmp) object_add demo,id=demo0,\
> > foo.0.bar=one,foo.0.wizz=1,\
> > foo.1.bar=two,foo.1.wizz=2
>
> Maybe mention that the indentation is not actually present in the real
> command lines typed.
Heh, yeah
> > @@ -120,6 +120,7 @@ Object *user_creatable_add_type(const char *type, const char *id,
> > obj = object_new(type);
> > if (qdict) {
> > for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
> > +
> > object_property_set(obj, v, e->key, &local_err);
> > if (local_err) {
> > goto out;
>
> Spurious hunk?
Indeed
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 02/10] qapi: allow QmpInputVisitor to auto-cast types
2016-03-22 15:49 ` Daniel P. Berrange
@ 2016-03-22 16:20 ` Eric Blake
0 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2016-03-22 16:20 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: qemu-block, Markus Armbruster, qemu-devel, Paolo Bonzini,
Max Reitz, Andreas Färber
[-- Attachment #1: Type: text/plain, Size: 2511 bytes --]
On 03/22/2016 09:49 AM, Daniel P. Berrange wrote:
> On Mon, Mar 21, 2016 at 05:18:01PM -0600, Eric Blake wrote:
>> On 03/10/2016 11:59 AM, Daniel P. Berrange wrote:
>>> Currently the QmpInputVisitor assumes that all scalar
>>> values are directly represented as their final types.
>>> ie it assumes an 'int' is using QInt, and a 'bool' is
>>> using QBool.
>>>
>>> This extends it so that QString is optionally permitted
>>> for any of the non-string scalar types. This behaviour
>>> is turned on by requesting the 'autocast' flag in the
>>> constructor.
>>>
>> Hmm. Do we need to worry about partial asymmetry? That is,
>> qint_get_int() returns a signed number, but qemu_strtoull() parses
>> unsigned; if the original conversion from JSON to qint uses a different
>> parser, then we could have funny results where we get different results
>> for things like:
>> "key1":9223372036854775807, "key2":"9223372036854775807",
>> even though the same string of digits is being parsed, based on whether
>> the different parsers handle numbers larger than INT64_MAX differently.
>
> Is this something you want me to change for re-post, or just a general
> point for future ? ie should I be using qemu_strtoll instead of
> qemu_strtoull or something else ? qint itself doesn't seem
> to concern itself with parsing ints from strintgs, so presumably
> this is from json code ?
General comment for now. We already know we need a bigger audit of
handling of values larger than INT64_MAX, so any cleanups related to
that can be deferred to that later audit. But maybe a FIXME or TODO
comment in the code in your submission, to remind us to think about it
during the later audit, would help.
>>> + qstr = qobject_to_qstring(qobj);
>>> + if (qstr && qstr->string && qiv->autocast) {
>>> + if (!strcasecmp(qstr->string, "on") ||
>>> + !strcasecmp(qstr->string, "yes") ||
>>> + !strcasecmp(qstr->string, "true")) {
>>> + *obj = true;
>>> + return;
>>> + }
>>
>> Do we also want to allow "0"/"1" for true/false?
>
> These 3 strings I took from opts-visitor.c, so to maintain compat
> we probably should not allow 0/1, unless we decide to extend
> opts-visitor too
Good point. Maybe a comment pointing in both places pointing out that
we should keep them in sync is a worthwhile addition.
--
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] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 04/10] util: add QAuthZ object as an authorization base class
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 04/10] util: add QAuthZ object as an authorization base class Daniel P. Berrange
@ 2016-03-22 16:33 ` Eric Blake
2016-03-22 16:43 ` Daniel P. Berrange
2016-03-22 16:44 ` Daniel P. Berrange
0 siblings, 2 replies; 32+ messages in thread
From: Eric Blake @ 2016-03-22 16:33 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Paolo Bonzini, qemu-block, Markus Armbruster,
Andreas Färber, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 1997 bytes --]
On 03/10/2016 11:59 AM, Daniel P. Berrange wrote:
> The current qemu_acl module provides a simple access control
> list facility inside QEMU, which is used via a set of monitor
> commands acl_show, acl_policy, acl_add, acl_remove & acl_reset.
>
> Note there is no ability to create ACLs - the network services
> (eg VNC server) were expected to create ACLs that they want to
> check.
>
> There is also no way to define ACLs on the command line, nor
> potentially integrate with external authorization systems like
> polkit, pam, ldap lookup, etc.
>
> The QAuthZ object defines a minimal abstract QOM class that can
> be subclassed for creating different authorization providers.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> +++ b/include/qemu/authz.h
> +
> +/**
> + * QAuthZ:
> + *
> + * The QAuthZ class defines an API contract to be used
> + * for providing an authorization driver for network
> + * services.
Just network services? Or is it broader than that?
> +/**
> + * qauthz_is_allowed:
> + * @authz: the authorization object
> + * @identity: the user identity to authorize
> + * @errp: pointer to a NULL initialized error object
> + *
> + * Check if a user @identity is authorized
> + *
> + * Returns: true if @identity is authorizd, false otherwise
s/authorizd/authorized/
I think you need more documentation on return semantics. Do we have
strict binary return (either we returned true and errp is unset, or we
returned false and errp is set), or is it a ternary (we return true and
errp is unset: permission is explicitly granted; we return false and
errp is unset: permission is explicitly denied; or we set errp: we could
not determine permission). And if a ternary, do we also want to require
that setting 'errp' also requires a return of false, or is the return
undefined in that case?
--
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] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 04/10] util: add QAuthZ object as an authorization base class
2016-03-22 16:33 ` Eric Blake
@ 2016-03-22 16:43 ` Daniel P. Berrange
2016-03-22 16:44 ` Daniel P. Berrange
1 sibling, 0 replies; 32+ messages in thread
From: Daniel P. Berrange @ 2016-03-22 16:43 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-block, Markus Armbruster, qemu-devel, Paolo Bonzini,
Max Reitz, Andreas Färber
On Tue, Mar 22, 2016 at 10:33:42AM -0600, Eric Blake wrote:
> On 03/10/2016 11:59 AM, Daniel P. Berrange wrote:
> > The current qemu_acl module provides a simple access control
> > list facility inside QEMU, which is used via a set of monitor
> > commands acl_show, acl_policy, acl_add, acl_remove & acl_reset.
> >
> > Note there is no ability to create ACLs - the network services
> > (eg VNC server) were expected to create ACLs that they want to
> > check.
> >
> > There is also no way to define ACLs on the command line, nor
> > potentially integrate with external authorization systems like
> > polkit, pam, ldap lookup, etc.
> >
> > The QAuthZ object defines a minimal abstract QOM class that can
> > be subclassed for creating different authorization providers.
> >
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
>
> > +++ b/include/qemu/authz.h
> > +
> > +/**
> > + * QAuthZ:
> > + *
> > + * The QAuthZ class defines an API contract to be used
> > + * for providing an authorization driver for network
> > + * services.
>
> Just network services? Or is it broader than that?
>
> > +/**
> > + * qauthz_is_allowed:
> > + * @authz: the authorization object
> > + * @identity: the user identity to authorize
> > + * @errp: pointer to a NULL initialized error object
> > + *
> > + * Check if a user @identity is authorized
> > + *
> > + * Returns: true if @identity is authorizd, false otherwise
>
> s/authorizd/authorized/
>
> I think you need more documentation on return semantics. Do we have
> strict binary return (either we returned true and errp is unset, or we
> returned false and errp is set), or is it a ternary (we return true and
> errp is unset: permission is explicitly granted; we return false and
> errp is unset: permission is explicitly denied; or we set errp: we could
> not determine permission). And if a ternary, do we also want to require
> that setting 'errp' also requires a return of false, or is the return
> undefined in that case?
It is intended to be ternary, and if errp is set, the return value
should be false.
ie you should be able todo
if (qauthz_is_allowed(authz, identity, NULL))
....
safe in the knowledge that any error that you're ignoring will
result in denial of permission
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 04/10] util: add QAuthZ object as an authorization base class
2016-03-22 16:33 ` Eric Blake
2016-03-22 16:43 ` Daniel P. Berrange
@ 2016-03-22 16:44 ` Daniel P. Berrange
1 sibling, 0 replies; 32+ messages in thread
From: Daniel P. Berrange @ 2016-03-22 16:44 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-block, Markus Armbruster, qemu-devel, Paolo Bonzini,
Max Reitz, Andreas Färber
On Tue, Mar 22, 2016 at 10:33:42AM -0600, Eric Blake wrote:
> On 03/10/2016 11:59 AM, Daniel P. Berrange wrote:
> > The current qemu_acl module provides a simple access control
> > list facility inside QEMU, which is used via a set of monitor
> > commands acl_show, acl_policy, acl_add, acl_remove & acl_reset.
> >
> > Note there is no ability to create ACLs - the network services
> > (eg VNC server) were expected to create ACLs that they want to
> > check.
> >
> > There is also no way to define ACLs on the command line, nor
> > potentially integrate with external authorization systems like
> > polkit, pam, ldap lookup, etc.
> >
> > The QAuthZ object defines a minimal abstract QOM class that can
> > be subclassed for creating different authorization providers.
> >
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
>
> > +++ b/include/qemu/authz.h
> > +
> > +/**
> > + * QAuthZ:
> > + *
> > + * The QAuthZ class defines an API contract to be used
> > + * for providing an authorization driver for network
> > + * services.
>
> Just network services? Or is it broader than that?
Any service that requires authentication. It is actually nothing
specific to networking
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 05/10] util: add QAuthZSimple object type for a simple access control list
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 05/10] util: add QAuthZSimple object type for a simple access control list Daniel P. Berrange
@ 2016-03-22 17:38 ` Eric Blake
2016-03-23 12:38 ` Daniel P. Berrange
0 siblings, 1 reply; 32+ messages in thread
From: Eric Blake @ 2016-03-22 17:38 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Paolo Bonzini, qemu-block, Markus Armbruster,
Andreas Färber, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 9973 bytes --]
On 03/10/2016 11:59 AM, Daniel P. Berrange wrote:
> Add a QAuthZSimple object type that implements the QAuthZ
> interface. This simple built-in implementation maintains
> a trivial access control list with a sequence of match
> rules and a final default policy. This replicates the
> functionality currently provided by the qemu_acl module.
>
> To create an instance of this object via the QMP monitor,
> the syntax used would be
>
> {
> "execute": "object-add",
> "arguments": {
> "qom-type": "authz-simple",
> "id": "auth0",
> "parameters": {
> "rules": [
> { "match": "fred", "policy": "allow" },
> { "match": "bob", "policy": "allow" },
> { "match": "danb", "policy": "deny" },
> { "match": "dan*", "policy": "allow" }
> ],
> "policy": "deny"
> }
> }
> }
So "match" appears to be a glob (as opposed to a regex). And order is
important (the first rule matched ends the lookup), so you correctly
used an array for the list of rules (a dict does not have to maintain
order).
>
> Or via the -object command line
>
> $QEMU \
> -object authz-simple,id=acl0,policy=deny,\
> match.0.name=fred,match.0.policy=allow, \
> match.1.name=bob,match.1.policy=allow, \
> match.2.name=danb,match.2.policy=deny, \
> match.3.name=dan*,match.3.policy=allow
The '*' in the command line will require shell quoting.
>
> This sets up an authorization rule that allows 'fred',
> 'bob' and anyone whose name starts with 'dan', except
> for 'danb'. Everyone unmatched is denied.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> +/**
> + * QAuthZSimple:
> + *
> + * This authorization driver provides a simple mechanism
> + * for granting access by matching user names against a
> + * list of globs. Each match rule has an associated policy
> + * and a catch all policy applies if no rule matches
> + *
> + * To create an instace of this class via QMP:
s/instace/instance/
> + *
> + * Or via the CLI:
> + *
> + * $QEMU \
> + * -object authz-simple,id=acl0,policy=deny, \
> + * match.0.name=fred,match.0.policy=allow, \
> + * match.1.name=bob,match.1.policy=allow, \
> + * match.2.name=danb,match.2.policy=deny, \
> + * match.3.name=dan*,match.3.policy=allow
Again, quoting the "*" is important, and maybe a comment that the
whitespace is for display purposes but must be omitted on the command line.
> +++ b/qapi-schema.json
> @@ -5,6 +5,9 @@
> # QAPI common definitions
> { 'include': 'qapi/common.json' }
>
> +# QAPI util definitions
> +{ 'include': 'qapi/util.json' }
> +
> # QAPI crypto definitions
> { 'include': 'qapi/crypto.json' }
>
> @@ -3652,7 +3655,8 @@
> # Since 2.5
> ##
> { 'struct': 'DummyForceArrays',
> - 'data': { 'unused': ['X86CPUFeatureWordInfo'] } }
> + 'data': { 'unused': ['X86CPUFeatureWordInfo'],
> + 'iamalsounused': ['QAuthZSimpleRule'] } }
Cute. I might have renamed things and gone 'unused1' and 'unused2'.
> +++ b/qapi/util.json
> @@ -0,0 +1,31 @@
> +# -*- Mode: Python -*-
> +#
> +# QAPI util definitions
> +
> +##
> +# QAuthZSimplePolicy:
> +#
> +# The authorization policy result
> +#
> +# @deny: deny access
> +# @allow: allow access
> +#
> +# Since: 2.6
> +##
> +{ 'enum': 'QAuthZSimplePolicy',
> + 'prefix': 'QAUTHZ_SIMPLE_POLICY',
> + 'data': ['deny', 'allow']}
I know Markus isn't a big fan of 'prefix', but I don't mind it.
We're awfully late in the 2.6 cycle; this feels like a feature addition
rather than a bug fix, so should it be 2.7?
> +
> +##
> +# QAuthZSimpleRule:
> +#
> +# A single authorization rule.
> +#
> +# @match: a glob to match against a user identity
> +# @policy: the result to return if @match evaluates to true
> +#
> +# Since: 2.6
> +##
> +{ 'struct': 'QAuthZSimpleRule',
> + 'data': {'match': 'str',
> + 'policy': 'QAuthZSimplePolicy'}}
Hmm. I was expecting something like:
{ 'struct': 'QAuthZSimple',
'data': { 'rules': [ 'QAuthZSimpleRule' ],
'policy': 'QAuthZSimplePolicy' } }
but I guess that's one of our short-comings of QOM: the top-level
structure does not have to be specified anywhere in QAPI.
> +++ b/tests/test-authz-simple.c
> @@ -0,0 +1,156 @@
> +/*
> + * QEMU simple authorization object
> + *
> + * Copyright (c) 2016 Red Hat, Inc.
> + *
> +static void test_authz_default_deny(void)
> +{
> + QAuthZSimple *auth = qauthz_simple_new("auth0",
> + QAUTHZ_SIMPLE_POLICY_DENY,
> + &error_abort);
> +
> + g_assert(!qauthz_is_allowed(QAUTHZ(auth), "fred", &error_abort));
> +
Okay, so you definitely intend to return false without setting errp, so
it is a ternary result.
> +
> +#ifdef CONFIG_FNMATCH
> +static void test_authz_complex(void)
> +{
Wait - the behavior depends on whether fnmatch() is available? That is,
a name is a literal match if fnmatch() is not present, and glob if
present? I'd argue that if fnmatch() is not present, we must explicitly
reject any "match" with glob metacharacters, rather than accidentally
matching only a literal "*" when a glob was intended.
> +++ b/util/authz-simple.c
> +
> +/* If no fnmatch, fallback to exact string matching
> + * instead of allowing wildcards */
> +#ifdef CONFIG_FNMATCH
> +#include <fnmatch.h>
> +#define qauthz_match(pattern, string) fnmatch(pattern, string, 0)
> +#else
> +#define qauthz_match(pattern, string) strcmp(pattern, string)
> +#endif
As mentioned above, I'd be more comfortable if you also explicitly
reject any attempt to add a pattern that resembles a glob when globbing
is not enabled. Or maybe it's worth a more complex QAPI definition:
# How to interpret 'match', as a literal string or a glob
{ 'enum': 'QAuthZSimpleStyle', 'data': [ 'literal', 'glob' ] }
# @style: #optional, default to 'literal'
{ 'struct': 'QAuthZSimpleRule',
'data': { 'match': 'str', '*style': 'QAuthZSimpleStyle',
'policy': 'QAuthZSimplePolicy' } }
and used as:
"rules": [
{ "match": "fred", "policy": "allow" },
{ "match": "bob", "policy": "allow" },
{ "match": "danb", "policy": "deny" },
{ "match": "dan*", "policy": "allow", "style": "glob" }
where you can then gracefully error out for ALL attempts to use
"style":"glob" if fnmatch() is not present, and use strcmp() even when
fnmatch() is present for rules that have the (default) "style":"literal".
> +static void
> +qauthz_simple_class_init(ObjectClass *oc, void *data)
> +{
> + UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
> + QAuthZClass *authz = QAUTHZ_CLASS(oc);
> +
> + ucc->complete = qauthz_simple_complete;
> + authz->is_allowed = qauthz_simple_is_allowed;
> +
> + object_class_property_add_enum(oc, "policy",
> + "QAuthZSimplePolicy",
> + QAuthZSimplePolicy_lookup,
> + qauthz_simple_prop_get_policy,
> + qauthz_simple_prop_set_policy,
> + NULL);
> +
> + object_class_property_add(oc, "rules", "QAuthZSimpleRule",
> + qauthz_simple_prop_get_rules,
> + qauthz_simple_prop_set_rules,
> + NULL, NULL, NULL);
> +}
Not for this patch, but it would be cool if we had enough framework to
just tell QOM to instantiate an object with callbacks by merely pointing
to the name of a QAPI type that the object implements (referring back to
my comment that I'm a bit surprised we didn't need a QAPI type for the
overall QAuthZSimple).
> +
> +size_t qauthz_simple_append_rule(QAuthZSimple *auth, const char *match,
> + QAuthZSimplePolicy policy)
> +{
> + QAuthZSimpleRule *rule;
> + QAuthZSimpleRuleList *rules, *tmp;
> + size_t i = 0;
> +
> + rule = g_new0(QAuthZSimpleRule, 1);
> + rule->policy = policy;
> + rule->match = g_strdup(match);
> +
> + tmp = g_new0(QAuthZSimpleRuleList, 1);
> + tmp->value = rule;
> +
> + rules = auth->rules;
> + if (rules) {
> + while (rules->next) {
> + i++;
> + rules = rules->next;
> + }
> + rules->next = tmp;
> + return i + 1;
No checks whether 'match' is already an existing rule...
> +ssize_t qauthz_simple_delete_rule(QAuthZSimple *auth, const char *match)
> +{
> + QAuthZSimpleRule *rule;
> + QAuthZSimpleRuleList *rules, *prev;
> + size_t i = 0;
> +
> + prev = NULL;
> + rules = auth->rules;
> + while (rules) {
> + rule = rules->value;
> + if (g_str_equal(rule->match, match)) {
> + if (prev) {
> + prev->next = rules->next;
> + } else {
> + auth->rules = rules->next;
> + }
> + rules->next = NULL;
> + qapi_free_QAuthZSimpleRuleList(rules);
> + return i;
...which means a rule can be inserted more than once, and this only
removes the first instance of the rule. Do we care enough to add in
additional checking that we aren't permitting duplicate 'match' strings
in the list of rules?
If you add the style=literal/glob to a rule, would you also want to be
able to constrain deletion to a particular style (delete the glob 'dan*'
but leave the literal 'dan*' intact)?
--
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] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 06/10] acl: delete existing ACL implementation
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 06/10] acl: delete existing ACL implementation Daniel P. Berrange
@ 2016-03-22 17:58 ` Eric Blake
0 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2016-03-22 17:58 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Paolo Bonzini, qemu-block, Markus Armbruster,
Andreas Färber, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 838 bytes --]
On 03/10/2016 11:59 AM, Daniel P. Berrange wrote:
> The 'qemu_acl' type was a previous non-QOM based attempt to
> provide an authorization facility in QEMU. Because it is
> non-QOM based it cannot be created via the command line and
> requires special monitor commands to manipulate it.
>
> The new QAuthZ and QAuthZSimple QOM classes provide a superset
> of the functionality in qemu_acl, so the latter can now be
> deleted. The HMP 'acl_*' monitor commands are converted to
> use the new QAuthZSimple data type instead in order to provide
> backwards compatibility, but their use is discouraged.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
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] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 07/10] qemu-nbd: add support for ACLs for TLS clients
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 07/10] qemu-nbd: add support for ACLs for TLS clients Daniel P. Berrange
@ 2016-03-22 18:14 ` Eric Blake
2016-03-23 12:40 ` Daniel P. Berrange
0 siblings, 1 reply; 32+ messages in thread
From: Eric Blake @ 2016-03-22 18:14 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Paolo Bonzini, qemu-block, Markus Armbruster,
Andreas Färber, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 2227 bytes --]
On 03/10/2016 11:59 AM, Daniel P. Berrange wrote:
> Currently any client which can complete the TLS handshake
> is able to use the NBD server. The server admin can turn
> on the 'verify-peer' option for the x509 creds to require
> the client to provide a x509 certificate. This means the
> client will have to acquire a certificate from the CA before
> they are permitted to use the NBD server. This is still a
> fairly weak bar.
>
> This adds a '--tls-acl ACL-ID' option to the qemu-nbd command
> which takes the ID of a previously added 'QAuthZ' object
> instance. This ACL will be used to validate the client's
> x509 distinguished name. Clients failing the ACL will not be
> permitted to use the NBD server.
>
> For example to setup an ACL that only allows connection from
> a client whose x509 certificate distinguished name contains
> 'CN=fred', you would use:
>
> qemu-nbd -object tls-creds-x509,id=tls0,dir=/home/berrange/qemutls,\
> endpoint=server,verify-peer=yes \
> -object authz-simple,id=acl0,policy=deny,\
> rules.0.match=*CN=fred,rules.0.policy=allow \
> -tls-creds tls0 \
> -tls-acl acl0
> ....other qemu-nbd args...
Ah, so you are arguing that this is feature-completion of work started
in 2.6, continuing work started before soft-freeze, and not a new
feature to be delayed to 2.7.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> qemu-nbd.c | 13 ++++++++++++-
> qemu-nbd.texi | 4 ++++
> 2 files changed, 16 insertions(+), 1 deletion(-)
>
> +++ b/qemu-nbd.texi
> @@ -86,6 +86,10 @@ the new style NBD protocol negotiation
> Enable mandatory TLS encryption for the server by setting the ID
> of the TLS credentials object previously created with the --object
> option.
> +@item --tls-acl=ID
> +Specify the ID of a qauthz object previously created with the
> +--object option. This will be used to authorize users who
> +connect against their x509 distinguish name.
s/distinguish/distinguished/
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] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 08/10] nbd: allow an ACL to be set with nbd-server-start QMP command
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 08/10] nbd: allow an ACL to be set with nbd-server-start QMP command Daniel P. Berrange
@ 2016-03-22 18:19 ` Eric Blake
0 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2016-03-22 18:19 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Paolo Bonzini, qemu-block, Markus Armbruster,
Andreas Färber, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 1113 bytes --]
On 03/10/2016 11:59 AM, Daniel P. Berrange wrote:
> As with the previous patch to qemu-nbd, the nbd-server-start
> QMP command also needs to be able to specify an ACL when
> enabling TLS encryption.
>
> First the client must create a QAuthZ object instance using
> the 'object-add' command:
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> +++ b/qapi/block.json
> @@ -147,6 +147,7 @@
> #
> # @addr: Address on which to listen.
> # @tls-creds: (optional) ID of the TLS credentials object. Since 2.6
> +# @tls-acl: (optional) ID of the QAuthZ authorization object. Since 2.6
> #
> # Returns: error if the server is already running.
> #
> @@ -154,7 +155,8 @@
> ##
> { 'command': 'nbd-server-start',
> 'data': { 'addr': 'SocketAddress',
> - '*tls-creds': 'str'} }
> + '*tls-creds': 'str',
> + '*tls-acl': 'str'} }
>
Interface change is deceptively simple :)
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] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 09/10] chardev: add support for ACLs for TLS clients
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 09/10] chardev: add support for ACLs for TLS clients Daniel P. Berrange
@ 2016-03-22 21:26 ` Eric Blake
0 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2016-03-22 21:26 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Paolo Bonzini, qemu-block, Markus Armbruster,
Andreas Färber, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 2348 bytes --]
On 03/10/2016 11:59 AM, Daniel P. Berrange wrote:
> Currently any client which can complete the TLS handshake
> is able to use a chardev server. The server admin can turn
> on the 'verify-peer' option for the x509 creds to require
> the client to provide a x509 certificate. This means the
> client will have to acquire a certificate from the CA before
> they are permitted to use the chardev server. This is still
> a fairly weak bar.
>
> This adds a 'tls-acl=ACL-ID' option to the socket chardev
> backend which takes the ID of a previously added 'QAuthZ'
> object instance. This ACL will be used to validate the client's
> x509 distinguished name. Clients failing the ACL will not be
> permitted to use the chardev server.
>
> For example to setup an ACL that only allows connection from
> a client whose x509 certificate distinguished name contains
> 'CN=fred', you would use:
>
> $QEMU -object tls-creds-x509,id=tls0,dir=/home/berrange/qemutls,\
> endpoint=server,verify-peer=yes \
> -object authz-simple,id=acl0,policy=deny,\
> rules.0.match=*CN=fred,rules.0.policy=allow \
Needs shell quoting for *, and also the same recurring comment about
whitespace for presentation not actually being in the command line.
Food for thought: should we enhance QemuOpts to skip all whitespace
after ',', since we _know_ that valid key names start with a letter
rather than a space? Then, we could represent command lines as:
$QEMU -object 'name,
param1=value,
param2=value'
with the same semantics as:
$QEMU -object name,param1=value,param2=value
and without having to worry about backslash-newline-whitespace
formatting. Obviously, such an enhancement would be a separate patch.
> -chardev socket,host=127.0.0.1,port=9000,server,\
> tls-creds=tls0,tls-acl=acl0 \
> ...other qemud args...
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> qapi-schema.json | 2 ++
> qemu-char.c | 11 ++++++++++-
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
Code is fine; my only comments were on the commit message.
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] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 10/10] vnc: allow specifying a custom ACL object name
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 10/10] vnc: allow specifying a custom ACL object name Daniel P. Berrange
@ 2016-03-22 21:38 ` Eric Blake
2016-03-23 12:43 ` Daniel P. Berrange
0 siblings, 1 reply; 32+ messages in thread
From: Eric Blake @ 2016-03-22 21:38 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: Paolo Bonzini, qemu-block, Markus Armbruster,
Andreas Färber, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 5342 bytes --]
On 03/10/2016 11:59 AM, Daniel P. Berrange wrote:
> The VNC server has historically had support for ACLs to check
> both the SASL username and the TLS x509 distinguished name.
> The VNC server was responsible for creating the initial ACL,
> and the client app was then responsible for populating it with
> rules using the HMP 'acl_add' command.
>
> This is not satisfactory for a variety of reasons. There is
> no way to populate the ACLs from the command line, users are
> forced to use the HMP. With multiple network services all
> supporting TLS and ACLs now, it is desirable to be able to
> define a single ACL that is referenced by all services.
>
> To address these limitations, two new options are added to the
> VNC server CLI. The 'tls-acl' option takes the ID of a QAuthZ
> object to use for checking TLS x509 distinguished names, and
> the 'sasl-acl' option takes the ID of another object to use for
> checking SASL usernames.
>
> In this example, we setup two ACLs. The first allows any client
> with a certificate issued by the 'RedHat' organization in the
> 'London' locality. The second ACL allows clients with either
> the 'joe@REDHAT.COM' or 'fred@REDHAT.COM' kerberos usernames.
> Both ACLs must pass for the user to be allowed.
>
> $QEMU -object tls-creds-x509,id=tls0,dir=/home/berrange/qemutls,\
> endpoint=server,verify-peer=yes \
> -object authz-simple,id=acl0,policy=deny,\
> rules.0.match=O=RedHat,,L=London,rules.0.policy=allow \
> -object authz-simple,id=acl0,policy=deny,\
Umm, you can't reuse 'acl0' as the id.
> rules.0.match=fred@REDHAT.COM,rules.0.policy=allow \
> rules.0.match=joe@REDHAT.COM,rules.0.policy=allow \
> -vnc 0.0.0.0:1,tls-creds=tls0,tls-acl=tlsacl0,
> sasl,sasl-acl=saslacl0 \
And this fails because the ids don't exist. I think you meant
authz-simple,id=tlsacl0 in the first instance, and
authz-simple,id=saslacl0 in the second instance.
> ...other QEMU args...
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> ui/vnc.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 60 insertions(+), 13 deletions(-)
>
> @@ -3670,6 +3680,21 @@ void vnc_display_open(const char *id, Error **errp)
> }
> }
> acl = qemu_opt_get_bool(opts, "acl", false);
> + tlsacl = qemu_opt_get(opts, "tls-acl");
> + if (acl && tlsacl) {
> + error_setg(errp, "'acl' option is mutually exclusive with the "
> + "'tls-acl' options");
> + goto fail;
> + }
> +
> +#ifdef CONFIG_VNC_SASL
> + saslacl = qemu_opt_get(opts, "sasl-acl");
> + if (acl && saslacl) {
> + error_setg(errp, "'acl' option is mutually exclusive with the "
> + "'sasl-acl' options");
> + goto fail;
> + }
> +#endif
Do we explicitly fail if sasl-acl was provided but CONFIG_VNC_SASL is
not defined? It looks here like you silently ignore it, which would not
be good.
> @@ -3710,19 +3737,39 @@ void vnc_display_open(const char *id, Error **errp)
> &error_abort);
> }
> #ifdef CONFIG_VNC_SASL
> - if (acl && sasl) {
> - char *aclname;
> + if (sasl) {
> + if (saslacl) {
> + Object *container, *acl;
> + container = object_get_objects_root();
> + acl = object_resolve_path_component(container, saslacl);
> + if (!acl) {
> + error_setg(errp, "Cannot find ACL %s", saslacl);
> + goto fail;
> + }
>
> - if (strcmp(vs->id, "default") == 0) {
> - aclname = g_strdup("vnc.username");
> - } else {
> - aclname = g_strdup_printf("vnc.%s.username", vs->id);
> - }
> - vs->sasl.acl =
> - QAUTHZ(qauthz_simple_new(aclname,
> - QAUTHZ_SIMPLE_POLICY_DENY,
> - &error_abort));
> - g_free(aclname);
> + if (!object_dynamic_cast(acl, TYPE_QAUTHZ)) {
> + error_setg(errp, "Object '%s' is not a QAuthZ subclass",
> + saslacl);
> + goto fail;
> + }
> + vs->sasl.acl = QAUTHZ(acl);
> + } else if (acl) {
> + char *aclname;
> +
> + if (strcmp(vs->id, "default") == 0) {
> + aclname = g_strdup("vnc.username");
> + } else {
> + aclname = g_strdup_printf("vnc.%s.username", vs->id);
> + }
> + vs->sasl.acl =
> + QAUTHZ(qauthz_simple_new(aclname,
> + QAUTHZ_SIMPLE_POLICY_DENY,
> + &error_abort));
> + g_free(aclname);
> + }
> + } else if (saslacl) {
> + error_setg(errp, "SASL ACL provided when SASL is disabled");
> + goto fail;
> }
> #endif
>
Again, the saslacl check is only mentioned inside the #if; what happens
when the #if is not compiled?
--
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] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 05/10] util: add QAuthZSimple object type for a simple access control list
2016-03-22 17:38 ` Eric Blake
@ 2016-03-23 12:38 ` Daniel P. Berrange
0 siblings, 0 replies; 32+ messages in thread
From: Daniel P. Berrange @ 2016-03-23 12:38 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-block, Markus Armbruster, qemu-devel, Paolo Bonzini,
Max Reitz, Andreas Färber
On Tue, Mar 22, 2016 at 11:38:53AM -0600, Eric Blake wrote:
> On 03/10/2016 11:59 AM, Daniel P. Berrange wrote:
> > Add a QAuthZSimple object type that implements the QAuthZ
> > interface. This simple built-in implementation maintains
> > a trivial access control list with a sequence of match
> > rules and a final default policy. This replicates the
> > functionality currently provided by the qemu_acl module.
> >
> > To create an instance of this object via the QMP monitor,
> > the syntax used would be
> >
> > {
> > "execute": "object-add",
> > "arguments": {
> > "qom-type": "authz-simple",
> > "id": "auth0",
> > "parameters": {
> > "rules": [
> > { "match": "fred", "policy": "allow" },
> > { "match": "bob", "policy": "allow" },
> > { "match": "danb", "policy": "deny" },
> > { "match": "dan*", "policy": "allow" }
> > ],
> > "policy": "deny"
> > }
> > }
> > }
>
> So "match" appears to be a glob (as opposed to a regex). And order is
> important (the first rule matched ends the lookup), so you correctly
> used an array for the list of rules (a dict does not have to maintain
> order).
Yes, its a glob (as defined by fnmatch)
> >
> > Or via the -object command line
> >
> > $QEMU \
> > -object authz-simple,id=acl0,policy=deny,\
> > match.0.name=fred,match.0.policy=allow, \
> > match.1.name=bob,match.1.policy=allow, \
> > match.2.name=danb,match.2.policy=deny, \
> > match.3.name=dan*,match.3.policy=allow
>
> The '*' in the command line will require shell quoting.
Yeah, CLI syntax escaping from shell is horrible, but fortunately
libvirt doesn't use shell :-) None the less I'll update the
example.
> > +##
> > +# QAuthZSimplePolicy:
> > +#
> > +# The authorization policy result
> > +#
> > +# @deny: deny access
> > +# @allow: allow access
> > +#
> > +# Since: 2.6
> > +##
> > +{ 'enum': 'QAuthZSimplePolicy',
> > + 'prefix': 'QAUTHZ_SIMPLE_POLICY',
> > + 'data': ['deny', 'allow']}
>
> I know Markus isn't a big fan of 'prefix', but I don't mind it.
It doesn't affect public API anyway, so we can change it at will
later if desired.
> We're awfully late in the 2.6 cycle; this feels like a feature addition
> rather than a bug fix, so should it be 2.7?
It is definitely a feature addition. I was hoping to get it into 2.6
since it is logically associated with the TLS enhancements I've been
doing. It isn't the end of the world if it waits until 2.7 though,
so I'm open minded either way.
Basically TLS provides encryption, x509 certs provide authentication,
and this ACL stuff provides the authorization piece of the puzzel.
If we miss the authorization piece in 2.6 we're still in a far better
position than we were historically because we at least have encryption
and authentication still :-) You can mitigate the lack of authorization
by having a dedicated CA certificate just for QEMU services, so that
you limit who has access to client certs. This is a crude authorization
setup that is none the less acceptable in many scenarios.
> > +##
> > +# QAuthZSimpleRule:
> > +#
> > +# A single authorization rule.
> > +#
> > +# @match: a glob to match against a user identity
> > +# @policy: the result to return if @match evaluates to true
> > +#
> > +# Since: 2.6
> > +##
> > +{ 'struct': 'QAuthZSimpleRule',
> > + 'data': {'match': 'str',
> > + 'policy': 'QAuthZSimplePolicy'}}
>
> Hmm. I was expecting something like:
>
> { 'struct': 'QAuthZSimple',
> 'data': { 'rules': [ 'QAuthZSimpleRule' ],
> 'policy': 'QAuthZSimplePolicy' } }
>
> but I guess that's one of our short-comings of QOM: the top-level
> structure does not have to be specified anywhere in QAPI.
I'm not sure I'd call it a short-coming but rather its just a difference
in approach. For anything that is a user-defined object, the QAPI schema
is defined implicitly by the QOM object or class definition. With the
ability to define QOM properties against the class instad of object,
we are actally in a position where we could auto-generate a QAPI schema
to represent each user-defined object type if desired. Alternatively
we could equally extend QAPI to have an 'object' type (which is a
specialization of the 'struct' type) and auto-generate the basic
boilerplate code to define a QOM object class from it.
> > +static void test_authz_default_deny(void)
> > +{
> > + QAuthZSimple *auth = qauthz_simple_new("auth0",
> > + QAUTHZ_SIMPLE_POLICY_DENY,
> > + &error_abort);
> > +
> > + g_assert(!qauthz_is_allowed(QAUTHZ(auth), "fred", &error_abort));
> > +
>
> Okay, so you definitely intend to return false without setting errp, so
> it is a ternary result.
Yep
> > +#ifdef CONFIG_FNMATCH
> > +static void test_authz_complex(void)
> > +{
>
> Wait - the behavior depends on whether fnmatch() is available? That is,
> a name is a literal match if fnmatch() is not present, and glob if
> present? I'd argue that if fnmatch() is not present, we must explicitly
> reject any "match" with glob metacharacters, rather than accidentally
> matching only a literal "*" when a glob was intended.
Historically we have the qemu/acl.c code which uses fnmatch and falls
back to plain strcmp if not available. Since this was intended to be an
exact drop-in replacement, I tried to implement the same logic here.
We could do something slightly different though - define a property
against the QAuthzSimple object which enumerates the type of matching
scheme - exact, glob or regex. The acl.c compatibility layer could
then simply set matchtype=exact when !CONFIG_FNMATCH, which would
in turn allow the QAuthzSimple impl to raise a fatall error for use
of globbing when fnmatch is missing.
> # How to interpret 'match', as a literal string or a glob
> { 'enum': 'QAuthZSimpleStyle', 'data': [ 'literal', 'glob' ] }
> # @style: #optional, default to 'literal'
> { 'struct': 'QAuthZSimpleRule',
> 'data': { 'match': 'str', '*style': 'QAuthZSimpleStyle',
> 'policy': 'QAuthZSimplePolicy' } }
>
> and used as:
>
> "rules": [
> { "match": "fred", "policy": "allow" },
> { "match": "bob", "policy": "allow" },
> { "match": "danb", "policy": "deny" },
> { "match": "dan*", "policy": "allow", "style": "glob" }
>
> where you can then gracefully error out for ALL attempts to use
> "style":"glob" if fnmatch() is not present, and use strcmp() even when
> fnmatch() is present for rules that have the (default) "style":"literal".
Yep, allowing style to be set per rule is another option - I wonder
if it is better todo it per-rule or per-ACL applying to all rules.
> > +static void
> > +qauthz_simple_class_init(ObjectClass *oc, void *data)
> > +{
> > + UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
> > + QAuthZClass *authz = QAUTHZ_CLASS(oc);
> > +
> > + ucc->complete = qauthz_simple_complete;
> > + authz->is_allowed = qauthz_simple_is_allowed;
> > +
> > + object_class_property_add_enum(oc, "policy",
> > + "QAuthZSimplePolicy",
> > + QAuthZSimplePolicy_lookup,
> > + qauthz_simple_prop_get_policy,
> > + qauthz_simple_prop_set_policy,
> > + NULL);
> > +
> > + object_class_property_add(oc, "rules", "QAuthZSimpleRule",
> > + qauthz_simple_prop_get_rules,
> > + qauthz_simple_prop_set_rules,
> > + NULL, NULL, NULL);
> > +}
>
> Not for this patch, but it would be cool if we had enough framework to
> just tell QOM to instantiate an object with callbacks by merely pointing
> to the name of a QAPI type that the object implements (referring back to
> my comment that I'm a bit surprised we didn't need a QAPI type for the
> overall QAuthZSimple).
Yep, as mentioned above this is just the difference between QAPI
and QOM user creatable objects that needs resolving somehow.
> > +size_t qauthz_simple_append_rule(QAuthZSimple *auth, const char *match,
> > + QAuthZSimplePolicy policy)
> > +{
> > + QAuthZSimpleRule *rule;
> > + QAuthZSimpleRuleList *rules, *tmp;
> > + size_t i = 0;
> > +
> > + rule = g_new0(QAuthZSimpleRule, 1);
> > + rule->policy = policy;
> > + rule->match = g_strdup(match);
> > +
> > + tmp = g_new0(QAuthZSimpleRuleList, 1);
> > + tmp->value = rule;
> > +
> > + rules = auth->rules;
> > + if (rules) {
> > + while (rules->next) {
> > + i++;
> > + rules = rules->next;
> > + }
> > + rules->next = tmp;
> > + return i + 1;
>
> No checks whether 'match' is already an existing rule...
>
>
> > +ssize_t qauthz_simple_delete_rule(QAuthZSimple *auth, const char *match)
> > +{
> > + QAuthZSimpleRule *rule;
> > + QAuthZSimpleRuleList *rules, *prev;
> > + size_t i = 0;
> > +
> > + prev = NULL;
> > + rules = auth->rules;
> > + while (rules) {
> > + rule = rules->value;
> > + if (g_str_equal(rule->match, match)) {
> > + if (prev) {
> > + prev->next = rules->next;
> > + } else {
> > + auth->rules = rules->next;
> > + }
> > + rules->next = NULL;
> > + qapi_free_QAuthZSimpleRuleList(rules);
> > + return i;
>
> ...which means a rule can be inserted more than once, and this only
> removes the first instance of the rule. Do we care enough to add in
> additional checking that we aren't permitting duplicate 'match' strings
> in the list of rules?
I don't think it really matters much - if you add 2 rules with the
same string, you still need 2 calls to delete it. The order of
deletion can be left undefined I think.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 07/10] qemu-nbd: add support for ACLs for TLS clients
2016-03-22 18:14 ` Eric Blake
@ 2016-03-23 12:40 ` Daniel P. Berrange
0 siblings, 0 replies; 32+ messages in thread
From: Daniel P. Berrange @ 2016-03-23 12:40 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-block, Markus Armbruster, qemu-devel, Paolo Bonzini,
Max Reitz, Andreas Färber
On Tue, Mar 22, 2016 at 12:14:27PM -0600, Eric Blake wrote:
> On 03/10/2016 11:59 AM, Daniel P. Berrange wrote:
> > Currently any client which can complete the TLS handshake
> > is able to use the NBD server. The server admin can turn
> > on the 'verify-peer' option for the x509 creds to require
> > the client to provide a x509 certificate. This means the
> > client will have to acquire a certificate from the CA before
> > they are permitted to use the NBD server. This is still a
> > fairly weak bar.
> >
> > This adds a '--tls-acl ACL-ID' option to the qemu-nbd command
> > which takes the ID of a previously added 'QAuthZ' object
> > instance. This ACL will be used to validate the client's
> > x509 distinguished name. Clients failing the ACL will not be
> > permitted to use the NBD server.
> >
> > For example to setup an ACL that only allows connection from
> > a client whose x509 certificate distinguished name contains
> > 'CN=fred', you would use:
> >
> > qemu-nbd -object tls-creds-x509,id=tls0,dir=/home/berrange/qemutls,\
> > endpoint=server,verify-peer=yes \
> > -object authz-simple,id=acl0,policy=deny,\
> > rules.0.match=*CN=fred,rules.0.policy=allow \
> > -tls-creds tls0 \
> > -tls-acl acl0
> > ....other qemu-nbd args...
>
> Ah, so you are arguing that this is feature-completion of work started
> in 2.6, continuing work started before soft-freeze, and not a new
> feature to be delayed to 2.7.
Yes and v1 of this patch series was in fact posted just a few days
before the soft-freeze deadline.
That said, as mentioned in the earlier patch, I'm open minded about
whether this goes in 2.6 or 2.7. It would be nice to have in 2.6
but not the end of the world if it misses it, as overall we're still
waaaaaaaay better off compared to 2.5 even if this doesn't mege :-)
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH v3 10/10] vnc: allow specifying a custom ACL object name
2016-03-22 21:38 ` Eric Blake
@ 2016-03-23 12:43 ` Daniel P. Berrange
0 siblings, 0 replies; 32+ messages in thread
From: Daniel P. Berrange @ 2016-03-23 12:43 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-block, Markus Armbruster, qemu-devel, Paolo Bonzini,
Max Reitz, Andreas Färber
On Tue, Mar 22, 2016 at 03:38:14PM -0600, Eric Blake wrote:
> On 03/10/2016 11:59 AM, Daniel P. Berrange wrote:
> > The VNC server has historically had support for ACLs to check
> > both the SASL username and the TLS x509 distinguished name.
> > The VNC server was responsible for creating the initial ACL,
> > and the client app was then responsible for populating it with
> > rules using the HMP 'acl_add' command.
> >
> > This is not satisfactory for a variety of reasons. There is
> > no way to populate the ACLs from the command line, users are
> > forced to use the HMP. With multiple network services all
> > supporting TLS and ACLs now, it is desirable to be able to
> > define a single ACL that is referenced by all services.
> >
> > To address these limitations, two new options are added to the
> > VNC server CLI. The 'tls-acl' option takes the ID of a QAuthZ
> > object to use for checking TLS x509 distinguished names, and
> > the 'sasl-acl' option takes the ID of another object to use for
> > checking SASL usernames.
> >
> > In this example, we setup two ACLs. The first allows any client
> > with a certificate issued by the 'RedHat' organization in the
> > 'London' locality. The second ACL allows clients with either
> > the 'joe@REDHAT.COM' or 'fred@REDHAT.COM' kerberos usernames.
> > Both ACLs must pass for the user to be allowed.
> >
> > $QEMU -object tls-creds-x509,id=tls0,dir=/home/berrange/qemutls,\
> > endpoint=server,verify-peer=yes \
> > -object authz-simple,id=acl0,policy=deny,\
> > rules.0.match=O=RedHat,,L=London,rules.0.policy=allow \
> > -object authz-simple,id=acl0,policy=deny,\
>
> Umm, you can't reuse 'acl0' as the id.
>
> > rules.0.match=fred@REDHAT.COM,rules.0.policy=allow \
> > rules.0.match=joe@REDHAT.COM,rules.0.policy=allow \
> > -vnc 0.0.0.0:1,tls-creds=tls0,tls-acl=tlsacl0,
> > sasl,sasl-acl=saslacl0 \
>
> And this fails because the ids don't exist. I think you meant
> authz-simple,id=tlsacl0 in the first instance, and
> authz-simple,id=saslacl0 in the second instance.
Heh, yeah, I really ought to try the examples I put in the commit
message tomake sure they work :-)
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> > ui/vnc.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++------------
> > 1 file changed, 60 insertions(+), 13 deletions(-)
> >
> > @@ -3670,6 +3680,21 @@ void vnc_display_open(const char *id, Error **errp)
> > }
> > }
> > acl = qemu_opt_get_bool(opts, "acl", false);
> > + tlsacl = qemu_opt_get(opts, "tls-acl");
> > + if (acl && tlsacl) {
> > + error_setg(errp, "'acl' option is mutually exclusive with the "
> > + "'tls-acl' options");
> > + goto fail;
> > + }
> > +
> > +#ifdef CONFIG_VNC_SASL
> > + saslacl = qemu_opt_get(opts, "sasl-acl");
> > + if (acl && saslacl) {
> > + error_setg(errp, "'acl' option is mutually exclusive with the "
> > + "'sasl-acl' options");
> > + goto fail;
> > + }
> > +#endif
>
> Do we explicitly fail if sasl-acl was provided but CONFIG_VNC_SASL is
> not defined? It looks here like you silently ignore it, which would not
> be good.
Yes, we should really raise the error unconditionally.
> > @@ -3710,19 +3737,39 @@ void vnc_display_open(const char *id, Error **errp)
> > &error_abort);
> > }
> > #ifdef CONFIG_VNC_SASL
> > - if (acl && sasl) {
> > - char *aclname;
> > + if (sasl) {
> > + if (saslacl) {
> > + Object *container, *acl;
> > + container = object_get_objects_root();
> > + acl = object_resolve_path_component(container, saslacl);
> > + if (!acl) {
> > + error_setg(errp, "Cannot find ACL %s", saslacl);
> > + goto fail;
> > + }
> >
> > - if (strcmp(vs->id, "default") == 0) {
> > - aclname = g_strdup("vnc.username");
> > - } else {
> > - aclname = g_strdup_printf("vnc.%s.username", vs->id);
> > - }
> > - vs->sasl.acl =
> > - QAUTHZ(qauthz_simple_new(aclname,
> > - QAUTHZ_SIMPLE_POLICY_DENY,
> > - &error_abort));
> > - g_free(aclname);
> > + if (!object_dynamic_cast(acl, TYPE_QAUTHZ)) {
> > + error_setg(errp, "Object '%s' is not a QAuthZ subclass",
> > + saslacl);
> > + goto fail;
> > + }
> > + vs->sasl.acl = QAUTHZ(acl);
> > + } else if (acl) {
> > + char *aclname;
> > +
> > + if (strcmp(vs->id, "default") == 0) {
> > + aclname = g_strdup("vnc.username");
> > + } else {
> > + aclname = g_strdup_printf("vnc.%s.username", vs->id);
> > + }
> > + vs->sasl.acl =
> > + QAUTHZ(qauthz_simple_new(aclname,
> > + QAUTHZ_SIMPLE_POLICY_DENY,
> > + &error_abort));
> > + g_free(aclname);
> > + }
> > + } else if (saslacl) {
> > + error_setg(errp, "SASL ACL provided when SASL is disabled");
> > + goto fail;
> > }
> > #endif
> >
>
> Again, the saslacl check is only mentioned inside the #if; what happens
> when the #if is not compiled?
Yeah, I should fix that.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2016-03-23 12:43 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-10 18:51 [Qemu-devel] [PATCH v3 00/10] Provide a QOM-based authorization API Daniel P. Berrange
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 01/10] qdict: implement a qdict_crumple method for un-flattening a dict Daniel P. Berrange
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 02/10] qapi: allow QmpInputVisitor to auto-cast types Daniel P. Berrange
2016-03-21 23:18 ` Eric Blake
2016-03-22 15:49 ` Daniel P. Berrange
2016-03-22 16:20 ` Eric Blake
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 03/10] qom: support arbitrary non-scalar properties with -object Daniel P. Berrange
2016-03-21 23:27 ` Eric Blake
2016-03-22 9:07 ` Markus Armbruster
2016-03-22 10:34 ` Daniel P. Berrange
2016-03-22 15:51 ` Daniel P. Berrange
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 04/10] util: add QAuthZ object as an authorization base class Daniel P. Berrange
2016-03-22 16:33 ` Eric Blake
2016-03-22 16:43 ` Daniel P. Berrange
2016-03-22 16:44 ` Daniel P. Berrange
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 05/10] util: add QAuthZSimple object type for a simple access control list Daniel P. Berrange
2016-03-22 17:38 ` Eric Blake
2016-03-23 12:38 ` Daniel P. Berrange
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 06/10] acl: delete existing ACL implementation Daniel P. Berrange
2016-03-22 17:58 ` Eric Blake
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 07/10] qemu-nbd: add support for ACLs for TLS clients Daniel P. Berrange
2016-03-22 18:14 ` Eric Blake
2016-03-23 12:40 ` Daniel P. Berrange
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 08/10] nbd: allow an ACL to be set with nbd-server-start QMP command Daniel P. Berrange
2016-03-22 18:19 ` Eric Blake
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 09/10] chardev: add support for ACLs for TLS clients Daniel P. Berrange
2016-03-22 21:26 ` Eric Blake
2016-03-10 18:59 ` [Qemu-devel] [PATCH v3 10/10] vnc: allow specifying a custom ACL object name Daniel P. Berrange
2016-03-22 21:38 ` Eric Blake
2016-03-23 12:43 ` Daniel P. Berrange
2016-03-21 22:45 ` [Qemu-devel] [PATCH v3 01/10] qdict: implement a qdict_crumple method for un-flattening a dict Eric Blake
2016-03-22 15:44 ` Daniel P. Berrange
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).