mbedtls 3 installs headers and calls the shared object differently than version 2, therefore we must now rely on pkgconfig to fill the right C/LDFLAGS.

Moreover the mbedtls3 library expects any base64 file to have their content on one line. Since this change does no break older versions, let's change the sample key file format and make mbedtls3 happy.

Cc: Shuah Khan shuah@kernel.org Signed-off-by: Antonio Quartulli antonio@openvpn.net --- tools/testing/selftests/net/ovpn/Makefile | 14 ++++++++++---- tools/testing/selftests/net/ovpn/data64.key | 6 +----- 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/net/ovpn/Makefile b/tools/testing/selftests/net/ovpn/Makefile index dbe0388c8512..d3a070db0bb5 100644 --- a/tools/testing/selftests/net/ovpn/Makefile +++ b/tools/testing/selftests/net/ovpn/Makefile @@ -2,19 +2,25 @@ # Copyright (C) 2020-2025 OpenVPN, Inc. # CFLAGS = -pedantic -Wextra -Wall -Wl,--no-as-needed -g -O0 -ggdb $(KHDR_INCLUDES) +CFLAGS += $(shell pkg-config --cflags mbedcrypto-3 mbedtls-3 2>/dev/null) + VAR_CFLAGS = $(shell pkg-config --cflags libnl-3.0 libnl-genl-3.0 2>/dev/null) ifeq ($(VAR_CFLAGS),) VAR_CFLAGS = -I/usr/include/libnl3 endif CFLAGS += $(VAR_CFLAGS)

+MTLS_LDLIBS= $(shell pkg-config --libs mbedcrypto-3 mbedtls-3 2>/dev/null) +ifeq ($(MTLS_LDLIBS),) +MTLS_LDLIBS = -lmbedtls -lmbedcrypto +endif +LDLIBS += $(MTLS_LDLIBS)

-LDLIBS = -lmbedtls -lmbedcrypto -VAR_LDLIBS = $(shell pkg-config --libs libnl-3.0 libnl-genl-3.0 2>/dev/null) +NL_LDLIBS = $(shell pkg-config --libs libnl-3.0 libnl-genl-3.0 2>/dev/null) ifeq ($(VAR_LDLIBS),) -VAR_LDLIBS = -lnl-genl-3 -lnl-3 +NL_LDLIBS = -lnl-genl-3 -lnl-3 endif -LDLIBS += $(VAR_LDLIBS) +LDLIBS += $(NL_LDLIBS)

TEST_FILES = common.sh diff --git a/tools/testing/selftests/net/ovpn/data64.key b/tools/testing/selftests/net/ovpn/data64.key index a99e88c4e290..d04febcdf5a2 100644 --- a/tools/testing/selftests/net/ovpn/data64.key +++ b/tools/testing/selftests/net/ovpn/data64.key @@ -1,5 +1 @@ -jRqMACN7d7/aFQNT8S7jkrBD8uwrgHbG5OQZP2eu4R1Y7tfpS2bf5RHv06Vi163CGoaIiTX99R3B -ia9ycAH8Wz1+9PWv51dnBLur9jbShlgZ2QHLtUc4a/gfT7zZwULXuuxdLnvR21DDeMBaTbkgbai9 -uvAa7ne1liIgGFzbv+Bas4HDVrygxIxuAnP5Qgc3648IJkZ0QEXPF+O9f0n5+QIvGCxkAUVx+5K6 -KIs+SoeWXnAopELmoGSjUpFtJbagXK82HfdqpuUxT2Tnuef0/14SzVE/vNleBNu2ZbyrSAaah8tE -BofkPJUBFY+YQcfZNM5Dgrw3i+Bpmpq/gpdg5w== +jRqMACN7d7/aFQNT8S7jkrBD8uwrgHbG5OQZP2eu4R1Y7tfpS2bf5RHv06Vi163CGoaIiTX99R3Bia9ycAH8Wz1+9PWv51dnBLur9jbShlgZ2QHLtUc4a/gfT7zZwULXuuxdLnvR21DDeMBaTbkgbai9uvAa7ne1liIgGFzbv+Bas4HDVrygxIxuAnP5Qgc3648IJkZ0QEXPF+O9f0n5+QIvGCxkAUVx+5K6KIs+SoeWXnAopELmoGSjUpFtJbagXK82HfdqpuUxT2Tnuef0/14SzVE/vNleBNu2ZbyrSAaah8tEBofkPJUBFY+YQcfZNM5Dgrw3i+Bpmpq/gpdg5w==

From: Sabrina Dubroca sd@queasysnail.net

In ovpn_nl_key_swap_doit, the attributes array used to parse the OVPN_A_KEYCONF uses OVPN_A_PEER_MAX instead of OVPN_A_KEYCONF_MAX. Note that this does not cause any bug, since currently OVPN_A_KEYCONF_MAX < OVPN_A_PEER_MAX.

The wrong constant was introduced by 203e2bf55990 ("ovpn: implement key add/get/del/swap via netlink")

Signed-off-by: Sabrina Dubroca sd@queasysnail.net Signed-off-by: Antonio Quartulli antonio@openvpn.net --- drivers/net/ovpn/netlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ovpn/netlink.c b/drivers/net/ovpn/netlink.c index c7f382437630..fed0e46b32a3 100644 --- a/drivers/net/ovpn/netlink.c +++ b/drivers/net/ovpn/netlink.c @@ -1061,8 +1061,8 @@ int ovpn_nl_key_get_doit(struct sk_buff *skb, struct genl_info *info)

int ovpn_nl_key_swap_doit(struct sk_buff *skb, struct genl_info *info) { + struct nlattr *attrs[OVPN_A_KEYCONF_MAX + 1]; struct ovpn_priv *ovpn = info->user_ptr[0]; - struct nlattr *attrs[OVPN_A_PEER_MAX + 1]; struct ovpn_peer *peer; u32 peer_id; int ret;

From: Ralf Lici ralf@mandelbit.com

Send a netlink notification when a client updates its remote UDP endpoint. The notification includes the new IP address, port, and scope ID (for IPv6).

Reviewed-by: Sabrina Dubroca sd@queasysnail.net Signed-off-by: Ralf Lici ralf@mandelbit.com Signed-off-by: Antonio Quartulli antonio@openvpn.net --- Documentation/netlink/specs/ovpn.yaml | 6 ++ drivers/net/ovpn/netlink.c | 82 +++++++++++++++++++++ drivers/net/ovpn/netlink.h | 2 + drivers/net/ovpn/peer.c | 2 + include/uapi/linux/ovpn.h | 1 + tools/testing/selftests/net/ovpn/ovpn-cli.c | 3 + 6 files changed, 96 insertions(+)

diff --git a/Documentation/netlink/specs/ovpn.yaml b/Documentation/netlink/specs/ovpn.yaml index 1b91045cee2e..0d0c028bf96f 100644 --- a/Documentation/netlink/specs/ovpn.yaml +++ b/Documentation/netlink/specs/ovpn.yaml @@ -502,6 +502,12 @@ operations: - ifindex - keyconf

+ - + name: peer-float-ntf + doc: Notification about a peer floating (changing its remote UDP endpoint) + notify: peer-get + mcgrp: peers + mcast-groups: list: - diff --git a/drivers/net/ovpn/netlink.c b/drivers/net/ovpn/netlink.c index fed0e46b32a3..3db056f4cd0a 100644 --- a/drivers/net/ovpn/netlink.c +++ b/drivers/net/ovpn/netlink.c @@ -1203,6 +1203,88 @@ int ovpn_nl_peer_del_notify(struct ovpn_peer *peer) return ret; }

+/** + * ovpn_nl_float_peer_notify - notify userspace about peer floating + * @peer: the floated peer + * @ss: sockaddr representing the new remote endpoint + * + * Return: 0 on success or a negative error code otherwise + */ +int ovpn_nl_peer_float_notify(struct ovpn_peer *peer, + const struct sockaddr_storage *ss) +{ + struct ovpn_socket *sock; + struct sockaddr_in6 *sa6; + struct sockaddr_in *sa; + struct sk_buff *msg; + struct nlattr *attr; + int ret = -EMSGSIZE; + void *hdr; + + msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC); + if (!msg) + return -ENOMEM; + + hdr = genlmsg_put(msg, 0, 0, &ovpn_nl_family, 0, + OVPN_CMD_PEER_FLOAT_NTF); + if (!hdr) { + ret = -ENOBUFS; + goto err_free_msg; + } + + if (nla_put_u32(msg, OVPN_A_IFINDEX, peer->ovpn->dev->ifindex)) + goto err_cancel_msg; + + attr = nla_nest_start(msg, OVPN_A_PEER); + if (!attr) + goto err_cancel_msg; + + if (nla_put_u32(msg, OVPN_A_PEER_ID, peer->id)) + goto err_cancel_msg; + + if (ss->ss_family == AF_INET) { + sa = (struct sockaddr_in *)ss; + if (nla_put_in_addr(msg, OVPN_A_PEER_REMOTE_IPV4, + sa->sin_addr.s_addr) || + nla_put_net16(msg, OVPN_A_PEER_REMOTE_PORT, sa->sin_port)) + goto err_cancel_msg; + } else if (ss->ss_family == AF_INET6) { + sa6 = (struct sockaddr_in6 *)ss; + if (nla_put_in6_addr(msg, OVPN_A_PEER_REMOTE_IPV6, + &sa6->sin6_addr) || + nla_put_u32(msg, OVPN_A_PEER_REMOTE_IPV6_SCOPE_ID, + sa6->sin6_scope_id) || + nla_put_net16(msg, OVPN_A_PEER_REMOTE_PORT, sa6->sin6_port)) + goto err_cancel_msg; + } else { + ret = -EAFNOSUPPORT; + goto err_cancel_msg; + } + + nla_nest_end(msg, attr); + genlmsg_end(msg, hdr); + + rcu_read_lock(); + sock = rcu_dereference(peer->sock); + if (!sock) { + ret = -EINVAL; + goto err_unlock; + } + genlmsg_multicast_netns(&ovpn_nl_family, sock_net(sock->sk), msg, + 0, OVPN_NLGRP_PEERS, GFP_ATOMIC); + rcu_read_unlock(); + + return 0; + +err_unlock: + rcu_read_unlock(); +err_cancel_msg: + genlmsg_cancel(msg, hdr); +err_free_msg: + nlmsg_free(msg); + return ret; +} + /** * ovpn_nl_key_swap_notify - notify userspace peer's key must be renewed * @peer: the peer whose key needs to be renewed diff --git a/drivers/net/ovpn/netlink.h b/drivers/net/ovpn/netlink.h index 8615dfc3c472..11ee7c681885 100644 --- a/drivers/net/ovpn/netlink.h +++ b/drivers/net/ovpn/netlink.h @@ -13,6 +13,8 @@ int ovpn_nl_register(void); void ovpn_nl_unregister(void);

int ovpn_nl_peer_del_notify(struct ovpn_peer *peer); +int ovpn_nl_peer_float_notify(struct ovpn_peer *peer, + const struct sockaddr_storage *ss); int ovpn_nl_key_swap_notify(struct ovpn_peer *peer, u8 key_id);

#endif /* _NET_OVPN_NETLINK_H_ */ diff --git a/drivers/net/ovpn/peer.c b/drivers/net/ovpn/peer.c index 4bfcab0c8652..9ad50f1ac2c3 100644 --- a/drivers/net/ovpn/peer.c +++ b/drivers/net/ovpn/peer.c @@ -287,6 +287,8 @@ void ovpn_peer_endpoints_update(struct ovpn_peer *peer, struct sk_buff *skb)

spin_unlock_bh(&peer->lock);

