From e849b3e1192f94f14f6ea0e0af50275956689171 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Fri, 8 May 2020 23:23:48 +0100 Subject: [PATCH 1/7] Initial support for pinning public keys in peering strings --- src/yggdrasil/link.go | 57 +++++++++++++++++++++++++++++++++++++------ src/yggdrasil/tcp.go | 42 ++++++++++++++++++------------- 2 files changed, 75 insertions(+), 24 deletions(-) diff --git a/src/yggdrasil/link.go b/src/yggdrasil/link.go index 157ea52..c0ae23b 100644 --- a/src/yggdrasil/link.go +++ b/src/yggdrasil/link.go @@ -1,6 +1,7 @@ package yggdrasil import ( + "bytes" "encoding/hex" "errors" "fmt" @@ -50,6 +51,7 @@ type linkInterface struct { name string link *link peer *peer + options linkOptions msgIO linkInterfaceMsgIO info linkInfo incoming bool @@ -67,6 +69,10 @@ type linkInterface struct { unstalled bool // False if an idle notification to the switch hasn't been sent because we stalled (or are first starting up) } +type linkOptions struct { + pinningInfo *url.Userinfo +} + func (l *link) init(c *Core) error { l.core = c l.mutex.Lock() @@ -92,13 +98,19 @@ func (l *link) call(uri string, sintf string) error { return fmt.Errorf("peer %s is not correctly formatted (%s)", uri, err) } pathtokens := strings.Split(strings.Trim(u.Path, "/"), "/") + tcpOpts := tcpOptions{} + if u.User != nil { + tcpOpts.pinningInfo = u.User + } switch u.Scheme { case "tcp": - l.tcp.call(u.Host, nil, sintf, nil) + l.tcp.call(u.Host, tcpOpts, sintf) case "socks": - l.tcp.call(pathtokens[0], u.Host, sintf, nil) + tcpOpts.socksProxyAddr = u.Host + l.tcp.call(pathtokens[0], tcpOpts, sintf) case "tls": - l.tcp.call(u.Host, nil, sintf, l.tcp.tls.forDialer) + tcpOpts.upgrade = l.tcp.tls.forDialer + l.tcp.call(u.Host, tcpOpts, sintf) default: return errors.New("unknown call scheme: " + u.Scheme) } @@ -122,12 +134,13 @@ func (l *link) listen(uri string) error { } } -func (l *link) create(msgIO linkInterfaceMsgIO, name, linkType, local, remote string, incoming, force bool) (*linkInterface, error) { +func (l *link) create(msgIO linkInterfaceMsgIO, name, linkType, local, remote string, incoming, force bool, options linkOptions) (*linkInterface, error) { // Technically anything unique would work for names, but let's pick something human readable, just for debugging intf := linkInterface{ - name: name, - link: l, - msgIO: msgIO, + name: name, + link: l, + options: options, + msgIO: msgIO, info: linkInfo{ linkType: linkType, local: local, @@ -181,6 +194,36 @@ func (intf *linkInterface) handler() error { intf.link.core.log.Errorln("Failed to connect to node: " + intf.name + " version: " + fmt.Sprintf("%d.%d", meta.ver, meta.minorVer)) return errors.New("failed to connect: wrong version") } + // Check if the remote side matches the keys we expected. This is a bit of a weak + // check - in future versions we really should check a signature or something like that. + if pinning := intf.options.pinningInfo; pinning != nil { + allowed := true + keytype := pinning.Username() + if pubkey, ok := pinning.Password(); ok { + switch keytype { + case "curve25519": + boxPub, err := hex.DecodeString(pubkey) + if err != nil || len(boxPub) != crypto.BoxPubKeyLen { + allowed = false + break + } + allowed = bytes.Compare(boxPub, meta.box[:]) == 0 + case "ed25519": + sigPub, err := hex.DecodeString(pubkey) + if err != nil || len(sigPub) != crypto.SigPubKeyLen { + allowed = false + break + } + allowed = bytes.Compare(sigPub, meta.sig[:]) == 0 + } + } else { + allowed = false + } + if !allowed { + intf.link.core.log.Errorf("Failed to connect to node: %q sent key that does not match pinned %q key", intf.name, keytype) + return fmt.Errorf("failed to connect: host does not match pinned %q key", pinning.Username()) + } + } // Check if we're authorized to connect to this key / IP if intf.incoming && !intf.force && !intf.link.core.peers.isAllowedEncryptionPublicKey(&meta.box) { intf.link.core.log.Warnf("%s connection from %s forbidden: AllowedEncryptionPublicKeys does not contain key %s", diff --git a/src/yggdrasil/tcp.go b/src/yggdrasil/tcp.go index 9cca419..a1b6402 100644 --- a/src/yggdrasil/tcp.go +++ b/src/yggdrasil/tcp.go @@ -57,6 +57,12 @@ type TcpUpgrade struct { name string } +type tcpOptions struct { + linkOptions + upgrade *TcpUpgrade + socksProxyAddr string +} + func (l *TcpListener) Stop() { defer func() { recover() }() close(l.stop) @@ -221,7 +227,10 @@ func (t *tcp) listener(l *TcpListener, listenaddr string) { return } t.waitgroup.Add(1) - go t.handler(sock, true, nil, l.upgrade) + options := tcpOptions{ + upgrade: l.upgrade, + } + go t.handler(sock, true, options) } } @@ -239,12 +248,12 @@ func (t *tcp) startCalling(saddr string) bool { // If the dial is successful, it launches the handler. // When finished, it removes the outgoing call, so reconnection attempts can be made later. // This all happens in a separate goroutine that it spawns. -func (t *tcp) call(saddr string, options interface{}, sintf string, upgrade *TcpUpgrade) { +func (t *tcp) call(saddr string, options tcpOptions, sintf string) { go func() { callname := saddr callproto := "TCP" - if upgrade != nil { - callproto = strings.ToUpper(upgrade.name) + if options.upgrade != nil { + callproto = strings.ToUpper(options.upgrade.name) } if sintf != "" { callname = fmt.Sprintf("%s/%s/%s", callproto, saddr, sintf) @@ -263,12 +272,11 @@ func (t *tcp) call(saddr string, options interface{}, sintf string, upgrade *Tcp }() var conn net.Conn var err error - socksaddr, issocks := options.(string) - if issocks { + if options.socksProxyAddr != "" { if sintf != "" { return } - dialerdst, er := net.ResolveTCPAddr("tcp", socksaddr) + dialerdst, er := net.ResolveTCPAddr("tcp", options.socksProxyAddr) if er != nil { return } @@ -282,7 +290,7 @@ func (t *tcp) call(saddr string, options interface{}, sintf string, upgrade *Tcp return } t.waitgroup.Add(1) - t.handler(conn, false, saddr, nil) + t.handler(conn, false, options) } else { dst, err := net.ResolveTCPAddr("tcp", saddr) if err != nil { @@ -348,19 +356,19 @@ func (t *tcp) call(saddr string, options interface{}, sintf string, upgrade *Tcp return } t.waitgroup.Add(1) - t.handler(conn, false, nil, upgrade) + t.handler(conn, false, options) } }() } -func (t *tcp) handler(sock net.Conn, incoming bool, options interface{}, upgrade *TcpUpgrade) { +func (t *tcp) handler(sock net.Conn, incoming bool, options tcpOptions) { defer t.waitgroup.Done() // Happens after sock.close defer sock.Close() t.setExtraOptions(sock) var upgraded bool - if upgrade != nil { + if options.upgrade != nil { var err error - if sock, err = upgrade.upgrade(sock); err != nil { + if sock, err = options.upgrade.upgrade(sock); err != nil { t.link.core.log.Errorln("TCP handler upgrade failed:", err) return } else { @@ -370,14 +378,14 @@ func (t *tcp) handler(sock net.Conn, incoming bool, options interface{}, upgrade stream := stream{} stream.init(sock) var name, proto, local, remote string - if socksaddr, issocks := options.(string); issocks { - name = "socks://" + sock.RemoteAddr().String() + "/" + socksaddr + if options.socksProxyAddr != "" { + name = "socks://" + sock.RemoteAddr().String() + "/" + options.socksProxyAddr proto = "socks" local, _, _ = net.SplitHostPort(sock.LocalAddr().String()) - remote, _, _ = net.SplitHostPort(socksaddr) + remote, _, _ = net.SplitHostPort(options.socksProxyAddr) } else { if upgraded { - proto = upgrade.name + proto = options.upgrade.name name = proto + "://" + sock.RemoteAddr().String() } else { proto = "tcp" @@ -387,7 +395,7 @@ func (t *tcp) handler(sock net.Conn, incoming bool, options interface{}, upgrade 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) + link, err := t.link.core.link.create(&stream, name, proto, local, remote, incoming, force, options.linkOptions) if err != nil { t.link.core.log.Println(err) panic(err) From fbf59184ee4a7005fc5768a251826c1633637160 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Sat, 9 May 2020 00:43:19 +0100 Subject: [PATCH 2/7] Use query string instead, allow specifying multiple keys (might be useful for DNS RR) --- src/yggdrasil/link.go | 66 +++++++++++++++++++++++++------------------ 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/src/yggdrasil/link.go b/src/yggdrasil/link.go index c0ae23b..40d128e 100644 --- a/src/yggdrasil/link.go +++ b/src/yggdrasil/link.go @@ -70,7 +70,8 @@ type linkInterface struct { } type linkOptions struct { - pinningInfo *url.Userinfo + pinnedCurve25519Keys []crypto.BoxPubKey + pinnedEd25519Keys []crypto.SigPubKey } func (l *link) init(c *Core) error { @@ -99,8 +100,27 @@ func (l *link) call(uri string, sintf string) error { } pathtokens := strings.Split(strings.Trim(u.Path, "/"), "/") tcpOpts := tcpOptions{} - if u.User != nil { - tcpOpts.pinningInfo = u.User + if pubkeys, ok := u.Query()["curve25519"]; ok && len(pubkeys) > 0 { + for _, pubkey := range pubkeys { + if boxPub, err := hex.DecodeString(pubkey); err != nil { + var boxPubKey crypto.BoxPubKey + copy(boxPubKey[:], boxPub) + tcpOpts.pinnedCurve25519Keys = append( + tcpOpts.pinnedCurve25519Keys, boxPubKey, + ) + } + } + } + if pubkeys, ok := u.Query()["ed25519"]; ok && len(pubkeys) > 0 { + for _, pubkey := range pubkeys { + if sigPub, err := hex.DecodeString(pubkey); err != nil { + var sigPubKey crypto.SigPubKey + copy(sigPubKey[:], sigPub) + tcpOpts.pinnedEd25519Keys = append( + tcpOpts.pinnedEd25519Keys, sigPubKey, + ) + } + } } switch u.Scheme { case "tcp": @@ -196,32 +216,24 @@ func (intf *linkInterface) handler() error { } // Check if the remote side matches the keys we expected. This is a bit of a weak // check - in future versions we really should check a signature or something like that. - if pinning := intf.options.pinningInfo; pinning != nil { - allowed := true - keytype := pinning.Username() - if pubkey, ok := pinning.Password(); ok { - switch keytype { - case "curve25519": - boxPub, err := hex.DecodeString(pubkey) - if err != nil || len(boxPub) != crypto.BoxPubKeyLen { - allowed = false - break - } - allowed = bytes.Compare(boxPub, meta.box[:]) == 0 - case "ed25519": - sigPub, err := hex.DecodeString(pubkey) - if err != nil || len(sigPub) != crypto.SigPubKeyLen { - allowed = false - break - } - allowed = bytes.Compare(sigPub, meta.sig[:]) == 0 - } - } else { - allowed = false + if pinned := intf.options.pinnedCurve25519Keys; len(pinned) > 0 { + allowed := false + for _, key := range pinned { + allowed = allowed || (bytes.Compare(key[:], meta.box[:]) == 0) } if !allowed { - intf.link.core.log.Errorf("Failed to connect to node: %q sent key that does not match pinned %q key", intf.name, keytype) - return fmt.Errorf("failed to connect: host does not match pinned %q key", pinning.Username()) + intf.link.core.log.Errorf("Failed to connect to node: %q sent curve25519 key that does not match pinned keys", intf.name) + return fmt.Errorf("failed to connect: host sent curve25519 key that does not match pinned keys") + } + } + if pinned := intf.options.pinnedEd25519Keys; len(pinned) > 0 { + allowed := false + for _, key := range pinned { + allowed = allowed || (bytes.Compare(key[:], meta.sig[:]) == 0) + } + if !allowed { + intf.link.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") } } // Check if we're authorized to connect to this key / IP From 58345ac198acafbfb219a52a1e66358483347394 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Sat, 9 May 2020 10:53:58 +0100 Subject: [PATCH 3/7] Track proxy addr and real peer addr in SOCKS mode --- src/yggdrasil/tcp.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/yggdrasil/tcp.go b/src/yggdrasil/tcp.go index a1b6402..6f97da8 100644 --- a/src/yggdrasil/tcp.go +++ b/src/yggdrasil/tcp.go @@ -61,6 +61,7 @@ type tcpOptions struct { linkOptions upgrade *TcpUpgrade socksProxyAddr string + socksPeerAddr string } func (l *TcpListener) Stop() { @@ -290,6 +291,7 @@ func (t *tcp) call(saddr string, options tcpOptions, sintf string) { return } t.waitgroup.Add(1) + options.socksPeerAddr = saddr t.handler(conn, false, options) } else { dst, err := net.ResolveTCPAddr("tcp", saddr) @@ -379,10 +381,10 @@ func (t *tcp) handler(sock net.Conn, incoming bool, options tcpOptions) { stream.init(sock) var name, proto, local, remote string if options.socksProxyAddr != "" { - name = "socks://" + sock.RemoteAddr().String() + "/" + options.socksProxyAddr + name = "socks://" + sock.RemoteAddr().String() + "/" + options.socksPeerAddr proto = "socks" local, _, _ = net.SplitHostPort(sock.LocalAddr().String()) - remote, _, _ = net.SplitHostPort(options.socksProxyAddr) + remote, _, _ = net.SplitHostPort(options.socksPeerAddr) } else { if upgraded { proto = options.upgrade.name From 8b180e941aa7ccf178a7e82c5a11d172c23d6331 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Sat, 9 May 2020 11:24:32 +0100 Subject: [PATCH 4/7] Add SOCKS proxy auth (closes #423) --- src/yggdrasil/link.go | 6 ++++++ src/yggdrasil/tcp.go | 3 ++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/yggdrasil/link.go b/src/yggdrasil/link.go index 40d128e..5c4f0e6 100644 --- a/src/yggdrasil/link.go +++ b/src/yggdrasil/link.go @@ -17,6 +17,7 @@ import ( "github.com/yggdrasil-network/yggdrasil-go/src/address" "github.com/yggdrasil-network/yggdrasil-go/src/crypto" "github.com/yggdrasil-network/yggdrasil-go/src/util" + "golang.org/x/net/proxy" "github.com/Arceliar/phony" ) @@ -127,6 +128,11 @@ func (l *link) call(uri string, sintf string) error { l.tcp.call(u.Host, tcpOpts, sintf) case "socks": tcpOpts.socksProxyAddr = u.Host + if u.User != nil { + tcpOpts.socksProxyAuth = &proxy.Auth{} + tcpOpts.socksProxyAuth.User = u.User.Username() + tcpOpts.socksProxyAuth.Password, _ = u.User.Password() + } l.tcp.call(pathtokens[0], tcpOpts, sintf) case "tls": tcpOpts.upgrade = l.tcp.tls.forDialer diff --git a/src/yggdrasil/tcp.go b/src/yggdrasil/tcp.go index 6f97da8..e137391 100644 --- a/src/yggdrasil/tcp.go +++ b/src/yggdrasil/tcp.go @@ -61,6 +61,7 @@ type tcpOptions struct { linkOptions upgrade *TcpUpgrade socksProxyAddr string + socksProxyAuth *proxy.Auth socksPeerAddr string } @@ -282,7 +283,7 @@ func (t *tcp) call(saddr string, options tcpOptions, sintf string) { return } var dialer proxy.Dialer - dialer, err = proxy.SOCKS5("tcp", dialerdst.String(), nil, proxy.Direct) + dialer, err = proxy.SOCKS5("tcp", dialerdst.String(), options.socksProxyAuth, proxy.Direct) if err != nil { return } From 13a2d99fdc427753b3ccd433e0bdfee8fa5930a8 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Sat, 9 May 2020 11:26:09 +0100 Subject: [PATCH 5/7] Set SOCKS peer addr to resolved address --- src/yggdrasil/tcp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/yggdrasil/tcp.go b/src/yggdrasil/tcp.go index e137391..c81c4dd 100644 --- a/src/yggdrasil/tcp.go +++ b/src/yggdrasil/tcp.go @@ -292,7 +292,7 @@ func (t *tcp) call(saddr string, options tcpOptions, sintf string) { return } t.waitgroup.Add(1) - options.socksPeerAddr = saddr + options.socksPeerAddr = conn.RemoteAddr().String() t.handler(conn, false, options) } else { dst, err := net.ResolveTCPAddr("tcp", saddr) From 2a2ad76479a58ca01842453d89d370b70cc24b4d Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Sat, 9 May 2020 12:38:20 +0100 Subject: [PATCH 6/7] Use maps instead of slices --- src/yggdrasil/link.go | 31 ++++++++++--------------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/src/yggdrasil/link.go b/src/yggdrasil/link.go index 5c4f0e6..03e6007 100644 --- a/src/yggdrasil/link.go +++ b/src/yggdrasil/link.go @@ -1,7 +1,6 @@ package yggdrasil import ( - "bytes" "encoding/hex" "errors" "fmt" @@ -71,8 +70,8 @@ type linkInterface struct { } type linkOptions struct { - pinnedCurve25519Keys []crypto.BoxPubKey - pinnedEd25519Keys []crypto.SigPubKey + pinnedCurve25519Keys map[crypto.BoxPubKey]struct{} + pinnedEd25519Keys map[crypto.SigPubKey]struct{} } func (l *link) init(c *Core) error { @@ -102,24 +101,22 @@ func (l *link) call(uri string, sintf string) error { pathtokens := strings.Split(strings.Trim(u.Path, "/"), "/") tcpOpts := tcpOptions{} if pubkeys, ok := u.Query()["curve25519"]; ok && len(pubkeys) > 0 { + tcpOpts.pinnedCurve25519Keys = make(map[crypto.BoxPubKey]struct{}) for _, pubkey := range pubkeys { if boxPub, err := hex.DecodeString(pubkey); err != nil { var boxPubKey crypto.BoxPubKey copy(boxPubKey[:], boxPub) - tcpOpts.pinnedCurve25519Keys = append( - tcpOpts.pinnedCurve25519Keys, boxPubKey, - ) + tcpOpts.pinnedCurve25519Keys[boxPubKey] = struct{}{} } } } if pubkeys, ok := u.Query()["ed25519"]; ok && len(pubkeys) > 0 { + tcpOpts.pinnedEd25519Keys = make(map[crypto.SigPubKey]struct{}) for _, pubkey := range pubkeys { if sigPub, err := hex.DecodeString(pubkey); err != nil { var sigPubKey crypto.SigPubKey copy(sigPubKey[:], sigPub) - tcpOpts.pinnedEd25519Keys = append( - tcpOpts.pinnedEd25519Keys, sigPubKey, - ) + tcpOpts.pinnedEd25519Keys[sigPubKey] = struct{}{} } } } @@ -222,22 +219,14 @@ func (intf *linkInterface) handler() error { } // Check if the remote side matches the keys we expected. This is a bit of a weak // check - in future versions we really should check a signature or something like that. - if pinned := intf.options.pinnedCurve25519Keys; len(pinned) > 0 { - allowed := false - for _, key := range pinned { - allowed = allowed || (bytes.Compare(key[:], meta.box[:]) == 0) - } - if !allowed { + if pinned := intf.options.pinnedCurve25519Keys; pinned != nil { + if _, allowed := pinned[meta.box]; !allowed { intf.link.core.log.Errorf("Failed to connect to node: %q sent curve25519 key that does not match pinned keys", intf.name) return fmt.Errorf("failed to connect: host sent curve25519 key that does not match pinned keys") } } - if pinned := intf.options.pinnedEd25519Keys; len(pinned) > 0 { - allowed := false - for _, key := range pinned { - allowed = allowed || (bytes.Compare(key[:], meta.sig[:]) == 0) - } - if !allowed { + if pinned := intf.options.pinnedEd25519Keys; pinned != nil { + if _, allowed := pinned[meta.sig]; !allowed { intf.link.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") } From f70b2ebceaff118e67541f3d9d0f714a00fe0dc7 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Sat, 9 May 2020 12:49:02 +0100 Subject: [PATCH 7/7] Fix bad check --- src/yggdrasil/link.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/yggdrasil/link.go b/src/yggdrasil/link.go index 03e6007..0b37eaa 100644 --- a/src/yggdrasil/link.go +++ b/src/yggdrasil/link.go @@ -103,7 +103,7 @@ func (l *link) call(uri string, sintf string) error { if pubkeys, ok := u.Query()["curve25519"]; ok && len(pubkeys) > 0 { tcpOpts.pinnedCurve25519Keys = make(map[crypto.BoxPubKey]struct{}) for _, pubkey := range pubkeys { - if boxPub, err := hex.DecodeString(pubkey); err != nil { + if boxPub, err := hex.DecodeString(pubkey); err == nil { var boxPubKey crypto.BoxPubKey copy(boxPubKey[:], boxPub) tcpOpts.pinnedCurve25519Keys[boxPubKey] = struct{}{} @@ -113,7 +113,7 @@ func (l *link) call(uri string, sintf string) error { if pubkeys, ok := u.Query()["ed25519"]; ok && len(pubkeys) > 0 { tcpOpts.pinnedEd25519Keys = make(map[crypto.SigPubKey]struct{}) for _, pubkey := range pubkeys { - if sigPub, err := hex.DecodeString(pubkey); err != nil { + if sigPub, err := hex.DecodeString(pubkey); err == nil { var sigPubKey crypto.SigPubKey copy(sigPubKey[:], sigPub) tcpOpts.pinnedEd25519Keys[sigPubKey] = struct{}{}