util.privateBlocks contains loopback address range

[woodsaj]

When determining the advertiseAddr the IP of each network interface is examined to see if it is a privateIP.

The set of privateIp ranges that the addresses are evaluated against include the loopback address range (127.0.0.0/8).

When iterating over the interface addresses, the loopback address (127.0.0.1) is always returned first on my system (Ubuntu 14.04- 3.13.0-100-generic #147-Ubuntu).

As a result, 127.0.0.1 is always selected as the privateIP address to advertise to peers, which results in the peers being unable to communicate with each other.

I really think that the loopback range should be removed from the privateBlocks or at the very least moved to the end of the slice so that all other private Ip ranges are checked first.

[slackpad]

@sean- any thoughts on this one?

[sean-]

This is the express reason why hashicorp/go-sockaddr sorts its addresses before making an address selection. Jiggling the sort order of utils.go's privateBlocks is suboptimal, but pragmatic in a pinch (

memberlist/util.go

Lines 25 to 34 in 9800c50

	/*
	* Contains an entry for each private block:
	* 10.0.0.0/8
	* 100.64.0.0/10
	* 127.0.0.0/8
	* 169.254.0.0/16
	* 172.16.0.0/12
	* 192.168.0.0/16
	*/
	var privateBlocks []*net.IPNet

).

From a correctness perspective, there are a few possible solutions, some more correct than others, but all solutions involve the use of hashicorp/go-sockaddr (mostly because that's the bestest hammer available, imnsho! 😁 ).

• Sort the addresses returned from the host by their mask length. Something like: https://github.com/hashicorp/go-sockaddr/blob/c403ce7245ed06f545550eb6b80e0f2b1bd7566f/ipaddrs_test.go#L163
• Expose a template that can be evaluated and use a sane default that filters out 127.0.0.1 or uses one of the template helper functions: https://github.com/hashicorp/go-sockaddr/blob/a99c57b5a5f48c51607e63089075fe7434fe219a/template/template.go#L29-L53
• Use sockaddr.GetPrivateIP() and remove most of the code in utils.go: https://github.com/hashicorp/go-sockaddr/blob/a99c57b5a5f48c51607e63089075fe7434fe219a/ifaddr.go#L11-L31 . @slackpad, now that we've just cut a release of both Serf and Consul, I think now would be a good time to do this so any breakage can marinate in master for a while.

@woodsaj , can you run the following few commands and see if they're correct for your environment:

$ go get -u github.com/hashicorp/go-sockaddr/cmd/sockaddr
$ sockaddr eval GetPrivateIP

[woodsaj]

$ sockaddr eval GetPrivateIP
192.168.0.102

looks to work correctly. :)

[sean-]

Ha!, excellent. Thank you for that point of validation.

@slackpad, your call now. :) I'm game for PRing this at some point unless you beat me to it. As far as impact, Consul already has a vendor dependency on go-sockaddr so that's a no op there. As far as other public projects, Serf and Nomad will need love because there will be a new library dependency. It's not that high on my list to tackle atm, but I can squeeze it in if needed.... or I'll do it on the train this morning (#108).

[slackpad]

Yep I'll pull this through Serf and Consul - /cc @dadgar for Nomad.

[slackpad]

Should be fixed at this level by #108.