Add TCP short-write regression test

configure.ac

@@ -376,6 +376,7 @@ case "$host" in
 esac
 
 AM_CONDITIONAL([CROSS_COMPILING], test "${cross_compiling}" = "yes")
+AM_CONDITIONAL([TARGET_LINUX], [case $host in *-*-linux*) true;; *) false;; esac])
 
 PKG_PROG_PKG_CONFIG
 # Add variable to print if pkg-config is found or not. Users often miss that

src/openvpn/forward.c

@@ -1748,7 +1748,11 @@ process_outgoing_link(struct context *c, struct link_socket *sock)
     struct gc_arena gc = gc_new();
     int error_code = 0;
 
-    if (c->c2.to_link.len > 0 && c->c2.to_link.len <= c->c2.frame.buf.payload_size)
+    /* For a partial TCP write, to_link.len includes the prepended size header,
+       so it can be greater than the payload size. */
+    if (c->c2.to_link.len > 0 
+        && (c->c2.to_link.len <= c->c2.frame.buf.payload_size 
+            || sock->stream_partial_write))
     {
         /*
          * Setup for call to send/sendto which will send
@@ -1766,7 +1770,7 @@ process_outgoing_link(struct context *c, struct link_socket *sock)
              * Let the traffic shaper know how many bytes
              * we wrote.
              */
-            if (c->options.shaper)
+            if (c->options.shaper && !sock->stream_partial_write)
             {
                 int overhead =
                     datagram_overhead(c->c2.to_link_addr->dest.addr.sa.sa_family, sock->info.proto);
@@ -1788,7 +1792,7 @@ process_outgoing_link(struct context *c, struct link_socket *sock)
 
             /* Log packet send */
 #ifdef LOG_RW
-            if (c->c2.log_rw)
+            if (c->c2.log_rw && !sock->stream_partial_write)
             {
                 fprintf(stderr, "W");
             }
@@ -1824,19 +1828,43 @@ process_outgoing_link(struct context *c, struct link_socket *sock)
         error_code = openvpn_errno();
         check_status(size, "write", sock, NULL);
 
-        if (size > 0)
+        if (proto_is_tcp(sock->info.proto) && !socket_is_dco_win(sock))
+        {
+            if (size == BLEN(&c->c2.to_link))
+            {
+                /* complete write */
+                sock->stream_partial_write = false;
+            }
+            else if (size > 0 && size < BLEN(&c->c2.to_link))
+            {
+                /* partial write */
+                buf_advance(&c->c2.to_link, size);
+                sock->stream_partial_write = true;
+            }
+            else if (size < 0 && (error_code == EAGAIN || error_code == EWOULDBLOCK))
+            {
+                /* 0 bytes written, but header might have been prepended */
+                sock->stream_partial_write = true;
+            }
+            else
+            {
+                /* error */
+                sock->stream_partial_write = false;
+            }
+        }
+        else
         {
             /* Did we write a different size packet than we intended? */
-            if (size != BLEN(&c->c2.to_link))
+            if (size > 0 &&  size != BLEN(&c->c2.to_link))
             {
                 msg(D_LINK_ERRORS,
-                    "TCP/UDP packet was truncated/expanded on write to %s (tried=%d,actual=%d)",
+                    "Packet was truncated/expanded on write to %s (tried=%d,actual=%d)",
                     print_link_socket_actual(c->c2.to_link_addr, &gc), BLEN(&c->c2.to_link), size);
             }
         }
 
         /* if not a ping/control message, indicate activity regarding --inactive parameter */
-        if (c->c2.buf.len > 0)
+        if (c->c2.buf.len > 0 && size > 0)
         {
             register_activity(c, size);
         }
@@ -1867,7 +1895,10 @@ process_outgoing_link(struct context *c, struct link_socket *sock)
         }
     }
 
-    buf_reset(&c->c2.to_link);
+    if (!sock->stream_partial_write)
+    {
+        buf_reset(&c->c2.to_link);
+    }
 
     gc_free(&gc);
 }

src/openvpn/socket.c

