From 10a828af2c42b42fdd9f35563b0badfe1039f6af Mon Sep 17 00:00:00 2001 From: Arceliar Date: Mon, 9 Sep 2019 19:20:46 -0500 Subject: [PATCH 01/11] when forwarding traffic, break distance ties by favoring the link that sent the most recent switch update the fastest --- src/yggdrasil/switch.go | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/src/yggdrasil/switch.go b/src/yggdrasil/switch.go index 163c85c..5ed7032 100644 --- a/src/yggdrasil/switch.go +++ b/src/yggdrasil/switch.go @@ -143,6 +143,7 @@ type switchPort uint64 type tableElem struct { port switchPort locator switchLocator + time time.Time } // This is the subset of the information about all peers needed to make routing decisions, and it stored separately in an atomically accessed table, which gets hammered in the "hot loop" of the routing logic (see: peer.handleTraffic in peers.go). @@ -562,6 +563,7 @@ func (t *switchTable) updateTable() { newTable.elems[pinfo.port] = tableElem{ locator: loc, port: pinfo.port, + time: pinfo.time, } } t.table.Store(newTable) @@ -581,7 +583,7 @@ func (t *switchTable) start() error { } type closerInfo struct { - port switchPort + elem tableElem dist int } @@ -598,7 +600,7 @@ func (t *switchTable) getCloser(dest []byte) []closerInfo { for _, info := range table.elems { dist := info.locator.dist(dest) if dist < myDist { - t.queues.closer = append(t.queues.closer, closerInfo{info.port, dist}) + t.queues.closer = append(t.queues.closer, closerInfo{info, dist}) } } return t.queues.closer @@ -671,13 +673,12 @@ func (t *switchTable) _handleIn(packet []byte, idle map[switchPort]time.Time) bo self.sendPacketsFrom(t, [][]byte{packet}) return true } - var best *peer - var bestDist int + var best *closerInfo var bestTime time.Time ports := t.core.peers.getPorts() for _, cinfo := range closer { - to := ports[cinfo.port] - thisTime, isIdle := idle[cinfo.port] + to := ports[cinfo.elem.port] + thisTime, isIdle := idle[cinfo.elem.port] var update bool switch { case to == nil: @@ -688,13 +689,24 @@ func (t *switchTable) _handleIn(packet []byte, idle map[switchPort]time.Time) bo // this is the first idle port we've found, so select it until we find a // better candidate port to use instead update = true - case cinfo.dist < bestDist: + case cinfo.dist < best.dist: // the port takes a shorter path/is more direct than our current // candidate, so select that instead update = true - case cinfo.dist > bestDist: + case cinfo.dist > best.dist: // the port takes a longer path/is less direct than our current candidate, // ignore it + case cinfo.elem.locator.tstamp > best.elem.locator.tstamp: + // has a newer tstamp from the root, so presumably a better path + update = true + case cinfo.elem.locator.tstamp < best.elem.locator.tstamp: + // has a n older tstamp, so presumably a worse path + case cinfo.elem.time.Before(best.elem.time): + // same tstamp, but got it earlier, so presumably a better path + update = true + case cinfo.elem.time.After(best.elem.time): + // same tstamp, but got it later, so presumably a worse path + // I do not expect the remaining cases to ever be reached... TODO cleanup case thisTime.After(bestTime): // all else equal, this port was used more recently than our current // candidate, so choose that instead. this should mean that, in low @@ -705,15 +717,15 @@ func (t *switchTable) _handleIn(packet []byte, idle map[switchPort]time.Time) bo // the search for a port has finished } if update { - best = to - bestDist = cinfo.dist + b := cinfo // because cinfo gets mutated by the iteration + best = &b bestTime = thisTime } } if best != nil { // Send to the best idle next hop - delete(idle, best.port) - best.sendPacketsFrom(t, [][]byte{packet}) + delete(idle, best.elem.port) + ports[best.elem.port].sendPacketsFrom(t, [][]byte{packet}) return true } // Didn't find anyone idle to send it to From 0141180279cc896f70f8f8b1b80d3e895c24b16a Mon Sep 17 00:00:00 2001 From: Arceliar Date: Mon, 9 Sep 2019 19:25:10 -0500 Subject: [PATCH 02/11] cleanup --- src/yggdrasil/switch.go | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/src/yggdrasil/switch.go b/src/yggdrasil/switch.go index 5ed7032..f4df60d 100644 --- a/src/yggdrasil/switch.go +++ b/src/yggdrasil/switch.go @@ -176,7 +176,7 @@ type switchTable struct { table atomic.Value // lookupTable phony.Inbox // Owns the below queues switch_buffers // Queues - not atomic so ONLY use through the actor - idle map[switchPort]time.Time // idle peers - not atomic so ONLY use through the actor + idle map[switchPort]struct{} // idle peers - not atomic so ONLY use through the actor } // Minimum allowed total size of switch queues. @@ -202,7 +202,7 @@ func (t *switchTable) init(core *Core) { } core.config.Mutex.RUnlock() t.queues.bufs = make(map[string]switch_buffer) - t.idle = make(map[switchPort]time.Time) + t.idle = make(map[switchPort]struct{}) }) } @@ -664,7 +664,7 @@ func (t *switchTable) bestPortForCoords(coords []byte) switchPort { // Handle an incoming packet // Either send it to ourself, or to the first idle peer that's free // Returns true if the packet has been handled somehow, false if it should be queued -func (t *switchTable) _handleIn(packet []byte, idle map[switchPort]time.Time) bool { +func (t *switchTable) _handleIn(packet []byte, idle map[switchPort]struct{}) bool { coords := switch_getPacketCoords(packet) closer := t.getCloser(coords) if len(closer) == 0 { @@ -674,11 +674,10 @@ func (t *switchTable) _handleIn(packet []byte, idle map[switchPort]time.Time) bo return true } var best *closerInfo - var bestTime time.Time ports := t.core.peers.getPorts() for _, cinfo := range closer { to := ports[cinfo.elem.port] - thisTime, isIdle := idle[cinfo.elem.port] + _, isIdle := idle[cinfo.elem.port] var update bool switch { case to == nil: @@ -704,22 +703,12 @@ func (t *switchTable) _handleIn(packet []byte, idle map[switchPort]time.Time) bo case cinfo.elem.time.Before(best.elem.time): // same tstamp, but got it earlier, so presumably a better path update = true - case cinfo.elem.time.After(best.elem.time): - // same tstamp, but got it later, so presumably a worse path - // I do not expect the remaining cases to ever be reached... TODO cleanup - case thisTime.After(bestTime): - // all else equal, this port was used more recently than our current - // candidate, so choose that instead. this should mean that, in low - // traffic scenarios, we consistently pick the same link which helps with - // packet ordering - update = true default: // the search for a port has finished } if update { b := cinfo // because cinfo gets mutated by the iteration best = &b - bestTime = thisTime } } if best != nil { @@ -882,6 +871,6 @@ func (t *switchTable) _idleIn(port switchPort) { // Try to find something to send to this peer if !t._handleIdle(port) { // Didn't find anything ready to send yet, so stay idle - t.idle[port] = time.Now() + t.idle[port] = struct{}{} } } From 80ba24d51255c3751e2b25aceee52b20d59ff746 Mon Sep 17 00:00:00 2001 From: Arceliar Date: Tue, 17 Sep 2019 19:42:07 -0500 Subject: [PATCH 03/11] force things to buffer in the switch if the best link is currently busy. note that other links can end up sending if they become non-idle for other reasons. this is a temporary workaround to packet reordering, until we can figure out a better solution --- src/yggdrasil/switch.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/yggdrasil/switch.go b/src/yggdrasil/switch.go index f4df60d..4a1999a 100644 --- a/src/yggdrasil/switch.go +++ b/src/yggdrasil/switch.go @@ -677,13 +677,10 @@ func (t *switchTable) _handleIn(packet []byte, idle map[switchPort]struct{}) boo ports := t.core.peers.getPorts() for _, cinfo := range closer { to := ports[cinfo.elem.port] - _, isIdle := idle[cinfo.elem.port] var update bool switch { case to == nil: // no port was found, ignore it - case !isIdle: - // the port is busy, ignore it case best == nil: // this is the first idle port we've found, so select it until we find a // better candidate port to use instead @@ -713,6 +710,9 @@ func (t *switchTable) _handleIn(packet []byte, idle map[switchPort]struct{}) boo } if best != nil { // Send to the best idle next hop + if _, isIdle := idle[best.elem.port]; !isIdle { + return false + } delete(idle, best.elem.port) ports[best.elem.port].sendPacketsFrom(t, [][]byte{packet}) return true From be35675d0f01aa5e22571f9c71ed36ba4a87b8ba Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 18 Sep 2019 13:37:01 +0100 Subject: [PATCH 04/11] Catch a nil pointer when sending a session packet to a conn, this shouldn't happen but it's caused multiple crashes in conn.recvMsg --- src/yggdrasil/session.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/yggdrasil/session.go b/src/yggdrasil/session.go index 8a6d16f..4980862 100644 --- a/src/yggdrasil/session.go +++ b/src/yggdrasil/session.go @@ -470,7 +470,14 @@ func (sinfo *sessionInfo) _recvPacket(p *wire_trafficPacket) { callback := func() { util.PutBytes(p.Payload) if !isOK || k != sinfo.sharedSesKey || !sinfo._nonceIsOK(&p.Nonce) { - // Either we failed to decrypt, or the session was updated, or we received this packet in the mean time + // Either we failed to decrypt, or the session was updated, or we + // received this packet in the mean time + util.PutBytes(bs) + return + } + if sinfo.conn == nil { + // There's no connection associated with this session for some reason + // TODO: Figure out why this happens sometimes, it shouldn't util.PutBytes(bs) return } From ae0b2672ff819dbea4a556525f9972556ca51768 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 18 Sep 2019 14:32:28 +0100 Subject: [PATCH 05/11] Fix #539 --- src/yggdrasil/tcp.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/yggdrasil/tcp.go b/src/yggdrasil/tcp.go index cce352b..5ac921c 100644 --- a/src/yggdrasil/tcp.go +++ b/src/yggdrasil/tcp.go @@ -249,7 +249,7 @@ func (t *tcp) call(saddr string, options interface{}, sintf string) { if err != nil { return } - t.handler(conn, false, dialerdst.String()) + t.handler(conn, false, saddr) } else { dst, err := net.ResolveTCPAddr("tcp", saddr) if err != nil { @@ -322,18 +322,19 @@ func (t *tcp) handler(sock net.Conn, incoming bool, options interface{}) { t.setExtraOptions(sock) stream := stream{} stream.init(sock) - local, _, _ := net.SplitHostPort(sock.LocalAddr().String()) - remote, _, _ := net.SplitHostPort(sock.RemoteAddr().String()) - force := net.ParseIP(strings.Split(remote, "%")[0]).IsLinkLocalUnicast() - var name string - var proto string + var name, proto, local, remote string if socksaddr, issocks := options.(string); issocks { - name = "socks://" + socksaddr + "/" + sock.RemoteAddr().String() + name = "socks://" + sock.RemoteAddr().String() + "/" + socksaddr proto = "socks" + local, _, _ = net.SplitHostPort(sock.LocalAddr().String()) + remote, _, _ = net.SplitHostPort(socksaddr) } else { name = "tcp://" + sock.RemoteAddr().String() proto = "tcp" + local, _, _ = net.SplitHostPort(sock.LocalAddr().String()) + remote, _, _ = net.SplitHostPort(sock.RemoteAddr().String()) } + force := net.ParseIP(strings.Split(remote, "%")[0]).IsLinkLocalUnicast() link, err := t.link.core.link.create(&stream, name, proto, local, remote, incoming, force) if err != nil { t.link.core.log.Println(err) From 368f499f1df53df85a1fc9ede1f662635b32f3e2 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 18 Sep 2019 14:37:25 +0100 Subject: [PATCH 06/11] Update apt before trying to pull in RPM dependencies --- .circleci/config.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index f9cd072..5778ffc 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -29,6 +29,7 @@ jobs: - run: name: Install RPM utilities command: | + sudo apt-get update sudo apt-get install -y rpm file mkdir -p ~/rpmbuild/BUILD ~/rpmbuild/RPMS ~/rpmbuild/SOURCES ~/rpmbuild/SPECS ~/rpmbuild/SRPMS From 94cf2854a93575db75aed221ec4ecb24a609a85b Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 18 Sep 2019 14:05:18 +0100 Subject: [PATCH 07/11] Fix panic where slice goes out of bounds because iface.Read returns less than zero (which might happen when the TUN/TAP interface is closed) --- src/tuntap/iface.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tuntap/iface.go b/src/tuntap/iface.go index 92ba36a..753a8d0 100644 --- a/src/tuntap/iface.go +++ b/src/tuntap/iface.go @@ -111,7 +111,7 @@ func (r *tunReader) _read() { recvd := util.ResizeBytes(util.GetBytes(), 65535+tun_ETHER_HEADER_LENGTH) // Wait for a packet to be delivered to us through the TUN/TAP adapter n, err := r.tun.iface.Read(recvd) - if n == 0 { + if n <= 0 { util.PutBytes(recvd) } else { r.tun.handlePacketFrom(r, recvd[:n], err) From ddaaa865cb264b375ff836b91549be4768569fe7 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 18 Sep 2019 15:01:19 +0100 Subject: [PATCH 08/11] Be more verbose when a peer or listener is badly formatted --- src/yggdrasil/core.go | 14 ++++++++++---- src/yggdrasil/link.go | 4 ++-- src/yggdrasil/tcp.go | 3 +++ 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/yggdrasil/core.go b/src/yggdrasil/core.go index 754d7d6..4dcd16f 100644 --- a/src/yggdrasil/core.go +++ b/src/yggdrasil/core.go @@ -91,15 +91,21 @@ func (c *Core) _addPeerLoop() { // Add peers from the Peers section for _, peer := range current.Peers { - go c.AddPeer(peer, "") // TODO: this should be acted and not in a goroutine? - time.Sleep(time.Second) + go func() { + if err := c.AddPeer(peer, ""); err != nil { + c.log.Errorln("Failed to add peer:", err) + } + }() // TODO: this should be acted and not in a goroutine? } // Add peers from the InterfacePeers section for intf, intfpeers := range current.InterfacePeers { for _, peer := range intfpeers { - go c.AddPeer(peer, intf) // TODO: this should be acted and not in a goroutine? - time.Sleep(time.Second) + go func() { + if err := c.AddPeer(peer, intf); err != nil { + c.log.Errorln("Failed to add peer:", err) + } + }() // TODO: this should be acted and not in a goroutine? } } diff --git a/src/yggdrasil/link.go b/src/yggdrasil/link.go index 6e39351..bdf1554 100644 --- a/src/yggdrasil/link.go +++ b/src/yggdrasil/link.go @@ -86,7 +86,7 @@ func (l *link) reconfigure() { func (l *link) call(uri string, sintf string) error { u, err := url.Parse(uri) if err != nil { - return err + return fmt.Errorf("peer %s is not correctly formatted (%s)", uri, err) } pathtokens := strings.Split(strings.Trim(u.Path, "/"), "/") switch u.Scheme { @@ -103,7 +103,7 @@ func (l *link) call(uri string, sintf string) error { func (l *link) listen(uri string) error { u, err := url.Parse(uri) if err != nil { - return err + return fmt.Errorf("listener %s is not correctly formatted (%s)", uri, err) } switch u.Scheme { case "tcp": diff --git a/src/yggdrasil/tcp.go b/src/yggdrasil/tcp.go index 5ac921c..93e39e4 100644 --- a/src/yggdrasil/tcp.go +++ b/src/yggdrasil/tcp.go @@ -85,6 +85,7 @@ func (t *tcp) init(l *link) error { defer t.link.core.config.Mutex.RUnlock() for _, listenaddr := range t.link.core.config.Current.Listen { if listenaddr[:6] != "tcp://" { + t.link.core.log.Errorln("Failed to add listener: listener", listenaddr, "is not correctly formatted, ignoring") continue } if _, err := t.listen(listenaddr[6:]); err != nil { @@ -103,6 +104,7 @@ func (t *tcp) reconfigure() { if len(added) > 0 || len(deleted) > 0 { for _, a := range added { if a[:6] != "tcp://" { + t.link.core.log.Errorln("Failed to add listener: listener", a, "is not correctly formatted, ignoring") continue } if _, err := t.listen(a[6:]); err != nil { @@ -113,6 +115,7 @@ func (t *tcp) reconfigure() { } for _, d := range deleted { if d[:6] != "tcp://" { + t.link.core.log.Errorln("Failed to delete listener: listener", d, "is not correctly formatted, ignoring") continue } t.mutex.Lock() From d44a7faa04375190f1b72c9f55c8f1a721df2319 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 18 Sep 2019 20:09:53 +0100 Subject: [PATCH 09/11] semver: Don't return failure codes when git history is not present --- contrib/semver/name.sh | 2 +- contrib/semver/version.sh | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/contrib/semver/name.sh b/contrib/semver/name.sh index 1fa2ce0..308e07b 100644 --- a/contrib/semver/name.sh +++ b/contrib/semver/name.sh @@ -6,7 +6,7 @@ BRANCH=$(git symbolic-ref --short HEAD 2>/dev/null) # Complain if the git history is not available if [ $? != 0 ] || [ -z "$BRANCH" ]; then printf "yggdrasil" - exit 1 + exit 0 fi # Remove "/" characters from the branch name if present diff --git a/contrib/semver/version.sh b/contrib/semver/version.sh index 3052094..db96e33 100644 --- a/contrib/semver/version.sh +++ b/contrib/semver/version.sh @@ -6,7 +6,7 @@ TAG=$(git describe --abbrev=0 --tags --match="v[0-9]*\.[0-9]*\.[0-9]*" 2>/dev/nu # Did getting the tag succeed? if [ $? != 0 ] || [ -z "$TAG" ]; then printf -- "unknown" - exit 1 + exit 0 fi # Get the current branch @@ -36,7 +36,7 @@ if [ "$BRANCH" != "master" ]; then # Did getting the count of commits since the tag succeed? if [ $? != 0 ] || [ -z "$BUILD" ]; then printf -- "-unknown" - exit 1 + exit 0 fi # Is the build greater than zero? From 0a12e4b1c1d50c910defbf0607ccb86165c16791 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 18 Sep 2019 20:26:06 +0100 Subject: [PATCH 10/11] Revert "Catch a nil pointer when sending a session packet to a conn, this shouldn't happen but it's caused multiple crashes in conn.recvMsg" This reverts commit be35675d0f01aa5e22571f9c71ed36ba4a87b8ba. --- src/yggdrasil/session.go | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/yggdrasil/session.go b/src/yggdrasil/session.go index 4980862..8a6d16f 100644 --- a/src/yggdrasil/session.go +++ b/src/yggdrasil/session.go @@ -470,14 +470,7 @@ func (sinfo *sessionInfo) _recvPacket(p *wire_trafficPacket) { callback := func() { util.PutBytes(p.Payload) if !isOK || k != sinfo.sharedSesKey || !sinfo._nonceIsOK(&p.Nonce) { - // Either we failed to decrypt, or the session was updated, or we - // received this packet in the mean time - util.PutBytes(bs) - return - } - if sinfo.conn == nil { - // There's no connection associated with this session for some reason - // TODO: Figure out why this happens sometimes, it shouldn't + // Either we failed to decrypt, or the session was updated, or we received this packet in the mean time util.PutBytes(bs) return } From 909e4e29a8f1de08b33afdd1e97c731a21211b37 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 18 Sep 2019 23:44:28 +0100 Subject: [PATCH 11/11] Don't spawn goroutines for addPeerLoop, TCP connect timeout of 5 seconds for now --- src/yggdrasil/core.go | 16 ++++++---------- src/yggdrasil/link.go | 2 +- src/yggdrasil/tcp.go | 1 + 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/yggdrasil/core.go b/src/yggdrasil/core.go index 4dcd16f..884ef9e 100644 --- a/src/yggdrasil/core.go +++ b/src/yggdrasil/core.go @@ -91,21 +91,17 @@ func (c *Core) _addPeerLoop() { // Add peers from the Peers section for _, peer := range current.Peers { - go func() { - if err := c.AddPeer(peer, ""); err != nil { - c.log.Errorln("Failed to add peer:", err) - } - }() // TODO: this should be acted and not in a goroutine? + if err := c.AddPeer(peer, ""); err != nil { + c.log.Errorln("Failed to add peer:", err) + } } // Add peers from the InterfacePeers section for intf, intfpeers := range current.InterfacePeers { for _, peer := range intfpeers { - go func() { - if err := c.AddPeer(peer, intf); err != nil { - c.log.Errorln("Failed to add peer:", err) - } - }() // TODO: this should be acted and not in a goroutine? + if err := c.AddPeer(peer, intf); err != nil { + c.log.Errorln("Failed to add peer:", err) + } } } diff --git a/src/yggdrasil/link.go b/src/yggdrasil/link.go index bdf1554..3686ab9 100644 --- a/src/yggdrasil/link.go +++ b/src/yggdrasil/link.go @@ -86,7 +86,7 @@ func (l *link) reconfigure() { func (l *link) call(uri string, sintf string) error { u, err := url.Parse(uri) if err != nil { - return fmt.Errorf("peer %s is not correctly formatted (%s)", uri, err) + return fmt.Errorf("peer %s is not correctly formatted", uri) } pathtokens := strings.Split(strings.Trim(u.Path, "/"), "/") switch u.Scheme { diff --git a/src/yggdrasil/tcp.go b/src/yggdrasil/tcp.go index 93e39e4..f81e49a 100644 --- a/src/yggdrasil/tcp.go +++ b/src/yggdrasil/tcp.go @@ -266,6 +266,7 @@ func (t *tcp) call(saddr string, options interface{}, sintf string) { } dialer := net.Dialer{ Control: t.tcpContext, + Timeout: time.Second * 5, } if sintf != "" { ief, err := net.InterfaceByName(sintf)