Only set IPv6 scope id for link-local addresses

[jelson]

Problem

When resolving an IPv6 .local name, nss-mdns copies the interface index Avahi reports into sin6_scope_id for every returned address. That is correct and necessary for link-local addresses (fe80::/10), which cannot be used without a zone index. But RFC 4007 Sec 11.1 requires the scope id to be 0 for global-scope (and ULA) addresses.

A non-zero scope id on a global address produces a malformed address like 2001:db8::1%3. This breaks downstream consumers such as mount.nfs (see Debian #898527).

Fix

In convert_userdata_to_addrtuple(), gate the scope id assignment on IN6_IS_ADDR_LINKLOCAL and explicitly clear it to zero otherwise:

if (result->af == AF_INET6 &&
    IN6_IS_ADDR_LINKLOCAL(
        (const struct in6_addr*)result->address.ipv6.address))
    tuple->scopeid = result->scopeid;
else
    tuple->scopeid = 0;

Link-local resolution is unchanged the scope id is still emitted for fe80::/10, just not for global addresses. This matches what systemd-resolved already does (it only scopes link-local results).

Tests

Added test_userdata_to_addrtuple_scopeid covering both cases: a link-local address keeps its scope id, a global address has it zeroed. Also updated the existing validate_addrtuples assertion, since its fixtures use the global documentation prefix and therefore must now have a zero scope id. make check passes; reverting the fix makes the new test fail as expected.

Fixes #69
Refs Debian #898527 (https://bugs.debian.org/898527)

[evverx]

Thank you for the PR

for example ping6 fails on such an address

I don't think it's accurate. As far as I can see ping6 from iputils at least passes those non-zero sin_scope_ids along with ULAs to connect and sendto and it works. It works fine on FreeBSD too.

mount.nfs rejects it

That was indeed the only reason nss-resolved stopped always setting scope ids in systemd/systemd@27007ef and looking at systemd/systemd#4465 I'm still not sure where those errors come from. Did you manage to find the code responsible for rejecting those addresses?

convert_userdata_to_addrtuple

nss-mdns on FreeBSD doesn't use it and processes the result of _nss_mdns_gethostbyname_impl directly so this patch is applicable to Linux only. Is there any reason for that?

[jelson]

for example ping6 fails on such an address

I don't think it's accurate

Huh, the apparent failure of ping to a .local name with a global IPv6 address is what motivated me to fix this -- but now, having done another deep dive on it, I think there were other confounding factors. Regardless, nss should, of course, follow the RFC and never return a scope ID for any address that is not link-local.

I'm still not sure where those errors come from. Did you manage to find the code responsible for rejecting those addresses?

Admittedly this is not the symptom I was seeing, but I think the breakage comes from the kernel's RPC address parser: rpc_parse_scope_id() fails for any address carrying a %scope suffix that isn't IPV6_ADDR_LINKLOCAL, so rpc_pton() rejects it and the NFS addr= option parser bails out with NFS: Bad IP address specified. So the kernel already enforces RFC 4007 11.1 (a scope id is only valid on link-local addresses).

convert_userdata_to_addrtuple

nss-mdns on FreeBSD doesn't use it and processes the result of _nss_mdns_gethostbyname_impl directly so this patch is applicable to Linux only. Is there any reason for that?

Ah, this was just my mistake. I found #69 and one of the comments (from 6 years ago) had a tentative patch that was never made into a PR, that I was resurrecting. I've pushed a new version of the patch that moves the logic to the shared utility as you suggest.

[evverx]

I think the breakage comes from the kernel's RPC address parser

Thank you for the pointer. Even though it seems to have been confirmed in different places that it works without scope ids it was interesting to figure out where the addresses get rejected.

Unfortunately the nss-mdns CI here isn't useful in terms of its feedback but it's integrated into the avahi integration test suite and looks like Valgrind isn't happy there

==62952== 16 errors in context 1 of 5:
==62952== Conditional jump or move depends on uninitialised value(s)
==62952==    at 0x116B08: append_address_to_userdata (util.c:350)
==62952==    by 0x111A87: create_address_userdata (check_util.c:613)
==62952==    by 0x113CC7: test_userdata_for_name_to_hostent_smallest_buffer_eventually_works_6_fn (check_util.c:861)
==62952==    by 0x119387: tcase_run_tfun_nofork (in /tmp/tmp.tgVo6GpwuU/check_util)
==62952==    by 0x11978F: srunner_run_tagged (in /tmp/tmp.tgVo6GpwuU/check_util)
==62952==    by 0x114DE7: main (check_util.c:1102)
==62952==  Uninitialised value was created by a stack allocation
==62952==    at 0x111044: create_address_result (check_util.c:549)
==62952== 
==62952== 
==62952== 16 errors in context 2 of 5:
==62952== Conditional jump or move depends on uninitialised value(s)
==62952==    at 0x116B08: append_address_to_userdata (util.c:350)
==62952==    by 0x111A87: create_address_userdata (check_util.c:613)
==62952==    by 0x113AF3: test_userdata_for_name_to_hostent_smallest_buffer_eventually_works_4_fn (check_util.c:830)
==62952==    by 0x119387: tcase_run_tfun_nofork (in /tmp/tmp.tgVo6GpwuU/check_util)
==62952==    by 0x11978F: srunner_run_tagged (in /tmp/tmp.tgVo6GpwuU/check_util)
==62952==    by 0x114DE7: main (check_util.c:1102)
==62952==  Uninitialised value was created by a stack allocation
==62952==    at 0x111044: create_address_result (check_util.c:549)
==62952== 
==62952== 
==62952== 16 errors in context 3 of 5:
==62952== Conditional jump or move depends on uninitialised value(s)
==62952==    at 0x116B08: append_address_to_userdata (util.c:350)
==62952==    by 0x111A87: create_address_userdata (check_util.c:613)
==62952==    by 0x1138CB: test_userdata_for_name_to_hostent_buffer_too_small_returns_erange_fn (check_util.c:812)
==62952==    by 0x119387: tcase_run_tfun_nofork (in /tmp/tmp.tgVo6GpwuU/check_util)
==62952==    by 0x11978F: srunner_run_tagged (in /tmp/tmp.tgVo6GpwuU/check_util)
==62952==    by 0x114DE7: main (check_util.c:1102)
==62952==  Uninitialised value was created by a stack allocation
==62952==    at 0x111044: create_address_result (check_util.c:549)
==62952== 
==62952== 
==62952== 16 errors in context 4 of 5:
==62952== Conditional jump or move depends on uninitialised value(s)
==62952==    at 0x116B08: append_address_to_userdata (util.c:350)
==62952==    by 0x111A87: create_address_userdata (check_util.c:613)
==62952==    by 0x113783: test_userdata_for_name_to_hostent_returns_hostent_6_fn (check_util.c:796)
==62952==    by 0x119387: tcase_run_tfun_nofork (in /tmp/tmp.tgVo6GpwuU/check_util)
==62952==    by 0x11978F: srunner_run_tagged (in /tmp/tmp.tgVo6GpwuU/check_util)
==62952==    by 0x114DE7: main (check_util.c:1102)
==62952==  Uninitialised value was created by a stack allocation
==62952==    at 0x111044: create_address_result (check_util.c:549)
==62952== 
==62952== 
==62952== 16 errors in context 5 of 5:
==62952== Conditional jump or move depends on uninitialised value(s)
==62952==    at 0x116B08: append_address_to_userdata (util.c:350)
==62952==    by 0x111A87: create_address_userdata (check_util.c:613)
==62952==    by 0x113637: test_userdata_for_name_to_hostent_returns_hostent_4_fn (check_util.c:780)
==62952==    by 0x119387: tcase_run_tfun_nofork (in /tmp/tmp.tgVo6GpwuU/check_util)
==62952==    by 0x11978F: srunner_run_tagged (in /tmp/tmp.tgVo6GpwuU/check_util)
==62952==    by 0x114DE7: main (check_util.c:1102)
==62952==  Uninitialised value was created by a stack allocation
==62952==    at 0x111044: create_address_result (check_util.c:549)
==62952== 
==62952== ERROR SUMMARY: 80 errors from 5 contexts (suppressed: 0 from 0)
FAIL check_util (exit status: 1)

[jelson]

Valgrind isn't happy there

Thanks for the pointer - fix pushed!

[evverx]

I ran the avahi tests and as far as I can see getaddrinfo on FreeBSD now resets the scope ids of non-link-local addresses as well (and is consistent with the default unicast resolver in that regard). I also reproduced the NFS issue with a host advertising its ULA and can confirm that the issue is gone with this PR applied so all in all I think it's moving in the right direction.

For some reason the OpenScanHub job failed with

It was not possible to download the SRPMs needed for the differential scan

can you rebase the commit on top of the master branch to pick 8c1be77 where clang-analyzer was turned on and force-push the commit to trigger it again?

[jelson]

Rebased and pushed. Tests seem happy.

[evverx]

They do. The PR looks good to me. Merging.