[Qemu-devel] [PATCH v6 11/11] authz: delete existing ACL implementation

Daniel P. Berrangé posted 11 patches 7 years, 3 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v6 11/11] authz: delete existing ACL implementation
Posted by Daniel P. Berrangé 7 years, 3 months ago
From: "Daniel P. Berrange" <berrange@redhat.com>

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 subclasses 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 temporary backwards compatibility.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 include/qemu/acl.h             |  66 ------------
 ui/vnc-auth-sasl.h             |   5 +-
 ui/vnc.h                       |   4 +-
 crypto/tlssession.c            |  35 +++----
 monitor.c                      | 185 ++++++++++++++++++++++-----------
 tests/test-crypto-tlssession.c |  15 ++-
 tests/test-io-channel-tls.c    |  16 ++-
 ui/vnc-auth-sasl.c             |  23 ++--
 ui/vnc-auth-vencrypt.c         |   2 +-
 ui/vnc-ws.c                    |   2 +-
 ui/vnc.c                       |  37 ++++---
 util/acl.c                     | 179 -------------------------------
 crypto/trace-events            |   2 +-
 tests/Makefile.include         |   4 +-
 util/Makefile.objs             |   1 -
 15 files changed, 215 insertions(+), 361 deletions(-)
 delete mode 100644 include/qemu/acl.h
 delete mode 100644 util/acl.c

diff --git a/include/qemu/acl.h b/include/qemu/acl.h
deleted file mode 100644
index 7c44119a47..0000000000
--- a/include/qemu/acl.h
+++ /dev/null
@@ -1,66 +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 */
diff --git a/ui/vnc-auth-sasl.h b/ui/vnc-auth-sasl.h
index 2ae224ee3a..fb55fe04ca 100644
--- a/ui/vnc-auth-sasl.h
+++ b/ui/vnc-auth-sasl.h
@@ -30,8 +30,8 @@
 typedef struct VncStateSASL VncStateSASL;
 typedef struct VncDisplaySASL VncDisplaySASL;
 
-#include "qemu/acl.h"
 #include "qemu/main-loop.h"
