Hi Florian,
On Tue, 27 Feb 2018 17:33:53 +0100
Florian Westphal <fw@xxxxxxxxx> wrote:
> Ahmed Abdelsalam <amsalam20@xxxxxxxxx> wrote:
> > Type 0 and 2 of the IPv6 Routing extension header are not handled
> > properly by exthdr_init_raw() in src/exthdr.c
> >
> > In order to fix the bug, we extended the "enum nft_exthdr_op" to
> > differentiate between rt, rt0, and rt2.
> >
> > This patch should fix the bug. We tested the patch against the
> > same configuration reported in the bug and the output is as
> > shown below.
> >
> > table ip6 filter {
> > chain input {
> > type filter hook input priority 0; policy accept;
> > rt0 addr[1] a::2
> > }
> > }
>
> I think this patch should be solved in userspace only.
>
>
> > + if (desc != NULL && desc->proto_key >= 0) {
> > + switch (desc->proto_key) {
> > + case 0:
> > + expr->exthdr.op = NFT_EXTHDR_OP_RT0;
>
> In particular, there is no need to store this in the kernel.
> I agree that doing it this way is easier, but still ...
>
> Here is a minimal patch.
>
> I write 'minimal' because it doesn't handle dependency correctly,
> but it should add correct rt0/rt2 (type was 0...) and also decode
> rt2 vs. hbh correctly.
>
I think Routing type 0, 2 and 4 (SRH) shouldn't be implemented as
an extension to General IPv6 routing header.
I agree they share some fields, but Routing header is just a template.
In real world, we use either routing type 0, 2, or 4.
I think, If I, as a user of nftables, want to write an nft rule for routing type_0,
I would prefer to write as below
$ nft add rule ip6 filter input rt0 nexthdr 6 rt0 seg-left 2 rt0 hdrlength rt0 addr [1]A::2
Instead, using the current implmentation, I will need to write half of the rule using rt
and the second half with rt0. something like
$ nft add rule ip6 filter input rt nexthdr 6 rt seg-left 2 rt hdrlength rt0 addr [1]A::2
If you agree, I think we should extend the templates of exthdr_rt0 and exthdr_rt2.
I can send another patch also for routing type 4.
> When asking for 'rt2' nft should insert a dependency for
> routing type 2, and similar, when asking for old rt0 it should add
> a check for rt type 0.
>
> Probably can be left as a future enhancement though.
>
> diff --git a/src/exthdr.c b/src/exthdr.c
> --- a/src/exthdr.c
> +++ b/src/exthdr.c
> @@ -156,13 +156,24 @@ void exthdr_init_raw(struct expr *expr, uint8_t type,
>
> if (expr->exthdr.desc == NULL)
> goto out;
> -
> +again:
> for (i = 0; i < array_size(expr->exthdr.desc->templates); i++) {
> tmpl = &expr->exthdr.desc->templates[i];
> if (tmpl->offset == offset && tmpl->len == len)
> goto out;
> }
>
> + if (type == IPPROTO_ROUTING) {
> + /* rt2/rt0 descriptions extend rt one */
> + if (expr->exthdr.desc == &exthdr_rt) {
> + expr->exthdr.desc = &exthdr_rt2;
> + goto again;
> + } else if (expr->exthdr.desc == &exthdr_rt2) {
> + expr->exthdr.desc = &exthdr_rt0;
> + goto again;
> + }
> + }
> +
> tmpl = &exthdr_unknown_template;
> out:
> expr->exthdr.tmpl = tmpl;
> @@ -236,6 +247,8 @@ const struct exthdr_desc exthdr_hbh = {
> */
>
> const struct exthdr_desc exthdr_rt2 = {
> + .name = "rt2",
> + .type = IPPROTO_ROUTING,
> .templates = {
> [RT2HDR_RESERVED] = {},
> [RT2HDR_ADDR] = {},
> @@ -246,6 +259,8 @@ const struct exthdr_desc exthdr_rt2 = {
> HDR_TEMPLATE(__name, __dtype, struct ip6_rthdr0, __member)
>
> const struct exthdr_desc exthdr_rt0 = {
> + .name = "rt0",
> + .type = IPPROTO_ROUTING,
> .templates = {
> [RT0HDR_RESERVED] = RT0_FIELD("reserved", ip6r0_reserved, &integer_type),
> [RT0HDR_ADDR_1] = RT0_FIELD("addr[1]", ip6r0_addr[0], &ip6addr_type),
> @@ -260,13 +275,6 @@ const struct exthdr_desc exthdr_rt0 = {
> const struct exthdr_desc exthdr_rt = {
> .name = "rt",
> .type = IPPROTO_ROUTING,
> -#if 0
> - .protocol_key = RTHDR_TYPE,
> - .protocols = {
> - [0] = &exthdr_rt0,
> - [2] = &exthdr_rt2,
> - },
> -#endif
> .templates = {
> [RTHDR_NEXTHDR] = RT_FIELD("nexthdr", ip6r_nxt, &inet_protocol_type),
> [RTHDR_HDRLENGTH] = RT_FIELD("hdrlength", ip6r_len, &integer_type),
Thanks,
Ahmed
--
Ahmed Abdelsalam <amsalam20@xxxxxxxxx>
--
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
