let nftables indicate incomplete dissections

let nftables indicate incomplete dissections

[Florian Westphal]

"nft list ruleset" currently omits things it does not understand
and that it cannot represent in any other way.

This includes:
1. expression is unknown
2. expression is known (e.g. "cmp"), but attr contains unexpected value
3. expression is known but there is an unknown netlink attr contained in
the dump

If backend (libnftl) could mark expressions as incomplete (from .parse
callbacks?), it would be then possible for the frontend (nft) to document
this, e.g. by adding something like "# unknown attributes", or similar.

This is mainly needed for container environments, where host environment
might be using a lot older version than what is used by a specific
container image.

Related problem: entity that is using the raw netlink interface, it
that case libnftnl might be able to parse everything but nft could
lack the ability to properly print this.

If noone has any objections, I would place this on my todo list and
start with adding to libnftnl the needed "expression is incomplete"
marking by extending the .parse callbacks.

Re: let nftables indicate incomplete dissections

[Phil Sutter]

Hi!

On Wed, Jun 12, 2024 at 09:50:13AM +0200, Florian Westphal wrote:
> "nft list ruleset" currently omits things it does not understand
> and that it cannot represent in any other way.
> 
> This includes:
> 1. expression is unknown
> 2. expression is known (e.g. "cmp"), but attr contains unexpected value
> 3. expression is known but there is an unknown netlink attr contained in
> the dump
> 
> If backend (libnftl) could mark expressions as incomplete (from .parse
> callbacks?), it would be then possible for the frontend (nft) to document
> this, e.g. by adding something like "# unknown attributes", or similar.
> 
> This is mainly needed for container environments, where host environment
> might be using a lot older version than what is used by a specific
> container image.

ACK, we'll certainly end up in a similar situation as with iptables-nft
so doing nothing is not an option.

> Related problem: entity that is using the raw netlink interface, it
> that case libnftnl might be able to parse everything but nft could
> lack the ability to properly print this.
> 
> If noone has any objections, I would place this on my todo list and
> start with adding to libnftnl the needed "expression is incomplete"
> marking by extending the .parse callbacks.

The JSON interface prefixes dumps by a metainfo object which holds nft
version number and a schema version (still "1"). Introducing a similar
"bytecode versioning" cached in and dumped by kernel space might be a
quick way to enable a current nft tool to detect a bytecode from the
future, assuming that we'll also take care and increment that version
when things change. 

OTOH, considering compatibility (or testing for it somehow) of a given
bytecode change may be much more tedious than a practical approach of
trying to parse and using a defined exit when failing.

Cheers, Phil

Re: let nftables indicate incomplete dissections

[Pablo Neira Ayuso]

On Wed, Jun 12, 2024 at 09:50:13AM +0200, Florian Westphal wrote:
> "nft list ruleset" currently omits things it does not understand
> and that it cannot represent in any other way.
> 
> This includes:
> 1. expression is unknown
> 2. expression is known (e.g. "cmp"), but attr contains unexpected value
> 3. expression is known but there is an unknown netlink attr contained in
> the dump
> 
> If backend (libnftl) could mark expressions as incomplete (from .parse
> callbacks?), it would be then possible for the frontend (nft) to document
> this, e.g. by adding something like "# unknown attributes", or similar.

ack, how do you plan to handle this?

> This is mainly needed for container environments, where host environment
> might be using a lot older version than what is used by a specific
> container image.
> 
> Related problem: entity that is using the raw netlink interface, it
> that case libnftnl might be able to parse everything but nft could
> lack the ability to properly print this.

There are two options here:

- Add more raw expressions and dump them, eg. meta@15, where 15 is the type.
  This is more compact. If there is a requirement to allow to restore
  this from older nftables versions, then it might be not enough since
  maybe there is a need for meta@type,somethingelse (as in the ct direction
  case).
- Use a netlink representation as raw expression: meta@1,3,0x0x000000004
  but this requires dumping the whole list of attributes which is chatty.

Or explore a combination of both.

I am telling all this because I suspect maybe this "forward
compatibility" (a.k.a. "old tools support the future") could rise the
bar again and have a requirement to be able to load rulesets that
contains features that old tools don't understand.