@@ -2439,13 +2439,17 @@ link_socket_read_udp_posix(struct link_socket *sock, struct buffer *buf,
 ssize_t
 link_socket_write_tcp(struct link_socket *sock, struct buffer *buf, struct link_socket_actual *to)
 {
-    const int blen = BLEN(buf);
-    ASSERT(blen >= 0 && blen <= PACKET_SIZE_MAX);
-    packet_size_type len = (packet_size_type)blen;
-    dmsg(D_STREAM_DEBUG, "STREAM: WRITE %u offset=%d", len, buf->offset);
-    ASSERT(len <= sock->stream_buf.maxlen);
-    len = htonps(len);
-    ASSERT(buf_write_prepend(buf, &len, sizeof(len)));
+    if (!sock->stream_partial_write)
+    {
+        /* prepend the length of the packet to the buffer */
+        const int blen = BLEN(buf);
+        ASSERT(blen >= 0 && blen <= PACKET_SIZE_MAX);
+        packet_size_type len = (packet_size_type)blen;
+        dmsg(D_STREAM_DEBUG, "STREAM: WRITE %u offset=%d", len, buf->offset);
+        ASSERT(len <= sock->stream_buf.maxlen);
+        len = htonps(len);
+        ASSERT(buf_write_prepend(buf, &len, sizeof(len)));
+    }
 #ifdef _WIN32
     return link_socket_write_win32(sock, buf, to);
 #else

src/openvpn/socket.h

@@ -223,6 +223,7 @@ struct link_socket
     struct stream_buf stream_buf;
     struct buffer stream_buf_data;
     bool stream_reset;
+    bool stream_partial_write; /* a partial write is outstanding */
 
     /* HTTP proxy */
     struct http_proxy_info *http_proxy;

tests/Makefile.am

@@ -21,12 +21,21 @@ if !WIN32
 test_scripts = t_client.sh t_lpback.sh t_cltsrv.sh t_server_null.sh
 
 check_PROGRAMS = ntlm_support
+
+if TARGET_LINUX
+test_scripts += t_short_write.sh
+check_LTLIBRARIES = short_write_shim.la
+short_write_shim_la_SOURCES = short_write_shim.c
+short_write_shim_la_LDFLAGS = -module -shared -avoid-version -no-undefined -rpath $(abs_builddir)
+short_write_shim_la_LIBADD  = -ldl
+endif
+
 if HAVE_SITNL
 test_scripts += t_net.sh
 endif
 endif
 
-TESTS_ENVIRONMENT = top_srcdir="$(top_srcdir)"
+TESTS_ENVIRONMENT = top_srcdir="$(top_srcdir)" top_builddir="$(top_builddir)"
 TEST_EXTENSIONS = .sh
 TESTS = $(test_scripts)
 
@@ -39,9 +48,11 @@ dist_noinst_SCRIPTS = \
 	t_server_null_client.sh \
 	t_server_null_server.sh \
 	t_server_null_default.rc \
+	t_short_write.sh \
 	update_t_client_ips.sh
 
 t_client.log: t_server_null.log
+t_short_write.log: short_write_shim.la
 
 dist_noinst_DATA = \
 	t_client.rc-sample

tests/short_write_shim.c

@@ -0,0 +1,44 @@
+/*
+ * short_write_shim.c -- LD_PRELOAD shim that injects short writes into send()
+ *                       Intercepts TCP data-channel packets only, letting the
+ *                       TLS handshake complete before corrupting the stream.
+ */
+
+#define _GNU_SOURCE
+#include <dlfcn.h>
+#include <sys/socket.h>
+
+/* Opcodes from ssl_pkt.h -- high 5 bits of the opcode byte */
+#define P_OPCODE_SHIFT  3
+#define P_DATA_V1       6
+#define P_DATA_V2       9
+
+static ssize_t (*real_send)(int, const void *, size_t, int);
+
+static void __attribute__((constructor))
+shim_init(void)
+{
+    real_send = dlsym(RTLD_NEXT, "send");
+}
+
+ssize_t
+send(int fd, const void *buf, size_t len, int flags)
+{
+    /*
+     * Over TCP, OpenVPN prepends a 2-byte big-endian length header to every
+     * packet. Byte 2 (index 2) carries the opcode in its high 5 bits.
+     * We need at least 3 bytes to inspect the opcode.
+     */
+    if (len >= 3)
+    {
+        const unsigned char *p = (const unsigned char *)buf;
+        int opcode = (p[2] >> P_OPCODE_SHIFT);
+
+        if (opcode == P_DATA_V1 || opcode == P_DATA_V2)
+        {
+            return real_send(fd, buf, len / 2, flags);
+        }
+    }
+
+    return real_send(fd, buf, len, flags);
+}

tests/t_short_write.sh

@@ -0,0 +1,153 @@
+#!/bin/sh
+#
+# t_short_write.sh - regression test for TCP short-write stream corruption
+#
+# An LD_PRELOAD shim halves every data-channel send().  If the send path does
+# not retry, forward.c logs a truncation warning and the stream is corrupted.
+#
+# Exit 0  (PASS): no truncation — short writes handled correctly
+# Exit 1  (FAIL): truncation warning found — bug is present
+# Exit 77 (SKIP): prerequisites missing or unsupported platform
+#
+
+set -e
+
+top_srcdir="${top_srcdir:-..}"
+top_builddir="${top_builddir:-..}"
+openvpn="${openvpn:-${top_builddir}/src/openvpn/openvpn}"
+shim="${shim:-$(cd "${top_builddir}/tests" && pwd)/.libs/short_write_shim.so}"
+
+# LD_PRELOAD interposition requires a Linux ELF dynamic linker.
+case $(uname -s) in
+    Linux) ;;
+    *) echo "$0: LD_PRELOAD shim requires Linux. SKIP." >&2; exit 77 ;;
+esac
+
+test -x "${openvpn}" || { echo "$0: openvpn binary not found. SKIP." >&2;      exit 77; }
+test -f "${shim}"    || { echo "$0: short_write_shim.so not built. SKIP." >&2; exit 77; }
+
+# Port chosen to avoid the loopback-{server,client} sample configs (16000/16001).
+SRV_PORT="${SRV_PORT:-16002}"
+
+root="${top_srcdir}/sample"
+WORKDIR="$(pwd)"
+SRV_LOG="${WORKDIR}/sw_srv_$$.log"
+CLI_LOG="${WORKDIR}/sw_cli_$$.log"
+SRV_PID="${WORKDIR}/sw_srv_$$.pid"
+
+cleanup() {
+    if [ -n "${CLI_PID}" ]; then
+        kill "${CLI_PID}" 2>/dev/null || true
+    fi
+    if [ -f "${SRV_PID}" ]; then
+        pid=$(cat "${SRV_PID}" 2>/dev/null)
+        [ -n "${pid}" ] && kill "${pid}" 2>/dev/null || true
+    fi
+    rm -f "${SRV_LOG}" "${CLI_LOG}" "${SRV_PID}"
+}
+
+CLI_PID=""
+
+trap "cleanup; trap 0; exit 77" 1 2 15
+trap "cleanup; exit 1"          0 3
+
+# Poll $CLI_LOG for regex $1 for up to $2 seconds.
+wait_for() {
+    _pat=$1; _tries=$2
+    while [ "${_tries}" -gt 0 ]; do
+        grep -qE "${_pat}" "${CLI_LOG}" 2>/dev/null && return 0
+        [ -n "${CLI_PID}" ] && ! kill -0 "${CLI_PID}" 2>/dev/null && return 1
+        sleep 1
+        _tries=$((_tries - 1))
+    done
+    return 1
+}
+
+success=0
+for i in 1 2 3; do
+    set +e
+
+    "${openvpn}" \
+        --cd "${root}" --dev null --proto tcp-server \
+        --local 127.0.0.1 --lport "${SRV_PORT}" \
+        --tls-server --dh none \
+        --ca  sample-keys/ca.crt \
+        --key sample-keys/server.key \
+        --cert sample-keys/server.crt \
+        --cipher AES-256-GCM --verb 3 \
+        --writepid "${SRV_PID}" --log "${SRV_LOG}" --daemon
+
+    j=0
+    while [ $j -lt 10 ] && [ ! -s "${SRV_PID}" ]; do
+        sleep 1; j=$((j+1))
+    done
+
+    if [ ! -s "${SRV_PID}" ]; then
+        if grep -q 'TCP/UDP: Socket bind failed on local address.*in use' \
+                "${SRV_LOG}" 2>/dev/null; then
+            echo "$0: port ${SRV_PORT} in use, retrying in 10 s" >&2
+            rm -f "${SRV_PID}" "${SRV_LOG}"
+            sleep 10
+            continue
+        fi
+        echo "$0: server did not start" >&2
+        cat "${SRV_LOG}" >&2
+        exit 1
+    fi
+
+    # The client never exits on its own: bug present → endless reconnect loop;
+    # bug fixed → healthy tunnel forever.  Poll the log and kill when done.
+    rm -f "${CLI_LOG}"
+    LD_PRELOAD="${shim}" \
+    "${openvpn}" \
+        --cd "${root}" --dev null --proto tcp-client \
+        --remote 127.0.0.1 "${SRV_PORT}" \
+        --tls-client --remote-cert-tls server \
+        --ca  sample-keys/ca.crt \
+        --key sample-keys/client.key \
+        --cert sample-keys/client.crt \
+        --cipher AES-256-GCM \
+        --ping 1 --ping-exit 30 \
+        --verb 3 --log "${CLI_LOG}" &
+    CLI_PID=$!
+
+    if ! wait_for "P2P mode NCP negotiation result" 15; then
+        echo "$0: tunnel did not establish, retrying" >&2
+        kill "${CLI_PID}" 2>/dev/null; wait "${CLI_PID}" 2>/dev/null
+        CLI_PID=""
+        kill "$(cat ${SRV_PID})" 2>/dev/null || true
+        rm -f "${SRV_PID}" "${SRV_LOG}" "${CLI_LOG}"
+        continue
+    fi
+
+    set -e
+    success=1
+    break
+done
+
+if [ $success -ne 1 ]; then
+    echo "$0: could not establish a tunnel after 3 attempts. SKIP." >&2
+    trap 0; cleanup; exit 77
+fi
+
+# Tunnel is up.  The first ping (~1 s) triggers the truncation warning if the
+# bug is present.  Wait a few ping cycles for it to surface.
+set +e
+wait_for "TCP/UDP packet was truncated/expanded on write" 5
+set -e
+
+kill "${CLI_PID}" 2>/dev/null || true
+wait "${CLI_PID}" 2>/dev/null || true
+CLI_PID=""
+
+ec=0
+if grep -q "TCP/UDP packet was truncated/expanded on write" "${CLI_LOG}"; then
+    echo "$0: FAIL: short-write caused stream truncation" >&2
+    ec=1
+else
+    echo "$0: PASS: no truncation detected" >&2
+fi
+
+cleanup
+trap 0
+exit $ec