+#include "authz/base.h"
 
 struct VncStateSASL {
     sasl_conn_t *conn;
@@ -60,7 +60,8 @@ struct VncStateSASL {
 };
 
 struct VncDisplaySASL {
-    qemu_acl *acl;
+    QAuthZ *authz;
+    char *authzid;
 };
 
 void vnc_sasl_client_cleanup(VncState *vs);
diff --git a/ui/vnc.h b/ui/vnc.h
index a86e0610e8..29ee1738a5 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -39,6 +39,7 @@
 #include "io/channel-socket.h"
 #include "io/channel-tls.h"
 #include "io/net-listener.h"
+#include "authz/base.h"
 #include <zlib.h>
 
 #include "keymaps.h"
@@ -177,7 +178,8 @@ struct VncDisplay
     bool lossy;
     bool non_adaptive;
     QCryptoTLSCreds *tlscreds;
-    char *tlsaclname;
+    QAuthZ *tlsauthz;
+    char *tlsauthzid;
 #ifdef CONFIG_VNC_SASL
     VncDisplaySASL sasl;
 #endif
diff --git a/crypto/tlssession.c b/crypto/tlssession.c
index 66a6fbe19c..23842aa95d 100644
--- a/crypto/tlssession.c
+++ b/crypto/tlssession.c
@@ -24,7 +24,7 @@
 #include "crypto/tlscredspsk.h"
 #include "crypto/tlscredsx509.h"
 #include "qapi/error.h"
-#include "qemu/acl.h"
+#include "authz/base.h"
 #include "trace.h"
 
 #ifdef CONFIG_GNUTLS
@@ -37,7 +37,7 @@ struct QCryptoTLSSession {
     QCryptoTLSCreds *creds;
     gnutls_session_t handle;
     char *hostname;
-    char *aclname;
+    char *authzid;
     bool handshakeComplete;
     QCryptoTLSSessionWriteFunc writeFunc;
     QCryptoTLSSessionReadFunc readFunc;
@@ -56,7 +56,7 @@ qcrypto_tls_session_free(QCryptoTLSSession *session)
     gnutls_deinit(session->handle);
     g_free(session->hostname);
     g_free(session->peername);
-    g_free(session->aclname);
+    g_free(session->authzid);
     object_unref(OBJECT(session->creds));
     g_free(session);
 }
@@ -101,7 +101,7 @@ qcrypto_tls_session_pull(void *opaque, void *buf, size_t len)
 QCryptoTLSSession *
 qcrypto_tls_session_new(QCryptoTLSCreds *creds,
                         const char *hostname,
-                        const char *aclname,
+                        const char *authzid,
                         QCryptoTLSCredsEndpoint endpoint,
                         Error **errp)
 {
@@ -111,13 +111,13 @@ qcrypto_tls_session_new(QCryptoTLSCreds *creds,
     session = g_new0(QCryptoTLSSession, 1);
     trace_qcrypto_tls_session_new(
         session, creds, hostname ? hostname : "<none>",
-        aclname ? aclname : "<none>", endpoint);
+        authzid ? authzid : "<none>", endpoint);
 
     if (hostname) {
         session->hostname = g_strdup(hostname);
     }
-    if (aclname) {
-        session->aclname = g_strdup(aclname);
+    if (authzid) {
+        session->authzid = g_strdup(authzid);
     }
     session->creds = creds;
     object_ref(OBJECT(creds));
@@ -268,6 +268,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)) {
@@ -355,19 +356,17 @@ qcrypto_tls_session_check_certificate(QCryptoTLSSession *session,
                            gnutls_strerror(ret));
                 goto error;
             }
-            if (session->aclname) {
-                qemu_acl *acl = qemu_acl_find(session->aclname);
-                int allow;
-                if (!acl) {
-                    error_setg(errp, "Cannot find ACL %s",
-                               session->aclname);
+            if (session->authzid) {
+                bool allow;
+
+                allow = qauthz_is_allowed_by_id(session->authzid,
+                                                session->peername, &err);
+                if (err) {
+                    error_propagate(errp, err);
                     goto error;
                 }
-
-                allow = qemu_acl_party_is_allowed(acl, session->peername);
-
                 if (!allow) {
-                    error_setg(errp, "TLS x509 ACL check for %s is denied",
+                    error_setg(errp, "TLS x509 authz check for %s is denied",
                                session->peername);
                     goto error;
                 }
@@ -558,7 +557,7 @@ qcrypto_tls_session_get_peer_name(QCryptoTLSSession *session)
 QCryptoTLSSession *
 qcrypto_tls_session_new(QCryptoTLSCreds *creds G_GNUC_UNUSED,
                         const char *hostname G_GNUC_UNUSED,
-                        const char *aclname G_GNUC_UNUSED,
+                        const char *authzid G_GNUC_UNUSED,
                         QCryptoTLSCredsEndpoint endpoint G_GNUC_UNUSED,
                         Error **errp)
 {
diff --git a/monitor.c b/monitor.c
index b9258a7438..2458322eb1 100644
--- a/monitor.c
+++ b/monitor.c
@@ -51,7 +51,8 @@
 #include "sysemu/balloon.h"
 #include "qemu/timer.h"
 #include "sysemu/hw_accel.h"
-#include "qemu/acl.h"
+#include "authz/list.h"
+#include "qapi/util.h"
 #include "sysemu/tpm.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qerror.h"
@@ -2040,93 +2041,154 @@ 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 QAuthZList *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_LIST(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);
-        }
+    QAuthZList *auth = find_auth(mon, aclname);
+    QAuthZListRuleList *rules;
+    size_t i = 0;
+
+    if (!auth) {
+        return;
+    }
+
+    monitor_printf(mon, "policy: %s\n",
+                   QAuthZListPolicy_lookup.array[auth->policy]);
+
+    rules = auth->rules;
+    while (rules) {
+        QAuthZListRule *rule = rules->value;
+        i++;
+        monitor_printf(mon, "%zu: %s %s\n", i,
+                       QAuthZListPolicy_lookup.array[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);
+    QAuthZList *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_LIST_POLICY_DENY;
+    qapi_free_QAuthZListRuleList(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);
+    QAuthZList *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(&QAuthZListPolicy_lookup,
+                          policy,
+                          QAUTHZ_LIST_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_LIST_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");
         }
     }
 }
 
+static QAuthZListFormat hmp_acl_get_format(const char *match)
+{
+#ifdef CONFIG_FNMATCH
+    if (strchr(match, '*')) {
+        return QAUTHZ_LIST_FORMAT_GLOB;
+    } else {
+        return QAUTHZ_LIST_FORMAT_EXACT;
+    }
+#else
+    /* Historically we silently degraded to plain strcmp
+     * when fnmatch() was missing */
+    return QAUTHZ_LIST_FORMAT_EXACT;
+#endif
+}
+
 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);
+    QAuthZList *auth = find_auth(mon, aclname);
+    Error *err = NULL;
+    QAuthZListPolicy policy;
+    QAuthZListFormat format;
+    size_t i = 0;
+
+    if (!auth) {
+        return;
+    }
+
+    policy = qapi_enum_parse(&QAuthZListPolicy_lookup,
+                             policystr,
+                             QAUTHZ_LIST_POLICY_DENY,
+                             &err);
+    if (err) {
+        error_free(err);
+        monitor_printf(mon, "acl: unknown policy '%s', "
+                       "expected 'deny' or 'allow'\n", policystr);
+        return;
+    }
+
+    format = hmp_acl_get_format(match);
+
+    if (has_index && index == 0) {
+        monitor_printf(mon, "acl: unable to add acl entry\n");
+        return;
+    }
+
+    if (has_index) {
+        i = qauthz_list_insert_rule(auth, match, policy,
+                                    format, index - 1, &err);
+    } else {
+        i = qauthz_list_append_rule(auth, match, policy,
+                                    format, &err);
+    }
+    if (err) {
+        monitor_printf(mon, "acl: unable to add rule: %s",
+                       error_get_pretty(err));
+        error_free(err);
+    } else {
+        monitor_printf(mon, "acl: added rule at position %zu\n", i + 1);
     }
 }
 
@@ -2134,15 +2196,18 @@ 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;
+    QAuthZList *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_list_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/test-crypto-tlssession.c b/tests/test-crypto-tlssession.c
index 6fa9950afb..15212ec276 100644
--- a/tests/test-crypto-tlssession.c
+++ b/tests/test-crypto-tlssession.c
@@ -28,7 +28,7 @@
 #include "qom/object_interfaces.h"
 #include "qapi/error.h"
 #include "qemu/sockets.h"
-#include "qemu/acl.h"
+#include "authz/list.h"
 
 #ifdef QCRYPTO_HAVE_TLS_TEST_SUPPORT
 
@@ -229,7 +229,7 @@ static void test_crypto_tls_session_x509(const void *opaque)
     QCryptoTLSCreds *serverCreds;
     QCryptoTLSSession *clientSess = NULL;
     QCryptoTLSSession *serverSess = NULL;
-    qemu_acl *acl;
+    QAuthZList *auth;
     const char * const *wildcards;
     int channel[2];
     bool clientShake = false;
@@ -285,11 +285,15 @@ static void test_crypto_tls_session_x509(const void *opaque)
         SERVER_CERT_DIR);
     g_assert(serverCreds != NULL);
 
-    acl = qemu_acl_init("tlssessionacl");
-    qemu_acl_reset(acl);
+    auth = qauthz_list_new("tlssessionacl",
+                           QAUTHZ_LIST_POLICY_DENY,
+                           &error_abort);
     wildcards = data->wildcards;
     while (wildcards && *wildcards) {
-        qemu_acl_append(acl, 0, *wildcards);
+        qauthz_list_append_rule(auth, *wildcards,
+                                QAUTHZ_LIST_POLICY_ALLOW,
+                                QAUTHZ_LIST_FORMAT_GLOB,
+                                &error_abort);
         wildcards++;
     }
 
@@ -377,6 +381,7 @@ static void test_crypto_tls_session_x509(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 4900c6d433..43b707eba7 100644
--- a/tests/test-io-channel-tls.c
+++ b/tests/test-io-channel-tls.c
@@ -29,8 +29,8 @@
 #include "io-channel-helpers.h"
 #include "crypto/init.h"
 #include "crypto/tlscredsx509.h"
-#include "qemu/acl.h"
 #include "qapi/error.h"
+#include "authz/list.h"
 #include "qom/object_interfaces.h"
 
 #ifdef QCRYPTO_HAVE_TLS_TEST_SUPPORT
@@ -113,7 +113,7 @@ static void test_io_channel_tls(const void *opaque)
     QIOChannelTLS *serverChanTLS;
     QIOChannelSocket *clientChanSock;
     QIOChannelSocket *serverChanSock;
-    qemu_acl *acl;
+    QAuthZList *auth;
     const char * const *wildcards;
     int channel[2];
     struct QIOChannelTLSHandshakeData clientHandshake = { false, false };
@@ -161,11 +161,15 @@ static void test_io_channel_tls(const void *opaque)
         SERVER_CERT_DIR);
     g_assert(serverCreds != NULL);
 
-    acl = qemu_acl_init("channeltlsacl");
-    qemu_acl_reset(acl);
+    auth = qauthz_list_new("channeltlsacl",
+                           QAUTHZ_LIST_POLICY_DENY,
+                           &error_abort);
     wildcards = data->wildcards;
     while (wildcards && *wildcards) {
-        qemu_acl_append(acl, 0, *wildcards);
+        qauthz_list_append_rule(auth, *wildcards,
+                                QAUTHZ_LIST_POLICY_ALLOW,
+                                QAUTHZ_LIST_FORMAT_GLOB,
+                                &error_abort);
         wildcards++;
     }
 
@@ -253,6 +257,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 3751a777a4..7b2b09f242 100644
--- a/ui/vnc-auth-sasl.c
+++ b/ui/vnc-auth-sasl.c
@@ -24,6 +24,7 @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
+#include "authz/base.h"
 #include "vnc.h"
 #include "trace.h"
 
@@ -146,13 +147,14 @@ size_t vnc_client_read_sasl(VncState *vs)
 static int vnc_auth_sasl_check_access(VncState *vs)
 {
     const void *val;
-    int err;
-    int allow;
+    int rv;
+    Error *err = NULL;
+    bool allow;
 
-    err = sasl_getprop(vs->sasl.conn, SASL_USERNAME, &val);
-    if (err != SASL_OK) {
+    rv = sasl_getprop(vs->sasl.conn, SASL_USERNAME, &val);
+    if (rv != SASL_OK) {
         trace_vnc_auth_fail(vs, vs->auth, "Cannot fetch SASL username",
-                            sasl_errstring(err, NULL, NULL));
+                            sasl_errstring(rv, NULL, NULL));
         return -1;
     }
     if (val == NULL) {
@@ -163,12 +165,19 @@ static int vnc_auth_sasl_check_access(VncState *vs)
     vs->sasl.username = g_strdup((const char*)val);
     trace_vnc_auth_sasl_username(vs, vs->sasl.username);
 
-    if (vs->vd->sasl.acl == NULL) {
+    if (vs->vd->sasl.authzid == NULL) {
         trace_vnc_auth_sasl_acl(vs, 1);
         return 0;
     }
 
-    allow = qemu_acl_party_is_allowed(vs->vd->sasl.acl, vs->sasl.username);
+    allow = qauthz_is_allowed_by_id(vs->vd->sasl.authzid,
+                                    vs->sasl.username, &err);
+    if (err) {
+        trace_vnc_auth_fail(vs, vs->auth, "Error from authz",
+                            error_get_pretty(err));
+        error_free(err);
+        return -1;
+    }
 
     trace_vnc_auth_sasl_acl(vs, allow);
     return allow ? 0 : -1;
diff --git a/ui/vnc-auth-vencrypt.c b/ui/vnc-auth-vencrypt.c
index d99ea362c1..f072e16ace 100644
--- a/ui/vnc-auth-vencrypt.c
+++ b/ui/vnc-auth-vencrypt.c
@@ -109,7 +109,7 @@ static int protocol_client_vencrypt_auth(VncState *vs, uint8_t *data, size_t len
         tls = qio_channel_tls_new_server(
             vs->ioc,
             vs->vd->tlscreds,
-            vs->vd->tlsaclname,
+            vs->vd->tlsauthzid,
             &err);
         if (!tls) {
             trace_vnc_auth_fail(vs, vs->auth, "TLS setup failed",
diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
index 950f1cd2ac..95c9703c72 100644
--- a/ui/vnc-ws.c
+++ b/ui/vnc-ws.c
@@ -62,7 +62,7 @@ gboolean vncws_tls_handshake_io(QIOChannel *ioc G_GNUC_UNUSED,
     tls = qio_channel_tls_new_server(
         vs->ioc,
         vs->vd->tlscreds,
-        vs->vd->tlsaclname,
+        vs->vd->tlsauthzid,
         &err);
     if (!tls) {
         VNC_DEBUG("Failed to setup TLS %s\n", error_get_pretty(err));
diff --git a/ui/vnc.c b/ui/vnc.c
index cf221c83cc..60cb7c2d3d 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -33,7 +33,7 @@
 #include "qemu/option.h"
 #include "qemu/sockets.h"
 #include "qemu/timer.h"
-#include "qemu/acl.h"
+#include "authz/list.h"
 #include "qemu/config-file.h"
 #include "qapi/qapi-events.h"
 #include "qapi/error.h"
@@ -3267,12 +3267,24 @@ static void vnc_display_close(VncDisplay *vd)
         object_unparent(OBJECT(vd->tlscreds));
         vd->tlscreds = NULL;
     }
-    g_free(vd->tlsaclname);
-    vd->tlsaclname = NULL;
+    if (vd->tlsauthz) {
+        object_unparent(OBJECT(vd->tlsauthz));
+        vd->tlsauthz = NULL;
+    }
+    g_free(vd->tlsauthzid);
+    vd->tlsauthzid = NULL;
     if (vd->lock_key_sync) {
         qemu_remove_led_event_handler(vd->led);
         vd->led = NULL;
     }
+#ifdef CONFIG_VNC_SASL
+    if (vd->sasl.authz) {
+        object_unparent(OBJECT(vd->sasl.authz));
+        vd->sasl.authz = NULL;
+    }
+    g_free(vd->sasl.authzid);
+    vd->sasl.authzid = NULL;
+#endif
 }
 
 int vnc_display_password(const char *id, const char *password)
@@ -3925,23 +3937,24 @@ void vnc_display_open(const char *id, Error **errp)
 
     if (acl) {
         if (strcmp(vd->id, "default") == 0) {
-            vd->tlsaclname = g_strdup("vnc.x509dname");
+            vd->tlsauthzid = g_strdup("vnc.x509dname");
         } else {
-            vd->tlsaclname = g_strdup_printf("vnc.%s.x509dname", vd->id);
+            vd->tlsauthzid = g_strdup_printf("vnc.%s.x509dname", vd->id);
         }
-        qemu_acl_init(vd->tlsaclname);
+        vd->tlsauthz = QAUTHZ(qauthz_list_new(vd->tlsauthzid,
+                                              QAUTHZ_LIST_POLICY_DENY,
+                                              &error_abort));
     }
 #ifdef CONFIG_VNC_SASL
     if (acl && sasl) {
-        char *aclname;
-
         if (strcmp(vd->id, "default") == 0) {
-            aclname = g_strdup("vnc.username");
+            vd->sasl.authzid = g_strdup("vnc.username");
         } else {
-            aclname = g_strdup_printf("vnc.%s.username", vd->id);
+            vd->sasl.authzid = g_strdup_printf("vnc.%s.username", vd->id);
         }
-        vd->sasl.acl = qemu_acl_init(aclname);
-        g_free(aclname);
+        vd->sasl.authz = QAUTHZ(qauthz_list_new(vd->sasl.authzid,
+                                                QAUTHZ_LIST_POLICY_DENY,
+                                                &error_abort));
     }
 #endif
 
diff --git a/util/acl.c b/util/acl.c
deleted file mode 100644
index c105addadc..0000000000
--- a/util/acl.c
+++ /dev/null
@@ -1,179 +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;
-}
diff --git a/crypto/trace-events b/crypto/trace-events
index 597389b73c..a38ad7b787 100644
--- a/crypto/trace-events
+++ b/crypto/trace-events
@@ -19,5 +19,5 @@ qcrypto_tls_creds_x509_load_cert(void *creds, int isServer, const char *file) "T
 qcrypto_tls_creds_x509_load_cert_list(void *creds, const char *file) "TLS creds x509 load cert list creds=%p file=%s"
 
 # crypto/tlssession.c
-qcrypto_tls_session_new(void *session, void *creds, const char *hostname, const char *aclname, int endpoint) "TLS session new session=%p creds=%p hostname=%s aclname=%s endpoint=%d"
+qcrypto_tls_session_new(void *session, void *creds, const char *hostname, const char *authzid, int endpoint) "TLS session new session=%p creds=%p hostname=%s authzid=%s endpoint=%d"
 qcrypto_tls_session_check_creds(void *session, const char *status) "TLS session check creds session=%p status=%s"
diff --git a/tests/Makefile.include b/tests/Makefile.include
index b2369c14cb..e8ceb4e5f6 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -510,8 +510,8 @@ 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-events.o tests/test-qapi-introspect.o \
 	$(test-qom-obj-y)
-benchmark-crypto-obj-y = $(crypto-obj-y) $(test-qom-obj-y)
-test-crypto-obj-y = $(crypto-obj-y) $(test-qom-obj-y)
+benchmark-crypto-obj-y = $(authz-obj-y) $(crypto-obj-y) $(test-qom-obj-y)
+test-crypto-obj-y = $(authz-obj-y) $(crypto-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) tests/iothread.o
 
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 4d7675d6e7..ca99e7e90d 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -20,7 +20,6 @@ util-obj-y += envlist.o path.o module.o
 util-obj-y += 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 += cacheinfo.o
 util-obj-y += error.o qemu-error.o
 util-obj-y += id.o
-- 
2.17.2


Re: [Qemu-devel] [PATCH v6 11/11] authz: delete existing ACL implementation
Posted by Philippe Mathieu-Daudé 7 years, 3 months ago
On 19/10/18 15:38, Daniel P. Berrangé wrote:
> From: "Daniel P. Berrange" <berrange@redhat.com>
> 
> 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 subclasses 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 temporary backwards compatibility.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   include/qemu/acl.h             |  66 ------------
>   ui/vnc-auth-sasl.h             |   5 +-
>   ui/vnc.h                       |   4 +-
>   crypto/tlssession.c            |  35 +++----
>   monitor.c                      | 185 ++++++++++++++++++++++-----------
>   tests/test-crypto-tlssession.c |  15 ++-
>   tests/test-io-channel-tls.c    |  16 ++-
>   ui/vnc-auth-sasl.c             |  23 ++--
>   ui/vnc-auth-vencrypt.c         |   2 +-
>   ui/vnc-ws.c                    |   2 +-
>   ui/vnc.c                       |  37 ++++---
>   util/acl.c                     | 179 -------------------------------
>   crypto/trace-events            |   2 +-
>   tests/Makefile.include         |   4 +-
>   util/Makefile.objs             |   1 -
>   15 files changed, 215 insertions(+), 361 deletions(-)
>   delete mode 100644 include/qemu/acl.h
>   delete mode 100644 util/acl.c
> 
> diff --git a/include/qemu/acl.h b/include/qemu/acl.h
> deleted file mode 100644
> index 7c44119a47..0000000000
> --- a/include/qemu/acl.h
> +++ /dev/null
> @@ -1,66 +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 */
> diff --git a/ui/vnc-auth-sasl.h b/ui/vnc-auth-sasl.h
> index 2ae224ee3a..fb55fe04ca 100644
> --- a/ui/vnc-auth-sasl.h
> +++ b/ui/vnc-auth-sasl.h
> @@ -30,8 +30,8 @@
>   typedef struct VncStateSASL VncStateSASL;
>   typedef struct VncDisplaySASL VncDisplaySASL;
>   
> -#include "qemu/acl.h"
>   #include "qemu/main-loop.h"
> +#include "authz/base.h"
>   
>   struct VncStateSASL {
>       sasl_conn_t *conn;
> @@ -60,7 +60,8 @@ struct VncStateSASL {
>   };
>   
>   struct VncDisplaySASL {
> -    qemu_acl *acl;
> +    QAuthZ *authz;
> +    char *authzid;
>   };
>   
>   void vnc_sasl_client_cleanup(VncState *vs);
> diff --git a/ui/vnc.h b/ui/vnc.h
> index a86e0610e8..29ee1738a5 100644
> --- a/ui/vnc.h
> +++ b/ui/vnc.h
> @@ -39,6 +39,7 @@
>   #include "io/channel-socket.h"
>   #include "io/channel-tls.h"
>   #include "io/net-listener.h"
> +#include "authz/base.h"
>   #include <zlib.h>
>   
>   #include "keymaps.h"
> @@ -177,7 +178,8 @@ struct VncDisplay
>       bool lossy;
>       bool non_adaptive;
>       QCryptoTLSCreds *tlscreds;
> -    char *tlsaclname;
> +    QAuthZ *tlsauthz;
> +    char *tlsauthzid;
>   #ifdef CONFIG_VNC_SASL
>       VncDisplaySASL sasl;
>   #endif
> diff --git a/crypto/tlssession.c b/crypto/tlssession.c
> index 66a6fbe19c..23842aa95d 100644
> --- a/crypto/tlssession.c
> +++ b/crypto/tlssession.c
> @@ -24,7 +24,7 @@
>   #include "crypto/tlscredspsk.h"
>   #include "crypto/tlscredsx509.h"
>   #include "qapi/error.h"
> -#include "qemu/acl.h"
> +#include "authz/base.h"
>   #include "trace.h"
>   
>   #ifdef CONFIG_GNUTLS
> @@ -37,7 +37,7 @@ struct QCryptoTLSSession {
>       QCryptoTLSCreds *creds;
>       gnutls_session_t handle;
>       char *hostname;
> -    char *aclname;
> +    char *authzid;
>       bool handshakeComplete;
>       QCryptoTLSSessionWriteFunc writeFunc;
>       QCryptoTLSSessionReadFunc readFunc;
> @@ -56,7 +56,7 @@ qcrypto_tls_session_free(QCryptoTLSSession *session)
>       gnutls_deinit(session->handle);
>       g_free(session->hostname);
>       g_free(session->peername);
> -    g_free(session->aclname);
> +    g_free(session->authzid);
>       object_unref(OBJECT(session->creds));
>       g_free(session);
>   }
> @@ -101,7 +101,7 @@ qcrypto_tls_session_pull(void *opaque, void *buf, size_t len)
>   QCryptoTLSSession *
>   qcrypto_tls_session_new(QCryptoTLSCreds *creds,
>                           const char *hostname,
> -                        const char *aclname,
> +                        const char *authzid,
>                           QCryptoTLSCredsEndpoint endpoint,
>                           Error **errp)
>   {
> @@ -111,13 +111,13 @@ qcrypto_tls_session_new(QCryptoTLSCreds *creds,
>       session = g_new0(QCryptoTLSSession, 1);
>       trace_qcrypto_tls_session_new(
>           session, creds, hostname ? hostname : "<none>",
> -        aclname ? aclname : "<none>", endpoint);
> +        authzid ? authzid : "<none>", endpoint);
>   
>       if (hostname) {
>           session->hostname = g_strdup(hostname);
>       }
> -    if (aclname) {
> -        session->aclname = g_strdup(aclname);
> +    if (authzid) {
> +        session->authzid = g_strdup(authzid);
>       }
>       session->creds = creds;
>       object_ref(OBJECT(creds));
> @@ -268,6 +268,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)) {
> @@ -355,19 +356,17 @@ qcrypto_tls_session_check_certificate(QCryptoTLSSession *session,
>                              gnutls_strerror(ret));
>                   goto error;
>               }
> -            if (session->aclname) {
> -                qemu_acl *acl = qemu_acl_find(session->aclname);
> -                int allow;
> -                if (!acl) {
> -                    error_setg(errp, "Cannot find ACL %s",
> -                               session->aclname);
> +            if (session->authzid) {
> +                bool allow;
> +
> +                allow = qauthz_is_allowed_by_id(session->authzid,
> +                                                session->peername, &err);
> +                if (err) {
> +                    error_propagate(errp, err);
>                       goto error;
>                   }
> -
> -                allow = qemu_acl_party_is_allowed(acl, session->peername);
> -
>                   if (!allow) {
> -                    error_setg(errp, "TLS x509 ACL check for %s is denied",
> +                    error_setg(errp, "TLS x509 authz check for %s is denied",
>                                  session->peername);
>                       goto error;
>                   }
> @@ -558,7 +557,7 @@ qcrypto_tls_session_get_peer_name(QCryptoTLSSession *session)
>   QCryptoTLSSession *
>   qcrypto_tls_session_new(QCryptoTLSCreds *creds G_GNUC_UNUSED,
>                           const char *hostname G_GNUC_UNUSED,
> -                        const char *aclname G_GNUC_UNUSED,
> +                        const char *authzid G_GNUC_UNUSED,
>                           QCryptoTLSCredsEndpoint endpoint G_GNUC_UNUSED,
>                           Error **errp)
>   {
> diff --git a/monitor.c b/monitor.c
> index b9258a7438..2458322eb1 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -51,7 +51,8 @@
>   #include "sysemu/balloon.h"
>   #include "qemu/timer.h"
>   #include "sysemu/hw_accel.h"
> -#include "qemu/acl.h"
> +#include "authz/list.h"
> +#include "qapi/util.h"
>   #include "sysemu/tpm.h"
>   #include "qapi/qmp/qdict.h"
>   #include "qapi/qmp/qerror.h"
> @@ -2040,93 +2041,154 @@ 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 QAuthZList *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_LIST(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);
> -        }
> +    QAuthZList *auth = find_auth(mon, aclname);
> +    QAuthZListRuleList *rules;
> +    size_t i = 0;
> +
> +    if (!auth) {
> +        return;
> +    }
> +
> +    monitor_printf(mon, "policy: %s\n",
> +                   QAuthZListPolicy_lookup.array[auth->policy]);
> +
> +    rules = auth->rules;
> +    while (rules) {
> +        QAuthZListRule *rule = rules->value;
> +        i++;
> +        monitor_printf(mon, "%zu: %s %s\n", i,
> +                       QAuthZListPolicy_lookup.array[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);
> +    QAuthZList *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_LIST_POLICY_DENY;
> +    qapi_free_QAuthZListRuleList(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);
> +    QAuthZList *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(&QAuthZListPolicy_lookup,
> +                          policy,
> +                          QAUTHZ_LIST_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_LIST_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");
>           }
>       }
>   }
>   
> +static QAuthZListFormat hmp_acl_get_format(const char *match)
> +{
> +#ifdef CONFIG_FNMATCH
> +    if (strchr(match, '*')) {
> +        return QAUTHZ_LIST_FORMAT_GLOB;
> +    } else {
> +        return QAUTHZ_LIST_FORMAT_EXACT;
> +    }
> +#else
> +    /* Historically we silently degraded to plain strcmp
> +     * when fnmatch() was missing */
> +    return QAUTHZ_LIST_FORMAT_EXACT;
> +#endif
> +}
> +
>   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);
> +    QAuthZList *auth = find_auth(mon, aclname);
> +    Error *err = NULL;
> +    QAuthZListPolicy policy;
> +    QAuthZListFormat format;
> +    size_t i = 0;
> +
> +    if (!auth) {
> +        return;
> +    }
> +
> +    policy = qapi_enum_parse(&QAuthZListPolicy_lookup,
> +                             policystr,
> +                             QAUTHZ_LIST_POLICY_DENY,
> +                             &err);
> +    if (err) {
> +        error_free(err);
> +        monitor_printf(mon, "acl: unknown policy '%s', "
> +                       "expected 'deny' or 'allow'\n", policystr);
> +        return;
> +    }
> +
> +    format = hmp_acl_get_format(match);
> +
> +    if (has_index && index == 0) {
> +        monitor_printf(mon, "acl: unable to add acl entry\n");
> +        return;
> +    }
> +
> +    if (has_index) {
> +        i = qauthz_list_insert_rule(auth, match, policy,
> +                                    format, index - 1, &err);
> +    } else {
> +        i = qauthz_list_append_rule(auth, match, policy,
> +                                    format, &err);
> +    }
> +    if (err) {
> +        monitor_printf(mon, "acl: unable to add rule: %s",
> +                       error_get_pretty(err));
> +        error_free(err);
> +    } else {
> +        monitor_printf(mon, "acl: added rule at position %zu\n", i + 1);
>       }
>   }
>   
> @@ -2134,15 +2196,18 @@ 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;
> +    QAuthZList *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_list_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/test-crypto-tlssession.c b/tests/test-crypto-tlssession.c
> index 6fa9950afb..15212ec276 100644
> --- a/tests/test-crypto-tlssession.c
> +++ b/tests/test-crypto-tlssession.c
> @@ -28,7 +28,7 @@
>   #include "qom/object_interfaces.h"
>   #include "qapi/error.h"
>   #include "qemu/sockets.h"
> -#include "qemu/acl.h"
> +#include "authz/list.h"
>   
>   #ifdef QCRYPTO_HAVE_TLS_TEST_SUPPORT
>   
> @@ -229,7 +229,7 @@ static void test_crypto_tls_session_x509(const void *opaque)
>       QCryptoTLSCreds *serverCreds;
>       QCryptoTLSSession *clientSess = NULL;
>       QCryptoTLSSession *serverSess = NULL;
> -    qemu_acl *acl;
> +    QAuthZList *auth;
>       const char * const *wildcards;
>       int channel[2];
>       bool clientShake = false;
> @@ -285,11 +285,15 @@ static void test_crypto_tls_session_x509(const void *opaque)
>           SERVER_CERT_DIR);
>       g_assert(serverCreds != NULL);
>   
> -    acl = qemu_acl_init("tlssessionacl");
> -    qemu_acl_reset(acl);
> +    auth = qauthz_list_new("tlssessionacl",
> +                           QAUTHZ_LIST_POLICY_DENY,
> +                           &error_abort);
>       wildcards = data->wildcards;
>       while (wildcards && *wildcards) {
> -        qemu_acl_append(acl, 0, *wildcards);
> +        qauthz_list_append_rule(auth, *wildcards,
> +                                QAUTHZ_LIST_POLICY_ALLOW,
> +                                QAUTHZ_LIST_FORMAT_GLOB,
> +                                &error_abort);
>           wildcards++;
>       }
>   
> @@ -377,6 +381,7 @@ static void test_crypto_tls_session_x509(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 4900c6d433..43b707eba7 100644
> --- a/tests/test-io-channel-tls.c
> +++ b/tests/test-io-channel-tls.c
> @@ -29,8 +29,8 @@
>   #include "io-channel-helpers.h"
>   #include "crypto/init.h"
>   #include "crypto/tlscredsx509.h"
> -#include "qemu/acl.h"
>   #include "qapi/error.h"
> +#include "authz/list.h"
>   #include "qom/object_interfaces.h"
>   
>   #ifdef QCRYPTO_HAVE_TLS_TEST_SUPPORT
> @@ -113,7 +113,7 @@ static void test_io_channel_tls(const void *opaque)
>       QIOChannelTLS *serverChanTLS;
>       QIOChannelSocket *clientChanSock;
>       QIOChannelSocket *serverChanSock;
> -    qemu_acl *acl;
> +    QAuthZList *auth;
>       const char * const *wildcards;
>       int channel[2];
>       struct QIOChannelTLSHandshakeData clientHandshake = { false, false };
> @@ -161,11 +161,15 @@ static void test_io_channel_tls(const void *opaque)
>           SERVER_CERT_DIR);
>       g_assert(serverCreds != NULL);
>   
> -    acl = qemu_acl_init("channeltlsacl");
> -    qemu_acl_reset(acl);
> +    auth = qauthz_list_new("channeltlsacl",
> +                           QAUTHZ_LIST_POLICY_DENY,
> +                           &error_abort);
>       wildcards = data->wildcards;
>       while (wildcards && *wildcards) {
> -        qemu_acl_append(acl, 0, *wildcards);
> +        qauthz_list_append_rule(auth, *wildcards,
> +                                QAUTHZ_LIST_POLICY_ALLOW,
> +                                QAUTHZ_LIST_FORMAT_GLOB,
> +                                &error_abort);
>           wildcards++;
>       }
>   
> @@ -253,6 +257,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 3751a777a4..7b2b09f242 100644
> --- a/ui/vnc-auth-sasl.c
> +++ b/ui/vnc-auth-sasl.c
> @@ -24,6 +24,7 @@
>   
>   #include "qemu/osdep.h"
>   #include "qapi/error.h"
> +#include "authz/base.h"
>   #include "vnc.h"
>   #include "trace.h"
>   
> @@ -146,13 +147,14 @@ size_t vnc_client_read_sasl(VncState *vs)
>   static int vnc_auth_sasl_check_access(VncState *vs)
>   {
>       const void *val;
> -    int err;
> -    int allow;
> +    int rv;
> +    Error *err = NULL;
> +    bool allow;
>   
> -    err = sasl_getprop(vs->sasl.conn, SASL_USERNAME, &val);
> -    if (err != SASL_OK) {
> +    rv = sasl_getprop(vs->sasl.conn, SASL_USERNAME, &val);
> +    if (rv != SASL_OK) {
>           trace_vnc_auth_fail(vs, vs->auth, "Cannot fetch SASL username",
> -                            sasl_errstring(err, NULL, NULL));
> +                            sasl_errstring(rv, NULL, NULL));
>           return -1;
>       }
>       if (val == NULL) {
> @@ -163,12 +165,19 @@ static int vnc_auth_sasl_check_access(VncState *vs)
>       vs->sasl.username = g_strdup((const char*)val);
>       trace_vnc_auth_sasl_username(vs, vs->sasl.username);
>   
> -    if (vs->vd->sasl.acl == NULL) {
> +    if (vs->vd->sasl.authzid == NULL) {
>           trace_vnc_auth_sasl_acl(vs, 1);
>           return 0;
>       }
>   
> -    allow = qemu_acl_party_is_allowed(vs->vd->sasl.acl, vs->sasl.username);
> +    allow = qauthz_is_allowed_by_id(vs->vd->sasl.authzid,
> +                                    vs->sasl.username, &err);
> +    if (err) {
> +        trace_vnc_auth_fail(vs, vs->auth, "Error from authz",
> +                            error_get_pretty(err));
> +        error_free(err);
> +        return -1;
> +    }
>   
>       trace_vnc_auth_sasl_acl(vs, allow);
>       return allow ? 0 : -1;
> diff --git a/ui/vnc-auth-vencrypt.c b/ui/vnc-auth-vencrypt.c
> index d99ea362c1..f072e16ace 100644
> --- a/ui/vnc-auth-vencrypt.c
> +++ b/ui/vnc-auth-vencrypt.c
> @@ -109,7 +109,7 @@ static int protocol_client_vencrypt_auth(VncState *vs, uint8_t *data, size_t len
>           tls = qio_channel_tls_new_server(
>               vs->ioc,
>               vs->vd->tlscreds,
> -            vs->vd->tlsaclname,
> +            vs->vd->tlsauthzid,
>               &err);
>           if (!tls) {
>               trace_vnc_auth_fail(vs, vs->auth, "TLS setup failed",
> diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
> index 950f1cd2ac..95c9703c72 100644
> --- a/ui/vnc-ws.c
> +++ b/ui/vnc-ws.c
> @@ -62,7 +62,7 @@ gboolean vncws_tls_handshake_io(QIOChannel *ioc G_GNUC_UNUSED,
>       tls = qio_channel_tls_new_server(
>           vs->ioc,
>           vs->vd->tlscreds,
> -        vs->vd->tlsaclname,
> +        vs->vd->tlsauthzid,
>           &err);
>       if (!tls) {
>           VNC_DEBUG("Failed to setup TLS %s\n", error_get_pretty(err));
> diff --git a/ui/vnc.c b/ui/vnc.c
> index cf221c83cc..60cb7c2d3d 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -33,7 +33,7 @@
>   #include "qemu/option.h"
>   #include "qemu/sockets.h"
>   #include "qemu/timer.h"
> -#include "qemu/acl.h"
> +#include "authz/list.h"
>   #include "qemu/config-file.h"
>   #include "qapi/qapi-events.h"
>   #include "qapi/error.h"
> @@ -3267,12 +3267,24 @@ static void vnc_display_close(VncDisplay *vd)
>           object_unparent(OBJECT(vd->tlscreds));
>           vd->tlscreds = NULL;
>       }
> -    g_free(vd->tlsaclname);
> -    vd->tlsaclname = NULL;
> +    if (vd->tlsauthz) {
> +        object_unparent(OBJECT(vd->tlsauthz));
> +        vd->tlsauthz = NULL;
> +    }
> +    g_free(vd->tlsauthzid);
> +    vd->tlsauthzid = NULL;
>       if (vd->lock_key_sync) {
>           qemu_remove_led_event_handler(vd->led);
>           vd->led = NULL;
>       }
> +#ifdef CONFIG_VNC_SASL
> +    if (vd->sasl.authz) {
> +        object_unparent(OBJECT(vd->sasl.authz));
> +        vd->sasl.authz = NULL;
> +    }
> +    g_free(vd->sasl.authzid);
> +    vd->sasl.authzid = NULL;
> +#endif
>   }
>   
>   int vnc_display_password(const char *id, const char *password)
> @@ -3925,23 +3937,24 @@ void vnc_display_open(const char *id, Error **errp)
>   
>       if (acl) {
>           if (strcmp(vd->id, "default") == 0) {
> -            vd->tlsaclname = g_strdup("vnc.x509dname");
> +            vd->tlsauthzid = g_strdup("vnc.x509dname");
>           } else {
> -            vd->tlsaclname = g_strdup_printf("vnc.%s.x509dname", vd->id);
> +            vd->tlsauthzid = g_strdup_printf("vnc.%s.x509dname", vd->id);
>           }
> -        qemu_acl_init(vd->tlsaclname);
> +        vd->tlsauthz = QAUTHZ(qauthz_list_new(vd->tlsauthzid,
> +                                              QAUTHZ_LIST_POLICY_DENY,
> +                                              &error_abort));
>       }
>   #ifdef CONFIG_VNC_SASL
>       if (acl && sasl) {
> -        char *aclname;
> -
>           if (strcmp(vd->id, "default") == 0) {
> -            aclname = g_strdup("vnc.username");
> +            vd->sasl.authzid = g_strdup("vnc.username");
>           } else {
> -            aclname = g_strdup_printf("vnc.%s.username", vd->id);
> +            vd->sasl.authzid = g_strdup_printf("vnc.%s.username", vd->id);
>           }
> -        vd->sasl.acl = qemu_acl_init(aclname);
> -        g_free(aclname);
> +        vd->sasl.authz = QAUTHZ(qauthz_list_new(vd->sasl.authzid,
> +                                                QAUTHZ_LIST_POLICY_DENY,
> +                                                &error_abort));
>       }
>   #endif
>   
> diff --git a/util/acl.c b/util/acl.c
> deleted file mode 100644
> index c105addadc..0000000000
> --- a/util/acl.c
> +++ /dev/null
> @@ -1,179 +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;
> -}
> diff --git a/crypto/trace-events b/crypto/trace-events
> index 597389b73c..a38ad7b787 100644
> --- a/crypto/trace-events
> +++ b/crypto/trace-events
> @@ -19,5 +19,5 @@ qcrypto_tls_creds_x509_load_cert(void *creds, int isServer, const char *file) "T
>   qcrypto_tls_creds_x509_load_cert_list(void *creds, const char *file) "TLS creds x509 load cert list creds=%p file=%s"
>   
>   # crypto/tlssession.c
> -qcrypto_tls_session_new(void *session, void *creds, const char *hostname, const char *aclname, int endpoint) "TLS session new session=%p creds=%p hostname=%s aclname=%s endpoint=%d"
> +qcrypto_tls_session_new(void *session, void *creds, const char *hostname, const char *authzid, int endpoint) "TLS session new session=%p creds=%p hostname=%s authzid=%s endpoint=%d"
>   qcrypto_tls_session_check_creds(void *session, const char *status) "TLS session check creds session=%p status=%s"
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index b2369c14cb..e8ceb4e5f6 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -510,8 +510,8 @@ 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-events.o tests/test-qapi-introspect.o \
>   	$(test-qom-obj-y)
> -benchmark-crypto-obj-y = $(crypto-obj-y) $(test-qom-obj-y)
> -test-crypto-obj-y = $(crypto-obj-y) $(test-qom-obj-y)
> +benchmark-crypto-obj-y = $(authz-obj-y) $(crypto-obj-y) $(test-qom-obj-y)
> +test-crypto-obj-y = $(authz-obj-y) $(crypto-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) tests/iothread.o
>   
> diff --git a/util/Makefile.objs b/util/Makefile.objs
> index 4d7675d6e7..ca99e7e90d 100644
> --- a/util/Makefile.objs
> +++ b/util/Makefile.objs
> @@ -20,7 +20,6 @@ util-obj-y += envlist.o path.o module.o
>   util-obj-y += 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 += cacheinfo.o
>   util-obj-y += error.o qemu-error.o
>   util-obj-y += id.o
> 

Re: [Qemu-devel] [PATCH v6 11/11] authz: delete existing ACL implementation
Posted by Marc-André Lureau 7 years, 3 months ago
Hi

On Fri, Oct 19, 2018 at 5:51 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> From: "Daniel P. Berrange" <berrange@redhat.com>
>
> 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 subclasses 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 temporary backwards compatibility.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  include/qemu/acl.h             |  66 ------------
>  ui/vnc-auth-sasl.h             |   5 +-
>  ui/vnc.h                       |   4 +-
>  crypto/tlssession.c            |  35 +++----
>  monitor.c                      | 185 ++++++++++++++++++++++-----------
>  tests/test-crypto-tlssession.c |  15 ++-
>  tests/test-io-channel-tls.c    |  16 ++-
>  ui/vnc-auth-sasl.c             |  23 ++--
>  ui/vnc-auth-vencrypt.c         |   2 +-
>  ui/vnc-ws.c                    |   2 +-
>  ui/vnc.c                       |  37 ++++---
>  util/acl.c                     | 179 -------------------------------
>  crypto/trace-events            |   2 +-
>  tests/Makefile.include         |   4 +-
>  util/Makefile.objs             |   1 -
>  15 files changed, 215 insertions(+), 361 deletions(-)
>  delete mode 100644 include/qemu/acl.h
>  delete mode 100644 util/acl.c
>
> diff --git a/include/qemu/acl.h b/include/qemu/acl.h
> deleted file mode 100644
> index 7c44119a47..0000000000
> --- a/include/qemu/acl.h
> +++ /dev/null
> @@ -1,66 +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 */
> diff --git a/ui/vnc-auth-sasl.h b/ui/vnc-auth-sasl.h
> index 2ae224ee3a..fb55fe04ca 100644
> --- a/ui/vnc-auth-sasl.h
> +++ b/ui/vnc-auth-sasl.h
> @@ -30,8 +30,8 @@
>  typedef struct VncStateSASL VncStateSASL;
>  typedef struct VncDisplaySASL VncDisplaySASL;
>
> -#include "qemu/acl.h"
>  #include "qemu/main-loop.h"
> +#include "authz/base.h"
>
>  struct VncStateSASL {
>      sasl_conn_t *conn;
> @@ -60,7 +60,8 @@ struct VncStateSASL {
>  };
>
>  struct VncDisplaySASL {
> -    qemu_acl *acl;
> +    QAuthZ *authz;
> +    char *authzid;
>  };
>
>  void vnc_sasl_client_cleanup(VncState *vs);
> diff --git a/ui/vnc.h b/ui/vnc.h
> index a86e0610e8..29ee1738a5 100644
> --- a/ui/vnc.h
> +++ b/ui/vnc.h
> @@ -39,6 +39,7 @@
>  #include "io/channel-socket.h"
>  #include "io/channel-tls.h"
>  #include "io/net-listener.h"
> +#include "authz/base.h"
>  #include <zlib.h>
>
>  #include "keymaps.h"
> @@ -177,7 +178,8 @@ struct VncDisplay
>      bool lossy;
>      bool non_adaptive;
>      QCryptoTLSCreds *tlscreds;
> -    char *tlsaclname;
> +    QAuthZ *tlsauthz;
> +    char *tlsauthzid;
>  #ifdef CONFIG_VNC_SASL
>      VncDisplaySASL sasl;
>  #endif
> diff --git a/crypto/tlssession.c b/crypto/tlssession.c
> index 66a6fbe19c..23842aa95d 100644
> --- a/crypto/tlssession.c
> +++ b/crypto/tlssession.c
> @@ -24,7 +24,7 @@
>  #include "crypto/tlscredspsk.h"
>  #include "crypto/tlscredsx509.h"
>  #include "qapi/error.h"
> -#include "qemu/acl.h"
> +#include "authz/base.h"
>  #include "trace.h"
>
>  #ifdef CONFIG_GNUTLS
> @@ -37,7 +37,7 @@ struct QCryptoTLSSession {
>      QCryptoTLSCreds *creds;
>      gnutls_session_t handle;
>      char *hostname;
> -    char *aclname;
> +    char *authzid;
>      bool handshakeComplete;
>      QCryptoTLSSessionWriteFunc writeFunc;
>      QCryptoTLSSessionReadFunc readFunc;
> @@ -56,7 +56,7 @@ qcrypto_tls_session_free(QCryptoTLSSession *session)
>      gnutls_deinit(session->handle);
>      g_free(session->hostname);
>      g_free(session->peername);
> -    g_free(session->aclname);
> +    g_free(session->authzid);
>      object_unref(OBJECT(session->creds));
>      g_free(session);
>  }
> @@ -101,7 +101,7 @@ qcrypto_tls_session_pull(void *opaque, void *buf, size_t len)
>  QCryptoTLSSession *
>  qcrypto_tls_session_new(QCryptoTLSCreds *creds,
>                          const char *hostname,
> -                        const char *aclname,
> +                        const char *authzid,
>                          QCryptoTLSCredsEndpoint endpoint,
>                          Error **errp)
>  {
> @@ -111,13 +111,13 @@ qcrypto_tls_session_new(QCryptoTLSCreds *creds,
>      session = g_new0(QCryptoTLSSession, 1);
>      trace_qcrypto_tls_session_new(
>          session, creds, hostname ? hostname : "<none>",
> -        aclname ? aclname : "<none>", endpoint);
> +        authzid ? authzid : "<none>", endpoint);
>
>      if (hostname) {
>          session->hostname = g_strdup(hostname);
>      }
> -    if (aclname) {
> -        session->aclname = g_strdup(aclname);
> +    if (authzid) {
> +        session->authzid = g_strdup(authzid);
>      }
>      session->creds = creds;
>      object_ref(OBJECT(creds));
> @@ -268,6 +268,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)) {
> @@ -355,19 +356,17 @@ qcrypto_tls_session_check_certificate(QCryptoTLSSession *session,
>                             gnutls_strerror(ret));
>                  goto error;
>              }
> -            if (session->aclname) {
> -                qemu_acl *acl = qemu_acl_find(session->aclname);
> -                int allow;
> -                if (!acl) {
> -                    error_setg(errp, "Cannot find ACL %s",
> -                               session->aclname);
> +            if (session->authzid) {
> +                bool allow;
> +
> +                allow = qauthz_is_allowed_by_id(session->authzid,
> +                                                session->peername, &err);
> +                if (err) {
> +                    error_propagate(errp, err);
>                      goto error;
>                  }
> -
> -                allow = qemu_acl_party_is_allowed(acl, session->peername);
> -
>                  if (!allow) {
> -                    error_setg(errp, "TLS x509 ACL check for %s is denied",
> +                    error_setg(errp, "TLS x509 authz check for %s is denied",
>                                 session->peername);
>                      goto error;
>                  }
> @@ -558,7 +557,7 @@ qcrypto_tls_session_get_peer_name(QCryptoTLSSession *session)
>  QCryptoTLSSession *
>  qcrypto_tls_session_new(QCryptoTLSCreds *creds G_GNUC_UNUSED,
>                          const char *hostname G_GNUC_UNUSED,
> -                        const char *aclname G_GNUC_UNUSED,
> +                        const char *authzid G_GNUC_UNUSED,
>                          QCryptoTLSCredsEndpoint endpoint G_GNUC_UNUSED,
>                          Error **errp)
>  {
> diff --git a/monitor.c b/monitor.c
> index b9258a7438..2458322eb1 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -51,7 +51,8 @@
>  #include "sysemu/balloon.h"
>  #include "qemu/timer.h"
>  #include "sysemu/hw_accel.h"
> -#include "qemu/acl.h"
> +#include "authz/list.h"
> +#include "qapi/util.h"
>  #include "sysemu/tpm.h"
>  #include "qapi/qmp/qdict.h"
>  #include "qapi/qmp/qerror.h"
> @@ -2040,93 +2041,154 @@ 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 QAuthZList *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_LIST(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);
> -        }
> +    QAuthZList *auth = find_auth(mon, aclname);
> +    QAuthZListRuleList *rules;
> +    size_t i = 0;
> +
> +    if (!auth) {
> +        return;
> +    }
> +
> +    monitor_printf(mon, "policy: %s\n",
> +                   QAuthZListPolicy_lookup.array[auth->policy]);

please use QAuthZListPolicy_str()

> +
> +    rules = auth->rules;
> +    while (rules) {
> +        QAuthZListRule *rule = rules->value;
> +        i++;
> +        monitor_printf(mon, "%zu: %s %s\n", i,
> +                       QAuthZListPolicy_lookup.array[rule->policy],

QAuthZListPolicy_str

> +                       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);
> +    QAuthZList *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_LIST_POLICY_DENY;
> +    qapi_free_QAuthZListRuleList(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);
> +    QAuthZList *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(&QAuthZListPolicy_lookup,
> +                          policy,
> +                          QAUTHZ_LIST_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_LIST_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");
>          }
>      }
>  }
>
> +static QAuthZListFormat hmp_acl_get_format(const char *match)
> +{
> +#ifdef CONFIG_FNMATCH
> +    if (strchr(match, '*')) {
> +        return QAUTHZ_LIST_FORMAT_GLOB;
> +    } else {
> +        return QAUTHZ_LIST_FORMAT_EXACT;
> +    }
> +#else
> +    /* Historically we silently degraded to plain strcmp
> +     * when fnmatch() was missing */
> +    return QAUTHZ_LIST_FORMAT_EXACT;
> +#endif
> +}
> +
>  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);
> +    QAuthZList *auth = find_auth(mon, aclname);
> +    Error *err = NULL;
> +    QAuthZListPolicy policy;
> +    QAuthZListFormat format;
> +    size_t i = 0;
> +
> +    if (!auth) {
> +        return;
> +    }
> +
> +    policy = qapi_enum_parse(&QAuthZListPolicy_lookup,
> +                             policystr,
> +                             QAUTHZ_LIST_POLICY_DENY,
> +                             &err);
> +    if (err) {
> +        error_free(err);
> +        monitor_printf(mon, "acl: unknown policy '%s', "
> +                       "expected 'deny' or 'allow'\n", policystr);
> +        return;
> +    }
> +
> +    format = hmp_acl_get_format(match);
> +
> +    if (has_index && index == 0) {
> +        monitor_printf(mon, "acl: unable to add acl entry\n");
> +        return;
> +    }
> +
> +    if (has_index) {
> +        i = qauthz_list_insert_rule(auth, match, policy,
> +                                    format, index - 1, &err);
> +    } else {
> +        i = qauthz_list_append_rule(auth, match, policy,
> +                                    format, &err);
> +    }
> +    if (err) {
> +        monitor_printf(mon, "acl: unable to add rule: %s",
> +                       error_get_pretty(err));
> +        error_free(err);
> +    } else {
> +        monitor_printf(mon, "acl: added rule at position %zu\n", i + 1);
>      }
>  }
>
> @@ -2134,15 +2196,18 @@ 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;
> +    QAuthZList *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_list_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/test-crypto-tlssession.c b/tests/test-crypto-tlssession.c
> index 6fa9950afb..15212ec276 100644
> --- a/tests/test-crypto-tlssession.c
> +++ b/tests/test-crypto-tlssession.c
> @@ -28,7 +28,7 @@
>  #include "qom/object_interfaces.h"
>  #include "qapi/error.h"
>  #include "qemu/sockets.h"
> -#include "qemu/acl.h"
> +#include "authz/list.h"
>
>  #ifdef QCRYPTO_HAVE_TLS_TEST_SUPPORT
>
> @@ -229,7 +229,7 @@ static void test_crypto_tls_session_x509(const void *opaque)
>      QCryptoTLSCreds *serverCreds;
>      QCryptoTLSSession *clientSess = NULL;
>      QCryptoTLSSession *serverSess = NULL;
> -    qemu_acl *acl;
> +    QAuthZList *auth;
>      const char * const *wildcards;
>      int channel[2];
>      bool clientShake = false;
> @@ -285,11 +285,15 @@ static void test_crypto_tls_session_x509(const void *opaque)
>          SERVER_CERT_DIR);
>      g_assert(serverCreds != NULL);
>
> -    acl = qemu_acl_init("tlssessionacl");
> -    qemu_acl_reset(acl);
> +    auth = qauthz_list_new("tlssessionacl",
> +                           QAUTHZ_LIST_POLICY_DENY,
> +                           &error_abort);
>      wildcards = data->wildcards;
>      while (wildcards && *wildcards) {
> -        qemu_acl_append(acl, 0, *wildcards);
> +        qauthz_list_append_rule(auth, *wildcards,
> +                                QAUTHZ_LIST_POLICY_ALLOW,
> +                                QAUTHZ_LIST_FORMAT_GLOB,
> +                                &error_abort);
>          wildcards++;
>      }
>
> @@ -377,6 +381,7 @@ static void test_crypto_tls_session_x509(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 4900c6d433..43b707eba7 100644
> --- a/tests/test-io-channel-tls.c
> +++ b/tests/test-io-channel-tls.c
> @@ -29,8 +29,8 @@
>  #include "io-channel-helpers.h"
>  #include "crypto/init.h"
>  #include "crypto/tlscredsx509.h"
> -#include "qemu/acl.h"
>  #include "qapi/error.h"
> +#include "authz/list.h"
>  #include "qom/object_interfaces.h"
>
>  #ifdef QCRYPTO_HAVE_TLS_TEST_SUPPORT
> @@ -113,7 +113,7 @@ static void test_io_channel_tls(const void *opaque)
>      QIOChannelTLS *serverChanTLS;
>      QIOChannelSocket *clientChanSock;
>      QIOChannelSocket *serverChanSock;
> -    qemu_acl *acl;
> +    QAuthZList *auth;
>      const char * const *wildcards;
>      int channel[2];
>      struct QIOChannelTLSHandshakeData clientHandshake = { false, false };
> @@ -161,11 +161,15 @@ static void test_io_channel_tls(const void *opaque)
>          SERVER_CERT_DIR);
>      g_assert(serverCreds != NULL);
>
> -    acl = qemu_acl_init("channeltlsacl");
> -    qemu_acl_reset(acl);
> +    auth = qauthz_list_new("channeltlsacl",
> +                           QAUTHZ_LIST_POLICY_DENY,
> +                           &error_abort);
>      wildcards = data->wildcards;
>      while (wildcards && *wildcards) {
> -        qemu_acl_append(acl, 0, *wildcards);
> +        qauthz_list_append_rule(auth, *wildcards,
> +                                QAUTHZ_LIST_POLICY_ALLOW,
> +                                QAUTHZ_LIST_FORMAT_GLOB,
> +                                &error_abort);
>          wildcards++;
>      }
>
> @@ -253,6 +257,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 3751a777a4..7b2b09f242 100644
> --- a/ui/vnc-auth-sasl.c
> +++ b/ui/vnc-auth-sasl.c
> @@ -24,6 +24,7 @@
>
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
> +#include "authz/base.h"
>  #include "vnc.h"
>  #include "trace.h"
>
> @@ -146,13 +147,14 @@ size_t vnc_client_read_sasl(VncState *vs)
>  static int vnc_auth_sasl_check_access(VncState *vs)
>  {
>      const void *val;
> -    int err;
> -    int allow;
> +    int rv;
> +    Error *err = NULL;
> +    bool allow;
>
> -    err = sasl_getprop(vs->sasl.conn, SASL_USERNAME, &val);
> -    if (err != SASL_OK) {
> +    rv = sasl_getprop(vs->sasl.conn, SASL_USERNAME, &val);
> +    if (rv != SASL_OK) {
>          trace_vnc_auth_fail(vs, vs->auth, "Cannot fetch SASL username",
> -                            sasl_errstring(err, NULL, NULL));
> +                            sasl_errstring(rv, NULL, NULL));
>          return -1;
>      }
>      if (val == NULL) {
> @@ -163,12 +165,19 @@ static int vnc_auth_sasl_check_access(VncState *vs)
>      vs->sasl.username = g_strdup((const char*)val);
>      trace_vnc_auth_sasl_username(vs, vs->sasl.username);
>
> -    if (vs->vd->sasl.acl == NULL) {
> +    if (vs->vd->sasl.authzid == NULL) {
>          trace_vnc_auth_sasl_acl(vs, 1);
>          return 0;
>      }
>
> -    allow = qemu_acl_party_is_allowed(vs->vd->sasl.acl, vs->sasl.username);
> +    allow = qauthz_is_allowed_by_id(vs->vd->sasl.authzid,
> +                                    vs->sasl.username, &err);

Why not use qauthz_is_allowed() with .authz ?

> +    if (err) {
> +        trace_vnc_auth_fail(vs, vs->auth, "Error from authz",
> +                            error_get_pretty(err));
> +        error_free(err);
> +        return -1;
> +    }
>
>      trace_vnc_auth_sasl_acl(vs, allow);
>      return allow ? 0 : -1;
> diff --git a/ui/vnc-auth-vencrypt.c b/ui/vnc-auth-vencrypt.c
> index d99ea362c1..f072e16ace 100644
> --- a/ui/vnc-auth-vencrypt.c
> +++ b/ui/vnc-auth-vencrypt.c
> @@ -109,7 +109,7 @@ static int protocol_client_vencrypt_auth(VncState *vs, uint8_t *data, size_t len
>          tls = qio_channel_tls_new_server(
>              vs->ioc,
>              vs->vd->tlscreds,
> -            vs->vd->tlsaclname,
> +            vs->vd->tlsauthzid,
>              &err);
>          if (!tls) {
>              trace_vnc_auth_fail(vs, vs->auth, "TLS setup failed",
> diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
> index 950f1cd2ac..95c9703c72 100644
> --- a/ui/vnc-ws.c
> +++ b/ui/vnc-ws.c
> @@ -62,7 +62,7 @@ gboolean vncws_tls_handshake_io(QIOChannel *ioc G_GNUC_UNUSED,
>      tls = qio_channel_tls_new_server(
>          vs->ioc,
>          vs->vd->tlscreds,
> -        vs->vd->tlsaclname,
> +        vs->vd->tlsauthzid,
>          &err);
>      if (!tls) {
>          VNC_DEBUG("Failed to setup TLS %s\n", error_get_pretty(err));
> diff --git a/ui/vnc.c b/ui/vnc.c
> index cf221c83cc..60cb7c2d3d 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -33,7 +33,7 @@
>  #include "qemu/option.h"
>  #include "qemu/sockets.h"
>  #include "qemu/timer.h"
> -#include "qemu/acl.h"
> +#include "authz/list.h"
>  #include "qemu/config-file.h"
>  #include "qapi/qapi-events.h"
>  #include "qapi/error.h"
> @@ -3267,12 +3267,24 @@ static void vnc_display_close(VncDisplay *vd)
>          object_unparent(OBJECT(vd->tlscreds));
>          vd->tlscreds = NULL;
>      }
> -    g_free(vd->tlsaclname);
> -    vd->tlsaclname = NULL;
> +    if (vd->tlsauthz) {
> +        object_unparent(OBJECT(vd->tlsauthz));
> +        vd->tlsauthz = NULL;
> +    }
> +    g_free(vd->tlsauthzid);
> +    vd->tlsauthzid = NULL;
>      if (vd->lock_key_sync) {
>          qemu_remove_led_event_handler(vd->led);
>          vd->led = NULL;
>      }
> +#ifdef CONFIG_VNC_SASL
> +    if (vd->sasl.authz) {
> +        object_unparent(OBJECT(vd->sasl.authz));
> +        vd->sasl.authz = NULL;
> +    }
> +    g_free(vd->sasl.authzid);
> +    vd->sasl.authzid = NULL;
> +#endif
>  }
>
>  int vnc_display_password(const char *id, const char *password)
> @@ -3925,23 +3937,24 @@ void vnc_display_open(const char *id, Error **errp)
>
>      if (acl) {
>          if (strcmp(vd->id, "default") == 0) {
> -            vd->tlsaclname = g_strdup("vnc.x509dname");
> +            vd->tlsauthzid = g_strdup("vnc.x509dname");
>          } else {
> -            vd->tlsaclname = g_strdup_printf("vnc.%s.x509dname", vd->id);
> +            vd->tlsauthzid = g_strdup_printf("vnc.%s.x509dname", vd->id);
>          }
> -        qemu_acl_init(vd->tlsaclname);
> +        vd->tlsauthz = QAUTHZ(qauthz_list_new(vd->tlsauthzid,
> +                                              QAUTHZ_LIST_POLICY_DENY,
> +                                              &error_abort));
>      }
>  #ifdef CONFIG_VNC_SASL
>      if (acl && sasl) {
> -        char *aclname;
> -
>          if (strcmp(vd->id, "default") == 0) {
> -            aclname = g_strdup("vnc.username");
> +            vd->sasl.authzid = g_strdup("vnc.username");
>          } else {
> -            aclname = g_strdup_printf("vnc.%s.username", vd->id);
> +            vd->sasl.authzid = g_strdup_printf("vnc.%s.username", vd->id);
>          }
> -        vd->sasl.acl = qemu_acl_init(aclname);
> -        g_free(aclname);
> +        vd->sasl.authz = QAUTHZ(qauthz_list_new(vd->sasl.authzid,
> +                                                QAUTHZ_LIST_POLICY_DENY,
> +                                                &error_abort));
>      }
>  #endif
>
> diff --git a/util/acl.c b/util/acl.c
> deleted file mode 100644
> index c105addadc..0000000000
> --- a/util/acl.c
> +++ /dev/null
> @@ -1,179 +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;
> -}
> diff --git a/crypto/trace-events b/crypto/trace-events
> index 597389b73c..a38ad7b787 100644
> --- a/crypto/trace-events
> +++ b/crypto/trace-events
> @@ -19,5 +19,5 @@ qcrypto_tls_creds_x509_load_cert(void *creds, int isServer, const char *file) "T
>  qcrypto_tls_creds_x509_load_cert_list(void *creds, const char *file) "TLS creds x509 load cert list creds=%p file=%s"
>
>  # crypto/tlssession.c
> -qcrypto_tls_session_new(void *session, void *creds, const char *hostname, const char *aclname, int endpoint) "TLS session new session=%p creds=%p hostname=%s aclname=%s endpoint=%d"
> +qcrypto_tls_session_new(void *session, void *creds, const char *hostname, const char *authzid, int endpoint) "TLS session new session=%p creds=%p hostname=%s authzid=%s endpoint=%d"
>  qcrypto_tls_session_check_creds(void *session, const char *status) "TLS session check creds session=%p status=%s"
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index b2369c14cb..e8ceb4e5f6 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -510,8 +510,8 @@ 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-events.o tests/test-qapi-introspect.o \
>         $(test-qom-obj-y)
> -benchmark-crypto-obj-y = $(crypto-obj-y) $(test-qom-obj-y)
> -test-crypto-obj-y = $(crypto-obj-y) $(test-qom-obj-y)
> +benchmark-crypto-obj-y = $(authz-obj-y) $(crypto-obj-y) $(test-qom-obj-y)
> +test-crypto-obj-y = $(authz-obj-y) $(crypto-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) tests/iothread.o
>
> diff --git a/util/Makefile.objs b/util/Makefile.objs
> index 4d7675d6e7..ca99e7e90d 100644
> --- a/util/Makefile.objs
> +++ b/util/Makefile.objs
> @@ -20,7 +20,6 @@ util-obj-y += envlist.o path.o module.o
>  util-obj-y += 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 += cacheinfo.o
>  util-obj-y += error.o qemu-error.o
>  util-obj-y += id.o
> --
> 2.17.2
>
>


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v6 11/11] authz: delete existing ACL implementation
Posted by Daniel P. Berrangé 7 years, 2 months ago
On Thu, Nov 08, 2018 at 12:15:54PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Oct 19, 2018 at 5:51 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > From: "Daniel P. Berrange" <berrange@redhat.com>
> >
> > 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 subclasses 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 temporary backwards compatibility.
> >
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > +    monitor_printf(mon, "policy: %s\n",
> > +                   QAuthZListPolicy_lookup.array[auth->policy]);
> 
> please use QAuthZListPolicy_str()
> 
> > +
> > +    rules = auth->rules;
> > +    while (rules) {
> > +        QAuthZListRule *rule = rules->value;
> > +        i++;
> > +        monitor_printf(mon, "%zu: %s %s\n", i,
> > +                       QAuthZListPolicy_lookup.array[rule->policy],
> 
> QAuthZListPolicy_str

Yes.


> > @@ -163,12 +165,19 @@ static int vnc_auth_sasl_check_access(VncState *vs)
> >      vs->sasl.username = g_strdup((const char*)val);
> >      trace_vnc_auth_sasl_username(vs, vs->sasl.username);
> >
> > -    if (vs->vd->sasl.acl == NULL) {
> > +    if (vs->vd->sasl.authzid == NULL) {
> >          trace_vnc_auth_sasl_acl(vs, 1);
> >          return 0;
> >      }
> >
> > -    allow = qemu_acl_party_is_allowed(vs->vd->sasl.acl, vs->sasl.username);
> > +    allow = qauthz_is_allowed_by_id(vs->vd->sasl.authzid,
> > +                                    vs->sasl.username, &err);
> 
> Why not use qauthz_is_allowed() with .authz ?

The .authz object is only non-NULL when using the legacy "-vnc ..,acl"
flag syntax. When using the modern syntax (introduced by the followup
series mentioned in the cover letter) we want to resolve "authzid"
every time. This allows the user to safely delete & recreate the
authorization objects on the fly.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|