From: David Woodhouse <dwmw2@infradead.org>
Date: Mon, 10 Jun 2019 12:53:14 +0000 (+0100)
Subject: pulse: Handle multiple IF-T/TLS records in a single SSL record
X-Git-Tag: v8.04~30
X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=a60d1cf8ef4f44ff548dea0b66612411b1d6ee68;p=users%2Fdwmw2%2Fopenconnect.git

pulse: Handle multiple IF-T/TLS records in a single SSL record

We are still assuming that IT-F/TLS record won't be *split* between SSL
records. That turned out to be a false assumption for Network Connect,
but hopefully they're saner here. We should cleanly complain about that
if it does happen.

There may be better ways to do this; perhaps we should receive the whole SSL
record then handle each record separately. In the common case there's no
real reason for the incoming packet queue anyway. We could just call
os_write_tun() directly. And then only have to resort to memcpy to
split packets up in the very rare case that the tun isn't taking writes
anyway.

This will do for now. The TCP connection *shouldn't* be the fast path
anyway. Not that we've worked out how to make the Pulse server actually
send data in ESP; even with the Windows client it still sends in TCP...

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
---

diff --git a/pulse.c b/pulse.c
index 426d0bbe..af60257a 100644
--- a/pulse.c
+++ b/pulse.c
@@ -1959,7 +1959,8 @@ int pulse_mainloop(struct openconnect_info *vpninfo, int *timeout, int readable)
 			}
 		}
 
-		len = ssl_nonblock_read(vpninfo, &pkt->pulse.vendor, receive_mtu + 16);
+		/* Receive packet header, if there's anything there... */
+		len = ssl_nonblock_read(vpninfo, &pkt->pulse.vendor, 16);
 		if (!len)
 			break;
 		if (len < 0)
@@ -1970,18 +1971,37 @@ int pulse_mainloop(struct openconnect_info *vpninfo, int *timeout, int readable)
 			return 1;
 		}
 
-		if (load_be32(&pkt->pulse.vendor) != VENDOR_JUNIPER ||
-		    load_be32(&pkt->pulse.len) != len)
+		/* Packets shouldn't cross SSL record boundaries (we hope!), so if there
+		 * was a header there, then rest of that packet should be there too. */
+		if (load_be32(&pkt->pulse.len) > receive_mtu + 0x10) {
+			/* This doesn't look right. Pull the rest of the SSL record
+			 * and complain about it (which we will, since the length
+			 * won't match the header */
+			len = receive_mtu;
+		} else
+			len = load_be32(&pkt->pulse.len) - 0x10;
+
+		payload_len = ssl_nonblock_read(vpninfo, &pkt->data, len);
+		if (payload_len != load_be32(&pkt->pulse.len) - 0x10) {
+			if (payload_len < 0)
+				len = 0x10;
+			else
+				len = payload_len + 0x10;
+			goto unknown_pkt;
+		}
+
+		if (load_be32(&pkt->pulse.vendor) != VENDOR_JUNIPER)
 			goto unknown_pkt;
 
 		vpninfo->ssl_times.last_rx = time(NULL);
-		payload_len = len - 16;
+		len = payload_len + 0x10;
 
 		switch(load_be32(&pkt->pulse.type)) {
 		case 4:
 			vpn_progress(vpninfo, PRG_TRACE,
 				     _("Received data packet of %d bytes\n"),
 				     payload_len);
+			dump_buf_hex(vpninfo, PRG_TRACE, '<', (void *)&vpninfo->cstp_pkt->pulse.vendor, len);
 			vpninfo->cstp_pkt->len = payload_len;
 			queue_packet(&vpninfo->incoming_queue, pkt);
 			vpninfo->cstp_pkt = pkt = NULL;