Re: [PATCH net-next v2] seg6: add per-route tunnel source address

[Nicolas Dichtel <nicolas.dichtel@6wind.com>]

Le 13/03/2026 à 09:44, Justin Iurman a écrit :
> On Fri, Mar 13, 2026 at 2:02 AM Andrea Mayer <andrea.mayer@uniroma2.it> wrote:
>>
>> On Wed, 11 Mar 2026 16:28:45 +0100
>> Justin Iurman <justin.iurman@6wind.com> wrote:
[snip]

>>> @@ -702,13 +721,21 @@ static int seg6_build_state(struct net *net, struct nlattr *nla,
>>>       slwt = seg6_lwt_lwtunnel(newts);
>>>
>>>       err = dst_cache_init(&slwt->cache, GFP_ATOMIC);
>>> -     if (err) {
>>> -             kfree(newts);
>>> -             return err;
>>> -     }
>>> +     if (err)
>>> +             goto free_lwt_state;
>>>
>>>       memcpy(&slwt->tuninfo, tuninfo, tuninfo_len);
>>>
>>> +     if (tb[SEG6_IPTUNNEL_SRC]) {
>>> +             slwt->tunsrc = nla_get_in6_addr(tb[SEG6_IPTUNNEL_SRC]);
>>> +
>>> +             if (ipv6_addr_any(&slwt->tunsrc)) {
>>> +                     err = -EINVAL;
>>> +                     NL_SET_ERR_MSG(extack, "tunsrc cannot be ::");
>>> +                     goto free_dst_cache;
>>> +             }
>>> +     }
>>> +
>>
>> build_state() only rejects ::. Multicast or loopback will be silently
>> accepted, leading to an asymmetric black hole:
>>
>>  - seg6_input() path: encapsulated packet enters ip6_forward()
>>    which has the "security critical" saddr check; drop on the same
>>    node, packet never leaves;
>>  - seg6_output() path: goes through ip6_output() which has no saddr
>>    check; packet leaves, first transit drops it in ip6_rcv_core()
>>    or ip6_forward().
>>
>> Either way, silent black hole.
>>
>> I think we could reject these at build_state time, matching a similar
>> logic already present in ip6_forward(), e.g.:
>>
>>         if (ipv6_addr_any(&slwt->tunsrc) ||
>>             ipv6_addr_is_multicast(&slwt->tunsrc) ||
>>             ipv6_addr_loopback(&slwt->tunsrc)) {
>>                 NL_SET_ERR_MSG(extack, "invalid tunsrc address");
>>                 err = -EINVAL;
>>                 goto free_dst_cache;
>>         }
> 
> +1, makes sense.
> 
>> Whether to also reject link-local is debatable: ip6_forward() does
>> drop it, but is there a legitimate use case where a link-local tunnel
>> source would actually be needed?!
>> I'd argue we should reject link-local addresses as well.
>> For instance, using them as a tunnel source violates scope boundaries and
>> breaks ICMPv6 error reporting.
> 
> IMHO, I also think we could reasonably forbid LLs.Why rejecting LL? It's legal to use LL as a source address of a tunnel (and
forwarding packets in this tunnel). Is this forbidden with srv6?