All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH nf] netfilter REJECT: Fix destination MAC in RST packets
@ 2021-03-08  1:16 Marc Aurèle La France
  2021-03-08 10:25 ` Pablo Neira Ayuso
  0 siblings, 1 reply; 9+ messages in thread
From: Marc Aurèle La France @ 2021-03-08  1:16 UTC (permalink / raw)
  To: netfilter-devel

[-- Attachment #1: Type: text/plain, Size: 4064 bytes --]

In the non-bridge case, the REJECT target code assumes the REJECTed
packets were originally emitted by the local host, but that's not
necessarily true when the local host is the default route of a subnet
it is on, resulting in RST packets being sent out with an incorrect
destination MAC.  Address this by refactoring the handling of bridged
packets which deals with a similar issue.  Modulo patch fuzz, the
following applies to v5 and later kernels.

Please Reply-To-All.

Thanks.

Marc.

Signed-off-by: Marc Aurèle La France <tsi@tuyoix.net>
Tested-by: Marc Aurèle La France <tsi@tuyoix.net>

--- a/net/ipv4/netfilter/nf_reject_ipv4.c
+++ b/net/ipv4/netfilter/nf_reject_ipv4.c
@@ -237,7 +237,7 @@ static int nf_reject_fill_skb_dst(struct sk_buff *skb_in)
 void nf_send_reset(struct net *net, struct sock *sk, struct sk_buff *oldskb,
 		   int hook)
 {
-	struct net_device *br_indev __maybe_unused;
+	struct net_device *indev;
 	struct sk_buff *nskb;
 	struct iphdr *niph;
 	const struct tcphdr *oth;
@@ -279,18 +279,20 @@ void nf_send_reset(struct net *net, struct sock *sk, struct sk_buff *oldskb,

 	nf_ct_attach(nskb, oldskb);

-#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
-	/* If we use ip_local_out for bridged traffic, the MAC source on
-	 * the RST will be ours, instead of the destination's.  This confuses
-	 * some routers/firewalls, and they drop the packet.  So we need to
-	 * build the eth header using the original destination's MAC as the
-	 * source, and send the RST packet directly.
+	/* Swap the source and destination MACs of the RST packet from that
+	 * of the REJECTed packet's, if available from it.  Otherwise, let
+	 * ip_local_out decide.
 	 */
-	br_indev = nf_bridge_get_physindev(oldskb);
-	if (br_indev) {
+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
+	indev = nf_bridge_get_physindev(oldskb);
+	if (!indev)
+#endif
+		indev = oldskb->dev;
+
+	if (indev) {
 		struct ethhdr *oeth = eth_hdr(oldskb);

-		nskb->dev = br_indev;
+		nskb->dev = indev;
 		niph->tot_len = htons(nskb->len);
 		ip_send_check(niph);
 		if (dev_hard_header(nskb, nskb->dev, ntohs(nskb->protocol),
@@ -298,7 +300,6 @@ void nf_send_reset(struct net *net, struct sock *sk, struct sk_buff *oldskb,
 			goto free_nskb;
 		dev_queue_xmit(nskb);
 	} else
-#endif
 		ip_local_out(net, nskb->sk, nskb);

 	return;
--- a/net/ipv6/netfilter/nf_reject_ipv6.c
+++ b/net/ipv6/netfilter/nf_reject_ipv6.c
@@ -278,7 +278,7 @@ static int nf_reject6_fill_skb_dst(struct sk_buff *skb_in)
 void nf_send_reset6(struct net *net, struct sock *sk, struct sk_buff *oldskb,
 		    int hook)
 {
-	struct net_device *br_indev __maybe_unused;
+	struct net_device *indev;
 	struct sk_buff *nskb;
 	struct tcphdr _otcph;
 	const struct tcphdr *otcph;
@@ -346,18 +346,20 @@ void nf_send_reset6(struct net *net, struct sock *sk, struct sk_buff *oldskb,

 	nf_ct_attach(nskb, oldskb);

-#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
-	/* If we use ip6_local_out for bridged traffic, the MAC source on
-	 * the RST will be ours, instead of the destination's.  This confuses
-	 * some routers/firewalls, and they drop the packet.  So we need to
-	 * build the eth header using the original destination's MAC as the
-	 * source, and send the RST packet directly.
+	/* Swap the source and destination MACs of the RST packet from that
+	 * of the REJECTed packet's, if available from it.  Otherwise, let
+	 * ip6_local_out decide.
 	 */
-	br_indev = nf_bridge_get_physindev(oldskb);
-	if (br_indev) {
+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
+	indev = nf_bridge_get_physindev(oldskb);
+	if (!indev)
+#endif
+		indev = oldskb->dev;
+
+	if (indev) {
 		struct ethhdr *oeth = eth_hdr(oldskb);

-		nskb->dev = br_indev;
+		nskb->dev = indev;
 		nskb->protocol = htons(ETH_P_IPV6);
 		ip6h->payload_len = htons(sizeof(struct tcphdr));
 		if (dev_hard_header(nskb, nskb->dev, ntohs(nskb->protocol),
@@ -367,7 +369,6 @@ void nf_send_reset6(struct net *net, struct sock *sk, struct sk_buff *oldskb,
 		}
 		dev_queue_xmit(nskb);
 	} else
-#endif
 		ip6_local_out(net, sk, nskb);
 }
 EXPORT_SYMBOL_GPL(nf_send_reset6);

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH nf] netfilter REJECT: Fix destination MAC in RST packets
  2021-03-08  1:16 [PATCH nf] netfilter REJECT: Fix destination MAC in RST packets Marc Aurèle La France
@ 2021-03-08 10:25 ` Pablo Neira Ayuso
  2021-03-08 16:21   ` Marc Aurèle La France
  0 siblings, 1 reply; 9+ messages in thread
From: Pablo Neira Ayuso @ 2021-03-08 10:25 UTC (permalink / raw)
  To: Marc Aurèle La France; +Cc: netfilter-devel

On Sun, Mar 07, 2021 at 06:16:34PM -0700, Marc Aurèle La France wrote:
> In the non-bridge case, the REJECT target code assumes the REJECTed
> packets were originally emitted by the local host, but that's not
> necessarily true when the local host is the default route of a subnet
> it is on, resulting in RST packets being sent out with an incorrect
> destination MAC.  Address this by refactoring the handling of bridged
> packets which deals with a similar issue.  Modulo patch fuzz, the
> following applies to v5 and later kernels.

The code this patch updates is related to BRIDGE_NETFILTER. Your patch
description refers to the non-bridge case. What are you trying to
achieve?

dev_queue_xmit() path should not be exercised from the prerouting
chain, packets generated from the IP later must follow the
ip_local_out() path.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH nf] netfilter REJECT: Fix destination MAC in RST packets
  2021-03-08 10:25 ` Pablo Neira Ayuso