+ ovpn_nl_peer_float_notify(peer, &ss); + /* rehashing is required only in MP mode as P2P has one peer * only and thus there is no hashtable */ diff --git a/include/uapi/linux/ovpn.h b/include/uapi/linux/ovpn.h index 680d1522dc87..b3c9ff0a6849 100644 --- a/include/uapi/linux/ovpn.h +++ b/include/uapi/linux/ovpn.h @@ -99,6 +99,7 @@ enum { OVPN_CMD_KEY_SWAP, OVPN_CMD_KEY_SWAP_NTF, OVPN_CMD_KEY_DEL, + OVPN_CMD_PEER_FLOAT_NTF,

__OVPN_CMD_MAX, OVPN_CMD_MAX = (__OVPN_CMD_MAX - 1) diff --git a/tools/testing/selftests/net/ovpn/ovpn-cli.c b/tools/testing/selftests/net/ovpn/ovpn-cli.c index 0a5226196a2e..064453d16fdd 100644 --- a/tools/testing/selftests/net/ovpn/ovpn-cli.c +++ b/tools/testing/selftests/net/ovpn/ovpn-cli.c @@ -1516,6 +1516,9 @@ static int ovpn_handle_msg(struct nl_msg *msg, void *arg) case OVPN_CMD_PEER_DEL_NTF: fprintf(stdout, "received CMD_PEER_DEL_NTF\n"); break; + case OVPN_CMD_PEER_FLOAT_NTF: + fprintf(stdout, "received CMD_PEER_FLOAT_NTF\n"); + break; case OVPN_CMD_KEY_SWAP_NTF: fprintf(stdout, "received CMD_KEY_SWAP_NTF\n"); break;

From: Ralf Lici ralf@mandelbit.com

Extend the base test to verify that the correct peer-id is set in data packet headers. This is done by capturing ping packets with ngrep during the initial exchange and matching the first portion of the header against the expected sequence for every connection.

Cc: Shuah Khan shuah@kernel.org Signed-off-by: Ralf Lici ralf@mandelbit.com Signed-off-by: Antonio Quartulli antonio@openvpn.net --- tools/testing/selftests/net/ovpn/common.sh | 20 +++---- .../selftests/net/ovpn/json/peer1.json | 2 +- .../selftests/net/ovpn/json/peer2.json | 2 +- .../selftests/net/ovpn/json/peer3.json | 2 +- .../selftests/net/ovpn/json/peer4.json | 2 +- .../selftests/net/ovpn/json/peer5.json | 2 +- .../selftests/net/ovpn/json/peer6.json | 2 +- tools/testing/selftests/net/ovpn/ovpn-cli.c | 53 ++++++++++++------- .../testing/selftests/net/ovpn/tcp_peers.txt | 12 ++--- .../selftests/net/ovpn/test-close-socket.sh | 2 +- tools/testing/selftests/net/ovpn/test.sh | 47 +++++++++++----- .../testing/selftests/net/ovpn/udp_peers.txt | 12 ++--- 12 files changed, 97 insertions(+), 61 deletions(-)

diff --git a/tools/testing/selftests/net/ovpn/common.sh b/tools/testing/selftests/net/ovpn/common.sh index b91cf17ab01f..d926413c9f16 100644 --- a/tools/testing/selftests/net/ovpn/common.sh +++ b/tools/testing/selftests/net/ovpn/common.sh @@ -75,13 +75,14 @@ add_peer() { data64.key done else - RADDR=$(awk "NR == ${1} {print $2}" ${UDP_PEERS_FILE}) - RPORT=$(awk "NR == ${1} {print $3}" ${UDP_PEERS_FILE}) - LPORT=$(awk "NR == ${1} {print $5}" ${UDP_PEERS_FILE}) - ip netns exec peer${1} ${OVPN_CLI} new_peer tun${1} ${1} ${LPORT} \ - ${RADDR} ${RPORT} - ip netns exec peer${1} ${OVPN_CLI} new_key tun${1} ${1} 1 0 ${ALG} 1 \ - data64.key + TX_ID=$(awk "NR == ${1} {print $2}" ${UDP_PEERS_FILE}) + RADDR=$(awk "NR == ${1} {print $3}" ${UDP_PEERS_FILE}) + RPORT=$(awk "NR == ${1} {print $4}" ${UDP_PEERS_FILE}) + LPORT=$(awk "NR == ${1} {print $6}" ${UDP_PEERS_FILE}) + ip netns exec peer${1} ${OVPN_CLI} new_peer tun${1} ${TX_ID} ${1} \ + ${LPORT} ${RADDR} ${RPORT} + ip netns exec peer${1} ${OVPN_CLI} new_key tun${1} ${TX_ID} 1 0 \ + ${ALG} 1 data64.key fi else if [ ${1} -eq 0 ]; then @@ -93,8 +94,9 @@ add_peer() { }) & sleep 5 else - ip netns exec peer${1} ${OVPN_CLI} connect tun${1} ${1} 10.10.${1}.1 1 \ - data64.key + TX_ID=$(awk "NR == ${1} {print $2}" ${TCP_PEERS_FILE}) + ip netns exec peer${1} ${OVPN_CLI} connect tun${1} ${TX_ID} ${1} \ + 10.10.${1}.1 1 data64.key fi fi } diff --git a/tools/testing/selftests/net/ovpn/json/peer1.json b/tools/testing/selftests/net/ovpn/json/peer1.json index 5da4ea9d51fb..1009d26dc14a 100644 --- a/tools/testing/selftests/net/ovpn/json/peer1.json +++ b/tools/testing/selftests/net/ovpn/json/peer1.json @@ -1 +1 @@ -{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 1}}} +{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 10}}} diff --git a/tools/testing/selftests/net/ovpn/json/peer2.json b/tools/testing/selftests/net/ovpn/json/peer2.json index 8f6db4f8c2ac..44e9fad2b622 100644 --- a/tools/testing/selftests/net/ovpn/json/peer2.json +++ b/tools/testing/selftests/net/ovpn/json/peer2.json @@ -1 +1 @@ -{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 2}}} +{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "userspace", "id": 11}}} diff --git a/tools/testing/selftests/net/ovpn/json/peer3.json b/tools/testing/selftests/net/ovpn/json/peer3.json index bdabd6fa2e64..d4be8ba130ae 100644 --- a/tools/testing/selftests/net/ovpn/json/peer3.json +++ b/tools/testing/selftests/net/ovpn/json/peer3.json @@ -1 +1 @@ -{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 3}}} +{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 12}}} diff --git a/tools/testing/selftests/net/ovpn/json/peer4.json b/tools/testing/selftests/net/ovpn/json/peer4.json index c3734bb9251b..67d27e2d48ac 100644 --- a/tools/testing/selftests/net/ovpn/json/peer4.json +++ b/tools/testing/selftests/net/ovpn/json/peer4.json @@ -1 +1 @@ -{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 4}}} +{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 13}}} diff --git a/tools/testing/selftests/net/ovpn/json/peer5.json b/tools/testing/selftests/net/ovpn/json/peer5.json index 46c4a348299d..ecd9bd0b2f37 100644 --- a/tools/testing/selftests/net/ovpn/json/peer5.json +++ b/tools/testing/selftests/net/ovpn/json/peer5.json @@ -1 +1 @@ -{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 5}}} +{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 14}}} diff --git a/tools/testing/selftests/net/ovpn/json/peer6.json b/tools/testing/selftests/net/ovpn/json/peer6.json index aa30f2cff625..7fded29c5804 100644 --- a/tools/testing/selftests/net/ovpn/json/peer6.json +++ b/tools/testing/selftests/net/ovpn/json/peer6.json @@ -1 +1 @@ -{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 6}}} +{"name": "peer-del-ntf", "msg": {"ifindex": 0, "peer": {"del-reason": "expired", "id": 15}}} diff --git a/tools/testing/selftests/net/ovpn/ovpn-cli.c b/tools/testing/selftests/net/ovpn/ovpn-cli.c index 064453d16fdd..baabb4c9120e 100644 --- a/tools/testing/selftests/net/ovpn/ovpn-cli.c +++ b/tools/testing/selftests/net/ovpn/ovpn-cli.c @@ -103,7 +103,7 @@ struct ovpn_ctx {

sa_family_t sa_family;

- unsigned long peer_id; + unsigned long peer_id, tx_id; unsigned long lport;

union { @@ -649,6 +649,7 @@ static int ovpn_new_peer(struct ovpn_ctx *ovpn, bool is_tcp)

attr = nla_nest_start(ctx->nl_msg, OVPN_A_PEER); NLA_PUT_U32(ctx->nl_msg, OVPN_A_PEER_ID, ovpn->peer_id); + NLA_PUT_U32(ctx->nl_msg, OVPN_A_PEER_TX_ID, ovpn->tx_id); NLA_PUT_U32(ctx->nl_msg, OVPN_A_PEER_SOCKET, ovpn->socket);

if (!is_tcp) { @@ -767,6 +768,10 @@ static int ovpn_handle_peer(struct nl_msg *msg, void (*arg)__always_unused) fprintf(stderr, "* Peer %u\n", nla_get_u32(pattrs[OVPN_A_PEER_ID]));

+ if (pattrs[OVPN_A_PEER_TX_ID]) + fprintf(stderr, "\tTX peer ID %u\n", + nla_get_u32(pattrs[OVPN_A_PEER_TX_ID])); + if (pattrs[OVPN_A_PEER_SOCKET_NETNSID]) fprintf(stderr, "\tsocket NetNS ID: %d\n", nla_get_s32(pattrs[OVPN_A_PEER_SOCKET_NETNSID])); @@ -1676,11 +1681,13 @@ static void usage(const char *cmd) "\tkey_file: file containing the symmetric key for encryption\n");

fprintf(stderr, - "* new_peer <iface> <peer_id> <lport> <raddr> <rport> [vpnaddr]: add new peer\n"); + "* new_peer <iface> <peer_id> <tx_id> <lport> <raddr> <rport> [vpnaddr]: add new peer\n"); fprintf(stderr, "\tiface: ovpn interface name\n"); fprintf(stderr, "\tlport: local UDP port to bind to\n"); fprintf(stderr, - "\tpeer_id: peer ID to be used in data packets to/from this peer\n"); + "\tpeer_id: peer ID found in data packets received from this peer\n"); + fprintf(stderr, + "\ttx_id: peer ID to be used when sending to this peer\n"); fprintf(stderr, "\traddr: peer IP address\n"); fprintf(stderr, "\trport: peer UDP port\n"); fprintf(stderr, "\tvpnaddr: peer VPN IP\n"); @@ -1691,7 +1698,7 @@ static void usage(const char *cmd) fprintf(stderr, "\tlport: local UDP port to bind to\n"); fprintf(stderr, "\tpeers_file: text file containing one peer per line. Line format:\n"); - fprintf(stderr, "\t\t<peer_id> <raddr> <rport> <vpnaddr>\n"); + fprintf(stderr, "\t\t<peer_id> <tx_id> <raddr> <rport> <laddr> <lport> <vpnaddr>\n");

fprintf(stderr, "* set_peer <iface> <peer_id> <keepalive_interval> <keepalive_timeout>: set peer attributes\n"); @@ -1804,12 +1811,18 @@ static int ovpn_parse_remote(struct ovpn_ctx *ovpn, const char *host, }

