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:   Tue, 15 Nov 2016 17:00:11 +0100
From:   Hannes Frederic Sowa <hannes@...essinduktion.org>
To:     Erik Nordmark <nordmark@...ic.net>, netdev@...r.kernel.org
Subject: Re: [PATCH net] ipv6 addrconf: Implemented enhanced DAD (RFC7527)

On 15.11.2016 08:57, Erik Nordmark wrote:
> Implemented RFC7527 Enhanced DAD.
> IPv6 duplicate address detection can fail if there is some temporary
> loopback of Ethernet frames. RFC7527 solves this by including a random
> nonce in the NS messages used for DAD, and if an NS is received with the
> same nonce it is assumed to be a looped back DAD probe and is ignored.
> RFC7527 is disabled by default. Can be enabled by setting either one of
> conf/{all,interface}/ipv6_rfc7527 to non-zero.
> 
> Signed-off-by: Erik Nordmark <nordmark@...sta.com>
> 
> Index: linux-stable/Documentation/networking/ip-sysctl.txt
> ===================================================================
> --- linux-stable.orig/Documentation/networking/ip-sysctl.txt
> +++ linux-stable/Documentation/networking/ip-sysctl.txt
> @@ -1713,6 +1713,15 @@ drop_unsolicited_na - BOOLEAN
> 
>      By default this is turned off.
> 
> +ipv6_rfc7527 - BOOLEAN

Could you rename the sysctl to enhanced_dad. As it will anyway show up
in the ipv6/ directory hierarchy you don't need to prefix it with 'ipv6_'

