Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Binding to an interface on MacOS does not function correctly #378

Closed
NonLogicalDev opened this issue Apr 1, 2020 · 16 comments · Fixed by #423
Closed

[BUG] Binding to an interface on MacOS does not function correctly #378

NonLogicalDev opened this issue Apr 1, 2020 · 16 comments · Fixed by #423

Comments

@NonLogicalDev
Copy link

NonLogicalDev commented Apr 1, 2020

When picking an interface on MacOS, the server will correctly listen for packets, but when doing an offer by replying to broadcast address, the system will route that to the Gateway interface only.

This is likely due to:

// BindToInterface emulates linux's SO_BINDTODEVICE option for a socket by using
// IP_RECVIF.
func BindToInterface(fd int, ifname string) error {
	iface, err := net.InterfaceByName(ifname)
	if err != nil {
		return err
	}
	return syscall.SetsockoptInt(fd, syscall.IPPROTO_IP, syscall.IP_RECVIF, iface.Index)
}

I am hardly an expert in stuff this low in the stack, though from the looks of it this is just filtering the received traffic, but does nothing to direct the outbound traffic.

I have also found this, perhaps that might be the solution: 🤷‍♂ :

https://stackoverflow.com/a/57013928

There is IP_BOUND_IF socket option
int idx = if_nametoindex("en0");
setsockopt(sockfd, IPPROTO_IP, IP_BOUND_IF, &idx, sizeof(idx))

Spent quite a few hours with Wireshark trying to figure out why the server was not sending an offer to the client, all the while not raising any alarms.

Will add a minimum viable example to illustrate the issue soon.

@Natolumin
Copy link
Collaborator

