Change of plans... I went throught the code and it seems to handle multiple addresses correct through all the calls / processes.
(It is beyond me why it passes IP addresses as strings around though.) OK florian On 2021-12-13 01:56 +01, Jeremie Courreges-Anglas <[email protected]> wrote: > 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 > -- I'm not entirely sure you are real.
