Re: Garbled IPv6 printout
> A sockaddr is not meant to store the address, ... But the API wants a sockaddr. int accept(int sockfd, struct sockaddr *addr, socklen_t *addrlen); There is no hint in the man page that an IPv6 address won't fit. sockaddr ends with sa_data[14]; That's not big enough for an IPv6 address. I assume somebody suggested making it bigger and/or changing the parameters to accept when IPv6 was added. It would be interesting to review that discussion. Does anybody collect bugs of this nature? It would be interesting to review a list, partly to see what can be learned about API design and partly as a warning for when I might use a dangerous/trickey function. -- These are my opinions. I hate spam. ___ devel mailing list devel@ntpsec.org http://lists.ntpsec.org/mailman/listinfo/devel
Re: Garbled IPv6 printout
Yo Hal! On Sat, 30 Mar 2019 14:35:18 -0700 Hal Murray via devel wrote: > I just pushed a fix. It was an interesting quirk. Looks good to me: 2019-03-30T15:02:20 ntpd[5666]: NTSc: nts_probe connecting to kong.rellim.com:12 3 => [2001:470:e815:0:225:90ff:fef3:55da]:123 I also see the ntp server port from NTS: 2019-03-30T15:04:40 ntpd[31301]: NTSc: Using port 11123 > There is a tool. I forget the name. I'd probably run it valgrind, like this: valgrind --tool=memcheck ntpd -nN It yields live data like: ==8341== Conditional jump or move depends on uninitialised value(s) ==8341==at 0x4C35744: memmove (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==8341==by 0x13D875: memcpy (string_fortified.h:34) ==8341==by 0x13D875: extens_client_recv (nts_extens.c:362) ==8341==by 0x125BD5: receive (ntp_proto.c:719) ==8341==by 0x11C7AF: read_network_packet (ntp_io.c:2233) ==8341==by 0x11DE9D: input_handler (ntp_io.c:2363) ==8341==by 0x11DE9D: io_handler (ntp_io.c:2270) ==8341==by 0x12B69D: mainloop (ntpd.c:933) ==8341==by 0x12B69D: ntpdmain (ntpd.c:913) ==8341==by 0x12B7DC: main (ntpd.c:422) When you ^C you get a leak report like this: ==8341== ==8341== HEAP SUMMARY: ==8341== in use at exit: 75,519 bytes in 543 blocks ==8341== total heap usage: 5,007 allocs, 4,464 frees, 1,360,368 bytes allocated ==8341== ==8341== LEAK SUMMARY: ==8341==definitely lost: 47 bytes in 8 blocks ==8341==indirectly lost: 0 bytes in 0 blocks ==8341== possibly lost: 21,680 bytes in 6 blocks ==8341==still reachable: 53,792 bytes in 529 blocks ==8341== suppressed: 0 bytes in 0 blocks ==8341== Rerun with --leak-check=full to see details of leaked memory ==8341== ==8341== For counts of detected and suppressed errors, rerun with: -v ==8341== Use --track-origins=yes to see where uninitialised values come from ==8341== ERROR SUMMARY: 8641 errors from 513 contexts (suppressed: 0 from 0) RGDS GARY --- Gary E. Miller Rellim 109 NW Wilmington Ave., Suite E, Bend, OR 97703 g...@rellim.com Tel:+1 541 382 8588 Veritas liberabit vos. -- Quid est veritas? "If you can’t measure it, you can’t improve it." - Lord Kelvin pgprb1HqtTytl.pgp Description: OpenPGP digital signature ___ devel mailing list devel@ntpsec.org http://lists.ntpsec.org/mailman/listinfo/devel
Re: Garbled IPv6 printout
On Sat, Mar 30, 2019 at 02:35:18PM -0700, Hal Murray via devel wrote: > I just pushed a fix. It was an interesting quirk. The API for accepet > includes a pointer and length to a place to put the IP Address of the remote > site. The type of that place is struct sockaddr. sockaddr is generic, > presumably big enough for the biggest address format. They botched > something, > I'm not sure what. A sockaddr isn't big enough for an IPv6 address. A sockaddr is not meant to store the address, it just provides you with fields that are there for all variants like sockaddr_in, sockaddr_in6 and sockaddr_un, so that you can at least see which type of address it is. You should consider using sockaddr_storage. Kurt ___ devel mailing list devel@ntpsec.org http://lists.ntpsec.org/mailman/listinfo/devel
Garbled IPv6 printout
I just pushed a fix. It was an interesting quirk. The API for accepet includes a pointer and length to a place to put the IP Address of the remote site. The type of that place is struct sockaddr. sockaddr is generic, presumably big enough for the biggest address format. They botched something, I'm not sure what. A sockaddr isn't big enough for an IPv6 address. I also added a missing free in the openssl area. I'm not sure how many are still missing. Much of our stuff gets free-ed. There isn't a cleanup hook for the NTS stuff. Anybody good at tracking this stuff? There is a tool. I forget the name. I'd probably run it with a bare system, then again with several NTS clients and look for differences. Then similar for the server side. FreeBSD is still using OpenSSL 1.1.1a which doesn't work for making S2C and C2S. It does work if you force TLS1.2 -- These are my opinions. I hate spam. ___ devel mailing list devel@ntpsec.org http://lists.ntpsec.org/mailman/listinfo/devel