static int ovpn_parse_new_peer(struct ovpn_ctx *ovpn, const char *peer_id, - const char *raddr, const char *rport, - const char *vpnip) + const char *tx_id, const char *raddr, + const char *rport, const char *vpnip) { ovpn->peer_id = strtoul(peer_id, NULL, 10); if (errno == ERANGE || ovpn->peer_id > PEER_ID_UNDEF) { - fprintf(stderr, "peer ID value out of range\n"); + fprintf(stderr, "rx peer ID value out of range\n"); + return -1; + } + + ovpn->tx_id = strtoul(tx_id, NULL, 10); + if (errno == ERANGE || ovpn->tx_id > PEER_ID_UNDEF) { + fprintf(stderr, "tx peer ID value out of range\n"); return -1; }

@@ -1939,7 +1952,7 @@ static void ovpn_waitbg(void)

static int ovpn_run_cmd(struct ovpn_ctx *ovpn) { - char peer_id[10], vpnip[INET6_ADDRSTRLEN], laddr[128], lport[10]; + char peer_id[10], tx_id[10], vpnip[INET6_ADDRSTRLEN], laddr[128], lport[10]; char raddr[128], rport[10]; int n, ret; FILE *fp; @@ -1967,7 +1980,7 @@ static int ovpn_run_cmd(struct ovpn_ctx *ovpn)

int num_peers = 0;

- while ((n = fscanf(fp, "%s %s\n", peer_id, vpnip)) == 2) { + while ((n = fscanf(fp, "%s %s %s\n", peer_id, tx_id, vpnip)) == 3) { struct ovpn_ctx peer_ctx = { 0 };

if (num_peers == MAX_PEERS) { @@ -1987,7 +2000,7 @@ static int ovpn_run_cmd(struct ovpn_ctx *ovpn) /* store peer sockets to test TCP I/O */ ovpn->cli_sockets[num_peers] = peer_ctx.socket;

- ret = ovpn_parse_new_peer(&peer_ctx, peer_id, NULL, + ret = ovpn_parse_new_peer(&peer_ctx, peer_id, tx_id, NULL, NULL, vpnip); if (ret < 0) { fprintf(stderr, "error while parsing line\n"); @@ -2056,15 +2069,15 @@ static int ovpn_run_cmd(struct ovpn_ctx *ovpn) return -1; }

- while ((n = fscanf(fp, "%s %s %s %s %s %s\n", peer_id, laddr, - lport, raddr, rport, vpnip)) == 6) { + while ((n = fscanf(fp, "%s %s %s %s %s %s %s\n", peer_id, tx_id, laddr, + lport, raddr, rport, vpnip)) == 7) { struct ovpn_ctx peer_ctx = { 0 };

peer_ctx.ifindex = ovpn->ifindex; peer_ctx.socket = ovpn->socket; peer_ctx.sa_family = AF_UNSPEC;

- ret = ovpn_parse_new_peer(&peer_ctx, peer_id, raddr, + ret = ovpn_parse_new_peer(&peer_ctx, peer_id, tx_id, raddr, rport, vpnip); if (ret < 0) { fprintf(stderr, "error while parsing line\n"); @@ -2177,25 +2190,25 @@ static int ovpn_parse_cmd_args(struct ovpn_ctx *ovpn, int argc, char *argv[]) ovpn->sa_family = AF_INET6; break; case CMD_CONNECT: - if (argc < 6) + if (argc < 7) return -EINVAL;

ovpn->sa_family = AF_INET;

- ret = ovpn_parse_new_peer(ovpn, argv[3], argv[4], argv[5], + ret = ovpn_parse_new_peer(ovpn, argv[3], argv[4], argv[5], argv[6], NULL); if (ret < 0) { fprintf(stderr, "Cannot parse remote peer data\n"); return -1; }

- if (argc > 6) { + if (argc > 7) { ovpn->key_slot = OVPN_KEY_SLOT_PRIMARY; ovpn->key_id = 0; ovpn->cipher = OVPN_CIPHER_ALG_AES_GCM; ovpn->key_dir = KEY_DIR_OUT;

- ret = ovpn_parse_key(argv[6], ovpn); + ret = ovpn_parse_key(argv[7], ovpn); if (ret) return -1; } @@ -2204,15 +2217,15 @@ static int ovpn_parse_cmd_args(struct ovpn_ctx *ovpn, int argc, char *argv[]) if (argc < 7) return -EINVAL;

- ovpn->lport = strtoul(argv[4], NULL, 10); + ovpn->lport = strtoul(argv[5], NULL, 10); if (errno == ERANGE || ovpn->lport > 65535) { fprintf(stderr, "lport value out of range\n"); return -1; }

- const char *vpnip = (argc > 7) ? argv[7] : NULL; + const char *vpnip = (argc > 8) ? argv[8] : NULL;

- ret = ovpn_parse_new_peer(ovpn, argv[3], argv[5], argv[6], + ret = ovpn_parse_new_peer(ovpn, argv[3], argv[4], argv[6], argv[7], vpnip); if (ret < 0) return -1; diff --git a/tools/testing/selftests/net/ovpn/tcp_peers.txt b/tools/testing/selftests/net/ovpn/tcp_peers.txt index b8f3cb33eaa2..3cb67b560705 100644 --- a/tools/testing/selftests/net/ovpn/tcp_peers.txt +++ b/tools/testing/selftests/net/ovpn/tcp_peers.txt @@ -1,6 +1,6 @@ -1 5.5.5.2 -2 5.5.5.3 -3 5.5.5.4 -4 5.5.5.5 -5 5.5.5.6 -6 5.5.5.7 +1 10 5.5.5.2 +2 11 5.5.5.3 +3 12 5.5.5.4 +4 13 5.5.5.5 +5 14 5.5.5.6 +6 15 5.5.5.7 diff --git a/tools/testing/selftests/net/ovpn/test-close-socket.sh b/tools/testing/selftests/net/ovpn/test-close-socket.sh index 5e48a8b67928..0d09df14fe8e 100755 --- a/tools/testing/selftests/net/ovpn/test-close-socket.sh +++ b/tools/testing/selftests/net/ovpn/test-close-socket.sh @@ -27,7 +27,7 @@ done

for p in $(seq 1 ${NUM_PEERS}); do ip netns exec peer0 ${OVPN_CLI} set_peer tun0 ${p} 60 120 - ip netns exec peer${p} ${OVPN_CLI} set_peer tun${p} ${p} 60 120 + ip netns exec peer${p} ${OVPN_CLI} set_peer tun${p} $((${p}+9)) 60 120 done

sleep 1 diff --git a/tools/testing/selftests/net/ovpn/test.sh b/tools/testing/selftests/net/ovpn/test.sh index 3ec036fd7ebc..7fadf35813bd 100755 --- a/tools/testing/selftests/net/ovpn/test.sh +++ b/tools/testing/selftests/net/ovpn/test.sh @@ -33,14 +33,35 @@ done

for p in $(seq 1 ${NUM_PEERS}); do ip netns exec peer0 ${OVPN_CLI} set_peer tun0 ${p} 60 120 - ip netns exec peer${p} ${OVPN_CLI} set_peer tun${p} ${p} 60 120 + ip netns exec peer${p} ${OVPN_CLI} set_peer tun${p} $((${p}+9)) 60 120 done

sleep 1

+NGREP_TIMEOUT="1.5s" for p in $(seq 1 ${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 ("${p}" one way and "${p} + 9" the other way) + HEADER1=$(printf "0x4800000%x" ${p}) + HEADER2=$(printf "0x4800000%x" $((${p} + 9))) + CAPTURE_LEN=8 + + timeout ${NGREP_TIMEOUT} ip netns exec peer${p} ngrep -xqn 1 -X "${HEADER1}" \ + -S ${CAPTURE_LEN} -d veth${p} 1>/dev/null & + NGREP_PID1=$! + timeout ${NGREP_TIMEOUT} ip netns exec peer${p} ngrep -xqn 1 -X "${HEADER2}" \ + -S ${CAPTURE_LEN} -d veth${p} 1>/dev/null & + NGREP_PID2=$! + + sleep 0.3 ip netns exec peer0 ping -qfc 500 -w 3 5.5.5.$((${p} + 1)) ip netns exec peer0 ping -qfc 500 -s 3000 -w 3 5.5.5.$((${p} + 1)) + + wait ${NGREP_PID1} + wait ${NGREP_PID2} done

# ping LAN behind client 1 @@ -64,8 +85,8 @@ ip netns exec peer1 iperf3 -Z -t 3 -c 5.5.5.1 echo "Adding secondary key and then swap:" for p in $(seq 1 ${NUM_PEERS}); do ip netns exec peer0 ${OVPN_CLI} new_key tun0 ${p} 2 1 ${ALG} 0 data64.key - ip netns exec peer${p} ${OVPN_CLI} new_key tun${p} ${p} 2 1 ${ALG} 1 data64.key - ip netns exec peer${p} ${OVPN_CLI} swap_keys tun${p} ${p} + ip netns exec peer${p} ${OVPN_CLI} new_key tun${p} $((${p} + 9)) 2 1 ${ALG} 1 data64.key + ip netns exec peer${p} ${OVPN_CLI} swap_keys tun${p} $((${p} + 9)) done

sleep 1 @@ -77,17 +98,17 @@ ip netns exec peer1 ${OVPN_CLI} get_peer tun1 echo "Querying peer 1:" ip netns exec peer0 ${OVPN_CLI} get_peer tun0 1

-echo "Querying non-existent peer 10:" -ip netns exec peer0 ${OVPN_CLI} get_peer tun0 10 || true +echo "Querying non-existent peer 20:" +ip netns exec peer0 ${OVPN_CLI} get_peer tun0 20 || true

echo "Deleting peer 1:" ip netns exec peer0 ${OVPN_CLI} del_peer tun0 1 -ip netns exec peer1 ${OVPN_CLI} del_peer tun1 1 +ip netns exec peer1 ${OVPN_CLI} del_peer tun1 10

echo "Querying keys:" for p in $(seq 2 ${NUM_PEERS}); do - ip netns exec peer${p} ${OVPN_CLI} get_key tun${p} ${p} 1 - ip netns exec peer${p} ${OVPN_CLI} get_key tun${p} ${p} 2 + ip netns exec peer${p} ${OVPN_CLI} get_key tun${p} $((${p} + 9)) 1 + ip netns exec peer${p} ${OVPN_CLI} get_key tun${p} $((${p} + 9)) 2 done

echo "Deleting peer while sending traffic:" @@ -96,25 +117,25 @@ sleep 2 ip netns exec peer0 ${OVPN_CLI} del_peer tun0 2 # following command fails in TCP mode # (both ends get conn reset when one peer disconnects) -ip netns exec peer2 ${OVPN_CLI} del_peer tun2 2 || true +ip netns exec peer2 ${OVPN_CLI} del_peer tun2 11 || true

echo "Deleting keys:" for p in $(seq 3 ${NUM_PEERS}); do - ip netns exec peer${p} ${OVPN_CLI} del_key tun${p} ${p} 1 - ip netns exec peer${p} ${OVPN_CLI} del_key tun${p} ${p} 2 + ip netns exec peer${p} ${OVPN_CLI} del_key tun${p} $((${p} + 9)) 1 + ip netns exec peer${p} ${OVPN_CLI} del_key tun${p} $((${p} + 9)) 2 done

echo "Setting timeout to 3s MP:" for p in $(seq 3 ${NUM_PEERS}); do ip netns exec peer0 ${OVPN_CLI} set_peer tun0 ${p} 3 3 || true - ip netns exec peer${p} ${OVPN_CLI} set_peer tun${p} ${p} 0 0 + ip netns exec peer${p} ${OVPN_CLI} set_peer tun${p} $((${p} + 9)) 0 0 done # wait for peers to timeout sleep 5

echo "Setting timeout to 3s P2P:" for p in $(seq 3 ${NUM_PEERS}); do - ip netns exec peer${p} ${OVPN_CLI} set_peer tun${p} ${p} 3 3 + ip netns exec peer${p} ${OVPN_CLI} set_peer tun${p} $((${p} + 9)) 3 3 done sleep 5

diff --git a/tools/testing/selftests/net/ovpn/udp_peers.txt b/tools/testing/selftests/net/ovpn/udp_peers.txt index e9773ddf875c..93de6465353c 100644 --- a/tools/testing/selftests/net/ovpn/udp_peers.txt +++ b/tools/testing/selftests/net/ovpn/udp_peers.txt @@ -1,6 +1,6 @@ -1 10.10.1.1 1 10.10.1.2 1 5.5.5.2 -2 10.10.2.1 1 10.10.2.2 1 5.5.5.3 -3 10.10.3.1 1 10.10.3.2 1 5.5.5.4 -4 fd00:0:0:4::1 1 fd00:0:0:4::2 1 5.5.5.5 -5 fd00:0:0:5::1 1 fd00:0:0:5::2 1 5.5.5.6 -6 fd00:0:0:6::1 1 fd00:0:0:6::2 1 5.5.5.7 +1 10 10.10.1.1 1 10.10.1.2 1 5.5.5.2 +2 11 10.10.2.1 1 10.10.2.2 1 5.5.5.3 +3 12 10.10.3.1 1 10.10.3.2 1 5.5.5.4 +4 13 fd00:0:0:4::1 1 fd00:0:0:4::2 1 5.5.5.5 +5 14 fd00:0:0:5::1 1 fd00:0:0:5::2 1 5.5.5.6 +6 15 fd00:0:0:6::1 1 fd00:0:0:6::2 1 5.5.5.7

From: Ralf Lici ralf@mandelbit.com

Currently ovpn uses three separate dynamically allocated structures to set up cryptographic operations for both encryption and decryption. This adds overhead to performance-critical paths and contribute to memory fragmentation.

This commit consolidates those allocations into a single temporary blob, similar to what esp_alloc_tmp() does.

The resulting performance gain is +7.7% and +4.3% for UDP when using AES and ChaChaPoly respectively, and +4.3% for TCP.

Reviewed-by: Sabrina Dubroca sd@queasysnail.net Signed-off-by: Ralf Lici ralf@mandelbit.com Signed-off-by: Antonio Quartulli antonio@openvpn.net --- drivers/net/ovpn/crypto_aead.c | 160 +++++++++++++++++++++++++-------- drivers/net/ovpn/io.c | 8 +- drivers/net/ovpn/skb.h | 13 ++- 3 files changed, 135 insertions(+), 46 deletions(-)

diff --git a/drivers/net/ovpn/crypto_aead.c b/drivers/net/ovpn/crypto_aead.c index cb6cdf8ec317..cd723140c8d0 100644 --- a/drivers/net/ovpn/crypto_aead.c +++ b/drivers/net/ovpn/crypto_aead.c @@ -36,6 +36,104 @@ static int ovpn_aead_encap_overhead(const struct ovpn_crypto_key_slot *ks) crypto_aead_authsize(ks->encrypt); /* Auth Tag */ }

+/** + * ovpn_aead_crypto_tmp_size - compute the size of a temporary object containing + * an AEAD request structure with extra space for SG + * and IV. + * @tfm: the AEAD cipher handle + * @nfrags: the number of fragments in the skb + * + * This function calculates the size of a contiguous memory block that includes + * the initialization vector (IV), the AEAD request, and an array of scatterlist + * entries. For alignment considerations, the IV is placed first, followed by + * the request, and then the scatterlist. + * Additional alignment is applied according to the requirements of the + * underlying structures. + * + * Return: the size of the temporary memory that needs to be allocated + */ +static unsigned int ovpn_aead_crypto_tmp_size(struct crypto_aead *tfm, + const unsigned int nfrags) +{ + unsigned int len = OVPN_NONCE_SIZE; + + DEBUG_NET_WARN_ON_ONCE(OVPN_NONCE_SIZE != crypto_aead_ivsize(tfm)); + + /* min size for a buffer of ivsize, aligned to alignmask */ + len += crypto_aead_alignmask(tfm) & ~(crypto_tfm_ctx_alignment() - 1); + /* round up to the next multiple of the crypto ctx alignment */ + len = ALIGN(len, crypto_tfm_ctx_alignment()); + + /* reserve space for the AEAD request */ + len += sizeof(struct aead_request) + crypto_aead_reqsize(tfm); + /* round up to the next multiple of the scatterlist alignment */ + len = ALIGN(len, __alignof__(struct scatterlist)); + + /* add enough space for nfrags + 2 scatterlist entries */ + len += array_size(sizeof(struct scatterlist), nfrags + 2); + return len; +} + +/** + * ovpn_aead_crypto_tmp_iv - retrieve the pointer to the IV within a temporary + * buffer allocated using ovpn_aead_crypto_tmp_size + * @aead: the AEAD cipher handle + * @tmp: a pointer to the beginning of the temporary buffer + * + * This function retrieves a pointer to the initialization vector (IV) in the + * temporary buffer. If the AEAD cipher specifies an IV size, the pointer is + * adjusted using the AEAD's alignment mask to ensure proper alignment. + * + * Returns: a pointer to the IV within the temporary buffer + */ +static u8 *ovpn_aead_crypto_tmp_iv(struct crypto_aead *aead, void *tmp) +{ + return likely(crypto_aead_ivsize(aead)) ? + PTR_ALIGN((u8 *)tmp, crypto_aead_alignmask(aead) + 1) : + tmp; +} + +/** + * ovpn_aead_crypto_tmp_req - retrieve the pointer to the AEAD request structure + * within a temporary buffer allocated using + * ovpn_aead_crypto_tmp_size + * @aead: the AEAD cipher handle + * @iv: a pointer to the initialization vector in the temporary buffer + * + * This function computes the location of the AEAD request structure that + * immediately follows the IV in the temporary buffer and it ensures the request + * is aligned to the crypto transform context alignment. + * + * Returns: a pointer to the AEAD request structure + */ +static struct aead_request *ovpn_aead_crypto_tmp_req(struct crypto_aead *aead, + const u8 *iv) +{ + return (void *)PTR_ALIGN(iv + crypto_aead_ivsize(aead), + crypto_tfm_ctx_alignment()); +} + +/** + * ovpn_aead_crypto_req_sg - locate the scatterlist following the AEAD request + * within a temporary buffer allocated using + * ovpn_aead_crypto_tmp_size + * @aead: the AEAD cipher handle + * @req: a pointer to the AEAD request structure in the temporary buffer + * + * This function computes the starting address of the scatterlist that is + * allocated immediately after the AEAD request structure. It aligns the pointer + * based on the alignment requirements of the scatterlist structure. + * + * Returns: a pointer to the scatterlist + */ +static struct scatterlist *ovpn_aead_crypto_req_sg(struct crypto_aead *aead, + struct aead_request *req) +{ + return (void *)ALIGN((unsigned long)(req + 1) + + crypto_aead_reqsize(aead), + __alignof__(struct scatterlist)); +} + int ovpn_aead_encrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks, struct sk_buff *skb) { @@ -45,6 +143,7 @@ int ovpn_aead_encrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks, struct scatterlist *sg; int nfrags, ret; u32 pktid, op; + void *tmp; u8 *iv;

ovpn_skb_cb(skb)->peer = peer; @@ -71,13 +170,17 @@ int ovpn_aead_encrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks, if (unlikely(nfrags + 2 > (MAX_SKB_FRAGS + 2))) return -ENOSPC;

- /* sg may be required by async crypto */ - ovpn_skb_cb(skb)->sg = kmalloc(sizeof(*ovpn_skb_cb(skb)->sg) * - (nfrags + 2), GFP_ATOMIC); - if (unlikely(!ovpn_skb_cb(skb)->sg)) + /* allocate temporary memory for iv, sg and req */ + tmp = kmalloc(ovpn_aead_crypto_tmp_size(ks->encrypt, nfrags), + GFP_ATOMIC); + if (unlikely(!tmp)) return -ENOMEM;

- sg = ovpn_skb_cb(skb)->sg; + ovpn_skb_cb(skb)->crypto_tmp = tmp; + + iv = ovpn_aead_crypto_tmp_iv(ks->encrypt, tmp); + req = ovpn_aead_crypto_tmp_req(ks->encrypt, iv); + sg = ovpn_aead_crypto_req_sg(ks->encrypt, req);

/* sg table: * 0: op, wire nonce (AD, len=OVPN_OP_SIZE_V2+OVPN_NONCE_WIRE_SIZE), @@ -105,13 +208,6 @@ int ovpn_aead_encrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks, if (unlikely(ret < 0)) return ret;

- /* iv may be required by async crypto */ - ovpn_skb_cb(skb)->iv = kmalloc(OVPN_NONCE_SIZE, GFP_ATOMIC); - if (unlikely(!ovpn_skb_cb(skb)->iv)) - return -ENOMEM; - - iv = ovpn_skb_cb(skb)->iv; - /* concat 4 bytes packet id and 8 bytes nonce tail into 12 bytes * nonce */ @@ -130,12 +226,6 @@ int ovpn_aead_encrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks, /* AEAD Additional data */ sg_set_buf(sg, skb->data, OVPN_AAD_SIZE);

- req = aead_request_alloc(ks->encrypt, GFP_ATOMIC); - if (unlikely(!req)) - return -ENOMEM; - - ovpn_skb_cb(skb)->req = req; - /* setup async crypto operation */ aead_request_set_tfm(req, ks->encrypt); aead_request_set_callback(req, 0, ovpn_encrypt_post, skb); @@ -156,6 +246,7 @@ int ovpn_aead_decrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks, struct aead_request *req; struct sk_buff *trailer; struct scatterlist *sg; + void *tmp; u8 *iv;

payload_offset = OVPN_AAD_SIZE + tag_size; @@ -184,13 +275,17 @@ int ovpn_aead_decrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks, if (unlikely(nfrags + 2 > (MAX_SKB_FRAGS + 2))) return -ENOSPC;

- /* sg may be required by async crypto */ - ovpn_skb_cb(skb)->sg = kmalloc(sizeof(*ovpn_skb_cb(skb)->sg) * - (nfrags + 2), GFP_ATOMIC); - if (unlikely(!ovpn_skb_cb(skb)->sg)) + /* allocate temporary memory for iv, sg and req */ + tmp = kmalloc(ovpn_aead_crypto_tmp_size(ks->decrypt, nfrags), + GFP_ATOMIC); + if (unlikely(!tmp)) return -ENOMEM;

- sg = ovpn_skb_cb(skb)->sg; + ovpn_skb_cb(skb)->crypto_tmp = tmp; + + iv = ovpn_aead_crypto_tmp_iv(ks->decrypt, tmp); + req = ovpn_aead_crypto_tmp_req(ks->decrypt, iv); + sg = ovpn_aead_crypto_req_sg(ks->decrypt, req);

/* sg table: * 0: op, wire nonce (AD, len=OVPN_OPCODE_SIZE+OVPN_NONCE_WIRE_SIZE), @@ -213,24 +308,11 @@ int ovpn_aead_decrypt(struct ovpn_peer *peer, struct ovpn_crypto_key_slot *ks, /* append auth_tag onto scatterlist */ sg_set_buf(sg + ret + 1, skb->data + OVPN_AAD_SIZE, tag_size);

- /* iv may be required by async crypto */ - ovpn_skb_cb(skb)->iv = kmalloc(OVPN_NONCE_SIZE, GFP_ATOMIC); - if (unlikely(!ovpn_skb_cb(skb)->iv)) - return -ENOMEM; - - iv = ovpn_skb_cb(skb)->iv; - /* copy nonce into IV buffer */ memcpy(iv, skb->data + OVPN_OPCODE_SIZE, OVPN_NONCE_WIRE_SIZE); memcpy(iv + OVPN_NONCE_WIRE_SIZE, ks->nonce_tail_recv, OVPN_NONCE_TAIL_SIZE);

- req = aead_request_alloc(ks->decrypt, GFP_ATOMIC); - if (unlikely(!req)) - return -ENOMEM; - - ovpn_skb_cb(skb)->req = req; - /* setup async crypto operation */ aead_request_set_tfm(req, ks->decrypt); aead_request_set_callback(req, 0, ovpn_decrypt_post, skb); @@ -273,7 +355,11 @@ static struct crypto_aead *ovpn_aead_init(const char *title, goto error; }

- /* basic AEAD assumption */ + /* basic AEAD assumption + * all current algorithms use OVPN_NONCE_SIZE. + * ovpn_aead_crypto_tmp_size and ovpn_aead_encrypt/decrypt + * expect this. + */ if (crypto_aead_ivsize(aead) != OVPN_NONCE_SIZE) { pr_err("%s IV size must be %d\n", title, OVPN_NONCE_SIZE); ret = -EINVAL; diff --git a/drivers/net/ovpn/io.c b/drivers/net/ovpn/io.c index 3e9e7f8444b3..2721ee8268b2 100644 --- a/drivers/net/ovpn/io.c +++ b/drivers/net/ovpn/io.c @@ -119,9 +119,7 @@ void ovpn_decrypt_post(void *data, int ret) peer = ovpn_skb_cb(skb)->peer;

/* crypto is done, cleanup skb CB and its members */ - kfree(ovpn_skb_cb(skb)->iv); - kfree(ovpn_skb_cb(skb)->sg); - aead_request_free(ovpn_skb_cb(skb)->req); + kfree(ovpn_skb_cb(skb)->crypto_tmp);

if (unlikely(ret < 0)) goto drop; @@ -248,9 +246,7 @@ void ovpn_encrypt_post(void *data, int ret) peer = ovpn_skb_cb(skb)->peer;

/* crypto is done, cleanup skb CB and its members */ - kfree(ovpn_skb_cb(skb)->iv); - kfree(ovpn_skb_cb(skb)->sg); - aead_request_free(ovpn_skb_cb(skb)->req); + kfree(ovpn_skb_cb(skb)->crypto_tmp);

if (unlikely(ret == -ERANGE)) { /* we ran out of IVs and we must kill the key as it can't be diff --git a/drivers/net/ovpn/skb.h b/drivers/net/ovpn/skb.h index 64430880f1da..4fb7ea025426 100644 --- a/drivers/net/ovpn/skb.h +++ b/drivers/net/ovpn/skb.h @@ -18,12 +18,19 @@ #include <linux/socket.h> #include <linux/types.h>

+/** + * struct ovpn_cb - ovpn skb control block + * @peer: the peer this skb was received from/sent to + * @ks: the crypto key slot used to encrypt/decrypt this skb + * @crypto_tmp: pointer to temporary memory used for crypto operations + * containing the IV, the scatter gather list and the aead request + * @payload_offset: offset in the skb where the payload starts + * @nosignal: whether this skb should be sent with the MSG_NOSIGNAL flag (TCP) + */ struct ovpn_cb { struct ovpn_peer *peer; struct ovpn_crypto_key_slot *ks; - struct aead_request *req; - struct scatterlist *sg; - u8 *iv; + void *crypto_tmp; unsigned int payload_offset; bool nosignal; };

From: Ralf Lici ralf@mandelbit.com

Add a selftest to verify that when a socket is bound to a device, UDP traffic from ovpn is correctly routed through the specified interface.

The test sets up a P2P session between two peers in separate network namespaces, connected via two veth pairs. It binds to both veth interfaces and uses tcpdump to confirm that traffic flows through the expected paths.

Cc: Shuah Khan shuah@kernel.org Signed-off-by: Ralf Lici ralf@mandelbit.com Signed-off-by: Antonio Quartulli antonio@openvpn.net --- tools/testing/selftests/net/ovpn/Makefile | 1 + tools/testing/selftests/net/ovpn/common.sh | 6 +- tools/testing/selftests/net/ovpn/ovpn-cli.c | 39 +++++-- tools/testing/selftests/net/ovpn/test-bind.sh | 103 ++++++++++++++++++ tools/testing/selftests/net/ovpn/test-mark.sh | 2 +- 5 files changed, 137 insertions(+), 14 deletions(-) create mode 100755 tools/testing/selftests/net/ovpn/test-bind.sh

diff --git a/tools/testing/selftests/net/ovpn/Makefile b/tools/testing/selftests/net/ovpn/Makefile index 7c87c95d957e..f219d87e2c44 100644 --- a/tools/testing/selftests/net/ovpn/Makefile +++ b/tools/testing/selftests/net/ovpn/Makefile @@ -26,6 +26,7 @@ LDLIBS += $(NL_LDLIBS) TEST_FILES = common.sh

TEST_PROGS := \ + test-bind.sh \ test-chachapoly.sh \ test-close-socket-tcp.sh \ test-close-socket.sh \ diff --git a/tools/testing/selftests/net/ovpn/common.sh b/tools/testing/selftests/net/ovpn/common.sh index d926413c9f16..c802e4e50054 100644 --- a/tools/testing/selftests/net/ovpn/common.sh +++ b/tools/testing/selftests/net/ovpn/common.sh @@ -66,9 +66,11 @@ setup_listener() { }

add_peer() { + dev=${2:-"any"} + if [ "${PROTO}" == "UDP" ]; then if [ ${1} -eq 0 ]; then - ip netns exec peer0 ${OVPN_CLI} new_multi_peer tun0 1 ${UDP_PEERS_FILE} + ip netns exec peer0 ${OVPN_CLI} new_multi_peer tun0 ${dev} 1 ${UDP_PEERS_FILE}

for p in $(seq 1 ${NUM_PEERS}); do ip netns exec peer0 ${OVPN_CLI} new_key tun0 ${p} 1 0 ${ALG} 0 \ @@ -79,7 +81,7 @@ add_peer() { RADDR=$(awk "NR == ${1} {print $3}" ${UDP_PEERS_FILE}) RPORT=$(awk "NR == ${1} {print $4}" ${UDP_PEERS_FILE}) LPORT=$(awk "NR == ${1} {print $6}" ${UDP_PEERS_FILE}) - ip netns exec peer${1} ${OVPN_CLI} new_peer tun${1} ${TX_ID} ${1} \ + ip netns exec peer${1} ${OVPN_CLI} new_peer tun${1} ${dev} ${TX_ID} ${1} \ ${LPORT} ${RADDR} ${RPORT} ip netns exec peer${1} ${OVPN_CLI} new_key tun${1} ${TX_ID} 1 0 \ ${ALG} 1 data64.key diff --git a/tools/testing/selftests/net/ovpn/ovpn-cli.c b/tools/testing/selftests/net/ovpn/ovpn-cli.c index 4df596d29b8c..6d84380c76ad 100644 --- a/tools/testing/selftests/net/ovpn/ovpn-cli.c +++ b/tools/testing/selftests/net/ovpn/ovpn-cli.c @@ -135,6 +135,7 @@ struct ovpn_ctx { int key_id;

uint32_t mark; + const char *bind_dev;

const char *peers_file; }; @@ -542,6 +543,14 @@ static int ovpn_socket(struct ovpn_ctx *ctx, sa_family_t family, int proto) } }

+ if (ctx->bind_dev) { + if (setsockopt(s, SOL_SOCKET, SO_BINDTODEVICE, ctx->bind_dev, + strlen(ctx->bind_dev) + 1) != 0) { + perror("setsockopt for SO_BINDTODEVICE"); + return -1; + } + } + ret = bind(s, (struct sockaddr *)&local_sock, sock_len); if (ret < 0) { perror("cannot bind socket"); @@ -1693,8 +1702,10 @@ static void usage(const char *cmd) "\tkey_file: file containing the symmetric key for encryption\n");

fprintf(stderr, - "* new_peer <iface> <peer_id> <tx_id> <lport> <raddr> <rport> [vpnaddr]: add new peer\n"); + "* new_peer <iface> <dev> <peer_id> <tx_id> <lport> <raddr> <rport> [vpnaddr]: add new peer\n"); fprintf(stderr, "\tiface: ovpn interface name\n"); + fprintf(stderr, + "\tdev: transport interface name to bind to, supports 'any'\n"); fprintf(stderr, "\tlport: local UDP port to bind to\n"); fprintf(stderr, "\tpeer_id: peer ID found in data packets received from this peer\n"); @@ -1705,8 +1716,10 @@ static void usage(const char *cmd) fprintf(stderr, "\tvpnaddr: peer VPN IP\n");

fprintf(stderr, - "* new_multi_peer <iface> <lport> <peers_file> [mark]: add multiple peers as listed in the file\n"); + "* new_multi_peer <iface> <dev> <lport> <peers_file> [mark]: add multiple peers as listed in the file\n"); fprintf(stderr, "\tiface: ovpn interface name\n"); + fprintf(stderr, + "\tdev: transport interface name to bind to, supports 'any'\n"); fprintf(stderr, "\tlport: local UDP port to bind to\n"); fprintf(stderr, "\tpeers_file: text file containing one peer per line. Line format:\n"); @@ -2227,37 +2240,41 @@ static int ovpn_parse_cmd_args(struct ovpn_ctx *ovpn, int argc, char *argv[]) } break; case CMD_NEW_PEER: - if (argc < 7) + if (argc < 8) return -EINVAL;

- ovpn->lport = strtoul(argv[5], NULL, 10); + ovpn->bind_dev = strcmp(argv[3], "any") == 0 ? NULL : argv[3]; + + ovpn->lport = strtoul(argv[6], NULL, 10); if (errno == ERANGE || ovpn->lport > 65535) { fprintf(stderr, "lport value out of range\n"); return -1; }

- const char *vpnip = (argc > 8) ? argv[8] : NULL; + const char *vpnip = (argc > 9) ? argv[9] : NULL;

- ret = ovpn_parse_new_peer(ovpn, argv[3], argv[4], argv[6], argv[7], + ret = ovpn_parse_new_peer(ovpn, argv[4], argv[5], argv[7], argv[8], vpnip); if (ret < 0) return -1; break; case CMD_NEW_MULTI_PEER: - if (argc < 5) + if (argc < 6) return -EINVAL;

- ovpn->lport = strtoul(argv[3], NULL, 10); + ovpn->bind_dev = strcmp(argv[3], "any") == 0 ? NULL : argv[3]; + + ovpn->lport = strtoul(argv[4], NULL, 10); if (errno == ERANGE || ovpn->lport > 65535) { fprintf(stderr, "lport value out of range\n"); return -1; }

- ovpn->peers_file = argv[4]; + ovpn->peers_file = argv[5];

ovpn->mark = 0; - if (argc > 5) { - ovpn->mark = strtoul(argv[5], NULL, 10); + if (argc > 6) { + ovpn->mark = strtoul(argv[6], NULL, 10); if (errno == ERANGE || ovpn->mark > UINT32_MAX) { fprintf(stderr, "mark value out of range\n"); return -1; diff --git a/tools/testing/selftests/net/ovpn/test-bind.sh b/tools/testing/selftests/net/ovpn/test-bind.sh new file mode 100755 index 000000000000..fd7c3c8fdf63 --- /dev/null +++ b/tools/testing/selftests/net/ovpn/test-bind.sh @@ -0,0 +1,103 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2020-2025 OpenVPN, Inc. +# +# Author: Ralf Lici ralf@mandelbit.com +# Antonio Quartulli antonio@openvpn.net + +#set -x +set -e + +PROTO=UDP +source ./common.sh + +cleanup + +modprobe -q ovpn || true + +# setup a P2P session between peer1 and peer2 + +ip netns add peer1 +ip netns add peer2 + +ip link add veth1 netns peer1 type veth peer name veth1 netns peer2 +ip link add veth2 netns peer1 type veth peer name veth2 netns peer2 + +ip -n peer1 addr add 10.10.10.1/24 dev veth1 +ip -n peer1 link set veth1 up + +ip -n peer1 addr add 20.20.20.1/24 dev veth2 +ip -n peer1 link set veth2 up + +ip -n peer2 addr add 10.10.10.2/24 dev veth1 +ip -n peer2 link set veth1 up + +ip -n peer2 addr add 20.20.20.2/24 dev veth2 +ip -n peer2 link set veth2 up + +ip netns exec peer1 ${OVPN_CLI} new_iface tun1 P2P +ip netns exec peer2 ${OVPN_CLI} new_iface tun2 P2P + +ip -n peer1 addr add 5.5.5.1 dev tun1 +ip -n peer1 link set tun1 up +ip -n peer2 addr add 5.5.5.2 dev tun2 +ip -n peer2 link set tun2 up + +ip -n peer1 route add 5.5.5.0/24 dev tun1 +ip -n peer2 route add 5.5.5.0/24 dev tun2 + +run_bind_test() { + dev1=${1} + dev2=${2} + raddr4_peer1=${3} + raddr4_peer2=${4} + + touch /tmp/ovpn-bind1.log + touch /tmp/ovpn-bind2.log + + ip netns exec peer1 ${OVPN_CLI} del_peer tun1 1 2>/dev/null || true + ip netns exec peer2 ${OVPN_CLI} del_peer tun2 10 2>/dev/null || true + + # close any active socket + killall $(basename ${OVPN_CLI}) 2>/dev/null || true + + ip netns exec peer1 ${OVPN_CLI} new_peer tun1 ${dev1} 1 10 1 ${raddr4_peer1} 1 + ip netns exec peer1 ${OVPN_CLI} new_key tun1 1 1 0 ${ALG} 0 data64.key + ip netns exec peer2 ${OVPN_CLI} new_peer tun2 ${dev2} 10 1 1 ${raddr4_peer2} 1 + ip netns exec peer2 ${OVPN_CLI} new_key tun2 10 1 0 ${ALG} 1 data64.key + + ip netns exec peer1 ${OVPN_CLI} set_peer tun1 1 60 120 + ip netns exec peer2 ${OVPN_CLI} set_peer tun2 10 60 120 + + timeout 2 ip netns exec peer1 tcpdump -i veth1 "${PROTO,,}" port 1 -n -q > /tmp/ovpn-bind1.log & + tcpdump1_pid=$! + timeout 2 ip netns exec peer1 tcpdump -i veth2 "${PROTO,,}" port 1 -n -q > /tmp/ovpn-bind2.log & + tcpdump2_pid=$! + sleep 0.5 + + ip netns exec peer1 ping -qfc 50 -w 1 5.5.5.2 + + wait ${tcpdump1_pid} || true + wait ${tcpdump2_pid} || true +} + +run_bind_test veth1 any 10.10.10.2 10.10.10.1 +[ "$(grep -c -i udp /tmp/ovpn-bind1.log)" -ge 100 ] +[ "$(grep -c -i udp /tmp/ovpn-bind2.log)" -eq 0 ] + +run_bind_test veth2 any 20.20.20.2 20.20.20.1 +[ "$(grep -c -i udp /tmp/ovpn-bind2.log)" -ge 100 ] +[ "$(grep -c -i udp /tmp/ovpn-bind1.log)" -eq 0 ] + +run_bind_test any veth1 10.10.10.2 10.10.10.1 +[ "$(grep -c -i udp /tmp/ovpn-bind1.log)" -ge 100 ] +[ "$(grep -c -i udp /tmp/ovpn-bind2.log)" -eq 0 ] + +run_bind_test any veth2 20.20.20.2 20.20.20.1 +[ "$(grep -c -i udp /tmp/ovpn-bind2.log)" -ge 100 ] +[ "$(grep -c -i udp /tmp/ovpn-bind1.log)" -eq 0 ] + +cleanup + +modprobe -r ovpn || true + diff --git a/tools/testing/selftests/net/ovpn/test-mark.sh b/tools/testing/selftests/net/ovpn/test-mark.sh index a4bfe938118d..c2600bb22e2c 100755 --- a/tools/testing/selftests/net/ovpn/test-mark.sh +++ b/tools/testing/selftests/net/ovpn/test-mark.sh @@ -26,7 +26,7 @@ for p in $(seq 0 3); do done

# add peer0 with mark -ip netns exec peer0 ${OVPN_CLI} new_multi_peer tun0 1 ${UDP_PEERS_FILE} ${MARK} +ip netns exec peer0 ${OVPN_CLI} new_multi_peer tun0 any 1 ${UDP_PEERS_FILE} ${MARK} for p in $(seq 1 3); do ip netns exec peer0 ${OVPN_CLI} new_key tun0 ${p} 1 0 ${ALG} 0 data64.key done

From: Ralf Lici ralf@mandelbit.com

Add a selftest to verify that when a socket is bound to a local address, UDP traffic from ovpn is correctly routed through that address.

This test extends test-bind.sh by binding to the addresses on each veth pair and uses tcpdump to confirm that traffic flows as expected.

Cc: Shuah Khan shuah@kernel.org Signed-off-by: Ralf Lici ralf@mandelbit.com Signed-off-by: Antonio Quartulli antonio@openvpn.net --- tools/testing/selftests/net/ovpn/Makefile | 1 + tools/testing/selftests/net/ovpn/common.sh | 6 +- tools/testing/selftests/net/ovpn/ovpn-cli.c | 202 +++++++++--------- .../selftests/net/ovpn/test-bind-addr.sh | 10 + tools/testing/selftests/net/ovpn/test-bind.sh | 52 +++-- tools/testing/selftests/net/ovpn/test-mark.sh | 2 +- 6 files changed, 152 insertions(+), 121 deletions(-) create mode 100755 tools/testing/selftests/net/ovpn/test-bind-addr.sh

diff --git a/tools/testing/selftests/net/ovpn/Makefile b/tools/testing/selftests/net/ovpn/Makefile index f219d87e2c44..7a5ad7a19273 100644 --- a/tools/testing/selftests/net/ovpn/Makefile +++ b/tools/testing/selftests/net/ovpn/Makefile @@ -26,6 +26,7 @@ LDLIBS += $(NL_LDLIBS) TEST_FILES = common.sh

TEST_PROGS := \ + test-bind-addr.sh \ test-bind.sh \ test-chachapoly.sh \ test-close-socket-tcp.sh \ diff --git a/tools/testing/selftests/net/ovpn/common.sh b/tools/testing/selftests/net/ovpn/common.sh index c802e4e50054..5de42948d189 100644 --- a/tools/testing/selftests/net/ovpn/common.sh +++ b/tools/testing/selftests/net/ovpn/common.sh @@ -67,10 +67,12 @@ setup_listener() {

add_peer() { dev=${2:-"any"} + laddr=${3:-"any"}

if [ "${PROTO}" == "UDP" ]; then if [ ${1} -eq 0 ]; then - ip netns exec peer0 ${OVPN_CLI} new_multi_peer tun0 ${dev} 1 ${UDP_PEERS_FILE} + ip netns exec peer0 ${OVPN_CLI} new_multi_peer tun0 ${dev} ${laddr} \ + 1 ${UDP_PEERS_FILE}

for p in $(seq 1 ${NUM_PEERS}); do ip netns exec peer0 ${OVPN_CLI} new_key tun0 ${p} 1 0 ${ALG} 0 \ @@ -82,7 +84,7 @@ add_peer() { RPORT=$(awk "NR == ${1} {print $4}" ${UDP_PEERS_FILE}) LPORT=$(awk "NR == ${1} {print $6}" ${UDP_PEERS_FILE}) ip netns exec peer${1} ${OVPN_CLI} new_peer tun${1} ${dev} ${TX_ID} ${1} \ - ${LPORT} ${RADDR} ${RPORT} + ${laddr} ${LPORT} ${RADDR} ${RPORT} ip netns exec peer${1} ${OVPN_CLI} new_key tun${1} ${TX_ID} 1 0 \ ${ALG} 1 data64.key fi diff --git a/tools/testing/selftests/net/ovpn/ovpn-cli.c b/tools/testing/selftests/net/ovpn/ovpn-cli.c index 6d84380c76ad..7f7780a515d1 100644 --- a/tools/testing/selftests/net/ovpn/ovpn-cli.c +++ b/tools/testing/selftests/net/ovpn/ovpn-cli.c @@ -105,7 +105,7 @@ struct ovpn_ctx { sa_family_t sa_family;

unsigned long peer_id, tx_id; - unsigned long lport; + const char *laddr, *lport;

union { struct sockaddr_in in4; @@ -470,59 +470,29 @@ static int ovpn_parse_key_direction(const char *dir, struct ovpn_ctx *ctx) return 0; }

-static int ovpn_socket(struct ovpn_ctx *ctx, sa_family_t family, int proto) +static int ovpn_socket(struct ovpn_ctx *ctx, sa_family_t family, int type) { - struct sockaddr_storage local_sock = { 0 }; - struct sockaddr_in6 *in6; - struct sockaddr_in *in; - int ret, s, sock_type; - size_t sock_len; - - if (proto == IPPROTO_UDP) - sock_type = SOCK_DGRAM; - else if (proto == IPPROTO_TCP) - sock_type = SOCK_STREAM; - else - return -EINVAL; + int ret, s;

- s = socket(family, sock_type, 0); + s = socket(family, type, 0); if (s < 0) { perror("cannot create socket"); return -1; }

- switch (family) { - case AF_INET: - in = (struct sockaddr_in *)&local_sock; - in->sin_family = family; - in->sin_port = htons(ctx->lport); - in->sin_addr.s_addr = htonl(INADDR_ANY); - sock_len = sizeof(*in); - break; - case AF_INET6: - in6 = (struct sockaddr_in6 *)&local_sock; - in6->sin6_family = family; - in6->sin6_port = htons(ctx->lport); - in6->sin6_addr = in6addr_any; - sock_len = sizeof(*in6); - break; - default: - return -1; - } - int opt = 1;

ret = setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt));

if (ret < 0) { perror("setsockopt for SO_REUSEADDR"); - return ret; + goto close; }

ret = setsockopt(s, SOL_SOCKET, SO_REUSEPORT, &opt, sizeof(opt)); if (ret < 0) { perror("setsockopt for SO_REUSEPORT"); - return ret; + goto close; }

if (ctx->mark != 0) { @@ -530,16 +500,17 @@ static int ovpn_socket(struct ovpn_ctx *ctx, sa_family_t family, int proto) sizeof(ctx->mark)); if (ret < 0) { perror("setsockopt for SO_MARK"); - return ret; + goto close; } }

if (family == AF_INET6) { opt = 0; - if (setsockopt(s, IPPROTO_IPV6, IPV6_V6ONLY, &opt, - sizeof(opt))) { + ret = setsockopt(s, IPPROTO_IPV6, IPV6_V6ONLY, &opt, + sizeof(opt)); + if (ret < 0) { perror("failed to set IPV6_V6ONLY"); - return -1; + goto close; } }

@@ -547,45 +518,83 @@ static int ovpn_socket(struct ovpn_ctx *ctx, sa_family_t family, int proto) if (setsockopt(s, SOL_SOCKET, SO_BINDTODEVICE, ctx->bind_dev, strlen(ctx->bind_dev) + 1) != 0) { perror("setsockopt for SO_BINDTODEVICE"); - return -1; + goto close; } }

- ret = bind(s, (struct sockaddr *)&local_sock, sock_len); - if (ret < 0) { - perror("cannot bind socket"); - goto err_socket; + return s; +close: + close(s); + return ret; +} + +static int ovpn_setup_socket(struct ovpn_ctx *ctx, sa_family_t family, + int socktype) +{ + struct addrinfo *list_ai, *curr_ai; + struct addrinfo hints; + int ret, socket; + + memset(&hints, 0, sizeof(hints)); + hints.ai_flags = AI_NUMERICHOST | AI_NUMERICSERV | + (socktype == SOCK_STREAM ? 0: AI_V4MAPPED) | + (ctx->laddr ? 0 : AI_PASSIVE); + hints.ai_family = family; + hints.ai_socktype = socktype; + ret = getaddrinfo(ctx->laddr, ctx->lport, &hints, &list_ai); + if (ret) { + fprintf(stderr, "laddr %s, lport %s, getaddrinfo on local address: %s\n", ctx->laddr, ctx->lport, gai_strerror(ret)); + return ret; }

- ctx->socket = s; - ctx->sa_family = family; - return 0; + for (curr_ai = list_ai; curr_ai != NULL; curr_ai = curr_ai->ai_next) { + socket = ovpn_socket(ctx, family, socktype); + if (socket < 0) + continue;

-err_socket: - close(s); - return -1; + ret = bind(socket, curr_ai->ai_addr, curr_ai->ai_addrlen); + if (ret == 0) + break; + + close(socket); + } + + freeaddrinfo(list_ai); + + if (ret < 0) { + perror("cannot setup socket\n"); + return ret; + } + + return socket; }

static int ovpn_udp_socket(struct ovpn_ctx *ctx, sa_family_t family) { - return ovpn_socket(ctx, family, IPPROTO_UDP); + int socket = ovpn_setup_socket(ctx, family, SOCK_DGRAM); + if (socket < 0) + return socket; + + ctx->sa_family = family; + ctx->socket = socket; + return 0; }

static int ovpn_listen(struct ovpn_ctx *ctx, sa_family_t family) { - int ret; + int ret, socket = ovpn_setup_socket(ctx, family, SOCK_STREAM); + if (socket < 0) + return socket;

- ret = ovpn_socket(ctx, family, IPPROTO_TCP); - if (ret < 0) - return ret; - - ret = listen(ctx->socket, 10); + ret = listen(socket, 10); if (ret < 0) { perror("listen"); - close(ctx->socket); + close(socket); return -1; }

+ ctx->sa_family = family; + ctx->socket = socket; return 0; }

@@ -620,18 +629,13 @@ static int ovpn_accept(struct ovpn_ctx *ctx) return ret; }

-static int ovpn_connect(struct ovpn_ctx *ovpn) +static int ovpn_connect(struct ovpn_ctx *ctx) { + const sa_family_t family = ctx->remote.in4.sin_family; socklen_t socklen; - int s, ret; + int ret, socket;

- s = socket(ovpn->remote.in4.sin_family, SOCK_STREAM, 0); - if (s < 0) { - perror("cannot create socket"); - return -1; - } - - switch (ovpn->remote.in4.sin_family) { + switch (family) { case AF_INET: socklen = sizeof(struct sockaddr_in); break; @@ -642,20 +646,22 @@ static int ovpn_connect(struct ovpn_ctx *ovpn) return -EOPNOTSUPP; }

- ret = connect(s, (struct sockaddr *)&ovpn->remote, socklen); + socket = ovpn_setup_socket(ctx, family, SOCK_STREAM); + if (socket < 0) + return socket; + + ret = connect(socket, (struct sockaddr *)&ctx->remote, socklen); if (ret < 0) { perror("connect"); - goto err; + close(socket); + return ret; }

fprintf(stderr, "connected\n");

- ovpn->socket = s; - + ctx->sa_family = family; + ctx->socket = socket; return 0; -err: - close(s); - return ret; }

static int ovpn_new_peer(struct ovpn_ctx *ovpn, bool is_tcp) @@ -1702,7 +1708,7 @@ static void usage(const char *cmd) "\tkey_file: file containing the symmetric key for encryption\n");

fprintf(stderr, - "* new_peer <iface> <dev> <peer_id> <tx_id> <lport> <raddr> <rport> [vpnaddr]: add new peer\n"); + "* new_peer <iface> <dev> <peer_id> <tx_id> <laddr> <lport> <raddr> <rport> [vpnaddr]: add new peer\n"); fprintf(stderr, "\tiface: ovpn interface name\n"); fprintf(stderr, "\tdev: transport interface name to bind to, supports 'any'\n"); @@ -1711,15 +1717,19 @@ static void usage(const char *cmd) "\tpeer_id: peer ID found in data packets received from this peer\n"); fprintf(stderr, "\ttx_id: peer ID to be used when sending to this peer\n"); + fprintf(stderr, + "\tladdr: local UDP address to bind to, supports 'any'\n"); fprintf(stderr, "\traddr: peer IP address\n"); fprintf(stderr, "\trport: peer UDP port\n"); fprintf(stderr, "\tvpnaddr: peer VPN IP\n");

fprintf(stderr, - "* new_multi_peer <iface> <dev> <lport> <peers_file> [mark]: add multiple peers as listed in the file\n"); + "* new_multi_peer <iface> <dev> <laddr> <lport> <peers_file> [mark]: add multiple peers as listed in the file\n"); fprintf(stderr, "\tiface: ovpn interface name\n"); fprintf(stderr, "\tdev: transport interface name to bind to, supports 'any'\n"); + fprintf(stderr, + "\tladdr: local UDP address to bind to, supports 'any'\n"); fprintf(stderr, "\tlport: local UDP port to bind to\n"); fprintf(stderr, "\tpeers_file: text file containing one peer per line. Line format:\n"); @@ -2203,11 +2213,8 @@ static int ovpn_parse_cmd_args(struct ovpn_ctx *ovpn, int argc, char *argv[]) if (argc < 5) return -EINVAL;

- ovpn->lport = strtoul(argv[3], NULL, 10); - if (errno == ERANGE || ovpn->lport > 65535) { - fprintf(stderr, "lport value out of range\n"); - return -1; - } + ovpn->laddr = NULL; + ovpn->lport = argv[3];

ovpn->peers_file = argv[4];

@@ -2221,6 +2228,9 @@ static int ovpn_parse_cmd_args(struct ovpn_ctx *ovpn, int argc, char *argv[])

ovpn->sa_family = AF_INET;

+ ovpn->laddr = NULL; + ovpn->lport = "1"; + ret = ovpn_parse_new_peer(ovpn, argv[3], argv[4], argv[5], argv[6], NULL); if (ret < 0) { @@ -2240,41 +2250,35 @@ static int ovpn_parse_cmd_args(struct ovpn_ctx *ovpn, int argc, char *argv[]) } break; case CMD_NEW_PEER: - if (argc < 8) + if (argc < 9) return -EINVAL;

ovpn->bind_dev = strcmp(argv[3], "any") == 0 ? NULL : argv[3];

- ovpn->lport = strtoul(argv[6], NULL, 10); - if (errno == ERANGE || ovpn->lport > 65535) { - fprintf(stderr, "lport value out of range\n"); - return -1; - } + ovpn->laddr = strcmp(argv[6], "any") == 0 ? NULL : argv[6]; + ovpn->lport = argv[7];

- const char *vpnip = (argc > 9) ? argv[9] : NULL; + const char *vpnip = (argc > 10) ? argv[10] : NULL;

- ret = ovpn_parse_new_peer(ovpn, argv[4], argv[5], argv[7], argv[8], + ret = ovpn_parse_new_peer(ovpn, argv[4], argv[5], argv[8], argv[9], vpnip); if (ret < 0) return -1; break; case CMD_NEW_MULTI_PEER: - if (argc < 6) + if (argc < 7) return -EINVAL;

ovpn->bind_dev = strcmp(argv[3], "any") == 0 ? NULL : argv[3];

- ovpn->lport = strtoul(argv[4], NULL, 10); - if (errno == ERANGE || ovpn->lport > 65535) { - fprintf(stderr, "lport value out of range\n"); - return -1; - } + ovpn->laddr = strcmp(argv[4], "any") == 0 ? NULL : argv[4]; + ovpn->lport = argv[5];

- ovpn->peers_file = argv[5]; + ovpn->peers_file = argv[6];

ovpn->mark = 0; - if (argc > 6) { - ovpn->mark = strtoul(argv[6], NULL, 10); + if (argc > 7) { + ovpn->mark = strtoul(argv[7], NULL, 10); if (errno == ERANGE || ovpn->mark > UINT32_MAX) { fprintf(stderr, "mark value out of range\n"); return -1; diff --git a/tools/testing/selftests/net/ovpn/test-bind-addr.sh b/tools/testing/selftests/net/ovpn/test-bind-addr.sh new file mode 100755 index 000000000000..e33a433ceb4b --- /dev/null +++ b/tools/testing/selftests/net/ovpn/test-bind-addr.sh @@ -0,0 +1,10 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2020-2025 OpenVPN, Inc. +# +# Author: Ralf Lici ralf@mandelbit.com +# Antonio Quartulli antonio@openvpn.net + +BIND_TYPE="ADDR" + +source test-bind.sh diff --git a/tools/testing/selftests/net/ovpn/test-bind.sh b/tools/testing/selftests/net/ovpn/test-bind.sh index fd7c3c8fdf63..b642f396c2a4 100755 --- a/tools/testing/selftests/net/ovpn/test-bind.sh +++ b/tools/testing/selftests/net/ovpn/test-bind.sh @@ -11,6 +11,8 @@ set -e PROTO=UDP source ./common.sh

+BIND_TYPE=${BIND_TYPE:-"DEV"} + cleanup

modprobe -q ovpn || true @@ -61,17 +63,19 @@ run_bind_test() { # close any active socket killall $(basename ${OVPN_CLI}) 2>/dev/null || true

- ip netns exec peer1 ${OVPN_CLI} new_peer tun1 ${dev1} 1 10 1 ${raddr4_peer1} 1 + ip netns exec peer1 ${OVPN_CLI} new_peer tun1 ${dev1} 1 10 ${raddr4_peer2} 1 ${raddr4_peer1} 1 ip netns exec peer1 ${OVPN_CLI} new_key tun1 1 1 0 ${ALG} 0 data64.key - ip netns exec peer2 ${OVPN_CLI} new_peer tun2 ${dev2} 10 1 1 ${raddr4_peer2} 1 + ip netns exec peer2 ${OVPN_CLI} new_peer tun2 ${dev2} 10 1 ${raddr4_peer1} 1 ${raddr4_peer2} 1 ip netns exec peer2 ${OVPN_CLI} new_key tun2 10 1 0 ${ALG} 1 data64.key

ip netns exec peer1 ${OVPN_CLI} set_peer tun1 1 60 120 ip netns exec peer2 ${OVPN_CLI} set_peer tun2 10 60 120

- timeout 2 ip netns exec peer1 tcpdump -i veth1 "${PROTO,,}" port 1 -n -q > /tmp/ovpn-bind1.log & + timeout 2 ip netns exec peer1 tcpdump -i veth1 "${PROTO,,}" and host ${raddr4_peer2} \ + and port 1 -n -q > /tmp/ovpn-bind1.log & tcpdump1_pid=$! - timeout 2 ip netns exec peer1 tcpdump -i veth2 "${PROTO,,}" port 1 -n -q > /tmp/ovpn-bind2.log & + timeout 2 ip netns exec peer1 tcpdump -i veth2 "${PROTO,,}" and host ${raddr4_peer2} \ + and port 1 -n -q > /tmp/ovpn-bind2.log & tcpdump2_pid=$! sleep 0.5

@@ -81,21 +85,31 @@ run_bind_test() { wait ${tcpdump2_pid} || true }

-run_bind_test veth1 any 10.10.10.2 10.10.10.1 -[ "$(grep -c -i udp /tmp/ovpn-bind1.log)" -ge 100 ] -[ "$(grep -c -i udp /tmp/ovpn-bind2.log)" -eq 0 ] - -run_bind_test veth2 any 20.20.20.2 20.20.20.1 -[ "$(grep -c -i udp /tmp/ovpn-bind2.log)" -ge 100 ] -[ "$(grep -c -i udp /tmp/ovpn-bind1.log)" -eq 0 ] - -run_bind_test any veth1 10.10.10.2 10.10.10.1 -[ "$(grep -c -i udp /tmp/ovpn-bind1.log)" -ge 100 ] -[ "$(grep -c -i udp /tmp/ovpn-bind2.log)" -eq 0 ] - -run_bind_test any veth2 20.20.20.2 20.20.20.1 -[ "$(grep -c -i udp /tmp/ovpn-bind2.log)" -ge 100 ] -[ "$(grep -c -i udp /tmp/ovpn-bind1.log)" -eq 0 ] +if [ "${BIND_TYPE}" == "DEV" ]; then + run_bind_test veth1 any 10.10.10.2 10.10.10.1 + [ "$(grep -c -i udp /tmp/ovpn-bind1.log)" -ge 100 ] + [ "$(grep -c -i udp /tmp/ovpn-bind2.log)" -eq 0 ] + + run_bind_test veth2 any 20.20.20.2 20.20.20.1 + [ "$(grep -c -i udp /tmp/ovpn-bind2.log)" -ge 100 ] + [ "$(grep -c -i udp /tmp/ovpn-bind1.log)" -eq 0 ] + + run_bind_test any veth1 10.10.10.2 10.10.10.1 + [ "$(grep -c -i udp /tmp/ovpn-bind1.log)" -ge 100 ] + [ "$(grep -c -i udp /tmp/ovpn-bind2.log)" -eq 0 ] + + run_bind_test any veth2 20.20.20.2 20.20.20.1 + [ "$(grep -c -i udp /tmp/ovpn-bind2.log)" -ge 100 ] + [ "$(grep -c -i udp /tmp/ovpn-bind1.log)" -eq 0 ] +else + run_bind_test any any 10.10.10.2 10.10.10.1 + [ "$(grep -c -i udp /tmp/ovpn-bind1.log)" -ge 100 ] + [ "$(grep -c -i udp /tmp/ovpn-bind2.log)" -eq 0 ] + + run_bind_test any any 20.20.20.2 20.20.20.1 + [ "$(grep -c -i udp /tmp/ovpn-bind2.log)" -ge 100 ] + [ "$(grep -c -i udp /tmp/ovpn-bind1.log)" -eq 0 ] +fi

cleanup

diff --git a/tools/testing/selftests/net/ovpn/test-mark.sh b/tools/testing/selftests/net/ovpn/test-mark.sh index c2600bb22e2c..6f575f7b0635 100755 --- a/tools/testing/selftests/net/ovpn/test-mark.sh +++ b/tools/testing/selftests/net/ovpn/test-mark.sh @@ -26,7 +26,7 @@ for p in $(seq 0 3); do done

# add peer0 with mark -ip netns exec peer0 ${OVPN_CLI} new_multi_peer tun0 any 1 ${UDP_PEERS_FILE} ${MARK} +ip netns exec peer0 ${OVPN_CLI} new_multi_peer tun0 any any 1 ${UDP_PEERS_FILE} ${MARK} for p in $(seq 1 3); do ip netns exec peer0 ${OVPN_CLI} new_key tun0 ${p} 1 0 ${ALG} 0 data64.key done

2025-11-21, 01:20:38 +0100, Antonio Quartulli wrote:

diff --git a/tools/testing/selftests/net/ovpn/common.sh b/tools/testing/selftests/net/ovpn/common.sh index b91cf17ab01f..d926413c9f16 100644 --- a/tools/testing/selftests/net/ovpn/common.sh +++ b/tools/testing/selftests/net/ovpn/common.sh @@ -75,13 +75,14 @@ add_peer() { data64.key done else

• 	RADDR=$(awk "NR == ${1} {print \$2}" ${UDP_PEERS_FILE})
  

• 	RPORT=$(awk "NR == ${1} {print \$3}" ${UDP_PEERS_FILE})
  

• 	LPORT=$(awk "NR == ${1} {print \$5}" ${UDP_PEERS_FILE})
  

• 	ip netns exec peer${1} ${OVPN_CLI} new_peer tun${1} ${1} ${LPORT} \
  

• 		${RADDR} ${RPORT}
  

• 	ip netns exec peer${1} ${OVPN_CLI} new_key tun${1} ${1} 1 0 ${ALG} 1 \
  

• 		data64.key
  

• 	TX_ID=$(awk "NR == ${1} {print \$2}" ${UDP_PEERS_FILE})
  

• 	RADDR=$(awk "NR == ${1} {print \$3}" ${UDP_PEERS_FILE})
  

• 	RPORT=$(awk "NR == ${1} {print \$4}" ${UDP_PEERS_FILE})
  

• 	LPORT=$(awk "NR == ${1} {print \$6}" ${UDP_PEERS_FILE})
  

• 	ip netns exec peer${1} ${OVPN_CLI} new_peer tun${1} ${TX_ID} ${1} \
  

• 		${LPORT} ${RADDR} ${RPORT}
  

• 	ip netns exec peer${1} ${OVPN_CLI} new_key tun${1} ${TX_ID} 1 0 \
  

• 		${ALG} 1 data64.key
  

IIUC, we're creating a "client" with peer_id=$TX_ID and tx_id=$ID so that they're flipped from what we installed on the multi-peer side?

diff --git a/tools/testing/selftests/net/ovpn/ovpn-cli.c b/tools/testing/selftests/net/ovpn/ovpn-cli.c index 064453d16fdd..baabb4c9120e 100644 --- a/tools/testing/selftests/net/ovpn/ovpn-cli.c +++ b/tools/testing/selftests/net/ovpn/ovpn-cli.c @@ -103,7 +103,7 @@ struct ovpn_ctx { sa_family_t sa_family;

• unsigned long peer_id;

• unsigned long peer_id, tx_id; unsigned long lport;

union { @@ -649,6 +649,7 @@ static int ovpn_new_peer(struct ovpn_ctx *ovpn, bool is_tcp) attr = nla_nest_start(ctx->nl_msg, OVPN_A_PEER); NLA_PUT_U32(ctx->nl_msg, OVPN_A_PEER_ID, ovpn->peer_id);

• NLA_PUT_U32(ctx->nl_msg, OVPN_A_PEER_TX_ID, ovpn->tx_id);

So, with these changes, it's no longer possible to test a userspace not passing the new OVPN_A_PEER_TX_ID attribute? But I guess we could simulate that behavior by passing TX_ID==ID (is there still a test doing that?).

Do we need to preserve some testing of the case where userspace is not passing the new attribute?

NLA_PUT_U32(ctx->nl_msg, OVPN_A_PEER_SOCKET, ovpn->socket); if (!is_tcp) { @@ -767,6 +768,10 @@ static int ovpn_handle_peer(struct nl_msg *msg, void (*arg)__always_unused) fprintf(stderr, "* Peer %u\n", nla_get_u32(pattrs[OVPN_A_PEER_ID]));

• if (pattrs[OVPN_A_PEER_TX_ID])

• fprintf(stderr, "\tTX peer ID %u\n",
  

• 	nla_get_u32(pattrs[OVPN_A_PEER_TX_ID]));
  

• if (pattrs[OVPN_A_PEER_SOCKET_NETNSID]) fprintf(stderr, "\tsocket NetNS ID: %d\n", nla_get_s32(pattrs[OVPN_A_PEER_SOCKET_NETNSID]));

@@ -1676,11 +1681,13 @@ static void usage(const char *cmd) "\tkey_file: file containing the symmetric key for encryption\n"); fprintf(stderr,

• "* new_peer <iface> <peer_id> <lport> <raddr> <rport> [vpnaddr]: add new peer\n");
  

• "* new_peer <iface> <peer_id> <tx_id> <lport> <raddr> <rport> [vpnaddr]: add new peer\n");
  

  fprintf(stderr, "\tiface: ovpn interface name\n"); fprintf(stderr, "\tlport: local UDP port to bind to\n"); fprintf(stderr,

• "\tpeer_id: peer ID to be used in data packets to/from this peer\n");
  

• "\tpeer_id: peer ID found in data packets received from this peer\n");
  

• fprintf(stderr,

• "\ttx_id: peer ID to be used when sending to this peer\n");
  

  fprintf(stderr, "\traddr: peer IP address\n"); fprintf(stderr, "\trport: peer UDP port\n"); fprintf(stderr, "\tvpnaddr: peer VPN IP\n");

@@ -1691,7 +1698,7 @@ static void usage(const char *cmd) fprintf(stderr, "\tlport: local UDP port to bind to\n"); fprintf(stderr, "\tpeers_file: text file containing one peer per line. Line format:\n");

• fprintf(stderr, "\t\t<peer_id> <raddr> <rport> <vpnaddr>\n");

• fprintf(stderr, "\t\t<peer_id> <tx_id> <raddr> <rport> <laddr> <lport> <vpnaddr>\n");

Looks like this is missing similar updates for connect and listen, based on changes to ovpn_run_cmd and ovpn_parse_cmd_args?

2025-11-21, 01:20:42 +0100, Antonio Quartulli wrote:

From: Ralf Lici ralf@mandelbit.com

Add a selftest to verify that when a socket is bound to a device, UDP traffic from ovpn is correctly routed through the specified interface.

The test sets up a P2P session between two peers in separate network namespaces, connected via two veth pairs. It binds to both veth interfaces and uses tcpdump to confirm that traffic flows through the expected paths.

The current setup doesn't really test that, since it would also work without SO_BINDTODEVICE (traffic still flows through the expected veth if I pass "any" instead of veth1/veth2 to the new_peer commands).

[...]

diff --git a/tools/testing/selftests/net/ovpn/common.sh b/tools/testing/selftests/net/ovpn/common.sh index d926413c9f16..c802e4e50054 100644 --- a/tools/testing/selftests/net/ovpn/common.sh +++ b/tools/testing/selftests/net/ovpn/common.sh @@ -66,9 +66,11 @@ setup_listener() { } add_peer() {

• dev=${2:-"any"}

nit: no user of add_peer is patched to pass this extra argument

diff --git a/tools/testing/selftests/net/ovpn/test-bind.sh b/tools/testing/selftests/net/ovpn/test-bind.sh new file mode 100755 index 000000000000..fd7c3c8fdf63 --- /dev/null +++ b/tools/testing/selftests/net/ovpn/test-bind.sh @@ -0,0 +1,103 @@

[...]

+run_bind_test() {

• dev1=${1}
• dev2=${2}
• raddr4_peer1=${3}
• raddr4_peer2=${4}
• touch /tmp/ovpn-bind1.log
• touch /tmp/ovpn-bind2.log
• ip netns exec peer1 ${OVPN_CLI} del_peer tun1 1 2>/dev/null || true
• ip netns exec peer2 ${OVPN_CLI} del_peer tun2 10 2>/dev/null || true
• # close any active socket
• killall $(basename ${OVPN_CLI}) 2>/dev/null || true
• ip netns exec peer1 ${OVPN_CLI} new_peer tun1 ${dev1} 1 10 1 ${raddr4_peer1} 1
• ip netns exec peer1 ${OVPN_CLI} new_key tun1 1 1 0 ${ALG} 0 data64.key
• ip netns exec peer2 ${OVPN_CLI} new_peer tun2 ${dev2} 10 1 1 ${raddr4_peer2} 1
• ip netns exec peer2 ${OVPN_CLI} new_key tun2 10 1 0 ${ALG} 1 data64.key
• ip netns exec peer1 ${OVPN_CLI} set_peer tun1 1 60 120
• ip netns exec peer2 ${OVPN_CLI} set_peer tun2 10 60 120
• timeout 2 ip netns exec peer1 tcpdump -i veth1 "${PROTO,,}" port 1 -n -q > /tmp/ovpn-bind1.log &

Maybe add 2> /dev/null to clean up a bit the script output?

• tcpdump1_pid=$!
• timeout 2 ip netns exec peer1 tcpdump -i veth2 "${PROTO,,}" port 1 -n -q > /tmp/ovpn-bind2.log &
• tcpdump2_pid=$!
• sleep 0.5
• ip netns exec peer1 ping -qfc 50 -w 1 5.5.5.2
• wait ${tcpdump1_pid} || true
• wait ${tcpdump2_pid} || true

+}

+run_bind_test veth1 any 10.10.10.2 10.10.10.1 +[ "$(grep -c -i udp /tmp/ovpn-bind1.log)" -ge 100 ] +[ "$(grep -c -i udp /tmp/ovpn-bind2.log)" -eq 0 ]

+run_bind_test veth2 any 20.20.20.2 20.20.20.1 +[ "$(grep -c -i udp /tmp/ovpn-bind2.log)" -ge 100 ] +[ "$(grep -c -i udp /tmp/ovpn-bind1.log)" -eq 0 ]

+run_bind_test any veth1 10.10.10.2 10.10.10.1 +[ "$(grep -c -i udp /tmp/ovpn-bind1.log)" -ge 100 ] +[ "$(grep -c -i udp /tmp/ovpn-bind2.log)" -eq 0 ]

+run_bind_test any veth2 20.20.20.2 20.20.20.1 +[ "$(grep -c -i udp /tmp/ovpn-bind2.log)" -ge 100 ] +[ "$(grep -c -i udp /tmp/ovpn-bind1.log)" -eq 0 ]

+cleanup

And also clean up the log files? (maybe via "trap <function> EXIT" so that they get removed as well if the test fails)

On Thu, 2025-11-27 at 01:13 +0100, Sabrina Dubroca wrote:

2025-11-21, 01:20:38 +0100, Antonio Quartulli wrote:

...

diff --git a/tools/testing/selftests/net/ovpn/common.sh b/tools/testing/selftests/net/ovpn/common.sh index b91cf17ab01f..d926413c9f16 100644 --- a/tools/testing/selftests/net/ovpn/common.sh +++ b/tools/testing/selftests/net/ovpn/common.sh @@ -75,13 +75,14 @@ add_peer() {   data64.key   done   else

• 	RADDR=$(awk "NR == ${1} {print \$2}"
  

${UDP_PEERS_FILE})

• 	RPORT=$(awk "NR == ${1} {print \$3}"
  

${UDP_PEERS_FILE})

• 	LPORT=$(awk "NR == ${1} {print \$5}"
  

${UDP_PEERS_FILE})

• 	ip netns exec peer${1} ${OVPN_CLI} new_peer
  

tun${1} ${1} ${LPORT} \

• 		${RADDR} ${RPORT}
  

• 	ip netns exec peer${1} ${OVPN_CLI} new_key
  

tun${1} ${1} 1 0 ${ALG} 1 \

• 		data64.key
  

• 	TX_ID=$(awk "NR == ${1} {print \$2}"
  

${UDP_PEERS_FILE})

• 	RADDR=$(awk "NR == ${1} {print \$3}"
  

${UDP_PEERS_FILE})

• 	RPORT=$(awk "NR == ${1} {print \$4}"
  

${UDP_PEERS_FILE})

• 	LPORT=$(awk "NR == ${1} {print \$6}"
  

${UDP_PEERS_FILE})

• 	ip netns exec peer${1} ${OVPN_CLI} new_peer
  

tun${1} ${TX_ID} ${1} \

• 		${LPORT} ${RADDR} ${RPORT}
  

• 	ip netns exec peer${1} ${OVPN_CLI} new_key
  

tun${1} ${TX_ID} 1 0 \

• 		${ALG} 1 data64.key
  

IIUC, we're creating a "client" with peer_id=$TX_ID and tx_id=$ID so that they're flipped from what we installed on the multi-peer side?

Exactly. I can see that this part is not very clear so I'll properly define PEER_ID and TX_ID for the clients in the next version of the patch.

...

diff --git a/tools/testing/selftests/net/ovpn/ovpn-cli.c b/tools/testing/selftests/net/ovpn/ovpn-cli.c index 064453d16fdd..baabb4c9120e 100644 --- a/tools/testing/selftests/net/ovpn/ovpn-cli.c +++ b/tools/testing/selftests/net/ovpn/ovpn-cli.c @@ -103,7 +103,7 @@ struct ovpn_ctx {     sa_family_t sa_family;

• unsigned long peer_id;

• unsigned long peer_id, tx_id;

unsigned long lport;     union { @@ -649,6 +649,7 @@ static int ovpn_new_peer(struct ovpn_ctx *ovpn, bool is_tcp)     attr = nla_nest_start(ctx->nl_msg, OVPN_A_PEER);   NLA_PUT_U32(ctx->nl_msg, OVPN_A_PEER_ID, ovpn->peer_id);

• NLA_PUT_U32(ctx->nl_msg, OVPN_A_PEER_TX_ID, ovpn->tx_id);

So, with these changes, it's no longer possible to test a userspace not passing the new OVPN_A_PEER_TX_ID attribute? But I guess we could simulate that behavior by passing TX_ID==ID (is there still a test doing that?).

Do we need to preserve some testing of the case where userspace is not passing the new attribute?

That's a valid concern as we're expected to still correctly handle userspace programs that don't pass the attribute. I'll add a boolean flag similar to FLOAT in order to extend the tests to cover this case.

...

NLA_PUT_U32(ctx->nl_msg, OVPN_A_PEER_SOCKET, ovpn->socket);     if (!is_tcp) { @@ -767,6 +768,10 @@ static int ovpn_handle_peer(struct nl_msg *msg, void (*arg)__always_unused)   fprintf(stderr, "* Peer %u\n",   nla_get_u32(pattrs[OVPN_A_PEER_ID]));

• if (pattrs[OVPN_A_PEER_TX_ID])

• fprintf(stderr, "\tTX peer ID %u\n",
  

• 	nla_get_u32(pattrs[OVPN_A_PEER_TX_ID]));
  

if (pattrs[OVPN_A_PEER_SOCKET_NETNSID])   fprintf(stderr, "\tsocket NetNS ID: %d\n",   nla_get_s32(pattrs[OVPN_A_PEER_SOCKET_NETNS ID])); @@ -1676,11 +1681,13 @@ static void usage(const char *cmd)   "\tkey_file: file containing the symmetric key for encryption\n");     fprintf(stderr,

• "* new_peer <iface> <peer_id> <lport> <raddr>
  

<rport> [vpnaddr]: add new peer\n");

• "* new_peer <iface> <peer_id> <tx_id> <lport>
  

<raddr> <rport> [vpnaddr]: add new peer\n");   fprintf(stderr, "\tiface: ovpn interface name\n");   fprintf(stderr, "\tlport: local UDP port to bind to\n");   fprintf(stderr,

• "\tpeer_id: peer ID to be used in data packets
  

to/from this peer\n");

• "\tpeer_id: peer ID found in data packets received
  

from this peer\n");

• fprintf(stderr,

• "\ttx_id: peer ID to be used when sending to this
  

peer\n");   fprintf(stderr, "\traddr: peer IP address\n");   fprintf(stderr, "\trport: peer UDP port\n");   fprintf(stderr, "\tvpnaddr: peer VPN IP\n"); @@ -1691,7 +1698,7 @@ static void usage(const char *cmd)   fprintf(stderr, "\tlport: local UDP port to bind to\n");   fprintf(stderr,   "\tpeers_file: text file containing one peer per line. Line format:\n");

• fprintf(stderr, "\t\t<peer_id> <raddr> <rport>

<vpnaddr>\n");

• fprintf(stderr, "\t\t<peer_id> <tx_id> <raddr> <rport>

<laddr> <lport> <vpnaddr>\n");

Looks like this is missing similar updates for connect and listen, based on changes to ovpn_run_cmd and ovpn_parse_cmd_args?

Good catch, thanks.

On Thu, 2025-11-27 at 12:29 +0100, Sabrina Dubroca wrote:

2025-11-21, 01:20:42 +0100, Antonio Quartulli wrote:

...

From: Ralf Lici ralf@mandelbit.com

Add a selftest to verify that when a socket is bound to a device, UDP traffic from ovpn is correctly routed through the specified interface.

The test sets up a P2P session between two peers in separate network namespaces, connected via two veth pairs. It binds to both veth interfaces and uses tcpdump to confirm that traffic flows through the expected paths.

The current setup doesn't really test that, since it would also work without SO_BINDTODEVICE (traffic still flows through the expected veth if I pass "any" instead of veth1/veth2 to the new_peer commands).

Thanks for catching this. I see the issue and will try a different approach in the next version of the patch.

[...]

...

diff --git a/tools/testing/selftests/net/ovpn/common.sh b/tools/testing/selftests/net/ovpn/common.sh index d926413c9f16..c802e4e50054 100644 --- a/tools/testing/selftests/net/ovpn/common.sh +++ b/tools/testing/selftests/net/ovpn/common.sh @@ -66,9 +66,11 @@ setup_listener() {  }    add_peer() {

• dev=${2:-"any"}

nit: no user of add_peer is patched to pass this extra argument

Yes, I initially considered using this function for test-bind.sh but later abandoned the idea. I'll remove the argument since it's not needed.

...

diff --git a/tools/testing/selftests/net/ovpn/test-bind.sh b/tools/testing/selftests/net/ovpn/test-bind.sh new file mode 100755 index 000000000000..fd7c3c8fdf63 --- /dev/null +++ b/tools/testing/selftests/net/ovpn/test-bind.sh @@ -0,0 +1,103 @@

[...]

...

+run_bind_test() {

• dev1=${1}
• dev2=${2}
• raddr4_peer1=${3}
• raddr4_peer2=${4}
• touch /tmp/ovpn-bind1.log
• touch /tmp/ovpn-bind2.log
• ip netns exec peer1 ${OVPN_CLI} del_peer tun1 1 2>/dev/null

|| true

• ip netns exec peer2 ${OVPN_CLI} del_peer tun2 10

2>/dev/null || true

• # close any active socket
• killall $(basename ${OVPN_CLI}) 2>/dev/null || true
• ip netns exec peer1 ${OVPN_CLI} new_peer tun1 ${dev1} 1 10

1 ${raddr4_peer1} 1

• ip netns exec peer1 ${OVPN_CLI} new_key tun1 1 1 0 ${ALG} 0

data64.key

• ip netns exec peer2 ${OVPN_CLI} new_peer tun2 ${dev2} 10 1

1 ${raddr4_peer2} 1

• ip netns exec peer2 ${OVPN_CLI} new_key tun2 10 1 0 ${ALG}

1 data64.key

• ip netns exec peer1 ${OVPN_CLI} set_peer tun1 1 60 120
• ip netns exec peer2 ${OVPN_CLI} set_peer tun2 10 60 120
• timeout 2 ip netns exec peer1 tcpdump -i veth1 "${PROTO,,}"

port 1 -n -q > /tmp/ovpn-bind1.log &

Maybe add 2> /dev/null to clean up a bit the script output?

ACK.

...

• tcpdump1_pid=$!
• timeout 2 ip netns exec peer1 tcpdump -i veth2 "${PROTO,,}"

port 1 -n -q > /tmp/ovpn-bind2.log &

• tcpdump2_pid=$!
• sleep 0.5
• ip netns exec peer1 ping -qfc 50 -w 1 5.5.5.2
• wait ${tcpdump1_pid} || true
• wait ${tcpdump2_pid} || true

+}

+run_bind_test veth1 any 10.10.10.2 10.10.10.1 +[ "$(grep -c -i udp /tmp/ovpn-bind1.log)" -ge 100 ] +[ "$(grep -c -i udp /tmp/ovpn-bind2.log)" -eq 0 ]

+run_bind_test veth2 any 20.20.20.2 20.20.20.1 +[ "$(grep -c -i udp /tmp/ovpn-bind2.log)" -ge 100 ] +[ "$(grep -c -i udp /tmp/ovpn-bind1.log)" -eq 0 ]

+run_bind_test any veth1 10.10.10.2 10.10.10.1 +[ "$(grep -c -i udp /tmp/ovpn-bind1.log)" -ge 100 ] +[ "$(grep -c -i udp /tmp/ovpn-bind2.log)" -eq 0 ]

+run_bind_test any veth2 20.20.20.2 20.20.20.1 +[ "$(grep -c -i udp /tmp/ovpn-bind2.log)" -ge 100 ] +[ "$(grep -c -i udp /tmp/ovpn-bind1.log)" -eq 0 ]

+cleanup

And also clean up the log files? (maybe via "trap <function> EXIT" so that they get removed as well if the test fails)

ACK.