[PATCH iptables] extensions: libebt_redirect: prevent translation

[PATCH iptables] extensions: libebt_redirect: prevent translation

[Miao Wang]

The redirect target in ebtables do two things: 1. set skb->pkt_type to
PACKET_HOST, and 2. set the destination mac address to the address of
the receiving bridge device (when not used in BROUTING chain), or the
receiving physical device (otherwise). However, the later cannot be
implemented in nftables not given the translated mac address. So it is
not appropriate to give a specious translation.

This patch adds xt target redirect to the translated nft rule, to ensure
it cannot be later loaded by nft, to prevent possible misunderstanding.

Fixes: 24ce7465056ae ("ebtables-compat: add redirect match extension")
Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
---
 extensions/libebt_redirect.c      | 2 +-
 extensions/libebt_redirect.txlate | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/extensions/libebt_redirect.c b/extensions/libebt_redirect.c
index a44dbaec6cc8b12f20acd31dcb1360ac7245e349..83d2b576cea5ae625f3bdf667ad56fc57c1665d9 100644
--- a/extensions/libebt_redirect.c
+++ b/extensions/libebt_redirect.c
@@ -77,7 +77,7 @@ static int brredir_xlate(struct xt_xlate *xl,
 {
 	const struct ebt_redirect_info *red = (const void*)params->target->data;
 
-	xt_xlate_add(xl, "meta pkttype set host");
+	xt_xlate_add(xl, "meta pkttype set host xt target redirect");
 	if (red->target != EBT_CONTINUE)
 		xt_xlate_add(xl, " %s ", brredir_verdict(red->target));
 	return 1;
diff --git a/extensions/libebt_redirect.txlate b/extensions/libebt_redirect.txlate
index d073ec774c4fa817e48422fb99aaf095dd9eab65..abafd8d15aef8349d29ad812a03f0ebeeaea118c 100644
--- a/extensions/libebt_redirect.txlate
+++ b/extensions/libebt_redirect.txlate
@@ -1,8 +1,8 @@
 ebtables-translate -t nat -A PREROUTING -d de:ad:00:00:be:ef -j redirect
-nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host accept'
+nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host xt target redirect accept'
 
 ebtables-translate -t nat -A PREROUTING -d de:ad:00:00:be:ef -j redirect --redirect-target RETURN
-nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host return'
+nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host xt target redirect return'
 
 ebtables-translate -t nat -A PREROUTING -d de:ad:00:00:be:ef -j redirect --redirect-target CONTINUE
-nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host'
+nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host xt target redirect'

---
base-commit: 192c3a6bc18f206895ec5e38812d648ccfe7e281
change-id: 20250425-xlat-ebt-redir-aa40928f6fae

Best regards,
-- 
Miao Wang <shankerwangmiao@gmail.com>

[PATCH iptables] extensions: libebt_redirect: prevent translation

[Miao Wang via B4 Relay]

From: Miao Wang <shankerwangmiao@gmail.com>

The redirect target in ebtables do two things: 1. set skb->pkt_type to
PACKET_HOST, and 2. set the destination mac address to the address of
the receiving bridge device (when not used in BROUTING chain), or the
receiving physical device (otherwise). However, the later cannot be
implemented in nftables not given the translated mac address. So it is
not appropriate to give a specious translation.

This patch adds xt target redirect to the translated nft rule, to ensure
it cannot be later loaded by nft, to prevent possible misunderstanding.

Fixes: 24ce7465056ae ("ebtables-compat: add redirect match extension")
Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
---
 extensions/libebt_redirect.c      | 2 +-
 extensions/libebt_redirect.txlate | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/extensions/libebt_redirect.c b/extensions/libebt_redirect.c
index a44dbaec6cc8b12f20acd31dcb1360ac7245e349..83d2b576cea5ae625f3bdf667ad56fc57c1665d9 100644
--- a/extensions/libebt_redirect.c
+++ b/extensions/libebt_redirect.c
@@ -77,7 +77,7 @@ static int brredir_xlate(struct xt_xlate *xl,
 {
 	const struct ebt_redirect_info *red = (const void*)params->target->data;
 
-	xt_xlate_add(xl, "meta pkttype set host");
+	xt_xlate_add(xl, "meta pkttype set host xt target redirect");
 	if (red->target != EBT_CONTINUE)
 		xt_xlate_add(xl, " %s ", brredir_verdict(red->target));
 	return 1;
diff --git a/extensions/libebt_redirect.txlate b/extensions/libebt_redirect.txlate
index d073ec774c4fa817e48422fb99aaf095dd9eab65..abafd8d15aef8349d29ad812a03f0ebeeaea118c 100644
--- a/extensions/libebt_redirect.txlate
+++ b/extensions/libebt_redirect.txlate
@@ -1,8 +1,8 @@
 ebtables-translate -t nat -A PREROUTING -d de:ad:00:00:be:ef -j redirect
-nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host accept'
+nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host xt target redirect accept'
 
 ebtables-translate -t nat -A PREROUTING -d de:ad:00:00:be:ef -j redirect --redirect-target RETURN
-nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host return'
+nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host xt target redirect return'
 
 ebtables-translate -t nat -A PREROUTING -d de:ad:00:00:be:ef -j redirect --redirect-target CONTINUE
-nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host'
+nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host xt target redirect'

---
base-commit: 192c3a6bc18f206895ec5e38812d648ccfe7e281
change-id: 20250425-xlat-ebt-redir-aa40928f6fae

Best regards,
-- 
Miao Wang <shankerwangmiao@gmail.com>

Re: [PATCH iptables] extensions: libebt_redirect: prevent translation

[Pablo Neira Ayuso]

On Fri, Apr 25, 2025 at 04:44:24PM +0800, Miao Wang via B4 Relay wrote:
> From: Miao Wang <shankerwangmiao@gmail.com>
> 
> The redirect target in ebtables do two things: 1. set skb->pkt_type to
> PACKET_HOST, and 2. set the destination mac address to the address of
> the receiving bridge device (when not used in BROUTING chain), or the
> receiving physical device (otherwise). However, the later cannot be
> implemented in nftables not given the translated mac address. So it is
> not appropriate to give a specious translation.
> 
> This patch adds xt target redirect to the translated nft rule, to ensure
> it cannot be later loaded by nft, to prevent possible misunderstanding.
> 
> Fixes: 24ce7465056ae ("ebtables-compat: add redirect match extension")
> Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
> ---
>  extensions/libebt_redirect.c      | 2 +-
>  extensions/libebt_redirect.txlate | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/extensions/libebt_redirect.c b/extensions/libebt_redirect.c
> index a44dbaec6cc8b12f20acd31dcb1360ac7245e349..83d2b576cea5ae625f3bdf667ad56fc57c1665d9 100644
> --- a/extensions/libebt_redirect.c
> +++ b/extensions/libebt_redirect.c
> @@ -77,7 +77,7 @@ static int brredir_xlate(struct xt_xlate *xl,
>  {
>  	const struct ebt_redirect_info *red = (const void*)params->target->data;
>  
> -	xt_xlate_add(xl, "meta pkttype set host");
> +	xt_xlate_add(xl, "meta pkttype set host xt target redirect");
>  	if (red->target != EBT_CONTINUE)
>  		xt_xlate_add(xl, " %s ", brredir_verdict(red->target));
>  	return 1;
> diff --git a/extensions/libebt_redirect.txlate b/extensions/libebt_redirect.txlate
> index d073ec774c4fa817e48422fb99aaf095dd9eab65..abafd8d15aef8349d29ad812a03f0ebeeaea118c 100644
> --- a/extensions/libebt_redirect.txlate
> +++ b/extensions/libebt_redirect.txlate
> @@ -1,8 +1,8 @@
>  ebtables-translate -t nat -A PREROUTING -d de:ad:00:00:be:ef -j redirect
> -nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host accept'
> +nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host xt target redirect accept'

this is not a working translation, I don't think this is leaving this
in a better situation than before.

>  ebtables-translate -t nat -A PREROUTING -d de:ad:00:00:be:ef -j redirect --redirect-target RETURN
> -nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host return'
> +nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host xt target redirect return'
>  
>  ebtables-translate -t nat -A PREROUTING -d de:ad:00:00:be:ef -j redirect --redirect-target CONTINUE
> -nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host'
> +nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host xt target redirect'
> 
> ---
> base-commit: 192c3a6bc18f206895ec5e38812d648ccfe7e281
> change-id: 20250425-xlat-ebt-redir-aa40928f6fae
> 
> Best regards,
> -- 
> Miao Wang <shankerwangmiao@gmail.com>
> 
> 
> 

Re: [PATCH iptables] extensions: libebt_redirect: prevent translation

[Miao Wang]

> 2025年4月25日 17:01，Pablo Neira Ayuso <pablo@netfilter.org> 写道：
> 
> On Fri, Apr 25, 2025 at 04:44:24PM +0800, Miao Wang via B4 Relay wrote:
>> From: Miao Wang <shankerwangmiao@gmail.com>
>> 
>> The redirect target in ebtables do two things: 1. set skb->pkt_type to
>> PACKET_HOST, and 2. set the destination mac address to the address of
>> the receiving bridge device (when not used in BROUTING chain), or the
>> receiving physical device (otherwise). However, the later cannot be
>> implemented in nftables not given the translated mac address. So it is
>> not appropriate to give a specious translation.
>> 
>> This patch adds xt target redirect to the translated nft rule, to ensure
>> it cannot be later loaded by nft, to prevent possible misunderstanding.
>> 
>> Fixes: 24ce7465056ae ("ebtables-compat: add redirect match extension")
>> Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
>> ---
>> extensions/libebt_redirect.c      | 2 +-
>> extensions/libebt_redirect.txlate | 6 +++---
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/extensions/libebt_redirect.c b/extensions/libebt_redirect.c
>> index a44dbaec6cc8b12f20acd31dcb1360ac7245e349..83d2b576cea5ae625f3bdf667ad56fc57c1665d9 100644
>> --- a/extensions/libebt_redirect.c
>> +++ b/extensions/libebt_redirect.c
>> @@ -77,7 +77,7 @@ static int brredir_xlate(struct xt_xlate *xl,
>> {
>> const struct ebt_redirect_info *red = (const void*)params->target->data;
>> 
>> - xt_xlate_add(xl, "meta pkttype set host");
>> + xt_xlate_add(xl, "meta pkttype set host xt target redirect");
>> if (red->target != EBT_CONTINUE)
>> xt_xlate_add(xl, " %s ", brredir_verdict(red->target));
>> return 1;
>> diff --git a/extensions/libebt_redirect.txlate b/extensions/libebt_redirect.txlate
>> index d073ec774c4fa817e48422fb99aaf095dd9eab65..abafd8d15aef8349d29ad812a03f0ebeeaea118c 100644
>> --- a/extensions/libebt_redirect.txlate
>> +++ b/extensions/libebt_redirect.txlate
>> @@ -1,8 +1,8 @@
>> ebtables-translate -t nat -A PREROUTING -d de:ad:00:00:be:ef -j redirect
>> -nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host accept'
>> +nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host xt target redirect accept'
> 
> this is not a working translation, I don't think this is leaving this
> in a better situation than before.

Or can we fully remove the translation? The translation result is
really misleading, because the result is a valid nft rule statement
but cannot work as intended.

Cheers,

Miao Wang

Re: [PATCH iptables] extensions: libebt_redirect: prevent translation

[Pablo Neira Ayuso]

On Fri, Apr 25, 2025 at 05:07:04PM +0800, Miao Wang wrote:
> 
> > 2025年4月25日 17:01，Pablo Neira Ayuso <pablo@netfilter.org> 写道：
> > 
> > On Fri, Apr 25, 2025 at 04:44:24PM +0800, Miao Wang via B4 Relay wrote:
> >> From: Miao Wang <shankerwangmiao@gmail.com>
> >> 
> >> The redirect target in ebtables do two things: 1. set skb->pkt_type to
> >> PACKET_HOST, and 2. set the destination mac address to the address of
> >> the receiving bridge device (when not used in BROUTING chain), or the
> >> receiving physical device (otherwise). However, the later cannot be
> >> implemented in nftables not given the translated mac address. So it is
> >> not appropriate to give a specious translation.
> >> 
> >> This patch adds xt target redirect to the translated nft rule, to ensure
> >> it cannot be later loaded by nft, to prevent possible misunderstanding.
> >> 
> >> Fixes: 24ce7465056ae ("ebtables-compat: add redirect match extension")
> >> Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
> >> ---
> >> extensions/libebt_redirect.c      | 2 +-
> >> extensions/libebt_redirect.txlate | 6 +++---
> >> 2 files changed, 4 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/extensions/libebt_redirect.c b/extensions/libebt_redirect.c
> >> index a44dbaec6cc8b12f20acd31dcb1360ac7245e349..83d2b576cea5ae625f3bdf667ad56fc57c1665d9 100644
> >> --- a/extensions/libebt_redirect.c
> >> +++ b/extensions/libebt_redirect.c
> >> @@ -77,7 +77,7 @@ static int brredir_xlate(struct xt_xlate *xl,
> >> {
> >> const struct ebt_redirect_info *red = (const void*)params->target->data;
> >> 
> >> - xt_xlate_add(xl, "meta pkttype set host");
> >> + xt_xlate_add(xl, "meta pkttype set host xt target redirect");
> >> if (red->target != EBT_CONTINUE)
> >> xt_xlate_add(xl, " %s ", brredir_verdict(red->target));
> >> return 1;
> >> diff --git a/extensions/libebt_redirect.txlate b/extensions/libebt_redirect.txlate
> >> index d073ec774c4fa817e48422fb99aaf095dd9eab65..abafd8d15aef8349d29ad812a03f0ebeeaea118c 100644
> >> --- a/extensions/libebt_redirect.txlate
> >> +++ b/extensions/libebt_redirect.txlate
> >> @@ -1,8 +1,8 @@
> >> ebtables-translate -t nat -A PREROUTING -d de:ad:00:00:be:ef -j redirect
> >> -nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host accept'
> >> +nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host xt target redirect accept'
> > 
> > this is not a working translation, I don't think this is leaving this
> > in a better situation than before.
> 
> Or can we fully remove the translation? The translation result is
> really misleading, because the result is a valid nft rule statement
> but cannot work as intended.

I suggest to remove it.

It should be possible to fix this by adding a new expression, eg. nft_dev
to retrieve information from a device.

        case NFT_DEV_BR_IN_DEV_ADDR:
                memcpy(..., br_port_get_rcu(xt_in(par))->br->dev->dev_addr, ...);
                break;
        case NFT_DEV_IN_DEV_ADDR:
                memcpy(..., br_port_get_rcu(xt_in(par))->br->dev->dev_addr, ...);
                break;

it should be easy to add a new expression to provide this. Then,
provide a translation.

Re: [PATCH iptables] extensions: libebt_redirect: prevent translation

[Phil Sutter]

On Fri, Apr 25, 2025 at 05:07:04PM +0800, Miao Wang wrote:
> 
> > 2025年4月25日 17:01，Pablo Neira Ayuso <pablo@netfilter.org> 写道：
> > 
> > On Fri, Apr 25, 2025 at 04:44:24PM +0800, Miao Wang via B4 Relay wrote:
> >> From: Miao Wang <shankerwangmiao@gmail.com>
> >> 
> >> The redirect target in ebtables do two things: 1. set skb->pkt_type to
> >> PACKET_HOST, and 2. set the destination mac address to the address of
> >> the receiving bridge device (when not used in BROUTING chain), or the
> >> receiving physical device (otherwise). However, the later cannot be
> >> implemented in nftables not given the translated mac address. So it is
> >> not appropriate to give a specious translation.
> >> 
> >> This patch adds xt target redirect to the translated nft rule, to ensure
> >> it cannot be later loaded by nft, to prevent possible misunderstanding.
> >> 
> >> Fixes: 24ce7465056ae ("ebtables-compat: add redirect match extension")
> >> Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
> >> ---
> >> extensions/libebt_redirect.c      | 2 +-
> >> extensions/libebt_redirect.txlate | 6 +++---
> >> 2 files changed, 4 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/extensions/libebt_redirect.c b/extensions/libebt_redirect.c
> >> index a44dbaec6cc8b12f20acd31dcb1360ac7245e349..83d2b576cea5ae625f3bdf667ad56fc57c1665d9 100644
> >> --- a/extensions/libebt_redirect.c
> >> +++ b/extensions/libebt_redirect.c
> >> @@ -77,7 +77,7 @@ static int brredir_xlate(struct xt_xlate *xl,
> >> {
> >> const struct ebt_redirect_info *red = (const void*)params->target->data;
> >> 
> >> - xt_xlate_add(xl, "meta pkttype set host");
> >> + xt_xlate_add(xl, "meta pkttype set host xt target redirect");
> >> if (red->target != EBT_CONTINUE)
> >> xt_xlate_add(xl, " %s ", brredir_verdict(red->target));
> >> return 1;
> >> diff --git a/extensions/libebt_redirect.txlate b/extensions/libebt_redirect.txlate
> >> index d073ec774c4fa817e48422fb99aaf095dd9eab65..abafd8d15aef8349d29ad812a03f0ebeeaea118c 100644
> >> --- a/extensions/libebt_redirect.txlate
> >> +++ b/extensions/libebt_redirect.txlate
> >> @@ -1,8 +1,8 @@
> >> ebtables-translate -t nat -A PREROUTING -d de:ad:00:00:be:ef -j redirect
> >> -nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host accept'
> >> +nft 'add rule bridge nat PREROUTING ether daddr de:ad:00:00:be:ef counter meta pkttype set host xt target redirect accept'
> > 
> > this is not a working translation, I don't think this is leaving this
> > in a better situation than before.
> 
> Or can we fully remove the translation? The translation result is
> really misleading, because the result is a valid nft rule statement
> but cannot work as intended.

Returning 0 from the xlate callback aborts the translation. This will
cause nft to print 'xt target redirect'. :)

Cheers, Phil