> +    Include a nonce option in the IPv6 neighbor solicitation messages
> used for
> +    duplicate address detection per RFC7527. A received DAD NS will
> only signal
> +    a duplicate address if the nonce is different. This avoids any false
> +    detection of duplicates due to loopback of the NS messages that we
> send.
> +    The nonce option will be sent on an interface if either one of
> +    conf/{all,interface}/ipv6_rfc7527 are TRUE.
> +    Default: FALSE
> +
>  icmp/*:
>  ratelimit - INTEGER
>      Limit the maximal rates for sending ICMPv6 packets.
> Index: linux-stable/include/linux/ipv6.h
> ===================================================================
> --- linux-stable.orig/include/linux/ipv6.h
> +++ linux-stable/include/linux/ipv6.h
> @@ -63,6 +63,7 @@ struct ipv6_devconf {
>      } stable_secret;
>      __s32        use_oif_addrs_only;
>      __s32        keep_addr_on_down;
> +    __u32        ipv6_rfc7527;

Same here, as above.

> 
>      struct ctl_table_header *sysctl_header;
>  };
> Index: linux-stable/include/net/if_inet6.h
> ===================================================================
> --- linux-stable.orig/include/net/if_inet6.h
> +++ linux-stable/include/net/if_inet6.h
> @@ -55,6 +55,7 @@ struct inet6_ifaddr {
>      __u8            stable_privacy_retry;
> 
>      __u16            scope;
> +    __u64            dad_nonce;
> 
>      unsigned long        cstamp;    /* created timestamp */
>      unsigned long        tstamp; /* updated timestamp */
> Index: linux-stable/include/net/ndisc.h
> ===================================================================
> --- linux-stable.orig/include/net/ndisc.h
> +++ linux-stable/include/net/ndisc.h
> @@ -31,6 +31,7 @@ enum {
>      ND_OPT_PREFIX_INFO = 3,        /* RFC2461 */
>      ND_OPT_REDIRECT_HDR = 4,    /* RFC2461 */
>      ND_OPT_MTU = 5,            /* RFC2461 */
> +    ND_OPT_NONCE = 14,              /* RFC7527 */
>      __ND_OPT_ARRAY_MAX,
>      ND_OPT_ROUTE_INFO = 24,        /* RFC4191 */
>      ND_OPT_RDNSS = 25,        /* RFC5006 */
> @@ -121,6 +122,7 @@ struct ndisc_options {
>  #define nd_opts_pi_end nd_opt_array[__ND_OPT_PREFIX_INFO_END]
>  #define nd_opts_rh            nd_opt_array[ND_OPT_REDIRECT_HDR]
>  #define nd_opts_mtu            nd_opt_array[ND_OPT_MTU]
> +#define nd_opts_nonce            nd_opt_array[ND_OPT_NONCE]
>  #define nd_802154_opts_src_lladdr
> nd_802154_opt_array[ND_OPT_SOURCE_LL_ADDR]
>  #define nd_802154_opts_tgt_lladdr
> nd_802154_opt_array[ND_OPT_TARGET_LL_ADDR]
> 
> @@ -398,7 +400,8 @@ void ndisc_cleanup(void);
>  int ndisc_rcv(struct sk_buff *skb);
> 
>  void ndisc_send_ns(struct net_device *dev, const struct in6_addr *solicit,
> -           const struct in6_addr *daddr, const struct in6_addr *saddr);
> +           const struct in6_addr *daddr, const struct in6_addr *saddr,
> +           u64 nonce);
> 
>  void ndisc_send_rs(struct net_device *dev,
>             const struct in6_addr *saddr, const struct in6_addr *daddr);
> Index: linux-stable/include/uapi/linux/ipv6.h
> ===================================================================
> --- linux-stable.orig/include/uapi/linux/ipv6.h
> +++ linux-stable/include/uapi/linux/ipv6.h
> @@ -177,6 +177,7 @@ enum {
>      DEVCONF_DROP_UNICAST_IN_L2_MULTICAST,
>      DEVCONF_DROP_UNSOLICITED_NA,
>      DEVCONF_KEEP_ADDR_ON_DOWN,
> +    DEVCONF_IPV6_RFC7527,

Ditto.

>      DEVCONF_MAX
>  };
> 
> Index: linux-stable/net/ipv6/addrconf.c
> ===================================================================
> --- linux-stable.orig/net/ipv6/addrconf.c
> +++ linux-stable/net/ipv6/addrconf.c
> @@ -217,6 +217,7 @@ static struct ipv6_devconf ipv6_devconf
>      .use_oif_addrs_only    = 0,
>      .ignore_routes_with_linkdown = 0,
>      .keep_addr_on_down    = 0,
> +    .ipv6_rfc7527           = 0,

What is your reason to not enable this by default? I haven't fully
studied the RFC, but it seems to be backwards compatible.

>  };
> 
>  static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
> @@ -262,6 +263,7 @@ static struct ipv6_devconf ipv6_devconf_
>      .use_oif_addrs_only    = 0,
>      .ignore_routes_with_linkdown = 0,
>      .keep_addr_on_down    = 0,
> +    .ipv6_rfc7527           = 0,
>  };
> 
>  /* Check if a valid qdisc is available */
> @@ -3722,12 +3724,18 @@ static void addrconf_dad_kick(struct ine
>  {
>      unsigned long rand_num;
>      struct inet6_dev *idev = ifp->idev;
> +    u64 nonce;
> 
>      if (ifp->flags & IFA_F_OPTIMISTIC)
>          rand_num = 0;
>      else
>          rand_num = prandom_u32() % (idev->cnf.rtr_solicit_delay ? : 1);
> 
> +    nonce = 0;
> +    if (ifp->idev->cnf.ipv6_rfc7527 ||
> + dev_net((ifp->idev)->dev)->ipv6.devconf_all->ipv6_rfc7527)

idev should already be in scope, so you can simplify this conditional.


> +        get_random_bytes(&nonce, 6);

Maybe:

do
	get_random_bytes(&nonce, 6);
while (!nonce);

> +    ifp->dad_nonce = nonce;
>      ifp->dad_probes = idev->cnf.dad_transmits;
>      addrconf_mod_dad_work(ifp, rand_num);
>  }
> @@ -3903,7 +3911,8 @@ static void addrconf_dad_work(struct wor
> 
>      /* send a neighbour solicitation for our addr */
>      addrconf_addr_solict_mult(&ifp->addr, &mcaddr);
> -    ndisc_send_ns(ifp->idev->dev, &ifp->addr, &mcaddr, &in6addr_any);
> +    ndisc_send_ns(ifp->idev->dev, &ifp->addr, &mcaddr, &in6addr_any,
> +              ifp->dad_nonce);
>  out:
>      in6_ifa_put(ifp);
>      rtnl_unlock();
> @@ -4937,6 +4946,7 @@ static inline void ipv6_store_devconf(st
>      array[DEVCONF_DROP_UNICAST_IN_L2_MULTICAST] =
> cnf->drop_unicast_in_l2_multicast;
>      array[DEVCONF_DROP_UNSOLICITED_NA] = cnf->drop_unsolicited_na;
>      array[DEVCONF_KEEP_ADDR_ON_DOWN] = cnf->keep_addr_on_down;
> +    array[DEVCONF_IPV6_RFC7527] = cnf->ipv6_rfc7527;
>  }
> 
>  static inline size_t inet6_ifla6_size(void)
> @@ -6027,6 +6037,13 @@ static const struct ctl_table addrconf_s
> 
>      },
>      {
> +        .procname       = "ipv6_rfc7527",
> +        .data           = &ipv6_devconf.ipv6_rfc7527,
> +        .maxlen         = sizeof(int),
> +        .mode           = 0644,
> +        .proc_handler   = proc_dointvec,
> +    },
> +    {
>          /* sentinel */
>      }
>  };
> Index: linux-stable/net/ipv6/ndisc.c
> ===================================================================
> --- linux-stable.orig/net/ipv6/ndisc.c
> +++ linux-stable/net/ipv6/ndisc.c
> @@ -234,6 +234,7 @@ struct ndisc_options *ndisc_parse_option
>          case ND_OPT_SOURCE_LL_ADDR:
>          case ND_OPT_TARGET_LL_ADDR:
>          case ND_OPT_MTU:
> +        case ND_OPT_NONCE:
>          case ND_OPT_REDIRECT_HDR:
>              if (ndopts->nd_opt_array[nd_opt->nd_opt_type]) {
>                  ND_PRINTK(2, warn,
> @@ -571,7 +572,8 @@ static void ndisc_send_unsol_na(struct n
>  }
> 
>  void ndisc_send_ns(struct net_device *dev, const struct in6_addr *solicit,
> -           const struct in6_addr *daddr, const struct in6_addr *saddr)
> +           const struct in6_addr *daddr, const struct in6_addr *saddr,
> +           u64 nonce)
>  {
>      struct sk_buff *skb;
>      struct in6_addr addr_buf;
> @@ -591,6 +593,8 @@ void ndisc_send_ns(struct net_device *de
>      if (inc_opt)
>          optlen += ndisc_opt_addr_space(dev,
>                             NDISC_NEIGHBOUR_SOLICITATION);
> +    if (nonce != 0)
> +        optlen += 8;
> 
>      skb = ndisc_alloc_skb(dev, sizeof(*msg) + optlen);
>      if (!skb)
> @@ -608,6 +612,13 @@ void ndisc_send_ns(struct net_device *de
>          ndisc_fill_addr_option(skb, ND_OPT_SOURCE_LL_ADDR,
>                         dev->dev_addr,
>                         NDISC_NEIGHBOUR_SOLICITATION);
> +    if (nonce != 0) {
> +        u8 *opt = skb_put(skb, 8);
> +
> +        opt[0] = ND_OPT_NONCE;
> +        opt[1] = 8 >> 3;
> +        memcpy(opt + 2, &nonce, 6);
> +    }
> 
>      ndisc_send_skb(skb, daddr, saddr);
>  }
> @@ -696,12 +707,12 @@ static void ndisc_solicit(struct neighbo
>                    "%s: trying to ucast probe in NUD_INVALID: %pI6\n",
>                    __func__, target);
>          }
> -        ndisc_send_ns(dev, target, target, saddr);
> +        ndisc_send_ns(dev, target, target, saddr, 0);
>      } else if ((probes -= NEIGH_VAR(neigh->parms, APP_PROBES)) < 0) {
>          neigh_app_ns(neigh);
>      } else {
>          addrconf_addr_solict_mult(target, &mcaddr);
> -        ndisc_send_ns(dev, target, &mcaddr, saddr);
> +        ndisc_send_ns(dev, target, &mcaddr, saddr, 0);
>      }
>  }
> 
> @@ -745,6 +756,7 @@ static void ndisc_recv_ns(struct sk_buff
>      int dad = ipv6_addr_any(saddr);
>      bool inc;
>      int is_router = -1;
> +    u64 nonce;
> 
>      if (skb->len < sizeof(struct nd_msg)) {
>          ND_PRINTK(2, warn, "NS: packet too short\n");
> @@ -789,6 +801,8 @@ static void ndisc_recv_ns(struct sk_buff
>              return;
>          }
>      }
> +    if (ndopts.nd_opts_nonce)
> +        memcpy(&nonce, (u8 *)(ndopts.nd_opts_nonce + 1), 6);

You only initialize 6 bytes of the nonce, with the other 2 being not
initialized.

> 
>      inc = ipv6_addr_is_multicast(daddr);
> 
> @@ -797,6 +811,16 @@ static void ndisc_recv_ns(struct sk_buff
>  have_ifp:
>          if (ifp->flags & (IFA_F_TENTATIVE|IFA_F_OPTIMISTIC)) {
>              if (dad) {
> +                if (nonce != 0 && ifp->dad_nonce == nonce) {
> +                    /* Matching nonce if looped back */
> +                    if (net_ratelimit())
> +                        ND_PRINTK(2, notice,
> +                              "%s: IPv6 DAD loopback for address %pI6c
> nonce %llu ignored\n",
> +                               ifp->idev->dev->name,
> +                               &ifp->addr,
> +                               nonce);

If we print the nonce for debugging reasons, we should keep it in
correct endianess on the wire vs. in the debug output.

> +                    goto out;
> +                }
>                  /*
>                   * We are colliding with another node
>                   * who is doing DAD
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