[riot-notifications] [RIOT-OS/RIOT] Address registration handling inappropriate (#15867)

tinstructor notifications at github.com
Wed Jan 27 12:57:03 CET 2021


#### Description
There are a few situations wherein the default behavior of a 6LR for handling an incoming registration attempt (i.e., processing a valid NS + ARO and SLLAO) seems inappropriate. For example, when a 6LR receives a valid NS with an ARO and SLLAO, and the ARO's Registration Lifetime is zero but the 6LR finds no matching NCE in its neighbor cache, it would make sense to simply ignore the NS (this is however debatable). Instead, it seems that a NA is returned, but without an ARO.  This behavior does not make sense in **any** case. If anything, when returning a NA in this case, it should at least contain an ARO with Status code 0, or a NA should not be sent in response at all.

#### Steps to reproduce the issue

In a [first step](https://github.com/RIOT-OS/RIOT/blob/master/sys/net/gnrc/network_layer/ipv6/nib/nib.c)  `_handle_nbr_sol()` is called when a NS arrives (line 310).
```c
void gnrc_ipv6_nib_handle_pkt(gnrc_netif_t *netif, const ipv6_hdr_t *ipv6,
                              const icmpv6_hdr_t *icmpv6, size_t icmpv6_len)
{
    ...
    switch (icmpv6->type) {
        ...
        case ICMPV6_NBR_SOL:
            _handle_nbr_sol(netif, ipv6, (ndp_nbr_sol_t *)icmpv6, icmpv6_len);
            break;
        ...
    }
    ...
}
```
[Second](https://github.com/RIOT-OS/RIOT/blob/master/sys/net/gnrc/network_layer/ipv6/nib/nib.c),  `_copy_and_handle_aro()` is called (line 994).
```c
static void _handle_nbr_sol(gnrc_netif_t *netif, const ipv6_hdr_t *ipv6,
                            const ndp_nbr_sol_t *nbr_sol, size_t icmpv6_len)
{
   ...
    if (ipv6_addr_is_unspecified(&ipv6->src)) {
        ...
    }
    else {
        ...
        FOREACH_OPT(nbr_sol, opt, tmp_len) {
            switch (opt->type) {
                case NDP_OPT_SL2A:
#if IS_ACTIVE(CONFIG_GNRC_IPV6_NIB_6LR)
                    if (gnrc_netif_is_6lr(netif)) {
                        DEBUG("nib: Storing SL2AO for later handling\n");
                        sl2ao = opt;
                        break;
                    }
#endif  /* CONFIG_GNRC_IPV6_NIB_6LR */
                    _handle_sl2ao(netif, ipv6, (const icmpv6_hdr_t *)nbr_sol,
                                  opt);
                    break;
#if IS_ACTIVE(CONFIG_GNRC_IPV6_NIB_6LR)
                case NDP_OPT_AR:
                    DEBUG("nib: Storing ARO for later handling\n");
                    aro = (sixlowpan_nd_opt_ar_t *)opt;
                    break;
#endif  /* CONFIG_GNRC_IPV6_NIB_6LR */
                default:
                    DEBUG("nib: Ignoring unrecognized option type %u for NS\n",
                          opt->type);
                    break;
            }
        }
        reply_aro = _copy_and_handle_aro(netif, ipv6, nbr_sol, aro, sl2ao);
        ...
        if (netif->ipv6.addrs_flags[tgt_idx] & GNRC_NETIF_IPV6_ADDRS_FLAGS_ANYCAST) {
            ...
        }
        else {
            gnrc_ndp_nbr_adv_send(&nbr_sol->tgt, netif, &ipv6->src,
                                  ipv6_addr_is_multicast(&ipv6->dst),
                                  reply_aro);
        }
    }
}
```
That's [where the issue lies](https://github.com/RIOT-OS/RIOT/blob/master/sys/net/gnrc/network_layer/ipv6/nib/_nib-6lr.c).  While the ` if (aro != NULL)` branch evaluates as `True`, `_handle_aro()` shall return a status of `_ADDR_REG_STATUS_IGNORE` but this case is not handled and hence `_copy_and_handle_aro(` returns a reply_aro of NULL. Besides that, it's also weird that a cache full status code does not seem to exist and the same issue arises when a new entry does need to be created but it isn't because the cache is full. In that case the debug statement even explicitly says that no NA is sent in response but that does in fact happen, albeit without an ARO, again. 
```c
gnrc_pktsnip_t *_copy_and_handle_aro(gnrc_netif_t *netif,
                                     const ipv6_hdr_t *ipv6,
                                     const ndp_nbr_sol_t *nbr_sol,
                                     const sixlowpan_nd_opt_ar_t *aro,
                                     const ndp_opt_t *sl2ao)
{
    gnrc_pktsnip_t *reply_aro = NULL;

    if (aro != NULL) {
        uint8_t status = _handle_aro(netif, ipv6, (icmpv6_hdr_t *)nbr_sol, aro,
                                     sl2ao, NULL);

        if ((status != _ADDR_REG_STATUS_TENTATIVE) &&
            (status != _ADDR_REG_STATUS_IGNORE)) {
            reply_aro = gnrc_sixlowpan_nd_opt_ar_build(status,
                                                       byteorder_ntohs(aro->ltime),
                                                       (eui64_t *)&aro->eui64,
                                                       NULL);
            if (reply_aro == NULL) {
                DEBUG("nib: No space left in packet buffer. Not replying NS");
            }
        }
#if IS_ACTIVE(CONFIG_GNRC_IPV6_NIB_MULTIHOP_DAD)
        else if (status != _ADDR_REG_STATUS_IGNORE) {
            DEBUG("nib: Address was marked TENTATIVE => not replying NS, "
                  "waiting for DAC\n");
        }
#endif  /* CONFIG_GNRC_IPV6_NIB_MULTIHOP_DAD */
    }
    return reply_aro;
}
```
For the rest of the flow, see following steps (in order)
[line 159 of _nib-6ln.c](https://github.com/RIOT-OS/RIOT/blob/master/sys/net/gnrc/network_layer/ipv6/nib/_nib-6ln.c)
[line 129 of _nib-6lr.c](https://github.com/RIOT-OS/RIOT/blob/master/sys/net/gnrc/network_layer/ipv6/nib/_nib-6lr.c)
[line 1000 of nib.c](https://github.com/RIOT-OS/RIOT/blob/master/sys/net/gnrc/network_layer/ipv6/nib/nib.c)


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/issues/15867
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210127/c68fbe9c/attachment-0001.htm>


More information about the notifications mailing list