@ 2021-03-08 16:21   ` Marc Aurèle La France
  2021-03-09  1:36     ` Pablo Neira Ayuso
  0 siblings, 1 reply; 9+ messages in thread
From: Marc Aurèle La France @ 2021-03-08 16:21 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

[-- Attachment #1: Type: text/plain, Size: 1485 bytes --]

On Mon, 8 Mar 2021, Pablo Neira Ayuso wrote:
> On Sun, Mar 07, 2021 at 06:16:34PM -0700, Marc Aurèle La France wrote:
>> In the non-bridge case, the REJECT target code assumes the REJECTed
>> packets were originally emitted by the local host, but that's not
>> necessarily true when the local host is the default route of a subnet
>> it is on, resulting in RST packets being sent out with an incorrect
>> destination MAC.  Address this by refactoring the handling of bridged
>> packets which deals with a similar issue.  Modulo patch fuzz, the
>> following applies to v5 and later kernels.

> The code this patch updates is related to BRIDGE_NETFILTER. Your patch
> description refers to the non-bridge case. What are you trying to
> achieve?

Via DHCP, my subnet's default route is a Linux system so that it can 
monitor all outbound traffic.  By doing so, for example, I have determined 
that my Android phone connects to Facebook despite the fact that I have no 
such app installed.  I want to know, and control, what other 
behind-the-scenes (under-handed) traffic devices on my subnet generate.

> dev_queue_xmit() path should not be exercised from the prerouting
> chain, packets generated from the IP later must follow the
> ip_local_out() path.

Well, I can tell you dev_queue_xmit() does in fact work in prerouting 
chains, as it must for the bridging case.  The only potential problem I've 
found so far is that the RST packet doesn't go through any netfilter 
hooks.

Marc.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH nf] netfilter REJECT: Fix destination MAC in RST packets
  2021-03-08 16:21   ` Marc Aurèle La France
@ 2021-03-09  1:36     ` Pablo Neira Ayuso
  2021-03-09  4:25       ` Marc Aurèle La France
  0 siblings, 1 reply; 9+ messages in thread
From: Pablo Neira Ayuso @ 2021-03-09  1:36 UTC (permalink / raw)
  To: Marc Aurèle La France; +Cc: netfilter-devel

On Mon, Mar 08, 2021 at 09:21:20AM -0700, Marc Aurèle La France wrote:
> On Mon, 8 Mar 2021, Pablo Neira Ayuso wrote:
> > On Sun, Mar 07, 2021 at 06:16:34PM -0700, Marc Aurèle La France wrote:
> > > In the non-bridge case, the REJECT target code assumes the REJECTed
> > > packets were originally emitted by the local host, but that's not
> > > necessarily true when the local host is the default route of a subnet
> > > it is on, resulting in RST packets being sent out with an incorrect
> > > destination MAC.  Address this by refactoring the handling of bridged
> > > packets which deals with a similar issue.  Modulo patch fuzz, the
> > > following applies to v5 and later kernels.
> 
> > The code this patch updates is related to BRIDGE_NETFILTER. Your patch
> > description refers to the non-bridge case. What are you trying to
> > achieve?
> 
> Via DHCP, my subnet's default route is a Linux system so that it can monitor
> all outbound traffic.  By doing so, for example, I have determined that my
> Android phone connects to Facebook despite the fact that I have no such app
> installed.  I want to know, and control, what other behind-the-scenes
> (under-handed) traffic devices on my subnet generate.
>
> > dev_queue_xmit() path should not be exercised from the prerouting
> > chain, packets generated from the IP later must follow the
> > ip_local_out() path.
> 
> Well, I can tell you dev_queue_xmit() does in fact work in prerouting
> chains, as it must for the bridging case.  The only potential problem I've
> found so far is that the RST packet doesn't go through any netfilter hooks.

That's the issue, Netfilter rejects code from the IP layer, so the
packets follows the ip_local_out() path.

You could use ingress to reject through dev_queue_xmit() / neigh_xmit().

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH nf] netfilter REJECT: Fix destination MAC in RST packets
  2021-03-09  1:36     ` Pablo Neira Ayuso