(For reference, I did some research on this issue in #316 and coredhcp/coredhcp#52 (comment) )

I think you're right and it needs {IP,IP6}_BOUND_IF, in addition to IP_RECVIF
The problem is, there is no testing for macOS and none of the main developers of this library use it that I know of. If you have the hardware available to run some tests, help would be welcome to test some patches for this, but I should warn you that you might encounter bugs in other places as well

@NonLogicalDev
Copy link
Author

NonLogicalDev commented Apr 1, 2020

@Natolumin I do have a few MacOS machines a linux host, and a few RaspberryPIs lying around.
I would gladly participate, and learn more about DHCP in the process.

@Natolumin
Copy link
Collaborator

I found some time to write a patch to try the IP_BOUND_IF option. Could you try this branch: https://github.com/Natolumin/dhcp/tree/darwin_bindtointerface ?

Note while trying this I found that client4/nclient4 don't even build on darwin so yeah, our testing isn't great there

Here are some instructions for using the fork if you ever need them, I hope they're correct (feel free to ignore them if you know how to do this):

I think you should be able to test it with the following:

cd $GOPATH/src/github.com/insomniacslk/dhcp
git remote add natolumin_fork https://github.com/Natolumin/dhcp
git fetch natolumin_fork
git checkout darwin_bindtointerface

Then build your downstream application as usual.

If you use modules in your downstream binary, you'll need to checkout the fork somewhere and create a go.mod file in the fork with go mod init, then add a replace directive in the go.mod of your own application:

replace github.com/insomniacslk/dhcp => ../path/to/forked/library

@NonLogicalDev
Copy link
Author

@Natolumin I will work on it this weekend if time allows, sorry for the delay.

@pmazzini
Copy link
Collaborator

@NonLogicalDev Were you able to give this a try?

@NonLogicalDev
Copy link
Author

NonLogicalDev commented Mar 13, 2021

Hey @pmazzini / @Natolumin Appologies for admittedly long time to get this tested. I did test it though and is seems to be working with a small caveat:

In this snippet:
https://github.com/Natolumin/dhcp/blob/darwin_bindtointerface/interfaces/bindtodevice_darwin.go

This is actually slightly wrong:

	socktype, err := unix.GetsockoptInt(fd, unix.SOL_SOCKET, unix.SO_TYPE)
	if err != nil {
		return fmt.Errorf("Could not decide socket address type: %w", err)
	}
	switch socktype {
	case unix.IPPROTO_IP:
		err = unix.SetsockoptInt(fd, unix.IPPROTO_IP, unix.IP_BOUND_IF, iface.Index)
	case unix.IPPROTO_IPV6:
		err = unix.SetsockoptInt(fd, unix.IPPROTO_IPV6, unix.IPV6_BOUND_IF, iface.Index)
	default:
		err = fmt.Errorf("Unsupported address family %d", socktype)

The unix.SOL_SOCKET, unix.SO_TYPE return the socket type, not proto or domain. It will actually return unix.SOCK_STREAM or unix.SOCK_DGRAM.

AFAIK there is no way that I know of to tease out the domain information out of socket FD on darwin. =[ The documentation is either missing or it is just not supported via socket options.

Here is Linux for reference: (https://man7.org/linux/man-pages/man7/socket.7.html)

       SO_DOMAIN (since Linux 2.6.32)
              Retrieves the socket domain as an integer, returning a
              value such as AF_INET6.  See socket(2) for details.  This
              socket option is read-only.

And darwin:
https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/getsockopt.2.html

Which only mentions SO_TYPE (which as I established does not return what we want).

The good news though, if I change the code by taking out the switch statement and only leaving the IPPROTO_IP socket option (essentially hardcoding the IPV4 only operation).

		err = unix.SetsockoptInt(fd, unix.IPPROTO_IP, unix.IP_BOUND_IF, iface.Index)

Diff:

diff --git a/interfaces/bindtodevice_darwin.go b/interfaces/bindtodevice_darwin.go
index f8aa4a1..66abc01 100644
--- a/interfaces/bindtodevice_darwin.go
+++ b/interfaces/bindtodevice_darwin.go
@@ -20,18 +20,6 @@ func BindToInterface(fd int, ifname string) error {
                return fmt.Errorf("Could not bind to an interface (receive path): %w", err)
        }

-       socktype, err := unix.GetsockoptInt(fd, unix.SOL_SOCKET, unix.SO_TYPE)
-       if err != nil {
-               return fmt.Errorf("Could not decide socket address type: %w", err)
-       }
-       switch socktype {
-       case unix.IPPROTO_IP:
-               err = unix.SetsockoptInt(fd, unix.IPPROTO_IP, unix.IP_BOUND_IF, iface.Index)
-       case unix.IPPROTO_IPV6:
-               err = unix.SetsockoptInt(fd, unix.IPPROTO_IPV6, unix.IPV6_BOUND_IF, iface.Index)
-       default:
-               err = fmt.Errorf("Unsupported address family %d", socktype)
-       }
-
+       err = unix.SetsockoptInt(fd, unix.IPPROTO_IP, unix.IP_BOUND_IF, iface.Index)
        return err
 }

Then it WORKS!!! I tested it and my RPI I connected to a USB Ethernet adapter got its IP just fine. So that can totally be used as a starting point for a more production ready code, but we still need to figure out how to correctly apply either:

unix.SetsockoptInt(fd, unix.IPPROTO_IP, unix.IP_BOUND_IF, iface.Index)
or
unix.SetsockoptInt(fd, unix.IPPROTO_IPV6, unix.IPV6_BOUND_IF, iface.Index)

Which is going to be the hard part... Unless there is someone with Darwin Fu who knows how to tease this information out of the file descriptor.

Update:

Found this:

https://github.com/apple/darwin-xnu/blob/8f02f2a044b9bb1ad951987ef5bab20ec9486310/bsd/netinet6/ip6_output.c?ts=4#L2359

	struct inpcb *in6p = sotoinpcb(so);

https://github.com/apple/darwin-xnu/blob/8f02f2a044b9bb1ad951987ef5bab20ec9486310/bsd/netinet6/ip6_output.c?ts=4#L2779

			case IPV6_BOUND_IF:
				/* This option is settable only on IPv6 */
				if (!(in6p->inp_vflag & INP_IPV6)) {
					error = EINVAL;
					break;
				}

Perhaps if someone can find if that in6p->inp_vflag is in any shape or form exposed via some syscall.


Here is the gist that details my test harness:
https://gist.github.com/NonLogicalDev/f7d1afdac2097f40997a194df60d59e1

@pmazzini
Copy link
Collaborator

@NonLogicalDev Thanks for getting back to this and testing it.

The good news though, if I change the code by taking out the switch statement and only leaving the IPPROTO_IP socket option (essentially hardcoding the IPV4 only operation).

Ok, this is a starting point. Better IPv4 than nothing.

If I am not mistaken the BSD BindToInterface is also IPv4 only: https://github.com/insomniacslk/dhcp/blob/master/interfaces/bindtodevice_bsd.go#L17

I tested it and my RPI

Do you have Darwin on a RPI?

Also thanks @Natolumin for looking at this. Would wither of you like to submit a PR?

@NonLogicalDev
Copy link
Author

NonLogicalDev commented Mar 13, 2021

@pmazzini RPI just has a basic debian linux on it. It is just nice to debug since it has a serial interface and network stack that is easier to introspect =]. It fulfills the role of a client.

I am running that Sever detailed in the Gist on a Macbook.

@NonLogicalDev
Copy link
Author

In regards to BSD, we are talking about the IP_BOUND_IF / IPV6_BOUND_IF which have different versions for ipv4 vs ipv6. IP_RECVIF weirdly does not have IPV6_RECVIF counterpart... 🤷

@NonLogicalDev
Copy link
Author

We could take a naive approach and attempt to set both, and see which one does not return an error.

@NonLogicalDev
Copy link
Author

Here is what you get for two scenarios, when IPV4 is setup on socket:

	err = unix.SetsockoptInt(fd, unix.IPPROTO_IPV6, unix.IPV6_BOUND_IF, iface.Index)
	if err != nil {
		panic(err)
	}
panic: invalid argument

goroutine 1 [running]:
github.com/insomniacslk/dhcp/interfaces.BindToInterface(0x3, 0x1119505, 0x3, 0xc000052dd4, 0x4)
	/Users/nonlogical/Documents/Developer/Mirror/github.com/insomniacslk/dhcp/interfaces/bindtodevice_darwin.go:38 +0x225
github.com/insomniacslk/dhcp/dhcpv4.BindToInterface(...)
	/Users/nonlogical/Documents/Developer/Mirror/github.com/insomniacslk/dhcp/dhcpv4/bindtointerface.go:9
github.com/insomniacslk/dhcp/dhcpv4/server4.NewIPv4UDPConn(0x1119505, 0x3, 0xc000052f48, 0x0, 0x0, 0x0)
	/Users/nonlogical/Documents/Developer/Mirror/github.com/insomniacslk/dhcp/dhcpv4/server4/conn.go:36 +0x7ea
github.com/insomniacslk/dhcp/dhcpv4/server4.NewServer(0x1119505, 0x3, 0xc000052f48, 0x1123320, 0x0, 0x0, 0x0, 0xc000052f48, 0x1043577, 0x11d4ce0)
	/Users/nonlogical/Documents/Developer/Mirror/github.com/insomniacslk/dhcp/dhcpv4/server4/server.go:139 +0xe5
main.main()
	/Users/nonlogical/Documents/Developer/Mirror/github.com/insomniacslk/dhcp/examples/server6/main.go:50 +0xc5
exit status 2

	err = unix.SetsockoptInt(fd, unix.IPPROTO_IP, unix.IPV6_BOUND_IF, iface.Index)
	if err != nil {
		panic(err)
	}
panic: protocol not available

goroutine 1 [running]:
github.com/insomniacslk/dhcp/interfaces.BindToInterface(0x3, 0x1119505, 0x3, 0xc000057dd4, 0x4)
	/Users/nonlogical/Documents/Developer/Mirror/github.com/insomniacslk/dhcp/interfaces/bindtodevice_darwin.go:38 +0x225
github.com/insomniacslk/dhcp/dhcpv4.BindToInterface(...)
	/Users/nonlogical/Documents/Developer/Mirror/github.com/insomniacslk/dhcp/dhcpv4/bindtointerface.go:9
github.com/insomniacslk/dhcp/dhcpv4/server4.NewIPv4UDPConn(0x1119505, 0x3, 0xc000057f48, 0x0, 0x0, 0x0)
	/Users/nonlogical/Documents/Developer/Mirror/github.com/insomniacslk/dhcp/dhcpv4/server4/conn.go:36 +0x7ea
github.com/insomniacslk/dhcp/dhcpv4/server4.NewServer(0x1119505, 0x3, 0xc000057f48, 0x1123320, 0x0, 0x0, 0x0, 0xc000057f48, 0x1043577, 0x11d4ce0)
	/Users/nonlogical/Documents/Developer/Mirror/github.com/insomniacslk/dhcp/dhcpv4/server4/server.go:139 +0xe5
main.main()
	/Users/nonlogical/Documents/Developer/Mirror/github.com/insomniacslk/dhcp/examples/server6/main.go:50 +0xc5
exit status 2

@NonLogicalDev
Copy link
Author

NonLogicalDev commented Mar 13, 2021

After some more experimentation I concluded that: (for ipv4 case)

// BindToInterface emulates linux's SO_BINDTODEVICE option for a socket by using
// IP_BOUND_IF.
func BindToInterface(fd int, ifname string) error {
	iface, err := net.InterfaceByName(ifname)
	if err != nil {
		return err
	}
	err = unix.SetsockoptInt(fd, unix.IPPROTO_IP, unix.IP_BOUND_IF, iface.Index)
	return err
}

is enough for correct operation. In other words this is not needed:

	if err := unix.SetsockoptInt(fd, unix.IPPROTO_IP, unix.IP_RECVIF, iface.Index); err != nil {
		return fmt.Errorf("Could not bind to an interface (receive path): %w", err)
	}

@NonLogicalDev
Copy link
Author

Also in case of IPV6 socket this does not raise any errors
(but could not test proper operation as I dont have dhcpv6 setup at the moment):

func BindToInterface(fd int, ifname string) error {
	iface, err := net.InterfaceByName(ifname)
	if err != nil {
		return err
	}
	err = unix.SetsockoptInt(fd, unix.IPPROTO_IPV6, unix.IPV6_BOUND_IF, iface.Index)
	return err
}

@Natolumin
Copy link
Collaborator

I can rebase and send a proper PR with these changes sometime this week. I still can't test it but now that the poc at least works it'll be quicker to test. Thanks a bunch for your research @NonLogicalDev

In regards to BSD, we are talking about the IP_BOUND_IF / IPV6_BOUND_IF which have different versions for ipv4 vs ipv6. IP_RECVIF weirdly does not have IPV6_RECVIF counterpart... shrug

IP_RECVIF is settable for both V4 and V6 sockets, like a few other ipv4-level socket options, eg IP_TOS or IP_TTL (you need to set them at the IPPROTO_IP level, but it's still accepted if the socket is bound to ipv6). And the darwin-xnu code it doesn't have the "option settable only for IPv4" block that you pointed out for IP_BOUND_IF:

[IP_BOUND_IF] is enough for correct operation

There's 2 paths here, we want the socket not to receive packets that are sent to other interfaces, in addition to sending them on the correct one. So to test it you need a darwin machine with 2 interfaces and sending dhcp messages to both, and check that the dhcp server only receives packets for the interface it is bound to. Afaict from the documentation, the receive path would still need IP_RECVIF
Otherwise IP_BOUND_IF should be enough to ensure we send on the correct interface yes

We could take a naive approach and attempt to set both, and see which one does not return an error.

Yeah I think that would be reasonable, try both and return success if at least one succeeds

@NonLogicalDev
Copy link
Author

NonLogicalDev commented Mar 15, 2021

By the way, @Natolumin where did you find all of the information about IP_RECVIF? And especially the IPV6_BOUND_IF I have been searching around and could not find anything reasonable that documents those options.

Best I found was this: (https://docs.oracle.com/cd/E86824_01/html/E54777/ip-7p.html)

IP_BOUND_IF

    Limit reception and transmission of packets to this interface. Takes an integer as an argument. The integer is the selected interface index.

But that is solaris documentation....

Seems like the IP_BOUND_IF does indeed behave differently under MacOS:

https://github.com/apple/darwin-xnu/blob/main/bsd/netinet/ip_output.c?ts=4#L2471-L2486

		/*
		 * On a multihomed system, scoped routing can be used to
		 * restrict the source interface used for sending packets.
		 * The socket option IP_BOUND_IF binds a particular AF_INET
		 * socket to an interface such that data sent on the socket
		 * is restricted to that interface.  This is unlike the
		 * SO_DONTROUTE option where the routing table is bypassed;
		 * therefore it allows for a greater flexibility and control
		 * over the system behavior, and does not place any restriction
		 * on the destination address type (e.g.  unicast, multicast,
		 * or broadcast if applicable) or whether or not the host is
		 * directly reachable.  Note that in the multicast transmit
		 * case, IP_MULTICAST_{IF,IFINDEX} takes precedence over
		 * IP_BOUND_IF, since the former practically bypasses the
		 * routing table; in this case, IP_BOUND_IF sets the default
		 * interface used for sending multicast packets in the absence
		 * of an explicit multicast transmit interface.
		 */
		case IP_BOUND_IF:

@NonLogicalDev
Copy link
Author

NonLogicalDev commented Mar 15, 2021

Updated my gist with a better test / interface code that seems to work and is easy to test under different conditions:

@Natolumin you were right about IPPROTO_IPV6 taking IP_RECVIF option. 🤯

In regards to the production version of BindToInterface, I earlier suggested to try seting IPV4 and IPV6 and see which one succeeds.

That is a bit dirty though and BindToInterface actually is only being called from either:

  • func NewIPv4UDPConn(iface string, addr *net.UDPAddr) (*net.UDPConn, error) {
  • func NewIPv6UDPConn(iface string, addr *net.UDPAddr) (*net.UDPConn, error) {

Both of which are in server{4,6}/conn.go and both have knowledge about the Socket domain.

It would be a lot less hacky to change BindToInterface signature to:
func BindToInterface(fd int, domain int, ifname string) error {

Where the domain is either unix.AF_INET or unix.AF_INET6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants