On Fri, Apr 03, 2020 at 02:03:51PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Apr 02, 2020 at 11:49:41PM +0200, Pablo Neira Ayuso wrote:
> > This patch adds a lazy check to validate that the first element is not a
> > concatenation. The segtree code does not support for concatenations,
> > bail out with EOPNOTSUPP.
> >
> > # nft add element x y { 10.0.0.0/8 . 192.168.1.3-192.168.1.9 . 1024-65535 }
> > Error: Could not process rule: Operation not supported
> > add element x y { 10.0.0.0/8 . 192.168.1.3-192.168.1.9 . 1024-65535 }
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >
> > Otherwise, the segtree code barfs with:
> >
> > BUG: invalid range expression type concat
>
> Hm.
>
> I'm afraid this patch is not enough, the following ruleset crashes
> in old kernels with recent nft:
>
> flush ruleset
>
> table inet filter {
> set test {
> type ipv4_addr . ipv4_addr . inet_service
> flags interval,timeout
> elements = { 1.1.1.1 . 2.2.2.2 . 30 ,
> 2.2.2.2 . 3.3.3.3 . 40 ,
> 3.3.3.3 . 4.4.4.4 . 50 }
> }
>
> chain output {
> type filter hook output priority 0; policy accept;
> ip saddr . ip daddr . tcp dport @test counter
> }
> }
>
> Old kernel obviously does not support for concatenation with intervals.
>
> I think Stefano's NFT_SET_CONCAT flag needs to be restored in the
> kernel (see attached patch) to deal with old kernel properly.
>
> On set creation, old kernels bail out since they do not know what to
> do with this patch.
... with this flag. *sigh*
> Then, the next problem is that the kernel says -EINVAL in this case,
> this error code is bad. It should only be used for malformed messages,
> if NFT_SET_CONCAT is set on and kernel does not support this, it
> should instead just report EOPNOTSUPP (see the second patch in this
> email). I'm also updating the error code for the object maps since
> there might be a need to introduce new objects in the future.
>
> Thanks.
> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
> index 30f2a87270dc..4565456c0ef4 100644
> --- a/include/uapi/linux/netfilter/nf_tables.h
> +++ b/include/uapi/linux/netfilter/nf_tables.h
> @@ -276,6 +276,7 @@ enum nft_rule_compat_attributes {
> * @NFT_SET_TIMEOUT: set uses timeouts
> * @NFT_SET_EVAL: set can be updated from the evaluation path
> * @NFT_SET_OBJECT: set contains stateful objects
> + * @NFT_SET_CONCAT: set contains a concatenation
> */
> enum nft_set_flags {
> NFT_SET_ANONYMOUS = 0x1,
> @@ -285,6 +286,7 @@ enum nft_set_flags {
> NFT_SET_TIMEOUT = 0x10,
> NFT_SET_EVAL = 0x20,
> NFT_SET_OBJECT = 0x40,
> + NFT_SET_CONCAT = 0x80,
> };
>
> /**
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index d0ab5ffa1e2c..235762b91dd4 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -3961,7 +3961,7 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk,
> if (flags & ~(NFT_SET_ANONYMOUS | NFT_SET_CONSTANT |
> NFT_SET_INTERVAL | NFT_SET_TIMEOUT |
> NFT_SET_MAP | NFT_SET_EVAL |
> - NFT_SET_OBJECT))
> + NFT_SET_OBJECT | NFT_SET_CONCAT))
> return -EINVAL;
> /* Only one of these operations is supported */
> if ((flags & (NFT_SET_MAP | NFT_SET_OBJECT)) ==
> --
> 2.11.0
>
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index d0ab5ffa1e2c..63e38ac1c0f8 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -3962,7 +3962,7 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk,
> NFT_SET_INTERVAL | NFT_SET_TIMEOUT |
> NFT_SET_MAP | NFT_SET_EVAL |
> NFT_SET_OBJECT | NFT_SET_CONCAT))
> - return -EINVAL;
> + return -EOPNOTSUPP;
> /* Only one of these operations is supported */
> if ((flags & (NFT_SET_MAP | NFT_SET_OBJECT)) ==
> (NFT_SET_MAP | NFT_SET_OBJECT))
> @@ -4000,7 +4000,7 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk,
> objtype = ntohl(nla_get_be32(nla[NFTA_SET_OBJ_TYPE]));
> if (objtype == NFT_OBJECT_UNSPEC ||
> objtype > NFT_OBJECT_MAX)
> - return -EINVAL;
> + return -EOPNOTSUPP;
> } else if (flags & NFT_SET_OBJECT)
> return -EINVAL;
> else
> --
> 2.11.0
>