> If noone has any objections, I would place this on my todo list and
> start with adding to libnftnl the needed "expression is incomplete"
> marking by extending the .parse callbacks.

Maybe it is worth exploring what I propose above instead of displaying
"expression is incomplete"?

Re: let nftables indicate incomplete dissections

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > If backend (libnftl) could mark expressions as incomplete (from .parse
> > callbacks?), it would be then possible for the frontend (nft) to document
> > this, e.g. by adding something like "# unknown attributes", or similar.
> 
> ack, how do you plan to handle this?

Add a "bool incomplete" to libnftnl strnct nftnl_expr, then set it
from each expr->parse callback if there is a new netlink attribute that
we did not understand.

nft then checks if this is incomplete-marker is set.

> > Related problem: entity that is using the raw netlink interface, it
> > that case libnftnl might be able to parse everything but nft could
> > lack the ability to properly print this.
> 
> There are two options here:
> 
> - Add more raw expressions and dump them, eg. meta@15, where 15 is the type.
>   This is more compact. If there is a requirement to allow to restore
>   this from older nftables versions, then it might be not enough since
>   maybe there is a need for meta@type,somethingelse (as in the ct direction
>   case).

Yes, for attributes that libnftnl knows about but where nft lacks a name
mapping (i.e., we can decode its META_KEY 0x42 but we have no idea what
that means we could in fact add such a representation scheme.

> - Use a netlink representation as raw expression: meta@1,3,0x0x000000004
>   but this requires dumping the whole list of attributes which is chatty.

Yes.  Perhaps its better to consider adding a new tool (script?) that
can dump the netlink soup without interpretation.  IIRC libmnl debug
already provides this functionality.

Very chatty but it would be good enough to figure out what such
hypothetical raw client did.

> Or explore a combination of both.

Right.

> I am telling all this because I suspect maybe this "forward
> compatibility" (a.k.a. "old tools support the future") could rise the
> bar again and have a requirement to be able to load rulesets that
> contains features that old tools don't understand.

Well, I don't think we can do that.  Perhaps with a new tool
that allows to assemble raw expressions, but I'm not sure its worth
extending nft for this, the parser (and grammar) is already huge.

> > If noone has any objections, I would place this on my todo list and
> > start with adding to libnftnl the needed "expression is incomplete"
> > marking by extending the .parse callbacks.
> 
> Maybe it is worth exploring what I propose above instead of displaying
> "expression is incomplete"?

For cases where libnftnl is fine but nft lacks a human-readable name I
think such @meta,42 would be fine.

But I don't think its good to allow decoding something arbitary, I think
we would have to acknowledge that this can't be done unless you have
a textual parser for raw netlink descriptions (i.e., full/unreadable TLV soup).

Re: let nftables indicate incomplete dissections

[Pablo Neira Ayuso]

Hi Florian,

On Tue, Jun 18, 2024 at 11:31:35AM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > If backend (libnftl) could mark expressions as incomplete (from .parse
> > > callbacks?), it would be then possible for the frontend (nft) to document
> > > this, e.g. by adding something like "# unknown attributes", or similar.
> > 
> > ack, how do you plan to handle this?
> 
> Add a "bool incomplete" to libnftnl strnct nftnl_expr, then set it
> from each expr->parse callback if there is a new netlink attribute that
> we did not understand.
> 
> nft then checks if this is incomplete-marker is set.

Is this sufficient? There are attribute values that could have an
unknown/unsupported value, eg. exthdr type (assuming a new extension
is supported).

I am afraid setting incomplete for an unknown attribute also restricts
netlink extensibility.

Netlink allows for adding new attributes. Attributes also determine
semantics, eg. exthdr type restricts the semantics of other existing
attributes. There are also flags attributes which provide a hint on
what is support or not, toggling such flag allows kernel to reject
something that is not support.

> > > Related problem: entity that is using the raw netlink interface, it
> > > that case libnftnl might be able to parse everything but nft could
> > > lack the ability to properly print this.
> > 
> > There are two options here:
> > 
> > - Add more raw expressions and dump them, eg. meta@15, where 15 is the type.
> >   This is more compact. If there is a requirement to allow to restore
> >   this from older nftables versions, then it might be not enough since
> >   maybe there is a need for meta@type,somethingelse (as in the ct direction
> >   case).
> 
> Yes, for attributes that libnftnl knows about but where nft lacks a name
> mapping (i.e., we can decode its META_KEY 0x42 but we have no idea what
> that means we could in fact add such a representation scheme.
> 
> > - Use a netlink representation as raw expression: meta@1,3,0x0x000000004
> >   but this requires dumping the whole list of attributes which is chatty.
> 
> Yes.  Perhaps its better to consider adding a new tool (script?) that
> can dump the netlink soup without interpretation.  IIRC libmnl debug
> already provides this functionality.
> 
> Very chatty but it would be good enough to figure out what such
> hypothetical raw client did.

I see, a binary netlink dump format.

> > Or explore a combination of both.
> 
> Right.
> 
> > I am telling all this because I suspect maybe this "forward
> > compatibility" (a.k.a. "old tools support the future") could rise the
> > bar again and have a requirement to be able to load rulesets that
> > contains features that old tools don't understand.
> 
> Well, I don't think we can do that.  Perhaps with a new tool
> that allows to assemble raw expressions, but I'm not sure its worth
> extending nft for this, the parser (and grammar) is already huge.
>
> > > If noone has any objections, I would place this on my todo list and
> > > start with adding to libnftnl the needed "expression is incomplete"
> > > marking by extending the .parse callbacks.
> > 
> > Maybe it is worth exploring what I propose above instead of displaying
> > "expression is incomplete"?
> 
> For cases where libnftnl is fine but nft lacks a human-readable name I
> think such @meta,42 would be fine.
> 
> But I don't think its good to allow decoding something arbitary, I think
> we would have to acknowledge that this can't be done unless you have
> a textual parser for raw netlink descriptions (i.e., full/unreadable TLV soup).

I agree such netlink binary dump should be last resort.

Re: let nftables indicate incomplete dissections

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > Add a "bool incomplete" to libnftnl strnct nftnl_expr, then set it
> > from each expr->parse callback if there is a new netlink attribute that
> > we did not understand.
> > 
> > nft then checks if this is incomplete-marker is set.
> 
> Is this sufficient? There are attribute values that could have an
> unknown/unsupported value, eg. exthdr type (assuming a new extension
> is supported).

No, but libnftnl can't known (and should not care) if nftables can
eat the value provided.

i.e., 'incomplete' means 'there were attributes that I did not understand',
nothing more.

> I am afraid setting incomplete for an unknown attribute also restricts
> netlink extensibility.

How so?

> Netlink allows for adding new attributes. Attributes also determine
> semantics, eg. exthdr type restricts the semantics of other existing
> attributes. There are also flags attributes which provide a hint on
> what is support or not, toggling such flag allows kernel to reject
> something that is not support.

But thats up to higher level tool, i.e. nftables?

I don't think libnftnl should try to guess if nftables can make
sense of the rule provided (or set, element, etc).

> > > > Related problem: entity that is using the raw netlink interface, it
> > > > that case libnftnl might be able to parse everything but nft could
> > > > lack the ability to properly print this.
> > > 
> > > There are two options here:
> > > 
> > > - Add more raw expressions and dump them, eg. meta@15, where 15 is the type.
> > >   This is more compact. If there is a requirement to allow to restore
> > >   this from older nftables versions, then it might be not enough since
> > >   maybe there is a need for meta@type,somethingelse (as in the ct direction
> > >   case).
> > 
> > Yes, for attributes that libnftnl knows about but where nft lacks a name
> > mapping (i.e., we can decode its META_KEY 0x42 but we have no idea what
> > that means we could in fact add such a representation scheme.
> > 
> > > - Use a netlink representation as raw expression: meta@1,3,0x0x000000004
> > >   but this requires dumping the whole list of attributes which is chatty.
> > 
> > Yes.  Perhaps its better to consider adding a new tool (script?) that
> > can dump the netlink soup without interpretation.  IIRC libmnl debug
> > already provides this functionality.
> > 
> > Very chatty but it would be good enough to figure out what such
> > hypothetical raw client did.
> 
> I see, a binary netlink dump format.

Yes, kinda like objump/readelf.