Hi Arturo,
On Sat, Jun 22, 2013 at 12:44:09AM +0200, Arturo Borrero Gonzalez wrote:
> Hi Pablo,
>
> I already have a patch series to address yours requests.
> But, please, I need some more details in the next issues:
>
> 2013/6/20 Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>:
> >> diff --git a/test/xmlfiles/rule_exthdr.xml b/test/xmlfiles/rule_exthdr.xml
> >> new file mode 100644
> >> index 0000000..0abeb3c
> >> --- /dev/null
> >> +++ b/test/xmlfiles/rule_exthdr.xml
> >> @@ -0,0 +1,12 @@
> >> +<rule family="2" table="filter" chain="INPUT" handle="100" version="0">
> >> + <rule_flags>0</rule_flags>
> >> + <flags>127</flags>
> >> + <compat_flags>0</compat_flags>
> >> + <compat_proto>0</compat_proto>
> >> + <expr type="exthdr">
> >> + <dreg>123</dreg>
> >
> > fix.
> >
> >> + <type>15</type>
> >
> > Possibilities are defined by: nft_exthdr_attributes
>
> I did not understand this.
> In include/uapi/linux/netfilter/nf_tables.h I have:
>
> enum nft_exthdr_attributes {
> NFTA_EXTHDR_UNSPEC,
> NFTA_EXTHDR_DREG,
> NFTA_EXTHDR_TYPE,
> NFTA_EXTHDR_OFFSET,
> NFTA_EXTHDR_LEN,
> __NFTA_EXTHDR_MAX
> };
>
> What should I do with this?
Bad pointer, sorry. Possible types for exthdr are defined by certain
IPPROTO_*, in nft:
static const struct exthdr_desc *exthdr_protocols[IPPROTO_MAX] = {
[IPPROTO_HOPOPTS] = &exthdr_hbh,
[IPPROTO_ROUTING] = &exthdr_rt,
[IPPROTO_FRAGMENT] = &exthdr_frag,
[IPPROTO_DSTOPTS] = &exthdr_dst,
[IPPROTO_MH] = &exthdr_mh,
};
To simplify, you can try to generate a rule with nft that uses exthdr
and dump it via libnftables, so you don't need to make up the
testbench. Send a private email if you have any trouble with this.
> >> diff --git a/test/xmlfiles/rule_immediate.xml b/test/xmlfiles/rule_immediate.xml
> >> new file mode 100644
> >> index 0000000..a566ca5
> >> --- /dev/null
> >> +++ b/test/xmlfiles/rule_immediate.xml
> >> @@ -0,0 +1,31 @@
> >> +<rule family="2" table="filter" chain="INPUT" handle="100" version="0">
> >> + <rule_flags>0</rule_flags>
> >> + <flags>127</flags>
> >> + <compat_flags>0</compat_flags>
> >> + <compat_proto>0</compat_proto>
> >> + <expr type="immediate">
> >> + <dreg>1</dreg>
> >> + <immdata>
> >> + <data_reg type="value">
> >> + <len>1</len>
> >> + <data0>0xaabbccdd</data0>
> >
> > Lenghs says 1 byte, but I can see way more stuff there.
>
> mmmm,
>
> the XML node 'len' means how many '<data>' nodes we have.
>
> Then, the actual length of the data is somehow hard-coded in the lib
> and is calculated this way:
> data_reg.len = xml_node_len_value * sizeof(data_reg.val[0])
That data_reg.len should be the number of bytes, not the number of
registers in use. You have to fix that.
> Maybe is not fully implemented yet, but I already have an incoming
> patch to address this.
>
> >> diff --git a/test/xmlfiles/rule_log.xml b/test/xmlfiles/rule_log.xml
> >> new file mode 100644
> >> index 0000000..5471fee
> >> --- /dev/null
> >> +++ b/test/xmlfiles/rule_log.xml
> >> @@ -0,0 +1,12 @@
> >> +<rule family="2" table="filter" chain="INPUT" handle="100" version="0">
> >> + <rule_flags>0</rule_flags>
> >> + <flags>127</flags>
> >> + <compat_flags>0</compat_flags>
> >> + <compat_proto>0</compat_proto>
> >> + <expr type="log">
> >> + <group>123123121</group>
> >
> > possible groups are 0-65535.
>
> Maybe I should also change the group data type to uint16_t.
We have to fix that in kernel code as well. I'll take care of it, just
use a 2^16 value in your testbench files.
> >> diff --git a/test/xmlfiles/rule_nat.xml b/test/xmlfiles/rule_nat.xml
> >> new file mode 100644
> >> index 0000000..868be50
> >> --- /dev/null
> >> +++ b/test/xmlfiles/rule_nat.xml
> >> @@ -0,0 +1,22 @@
> >> +<rule family="2" table="filter" chain="INPUT" handle="100" version="0">
> >> + <rule_flags>0</rule_flags>
> >> + <flags>127</flags>
> >> + <compat_flags>0</compat_flags>
> >> + <compat_proto>0</compat_proto>
> >> + <expr type="nat">
> >> + <sreg_addr_min>1</sreg_addr_min>
> >> + <sreg_addr_max>1</sreg_addr_max>
> >
> > These above are IPv4 / IPv6 addresses. Should be printable ini
> > human readable format, you probably use inet_ntop for output and
> > inet_pton for input.
> >
> >> + <sreg_proto_min>1</sreg_proto_min>
> >> + <sreg_proto_max>1</sreg_proto_max>
> >
> > max here is 2^16 as they are port numbers.
> >
> >> + <family>AF_INET6</family>
> >
> > would be good to replace this by ip6.
> >
> >> + <type>NFT_NAT_DNAT</type>
> >
> > and this by dnat.
> >
> >> + </expr>
> >> + <expr type="nat">
> >
> > The ipv4 part is asking for a new file, add rule_nat-ipv4.xml and
> > rule_nat-ipv6.xml
> >
>
> I will do it. But I think that from the parsing point of view, is the
> same having two nat expr in one <rule> file.
Yes, the parser will take it. But from the semantic point of view it
does not make sense.
> >> diff --git a/test/xmlfiles/table2.xml b/test/xmlfiles/table2.xml
> >> new file mode 100644
> >> index 0000000..de8e570
> >> --- /dev/null
> >> +++ b/test/xmlfiles/table2.xml
> >> @@ -0,0 +1,6 @@
> >> +<table name="nat" version="0">
> >> + <properties>
> >> + <family>10</family>
> >> + <table_flags>123</table_flags>
> >
> > The only table flag is defined by enum nft_table_flags.
> >
>
> What if we have some kind of general validation functions?
>
> - The xml_parse() will just translate the XML to an object, with no
> additional validations.
> - The validate() will take care of values being 'real'.
>
> Example:
>
> int nft_table_validate(struct nft_table t)
> {
> /* validate family or return -1 */
> /* validate table_flags or return -1*/
> /* validate...maybe name length or return -1*/
> }
> EXPORT_SYMBOL(nft_table_validate);
>
> I think this may be useful both for userspace programs (who used
> _set() and _unset() funcs) and the JSON stuff.
> And it will not require so many lines of code.
> At the end, from inside the xml_parse() function we can call
> _validate() and only consider the parsing done if the object
> validates.
> Its a good idea? Should I work on this?
Let's revisit this later. The kernel will just bail out if you pass
some invalid configuration, that's just fine by now.
Just stick to fixing the issues I spotted and make sure we have a
realistic testbench.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
