Fix ioctl(2) code for OpenBSD #1175
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The address ioctl code is just plain broken. I fixed it for OpenBSD, but have no resources or time for FreeBSD, so there the code will stay as-is. FreeBSD also supports stuff like netlink, so perhaps a future rewrite will look different from OpenBSD's ioctl code (the only approach) anyway.
Start by deleting stuff we will not need or want in the end. This also seems to be the only code doing execve(2), which is best to be prevented from ever being called via pledge(2), but that is another story (I am already running yggdrasil with pledge(2) and unveil(2)).
Otherwise you get "panic: inappropriate ioctl for device", even for a perfectly fine ioctl(2) call.
See <netinet6/in6_var.h> and <netinet6/in6.h>, respectively.
SIOCSIFADDR_IN6 simply does not exist, on no system out there. SIOCSIFADDR exists, but long ago was deprecated by SIOCAIFADDR which is IPv4-only, see netintro(4). SIOCAIFADDR_IN6 is correct, but does uses a different struct, so bring that in, also. NB: The reason we have to handroll ioctl(2) ourselves is because golang.org/sys/unix does ship IPv6 ioctl and/or struct definitions. That should really be fixed upstream.
The net package has all we need and internally repesents addresses as byte arrays. <netinet6/in6.h> `struct in6_addr` is just a union over differnt sized byte arrays, so chosing u_int8_t[16] lets us access addresses byte-wise and thus no longer need to fiddle with multi-byte values and host vs. network byte order.
An ioctl request to set an address without gateway address or prefix mask is rather unproductive. Reuse code my moving it into a helper function; again the net package does all the parsing for us.
At this point, the ioctl success without any error, but no address appears on the interface. Turns out telling NDP to not expire helps. Congrats, yggdrasil is now operational on OpenBSD.
Direct syscalls are discouraged and usually do not work. (Not entirely sure why they do with Golang, but that is another story.)
klemensn
force-pushed
the
openbsd-ioctl
branch
from
6936a78 to
93b071e
Compare
Contributor
Author
|
Rebased onto develop as of now. |
Contributor
Author
|
Sadly, I just noticed you squashed all commits, thereby omitting all their messages, i.e. insights into what was wrong, from the commit history. |
klemensn
added a commit
to klemensn/yggdrasil-go
that referenced
this pull request
Oct 22, 2024
After yggdrasil-network#1175 removed ioctl(2) fallback code shelling out to ifconfig(8), there is no code left (compiled on OpenBSD) that would fork(2) or execve(2). Drop the ability to run any executable file to double down on this, thus reducing the attack surface of this this experimental, internet facing daemon running as root. pledge(2) is doable, but needs more polish. unveil(2), however, is as simple as it gets. On other systems, this code is a NOOP, but can still help to implement similar safety belts.
klemensn
added a commit
to klemensn/yggdrasil-go
that referenced
this pull request
Oct 22, 2024
After yggdrasil-network#1175 removed ioctl(2) fallback code shelling out to ifconfig(8), there is no code left (compiled on OpenBSD) that would fork(2) or execve(2). Drop the ability to run any executable file to double down on this, thus reducing the attack surface of this this experimental, internet facing daemon running as root. pledge(2) is doable, but needs more polish. unveil(2), however, is as simple as it gets. On other systems, this code is a NOOP, but can still help to implement similar safety belts.
klemensn
added a commit
to klemensn/yggdrasil-go
that referenced
this pull request
Nov 3, 2024
After yggdrasil-network#1175 removed ioctl(2) fallback code shelling out to ifconfig(8), there is no code left (compiled on OpenBSD) that would fork(2) or execve(2). Drop the ability to run any executable file to double down on this, thus reducing the attack surface of this this experimental, internet facing daemon running as root. pledge(2) is doable, but needs more polish. unveil(2), however, is as simple as it gets. On other systems, this code is a NOOP, but can still help to implement similar safety belts.
neilalexander
pushed a commit
that referenced
this pull request
Dec 12, 2024
After #1175 removed ioctl(2) fallback code shelling out to ifconfig(8), there is no code left (compiled on OpenBSD) that would fork(2) or execve(2). Drop the ability to run any executable file to double down on this, thus reducing the attack surface of this this experimental, internet facing daemon running as root. pledge(2) is doable, but needs more polish. unveil(2), however, is as simple as it gets. On other systems, this code is a NOOP, but can still help to implement similar safety belts.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This cleans up the mess to configure an IP address on a tun(4) device.
Handrolling a hardcoded ioctl(2) request is far from perfect, but Go (golang.org/sys/unix) is to blame here.
Tested on OpenBSD 7.6 -current where yggdrasil now drives the interface would use of ifconfig or other helpers.