Page MenuHomeFreeBSD

libc: Import OpenBSD's inet_net_{ntop,pton}
ClosedPublic

Authored by ivy on Sep 19 2025, 8:12 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Oct 20, 2:35 AM
Unknown Object (File)
Mon, Oct 20, 2:34 AM
Unknown Object (File)
Mon, Oct 20, 2:34 AM
Unknown Object (File)
Mon, Oct 20, 2:34 AM
Unknown Object (File)
Mon, Oct 20, 2:34 AM
Unknown Object (File)
Mon, Oct 20, 2:34 AM
Unknown Object (File)
Mon, Oct 20, 2:34 AM
Unknown Object (File)
Sun, Oct 19, 12:13 PM
Subscribers

Details

Summary

Our versions of these functions (originally taken from BIND) simply
don't work correctly for AF_INET6. These were removed from BIND itself
quite a while ago, but OpenBSD has made quite a few fixes in the mean
time, so import their code.

Add tests for both functions.

MFC after: 1 week
Obtained from: OpenBSD (lib/libc/net)
Reported by: Nico Sonack <nsonack@herrhotzenplotz.de>
PR: 289198

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ivy requested review of this revision.Sep 19 2025, 8:12 PM

inet_net_test: add a test for an overlong prefix

the comment says we want to test this, and we do, so actually test it.

add tests for invalid input

Oh, that's a lot to review :) Thanks a lot for creating the tests. Quick question: do the tests pass on the previous implementation?

do the tests pass on the previous implementation?

no. as the reporter mentioned in the PR, our current implementation is completely broken -- it almost never produces valid output. i'm not sure how no one noticed that, but i assume these functions just aren't very widely used (i'd never even heard of them until i looked at the PR).

fix a few nits in inet_net_test, and while here, add the full license text to the header (for legal reasons, my new preference for files i create)

This change touches inet_net_ntop_ipv4(). I think this deserves careful review.

Nothing uses these functions, they are non-standard and never worked properly. Even BIND (which is where we got them) didn't use them.

lib/libc/inet/inet_net_ntop.c
73

This is wrong.

lib/libc/inet/inet_net_pton.c
177

If these are currently broken and do not work we should just not support net classes.

lib/libc/inet/inet_net_pton.c
177

this change is not related to network classes, which don't exist in IPv6 at all. as far as i'm aware our current implementation supports classes okay.

i've written a standalone test case for IPv4: https://people.freebsd.org/~ivy/tmp/inet.tar.Z

usage:

% make
% ./test

this runs both the FreeBSD and OpenBSD versions of inet_net_{pton,ntop} on every possible IP address and netmask combination and checks for any differences in return value. due to the number of addresses involved, it's quite slow, but i'm currently running it on universe16b and it's processes enough addresses that i'm reasonably confident the output is already the same.

this doesn't cover the IPv6 case, but since our IPv6 implementation is completely broken anyway, it can't really get worse.

ivy marked an inline comment as done.Fri, Oct 17, 9:29 AM
This revision is now accepted and ready to land.Fri, Oct 17, 11:34 AM
lib/libc/inet/inet_net_pton.c
177

Oh, currently completely broken for IPv6 only. OK.

Your run on entire IPv4 space suggested an extra test, that would be really nice:

  • generate a salt with arc4random and print it.
  • using the seed and nrand48() generate a set of IP addresses, maybe 1000
  • run inet_ntop() and then inet_pton() and make sure that they translate forth and back to original

Please don't consider this idea as a blocking requirement to commit. You may add it later or you may ignore this idea.

Your run on entire IPv4 space suggested an extra test, that would be really nice:

  • generate a salt with arc4random and print it.
  • using the seed and nrand48() generate a set of IP addresses, maybe 1000
  • run inet_ntop() and then inet_pton() and make sure that they translate forth and back to original

i'm not sure we want non-deterministic test cases; aside from anything, this would probably confuse Jenkins when it's trying to bisect a failure. (not that it's much good at that anyway, but we may as well not make it worse...)

some test frameworks, like Catch2, allow you to use a specific random seed to make test failures reproducible, but i don't know if this have this in Kyua at all, or if any of our CI runners support it.

if no one has an objection, i plan to land this in the next day or two so we can get some testing (to the extent that anything even uses these functions) and hopefully sneak it into 15.0-BETA3

This revision was automatically updated to reflect the committed changes.