All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Yongji Xie" <elohimes@gmail.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Thomas Huth" <thuth@redhat.com>,
	"Laurent Vivier" <lvivier@redhat.com>
Subject: [Qemu-devel] [PATCH v3 03/16] chardev: fix validation of options for QMP created chardevs
Date: Mon, 11 Feb 2019 18:24:29 +0000	[thread overview]
Message-ID: <20190211182442.8542-4-berrange@redhat.com> (raw)
In-Reply-To: <20190211182442.8542-1-berrange@redhat.com>

The TLS creds option is not valid with certain address types. The user
config was only checked for errors when parsing legacy QemuOpts, thus
the user could pass unsupported values via QMP.

Pull all code for validating options out into a new method
qmp_chardev_validate_socket, that is called from the main
qmp_chardev_open_socket method. This adds a missing check for rejecting
TLS creds with the vsock address type.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 chardev/char-socket.c | 92 +++++++++++++++++++++++++++++++------------
 1 file changed, 66 insertions(+), 26 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index eaa8e8b68f..e85250b624 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -987,6 +987,65 @@ static gboolean socket_reconnect_timeout(gpointer opaque)
     return false;
 }
 
+
+static bool qmp_chardev_validate_socket(ChardevSocket *sock,
+                                        SocketAddress *addr,
+                                        Error **errp)
+{
+    /* Validate any options which have a dependency on address type */
+    switch (addr->type) {
+    case SOCKET_ADDRESS_TYPE_FD:
+        if (sock->has_reconnect) {
+            error_setg(errp,
+                       "'reconnect' option is incompatible with "
+                       "'fd' address type");
+            return false;
+        }
+        if (sock->has_tls_creds &&
+            !(sock->has_server && sock->server)) {
+            error_setg(errp,
+                       "'tls_creds' option is incompatible with "
+                       "'fd' address type as client");
+            return false;
+        }
+        break;
+
+    case SOCKET_ADDRESS_TYPE_UNIX:
+        if (sock->has_tls_creds) {
+            error_setg(errp,
+                       "'tls_creds' option is incompatible with "
+                       "'unix' address type");
+            return false;
+        }
+        break;
+
+    case SOCKET_ADDRESS_TYPE_INET:
+        break;
+
+    case SOCKET_ADDRESS_TYPE_VSOCK:
+        if (sock->has_tls_creds) {
+            error_setg(errp,
+                       "'tls_creds' option is incompatible with "
+                       "'vsock' address type");
+            return false;
+        }
+
+    default:
+        break;
+    }
+
+    /* Validate any options which have a dependancy on client vs server */
+    if (!(sock->has_server && sock->server)) {
+        if (sock->has_websocket && sock->websocket) {
+            error_setg(errp, "%s", "Websocket client is not implemented");
+            return false;
+        }
+    }
+
+    return true;
+}
+
+
 static void qmp_chardev_open_socket(Chardev *chr,
                                     ChardevBackend *backend,
                                     bool *be_opened,
@@ -1004,11 +1063,6 @@ static void qmp_chardev_open_socket(Chardev *chr,
     QIOChannelSocket *sioc = NULL;
     SocketAddress *addr;
 
-    if (!is_listen && is_websock) {
-        error_setg(errp, "%s", "Websocket client is not implemented");
-        goto error;
-    }
-
     s->is_listen = is_listen;
     s->is_telnet = is_telnet;
     s->is_tn3270 = is_tn3270;
@@ -1049,10 +1103,10 @@ static void qmp_chardev_open_socket(Chardev *chr,
 
     s->addr = addr = socket_address_flatten(sock->addr);
 
-    if (sock->has_reconnect && addr->type == SOCKET_ADDRESS_TYPE_FD) {
-        error_setg(errp, "'reconnect' option is incompatible with 'fd'");
+    if (!qmp_chardev_validate_socket(sock, addr, errp)) {
         goto error;
     }
+
     qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_RECONNECTABLE);
     /* TODO SOCKET_ADDRESS_FD where fd has AF_UNIX */
     if (addr->type == SOCKET_ADDRESS_TYPE_UNIX) {
@@ -1140,27 +1194,12 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
         return;
     }
 
-    backend->type = CHARDEV_BACKEND_KIND_SOCKET;
-    if (path) {
-        if (tls_creds) {
-            error_setg(errp, "TLS can only be used over TCP socket");
-            return;
-        }
-    } else if (host) {
-        if (!port) {
-            error_setg(errp, "chardev: socket: no port given");
-            return;
-        }
-    } else if (fd) {
-        /* We don't know what host to validate against when in client mode */
-        if (tls_creds && !is_listen) {
-            error_setg(errp, "TLS can not be used with pre-opened client FD");
-            return;
-        }
-    } else {
-        g_assert_not_reached();
+    if (host && !port) {
+        error_setg(errp, "chardev: socket: no port given");
+        return;
     }
 
+    backend->type = CHARDEV_BACKEND_KIND_SOCKET;
     sock = backend->u.socket.data = g_new0(ChardevSocket, 1);
     qemu_chr_parse_common(opts, qapi_ChardevSocket_base(sock));
 
@@ -1178,6 +1217,7 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
     sock->wait = is_waitconnect;
     sock->has_reconnect = qemu_opt_find(opts, "reconnect");
     sock->reconnect = reconnect;
+    sock->has_tls_creds = tls_creds;
     sock->tls_creds = g_strdup(tls_creds);
 
     addr = g_new0(SocketAddressLegacy, 1);
-- 
2.20.1

  parent reply	other threads:[~2019-02-11 18:25 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-11 18:24 [Qemu-devel] [PATCH v3 00/16] chardev: refactoring & many bugfixes related tcp_chr_wait_connected Daniel P. Berrangé
2019-02-11 18:24 ` [Qemu-devel] [PATCH v3 01/16] io: store reference to thread information in the QIOTask struct Daniel P. Berrangé
2019-02-11 18:24 ` [Qemu-devel] [PATCH v3 02/16] io: add qio_task_wait_thread to join with a background thread Daniel P. Berrangé
2019-02-11 18:24 ` Daniel P. Berrangé [this message]
2019-02-11 18:24 ` [Qemu-devel] [PATCH v3 04/16] chardev: forbid 'reconnect' option with server sockets Daniel P. Berrangé
2019-02-11 18:24 ` [Qemu-devel] [PATCH v3 05/16] chardev: forbid 'wait' option with client sockets Daniel P. Berrangé
2019-02-11 18:24 ` [Qemu-devel] [PATCH v3 06/16] chardev: remove many local variables in qemu_chr_parse_socket Daniel P. Berrangé
2019-02-11 18:24 ` [Qemu-devel] [PATCH v3 07/16] chardev: ensure qemu_chr_parse_compat reports missing driver error Daniel P. Berrangé
2019-02-11 18:24 ` [Qemu-devel] [PATCH v3 08/16] chardev: remove unused 'sioc' variable & cleanup paths Daniel P. Berrangé
2019-02-11 18:24 ` [Qemu-devel] [PATCH v3 09/16] chardev: split tcp_chr_wait_connected into two methods Daniel P. Berrangé
2019-02-11 18:24 ` [Qemu-devel] [PATCH v3 10/16] chardev: split up qmp_chardev_open_socket connection code Daniel P. Berrangé
2019-02-11 18:24 ` [Qemu-devel] [PATCH v3 11/16] chardev: use a state machine for socket connection state Daniel P. Berrangé
2019-02-11 18:24 ` [Qemu-devel] [PATCH v3 12/16] chardev: honour the reconnect setting in tcp_chr_wait_connected Daniel P. Berrangé
2019-02-11 18:24 ` [Qemu-devel] [PATCH v3 13/16] chardev: disallow TLS/telnet/websocket with tcp_chr_wait_connected Daniel P. Berrangé
2019-02-11 18:24 ` [Qemu-devel] [PATCH v3 14/16] chardev: fix race with client connections in tcp_chr_wait_connected Daniel P. Berrangé
2019-02-11 18:24 ` [Qemu-devel] [PATCH v3 15/16] tests: expand coverage of socket chardev test Daniel P. Berrangé
2019-02-11 18:24 ` [Qemu-devel] [PATCH v3 16/16] chardev: ensure termios is fully initialized Daniel P. Berrangé
2019-04-22 14:51 ` [Qemu-devel] [PATCH v3 00/16] chardev: refactoring & many bugfixes related tcp_chr_wait_connected Eric Blake
2019-04-22 14:51   ` Eric Blake
2019-04-23 14:13   ` Daniel P. Berrangé
2019-04-23 14:13     ` Daniel P. Berrangé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190211182442.8542-4-berrange@redhat.com \
    --to=berrange@redhat.com \
    --cc=elohimes@gmail.com \
    --cc=lvivier@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.