netdev - Re: [PATCH net 1/5] net: ip_tunnel: add header

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 28 Jun 2020 16:41:21 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     "Jason A. Donenfeld" <Jason@...c4.com>
Cc:     Network Development <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>
Subject: Re: [PATCH net 1/5] net: ip_tunnel: add header_ops for layer 3 devices

On Sat, Jun 27, 2020 at 4:07 AM Jason A. Donenfeld <Jason@...c4.com> wrote:
>
> Some devices that take straight up layer 3 packets benefit from having a
> shared header_ops so that AF_PACKET sockets can inject packets that are
> recognized. This shared infrastructure will be used by other drivers
> that currently can't inject packets using AF_PACKET. It also exposes the
> parser function, as it is useful in standalone form too.
>
> Signed-off-by: Jason A. Donenfeld <Jason@...c4.com>
> ---
>  include/net/ip_tunnels.h  |  3 +++
>  net/ipv4/ip_tunnel_core.c | 19 +++++++++++++++++++
>  2 files changed, 22 insertions(+)
>
> diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
> index 076e5d7db7d3..36025dea7612 100644
> --- a/include/net/ip_tunnels.h
> +++ b/include/net/ip_tunnels.h
> @@ -290,6 +290,9 @@ int ip_tunnel_newlink(struct net_device *dev, struct nlattr *tb[],
>                       struct ip_tunnel_parm *p, __u32 fwmark);
>  void ip_tunnel_setup(struct net_device *dev, unsigned int net_id);
>
> +extern const struct header_ops ip_tunnel_header_ops;
> +__be16 ip_tunnel_parse_protocol(const struct sk_buff *skb);
> +
>  struct ip_tunnel_encap_ops {
>         size_t (*encap_hlen)(struct ip_tunnel_encap *e);
>         int (*build_header)(struct sk_buff *skb, struct ip_tunnel_encap *e,
> diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
> index 181b7a2a0247..07d958aa03f8 100644
> --- a/net/ipv4/ip_tunnel_core.c
> +++ b/net/ipv4/ip_tunnel_core.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
>   * Copyright (c) 2013 Nicira, Inc.
> + * Copyright (C) 2020 Jason A. Donenfeld <Jason@...c4.com>. All Rights Reserved.
>   */

It's not customary to append these when touching existing files.
Though I'm not sure what the policy is.

>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> @@ -844,3 +845,21 @@ void ip_tunnel_unneed_metadata(void)
>         static_branch_dec(&ip_tunnel_metadata_cnt);
>  }
>  EXPORT_SYMBOL_GPL(ip_tunnel_unneed_metadata);
> +
> +/* Returns either the correct skb->protocol value, or 0 if invalid. */
> +__be16 ip_tunnel_parse_protocol(const struct sk_buff *skb)
> +{
> +       if (skb_network_header(skb) >= skb->head &&
> +           (skb_network_header(skb) + sizeof(struct iphdr)) <= skb_tail_pointer(skb) &&
> +           ip_hdr(skb)->version == 4)
> +               return htons(ETH_P_IP);
> +       if (skb_network_header(skb) >= skb->head &&
> +           (skb_network_header(skb) + sizeof(struct ipv6hdr)) <= skb_tail_pointer(skb) &&
> +           ipv6_hdr(skb)->version == 6)
> +               return htons(ETH_P_IPV6);
> +       return 0;

Perhaps worth stressing that this works in a closed environment that
only handles IP, but not for arbitrary L3 tunnel devices that may
accept a broader set of inner protocols.

Even the ipip device supports MPLS as of commit 1b69e7e6c4da ("ipip:
support MPLS over IPv4"). When injecting MPLS + IP over packet
sockets, a label could easily be misinterpreted. That said, such
processes should pass the protocol explicitly to avoid
misclassification.

Applied strictly to ipip, ip6ip6 this definitely improves packet
socket support for those, and is similar to what tun does:

                if (tun->flags & IFF_NO_PI) {
                        u8 ip_version = skb->len ? (skb->data[0] >> 4) : 0;

Sit is another candidate.



> +}
> +EXPORT_SYMBOL(ip_tunnel_parse_protocol);
> +
> +const struct header_ops ip_tunnel_header_ops = { .parse_protocol = ip_tunnel_parse_protocol };
> +EXPORT_SYMBOL(ip_tunnel_header_ops);
> --
> 2.27.0
>