* [PATCH v4 nf-next 0/3] ipvs: add per-service secure_tcp
@ 2026-09-20 5:47 Adriano Cordova
2026-09-20 5:47 ` [PATCH v4 nf-next 1/3] ipvs: add flags for per-service secure TCP state table Adriano Cordova
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Adriano Cordova @ 2026-09-20 5:47 UTC (permalink / raw)
To: Simon Horman, Julian Anastasov, Pablo Neira Ayuso
Cc: Florian Westphal, netfilter-devel, lvs-devel, netdev, linux-kernel
IPVS currently exposes secure_tcp as a per-netns sysctl. It switches the
TCP state machine to the hardened tcp_states_dos table.
These patches make it per-service: a virtual service can set
IP_VS_SVC_F_SECURE_TCP which is passed into IP_VS_CONN_F_SECURE_TCP at
connection creation. set_tcp_state() then selects tcp_states_dos for those
connections with IP_VS_CONN_F_SECURE_TCP set and keeps pd->tcp_state_table
(the netns default, including the nomem floor) otherwise.
1. uapi: define the per-service secure_tcp flags and stamp the flag in
ip_vs_bind_dest() (cache-bypass has no dest)
2. honor it in the TCP state machine, resolving the stale FIXME
3. kselftest contrasting a secure vs. a plain service
Changes in v2:
- Let IP_VS_SVC_F_SECURE_TCP be 0x0100, leaving 0x0040/0x0080 for the
scheduler flags. Let IP_VS_CONN_F_SECURE_TCP be (1 << 17) and no longer
in IP_VS_CONN_F_BACKUP_MASK.
- Set flag in ip_vs_bind_dest() instead of at every ip_vs_conn_new()
call site.
- selftest: send probes with TTL=1 instead of an nft drop, and fix
Sashiko comments.
Changes in v3:
- Merge patch 1/4 and 2/4
- Re-send patches (v2 dropped patches 3/4 and 4/4)
Changes in v4:
- selftest: check the return values of setsockopt(IP_HDRINCL), sendto()
and mnl_socket_bind(), and fail do_add() when IPVS does not reply.
- selftest: fix the remaining Sashiko comments: check inet_pton(),
initialize fam and the parsed addresses, and verify the flags attribute
length before copying it.
Adriano Cordova (3):
ipvs: add flags for per-service secure TCP state table
ipvs: tcp: enable per-connection secure_tcp in state machine
selftests: netfilter: ipvs: add per-service secure_tcp test
include/uapi/linux/ip_vs.h | 2 +
net/netfilter/ipvs/ip_vs_conn.c | 4 +
net/netfilter/ipvs/ip_vs_core.c | 3 +
net/netfilter/ipvs/ip_vs_proto_tcp.c | 16 +-
.../testing/selftests/net/netfilter/Makefile | 6 +
.../selftests/net/netfilter/gen_tcp_probe.c | 158 ++++++++
.../net/netfilter/ipvs_secure_tcp.sh | 153 ++++++++
.../net/netfilter/ipvs_secure_tcp_mln.c | 337 ++++++++++++++++++
8 files changed, 672 insertions(+), 7 deletions(-)
create mode 100644 tools/testing/selftests/net/netfilter/gen_tcp_probe.c
create mode 100755 tools/testing/selftests/net/netfilter/ipvs_secure_tcp.sh
create mode 100644 tools/testing/selftests/net/netfilter/ipvs_secure_tcp_mln.c
base-commit: 87b80c2f6b05cad9f0ff9136709c62a0f59923e3
--
2.51.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v4 nf-next 1/3] ipvs: add flags for per-service secure TCP state table 2026-09-20 5:47 [PATCH v4 nf-next 0/3] ipvs: add per-service secure_tcp Adriano Cordova @ 2026-09-20 5:47 ` Adriano Cordova 2026-09-21 5:50 ` netdev-bot+sashiko 2026-09-20 5:47 ` [PATCH v4 nf-next 2/3] ipvs: tcp: enable per-connection secure_tcp in state machine Adriano Cordova 2026-09-20 5:47 ` [PATCH v4 nf-next 3/3] selftests: netfilter: ipvs: add per-service secure_tcp test Adriano Cordova 2 siblings, 1 reply; 7+ messages in thread From: Adriano Cordova @ 2026-09-20 5:47 UTC (permalink / raw) To: Simon Horman, Julian Anastasov, Pablo Neira Ayuso Cc: Florian Westphal, netfilter-devel, lvs-devel, netdev, linux-kernel Add IP_VS_SVC_F_SECURE_TCP to mark a virtual service for the DoS-hardened TCP connection state table, and IP_VS_CONN_F_SECURE_TCP to carry that capability on a connection so the TCP state machine can select the hardened table for it. The service flag is 0x0100: bits 0x0008/0x0010/0x0020 are the scheduler bits, so leave 0x0040/0x0080 free for schedulers. Set IP_VS_CONN_F_SECURE_TCP on every connection bound to a destination whose service carries IP_VS_SVC_F_SECURE_TCP. Signed-off-by: Adriano Cordova <adrianox@gmail.com> --- Changes in v2: - Let IP_VS_SVC_F_SECURE_TCP be 0x0100, leaving 0x0040/0x0080 free for scheduler flags. - Let IP_VS_CONN_F_SECURE_TCP be (1 << 17) and drop it from IP_VS_CONN_F_BACKUP_MASK. - Set the flag in ip_vs_bind_dest() instead of at every ip_vs_conn_new() call site. Changes in v3: - Merge the uapi flag definition and the ip_vs_bind_dest() stamping into a single patch. (no changes since v3) include/uapi/linux/ip_vs.h | 2 ++ net/netfilter/ipvs/ip_vs_conn.c | 4 ++++ net/netfilter/ipvs/ip_vs_core.c | 3 +++ 3 files changed, 9 insertions(+) diff --git a/include/uapi/linux/ip_vs.h b/include/uapi/linux/ip_vs.h index 2c37c6ac7525..ade170109ff4 100644 --- a/include/uapi/linux/ip_vs.h +++ b/include/uapi/linux/ip_vs.h @@ -27,6 +27,7 @@ #define IP_VS_SVC_F_SCHED_SH_FALLBACK IP_VS_SVC_F_SCHED1 /* SH fallback */ #define IP_VS_SVC_F_SCHED_SH_PORT IP_VS_SVC_F_SCHED2 /* SH use port */ +#define IP_VS_SVC_F_SECURE_TCP 0x0100 /* use the hardened TCP table */ /* * IPVS sync daemon states @@ -105,6 +106,7 @@ /* Flags that are not sent to backup server start from bit 16 */ #define IP_VS_CONN_F_NFCT (1 << 16) /* use netfilter conntrack */ +#define IP_VS_CONN_F_SECURE_TCP (1 << 17) /* use the hardened TCP table */ /* Connection flags from destination that can be changed by user space */ #define IP_VS_CONN_F_DEST_MASK (IP_VS_CONN_F_FWD_MASK | \ diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c index 6fa3e1dc534c..0b465298b64b 100644 --- a/net/netfilter/ipvs/ip_vs_conn.c +++ b/net/netfilter/ipvs/ip_vs_conn.c @@ -1090,6 +1090,7 @@ ip_vs_bind_dest(struct ip_vs_conn *cp, struct ip_vs_dest *dest) { unsigned int conn_flags; __u32 flags; + struct ip_vs_service *svc; /* if dest is NULL, then return directly */ if (!dest) @@ -1102,6 +1103,9 @@ ip_vs_bind_dest(struct ip_vs_conn *cp, struct ip_vs_dest *dest) if (cp->protocol != IPPROTO_UDP) conn_flags &= ~IP_VS_CONN_F_ONE_PACKET; flags = cp->flags; + svc = rcu_dereference(dest->svc); + if (svc && (svc->flags & IP_VS_SVC_F_SECURE_TCP)) + flags |= IP_VS_CONN_F_SECURE_TCP; /* Bind with the destination and its corresponding transmitter */ if (flags & IP_VS_CONN_F_SYNC) { /* Synced conns are hashed, so they can not get this flag */ diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c index ba0957798bad..aa8a3a964ffd 100644 --- a/net/netfilter/ipvs/ip_vs_core.c +++ b/net/netfilter/ipvs/ip_vs_core.c @@ -784,6 +784,9 @@ int ip_vs_leave(struct ip_vs_service *svc, struct sk_buff *skb, IP_VS_CONN_F_ONE_PACKET : 0; union nf_inet_addr daddr = { .all = { 0, 0, 0, 0 } }; + if (svc->flags & IP_VS_SVC_F_SECURE_TCP) + flags |= IP_VS_CONN_F_SECURE_TCP; + /* create a new connection entry */ IP_VS_DBG(6, "%s(): create a cache_bypass entry\n", __func__); { -- 2.51.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 nf-next 1/3] ipvs: add flags for per-service secure TCP state table 2026-09-20 5:47 ` [PATCH v4 nf-next 1/3] ipvs: add flags for per-service secure TCP state table Adriano Cordova @ 2026-09-21 5:50 ` netdev-bot+sashiko 0 siblings, 0 replies; 7+ messages in thread From: netdev-bot+sashiko @ 2026-09-21 5:50 UTC (permalink / raw) To: adrianox Cc: horms, ja, pablo, fw, netfilter-devel, lvs-devel, netdev, linux-kernel, kuba Thank you for your contribution! Sashiko AI review found more severe issue(s) in other patches of this series. This mail is only an FYI, in case you want to address the nit picks below when you respin. Low priority / nit picks for your consideration: - [Low] Reported for policy completeness only; my verification concludes this is NOT a defect in the series. Pre-existing issues: - [Low] Reported for policy completeness only; verified as a pre-existing cosmetic discrepancy with no functional effect. -- ipvs: add flags for per-service secure TCP state table This adds the uAPI bits IP_VS_SVC_F_SECURE_TCP (0x0100) and IP_VS_CONN_F_SECURE_TCP (1 << 17), and stamps the connection flag in ip_vs_bind_dest() for connections bound to a destination whose service carries the service flag, plus in the cache_bypass path of ip_vs_leave(). A couple of small observations on the new header bits below. > diff --git a/include/uapi/linux/ip_vs.h b/include/uapi/linux/ip_vs.h > index 2c37c6ac7525..ade170109ff4 100644 > --- a/include/uapi/linux/ip_vs.h > +++ b/include/uapi/linux/ip_vs.h > @@ -27,6 +27,7 @@ > > #define IP_VS_SVC_F_SCHED_SH_FALLBACK IP_VS_SVC_F_SCHED1 /* SH fallback */ > #define IP_VS_SVC_F_SCHED_SH_PORT IP_VS_SVC_F_SCHED2 /* SH use port */ > +#define IP_VS_SVC_F_SECURE_TCP 0x0100 /* use the hardened TCP table */ [Severity: Low] Both new bits are documented as "use the hardened TCP table", but nothing in this commit reads either of them, so in this tree state the bits are accepted from user space and stamped onto connections without changing behaviour. Checking the rest of the series, the consumer does arrive in the following patch, "ipvs: tcp: enable per-connection secure_tcp in state machine", which changes set_tcp_state() in net/netfilter/ipvs/ip_vs_proto_tcp.c to: table = pd->tcp_state_table; if (cp->flags & IP_VS_CONN_F_SECURE_TCP) table = tcp_states_dos; new_state = table[state_off + state_idx].next_state[cp->state]; So by the end of the series the header comment matches the implementation and this looks like a plain producer/consumer split rather than a defect. Noting it only in case the intermediate state matters for bisect, since the bit is settable by CAP_NET_ADMIN through ip_vs_ctl.c before it has any effect. > > /* > * IPVS sync daemon states > @@ -105,6 +106,7 @@ > > /* Flags that are not sent to backup server start from bit 16 */ > #define IP_VS_CONN_F_NFCT (1 << 16) /* use netfilter conntrack */ > +#define IP_VS_CONN_F_SECURE_TCP (1 << 17) /* use the hardened TCP table */ [Severity: Low] This isn't an issue introduced by this patch, but the new bit lands under the comment saying these flags are not sent to the backup server, while the v1 serializer actually puts the whole flags word on the wire. net/netfilter/ipvs/ip_vs_sync.c:ip_vs_sync_conn() s->v4.flags = htonl(cp->flags & ~IP_VS_CONN_F_HASHED); So a connection carrying IP_VS_CONN_F_SECURE_TCP does transmit bit 17. The receiving side drops it again in ip_vs_process_message_v1(): flags = ntohl(s->v4.flags) & IP_VS_CONN_F_BACKUP_MASK; and IP_VS_CONN_F_BACKUP_MASK only contains FWD_MASK, NOOUTPUT, INACTIVE, SEQ_MASK, NO_CPORT and TEMPLATE, so no high bit survives on the backup. IP_VS_CONN_F_NFCT at bit 16 is already transmitted the same way, so the mismatch between the comment and the wire format predates this change; this just adds one more flag to it. Would it be worth adjusting the comment to say the backup masks these bits off on receive rather than that they are not sent? > > /* Connection flags from destination that can be changed by user space */ > #define IP_VS_CONN_F_DEST_MASK (IP_VS_CONN_F_FWD_MASK | \ [ ... ] -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260920054708.988867-1-adrianox%40gmail.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 nf-next 2/3] ipvs: tcp: enable per-connection secure_tcp in state machine 2026-09-20 5:47 [PATCH v4 nf-next 0/3] ipvs: add per-service secure_tcp Adriano Cordova 2026-09-20 5:47 ` [PATCH v4 nf-next 1/3] ipvs: add flags for per-service secure TCP state table Adriano Cordova @ 2026-09-20 5:47 ` Adriano Cordova 2026-09-21 5:50 ` netdev-bot+sashiko 2026-09-20 5:47 ` [PATCH v4 nf-next 3/3] selftests: netfilter: ipvs: add per-service secure_tcp test Adriano Cordova 2 siblings, 1 reply; 7+ messages in thread From: Adriano Cordova @ 2026-09-20 5:47 UTC (permalink / raw) To: Simon Horman, Julian Anastasov, Pablo Neira Ayuso Cc: Florian Westphal, netfilter-devel, lvs-devel, netdev, linux-kernel Let set_tcp_state select tcp_states_dos when the connection carries IP_VS_CONN_F_SECURE_TCP, otherwise keep using the global pd->tcp_state_table (the netns default / or if nomem). Signed-off-by: Adriano Cordova <adrianox@gmail.com> --- (no changes since v1) net/netfilter/ipvs/ip_vs_proto_tcp.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/net/netfilter/ipvs/ip_vs_proto_tcp.c b/net/netfilter/ipvs/ip_vs_proto_tcp.c index fec0e8b47b71..3b9a2c8e9a52 100644 --- a/net/netfilter/ipvs/ip_vs_proto_tcp.c +++ b/net/netfilter/ipvs/ip_vs_proto_tcp.c @@ -451,11 +451,10 @@ static void tcp_timeout_change(struct ip_vs_proto_data *pd, int flags) int on = (flags & 1); /* secure_tcp */ /* - ** FIXME: change secure_tcp to independent sysctl var - ** or make it per-service or per-app because it is valid - ** for most if not for all of the applications. Something - ** like "capabilities" (flags) for each object. - */ + * This remains the netns-wide default / global floor (e.g. when + * memory pressure kicks in). Per-service hardening is now carried + * by IP_VS_CONN_F_SECURE_TCP on each connection (set_tcp_state). + */ pd->tcp_state_table = (on ? tcp_states_dos : tcp_states); } @@ -479,6 +478,7 @@ set_tcp_state(struct ip_vs_proto_data *pd, struct ip_vs_conn *cp, int state_idx; int new_state = IP_VS_TCP_S_CLOSE; int state_off = tcp_state_off[direction]; + const struct tcp_states_t *table; /* * Update state offset to INPUT_ONLY if necessary @@ -496,8 +496,10 @@ set_tcp_state(struct ip_vs_proto_data *pd, struct ip_vs_conn *cp, goto tcp_state_out; } - new_state = - pd->tcp_state_table[state_off+state_idx].next_state[cp->state]; + table = pd->tcp_state_table; + if (cp->flags & IP_VS_CONN_F_SECURE_TCP) + table = tcp_states_dos; + new_state = table[state_off + state_idx].next_state[cp->state]; tcp_state_out: if (new_state != cp->state) { -- 2.51.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 nf-next 2/3] ipvs: tcp: enable per-connection secure_tcp in state machine 2026-09-20 5:47 ` [PATCH v4 nf-next 2/3] ipvs: tcp: enable per-connection secure_tcp in state machine Adriano Cordova @ 2026-09-21 5:50 ` netdev-bot+sashiko 0 siblings, 0 replies; 7+ messages in thread From: netdev-bot+sashiko @ 2026-09-21 5:50 UTC (permalink / raw) To: adrianox Cc: horms, ja, pablo, fw, netfilter-devel, lvs-devel, netdev, linux-kernel, kuba Thank you for your contribution! Sashiko AI review found more severe issue(s) in other patches of this series. This mail is only an FYI, in case you want to address the nit picks below when you respin. Low priority / nit picks for your consideration: - [Low] Documentation/networking/ipvs-sysctl.rst:283-288 documents the hardened ("more complicated") TCP state transition table as reachable… -- ipvs: tcp: enable per-connection secure_tcp in state machine This changes set_tcp_state() so that a connection carrying IP_VS_CONN_F_SECURE_TCP is driven by tcp_states_dos instead of pd->tcp_state_table, leaving the netns-wide table as the fallback. The old FIXME comment in tcp_timeout_change() is replaced with a note describing the sysctl as the netns-wide default. > diff --git a/net/netfilter/ipvs/ip_vs_proto_tcp.c b/net/netfilter/ipvs/ip_vs_proto_tcp.c > index fec0e8b47b71..3b9a2c8e9a52 100644 > --- a/net/netfilter/ipvs/ip_vs_proto_tcp.c > +++ b/net/netfilter/ipvs/ip_vs_proto_tcp.c > @@ -451,11 +451,10 @@ static void tcp_timeout_change(struct ip_vs_proto_data *pd, int flags) > int on = (flags & 1); /* secure_tcp */ > > /* > - ** FIXME: change secure_tcp to independent sysctl var > - ** or make it per-service or per-app because it is valid > - ** for most if not for all of the applications. Something > - ** like "capabilities" (flags) for each object. > - */ > + * This remains the netns-wide default / global floor (e.g. when > + * memory pressure kicks in). Per-service hardening is now carried > + * by IP_VS_CONN_F_SECURE_TCP on each connection (set_tcp_state). > + */ > pd->tcp_state_table = (on ? tcp_states_dos : tcp_states); > } [Severity: Low] Should Documentation/networking/ipvs-sysctl.rst be updated along with this comment change? The only in-tree description of the hardened table currently ties it exclusively to the sysctl: Documentation/networking/ipvs-sysctl.rst: secure_tcp - INTEGER - 0 - disabled (default) The secure_tcp defense is to use a more complicated TCP state transition table. ... The new comment recharacterises that knob as a netns-wide default/floor, but the .rst still reads as if 0 means no connection uses the hardened table. > > @@ -479,6 +478,7 @@ set_tcp_state(struct ip_vs_proto_data *pd, struct ip_vs_conn *cp, > int state_idx; > int new_state = IP_VS_TCP_S_CLOSE; > int state_off = tcp_state_off[direction]; > + const struct tcp_states_t *table; [ ... ] > @@ -496,8 +496,10 @@ set_tcp_state(struct ip_vs_proto_data *pd, struct ip_vs_conn *cp, > goto tcp_state_out; > } > > - new_state = > - pd->tcp_state_table[state_off+state_idx].next_state[cp->state]; > + table = pd->tcp_state_table; > + if (cp->flags & IP_VS_CONN_F_SECURE_TCP) > + table = tcp_states_dos; > + new_state = table[state_off + state_idx].next_state[cp->state]; [Severity: Low] With this override in set_tcp_state(), can an operator still conclude from net.ipv4.vs.secure_tcp = 0 that no connection is using tcp_states_dos? A service carrying the new IP_VS_SVC_F_SECURE_TCP bit (include/uapi/linux/ip_vs.h, settable via IPVS_SVC_ATTR_FLAGS) propagates IP_VS_CONN_F_SECURE_TCP to its connections, and then set_tcp_state() picks tcp_states_dos regardless of the sysctl value, so for example SYN_RECV no longer advances to ESTABLISHED on a bare SYN+ACK. There is no entry under Documentation/ for IP_VS_SVC_F_SECURE_TCP or for this new interaction with secure_tcp. Could a documentation update be added for the new user-settable flag? -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260920054708.988867-1-adrianox%40gmail.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 nf-next 3/3] selftests: netfilter: ipvs: add per-service secure_tcp test 2026-09-20 5:47 [PATCH v4 nf-next 0/3] ipvs: add per-service secure_tcp Adriano Cordova 2026-09-20 5:47 ` [PATCH v4 nf-next 1/3] ipvs: add flags for per-service secure TCP state table Adriano Cordova 2026-09-20 5:47 ` [PATCH v4 nf-next 2/3] ipvs: tcp: enable per-connection secure_tcp in state machine Adriano Cordova @ 2026-09-20 5:47 ` Adriano Cordova 2026-09-21 5:50 ` netdev-bot+sashiko 2 siblings, 1 reply; 7+ messages in thread From: Adriano Cordova @ 2026-09-20 5:47 UTC (permalink / raw) To: Simon Horman, Julian Anastasov, Pablo Neira Ayuso Cc: Florian Westphal, netfilter-devel, lvs-devel, netdev, linux-kernel Two services share a VIP, one carrying IP_VS_SVC_F_SECURE_TCP. A bare SYN+ACK suffices to test the state machine: the normal service reaches ESTABLISHED, but the secure one stays in SYN_RECV. Assisted-by: opencode:deepseek-flash Signed-off-by: Adriano Cordova <adrianox@gmail.com> --- Changes in v2: - send probes with TTL=1 instead of an nft drop, and fix Sashiko comments. Changes in v4: - Check the return value of setsockopt(IP_HDRINCL), sendto() and mnl_socket_bind(), and fail do_add() when IPVS does not reply. - Check inet_pton(), initialize fam and the parsed addresses, and verify the flags attribute length before copying it (Sashiko). .../testing/selftests/net/netfilter/Makefile | 6 + .../selftests/net/netfilter/gen_tcp_probe.c | 158 ++++++++ .../net/netfilter/ipvs_secure_tcp.sh | 153 ++++++++ .../net/netfilter/ipvs_secure_tcp_mln.c | 337 ++++++++++++++++++ 4 files changed, 654 insertions(+) create mode 100644 tools/testing/selftests/net/netfilter/gen_tcp_probe.c create mode 100755 tools/testing/selftests/net/netfilter/ipvs_secure_tcp.sh create mode 100644 tools/testing/selftests/net/netfilter/ipvs_secure_tcp_mln.c diff --git a/tools/testing/selftests/net/netfilter/Makefile b/tools/testing/selftests/net/netfilter/Makefile index df3c20c90f5d..f88bf69cc874 100644 --- a/tools/testing/selftests/net/netfilter/Makefile +++ b/tools/testing/selftests/net/netfilter/Makefile @@ -22,6 +22,7 @@ TEST_PROGS := \ conntrack_tcp_unreplied.sh \ conntrack_vrf.sh \ ipvs.sh \ + ipvs_secure_tcp.sh \ nf_conntrack_packetdrill.sh \ nf_nat_edemux.sh \ nft_audit.sh \ @@ -52,6 +53,8 @@ TEST_GEN_FILES = \ connect_close \ conntrack_dump_flush \ conntrack_reverse_clash \ + gen_tcp_probe \ + ipvs_secure_tcp_mln \ nf_queue \ sctp_collision \ udpclash \ @@ -62,6 +65,9 @@ include ../../lib.mk $(OUTPUT)/nf_queue: CFLAGS += $(MNL_CFLAGS) $(OUTPUT)/nf_queue: LDLIBS += $(MNL_LDLIBS) +$(OUTPUT)/ipvs_secure_tcp_mln: CFLAGS += $(MNL_CFLAGS) +$(OUTPUT)/ipvs_secure_tcp_mln: LDLIBS += $(MNL_LDLIBS) + $(OUTPUT)/conntrack_dump_flush: CFLAGS += $(MNL_CFLAGS) $(OUTPUT)/conntrack_dump_flush: LDLIBS += $(MNL_LDLIBS) $(OUTPUT)/udpclash: LDLIBS += -lpthread diff --git a/tools/testing/selftests/net/netfilter/gen_tcp_probe.c b/tools/testing/selftests/net/netfilter/gen_tcp_probe.c new file mode 100644 index 000000000000..185c3b1146dc --- /dev/null +++ b/tools/testing/selftests/net/netfilter/gen_tcp_probe.c @@ -0,0 +1,158 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Send a TCP SYN then a TCP ACK (no SYN-ACK, no data) to the VIP. + * IPVS's TCP state machine only inspects SYN/FIN/ACK/RST bits, so this + * exercises the INPUT-direction state transition: + * + * SYN: NONE -> SYN_RECV + * ACK: SYN_RECV -> ESTABLISHED (tcp_states, normal) + * SYN_RECV -> SYN_RECV (tcp_states_dos, secure_tcp) + * + * A TTL of 1 keeps the packets from reaching the real server: IPVS updates + * the connection state before forwarding, then the packet expires and an + * ICMP time-exceeded is sent back to us. + * + * Requires CAP_NET_RAW. + */ + +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> +#include <stdint.h> +#include <arpa/inet.h> +#include <sys/socket.h> +#include <netinet/ip.h> +#include <netinet/tcp.h> +#include <linux/if_ether.h> + +static inline uint16_t csump(const void *data, size_t len) +{ + const uint8_t *p = data; + uint32_t sum = 0; + + while (len > 1) { + uint16_t w; + + memcpy(&w, p, sizeof(w)); + sum += w; + p += 2; + len -= 2; + } + if (len) { + uint16_t w = 0; + + memcpy(&w, p, 1); + sum += w; + } + while (sum >> 16) + sum = (sum & 0xffff) + (sum >> 16); + return ~sum; +} + +static int send_seg(int fd, const struct in_addr *sip, uint16_t sport, + const struct in_addr *dip, uint16_t dport, + uint32_t seq, int syn, int ack) +{ + struct { + struct iphdr ip; + struct tcphdr tcp; + } pkt = { }; + struct iphdr *ip = &pkt.ip; + struct tcphdr *tcp = &pkt.tcp; + struct sockaddr_in dst; + + ip->version = 4; + ip->ihl = 5; + ip->tot_len = htons(sizeof(pkt)); + ip->id = htons((uint16_t)(seq & 0xffff)); + ip->ttl = 1; + ip->protocol = IPPROTO_TCP; + ip->saddr = sip->s_addr; + ip->daddr = dip->s_addr; + + tcp->source = sport; + tcp->dest = dport; + tcp->seq = htonl(seq); + tcp->ack_seq = htonl(seq + 1); + tcp->doff = 5; + if (syn) + tcp->syn = 1; + if (ack) + tcp->ack = 1; + tcp->window = htons(1024); + + ip->check = csump(ip, sizeof(struct iphdr)); + /* pseudo header for TCP checksum */ + { + uint8_t ph[12]; + uint8_t tcpbuf[12 + sizeof(struct tcphdr)]; + + memcpy(ph, &ip->saddr, 4); + memcpy(ph + 4, &ip->daddr, 4); + ph[8] = 0; + ph[9] = IPPROTO_TCP; + ph[10] = (sizeof(struct tcphdr) >> 8) & 0xff; + ph[11] = sizeof(struct tcphdr) & 0xff; + + memcpy(tcpbuf, ph, 12); + memcpy(tcpbuf + 12, tcp, sizeof(struct tcphdr)); + tcp->check = csump(tcpbuf, sizeof(tcpbuf)); + } + + memset(&dst, 0, sizeof(dst)); + dst.sin_family = AF_INET; + dst.sin_addr = *dip; + dst.sin_port = dport; + if (sendto(fd, &pkt, sizeof(pkt), 0, (struct sockaddr *)&dst, + sizeof(dst)) < 0) { + perror("sendto"); + return -1; + } + return 0; +} + +int main(int argc, char *argv[]) +{ + struct in_addr sip = { }, dip = { }; + uint16_t sport, dport; + int fd, one = 1; + uint32_t seq = 0x12345678; + + if (argc != 5) { + fprintf(stderr, "usage: %s <src_ip> <src_port> <dst_ip> <dst_port>\n", + argv[0]); + return 2; + } + if (inet_pton(AF_INET, argv[1], &sip) != 1 || + inet_pton(AF_INET, argv[3], &dip) != 1) { + fprintf(stderr, "bad address\n"); + return 2; + } + sport = htons((uint16_t)atoi(argv[2])); + dport = htons((uint16_t)atoi(argv[4])); + + fd = socket(AF_INET, SOCK_RAW, IPPROTO_RAW); + if (fd < 0) { + perror("raw socket"); + return 1; + } + if (setsockopt(fd, IPPROTO_IP, IP_HDRINCL, &one, sizeof(one)) < 0) { + perror("setsockopt"); + close(fd); + return 1; + } + + if (send_seg(fd, &sip, sport, &dip, dport, seq, 1, 0) < 0) { + close(fd); + return 1; + } + usleep(100000); + if (send_seg(fd, &sip, sport, &dip, dport, seq + 1, 0, 1) < 0) { + close(fd); + return 1; + } + + close(fd); + return 0; +} diff --git a/tools/testing/selftests/net/netfilter/ipvs_secure_tcp.sh b/tools/testing/selftests/net/netfilter/ipvs_secure_tcp.sh new file mode 100755 index 000000000000..4d250bcbb2cf --- /dev/null +++ b/tools/testing/selftests/net/netfilter/ipvs_secure_tcp.sh @@ -0,0 +1,153 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# +# Runtime test for per-service secure_tcp (IP_VS_SVC_F_SECURE_TCP). +# +# Sets up the same 3-namespace topology as ipvs.sh +# but checks the TCP state machine, not data forwarding. Two +# identical TCP services are added on the same VIP on different ports, +# one is marked secure_tcp, the other is not. For each a bare SYN is +# followed by a bare ACK (no SYN-ACK / no data). IPVS classifies the +# connection from the flag bits: +# * normal service: SYN -> SYN_RECV, ACK -> ESTABLISHED +# * secure_tcp service: SYN -> SYN_RECV, ACK -> SYN_RECV +# This test checks that this is the case via `ipvsadm -Lnc`. +# +# Requires root, netns, ipvsadm, and the built helpers +# ipvs_secure_tcp_mln and gen_tcp_probe. + +source lib.sh + +ret=0 +readonly vip="207.175.44.110" +readonly gip="10.0.0.1" +readonly dip="172.16.0.1" +readonly rip="172.16.0.2" +readonly cip="10.0.0.2" +readonly sip="10.0.0.3" +readonly port_secure=8081 +readonly port_plain=8080 + +GREEN='\033[0;92m' +RED='\033[0;31m' +NC='\033[0m' + +checktool "ipvsadm -v" "run test without ipvsadm" + +setup() { + setup_ns ns0 ns1 ns2 + + ip link add veth01 netns "${ns0}" type veth peer name veth10 netns "${ns1}" + ip link add veth02 netns "${ns0}" type veth peer name veth20 netns "${ns2}" + ip link add veth12 netns "${ns1}" type veth peer name veth21 netns "${ns2}" + + ip netns exec "${ns0}" ip link set veth01 up + ip netns exec "${ns0}" ip link set veth02 up + ip netns exec "${ns0}" ip link add br0 type bridge + ip netns exec "${ns0}" ip link set veth01 master br0 + ip netns exec "${ns0}" ip link set veth02 master br0 + ip netns exec "${ns0}" ip link set br0 up + ip netns exec "${ns0}" ip addr add "${cip}/24" dev br0 + + ip netns exec "${ns1}" ip link set veth10 up + ip netns exec "${ns1}" ip addr add "${gip}/24" dev veth10 + ip netns exec "${ns1}" ip link set veth12 up + ip netns exec "${ns1}" ip addr add "${dip}/24" dev veth12 + ip netns exec "${ns1}" ip link set lo up + ip netns exec "${ns1}" ip addr add "${vip}/32" dev lo:1 + ip netns exec "${ns1}" sysctl -qw net.ipv4.ip_forward=1 + + ip netns exec "${ns2}" ip link set veth20 up + ip netns exec "${ns2}" ip addr add "${sip}/24" dev veth20 + ip netns exec "${ns2}" ip link set veth21 up + ip netns exec "${ns2}" ip addr add "${rip}/24" dev veth21 + + ip netns exec "${ns2}" ip addr add "${vip}/32" dev lo:1 + + ip netns exec "${ns0}" ip route add "${vip}/32" via "${gip}" dev br0 + + # load ipvs, then the rr scheduler (separate calls: modprobe treats + # the second name as a module parameter, not a second module) + ip netns exec "${ns1}" modprobe ip_vs + ip netns exec "${ns1}" modprobe ip_vs_rr + + sleep 1 +} + +cleanup() { + cleanup_all_ns +} + +# State of the connection to the VIP:port, from `ipvsadm -Lnc`. +# Fields: pro expire state source virtual destination +conn_state() { + local vport=$1 + ip netns exec "${ns1}" ipvsadm -Lnc 2>/dev/null | + awk -v vt="${vip}:${vport}" '$5==vt { print $3; exit }' +} + +assert_state() { + local port=$1 want=$2 + local got + got="$(conn_state "$port")" + echo " vip ${vip}:${port}: state=${got:-?}" + if [ "${got:-}" != "$want" ]; then + echo -e "${RED}FAIL${NC}: vip ${vip}:${port} expected state" \ + "${want}, got ${got:-none}" + ret=1 + fi +} + +test_secure() { + local bin probe + + # Register the two services (secure_tcp on the secure port) + bin="$(pwd)/ipvs_secure_tcp_mln" + probe="$(pwd)/gen_tcp_probe" + ip netns exec "${ns1}" "$bin" add "${vip}" "${port_secure}" secure + ip netns exec "${ns1}" "$bin" add "${vip}" "${port_plain}" plain + + # Add a real server to both services. Use NAT (-m): in DR the conn gets + # IP_VS_CONN_F_NOOUTPUT, which makes the client ACK an INPUT_ONLY event + # and even tcp_states_dos promotes to ESTABLISHED, hiding the difference. + ip netns exec "${ns1}" ipvsadm -a -m -t "${vip}:${port_secure}" -r "${rip}:${port_secure}" + ip netns exec "${ns1}" ipvsadm -a -m -t "${vip}:${port_plain}" -r "${rip}:${port_plain}" + + # verify the flag was actually set + local got + got="$(ip netns exec "${ns1}" "$bin" get "${vip}" "${port_secure}")" + echo " secured service reports: ${got}" + echo "${got}" | grep -q "secure_tcp=1" || + { echo -e "${RED}FAIL${NC}: flag not set"; ret=1; } + got="$(ip netns exec "${ns1}" "$bin" get "${vip}" "${port_plain}")" + echo "${got}" | grep -q "secure_tcp=0" || + { echo -e "${RED}FAIL${NC}: flag unexpectedly set"; ret=1; } + + # The probes carry TTL=1, so IPVS updates the connection state and + # then the packet expires before reaching the real server (which + # stays silent, no RST to interfere with the observation). + + # Push SYN then ACK to each service from the client + ip netns exec "${ns0}" "$probe" "${cip}" 40000 "${vip}" "${port_secure}" + ip netns exec "${ns0}" "$probe" "${cip}" 40001 "${vip}" "${port_plain}" + sleep 1 + + echo "Testing per-service secure_tcp..." + echo " --- connection table (ipvsadm -Lnc) ---" + ip netns exec "${ns1}" ipvsadm -Lnc 2>/dev/null + echo " --- end connection table ---" + assert_state "${port_plain}" ESTABLISHED + assert_state "${port_secure}" SYN_RECV +} + +trap cleanup EXIT + +setup +test_secure + +if [ "$ret" -ne 0 ]; then + echo -e "$(basename $0): ${RED}FAIL${NC}" + exit 1 +fi +echo -e "$(basename $0): ${GREEN}PASS${NC}" +exit 0 diff --git a/tools/testing/selftests/net/netfilter/ipvs_secure_tcp_mln.c b/tools/testing/selftests/net/netfilter/ipvs_secure_tcp_mln.c new file mode 100644 index 000000000000..54c6d860addf --- /dev/null +++ b/tools/testing/selftests/net/netfilter/ipvs_secure_tcp_mln.c @@ -0,0 +1,337 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * libmnl helper to set/query the per-service secure_tcp flag + * (IP_VS_SVC_F_SECURE_TCP), which ipvsadm does not expose. + * + * Usage: + * ipvs_secure_tcp_mln add <vip> <port> <secure|plain> + * Create a TCP virtual service (scheduler "rr") with the flag either + * set or not. Add real servers afterwards with: + * ipvsadm -a -t <vip>:<port> -r <rs>:<port> + * ipvs_secure_tcp_mln get <vip> <port> + * Print "secure_tcp=<0|1>" for the service. + */ + +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <errno.h> +#include <arpa/inet.h> + +#include <linux/netlink.h> +#include <linux/genetlink.h> +#include <linux/ip_vs.h> + +#include <libmnl/libmnl.h> + +/* Fallback in case the kernel's installed uapi header is older */ +#ifndef IP_VS_SVC_F_SECURE_TCP +#define IP_VS_SVC_F_SECURE_TCP 0x0100 +#endif + +/* 16-byte address storage, matching union nf_inet_addr for AF_INET */ +struct inet_addr16 { + uint8_t all[16]; +}; + +/* ---------------- family resolver ---------------- */ +static int ctrl_attr_cb(const struct nlattr *attr, void *data) +{ + const struct nlattr **tb = data; + int type = mnl_attr_get_type(attr); + + if (mnl_attr_type_valid(attr, CTRL_ATTR_MAX) < 0) + return MNL_CB_ERROR; + if (type == CTRL_ATTR_FAMILY_ID) { + if (mnl_attr_validate(attr, MNL_TYPE_U16) < 0) + return MNL_CB_ERROR; + tb[CTRL_ATTR_FAMILY_ID] = attr; + } + return MNL_CB_OK; +} + +static int ctrl_data_cb(const struct nlmsghdr *nlh, void *data) +{ + const struct nlattr *tb[CTRL_ATTR_MAX + 1] = { 0 }; + uint16_t *fam = data; + + if (nlh->nlmsg_type != GENL_ID_CTRL) + return MNL_CB_OK; + mnl_attr_parse(nlh, sizeof(struct genlmsghdr), + (mnl_attr_cb_t)ctrl_attr_cb, tb); + if (tb[CTRL_ATTR_FAMILY_ID]) { + *fam = mnl_attr_get_u16(tb[CTRL_ATTR_FAMILY_ID]); + return MNL_CB_STOP; + } + return MNL_CB_OK; +} + +static int resolve_family(const char *name, uint16_t *fam) +{ + struct mnl_socket *nl; + char buf[MNL_SOCKET_BUFFER_SIZE]; + struct nlmsghdr *nlh; + struct genlmsghdr *genl; + int ret; + + *fam = 0; + nl = mnl_socket_open(NETLINK_GENERIC); + if (!nl) + return -errno; + if (mnl_socket_bind(nl, 0, 0) < 0) { + mnl_socket_close(nl); + return -errno; + } + + nlh = mnl_nlmsg_put_header(buf); + genl = mnl_nlmsg_put_extra_header(nlh, sizeof(struct genlmsghdr)); + genl->cmd = CTRL_CMD_GETFAMILY; + genl->version = 1; + nlh->nlmsg_type = GENL_ID_CTRL; + nlh->nlmsg_flags = NLM_F_REQUEST; + mnl_attr_put_strz(nlh, CTRL_ATTR_FAMILY_NAME, name); + + if (mnl_socket_sendto(nl, nlh, nlh->nlmsg_len) < 0) { + mnl_socket_close(nl); + return -errno; + } + do { + ret = mnl_socket_recvfrom(nl, buf, sizeof(buf)); + if (ret < 0) { + if (errno == EAGAIN) + continue; + mnl_socket_close(nl); + return -errno; + } + ret = mnl_cb_run(buf, ret, 0, mnl_socket_get_portid(nl), + (mnl_cb_t)ctrl_data_cb, fam); + } while (ret > 0 && *fam == 0); + + mnl_socket_close(nl); + return *fam ? 0 : -ENOENT; +} + +/* ---------------- fill service identifying attrs ---------------- */ +static int fill_service(struct nlmsghdr *nlh, const char *vip, + uint16_t port, int full, int secure) +{ + struct inet_addr16 vaddr = { 0 }; + struct nlattr *nest; + struct ip_vs_flags fl; + int af = AF_INET; + + if (inet_pton(af, vip, vaddr.all) != 1) { + fprintf(stderr, "bad VIP %s\n", vip); + return -EINVAL; + } + + nest = mnl_attr_nest_start(nlh, IPVS_CMD_ATTR_SERVICE); + mnl_attr_put_u16(nlh, IPVS_SVC_ATTR_AF, af); + mnl_attr_put_u16(nlh, IPVS_SVC_ATTR_PROTOCOL, IPPROTO_TCP); + mnl_attr_put(nlh, IPVS_SVC_ATTR_ADDR, sizeof(vaddr), &vaddr); + /* port/be16: port is passed in network order from main() */ + mnl_attr_put_u16(nlh, IPVS_SVC_ATTR_PORT, port); + + if (full) { + mnl_attr_put_strz(nlh, IPVS_SVC_ATTR_SCHED_NAME, "rr"); + memset(&fl, 0, sizeof(fl)); + fl.mask = IP_VS_SVC_F_SECURE_TCP; + if (secure) + fl.flags = IP_VS_SVC_F_SECURE_TCP; + mnl_attr_put(nlh, IPVS_SVC_ATTR_FLAGS, sizeof(fl), &fl); + mnl_attr_put_u32(nlh, IPVS_SVC_ATTR_TIMEOUT, 0); + mnl_attr_put_u32(nlh, IPVS_SVC_ATTR_NETMASK, 0xffffffff); + } + mnl_attr_nest_end(nlh, nest); + return 0; +} + +static int send_cmd(struct mnl_socket *nl, struct nlmsghdr *nlh) +{ + if (mnl_socket_sendto(nl, nlh, nlh->nlmsg_len) < 0) { + perror("sendto"); + return -1; + } + return 0; +} + +/* ---------------- get secure flag ---------------- */ +static int svc_attr_cb(const struct nlattr *attr, void *data) +{ + const struct nlattr **tb = data; + int type = mnl_attr_get_type(attr); + + if (mnl_attr_type_valid(attr, IPVS_SVC_ATTR_MAX) < 0) + return MNL_CB_ERROR; + tb[type] = attr; + return MNL_CB_OK; +} + +static int get_cb(const struct nlmsghdr *nlh, void *data) +{ + const struct nlattr *tb[IPVS_SVC_ATTR_MAX + 1] = { 0 }; + struct ip_vs_flags fl; + int *secure = data; + struct nlattr *nest; + + mnl_attr_for_each(nest, nlh, sizeof(struct genlmsghdr)) { + if (mnl_attr_get_type(nest) == IPVS_CMD_ATTR_SERVICE) + mnl_attr_parse_nested(nest, (mnl_attr_cb_t)svc_attr_cb, tb); + } + if (tb[IPVS_SVC_ATTR_FLAGS] && + mnl_attr_get_payload_len(tb[IPVS_SVC_ATTR_FLAGS]) >= sizeof(fl)) { + memcpy(&fl, mnl_attr_get_payload(tb[IPVS_SVC_ATTR_FLAGS]), + sizeof(fl)); + *secure = !!(fl.flags & IP_VS_SVC_F_SECURE_TCP); + } + return MNL_CB_STOP; +} + +static int do_get(uint16_t fam, const char *vip, uint16_t port) +{ + struct mnl_socket *nl; + char buf[MNL_SOCKET_BUFFER_SIZE]; + struct nlmsghdr *nlh; + struct genlmsghdr *genl; + int ret, secure = -1; + + nl = mnl_socket_open(NETLINK_GENERIC); + if (!nl) + return -errno; + if (mnl_socket_bind(nl, 0, 0) < 0) { + mnl_socket_close(nl); + return -errno; + } + nlh = mnl_nlmsg_put_header(buf); + genl = mnl_nlmsg_put_extra_header(nlh, sizeof(struct genlmsghdr)); + genl->cmd = IPVS_CMD_GET_SERVICE; + genl->version = IPVS_GENL_VERSION; + nlh->nlmsg_type = fam; + nlh->nlmsg_flags = NLM_F_REQUEST; + ret = fill_service(nlh, vip, port, 0, 0); + if (ret < 0) { + mnl_socket_close(nl); + return ret; + } + if (send_cmd(nl, nlh) < 0) { + mnl_socket_close(nl); + return -1; + } + + ret = mnl_socket_recvfrom(nl, buf, sizeof(buf)); + while (ret >= 0) { + ret = mnl_cb_run(buf, ret, 0, mnl_socket_get_portid(nl), + (mnl_cb_t)get_cb, &secure); + if (ret <= MNL_CB_STOP || secure >= 0) + break; + ret = mnl_socket_recvfrom(nl, buf, sizeof(buf)); + } + mnl_socket_close(nl); + if (secure < 0) + return -ENOENT; + printf("secure_tcp=%d\n", secure); + return 0; +} + +/* ---------------- add service with flag ---------------- */ +static int do_add(uint16_t fam, const char *vip, uint16_t port, int secure) +{ + struct mnl_socket *nl; + char buf[MNL_SOCKET_BUFFER_SIZE]; + struct nlmsghdr *nlh; + struct genlmsghdr *genl; + int ret; + + /* NLM_F_EXCL: fail if the service already exists */ + nlh = mnl_nlmsg_put_header(buf); + genl = mnl_nlmsg_put_extra_header(nlh, sizeof(struct genlmsghdr)); + genl->cmd = IPVS_CMD_NEW_SERVICE; + genl->version = IPVS_GENL_VERSION; + nlh->nlmsg_type = fam; + nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_EXCL; + ret = fill_service(nlh, vip, port, 1, secure); + if (ret < 0) + return 1; + + nl = mnl_socket_open(NETLINK_GENERIC); + if (!nl) + return 1; + if (mnl_socket_bind(nl, 0, 0) < 0) { + mnl_socket_close(nl); + return 1; + } + if (send_cmd(nl, nlh) < 0) { + mnl_socket_close(nl); + return 1; + } + + /* Read the reply so we can report why a command may have failed */ + for (;;) { + ret = mnl_socket_recvfrom(nl, buf, sizeof(buf)); + if (ret <= 0) { + fprintf(stderr, "no reply from IPVS\n"); + mnl_socket_close(nl); + return 1; + } + ret = mnl_cb_run(buf, ret, 0, mnl_socket_get_portid(nl), + NULL, NULL); + if (ret < 0) { + int e = errno; + + fprintf(stderr, "IPVS netlink error: ret=%d errno=%d (%s)\n", + ret, e, strerror(e)); + mnl_socket_close(nl); + return 1; + } + if (ret <= MNL_CB_STOP) + break; + } + mnl_socket_close(nl); + return 0; +} + +int main(int argc, char *argv[]) +{ + const char *cmd, *vip; + uint16_t fam = 0; + uint16_t port; + int ret, secure = 0; + + if (argc < 4) { + fprintf(stderr, + "usage: %s add <vip> <port> <secure|plain>\n" + " %s get <vip> <port>\n", argv[0], argv[0]); + return 2; + } + cmd = argv[1]; + vip = argv[2]; + port = (uint16_t)atoi(argv[3]); + port = htons(port); + + ret = resolve_family(IPVS_GENL_NAME, &fam); + if (ret) { + fprintf(stderr, "cannot resolve IPVS genl family: %s\n", + strerror(-ret)); + return 1; + } + + if (strcmp(cmd, "add") == 0) { + if (argc < 5) { + fprintf(stderr, "usage: %s add ... <secure|plain>\n", + argv[0]); + return 2; + } + if (strcmp(argv[4], "secure") == 0) { + secure = 1; + } else if (strcmp(argv[4], "plain") != 0) { + fprintf(stderr, "unknown mode %s\n", argv[4]); + return 2; + } + return do_add(fam, vip, port, secure); + } else if (strcmp(cmd, "get") == 0) { + return do_get(fam, vip, port); + } + + fprintf(stderr, "unknown command %s\n", cmd); + return 2; +} -- 2.51.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 nf-next 3/3] selftests: netfilter: ipvs: add per-service secure_tcp test 2026-09-20 5:47 ` [PATCH v4 nf-next 3/3] selftests: netfilter: ipvs: add per-service secure_tcp test Adriano Cordova @ 2026-09-21 5:50 ` netdev-bot+sashiko 0 siblings, 0 replies; 7+ messages in thread From: netdev-bot+sashiko @ 2026-09-21 5:50 UTC (permalink / raw) To: adrianox Cc: horms, ja, pablo, fw, netfilter-devel, lvs-devel, netdev, linux-kernel, kuba Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider. Critical: 0 · High: 0 · Medium: 2 · Low: 1 If you believe none of the issues are legitimate, please reply to each issue of Medium severity or higher and explain why the AI is wrong. If any of them are legitimate and you plan to respin, please let patchwork know by including "pw-bot: cr" as a separate line at the end of your reply (one such reply per series is enough). - [Medium] tools/testing/selftests/net/netfilter/ipvs_secure_tcp.sh has no kernel-side prerequisite check and no $ksft_skip path. - [Medium] The secure-service assertion cannot distinguish 'per-service secure_tcp worked' from 'the ACK probe never arrived'. - [Low] The commit message states "A bare SYN+ACK suffices to test the state machine: the normal service reaches ESTABLISHED, but the secure… -- selftests: netfilter: ipvs: add per-service secure_tcp test This adds a new netfilter selftest that creates two IPVS TCP services on the same VIP, one with IP_VS_SVC_F_SECURE_TCP, and checks the resulting connection states with ipvsadm -Lnc. Two helpers are added: a libmnl program to set/query the per-service flag and a raw-socket probe that sends a bare SYN followed by a bare ACK. > Two services share a VIP, one carrying IP_VS_SVC_F_SECURE_TCP. > A bare SYN+ACK suffices to test the state machine: the normal > service reaches ESTABLISHED, but the secure one stays in SYN_RECV. [Severity: Low] Does the changelog wording match what gen_tcp_probe.c actually sends? main() transmits two separate bare segments, a SYN and then a pure ACK: if (send_seg(fd, &sip, sport, &dip, dport, seq, 1, 0) < 0) { ... usleep(100000); if (send_seg(fd, &sip, sport, &dip, dport, seq + 1, 0, 1) < 0) { A single segment with both bits set would not show anything, since tcp_state_idx() in net/netfilter/ipvs/ip_vs_proto_tcp.c tests rst, then syn, then fin, then ack: if (th->rst) return 3; if (th->syn) return 0; if (th->fin) return 1; if (th->ack) return 2; so SYN|ACK is classified as syn, and the INPUT syn row of both tcp_states and tcp_states_dos maps sNO -> sSR, i.e. no difference between the secure and the plain service. Would "a bare SYN followed by a bare ACK" (the wording already used in the gen_tcp_probe.c header comment) be more accurate here? > diff --git a/tools/testing/selftests/net/netfilter/ipvs_secure_tcp.sh b/tools/testing/selftests/net/netfilter/ipvs_secure_tcp.sh > new file mode 100755 > index 000000000000..4d250bcbb2cf > --- /dev/null > +++ b/tools/testing/selftests/net/netfilter/ipvs_secure_tcp.sh > @@ -0,0 +1,153 @@ [ ... ] > +source lib.sh > + > +ret=0 [ ... ] > +checktool "ipvsadm -v" "run test without ipvsadm" > + > +setup() { > + setup_ns ns0 ns1 ns2 [ ... ] > + # load ipvs, then the rr scheduler (separate calls: modprobe treats > + # the second name as a module parameter, not a second module) > + ip netns exec "${ns1}" modprobe ip_vs > + ip netns exec "${ns1}" modprobe ip_vs_rr [Severity: Medium] What happens on a kernel built without CONFIG_IP_VS or without the rr scheduler? The only pre-flight gate is checktool "ipvsadm -v", which validates the userspace binary only. Both modprobe calls in setup() have their exit status discarded (no set -e, no ||), so a missing ip_vs goes unnoticed: ipvs_secure_tcp_mln add fails, the connection table stays empty, both assert_state() calls set ret=1, and the script exits 1. $ksft_skip is never used anywhere in this script even though lib.sh is sourced. The pre-existing ipvs.sh in the same directory guards this: if [ ! -d $sysipvsnet ]; then if ! modprobe -q ip_vs; then echo "skip: could not run test without ipvs module" exit $ksft_skip fi fi Could the same skip be added here, so unsupported configurations are reported as skipped rather than failed? Note that the helper's get readback cannot be used for feature detection either: ip_vs_genl_parse_service() stores the requested bits without rejecting unknown ones, usvc->flags = (usvc->flags & ~flags.mask) | (flags.flags & flags.mask); so a kernel lacking per-service secure_tcp still echoes secure_tcp=1. > + > + sleep 1 > +} [ ... ] > +assert_state() { > + local port=$1 want=$2 > + local got > + got="$(conn_state "$port")" > + echo " vip ${vip}:${port}: state=${got:-?}" > + if [ "${got:-}" != "$want" ]; then > + echo -e "${RED}FAIL${NC}: vip ${vip}:${port} expected state" \ > + "${want}, got ${got:-none}" > + ret=1 > + fi > +} [ ... ] > + # Push SYN then ACK to each service from the client > + ip netns exec "${ns0}" "$probe" "${cip}" 40000 "${vip}" "${port_secure}" > + ip netns exec "${ns0}" "$probe" "${cip}" 40001 "${vip}" "${port_plain}" > + sleep 1 [ ... ] > + assert_state "${port_plain}" ESTABLISHED > + assert_state "${port_secure}" SYN_RECV > +} [Severity: Medium] Can assert_state "${port_secure}" SYN_RECV pass without the ACK ever being processed? SYN_RECV is exactly the state the first SYN alone produces. In net/netfilter/ipvs/ip_vs_proto_tcp.c the INPUT syn row is sNO -> sSR in both tables, and only the following input ACK differs: /*syn*/ {{sSR, sES, sES, sSR, sSR, sSR, sSR, sSR, sSR, sSR, sSR }}, /* tcp_states */ /*syn*/ {{sSR, sES, sES, sSR, sSR, sSR, sSR, sSR, sSR, sSR, sSA }}, /* tcp_states_dos */ The probe exit status is discarded here: ip netns exec "${ns0}" "$probe" "${cip}" 40000 "${vip}" "${port_secure}" There is no || and no set -e, while gen_tcp_probe.c main() returns 1 when the second send_seg() fails: usleep(100000); if (send_seg(fd, &sip, sport, &dip, dport, seq + 1, 0, 1) < 0) { close(fd); return 1; } So a probe run that dies after the SYN (sendto failure, or the helper being killed during the usleep) leaves the connection in SYN_RECV and the secure-side assertion still succeeds. The plain-service assertion is only a partial control, since it is a separate invocation with a different source port. Would checking the probe exit status, or asserting something that only the processed ACK can produce, make this assertion distinguish the two cases? > + > +trap cleanup EXIT > + > +setup > +test_secure > + > +if [ "$ret" -ne 0 ]; then > + echo -e "$(basename $0): ${RED}FAIL${NC}" > + exit 1 > +fi > +echo -e "$(basename $0): ${GREEN}PASS${NC}" > +exit 0 [ ... ] -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260920054708.988867-1-adrianox%40gmail.com ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-09-21 5:50 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-09-20 5:47 [PATCH v4 nf-next 0/3] ipvs: add per-service secure_tcp Adriano Cordova 2026-09-20 5:47 ` [PATCH v4 nf-next 1/3] ipvs: add flags for per-service secure TCP state table Adriano Cordova 2026-09-21 5:50 ` netdev-bot+sashiko 2026-09-20 5:47 ` [PATCH v4 nf-next 2/3] ipvs: tcp: enable per-connection secure_tcp in state machine Adriano Cordova 2026-09-21 5:50 ` netdev-bot+sashiko 2026-09-20 5:47 ` [PATCH v4 nf-next 3/3] selftests: netfilter: ipvs: add per-service secure_tcp test Adriano Cordova 2026-09-21 5:50 ` netdev-bot+sashiko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®