summary refs log tree commit diff stats
path: root/ui/vnc-auth-sasl.c
diff options
context:
space:
mode:
authorPeter Maydell <peter.maydell@linaro.org>2024-10-24 11:23:38 +0100
committerPeter Maydell <peter.maydell@linaro.org>2024-10-24 11:23:38 +0100
commite51d8fbb7e673e487e98327fc067700b5a3edf30 (patch)
treefed50995641692b53c858be30d3f485e49eaead2 /ui/vnc-auth-sasl.c
parent55522f72149fbf95ee3b057f1419da0cad46d0dd (diff)
parentc64df333f92798823c4897ae6d4bd7f49d060225 (diff)
downloadfocaccia-qemu-e51d8fbb7e673e487e98327fc067700b5a3edf30.tar.gz
focaccia-qemu-e51d8fbb7e673e487e98327fc067700b5a3edf30.zip
Merge tag 'misc-fixes-pull-request' of https://gitlab.com/berrange/qemu into staging
Misc sockets, crypto and VNC fixes

* Fix rare EADDRINUSE failures on OpenBSD platforms seen
  with migration
* Fix & test overwriting of hash output buffer
* Close connection instead of returning empty SASL mechlist to
  VNC clients
* Fix handling of SASL SSF on VNC server UNIX sockets
* Fix handling of NULL SASL server data in VNC server
* Validate trailing NUL padding byte from SASL client
* Fix & test AF_ALG crypto backend build
* Remove unused code in sockets and crypto subsystems

# -----BEGIN PGP SIGNATURE-----
#
# iQIzBAABCAAdFiEE2vOm/bJrYpEtDo4/vobrtBUQT98FAmcXscUACgkQvobrtBUQ
# T9+S+Q//W9fywFY42VnsPqIAi7Q+QPDvXrPVVQ1z817hcyxdMVWC+eAg97i3QsE8
# f/+nwrigV9CIv9jqdBdMUIRLm4XhyuDspksgBAQUJ1XYmmVSmFwh2ej31m/qI8fK
# fu0v6N6udkcg+5eoWEOL873hKAA+vjq30tM5Zp74fMHZahnvgjThgaJY6Z6OsCyX
# 6Pgxl3Z1gym1IqQFz0nOdTMnzsQrAJbV8z2FWMKgHayg01nVoXlo5FMnNgIdItJC
# v+4qX5sfRJIENJcRKMNY4dQUqbO1004+HXECLbge8pR7vsUli06xjLBkSbt/9M6r
# x3lfDGKavPrKfsPk1H+eTlge/43IjJk+mXMgZxmyvrvgnyVulxRvz7ABKJ+VBUeq
# CDrAuAK4tm5BIxKu6cg4CcmlqsDXwq6Sb+NdsbxTv0Deop73WZR3HCamRNU1JXkA
# eXBY4QSuVA96s5TnlfZWZytIY9NmyjN48ov+ly2fOkbv/xxoUNFBY8TApSJZ/Veo
# 4EvGlIfgxjv668n/2eyt67E00dGC3idTbaWYeGjgUKVyNPpxicDOnM3NTwMP3/0k
# DZbvfoJcwfhPVoFMdV7ZvJKA3i8v11HdaEI0urfjm5nJWbyik6+++skan9F/femL
# eRTnH2hr5sUV+eQAl2YhGuBElLmKf/HqTCeNs3lwrUQsnb9bPNc=
# =fK8K
# -----END PGP SIGNATURE-----
# gpg: Signature made Tue 22 Oct 2024 15:08:05 BST
# gpg:                using RSA key DAF3A6FDB26B62912D0E8E3FBE86EBB415104FDF
# gpg: Good signature from "Daniel P. Berrange <dan@berrange.com>" [full]
# gpg:                 aka "Daniel P. Berrange <berrange@redhat.com>" [full]
# Primary key fingerprint: DAF3 A6FD B26B 6291 2D0E  8E3F BE86 EBB4 1510 4FDF

