* nf_nat_icmp_reply_translation dropped icmp redirect packet
@ 2023-10-17 17:09 sun miller
2023-10-17 21:31 ` Florian Westphal
0 siblings, 1 reply; 5+ messages in thread
From: sun miller @ 2023-10-17 17:09 UTC (permalink / raw)
To: netfilter
Hello,
I've noticed an issue during my testing, and I believe I've identified
the root cause in the code. However, I'm not sure if it's a bug or a
deliberate feature.
Here's the testing process:
1. When a request (src: A, dst: B) is sent to the machine (R) for some
reason. R has ip_forward enabled (echo 1 >
/proc/sys/net/ipv4/ip_forward). // A, B, and R are in the same
subnet.
2. When there's no firewall enabled, R sends an ICMP redirect packet to A.
3. When the firewall is enabled, R doesn't send an ICMP redirect
packet to A // tcpdump -i any icmp -nn shows no packet
I've traced the code path as follows:
1. ip_rcv --> nf_hook_slow (PREROUTING) --> ... -->
nf_nat_alloc_null_binding --> nf_nat_setup_info // ct->status = 256
2. ip_rcv_finish --> ip_forward --> ip_rt_send_redirect --> icmp_send
--> icmp_push_reply --> ... --> nf_conntrack_attach // nskb ct->status
= 256
3. then, icmp_push_reply --> ip_push_pending_frames --> ... -->
iptable_nat_ipv4_local_fn (OUTPUT) --> nf_nat_ipv4_fn -->
nf_nat_icmp_reply_translation
Here's the relevant code:
int nf_nat_icmp_reply_translation(...)
{
// ...
inside = (void *)skb->data + hdrlen;
if (inside->icmp.type == ICMP_REDIRECT) {
// ct->status is 256, but IPS_NAT_DONE_MASK is 384
if ((ct->status & IPS_NAT_DONE_MASK) != IPS_NAT_DONE_MASK)
return 0;
// ...
}
unsigned int nf_nat_ipv4_fn(...)
{
// ...
switch (ctinfo) {
case IP_CT_RELATED:
case IP_CT_RELATED_REPLY:
if (ip_hdr(skb)->protocol == IPPROTO_ICMP) {
if (!nf_nat_icmp_reply_translation(skb, ct, ctinfo, ops->hooknum))
return NF_DROP; // <------- DROP
else
return NF_ACCEPT;
}
// ...
}
Because PREOUTING and OUTPUT have the same maniptype
(NF_NAT_MANIP_DST), ct->status can't equal IPS_NAT_DONE_MASK.
I've tested this on multiple kernel versions, including 3.10, 5.15,
and 6.5, and I can reproduce the issue.
Currently, I'm unsure if this is a bug or an intentional feature. If
it's a feature, where can I find reference documentation that explains
this behavior?
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: nf_nat_icmp_reply_translation dropped icmp redirect packet
2023-10-17 17:09 nf_nat_icmp_reply_translation dropped icmp redirect packet sun miller
@ 2023-10-17 21:31 ` Florian Westphal
2023-10-19 8:34 ` sun miller
0 siblings, 1 reply; 5+ messages in thread
From: Florian Westphal @ 2023-10-17 21:31 UTC (permalink / raw)
To: sun miller; +Cc: netfilter
sun miller <sunminlei@gmail.com> wrote:
> I've noticed an issue during my testing, and I believe I've identified
> the root cause in the code. However, I'm not sure if it's a bug or a
> deliberate feature.
deliberate.
> Here's the testing process:
>
> 1. When a request (src: A, dst: B) is sent to the machine (R) for some
> reason. R has ip_forward enabled (echo 1 >
> /proc/sys/net/ipv4/ip_forward). // A, B, and R are in the same
> subnet.
>
> 2. When there's no firewall enabled, R sends an ICMP redirect packet to A.
>
> 3. When the firewall is enabled, R doesn't send an ICMP redirect
> packet to A // tcpdump -i any icmp -nn shows no packet
>
> I've traced the code path as follows:
>
> 1. ip_rcv --> nf_hook_slow (PREROUTING) --> ... -->
> nf_nat_alloc_null_binding --> nf_nat_setup_info // ct->status = 256
> 2. ip_rcv_finish --> ip_forward --> ip_rt_send_redirect --> icmp_send
> --> icmp_push_reply --> ... --> nf_conntrack_attach // nskb ct->status
> = 256
> 3. then, icmp_push_reply --> ip_push_pending_frames --> ... -->
> iptable_nat_ipv4_local_fn (OUTPUT) --> nf_nat_ipv4_fn -->
> nf_nat_icmp_reply_translation
>
> Here's the relevant code:
>
>
> int nf_nat_icmp_reply_translation(...)
> {
> // ...
> inside = (void *)skb->data + hdrlen;
> if (inside->icmp.type == ICMP_REDIRECT) {
> // ct->status is 256, but IPS_NAT_DONE_MASK is 384
> if ((ct->status & IPS_NAT_DONE_MASK) != IPS_NAT_DONE_MASK)
> return 0;
> // ...
> }
>
> unsigned int nf_nat_ipv4_fn(...)
> {
> // ...
> switch (ctinfo) {
> case IP_CT_RELATED:
> case IP_CT_RELATED_REPLY:
> if (ip_hdr(skb)->protocol == IPPROTO_ICMP) {
> if (!nf_nat_icmp_reply_translation(skb, ct, ctinfo, ops->hooknum))
> return NF_DROP; // <------- DROP
> else
> return NF_ACCEPT;
> }
> // ...
> }
>
>
> Because PREOUTING and OUTPUT have the same maniptype
> (NF_NAT_MANIP_DST), ct->status can't equal IPS_NAT_DONE_MASK.
Yes.
> I've tested this on multiple kernel versions, including 3.10, 5.15,
> and 6.5, and I can reproduce the issue.
>
> Currently, I'm unsure if this is a bug or an intentional feature. If
> it's a feature, where can I find reference documentation that explains
> this behavior?
For some reason the comment that explains this got dropped when
code was moved around. The original comment was:
/* Redirects on non-null nats must be dropped, else they'll
start talking to each other without our translation, and be
confused... --RR */
... which is exactly what this does, it drops the redirect
if it can't verify that no nat is in place in either direction.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: nf_nat_icmp_reply_translation dropped icmp redirect packet
2023-10-17 21:31 ` Florian Westphal
@ 2023-10-19 8:34 ` sun miller
2023-10-19 8:47 ` Florian Westphal
0 siblings, 1 reply; 5+ messages in thread
From: sun miller @ 2023-10-19 8:34 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter
I'm sorry, I forgot to cc netfilter in my previous reply, so I'm
sending it again. I apologize for my oversight. I hope I didn't
disturb you.
> For some reason the comment that explains this got dropped when
> code was moved around. The original comment was:
>
> /* Redirects on non-null nats must be dropped, else they'll
> start talking to each other without our translation, and be
> confused... --RR */
>
> ... which is exactly what this does, it drops the redirect
> if it can't verify that no nat is in place in either direction.
Thank you for your reply and explanation.
I searched globally for ip_rt_send_redirect and icmp_send(skb,
ICMP_REDIRECT, ICMP_REDIR_HOST...), and it seems that the related code
paths do not meet ct->status & IPS_NAT_DONE_MASK == IPS_NAT_DONE_MASK.
Does this mean that icmp_redirect caused by the same subnet request
packet will be DROPPED when the firewall is enabled? Does this break
the correct functionality that the kernel should have originally? (Of
course, this functionality seems to be unimportant from my limited
work experience.)
Additionally, if it's convenient for you, would you provide an example
scenario for the code comment that mentions "they'll start talking to
each other without our translation, and be confused"? I am unable to
think of any related issues.
I hope I'm not bothering you.
Looking forward to your reply. Best regards.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: nf_nat_icmp_reply_translation dropped icmp redirect packet
2023-10-19 8:34 ` sun miller
@ 2023-10-19 8:47 ` Florian Westphal
2023-10-20 14:02 ` sun miller
0 siblings, 1 reply; 5+ messages in thread
From: Florian Westphal @ 2023-10-19 8:47 UTC (permalink / raw)
To: sun miller; +Cc: Florian Westphal, netfilter
sun miller <sunminlei@gmail.com> wrote:
> I'm sorry, I forgot to cc netfilter in my previous reply, so I'm
> sending it again. I apologize for my oversight. I hope I didn't
> disturb you.
>
> > For some reason the comment that explains this got dropped when
> > code was moved around. The original comment was:
> >
> > /* Redirects on non-null nats must be dropped, else they'll
> > start talking to each other without our translation, and be
> > confused... --RR */
> >
> > ... which is exactly what this does, it drops the redirect
> > if it can't verify that no nat is in place in either direction.
>
> Thank you for your reply and explanation.
>
> I searched globally for ip_rt_send_redirect and icmp_send(skb,
> ICMP_REDIRECT, ICMP_REDIR_HOST...), and it seems that the related code
> paths do not meet ct->status & IPS_NAT_DONE_MASK == IPS_NAT_DONE_MASK.
>
> Does this mean that icmp_redirect caused by the same subnet request
> packet will be DROPPED when the firewall is enabled? Does this break
> the correct functionality that the kernel should have originally? (Of
> course, this functionality seems to be unimportant from my limited
> work experience.)
>
> Additionally, if it's convenient for you, would you provide an example
> scenario for the code comment that mentions "they'll start talking to
> each other without our translation, and be confused"? I am unable to
> think of any related issues.
A -> B, A sends packet to R. R has a DNAT rule to redirect to C
(or redirect to R).
If we let icmp redirect through, this won't work.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: nf_nat_icmp_reply_translation dropped icmp redirect packet
2023-10-19 8:47 ` Florian Westphal
@ 2023-10-20 14:02 ` sun miller
0 siblings, 0 replies; 5+ messages in thread
From: sun miller @ 2023-10-20 14:02 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter
> A -> B, A sends packet to R. R has a DNAT rule to redirect to C
> (or redirect to R).
>
> If we let icmp redirect through, this won't work.
Thank you very much for your response.
I had previously encountered an issue with the correctness of the
ip_rt_send_redirect function when the firewall was disabled. I
overlooked the fact that the packet goes through DNAT in the
PREROUTING chain before entering the ip_rt_send_redirect function. In
my test environment, there were no DNAT policies, so I overlooked this
scenario.
Indeed, with the firewall enabled, DNAT rules will modify the
destination IP address, and sending ICMP redirects in such cases can
cause confusion. However, when the firewall is disabled, both SRC and
DST remain unchanged, so the kernel can safely send ICMP redirects.
Once again, thank you for your response.
Best regards.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-10-20 14:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-17 17:09 nf_nat_icmp_reply_translation dropped icmp redirect packet sun miller
2023-10-17 21:31 ` Florian Westphal
2023-10-19 8:34 ` sun miller
2023-10-19 8:47 ` Florian Westphal
2023-10-20 14:02 ` sun miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox