| Message ID | 20221211101204.1751-6-pablo@netfilter.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 7d7cfb48d81353e826493d24c7cec7360950968f |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,01/12] netfilter: nft_inner: fix IS_ERR() vs NULL check | expand |
在 2022/12/11 18:11, Pablo Neira Ayuso 写道: > From: Florian Westphal <fw@strlen.de> > > icmp conntrack will set icmp redirects as RELATED, but icmpv6 will not > do this. > > For icmpv6, only icmp errors (code <= 128) are examined for RELATED state. > ICMPV6 Redirects are part of neighbour discovery mechanism, those are > handled by marking a selected subset (e.g. neighbour solicitations) as > UNTRACKED, but not REDIRECT -- they will thus be flagged as INVALID. > > Add minimal support for REDIRECTs. No parsing of neighbour options is > added for simplicity, so this will only check that we have the embeeded > original header (ND_OPT_REDIRECT_HDR), and then attempt to do a flow > lookup for this tuple. > > Also extend the existing test case to cover redirects. > > Fixes: 9fb9cbb1082d ("[NETFILTER]: Add nf_conntrack subsystem.") > Reported-by: Eric Garver <eric@garver.life> > Link: https://github.com/firewalld/firewalld/issues/1046 > Signed-off-by: Florian Westphal <fw@strlen.de> > Acked-by: Eric Garver <eric@garver.life> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > --- > net/netfilter/nf_conntrack_proto_icmpv6.c | 53 +++++++++++++++++++ > .../netfilter/conntrack_icmp_related.sh | 36 ++++++++++++- > 2 files changed, 87 insertions(+), 2 deletions(-) Hi, Florian. The new ipv4 redirects test case doesn't seem to work, is there a problem with my testing steps? # sh tools/testing/selftests/netfilter/conntrack_icmp_related.sh PASS: icmp mtu error had RELATED state ERROR: counter redir4 in nsclient1 has unexpected value (expected packets 1 bytes 112) table inet filter { counter redir4 { packets 0 bytes 0 } } ERROR: icmp redirect RELATED state test has failed. The test is based on commit f6feea56f66d ("Merge tag 'mm-hotfixes-stable-2023-02-13-13-50' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm")
在 2023/2/14 16:19, Wang Hai 写道: > > 在 2022/12/11 18:11, Pablo Neira Ayuso 写道: >> From: Florian Westphal <fw@strlen.de> >> >> icmp conntrack will set icmp redirects as RELATED, but icmpv6 will not >> do this. >> >> For icmpv6, only icmp errors (code <= 128) are examined for RELATED >> state. >> ICMPV6 Redirects are part of neighbour discovery mechanism, those are >> handled by marking a selected subset (e.g. neighbour solicitations) as >> UNTRACKED, but not REDIRECT -- they will thus be flagged as INVALID. >> >> Add minimal support for REDIRECTs. No parsing of neighbour options is >> added for simplicity, so this will only check that we have the embeeded >> original header (ND_OPT_REDIRECT_HDR), and then attempt to do a flow >> lookup for this tuple. >> >> Also extend the existing test case to cover redirects. >> >> Fixes: 9fb9cbb1082d ("[NETFILTER]: Add nf_conntrack subsystem.") >> Reported-by: Eric Garver <eric@garver.life> >> Link: https://github.com/firewalld/firewalld/issues/1046 >> Signed-off-by: Florian Westphal <fw@strlen.de> >> Acked-by: Eric Garver <eric@garver.life> >> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> >> --- >> net/netfilter/nf_conntrack_proto_icmpv6.c | 53 +++++++++++++++++++ >> .../netfilter/conntrack_icmp_related.sh | 36 ++++++++++++- >> 2 files changed, 87 insertions(+), 2 deletions(-) > Hi, Florian. > > The new ipv4 redirects test case doesn't seem to work, is there a > problem with my testing steps? > > # sh tools/testing/selftests/netfilter/conntrack_icmp_related.sh > PASS: icmp mtu error had RELATED state > ERROR: counter redir4 in nsclient1 has unexpected value (expected > packets 1 bytes 112) > table inet filter { > counter redir4 { > packets 0 bytes 0 > } > } > ERROR: icmp redirect RELATED state test has failed. > > The test is based on commit f6feea56f66d ("Merge tag > 'mm-hotfixes-stable-2023-02-13-13-50' of > git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm") > Hi, Florian. I found the reason why it failed. This needs to be configured on the host with net.ipv4.conf.default.send_redirects=1. Sorry to bother you.
diff --git a/net/netfilter/nf_conntrack_proto_icmpv6.c b/net/netfilter/nf_conntrack_proto_icmpv6.c index 61e3b05cf02c..1020d67600a9 100644 --- a/net/netfilter/nf_conntrack_proto_icmpv6.c +++ b/net/netfilter/nf_conntrack_proto_icmpv6.c @@ -129,6 +129,56 @@ static void icmpv6_error_log(const struct sk_buff *skb, nf_l4proto_log_invalid(skb, state, IPPROTO_ICMPV6, "%s", msg); } +static noinline_for_stack int +nf_conntrack_icmpv6_redirect(struct nf_conn *tmpl, struct sk_buff *skb, + unsigned int dataoff, + const struct nf_hook_state *state) +{ + u8 hl = ipv6_hdr(skb)->hop_limit; + union nf_inet_addr outer_daddr; + union { + struct nd_opt_hdr nd_opt; + struct rd_msg rd_msg; + } tmp; + const struct nd_opt_hdr *nd_opt; + const struct rd_msg *rd_msg; + + rd_msg = skb_header_pointer(skb, dataoff, sizeof(*rd_msg), &tmp.rd_msg); + if (!rd_msg) { + icmpv6_error_log(skb, state, "short redirect"); + return -NF_ACCEPT; + } + + if (rd_msg->icmph.icmp6_code != 0) + return NF_ACCEPT; + + if (hl != 255 || !(ipv6_addr_type(&ipv6_hdr(skb)->saddr) & IPV6_ADDR_LINKLOCAL)) { + icmpv6_error_log(skb, state, "invalid saddr or hoplimit for redirect"); + return -NF_ACCEPT; + } + + dataoff += sizeof(*rd_msg); + + /* warning: rd_msg no longer usable after this call */ + nd_opt = skb_header_pointer(skb, dataoff, sizeof(*nd_opt), &tmp.nd_opt); + if (!nd_opt || nd_opt->nd_opt_len == 0) { + icmpv6_error_log(skb, state, "redirect without options"); + return -NF_ACCEPT; + } + + /* We could call ndisc_parse_options(), but it would need + * skb_linearize() and a bit more work. + */ + if (nd_opt->nd_opt_type != ND_OPT_REDIRECT_HDR) + return NF_ACCEPT; + + memcpy(&outer_daddr.ip6, &ipv6_hdr(skb)->daddr, + sizeof(outer_daddr.ip6)); + dataoff += 8; + return nf_conntrack_inet_error(tmpl, skb, dataoff, state, + IPPROTO_ICMPV6, &outer_daddr); +} + int nf_conntrack_icmpv6_error(struct nf_conn *tmpl, struct sk_buff *skb, unsigned int dataoff, @@ -159,6 +209,9 @@ int nf_conntrack_icmpv6_error(struct nf_conn *tmpl, return NF_ACCEPT; } + if (icmp6h->icmp6_type == NDISC_REDIRECT) + return nf_conntrack_icmpv6_redirect(tmpl, skb, dataoff, state); + /* is not error message ? */ if (icmp6h->icmp6_type >= 128) return NF_ACCEPT; diff --git a/tools/testing/selftests/netfilter/conntrack_icmp_related.sh b/tools/testing/selftests/netfilter/conntrack_icmp_related.sh index b48e1833bc89..76645aaf2b58 100755 --- a/tools/testing/selftests/netfilter/conntrack_icmp_related.sh +++ b/tools/testing/selftests/netfilter/conntrack_icmp_related.sh @@ -35,6 +35,8 @@ cleanup() { for i in 1 2;do ip netns del nsrouter$i;done } +trap cleanup EXIT + ipv4() { echo -n 192.168.$1.2 } @@ -146,11 +148,17 @@ ip netns exec nsclient1 nft -f - <<EOF table inet filter { counter unknown { } counter related { } + counter redir4 { } + counter redir6 { } chain input { type filter hook input priority 0; policy accept; - meta l4proto { icmp, icmpv6 } ct state established,untracked accept + icmp type "redirect" ct state "related" counter name "redir4" accept + icmpv6 type "nd-redirect" ct state "related" counter name "redir6" accept + + meta l4proto { icmp, icmpv6 } ct state established,untracked accept meta l4proto { icmp, icmpv6 } ct state "related" counter name "related" accept + counter name "unknown" drop } } @@ -279,5 +287,29 @@ else echo "ERROR: icmp error RELATED state test has failed" fi -cleanup +# add 'bad' route, expect icmp REDIRECT to be generated +ip netns exec nsclient1 ip route add 192.168.1.42 via 192.168.1.1 +ip netns exec nsclient1 ip route add dead:1::42 via dead:1::1 + +ip netns exec "nsclient1" ping -q -c 2 192.168.1.42 > /dev/null + +expect="packets 1 bytes 112" +check_counter nsclient1 "redir4" "$expect" +if [ $? -ne 0 ];then + ret=1 +fi + +ip netns exec "nsclient1" ping -c 1 dead:1::42 > /dev/null +expect="packets 1 bytes 192" +check_counter nsclient1 "redir6" "$expect" +if [ $? -ne 0 ];then + ret=1 +fi + +if [ $ret -eq 0 ];then + echo "PASS: icmp redirects had RELATED state" +else + echo "ERROR: icmp redirect RELATED state test has failed" +fi + exit $ret