Userspace now enables TCP_NODELAY by default. Enable it for
ovpn-cli's TCP sockets too.

The TCP peer ID capture assumes that every TCP segment starts with an
ovpn length prefix followed by a data header. TCP does not preserve
record boundaries, and enabling TCP_NODELAY makes this check unreliable.
Restrict the capture-based peer ID check to UDP.

Signed-off-by: Marco Baffo <[email protected]>
---
 tools/testing/selftests/net/ovpn/common.sh  | 20 +++-----
 tools/testing/selftests/net/ovpn/ovpn-cli.c | 22 +++++++++
 tools/testing/selftests/net/ovpn/test.sh    | 54 ++++++++++-----------
 3 files changed, 56 insertions(+), 40 deletions(-)

diff --git a/tools/testing/selftests/net/ovpn/common.sh 
b/tools/testing/selftests/net/ovpn/common.sh
index 2d844eb3aa6e..de96d333ee2b 100644
--- a/tools/testing/selftests/net/ovpn/common.sh
+++ b/tools/testing/selftests/net/ovpn/common.sh
@@ -178,20 +178,14 @@ ovpn_setup_ns() {
 
 ovpn_build_capture_filter() {
        # match the first four bytes of the openvpn data payload
-       if [ "${OVPN_PROTO}" == "UDP" ]; then
-               # For UDP, libpcap transport indexing only works for IPv4, so
-               # use an explicit IPv4 or IPv6 expression based on the peer
-               # address. The IPv6 branch assumes there are no extension
-               # headers in the outer packet.
-               if [[ "${2}" == *:* ]]; then
-                       printf "ip6 and ip6[6] = 17 and ip6[48:4] = %s" "${1}"
-               else
-                       printf "ip and udp[8:4] = %s" "${1}"
-               fi
+       # For UDP, libpcap transport indexing only works for IPv4, so
+       # use an explicit IPv4 or IPv6 expression based on the peer
+       # address. The IPv6 branch assumes there are no extension
+       # headers in the outer packet.
+       if [[ "${2}" == *:* ]]; then
+               printf "ip6 and ip6[6] = 17 and ip6[48:4] = %s" "${1}"
        else
-               # openvpn over TCP prepends a 2-byte packet length ahead of the
-               # DATA_V2 opcode, so skip it before matching the payload header
-               printf "ip and tcp[(((tcp[12] & 0xf0) >> 2) + 2):4] = %s" "${1}"
+               printf "ip and udp[8:4] = %s" "${1}"
        fi
 }
 
diff --git a/tools/testing/selftests/net/ovpn/ovpn-cli.c 
b/tools/testing/selftests/net/ovpn/ovpn-cli.c
index f4effa7580c0..6b458a654a33 100644
--- a/tools/testing/selftests/net/ovpn/ovpn-cli.c
+++ b/tools/testing/selftests/net/ovpn/ovpn-cli.c
@@ -470,6 +470,18 @@ static int ovpn_parse_key_direction(const char *dir, 
struct ovpn_ctx *ctx)
        return 0;
 }
 
+static int ovpn_tcp_nodelay(int socket)
+{
+       int opt = 1;
+       int ret;
+
+       ret = setsockopt(socket, IPPROTO_TCP, TCP_NODELAY, &opt, sizeof(opt));
+       if (ret < 0)
+               perror("setsockopt for TCP_NODELAY");
+
+       return ret;
+}
+
 static int ovpn_socket(struct ovpn_ctx *ctx, sa_family_t family, int proto)
 {
        struct sockaddr_storage local_sock = { 0 };
@@ -606,6 +618,12 @@ static int ovpn_accept(struct ovpn_ctx *ctx)
                goto err;
        }
 
+       if (ovpn_tcp_nodelay(ret) < 0) {
+               close(ret);
+               ret = -1;
+               goto err;
+       }
+
        return ret;
 err:
        close(ctx->socket);
@@ -623,6 +641,10 @@ static int ovpn_connect(struct ovpn_ctx *ovpn)
                return -1;
        }
 
