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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Thu, 7 May 2020 06:06:40 -0700
From:   William Tu <u9012063@...il.com>
To:     Eric Dumazet <eric.dumazet@...il.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>
Subject: Re: [PATCHv2] erspan: Add type I version 0 support.

On Wed, May 6, 2020 at 7:42 PM Eric Dumazet <eric.dumazet@...il.com> wrote:
>
>
>
> On 5/5/20 9:05 AM, William Tu wrote:
> > The Type I ERSPAN frame format is based on the barebones
> > IP + GRE(4-byte) encapsulation on top of the raw mirrored frame.
> > Both type I and II use 0x88BE as protocol type. Unlike type II
> > and III, no sequence number or key is required.
> > To creat a type I erspan tunnel device:
> >   $ ip link add dev erspan11 type erspan \
> >             local 172.16.1.100 remote 172.16.1.200 \
> >             erspan_ver 0
> >
> > Signed-off-by: William Tu <u9012063@...il.com>
> > ---
> > v2:
> >   remove the inline keyword, let compiler decide.
> > v1:
> > I didn't notice there is Type I when I did first erspan implementation
> > because it is not in the ietf draft 00 and 01. It's until recently I got
> > request for adding type I. Spec is below at draft 02:
> > https://tools.ietf.org/html/draft-foschiano-erspan-02#section-4.1
> >
> > To verify with Wireshark, make sure you have:
> > commit ef76d65fc61d01c2ce5184140f4b1bba0019078b
> > Author: Guy Harris <guy@...m.mit.edu>
> > Date:   Mon Sep 30 16:35:35 2019 -0700
> >
> >     Fix checks for "do we have an ERSPAN header?"
> > ---
> >  include/net/erspan.h | 19 +++++++++++++++--
> >  net/ipv4/ip_gre.c    | 58 ++++++++++++++++++++++++++++++++++++++--------------
> >  2 files changed, 60 insertions(+), 17 deletions(-)
> >
> > diff --git a/include/net/erspan.h b/include/net/erspan.h
> > index b39643ef4c95..0d9e86bd9893 100644
> > --- a/include/net/erspan.h
> > +++ b/include/net/erspan.h
> > @@ -2,7 +2,19 @@
> >  #define __LINUX_ERSPAN_H
> >
> >  /*
> > - * GRE header for ERSPAN encapsulation (8 octets [34:41]) -- 8 bytes
> > + * GRE header for ERSPAN type I encapsulation (4 octets [34:37])
> > + *      0                   1                   2                   3
> > + *      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> > + *     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> > + *     |0|0|0|0|0|00000|000000000|00000|    Protocol Type for ERSPAN   |
> > + *     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> > + *
> > + *  The Type I ERSPAN frame format is based on the barebones IP + GRE
> > + *  encapsulation (as described above) on top of the raw mirrored frame.
> > + *  There is no extra ERSPAN header.
> > + *
> > + *
> > + * GRE header for ERSPAN type II and II encapsulation (8 octets [34:41])
> >   *       0                   1                   2                   3
> >   *      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> >   *     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> > @@ -43,7 +55,7 @@
> >   * |                  Platform Specific Info                       |
> >   * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >   *
> > - * GRE proto ERSPAN type II = 0x88BE, type III = 0x22EB
> > + * GRE proto ERSPAN type I/II = 0x88BE, type III = 0x22EB
> >   */
> >
> >  #include <uapi/linux/erspan.h>
> > @@ -139,6 +151,9 @@ static inline u8 get_hwid(const struct erspan_md2 *md2)
> >
> >  static inline int erspan_hdr_len(int version)
> >  {
> > +     if (version == 0)
> > +             return 0;
> > +
> >       return sizeof(struct erspan_base_hdr) +
> >              (version == 1 ? ERSPAN_V1_MDSIZE : ERSPAN_V2_MDSIZE);
> >  }
> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> > index 029b24eeafba..e29cd48674d7 100644
> > --- a/net/ipv4/ip_gre.c
> > +++ b/net/ipv4/ip_gre.c
> > @@ -248,6 +248,15 @@ static void gre_err(struct sk_buff *skb, u32 info)
> >       ipgre_err(skb, info, &tpi);
> >  }
> >
> > +static bool is_erspan_type1(int gre_hdr_len)
> > +{
> > +     /* Both ERSPAN type I (version 0) and type II (version 1) use
> > +      * protocol 0x88BE, but the type I has only 4-byte GRE header,
> > +      * while type II has 8-byte.
> > +      */
> > +     return gre_hdr_len == 4;
> > +}
> > +
> >  static int erspan_rcv(struct sk_buff *skb, struct tnl_ptk_info *tpi,
> >                     int gre_hdr_len)
> >  {
> > @@ -262,17 +271,26 @@ static int erspan_rcv(struct sk_buff *skb, struct tnl_ptk_info *tpi,
> >       int len;
> >
> >       itn = net_generic(net, erspan_net_id);
> > -
> >       iph = ip_hdr(skb);
> > -     ershdr = (struct erspan_base_hdr *)(skb->data + gre_hdr_len);
> > -     ver = ershdr->ver;
> > -
> > -     tunnel = ip_tunnel_lookup(itn, skb->dev->ifindex,
> > -                               tpi->flags | TUNNEL_KEY,
> > -                               iph->saddr, iph->daddr, tpi->key);
> > +     if (is_erspan_type1(gre_hdr_len)) {
> > +             ver = 0;
> > +             tunnel = ip_tunnel_lookup(itn, skb->dev->ifindex,
> > +                                       tpi->flags | TUNNEL_NO_KEY,
> > +                                       iph->saddr, iph->daddr, 0);
> > +     } else {
> > +             ershdr = (struct erspan_base_hdr *)(skb->data + gre_hdr_len);
> > +             ver = ershdr->ver;
> > +             tunnel = ip_tunnel_lookup(itn, skb->dev->ifindex,
> > +                                       tpi->flags | TUNNEL_KEY,
> > +                                       iph->saddr, iph->daddr, tpi->key);
> > +     }
> >
> >       if (tunnel) {
> > -             len = gre_hdr_len + erspan_hdr_len(ver);
> > +             if (is_erspan_type1(gre_hdr_len))
> > +                     len = gre_hdr_len;
> > +             else
> > +                     len = gre_hdr_len + erspan_hdr_len(ver);
> > +
> >               if (unlikely(!pskb_may_pull(skb, len)))
> >                       return PACKET_REJECT;
> >
> > @@ -665,7 +683,10 @@ static netdev_tx_t erspan_xmit(struct sk_buff *skb,
> >       }
> >
> >       /* Push ERSPAN header */
> > -     if (tunnel->erspan_ver == 1) {
> > +     if (tunnel->erspan_ver == 0) {
> > +             proto = htons(ETH_P_ERSPAN);
> > +             tunnel->parms.o_flags &= ~TUNNEL_SEQ;
> > +     } else if (tunnel->erspan_ver == 1) {
> >               erspan_build_header(skb, ntohl(tunnel->parms.o_key),
> >                                   tunnel->index,
> >                                   truncate, true);
> > @@ -1066,7 +1087,10 @@ static int erspan_validate(struct nlattr *tb[], struct nlattr *data[],
> >       if (ret)
> >               return ret;
> >
> > -     /* ERSPAN should only have GRE sequence and key flag */
> > +     if (nla_get_u8(data[IFLA_GRE_ERSPAN_VER]) == 0)
>
> I do not see anything in the code making sure IFLA_GRE_ERSPAN_VER has been provided by the user ?
>
Hi Eric,

Thanks! at the iproute2 we always set the IFLA_GRE_ERSPAN_VER to 1 as
default when user
doesn't specify. But I should also make sure it is checked here for
non-iproute2 users.

If I understand correctly, I should add
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -1087,7 +1087,7 @@ static int erspan_validate(struct nlattr *tb[],
struct nlattr *data[],
        if (ret)
                return ret;

-       if (nla_get_u8(data[IFLA_GRE_ERSPAN_VER]) == 0)
+       if (data[IFLA_GRE_ERSPAN_VER] &&
nla_get_u8(data[IFLA_GRE_ERSPAN_VER]) == 0)
                return 0;

        /* ERSPAN type II/III should only have GRE sequence and key flag */

Regards,
William

Powered by blists - more mailing lists