@ 2021-03-09  4:25       ` Marc Aurèle La France
  2021-03-09 10:27         ` Pablo Neira Ayuso
  0 siblings, 1 reply; 9+ messages in thread
From: Marc Aurèle La France @ 2021-03-09  4:25 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

[-- Attachment #1: Type: text/plain, Size: 2277 bytes --]

On Tue, 9 Mar 2021, Pablo Neira Ayuso wrote:
> On Mon, Mar 08, 2021 at 09:21:20AM -0700, Marc Aurèle La France wrote:
>> On Mon, 8 Mar 2021, Pablo Neira Ayuso wrote:
>>> On Sun, Mar 07, 2021 at 06:16:34PM -0700, Marc Aurèle La France wrote:

>>>> In the non-bridge case, the REJECT target code assumes the REJECTed
>>>> packets were originally emitted by the local host, but that's not
>>>> necessarily true when the local host is the default route of a subnet
>>>> it is on, resulting in RST packets being sent out with an incorrect
>>>> destination MAC.  Address this by refactoring the handling of bridged
>>>> packets which deals with a similar issue.  Modulo patch fuzz, the
>>>> following applies to v5 and later kernels.

>>> The code this patch updates is related to BRIDGE_NETFILTER. Your patch
>>> description refers to the non-bridge case. What are you trying to
>>> achieve?

>> Via DHCP, my subnet's default route is a Linux system so that it can monitor
>> all outbound traffic.  By doing so, for example, I have determined that my
>> Android phone connects to Facebook despite the fact that I have no such app
>> installed.  I want to know, and control, what other behind-the-scenes
>> (under-handed) traffic devices on my subnet generate.

>>> dev_queue_xmit() path should not be exercised from the prerouting
>>> chain, packets generated from the IP later must follow the
>>> ip_local_out() path.

>> Well, I can tell you dev_queue_xmit() does in fact work in prerouting
>> chains, as it must for the bridging case.  The only potential problem I've
>> found so far is that the RST packet doesn't go through any netfilter hooks.

> That's the issue, Netfilter rejects code from the IP layer, so the
> packets follows the ip_local_out() path.

... which sets an incorrect destination MAC.  Also, in this case, 
netfilter doesn't reject any such thing.  It doesn't even "see" the RST 
packet dev_queue_xmit() sends out.  That's OK as there is no further need 
to process such a packet.  At least, the device whose connection 
request is being denied doesn't hang anymore...

> You could use ingress to reject through dev_queue_xmit() / neigh_xmit().

I'm sticking to my guns.  I'm a firm believer in the KISS principle, part 
of a dying breed to be sure.

Marc.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH nf] netfilter REJECT: Fix destination MAC in RST packets
  2021-03-09  4:25       ` Marc Aurèle La France
@ 2021-03-09 10:27         ` Pablo Neira Ayuso
  2021-03-10 23:51           ` Marc Aurèle La France
  0 siblings, 1 reply; 9+ messages in thread
From: Pablo Neira Ayuso @ 2021-03-09 10:27 UTC (permalink / raw)
  To: Marc Aurèle La France; +Cc: netfilter-devel

On Mon, Mar 08, 2021 at 09:25:28PM -0700, Marc Aurèle La France wrote:
> On Tue, 9 Mar 2021, Pablo Neira Ayuso wrote:
> > On Mon, Mar 08, 2021 at 09:21:20AM -0700, Marc Aurèle La France wrote:
> > > On Mon, 8 Mar 2021, Pablo Neira Ayuso wrote:
> > > > On Sun, Mar 07, 2021 at 06:16:34PM -0700, Marc Aurèle La France wrote:
> 
> > > > > In the non-bridge case, the REJECT target code assumes the REJECTed
> > > > > packets were originally emitted by the local host, but that's not
> > > > > necessarily true when the local host is the default route of a subnet
> > > > > it is on, resulting in RST packets being sent out with an incorrect
> > > > > destination MAC.  Address this by refactoring the handling of bridged
> > > > > packets which deals with a similar issue.  Modulo patch fuzz, the
> > > > > following applies to v5 and later kernels.
> 
> > > > The code this patch updates is related to BRIDGE_NETFILTER. Your patch
> > > > description refers to the non-bridge case. What are you trying to
> > > > achieve?
> 
> > > Via DHCP, my subnet's default route is a Linux system so that it can monitor
> > > all outbound traffic.  By doing so, for example, I have determined that my
> > > Android phone connects to Facebook despite the fact that I have no such app
> > > installed.  I want to know, and control, what other behind-the-scenes
> > > (under-handed) traffic devices on my subnet generate.
> 
> > > > dev_queue_xmit() path should not be exercised from the prerouting
> > > > chain, packets generated from the IP later must follow the
> > > > ip_local_out() path.
> 
> > > Well, I can tell you dev_queue_xmit() does in fact work in prerouting
> > > chains, as it must for the bridging case.  The only potential problem I've
> > > found so far is that the RST packet doesn't go through any netfilter hooks.
> 
> > That's the issue, Netfilter rejects code from the IP layer, so the
> > packets follows the ip_local_out() path.
> 
> ... which sets an incorrect destination MAC.  Also, in this case, netfilter
> doesn't reject any such thing.  It doesn't even "see" the RST packet
> dev_queue_xmit() sends out.  That's OK as there is no further need to
> process such a packet.

dev_queue_xmit() skips the policy in the local out path for the
generated RST packet. If you want to plain reject using
dev_queue_xmit() then you have to use the ingress hook.

> At least, the device whose connection request is being denied
> doesn't hang anymore...

The neighbour cache selects the destination MAC from the destination
IP address of the RST packet.

Your patch also refers to non-bridge scenario (no br_netfilter in
place).

Could you describe what you're trying to achieve in plain layman terms?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH nf] netfilter REJECT: Fix destination MAC in RST packets
  2021-03-09 10:27         ` Pablo Neira Ayuso