+       ret = ovpn_tcp_nodelay(s);
+       if (ret < 0)
+               goto err;
+
        switch (ovpn->remote.in4.sin_family) {
        case AF_INET:
                socklen = sizeof(struct sockaddr_in);
diff --git a/tools/testing/selftests/net/ovpn/test.sh 
b/tools/testing/selftests/net/ovpn/test.sh
index 9b5610837032..d744c1a97d5f 100755
--- a/tools/testing/selftests/net/ovpn/test.sh
+++ b/tools/testing/selftests/net/ovpn/test.sh
@@ -67,35 +67,33 @@ ovpn_run_basic_traffic() {
        local tcpdump_timeout="1.5s"
 
        for p in $(seq 1 ${OVPN_NUM_PEERS}); do
-               # The first part of the data packet header consists of:
-               # - TCP only: 2 bytes for the packet length
-               # - 5 bits for opcode ("9" for DATA_V2)
-               # - 3 bits for key-id ("0" at this point)
-               # - 12 bytes for peer-id:
-               #     - with asymmetric ID: "${p}" one way and "${p} + 9" the
-               #       other way
-               #     - with symmetric ID: "${p}" both ways
-               header1=$(printf "0x4800000%x" ${p})
-               header2=$(printf "0x4800000%x" $((p + OVPN_ID_OFFSET)))
-               raddr=""
                if [ "${OVPN_PROTO}" == "UDP" ]; then
+                       # The first part of the data packet header consists of:
+                       # - 5 bits for opcode ("9" for DATA_V2)
+                       # - 3 bits for key-id ("0" at this point)
+                       # - 3 bytes for peer-id:
+                       #     - with asymmetric ID: "${p}" one way and "${p} + 
9" the
+                       #       other way
+                       #     - with symmetric ID: "${p}" both ways
+                       header1=$(printf "0x4800000%x" ${p})
+                       header2=$(printf "0x4800000%x" $((p + OVPN_ID_OFFSET)))
                        raddr=$(awk "NR == ${p} {print \$3}" \
                                "${OVPN_UDP_PEERS_FILE}")
+                       peer_ns="ovpn_peer${p}"
+
+                       timeout ${tcpdump_timeout} ip netns exec "${peer_ns}" \
+                               tcpdump --immediate-mode -p -ni veth${p} -c 1 \
+                               "$(ovpn_build_capture_filter "${header1}" 
"${raddr}")" \
+                               >/dev/null 2>&1 &
+                       tcpdump_pid1=$!
+                       timeout ${tcpdump_timeout} ip netns exec "${peer_ns}" \
+                               tcpdump --immediate-mode -p -ni veth${p} -c 1 \
+                               "$(ovpn_build_capture_filter "${header2}" 
"${raddr}")" \
+                               >/dev/null 2>&1 &
+                       tcpdump_pid2=$!
+
+                       sleep 0.3
                fi
-               peer_ns="ovpn_peer${p}"
-
-               timeout ${tcpdump_timeout} ip netns exec "${peer_ns}" \
-                       tcpdump --immediate-mode -p -ni veth${p} -c 1 \
-                       "$(ovpn_build_capture_filter "${header1}" "${raddr}")" \
-                       >/dev/null 2>&1 &
-               tcpdump_pid1=$!
-               timeout ${tcpdump_timeout} ip netns exec "${peer_ns}" \
-                       tcpdump --immediate-mode -p -ni veth${p} -c 1 \
-                       "$(ovpn_build_capture_filter "${header2}" "${raddr}")" \
-                       >/dev/null 2>&1 &
-               tcpdump_pid2=$!
-
-               sleep 0.3
                ovpn_cmd_ok "send baseline traffic to peer ${p}" \
                        ip netns exec ovpn_peer0 \
                        ping -qfc 100 -w 3 5.5.5.$((p + 1))
@@ -103,8 +101,10 @@ ovpn_run_basic_traffic() {
                        ip netns exec ovpn_peer0 \
                        ping -qfc 100 -s 3000 -w 3 5.5.5.$((p + 1))
 
-               wait "${tcpdump_pid1}" || return 1
-               wait "${tcpdump_pid2}" || return 1
+               if [ "${OVPN_PROTO}" == "UDP" ]; then
+                       wait "${tcpdump_pid1}" || return 1
+                       wait "${tcpdump_pid2}" || return 1
+               fi
        done
 }
 
-- 
2.43.0



_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to