* [PATCH nf] netfilter: nf_tables: report error if stateful obj's name is truncated
@ 2017-01-19 14:00 Liping Zhang
2017-01-19 14:09 ` Pablo Neira Ayuso
0 siblings, 1 reply; 6+ messages in thread
From: Liping Zhang @ 2017-01-19 14:00 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, Liping Zhang
From: Liping Zhang <zlpnobody@gmail.com>
Currently, if the user add a stateful object with the name size exceed
NFT_OBJ_MAXNAMELEN - 1 (i.e. 31), we truncate it down to 31 silently.
This is not friendly, furthermore, this will cause duplicated stateful
objects when the first 31 characters of the name is same. So limit the
stateful object's name size to NFT_OBJ_MAXNAMELEN - 1.
After apply this patch, error message will be printed out like this:
# name_32=$(printf "%0.sQ" {1..32})
# nft add counter filter $name_32
<cmdline>:1:1-52: Error: Could not process rule: Numerical result out
of range
add counter filter QQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQ
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Fixes: e50092404c1b ("netfilter: nf_tables: add stateful objects")
Signed-off-by: Liping Zhang <zlpnobody@gmail.com>
---
net/netfilter/nf_tables_api.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 091d2dc..9112ca9 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -4033,7 +4033,8 @@ EXPORT_SYMBOL_GPL(nf_tables_obj_lookup);
static const struct nla_policy nft_obj_policy[NFTA_OBJ_MAX + 1] = {
[NFTA_OBJ_TABLE] = { .type = NLA_STRING },
- [NFTA_OBJ_NAME] = { .type = NLA_STRING },
+ [NFTA_OBJ_NAME] = { .type = NLA_STRING,
+ .len = NFT_OBJ_MAXNAMELEN - 1 },
[NFTA_OBJ_TYPE] = { .type = NLA_U32 },
[NFTA_OBJ_DATA] = { .type = NLA_NESTED },
};
--
2.5.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH nf] netfilter: nf_tables: report error if stateful obj's name is truncated
2017-01-19 14:00 [PATCH nf] netfilter: nf_tables: report error if stateful obj's name is truncated Liping Zhang
@ 2017-01-19 14:09 ` Pablo Neira Ayuso
2017-01-19 14:41 ` Liping Zhang
0 siblings, 1 reply; 6+ messages in thread
From: Pablo Neira Ayuso @ 2017-01-19 14:09 UTC (permalink / raw)
To: Liping Zhang; +Cc: netfilter-devel, Liping Zhang
On Thu, Jan 19, 2017 at 10:00:20PM +0800, Liping Zhang wrote:
> From: Liping Zhang <zlpnobody@gmail.com>
>
> Currently, if the user add a stateful object with the name size exceed
> NFT_OBJ_MAXNAMELEN - 1 (i.e. 31), we truncate it down to 31 silently.
> This is not friendly, furthermore, this will cause duplicated stateful
> objects when the first 31 characters of the name is same. So limit the
> stateful object's name size to NFT_OBJ_MAXNAMELEN - 1.
>
> After apply this patch, error message will be printed out like this:
> # name_32=$(printf "%0.sQ" {1..32})
> # nft add counter filter $name_32
> <cmdline>:1:1-52: Error: Could not process rule: Numerical result out
> of range
> add counter filter QQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQ
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Good catch.
At quick glance, I can see other spots lacking this validation:
static const struct nla_policy nft_chain_policy[NFTA_CHAIN_MAX + 1] =
{
[NFTA_CHAIN_TABLE] = { .type = NLA_STRING },
Probably review and fix them in one go?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH nf] netfilter: nf_tables: report error if stateful obj's name is truncated
2017-01-19 14:09 ` Pablo Neira Ayuso
@ 2017-01-19 14:41 ` Liping Zhang
2017-01-19 15:22 ` Florian Westphal
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Liping Zhang @ 2017-01-19 14:41 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Liping Zhang, Netfilter Developer Mailing List
2017-01-19 22:09 GMT+08:00 Pablo Neira Ayuso <pablo@netfilter.org>:
> On Thu, Jan 19, 2017 at 10:00:20PM +0800, Liping Zhang wrote:
>> From: Liping Zhang <zlpnobody@gmail.com>
>>
>> Currently, if the user add a stateful object with the name size exceed
>> NFT_OBJ_MAXNAMELEN - 1 (i.e. 31), we truncate it down to 31 silently.
>> This is not friendly, furthermore, this will cause duplicated stateful
>> objects when the first 31 characters of the name is same. So limit the
>> stateful object's name size to NFT_OBJ_MAXNAMELEN - 1.
>>
>> After apply this patch, error message will be printed out like this:
>> # name_32=$(printf "%0.sQ" {1..32})
>> # nft add counter filter $name_32
>> <cmdline>:1:1-52: Error: Could not process rule: Numerical result out
>> of range
>> add counter filter QQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQ
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> Good catch.
>
> At quick glance, I can see other spots lacking this validation:
>
> static const struct nla_policy nft_chain_policy[NFTA_CHAIN_MAX + 1] =
> {
> [NFTA_CHAIN_TABLE] = { .type = NLA_STRING },
>
> Probably review and fix them in one go?
The nft table name's size is limited at this place:
static const struct nla_policy nft_table_policy[NFTA_TABLE_MAX + 1] = {
[NFTA_TABLE_NAME] = { .type = NLA_STRING,
.len =
NFT_TABLE_MAXNAMELEN - 1 },
If NFTA_CHAIN_TABLE's size exceeded 31, nf_tables_table_lookup will
fail eventually.
So I think adding the validation of NFTA_CHAIN_TABLE's size is not
important.
I had checked the table, chain, rule(no name), set, setelem(no name)
and object, I only found the validation of the object's name was missed.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH nf] netfilter: nf_tables: report error if stateful obj's name is truncated
2017-01-19 14:41 ` Liping Zhang
@ 2017-01-19 15:22 ` Florian Westphal
2017-01-19 15:55 ` Patrick PIGNOL
2017-01-21 11:47 ` Patrick PIGNOL
2 siblings, 0 replies; 6+ messages in thread
From: Florian Westphal @ 2017-01-19 15:22 UTC (permalink / raw)
To: Liping Zhang
Cc: Pablo Neira Ayuso, Liping Zhang, Netfilter Developer Mailing List
Liping Zhang <zlpnobody@gmail.com> wrote:
> > At quick glance, I can see other spots lacking this validation:
> >
> > static const struct nla_policy nft_chain_policy[NFTA_CHAIN_MAX + 1] =
> > {
> > [NFTA_CHAIN_TABLE] = { .type = NLA_STRING },
> >
> > Probably review and fix them in one go?
>
> The nft table name's size is limited at this place:
> static const struct nla_policy nft_table_policy[NFTA_TABLE_MAX + 1] = {
> [NFTA_TABLE_NAME] = { .type = NLA_STRING,
> .len =
> NFT_TABLE_MAXNAMELEN - 1 },
>
> If NFTA_CHAIN_TABLE's size exceeded 31, nf_tables_table_lookup will
> fail eventually.
>
> So I think adding the validation of NFTA_CHAIN_TABLE's size is not
> important.
Perhaps but its better to have it anyway so that you don't need this
extra context to understand that its limited in practice.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH nf] netfilter: nf_tables: report error if stateful obj's name is truncated
2017-01-19 14:41 ` Liping Zhang
2017-01-19 15:22 ` Florian Westphal
@ 2017-01-19 15:55 ` Patrick PIGNOL
2017-01-21 11:47 ` Patrick PIGNOL
2 siblings, 0 replies; 6+ messages in thread
From: Patrick PIGNOL @ 2017-01-19 15:55 UTC (permalink / raw)
To: Liping Zhang, Pablo Neira Ayuso
Cc: Liping Zhang, Netfilter Developer Mailing List
Hi all,
I KNOW that I will say something stupid but I'm thinking 'Why don't set NFT_OBJ_MAXNAMELEN to 255(or maybe size_t|SIZE_MAX|) on x86 ?'.
The true question that I can't answer now is : 'Why it is so stupid ?'
Best regards,
Patrick
Le 19/01/2017 à 15:41, Liping Zhang a écrit :
> 2017-01-19 22:09 GMT+08:00 Pablo Neira Ayuso <pablo@netfilter.org>:
>> On Thu, Jan 19, 2017 at 10:00:20PM +0800, Liping Zhang wrote:
>>> From: Liping Zhang <zlpnobody@gmail.com>
>>>
>>> Currently, if the user add a stateful object with the name size exceed
>>> NFT_OBJ_MAXNAMELEN - 1 (i.e. 31), we truncate it down to 31 silently.
>>> This is not friendly, furthermore, this will cause duplicated stateful
>>> objects when the first 31 characters of the name is same. So limit the
>>> stateful object's name size to NFT_OBJ_MAXNAMELEN - 1.
>>>
>>> After apply this patch, error message will be printed out like this:
>>> # name_32=$(printf "%0.sQ" {1..32})
>>> # nft add counter filter $name_32
>>> <cmdline>:1:1-52: Error: Could not process rule: Numerical result out
>>> of range
>>> add counter filter QQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQ
>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> Good catch.
>>
>> At quick glance, I can see other spots lacking this validation:
>>
>> static const struct nla_policy nft_chain_policy[NFTA_CHAIN_MAX + 1] =
>> {
>> [NFTA_CHAIN_TABLE] = { .type = NLA_STRING },
>>
>> Probably review and fix them in one go?
> The nft table name's size is limited at this place:
> static const struct nla_policy nft_table_policy[NFTA_TABLE_MAX + 1] = {
> [NFTA_TABLE_NAME] = { .type = NLA_STRING,
> .len =
> NFT_TABLE_MAXNAMELEN - 1 },
>
> If NFTA_CHAIN_TABLE's size exceeded 31, nf_tables_table_lookup will
> fail eventually.
>
> So I think adding the validation of NFTA_CHAIN_TABLE's size is not
> important.
>
> I had checked the table, chain, rule(no name), set, setelem(no name)
> and object, I only found the validation of the object's name was missed.
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH nf] netfilter: nf_tables: report error if stateful obj's name is truncated
2017-01-19 14:41 ` Liping Zhang
2017-01-19 15:22 ` Florian Westphal
2017-01-19 15:55 ` Patrick PIGNOL
@ 2017-01-21 11:47 ` Patrick PIGNOL
2 siblings, 0 replies; 6+ messages in thread
From: Patrick PIGNOL @ 2017-01-21 11:47 UTC (permalink / raw)
To: Liping Zhang, Pablo Neira Ayuso
Cc: Liping Zhang, Netfilter Developer Mailing List
Le 19/01/2017 à 15:41, Liping Zhang a écrit :
> The nft table name's size is limited at this place:
> static const struct nla_policy nft_table_policy[NFTA_TABLE_MAX + 1] = {
> [NFTA_TABLE_NAME] = { .type = NLA_STRING,
> .len =
> NFT_TABLE_MAXNAMELEN - 1 },
>
> If NFTA_CHAIN_TABLE's size exceeded 31, nf_tables_table_lookup will
> fail eventually.
>
Why ? :
typedef unsigned short __u16; // minimum MaxValue = 65535
/*
* <------- NLA_HDRLEN ------> <-- NLA_ALIGN(payload)-->
* +---------------------+- - -+- - - - - - - - - -+- - -+
* | Header | Pad | Payload | Pad |
* | (struct nlattr) | ing | | ing |
* +---------------------+- - -+- - - - - - - - - -+- - -+
* <-------------- nlattr->nla_len --------------> // Payload minimum
MaxValue = 65535 - (at least)32 = 65503
*/
struct nlattr {
__u16 nla_len;
__u16 nla_type;
};
#define NFT_SET_MAXNAMELEN 32 // <- Why 32 ? Why not 65535 or 65503
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-01-21 11:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-19 14:00 [PATCH nf] netfilter: nf_tables: report error if stateful obj's name is truncated Liping Zhang
2017-01-19 14:09 ` Pablo Neira Ayuso
2017-01-19 14:41 ` Liping Zhang
2017-01-19 15:22 ` Florian Westphal
2017-01-19 15:55 ` Patrick PIGNOL
2017-01-21 11:47 ` Patrick PIGNOL
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.