@ 2021-03-10 23:51           ` Marc Aurèle La France
  2021-03-11  0:02             ` Pablo Neira Ayuso
  0 siblings, 1 reply; 9+ messages in thread
From: Marc Aurèle La France @ 2021-03-10 23:51 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

[-- Attachment #1: Type: text/plain, Size: 3325 bytes --]

On Tue, 9 Mar 2021, Pablo Neira Ayuso wrote:
> On Mon, Mar 08, 2021 at 09:25:28PM -0700, Marc Aurèle La France wrote:
>> On Tue, 9 Mar 2021, Pablo Neira Ayuso wrote:
>>> On Mon, Mar 08, 2021 at 09:21:20AM -0700, Marc Aurèle La France wrote:
>>>> On Mon, 8 Mar 2021, Pablo Neira Ayuso wrote:
>>>>> On Sun, Mar 07, 2021 at 06:16:34PM -0700, Marc Aurèle La France wrote:

>>>>>> In the non-bridge case, the REJECT target code assumes the REJECTed
>>>>>> packets were originally emitted by the local host, but that's not
>>>>>> necessarily true when the local host is the default route of a subnet
>>>>>> it is on, resulting in RST packets being sent out with an incorrect
>>>>>> destination MAC.  Address this by refactoring the handling of bridged
>>>>>> packets which deals with a similar issue.  Modulo patch fuzz, the
>>>>>> following applies to v5 and later kernels.

>>>>> The code this patch updates is related to BRIDGE_NETFILTER. Your patch
>>>>> description refers to the non-bridge case. What are you trying to
>>>>> achieve?

>>>> Via DHCP, my subnet's default route is a Linux system so that it can monitor
>>>> all outbound traffic.  By doing so, for example, I have determined that my
>>>> Android phone connects to Facebook despite the fact that I have no such app
>>>> installed.  I want to know, and control, what other behind-the-scenes
>>>> (under-handed) traffic devices on my subnet generate.

>>>>> dev_queue_xmit() path should not be exercised from the prerouting
>>>>> chain, packets generated from the IP later must follow the
>>>>> ip_local_out() path.

>>>> Well, I can tell you dev_queue_xmit() does in fact work in prerouting
>>>> chains, as it must for the bridging case.  The only potential problem I've
>>>> found so far is that the RST packet doesn't go through any netfilter hooks.

>>> That's the issue, Netfilter rejects code from the IP layer, so the
>>> packets follows the ip_local_out() path.

>> ... which sets an incorrect destination MAC.  Also, in this case, netfilter
>> doesn't reject any such thing.  It doesn't even "see" the RST packet
>> dev_queue_xmit() sends out.  That's OK as there is no further need to
>> process such a packet.

> dev_queue_xmit() skips the policy in the local out path for the
> generated RST packet. If you want to plain reject using
> dev_queue_xmit() then you have to use the ingress hook.

>> At least, the device whose connection request is being denied
>> doesn't hang anymore...

> The neighbour cache selects the destination MAC from the destination
> IP address of the RST packet.

> Your patch also refers to non-bridge scenario (no br_netfilter in
> place).

> Could you describe what you're trying to achieve in plain layman terms?

I will (re-)do no such thing because you are refusing to make sense.

It's OK that the bridge code uses dev_queue_xmit() to send out an RST 
packet that has correct MACs, but that doesn't make another trip through 
netfilter.  And yet you object to my leveraging of the very same behaviour 
to solve the exact same issue in a non-bridge scenario.

Wow!  What the hell is going on here?!?!?

I'll tell you one thing that won't change, regardless of how many 
irrelevant complications you throw at it.  To wit, by using existing code, 
the patch I submitted solves the issue I was seeing.  Full stop.

Marc.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH nf] netfilter REJECT: Fix destination MAC in RST packets
  2021-03-10 23:51           ` Marc Aurèle La France
@ 2021-03-11  0:02             ` Pablo Neira Ayuso
  2021-03-11  3:39               ` Marc Aurèle La France
  0 siblings, 1 reply; 9+ messages in thread
From: Pablo Neira Ayuso @ 2021-03-11  0:02 UTC (permalink / raw)
  To: Marc Aurèle La France; +Cc: netfilter-devel

On Wed, Mar 10, 2021 at 04:51:26PM -0700, Marc Aurèle La France wrote:
> On Tue, 9 Mar 2021, Pablo Neira Ayuso wrote:
> > On Mon, Mar 08, 2021 at 09:25:28PM -0700, Marc Aurèle La France wrote:
> > > On Tue, 9 Mar 2021, Pablo Neira Ayuso wrote:
> > > > On Mon, Mar 08, 2021 at 09:21:20AM -0700, Marc Aurèle La France wrote:
> > > > > On Mon, 8 Mar 2021, Pablo Neira Ayuso wrote:
> > > > > > On Sun, Mar 07, 2021 at 06:16:34PM -0700, Marc Aurèle La France wrote:
> 
> > > > > > > In the non-bridge case, the REJECT target code assumes the REJECTed
> > > > > > > packets were originally emitted by the local host, but that's not
> > > > > > > necessarily true when the local host is the default route of a subnet
> > > > > > > it is on, resulting in RST packets being sent out with an incorrect
> > > > > > > destination MAC.  Address this by refactoring the handling of bridged
> > > > > > > packets which deals with a similar issue.  Modulo patch fuzz, the
> > > > > > > following applies to v5 and later kernels.
> 
> > > > > > The code this patch updates is related to BRIDGE_NETFILTER. Your patch
> > > > > > description refers to the non-bridge case. What are you trying to
> > > > > > achieve?
> 
> > > > > Via DHCP, my subnet's default route is a Linux system so that it can monitor
> > > > > all outbound traffic.  By doing so, for example, I have determined that my
> > > > > Android phone connects to Facebook despite the fact that I have no such app
> > > > > installed.  I want to know, and control, what other behind-the-scenes
> > > > > (under-handed) traffic devices on my subnet generate.
> 
> > > > > > dev_queue_xmit() path should not be exercised from the prerouting
> > > > > > chain, packets generated from the IP later must follow the
> > > > > > ip_local_out() path.
> 
> > > > > Well, I can tell you dev_queue_xmit() does in fact work in prerouting
> > > > > chains, as it must for the bridging case.  The only potential problem I've
> > > > > found so far is that the RST packet doesn't go through any netfilter hooks.
> 
> > > > That's the issue, Netfilter rejects code from the IP layer, so the
> > > > packets follows the ip_local_out() path.
> 
> > > ... which sets an incorrect destination MAC.  Also, in this case, netfilter
> > > doesn't reject any such thing.  It doesn't even "see" the RST packet
> > > dev_queue_xmit() sends out.  That's OK as there is no further need to
> > > process such a packet.
> 
> > dev_queue_xmit() skips the policy in the local out path for the
> > generated RST packet. If you want to plain reject using
> > dev_queue_xmit() then you have to use the ingress hook.
> 
> > > At least, the device whose connection request is being denied
> > > doesn't hang anymore...
> 
> > The neighbour cache selects the destination MAC from the destination
> > IP address of the RST packet.
> 
> > Your patch also refers to non-bridge scenario (no br_netfilter in
> > place).
> 
> > Could you describe what you're trying to achieve in plain layman terms?
> 
> I will (re-)do no such thing because you are refusing to make sense.
> 
> It's OK that the bridge code uses dev_queue_xmit() to send out an RST packet
> that has correct MACs, but that doesn't make another trip through netfilter.

It's not OK that the bridge uses dev_queue_xmit().

That was an ugly solution to make the REJECT target work from
br_netfilter, because there was absolutely no other better way at that
time to make it work.

There has been now native support to reject traffic from the bridge
from many years on through br_forward(), which is the way to go.

If you want to reject from a non-bridge setup, there is also ingress
support where you can reject exactly in the way you want, way earlier
than from prerouting.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH nf] netfilter REJECT: Fix destination MAC in RST packets
  2021-03-11  0:02             ` Pablo Neira Ayuso
