* [libnftnl PATCH] utils: Add helpers for interface name wildcards
@ 2025-07-16 13:22 Phil Sutter
2025-07-22 2:46 ` Pablo Neira Ayuso
0 siblings, 1 reply; 6+ messages in thread
From: Phil Sutter @ 2025-07-16 13:22 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel
Support simple (suffix) wildcards in NFTNL_CHAIN_DEV(ICES) and
NFTA_FLOWTABLE_HOOK_DEVS identified by non-NUL-terminated strings. Add
helpers converting to and from the human-readable asterisk-suffix
notation.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v3:
- Use uint16_t for 'attr' parameter and size_t for internal 'len'
variable
- nftnl_attr_get_ifname to return allocated buffer, 'attr' parameter may
be const
Changes since v2:
- Use 'nftnl' prefix for introduced helpers
- Forward-declare struct nlattr to avoid extra include in utils.h
- Sanity-check array indexes to avoid out-of-bounds access
---
include/utils.h | 6 ++++++
src/chain.c | 8 +++++---
src/flowtable.c | 2 +-
src/str_array.c | 2 +-
src/utils.c | 31 +++++++++++++++++++++++++++++++
5 files changed, 44 insertions(+), 5 deletions(-)
diff --git a/include/utils.h b/include/utils.h
index 247d99d19dd7f..f232e7e2f3dd7 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -83,4 +83,10 @@ int nftnl_fprintf(FILE *fpconst, const void *obj, uint32_t cmd, uint32_t type,
int nftnl_set_str_attr(const char **dptr, uint32_t *flags,
uint16_t attr, const void *data, uint32_t data_len);
+struct nlattr;
+
+void nftnl_attr_put_ifname(struct nlmsghdr *nlh,
+ uint16_t attr, const char *ifname);
+char *nftnl_attr_get_ifname(const struct nlattr *attr);
+
#endif
diff --git a/src/chain.c b/src/chain.c
index 895108cddad51..b401526c367fb 100644
--- a/src/chain.c
+++ b/src/chain.c
@@ -457,14 +457,14 @@ void nftnl_chain_nlmsg_build_payload(struct nlmsghdr *nlh, const struct nftnl_ch
mnl_attr_put_u32(nlh, NFTA_HOOK_PRIORITY, htonl(c->prio));
if (c->flags & (1 << NFTNL_CHAIN_DEV))
- mnl_attr_put_strz(nlh, NFTA_HOOK_DEV, c->dev);
+ nftnl_attr_put_ifname(nlh, NFTA_HOOK_DEV, c->dev);
else if (c->flags & (1 << NFTNL_CHAIN_DEVICES)) {
struct nlattr *nest_dev;
const char *dev;
nest_dev = mnl_attr_nest_start(nlh, NFTA_HOOK_DEVS);
nftnl_str_array_foreach(dev, &c->dev_array, i)
- mnl_attr_put_strz(nlh, NFTA_DEVICE_NAME, dev);
+ nftnl_attr_put_ifname(nlh, NFTA_DEVICE_NAME, dev);
mnl_attr_nest_end(nlh, nest_dev);
}
@@ -648,7 +648,9 @@ static int nftnl_chain_parse_hook(struct nlattr *attr, struct nftnl_chain *c)
c->flags |= (1 << NFTNL_CHAIN_PRIO);
}
if (tb[NFTA_HOOK_DEV]) {
- c->dev = strdup(mnl_attr_get_str(tb[NFTA_HOOK_DEV]));
+ if (c->flags & (1 << NFTNL_CHAIN_DEV))
+ xfree(c->dev);
+ c->dev = nftnl_attr_get_ifname(tb[NFTA_HOOK_DEV]);
if (!c->dev)
return -1;
c->flags |= (1 << NFTNL_CHAIN_DEV);
diff --git a/src/flowtable.c b/src/flowtable.c
index fbbe0a866368d..006d50743e78a 100644
--- a/src/flowtable.c
+++ b/src/flowtable.c
@@ -299,7 +299,7 @@ void nftnl_flowtable_nlmsg_build_payload(struct nlmsghdr *nlh,
nest_dev = mnl_attr_nest_start(nlh, NFTA_FLOWTABLE_HOOK_DEVS);
nftnl_str_array_foreach(dev, &c->dev_array, i)
- mnl_attr_put_strz(nlh, NFTA_DEVICE_NAME, dev);
+ nftnl_attr_put_ifname(nlh, NFTA_DEVICE_NAME, dev);
mnl_attr_nest_end(nlh, nest_dev);
}
diff --git a/src/str_array.c b/src/str_array.c
index 5669c6154d394..02501c0cbdd79 100644
--- a/src/str_array.c
+++ b/src/str_array.c
@@ -56,7 +56,7 @@ int nftnl_parse_devs(struct nftnl_str_array *sa, const struct nlattr *nest)
return -1;
mnl_attr_for_each_nested(attr, nest) {
- sa->array[sa->len] = strdup(mnl_attr_get_str(attr));
+ sa->array[sa->len] = nftnl_attr_get_ifname(attr);
if (!sa->array[sa->len]) {
nftnl_str_array_clear(sa);
return -1;
diff --git a/src/utils.c b/src/utils.c
index 5f2c5bff7c650..2a8669c6242b7 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -13,8 +13,11 @@
#include <errno.h>
#include <inttypes.h>
+#include <libmnl/libmnl.h>
+
#include <libnftnl/common.h>
+#include <linux/if.h>
#include <linux/netfilter.h>
#include <linux/netfilter/nf_tables.h>
@@ -146,3 +149,31 @@ int nftnl_set_str_attr(const char **dptr, uint32_t *flags,
*flags |= (1 << attr);
return 0;
}
+
+void nftnl_attr_put_ifname(struct nlmsghdr *nlh,
+ uint16_t attr, const char *ifname)
+{
+ size_t len = strlen(ifname) + 1;
+
+ if (len >= 2 && ifname[len - 2] == '*')
+ len -= 2;
+
+ mnl_attr_put(nlh, attr, len, ifname);
+}
+
+char *nftnl_attr_get_ifname(const struct nlattr *attr)
+{
+ size_t slen = mnl_attr_get_payload_len(attr);
+ const char *dev = mnl_attr_get_str(attr);
+ char buf[IFNAMSIZ];
+
+ if (slen > 0 && dev[slen - 1] == '\0')
+ return strdup(dev);
+
+ if (slen > IFNAMSIZ - 2)
+ slen = IFNAMSIZ - 2;
+
+ memcpy(buf, dev, slen);
+ memcpy(buf + slen, "*\0", 2);
+ return strdup(buf);
+}
--
2.49.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [libnftnl PATCH] utils: Add helpers for interface name wildcards
2025-07-16 13:22 [libnftnl PATCH] utils: Add helpers for interface name wildcards Phil Sutter
@ 2025-07-22 2:46 ` Pablo Neira Ayuso
2025-07-22 13:43 ` Phil Sutter
0 siblings, 1 reply; 6+ messages in thread
From: Pablo Neira Ayuso @ 2025-07-22 2:46 UTC (permalink / raw)
To: Phil Sutter; +Cc: Florian Westphal, netfilter-devel
Hi Phil,
On Wed, Jul 16, 2025 at 03:22:06PM +0200, Phil Sutter wrote:
> Support simple (suffix) wildcards in NFTNL_CHAIN_DEV(ICES) and
> NFTA_FLOWTABLE_HOOK_DEVS identified by non-NUL-terminated strings. Add
> helpers converting to and from the human-readable asterisk-suffix
> notation.
We spent some time discussing scenarios where host and container could
use different userspace versions (older vs. newer).
In this case, newer version will send a string without the trailing
null character to the kernel. Then, the older version will just
_crash_ when parsing the netlink message from the kernel because it
expects a string that is nul-terminated (and we cannot fix an old
libnftnl library... it is not possible to fix the past, but it is
better if you can just deal with it).
I suggest you maybe pass the * at the end of the string to the kernel
so nft_netdev_hook_alloc() can just handle this special case and we
always have a nul-terminated string? There is ifnamelen which does in
the kernel what you need to compare the strings, while ifname can
still contain the *.
Worth a fix? Not much time ahead, but we are still in -rc7.
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
> Changes since v3:
> - Use uint16_t for 'attr' parameter and size_t for internal 'len'
> variable
> - nftnl_attr_get_ifname to return allocated buffer, 'attr' parameter may
> be const
>
> Changes since v2:
> - Use 'nftnl' prefix for introduced helpers
> - Forward-declare struct nlattr to avoid extra include in utils.h
> - Sanity-check array indexes to avoid out-of-bounds access
> ---
> include/utils.h | 6 ++++++
> src/chain.c | 8 +++++---
> src/flowtable.c | 2 +-
> src/str_array.c | 2 +-
> src/utils.c | 31 +++++++++++++++++++++++++++++++
> 5 files changed, 44 insertions(+), 5 deletions(-)
>
> diff --git a/include/utils.h b/include/utils.h
> index 247d99d19dd7f..f232e7e2f3dd7 100644
> --- a/include/utils.h
> +++ b/include/utils.h
> @@ -83,4 +83,10 @@ int nftnl_fprintf(FILE *fpconst, const void *obj, uint32_t cmd, uint32_t type,
> int nftnl_set_str_attr(const char **dptr, uint32_t *flags,
> uint16_t attr, const void *data, uint32_t data_len);
>
> +struct nlattr;
> +
> +void nftnl_attr_put_ifname(struct nlmsghdr *nlh,
> + uint16_t attr, const char *ifname);
> +char *nftnl_attr_get_ifname(const struct nlattr *attr);
> +
> #endif
> diff --git a/src/chain.c b/src/chain.c
> index 895108cddad51..b401526c367fb 100644
> --- a/src/chain.c
> +++ b/src/chain.c
> @@ -457,14 +457,14 @@ void nftnl_chain_nlmsg_build_payload(struct nlmsghdr *nlh, const struct nftnl_ch
> mnl_attr_put_u32(nlh, NFTA_HOOK_PRIORITY, htonl(c->prio));
>
> if (c->flags & (1 << NFTNL_CHAIN_DEV))
> - mnl_attr_put_strz(nlh, NFTA_HOOK_DEV, c->dev);
> + nftnl_attr_put_ifname(nlh, NFTA_HOOK_DEV, c->dev);
> else if (c->flags & (1 << NFTNL_CHAIN_DEVICES)) {
> struct nlattr *nest_dev;
> const char *dev;
>
> nest_dev = mnl_attr_nest_start(nlh, NFTA_HOOK_DEVS);
> nftnl_str_array_foreach(dev, &c->dev_array, i)
> - mnl_attr_put_strz(nlh, NFTA_DEVICE_NAME, dev);
> + nftnl_attr_put_ifname(nlh, NFTA_DEVICE_NAME, dev);
> mnl_attr_nest_end(nlh, nest_dev);
> }
>
> @@ -648,7 +648,9 @@ static int nftnl_chain_parse_hook(struct nlattr *attr, struct nftnl_chain *c)
> c->flags |= (1 << NFTNL_CHAIN_PRIO);
> }
> if (tb[NFTA_HOOK_DEV]) {
> - c->dev = strdup(mnl_attr_get_str(tb[NFTA_HOOK_DEV]));
> + if (c->flags & (1 << NFTNL_CHAIN_DEV))
> + xfree(c->dev);
> + c->dev = nftnl_attr_get_ifname(tb[NFTA_HOOK_DEV]);
> if (!c->dev)
> return -1;
> c->flags |= (1 << NFTNL_CHAIN_DEV);
> diff --git a/src/flowtable.c b/src/flowtable.c
> index fbbe0a866368d..006d50743e78a 100644
> --- a/src/flowtable.c
> +++ b/src/flowtable.c
> @@ -299,7 +299,7 @@ void nftnl_flowtable_nlmsg_build_payload(struct nlmsghdr *nlh,
>
> nest_dev = mnl_attr_nest_start(nlh, NFTA_FLOWTABLE_HOOK_DEVS);
> nftnl_str_array_foreach(dev, &c->dev_array, i)
> - mnl_attr_put_strz(nlh, NFTA_DEVICE_NAME, dev);
> + nftnl_attr_put_ifname(nlh, NFTA_DEVICE_NAME, dev);
> mnl_attr_nest_end(nlh, nest_dev);
> }
>
> diff --git a/src/str_array.c b/src/str_array.c
> index 5669c6154d394..02501c0cbdd79 100644
> --- a/src/str_array.c
> +++ b/src/str_array.c
> @@ -56,7 +56,7 @@ int nftnl_parse_devs(struct nftnl_str_array *sa, const struct nlattr *nest)
> return -1;
>
> mnl_attr_for_each_nested(attr, nest) {
> - sa->array[sa->len] = strdup(mnl_attr_get_str(attr));
> + sa->array[sa->len] = nftnl_attr_get_ifname(attr);
> if (!sa->array[sa->len]) {
> nftnl_str_array_clear(sa);
> return -1;
> diff --git a/src/utils.c b/src/utils.c
> index 5f2c5bff7c650..2a8669c6242b7 100644
> --- a/src/utils.c
> +++ b/src/utils.c
> @@ -13,8 +13,11 @@
> #include <errno.h>
> #include <inttypes.h>
>
> +#include <libmnl/libmnl.h>
> +
> #include <libnftnl/common.h>
>
> +#include <linux/if.h>
> #include <linux/netfilter.h>
> #include <linux/netfilter/nf_tables.h>
>
> @@ -146,3 +149,31 @@ int nftnl_set_str_attr(const char **dptr, uint32_t *flags,
> *flags |= (1 << attr);
> return 0;
> }
> +
> +void nftnl_attr_put_ifname(struct nlmsghdr *nlh,
> + uint16_t attr, const char *ifname)
> +{
> + size_t len = strlen(ifname) + 1;
> +
> + if (len >= 2 && ifname[len - 2] == '*')
> + len -= 2;
> +
> + mnl_attr_put(nlh, attr, len, ifname);
> +}
> +
> +char *nftnl_attr_get_ifname(const struct nlattr *attr)
> +{
> + size_t slen = mnl_attr_get_payload_len(attr);
> + const char *dev = mnl_attr_get_str(attr);
> + char buf[IFNAMSIZ];
> +
> + if (slen > 0 && dev[slen - 1] == '\0')
> + return strdup(dev);
> +
> + if (slen > IFNAMSIZ - 2)
> + slen = IFNAMSIZ - 2;
> +
> + memcpy(buf, dev, slen);
> + memcpy(buf + slen, "*\0", 2);
> + return strdup(buf);
> +}
> --
> 2.49.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [libnftnl PATCH] utils: Add helpers for interface name wildcards
2025-07-22 2:46 ` Pablo Neira Ayuso
@ 2025-07-22 13:43 ` Phil Sutter
2025-07-22 23:33 ` Pablo Neira Ayuso
0 siblings, 1 reply; 6+ messages in thread
From: Phil Sutter @ 2025-07-22 13:43 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel
On Tue, Jul 22, 2025 at 04:46:59AM +0200, Pablo Neira Ayuso wrote:
> Hi Phil,
>
> On Wed, Jul 16, 2025 at 03:22:06PM +0200, Phil Sutter wrote:
> > Support simple (suffix) wildcards in NFTNL_CHAIN_DEV(ICES) and
> > NFTA_FLOWTABLE_HOOK_DEVS identified by non-NUL-terminated strings. Add
> > helpers converting to and from the human-readable asterisk-suffix
> > notation.
>
> We spent some time discussing scenarios where host and container could
> use different userspace versions (older vs. newer).
>
> In this case, newer version will send a string without the trailing
> null character to the kernel. Then, the older version will just
> _crash_ when parsing the netlink message from the kernel because it
> expects a string that is nul-terminated (and we cannot fix an old
> libnftnl library... it is not possible to fix the past, but it is
> better if you can just deal with it).
Yes, this sucks. In a quick test, my host's nft would display "foo" for
a device spec of "foo*", but I believe this largely depends upon string
lengths, alignment and function-local buffer initial contents.
> I suggest you maybe pass the * at the end of the string to the kernel
> so nft_netdev_hook_alloc() can just handle this special case and we
> always have a nul-terminated string? There is ifnamelen which does in
> the kernel what you need to compare the strings, while ifname can
> still contain the *.
We can't distinguish this from real device names ending with asterisk,
though (Yes, no sane person would create those but since it's possible
there must be at least one doing it).
We could use a forbidden character to signal the wildcard instead.
Looking at dev_valid_name(), we may choose between '/', ':' and any of
the characters recognized by isspace(). I'd suggest to use something
fancy like '\v' (vertical tab) to lower the risk of hiding a user space
bug appending something the user may have inserted.
> Worth a fix? Not much time ahead, but we are still in -rc7.
Fine with me if we find a solution that works!
Cheers, Phil
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [libnftnl PATCH] utils: Add helpers for interface name wildcards
2025-07-22 13:43 ` Phil Sutter
@ 2025-07-22 23:33 ` Pablo Neira Ayuso
2025-07-23 0:07 ` Pablo Neira Ayuso
2025-07-23 11:49 ` Phil Sutter
0 siblings, 2 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2025-07-22 23:33 UTC (permalink / raw)
To: Phil Sutter, Florian Westphal, netfilter-devel
Hi Phil,
On Tue, Jul 22, 2025 at 03:43:49PM +0200, Phil Sutter wrote:
> On Tue, Jul 22, 2025 at 04:46:59AM +0200, Pablo Neira Ayuso wrote:
> > Hi Phil,
> >
> > On Wed, Jul 16, 2025 at 03:22:06PM +0200, Phil Sutter wrote:
> > > Support simple (suffix) wildcards in NFTNL_CHAIN_DEV(ICES) and
> > > NFTA_FLOWTABLE_HOOK_DEVS identified by non-NUL-terminated strings. Add
> > > helpers converting to and from the human-readable asterisk-suffix
> > > notation.
> >
> > We spent some time discussing scenarios where host and container could
> > use different userspace versions (older vs. newer).
> >
> > In this case, newer version will send a string without the trailing
> > null character to the kernel. Then, the older version will just
> > _crash_ when parsing the netlink message from the kernel because it
> > expects a string that is nul-terminated (and we cannot fix an old
> > libnftnl library... it is not possible to fix the past, but it is
> > better if you can just deal with it).
>
> Yes, this sucks. In a quick test, my host's nft would display "foo" for
> a device spec of "foo*", but I believe this largely depends upon string
> lengths, alignment and function-local buffer initial contents.
I see.
> > I suggest you maybe pass the * at the end of the string to the kernel
> > so nft_netdev_hook_alloc() can just handle this special case and we
> > always have a nul-terminated string? There is ifnamelen which does in
> > the kernel what you need to compare the strings, while ifname can
> > still contain the *.
>
> We can't distinguish this from real device names ending with asterisk,
> though (Yes, no sane person would create those but since it's possible
> there must be at least one doing it).
This is hard by looking only at the Value of the TLV.
> We could use a forbidden character to signal the wildcard instead.
> Looking at dev_valid_name(), we may choose between '/', ':' and any of
> the characters recognized by isspace(). I'd suggest to use something
> fancy like '\v' (vertical tab) to lower the risk of hiding a user space
> bug appending something the user may have inserted.
Let's look at this problem from a different side.
I'd suggest you add new netlink attribute NFTA_DEVICE_WILDCARD to
address this, ie.
enum nft_devices_attributes {
NFTA_DEVICE_UNSPEC,
NFTA_DEVICE_NAME,
+ NFTA_DEVICE_WILDCARD,
__NFTA_DEVICE_MAX
};
And use this new attribute for wildcard interface matching.
> > Worth a fix? Not much time ahead, but we are still in -rc7.
>
> Fine with me if we find a solution that works!
This approach allows for newer nftables version to fail with old
kernels, ie. user requests to match on wildcard device and kernel does
not support it. I think it is convenient to bail out if user requests
an unsupported kernel feature.
As for matching on an interface whose name is really eth*, nftables
userspace already allows for ifname eth\* to represent this, ie.
iifname eth* <-- wildcard matching (99% use-case)
iifname eth\* <-- to match on exotic (still valid) device name (1% use-case)
See special for '\\' in expr_evaluate_string() for handling this
special case.
It would be good if evaluate_device_expr() already provides an easy
way for the mnl backend to distinguish between wildcard matching or
exact device name matching.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [libnftnl PATCH] utils: Add helpers for interface name wildcards
2025-07-22 23:33 ` Pablo Neira Ayuso
@ 2025-07-23 0:07 ` Pablo Neira Ayuso
2025-07-23 11:49 ` Phil Sutter
1 sibling, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2025-07-23 0:07 UTC (permalink / raw)
To: Phil Sutter, Florian Westphal, netfilter-devel
On Wed, Jul 23, 2025 at 01:33:43AM +0200, Pablo Neira Ayuso wrote:
> Hi Phil,
>
> On Tue, Jul 22, 2025 at 03:43:49PM +0200, Phil Sutter wrote:
> > On Tue, Jul 22, 2025 at 04:46:59AM +0200, Pablo Neira Ayuso wrote:
> > > Hi Phil,
> > >
> > > On Wed, Jul 16, 2025 at 03:22:06PM +0200, Phil Sutter wrote:
> > > > Support simple (suffix) wildcards in NFTNL_CHAIN_DEV(ICES) and
> > > > NFTA_FLOWTABLE_HOOK_DEVS identified by non-NUL-terminated strings. Add
> > > > helpers converting to and from the human-readable asterisk-suffix
> > > > notation.
> > >
> > > We spent some time discussing scenarios where host and container could
> > > use different userspace versions (older vs. newer).
> > >
> > > In this case, newer version will send a string without the trailing
> > > null character to the kernel. Then, the older version will just
> > > _crash_ when parsing the netlink message from the kernel because it
> > > expects a string that is nul-terminated (and we cannot fix an old
> > > libnftnl library... it is not possible to fix the past, but it is
> > > better if you can just deal with it).
> >
> > Yes, this sucks. In a quick test, my host's nft would display "foo" for
> > a device spec of "foo*", but I believe this largely depends upon string
> > lengths, alignment and function-local buffer initial contents.
>
> I see.
>
> > > I suggest you maybe pass the * at the end of the string to the kernel
> > > so nft_netdev_hook_alloc() can just handle this special case and we
> > > always have a nul-terminated string? There is ifnamelen which does in
> > > the kernel what you need to compare the strings, while ifname can
> > > still contain the *.
> >
> > We can't distinguish this from real device names ending with asterisk,
> > though (Yes, no sane person would create those but since it's possible
> > there must be at least one doing it).
>
> This is hard by looking only at the Value of the TLV.
>
> > We could use a forbidden character to signal the wildcard instead.
> > Looking at dev_valid_name(), we may choose between '/', ':' and any of
> > the characters recognized by isspace(). I'd suggest to use something
> > fancy like '\v' (vertical tab) to lower the risk of hiding a user space
> > bug appending something the user may have inserted.
>
> Let's look at this problem from a different side.
>
> I'd suggest you add new netlink attribute NFTA_DEVICE_WILDCARD to
> address this, ie.
>
> enum nft_devices_attributes {
> NFTA_DEVICE_UNSPEC,
> NFTA_DEVICE_NAME,
> + NFTA_DEVICE_WILDCARD,
> __NFTA_DEVICE_MAX
> };
>
> And use this new attribute for wildcard interface matching.
>
> > > Worth a fix? Not much time ahead, but we are still in -rc7.
> >
> > Fine with me if we find a solution that works!
>
> This approach allows for newer nftables version to fail with old
> kernels, ie. user requests to match on wildcard device and kernel does
> not support it. I think it is convenient to bail out if user requests
> an unsupported kernel feature.
>
> As for matching on an interface whose name is really eth*, nftables
> userspace already allows for ifname eth\* to represent this, ie.
>
> iifname eth* <-- wildcard matching (99% use-case)
> iifname eth\* <-- to match on exotic (still valid) device name (1% use-case)
Actually, this example above is missing quotes:
iifname "eth*"
iifname "eth\*"
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [libnftnl PATCH] utils: Add helpers for interface name wildcards
2025-07-22 23:33 ` Pablo Neira Ayuso
2025-07-23 0:07 ` Pablo Neira Ayuso
@ 2025-07-23 11:49 ` Phil Sutter
1 sibling, 0 replies; 6+ messages in thread
From: Phil Sutter @ 2025-07-23 11:49 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel
On Wed, Jul 23, 2025 at 01:33:40AM +0200, Pablo Neira Ayuso wrote:
> Hi Phil,
>
> On Tue, Jul 22, 2025 at 03:43:49PM +0200, Phil Sutter wrote:
> > On Tue, Jul 22, 2025 at 04:46:59AM +0200, Pablo Neira Ayuso wrote:
> > > Hi Phil,
> > >
> > > On Wed, Jul 16, 2025 at 03:22:06PM +0200, Phil Sutter wrote:
> > > > Support simple (suffix) wildcards in NFTNL_CHAIN_DEV(ICES) and
> > > > NFTA_FLOWTABLE_HOOK_DEVS identified by non-NUL-terminated strings. Add
> > > > helpers converting to and from the human-readable asterisk-suffix
> > > > notation.
> > >
> > > We spent some time discussing scenarios where host and container could
> > > use different userspace versions (older vs. newer).
> > >
> > > In this case, newer version will send a string without the trailing
> > > null character to the kernel. Then, the older version will just
> > > _crash_ when parsing the netlink message from the kernel because it
> > > expects a string that is nul-terminated (and we cannot fix an old
> > > libnftnl library... it is not possible to fix the past, but it is
> > > better if you can just deal with it).
> >
> > Yes, this sucks. In a quick test, my host's nft would display "foo" for
> > a device spec of "foo*", but I believe this largely depends upon string
> > lengths, alignment and function-local buffer initial contents.
>
> I see.
>
> > > I suggest you maybe pass the * at the end of the string to the kernel
> > > so nft_netdev_hook_alloc() can just handle this special case and we
> > > always have a nul-terminated string? There is ifnamelen which does in
> > > the kernel what you need to compare the strings, while ifname can
> > > still contain the *.
> >
> > We can't distinguish this from real device names ending with asterisk,
> > though (Yes, no sane person would create those but since it's possible
> > there must be at least one doing it).
>
> This is hard by looking only at the Value of the TLV.
>
> > We could use a forbidden character to signal the wildcard instead.
> > Looking at dev_valid_name(), we may choose between '/', ':' and any of
> > the characters recognized by isspace(). I'd suggest to use something
> > fancy like '\v' (vertical tab) to lower the risk of hiding a user space
> > bug appending something the user may have inserted.
>
> Let's look at this problem from a different side.
>
> I'd suggest you add new netlink attribute NFTA_DEVICE_WILDCARD to
> address this, ie.
>
> enum nft_devices_attributes {
> NFTA_DEVICE_UNSPEC,
> NFTA_DEVICE_NAME,
> + NFTA_DEVICE_WILDCARD,
> __NFTA_DEVICE_MAX
> };
>
> And use this new attribute for wildcard interface matching.
>
> > > Worth a fix? Not much time ahead, but we are still in -rc7.
> >
> > Fine with me if we find a solution that works!
>
> This approach allows for newer nftables version to fail with old
> kernels, ie. user requests to match on wildcard device and kernel does
> not support it. I think it is convenient to bail out if user requests
> an unsupported kernel feature.
Yes, this seems reasonable. Right now old kernels would search for the
prefix as full interface name which is sane but may lead to unexpected
results: E.g. on a system with eth1, eth10 and eth11, the kernel would
find a device matching "eth1*" and users had to check 'list hooks' to
see what really happened.
> As for matching on an interface whose name is really eth*, nftables
> userspace already allows for ifname eth\* to represent this, ie.
>
> iifname eth* <-- wildcard matching (99% use-case)
> iifname eth\* <-- to match on exotic (still valid) device name (1% use-case)
>
> See special for '\\' in expr_evaluate_string() for handling this
> special case.
>
> It would be good if evaluate_device_expr() already provides an easy
> way for the mnl backend to distinguish between wildcard matching or
> exact device name matching.
Looks like I opened a new can by mentioning interfaces named "eth*", as
my proposed user space implementation doesn't support them it seems.
Changing my code to use a new attribute NFTA_DEVICE_WILDCARD should be
easy, but I'll look into proper evaluate integration.
Thanks, Phil
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-07-23 11:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-16 13:22 [libnftnl PATCH] utils: Add helpers for interface name wildcards Phil Sutter
2025-07-22 2:46 ` Pablo Neira Ayuso
2025-07-22 13:43 ` Phil Sutter
2025-07-22 23:33 ` Pablo Neira Ayuso
2025-07-23 0:07 ` Pablo Neira Ayuso
2025-07-23 11:49 ` Phil Sutter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).