From 81839ad50d11ea0cf6d44f2eb5841022a38a1aa6 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Fri, 21 Oct 2022 19:49:15 +0100 Subject: [PATCH 1/7] Fix `InterfacePeers` --- src/core/link_tcp.go | 13 +++++-------- src/core/link_tls.go | 3 +-- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/core/link_tcp.go b/src/core/link_tcp.go index c5a73c9e..30749983 100644 --- a/src/core/link_tcp.go +++ b/src/core/link_tcp.go @@ -39,8 +39,7 @@ func (l *linkTCP) dial(url *url.URL, options linkOptions, sintf string) error { if err != nil { return err } - addr.Zone = sintf - dialer, err := l.dialerFor(addr.String(), sintf) + dialer, err := l.dialerFor(addr, sintf) if err != nil { return err } @@ -121,13 +120,11 @@ func (l *linkTCP) getAddr() *net.TCPAddr { return addr } -func (l *linkTCP) dialerFor(saddr, sintf string) (*net.Dialer, error) { - dst, err := net.ResolveTCPAddr("tcp", saddr) - if err != nil { - return nil, err - } +func (l *linkTCP) dialerFor(dst *net.TCPAddr, sintf string) (*net.Dialer, error) { if dst.IP.IsLinkLocalUnicast() { - dst.Zone = sintf + if sintf != "" { + dst.Zone = sintf + } if dst.Zone == "" { return nil, fmt.Errorf("link-local address requires a zone") } diff --git a/src/core/link_tls.go b/src/core/link_tls.go index 1e932b66..2dc2daf1 100644 --- a/src/core/link_tls.go +++ b/src/core/link_tls.go @@ -55,8 +55,7 @@ func (l *linkTLS) dial(url *url.URL, options linkOptions, sintf, sni string) err if err != nil { return err } - addr.Zone = sintf - dialer, err := l.tcp.dialerFor(addr.String(), sintf) + dialer, err := l.tcp.dialerFor(addr, sintf) if err != nil { return err } From 22caddef63b1e46a4fc6e8c9a93e425b391fa723 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Fri, 21 Oct 2022 19:49:49 +0100 Subject: [PATCH 2/7] Don't log `duplicate connection attempt` --- src/core/link_socks.go | 2 +- src/core/link_tcp.go | 2 +- src/core/link_tls.go | 2 +- src/core/link_unix.go | 3 +-- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/core/link_socks.go b/src/core/link_socks.go index ad5b8c98..036de992 100644 --- a/src/core/link_socks.go +++ b/src/core/link_socks.go @@ -23,7 +23,7 @@ func (l *links) newLinkSOCKS() *linkSOCKS { func (l *linkSOCKS) dial(url *url.URL, options linkOptions) error { info := linkInfoFor("socks", "", url.Path) if l.links.isConnectedTo(info) { - return fmt.Errorf("duplicate connection attempt") + return nil } proxyAuth := &proxy.Auth{} proxyAuth.User = url.User.Username() diff --git a/src/core/link_tcp.go b/src/core/link_tcp.go index 30749983..6a04bb35 100644 --- a/src/core/link_tcp.go +++ b/src/core/link_tcp.go @@ -33,7 +33,7 @@ func (l *links) newLinkTCP() *linkTCP { func (l *linkTCP) dial(url *url.URL, options linkOptions, sintf string) error { info := linkInfoFor("tcp", sintf, strings.SplitN(url.Host, "%", 2)[0]) if l.links.isConnectedTo(info) { - return fmt.Errorf("duplicate connection attempt") + return nil } addr, err := net.ResolveTCPAddr("tcp", url.Host) if err != nil { diff --git a/src/core/link_tls.go b/src/core/link_tls.go index 2dc2daf1..bc39b6c0 100644 --- a/src/core/link_tls.go +++ b/src/core/link_tls.go @@ -49,7 +49,7 @@ func (l *links) newLinkTLS(tcp *linkTCP) *linkTLS { func (l *linkTLS) dial(url *url.URL, options linkOptions, sintf, sni string) error { info := linkInfoFor("tls", sintf, strings.SplitN(url.Host, "%", 2)[0]) if l.links.isConnectedTo(info) { - return fmt.Errorf("duplicate connection attempt") + return nil } addr, err := net.ResolveTCPAddr("tcp", url.Host) if err != nil { diff --git a/src/core/link_unix.go b/src/core/link_unix.go index e71f9362..2d1b9398 100644 --- a/src/core/link_unix.go +++ b/src/core/link_unix.go @@ -2,7 +2,6 @@ package core import ( "context" - "fmt" "net" "net/url" "time" @@ -36,7 +35,7 @@ func (l *links) newLinkUNIX() *linkUNIX { func (l *linkUNIX) dial(url *url.URL, options linkOptions, _ string) error { info := linkInfoFor("unix", "", url.Path) if l.links.isConnectedTo(info) { - return fmt.Errorf("duplicate connection attempt") + return nil } addr, err := net.ResolveUnixAddr("unix", url.Path) if err != nil { From c55611a478bc725a462db241c56758eb613cd4cf Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Sat, 22 Oct 2022 14:56:11 +0100 Subject: [PATCH 3/7] Tweak logging for connections --- src/core/link.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/core/link.go b/src/core/link.go index b4515276..f8e5be22 100644 --- a/src/core/link.go +++ b/src/core/link.go @@ -286,7 +286,7 @@ func (intf *link) handler() error { } } if intf.incoming && !intf.force && !isallowed { - intf.links.core.log.Warnf("%s connection from %s forbidden: AllowedEncryptionPublicKeys does not contain key %s", + intf.links.core.log.Warnf("%s connection from %s forbidden: AllowedPublicKeys does not contain key %s", strings.ToUpper(intf.info.linkType), intf.info.remote, hex.EncodeToString(meta.key)) _ = intf.close() return fmt.Errorf("forbidden connection") @@ -302,15 +302,15 @@ func (intf *link) handler() error { intf.links.core.log.Infof("Connected %s: %s, source %s", strings.ToUpper(intf.info.linkType), remoteStr, localStr) - // TODO don't report an error if it's just a 'use of closed network connection' - if err = intf.links.core.HandleConn(meta.key, intf.conn); err != nil && err != io.EOF { - intf.links.core.log.Infof("Disconnected %s: %s, source %s; error: %s", - strings.ToUpper(intf.info.linkType), remoteStr, localStr, err) - } else { + err = intf.links.core.HandleConn(meta.key, intf.conn) + switch err { + case io.EOF, net.ErrClosed, nil: intf.links.core.log.Infof("Disconnected %s: %s, source %s", strings.ToUpper(intf.info.linkType), remoteStr, localStr) + default: + intf.links.core.log.Infof("Disconnected %s: %s, source %s; error: %s", + strings.ToUpper(intf.info.linkType), remoteStr, localStr, err) } - return nil } From 0a1a155e66ace9ee30841348a3eb1cdbafca2264 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Sat, 22 Oct 2022 14:56:29 +0100 Subject: [PATCH 4/7] Use `SO_REUSEADDR` instead of `SO_REUSEPORT` on Linux --- src/multicast/multicast_unix.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/multicast/multicast_unix.go b/src/multicast/multicast_unix.go index c59d876b..08230735 100644 --- a/src/multicast/multicast_unix.go +++ b/src/multicast/multicast_unix.go @@ -15,15 +15,19 @@ func (m *Multicast) _multicastStarted() { func (m *Multicast) multicastReuse(network string, address string, c syscall.RawConn) error { var control error - var reuseport error + var reuseaddr error control = c.Control(func(fd uintptr) { - reuseport = unix.SetsockoptInt(int(fd), unix.SOL_SOCKET, unix.SO_REUSEPORT, 1) + // Previously we used SO_REUSEPORT here, but that meant that machines running + // Yggdrasil nodes as different users would inevitably fail with EADDRINUSE. + // The behaviour for multicast is similar with both, so we'll use SO_REUSEADDR + // instead. + reuseaddr = unix.SetsockoptInt(int(fd), unix.SOL_SOCKET, unix.SO_REUSEADDR, 1) }) switch { - case reuseport != nil: - return reuseport + case reuseaddr != nil: + return reuseaddr default: return control } From 63c4cb5c211aeca37bbe62534bb3675db03a96f6 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Sat, 22 Oct 2022 15:47:09 +0100 Subject: [PATCH 5/7] Fix reporting name for TCP --- src/core/link_tcp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/link_tcp.go b/src/core/link_tcp.go index 6a04bb35..a388fcdb 100644 --- a/src/core/link_tcp.go +++ b/src/core/link_tcp.go @@ -82,7 +82,7 @@ func (l *linkTCP) listen(url *url.URL, sintf string) (*Listener, error) { break } addr := conn.RemoteAddr().(*net.TCPAddr) - name := fmt.Sprintf("tls://%s", addr) + name := fmt.Sprintf("tcp://%s", addr) info := linkInfoFor("tcp", sintf, strings.SplitN(addr.IP.String(), "%", 2)[0]) if err = l.handler(name, info, conn, linkOptions{}, true); err != nil { l.core.log.Errorln("Failed to create inbound link:", err) From d66b3ffb7ae67b7996033acf5e54a9174e8856bd Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Sat, 22 Oct 2022 16:23:25 +0100 Subject: [PATCH 6/7] Always allow link-local peerings again --- src/core/link.go | 27 ++++++++++++--------------- src/core/link_tcp.go | 8 ++++---- src/core/link_tls.go | 8 ++++---- 3 files changed, 20 insertions(+), 23 deletions(-) diff --git a/src/core/link.go b/src/core/link.go index f8e5be22..e822acaf 100644 --- a/src/core/link.go +++ b/src/core/link.go @@ -272,8 +272,7 @@ func (intf *link) handler() error { var key keyArray copy(key[:], meta.key) if _, allowed := pinned[key]; !allowed { - intf.links.core.log.Errorf("Failed to connect to node: %q sent ed25519 key that does not match pinned keys", intf.name()) - return fmt.Errorf("failed to connect: host sent ed25519 key that does not match pinned keys") + return fmt.Errorf("node public key that does not match pinned keys") } } // Check if we're authorized to connect to this key / IP @@ -286,30 +285,32 @@ func (intf *link) handler() error { } } if intf.incoming && !intf.force && !isallowed { - intf.links.core.log.Warnf("%s connection from %s forbidden: AllowedPublicKeys does not contain key %s", - strings.ToUpper(intf.info.linkType), intf.info.remote, hex.EncodeToString(meta.key)) _ = intf.close() - return fmt.Errorf("forbidden connection") + return fmt.Errorf("node public key %q is not in AllowedPublicKeys", hex.EncodeToString(meta.key)) } phony.Block(intf.links, func() { intf.links._links[intf.info] = intf }) + dir := "outbound" + if intf.incoming { + dir = "inbound" + } remoteAddr := net.IP(address.AddrForKey(meta.key)[:]).String() remoteStr := fmt.Sprintf("%s@%s", remoteAddr, intf.info.remote) localStr := intf.conn.LocalAddr() - intf.links.core.log.Infof("Connected %s: %s, source %s", - strings.ToUpper(intf.info.linkType), remoteStr, localStr) + intf.links.core.log.Infof("Connected %s %s: %s, source %s", + dir, strings.ToUpper(intf.info.linkType), remoteStr, localStr) err = intf.links.core.HandleConn(meta.key, intf.conn) switch err { case io.EOF, net.ErrClosed, nil: - intf.links.core.log.Infof("Disconnected %s: %s, source %s", - strings.ToUpper(intf.info.linkType), remoteStr, localStr) + intf.links.core.log.Infof("Disconnected %s %s: %s, source %s", + dir, strings.ToUpper(intf.info.linkType), remoteStr, localStr) default: - intf.links.core.log.Infof("Disconnected %s: %s, source %s; error: %s", - strings.ToUpper(intf.info.linkType), remoteStr, localStr, err) + intf.links.core.log.Infof("Disconnected %s %s: %s, source %s; error: %s", + dir, strings.ToUpper(intf.info.linkType), remoteStr, localStr, err) } return nil } @@ -318,10 +319,6 @@ func (intf *link) close() error { return intf.conn.Close() } -func (intf *link) name() string { - return intf.lname -} - func linkInfoFor(linkType, sintf, remote string) linkInfo { if h, _, err := net.SplitHostPort(remote); err == nil { remote = h diff --git a/src/core/link_tcp.go b/src/core/link_tcp.go index a388fcdb..986eda30 100644 --- a/src/core/link_tcp.go +++ b/src/core/link_tcp.go @@ -47,7 +47,7 @@ func (l *linkTCP) dial(url *url.URL, options linkOptions, sintf string) error { if err != nil { return err } - return l.handler(url.String(), info, conn, options, false) + return l.handler(url.String(), info, conn, options, false, false) } func (l *linkTCP) listen(url *url.URL, sintf string) (*Listener, error) { @@ -84,7 +84,7 @@ func (l *linkTCP) listen(url *url.URL, sintf string) (*Listener, error) { addr := conn.RemoteAddr().(*net.TCPAddr) name := fmt.Sprintf("tcp://%s", addr) info := linkInfoFor("tcp", sintf, strings.SplitN(addr.IP.String(), "%", 2)[0]) - if err = l.handler(name, info, conn, linkOptions{}, true); err != nil { + if err = l.handler(name, info, conn, linkOptions{}, true, addr.IP.IsLinkLocalUnicast()); err != nil { l.core.log.Errorln("Failed to create inbound link:", err) } } @@ -95,13 +95,13 @@ func (l *linkTCP) listen(url *url.URL, sintf string) (*Listener, error) { return entry, nil } -func (l *linkTCP) handler(name string, info linkInfo, conn net.Conn, options linkOptions, incoming bool) error { +func (l *linkTCP) handler(name string, info linkInfo, conn net.Conn, options linkOptions, incoming, force bool) error { return l.links.create( conn, // connection name, // connection name info, // connection info incoming, // not incoming - false, // not forced + force, // not forced options, // connection options ) } diff --git a/src/core/link_tls.go b/src/core/link_tls.go index bc39b6c0..9e7dda9b 100644 --- a/src/core/link_tls.go +++ b/src/core/link_tls.go @@ -69,7 +69,7 @@ func (l *linkTLS) dial(url *url.URL, options linkOptions, sintf, sni string) err if err != nil { return err } - return l.handler(url.String(), info, conn, options, false) + return l.handler(url.String(), info, conn, options, false, false) } func (l *linkTLS) listen(url *url.URL, sintf string) (*Listener, error) { @@ -107,7 +107,7 @@ func (l *linkTLS) listen(url *url.URL, sintf string) (*Listener, error) { addr := conn.RemoteAddr().(*net.TCPAddr) name := fmt.Sprintf("tls://%s", addr) info := linkInfoFor("tls", sintf, strings.SplitN(addr.IP.String(), "%", 2)[0]) - if err = l.handler(name, info, conn, linkOptions{}, true); err != nil { + if err = l.handler(name, info, conn, linkOptions{}, true, addr.IP.IsLinkLocalUnicast()); err != nil { l.core.log.Errorln("Failed to create inbound link:", err) } } @@ -165,6 +165,6 @@ func (l *linkTLS) generateConfig() (*tls.Config, error) { }, nil } -func (l *linkTLS) handler(name string, info linkInfo, conn net.Conn, options linkOptions, incoming bool) error { - return l.tcp.handler(name, info, conn, options, incoming) +func (l *linkTLS) handler(name string, info linkInfo, conn net.Conn, options linkOptions, incoming, force bool) error { + return l.tcp.handler(name, info, conn, options, incoming, force) } From 8fe1c41295e568df75d89cf2226f0260f32f7e9c Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Sat, 22 Oct 2022 16:59:25 +0100 Subject: [PATCH 7/7] Don't reject multiple genuine links from the same host --- src/core/link.go | 3 --- src/core/link_tcp.go | 13 +++++++------ src/core/link_tls.go | 13 +++++++------ 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/core/link.go b/src/core/link.go index e822acaf..67184625 100644 --- a/src/core/link.go +++ b/src/core/link.go @@ -320,9 +320,6 @@ func (intf *link) close() error { } func linkInfoFor(linkType, sintf, remote string) linkInfo { - if h, _, err := net.SplitHostPort(remote); err == nil { - remote = h - } return linkInfo{ linkType: linkType, local: sintf, diff --git a/src/core/link_tcp.go b/src/core/link_tcp.go index 986eda30..ee0dd001 100644 --- a/src/core/link_tcp.go +++ b/src/core/link_tcp.go @@ -31,14 +31,14 @@ func (l *links) newLinkTCP() *linkTCP { } func (l *linkTCP) dial(url *url.URL, options linkOptions, sintf string) error { - info := linkInfoFor("tcp", sintf, strings.SplitN(url.Host, "%", 2)[0]) - if l.links.isConnectedTo(info) { - return nil - } addr, err := net.ResolveTCPAddr("tcp", url.Host) if err != nil { return err } + info := linkInfoFor("tcp", sintf, addr.String()) + if l.links.isConnectedTo(info) { + return nil + } dialer, err := l.dialerFor(addr, sintf) if err != nil { return err @@ -47,7 +47,8 @@ func (l *linkTCP) dial(url *url.URL, options linkOptions, sintf string) error { if err != nil { return err } - return l.handler(url.String(), info, conn, options, false, false) + uri := strings.TrimRight(strings.SplitN(url.String(), "?", 2)[0], "/") + return l.handler(uri, info, conn, options, false, false) } func (l *linkTCP) listen(url *url.URL, sintf string) (*Listener, error) { @@ -83,7 +84,7 @@ func (l *linkTCP) listen(url *url.URL, sintf string) (*Listener, error) { } addr := conn.RemoteAddr().(*net.TCPAddr) name := fmt.Sprintf("tcp://%s", addr) - info := linkInfoFor("tcp", sintf, strings.SplitN(addr.IP.String(), "%", 2)[0]) + info := linkInfoFor("tcp", sintf, addr.String()) if err = l.handler(name, info, conn, linkOptions{}, true, addr.IP.IsLinkLocalUnicast()); err != nil { l.core.log.Errorln("Failed to create inbound link:", err) } diff --git a/src/core/link_tls.go b/src/core/link_tls.go index 9e7dda9b..ee3363ec 100644 --- a/src/core/link_tls.go +++ b/src/core/link_tls.go @@ -47,14 +47,14 @@ func (l *links) newLinkTLS(tcp *linkTCP) *linkTLS { } func (l *linkTLS) dial(url *url.URL, options linkOptions, sintf, sni string) error { - info := linkInfoFor("tls", sintf, strings.SplitN(url.Host, "%", 2)[0]) - if l.links.isConnectedTo(info) { - return nil - } addr, err := net.ResolveTCPAddr("tcp", url.Host) if err != nil { return err } + info := linkInfoFor("tls", sintf, addr.String()) + if l.links.isConnectedTo(info) { + return nil + } dialer, err := l.tcp.dialerFor(addr, sintf) if err != nil { return err @@ -69,7 +69,8 @@ func (l *linkTLS) dial(url *url.URL, options linkOptions, sintf, sni string) err if err != nil { return err } - return l.handler(url.String(), info, conn, options, false, false) + uri := strings.TrimRight(strings.SplitN(url.String(), "?", 2)[0], "/") + return l.handler(uri, info, conn, options, false, false) } func (l *linkTLS) listen(url *url.URL, sintf string) (*Listener, error) { @@ -106,7 +107,7 @@ func (l *linkTLS) listen(url *url.URL, sintf string) (*Listener, error) { } addr := conn.RemoteAddr().(*net.TCPAddr) name := fmt.Sprintf("tls://%s", addr) - info := linkInfoFor("tls", sintf, strings.SplitN(addr.IP.String(), "%", 2)[0]) + info := linkInfoFor("tls", sintf, addr.String()) if err = l.handler(name, info, conn, linkOptions{}, true, addr.IP.IsLinkLocalUnicast()); err != nil { l.core.log.Errorln("Failed to create inbound link:", err) }