* tag 'misc-fixes-pull-request' of https://gitlab.com/berrange/qemu:
  gitlab: enable afalg tests in fedora system test
  ui: validate NUL byte padding in SASL client data more strictly
  ui: fix handling of NULL SASL server data
  ui/vnc: don't check for SSF after SASL authentication on UNIX sockets
  ui/vnc: fix skipping SASL SSF on UNIX sockets
  ui/vnc: don't raise error formatting socket address for non-inet
  ui/vnc: don't return an empty SASL mechlist to the client
  crypto/hash-afalg: Fix broken build
  include/crypto: clarify @result/@result_len for hash/hmac APIs
  tests: correctly validate result buffer in hash/hmac tests
  crypto/hash: avoid overwriting user supplied result pointer
  util: don't set SO_REUSEADDR on client sockets
  sockets: Remove deadcode
  crypto: Remove unused DER string functions

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Diffstat (limited to 'ui/vnc-auth-sasl.c')
-rw-r--r--ui/vnc-auth-sasl.c75
1 files changed, 52 insertions, 23 deletions
diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c
index 47fdae5b21..3f4cfc471d 100644
--- a/ui/vnc-auth-sasl.c
+++ b/ui/vnc-auth-sasl.c
@@ -263,8 +263,14 @@ static int protocol_client_auth_sasl_step(VncState *vs, uint8_t *data, size_t le
     /* NB, distinction of NULL vs "" is *critical* in SASL */
     if (datalen) {
         clientdata = (char*)data;
-        clientdata[datalen-1] = '\0'; /* Wire includes '\0', but make sure */
-        datalen--; /* Don't count NULL byte when passing to _start() */
+        if (clientdata[datalen - 1] != '\0') {
+            trace_vnc_auth_fail(vs, vs->auth, "Malformed SASL client data",
+                                "Missing SASL NUL padding byte");
+            sasl_dispose(&vs->sasl.conn);
+            vs->sasl.conn = NULL;
+            goto authabort;
+        }
+        datalen--; /* Discard the extra NUL padding byte */
     }
 
     err = sasl_server_step(vs->sasl.conn,
@@ -289,9 +295,10 @@ static int protocol_client_auth_sasl_step(VncState *vs, uint8_t *data, size_t le
         goto authabort;
     }
 
-    if (serveroutlen) {
+    if (serverout) {
         vnc_write_u32(vs, serveroutlen + 1);
-        vnc_write(vs, serverout, serveroutlen + 1);
+        vnc_write(vs, serverout, serveroutlen);
+        vnc_write_u8(vs, '\0');
     } else {
         vnc_write_u32(vs, 0);
     }
@@ -384,8 +391,14 @@ static int protocol_client_auth_sasl_start(VncState *vs, uint8_t *data, size_t l
     /* NB, distinction of NULL vs "" is *critical* in SASL */
     if (datalen) {
         clientdata = (char*)data;
-        clientdata[datalen-1] = '\0'; /* Should be on wire, but make sure */
-        datalen--; /* Don't count NULL byte when passing to _start() */
+        if (clientdata[datalen - 1] != '\0') {
+            trace_vnc_auth_fail(vs, vs->auth,  "Malformed SASL client data",
+                                "Missing SASL NUL padding byte");
+            sasl_dispose(&vs->sasl.conn);
+            vs->sasl.conn = NULL;
+            goto authabort;
+        }
+        datalen--; /* Discard the extra NUL padding byte */
     }
 
     err = sasl_server_start(vs->sasl.conn,
@@ -410,9 +423,10 @@ static int protocol_client_auth_sasl_start(VncState *vs, uint8_t *data, size_t l
         goto authabort;
     }
 
-    if (serveroutlen) {
+    if (serverout) {
         vnc_write_u32(vs, serveroutlen + 1);
-        vnc_write(vs, serverout, serveroutlen + 1);
+        vnc_write(vs, serverout, serveroutlen);
+        vnc_write_u8(vs, '\0');
     } else {
         vnc_write_u32(vs, 0);
     }
@@ -524,13 +538,13 @@ static int protocol_client_auth_sasl_mechname_len(VncState *vs, uint8_t *data, s
     return 0;
 }
 
-static char *
+static int
 vnc_socket_ip_addr_string(QIOChannelSocket *ioc,
                           bool local,
+                          char **addrstr,
                           Error **errp)
 {
     SocketAddress *addr;
-    char *ret;
 
     if (local) {
         addr = qio_channel_socket_get_local_address(ioc, errp);
@@ -538,17 +552,24 @@ vnc_socket_ip_addr_string(QIOChannelSocket *ioc,
         addr = qio_channel_socket_get_remote_address(ioc, errp);
     }
     if (!addr) {
-        return NULL;
+        return -1;
     }
 
     if (addr->type != SOCKET_ADDRESS_TYPE_INET) {
-        error_setg(errp, "Not an inet socket type");
+        *addrstr = NULL;
         qapi_free_SocketAddress(addr);
-        return NULL;
+        return 0;
     }
-    ret = g_strdup_printf("%s;%s", addr->u.inet.host, addr->u.inet.port);
+    *addrstr = g_strdup_printf("%s;%s", addr->u.inet.host, addr->u.inet.port);
     qapi_free_SocketAddress(addr);
-    return ret;
+    return 0;
+}
+
+static bool
+vnc_socket_is_unix(QIOChannelSocket *ioc)
+{
+    SocketAddress *addr = qio_channel_socket_get_local_address(ioc, NULL);
+    return addr && addr->type == SOCKET_ADDRESS_TYPE_UNIX;
 }
 
 void start_auth_sasl(VncState *vs)
@@ -561,15 +582,15 @@ void start_auth_sasl(VncState *vs)
     int mechlistlen;
 
     /* Get local & remote client addresses in form  IPADDR;PORT */
-    localAddr = vnc_socket_ip_addr_string(vs->sioc, true, &local_err);
-    if (!localAddr) {
+    if (vnc_socket_ip_addr_string(vs->sioc, true,
+                                  &localAddr, &local_err) < 0) {
         trace_vnc_auth_fail(vs, vs->auth, "Cannot format local IP",
                             error_get_pretty(local_err));
         goto authabort;
     }
 
-    remoteAddr = vnc_socket_ip_addr_string(vs->sioc, false, &local_err);
-    if (!remoteAddr) {
+    if (vnc_socket_ip_addr_string(vs->sioc, false,
+                                  &remoteAddr, &local_err) < 0) {
         trace_vnc_auth_fail(vs, vs->auth, "Cannot format remote IP",
                             error_get_pretty(local_err));
         g_free(localAddr);
@@ -621,16 +642,17 @@ void start_auth_sasl(VncState *vs)
             goto authabort;
         }
     } else {
-        vs->sasl.wantSSF = 1;
+        vs->sasl.wantSSF = !vnc_socket_is_unix(vs->sioc);
     }
 
     memset (&secprops, 0, sizeof secprops);
     /* Inform SASL that we've got an external SSF layer from TLS.
      *
-     * Disable SSF, if using TLS+x509+SASL only. TLS without x509
-     * is not sufficiently strong
+     * Disable SSF, if using TLS+x509+SASL only, or UNIX sockets.
+     * TLS without x509 is not sufficiently strong, nor is plain
+     * TCP
      */
-    if (vs->vd->is_unix ||
+    if (vnc_socket_is_unix(vs->sioc) ||
         (vs->auth == VNC_AUTH_VENCRYPT &&
          vs->subauth == VNC_AUTH_VENCRYPT_X509SASL)) {
         /* If we've got TLS or UNIX domain sock, we don't care about SSF */
@@ -674,6 +696,13 @@ void start_auth_sasl(VncState *vs)
     }
     trace_vnc_auth_sasl_mech_list(vs, mechlist);
 
+    if (g_str_equal(mechlist, "")) {
+        trace_vnc_auth_fail(vs, vs->auth, "no available SASL mechanisms", "");
+        sasl_dispose(&vs->sasl.conn);
+        vs->sasl.conn = NULL;
+        goto authabort;
+    }
+
     vs->sasl.mechlist = g_strdup(mechlist);
     mechlistlen = strlen(mechlist);
     vnc_write_u32(vs, mechlistlen);