On Sun, Dec 12 2021, Florian Obser <[email protected]> wrote:
> On 12 December 2021 21:19:21 CET, Jeremie Courreges-Anglas <[email protected]>
> wrote:
>>
>>dnsproc.c only returns a single address even if the code pretends to
>>support multiple addresses. This leads to weird behavior in edge cases,
>>as experienced by a user on IRC.
>>
>>Take a machine with both IPv4 and IPv6 addresses configured, but no
>>IPv4 default route (on purpose). Since there is at least one IPv4
>>address different from 127.0.0.1, AI_ADDRCONFIG doesn't filter out
>>A records. Let's encrypt ACME service is dual stacked but the first and
>>only address returned by dnsproc.c is always IPv4 with our "family inet
>>inet6". In this situation acme-client can't connect over IPv4 and errors
>>out even though there's a working IPv6 default route.
>>
>
> Doctor, Doctor! When I do this, it hurts!
Ah well, the user wasn't complaining, really. :)
I just got curious because the behavior and the code seemed strange.
>>I don't know much about ACME and its requirements / good practices for
>
> I can't think of a reason to not try all address families.
Also I can't think of a reason not to try multiple addresses in the same
address family.
>>clients, but clearly acme-client doesn't behave like most of our
>>programs which try to connect to all available addresses. This break
>>statement has been there since import, but was it added on purpose?
>>Input welcome.
>>
>>
>
> I probably won't be able to look at this for a week. I am very surprised that
> this is the correct fix though.
Well I was puzzled too, I stared perhaps 30 seconds trying to understand
what this statement was doing there. Its presence defeats the point of
the loop, variables, etc around it. To me this looks like a leftover
from initial experiments. The initial copy in ntpd/config.c doesn't
have this break statement.
> I trust you checked that multiple IP addresses can be passed between
> processes?
I added some extra debug statements and could see that 2 adresses (v4
and v6) were passed to netproc.c. I was able to reproduce the problem
of the original reporter, and see that the diff fixed renewing
a certificate.
>>diff --git a/usr.sbin/acme-client/dnsproc.c b/usr.sbin/acme-client/dnsproc.c
>>index 664ef8d9b8b..c4c612e521a 100644
>>--- a/usr.sbin/acme-client/dnsproc.c
>>+++ b/usr.sbin/acme-client/dnsproc.c
>>@@ -102,7 +102,6 @@ host_dns(const char *s, struct addr *vec)
>>
>> dodbg("%s: DNS: %s", s, vec[vecsz].ip);
>> vecsz++;
>>- break;
>> }
>>
>> freeaddrinfo(res0);
>>
>>
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