@ 2021-03-11  3:39               ` Marc Aurèle La France
  0 siblings, 0 replies; 9+ messages in thread
From: Marc Aurèle La France @ 2021-03-11  3:39 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

[-- Attachment #1: Type: text/plain, Size: 3571 bytes --]

On Thu, 11 Mar 2021, Pablo Neira Ayuso wrote:
> On Wed, Mar 10, 2021 at 04:51:26PM -0700, Marc Aurèle La France wrote:
>> On Tue, 9 Mar 2021, Pablo Neira Ayuso wrote:
>>> On Mon, Mar 08, 2021 at 09:25:28PM -0700, Marc Aurèle La France wrote:
>>>> On Tue, 9 Mar 2021, Pablo Neira Ayuso wrote:
>>>>> On Mon, Mar 08, 2021 at 09:21:20AM -0700, Marc Aurèle La France wrote:
>>>>>> On Mon, 8 Mar 2021, Pablo Neira Ayuso wrote:
>>>>>>> On Sun, Mar 07, 2021 at 06:16:34PM -0700, Marc Aurèle La France wrote:
>>>>>>>> In the non-bridge case, the REJECT target code assumes the REJECTed
>>>>>>>> packets were originally emitted by the local host, but that's not
>>>>>>>> necessarily true when the local host is the default route of a subnet
>>>>>>>> it is on, resulting in RST packets being sent out with an incorrect
>>>>>>>> destination MAC.  Address this by refactoring the handling of bridged
>>>>>>>> packets which deals with a similar issue.  Modulo patch fuzz, the
>>>>>>>> following applies to v5 and later kernels.

>>>>>>> The code this patch updates is related to BRIDGE_NETFILTER. Your patch
>>>>>>> description refers to the non-bridge case. What are you trying to
>>>>>>> achieve?

>>>>>> Via DHCP, my subnet's default route is a Linux system so that it can monitor
>>>>>> all outbound traffic.  By doing so, for example, I have determined that my
>>>>>> Android phone connects to Facebook despite the fact that I have no such app
>>>>>> installed.  I want to know, and control, what other behind-the-scenes
>>>>>> (under-handed) traffic devices on my subnet generate.

>>>>>>> dev_queue_xmit() path should not be exercised from the prerouting
>>>>>>> chain, packets generated from the IP later must follow the
>>>>>>> ip_local_out() path.

>>>>>> Well, I can tell you dev_queue_xmit() does in fact work in prerouting
>>>>>> chains, as it must for the bridging case.  The only potential problem I've
>>>>>> found so far is that the RST packet doesn't go through any netfilter hooks.

>>>>> That's the issue, Netfilter rejects code from the IP layer, so the
>>>>> packets follows the ip_local_out() path.

>>>> ... which sets an incorrect destination MAC.  Also, in this case, netfilter
>>>> doesn't reject any such thing.  It doesn't even "see" the RST packet
>>>> dev_queue_xmit() sends out.  That's OK as there is no further need to
>>>> process such a packet.

>>> dev_queue_xmit() skips the policy in the local out path for the
>>> generated RST packet. If you want to plain reject using
>>> dev_queue_xmit() then you have to use the ingress hook.

>>>> At least, the device whose connection request is being denied
>>>> doesn't hang anymore...

>>> The neighbour cache selects the destination MAC from the destination
>>> IP address of the RST packet.

>>> Your patch also refers to non-bridge scenario (no br_netfilter in
>>> place).

>>> Could you describe what you're trying to achieve in plain layman terms?

>> I will (re-)do no such thing because you are refusing to make sense.

>> It's OK that the bridge code uses dev_queue_xmit() to send out an RST packet
>> that has correct MACs, but that doesn't make another trip through netfilter.

> It's not OK that the bridge uses dev_queue_xmit().

> That was an ugly solution to make the REJECT target work from
> br_netfilter, because there was absolutely no other better way at that
> time to make it work.

> There has been now native support to reject traffic from the bridge
> from many years on through br_forward(), which is the way to go.

Ah, the cart before the horse.  I'll wait.

Later.

Marc.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-03-11  3:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-08  1:16 [PATCH nf] netfilter REJECT: Fix destination MAC in RST packets Marc Aurèle La France
2021-03-08 10:25 ` Pablo Neira Ayuso
2021-03-08 16:21   ` Marc Aurèle La France
2021-03-09  1:36     ` Pablo Neira Ayuso
2021-03-09  4:25       ` Marc Aurèle La France
2021-03-09 10:27         ` Pablo Neira Ayuso
2021-03-10 23:51           ` Marc Aurèle La France
2021-03-11  0:02             ` Pablo Neira Ayuso
2021-03-11  3:39               ` Marc Aurèle La France

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.