derp: move away from [32]byte key types

And some minor cleanup in the process.

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

derp/derp.go

@@ -0,0 +1,76 @@
+// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package derp implements DERP, the Detour Encrypted Routing Protocol.
+//
+// DERP routes packets to clients using curve25519 keys as addresses.
+//
+// DERP is used by Tailscale nodes to proxy encrypted WireGuard
+// packets through the Tailscale cloud servers when a direct path
+// cannot be found or opened. DERP is a last resort. Both sides
+// between very aggressive NATs, firewalls, no IPv6, etc? Well, DERP.
+package derp
+
+import (
+	"bufio"
+	"encoding/binary"
+	"fmt"
+	"io"
+	"time"
+)
+
+// magic is the derp magic number, sent on the wire as a uint32.
+// It's "DERP" with a non-ASCII high-bit.
+const magic = 0x44c55250
+
+// frameType is the one byte frame type header in frame headers.
+type frameType byte
+
+const (
+	typeServerKey  = frameType(0x01)
+	typeServerInfo = frameType(0x02)
+	typeSendPacket = frameType(0x03)
+	typeRecvPacket = frameType(0x04)
+	typeKeepAlive  = frameType(0x05)
+)
+
+func (b frameType) Write(w io.ByteWriter) error {
+	return w.WriteByte(byte(b))
+}
+
+const keepAlive = 60 * time.Second
+
+var bin = binary.BigEndian
+
+const oneMB = 1 << 20
+
+func readType(r *bufio.Reader, t frameType) error {
+	packetType, err := r.ReadByte()
+	if err != nil {
+		return err
+	}
+	if frameType(packetType) != t {
+		return fmt.Errorf("bad packet type 0x%X, want 0x%X", packetType, t)
+	}
+	return nil
+}
+
+func putUint32(w io.Writer, v uint32) error {
+	var b [4]byte
+	bin.PutUint32(b[:], v)
+	_, err := w.Write(b[:])
+	return err
+}
+
+func readUint32(r io.Reader, maxVal uint32) (uint32, error) {
+	b := make([]byte, 4)
+	if _, err := io.ReadFull(r, b); err != nil {
+		return 0, err
+	}
+	val := bin.Uint32(b)
+	if val > maxVal {
+		return 0, fmt.Errorf("uint32 %d exceeds limit %d", val, maxVal)
+	}
+	return val, nil
+}

derp/derp_client.go

@@ -117,14 +117,14 @@ func (c *Client) sendClientKey() error {
 	return c.conn.Flush()
 }
 
-func (c *Client) Send(dstKey [32]byte, msg []byte) (err error) {
+func (c *Client) Send(dstKey key.Public, msg []byte) (err error) {
 	defer func() {
 		if err != nil {
 			err = fmt.Errorf("derp.Send: %v", err)
 		}
 	}()
 
-	if err := c.conn.WriteByte(typeSendPacket); err != nil {
+	if err := typeSendPacket.Write(c.conn); err != nil {
 		return err
 	}
 	if _, err := c.conn.Write(dstKey[:]); err != nil {
@@ -153,17 +153,17 @@ func (c *Client) Recv(b []byte) (n int, err error) {
 loop:
 	for {
 		c.netConn.SetReadDeadline(time.Now().Add(120 * time.Second))
-		packetType, err := c.conn.ReadByte()
+		typ, err := c.conn.ReadByte()
 		if err != nil {
 			return 0, err
 		}
-		switch packetType {
+		switch frameType(typ) {
 		case typeKeepAlive:
 			continue
 		case typeRecvPacket:
 			break loop
 		default:
-			return 0, fmt.Errorf("derp.Recv: unknown packet type %d", packetType)
+			return 0, fmt.Errorf("derp.Recv: unknown packet type 0x%X", typ)
 		}
 	}
 

derp/derp_server.go

@@ -11,8 +11,7 @@
 import (
 	"bufio"
 	"context"
-	"crypto/rand"
+	crand "crypto/rand"
-	"encoding/binary"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -21,45 +20,29 @@
 	"sync"
 	"time"
 
-	"golang.org/x/crypto/curve25519"
 	"golang.org/x/crypto/nacl/box"
+	"tailscale.com/types/key"
 	"tailscale.com/types/logger"
 )
 
-const magic = 0x44c55250 // "DERP" with a non-ASCII high-bit
-
-const (
-	typeServerKey  = 0x01
-	typeServerInfo = 0x02
-	typeSendPacket = 0x03
-	typeRecvPacket = 0x04
-	typeKeepAlive  = 0x05
-)
-
-const keepAlive = 60 * time.Second
-
-var bin = binary.BigEndian
-
-const oneMB = 1 << 20
-
 type Server struct {
-	privateKey [32]byte // TODO(crawshaw): make this wgcfg.PrivateKey?
+	privateKey key.Private
-	publicKey  [32]byte
+	publicKey  key.Public
 	logf       logger.Logf
 
 	mu       sync.Mutex
-	netConns map[net.Conn]chan struct{}
+	netConns map[net.Conn]chan struct{} // chan is closed when conn closes
-	clients  map[[32]byte]*client
+	clients  map[key.Public]*client
 }
 
-func NewServer(privateKey [32]byte, logf logger.Logf) *Server {
+func NewServer(privateKey key.Private, logf logger.Logf) *Server {
 	s := &Server{
 		privateKey: privateKey,
+		publicKey:  privateKey.Public(),
 		logf:       logf,
-		clients:    make(map[[32]byte]*client),
+		clients:    make(map[key.Public]*client),
 		netConns:   make(map[net.Conn]chan struct{}),
 	}
-	curve25519.ScalarBaseMult(&s.publicKey, &s.privateKey)
 	return s
 }
 
@@ -182,7 +165,7 @@ func (s *Server) accept(netConn net.Conn, conn *bufio.ReadWriter) error {
 		}
 
 		dst.mu.Lock()
-		err = s.sendPacket(dst.conn, c.key, contents)
+		err = s.sendPacket(dst.conn.Writer, c.key, contents)
 		dst.mu.Unlock()
 
 		if err != nil {
@@ -199,7 +182,7 @@ func (s *Server) accept(netConn net.Conn, conn *bufio.ReadWriter) error {
 	}
 }
 
-func (s *Server) verifyClient(clientKey [32]byte, info *clientInfo) error {
+func (s *Server) verifyClient(clientKey key.Public, info *clientInfo) error {
 	// TODO(crawshaw): implement policy constraints on who can use the DERP server
 	return nil
 }
@@ -208,7 +191,7 @@ func (s *Server) sendServerKey(conn *bufio.ReadWriter) error {
 	if err := putUint32(conn, magic); err != nil {
 		return err
 	}
-	if err := conn.WriteByte(typeServerKey); err != nil {
+	if err := typeServerKey.Write(conn); err != nil {
 		return err
 	}
 	if _, err := conn.Write(s.publicKey[:]); err != nil {
@@ -217,15 +200,15 @@ func (s *Server) sendServerKey(conn *bufio.ReadWriter) error {
 	return conn.Flush()
 }
 
-func (s *Server) sendServerInfo(conn *bufio.ReadWriter, clientKey [32]byte) error {
+func (s *Server) sendServerInfo(conn *bufio.ReadWriter, clientKey key.Public) error {
 	var nonce [24]byte
-	if _, err := rand.Read(nonce[:]); err != nil {
+	if _, err := crand.Read(nonce[:]); err != nil {
 		return err
 	}
 	msg := []byte("{}") // no serverInfo for now
-	msgbox := box.Seal(nil, msg, &nonce, &clientKey, &s.privateKey)
+	msgbox := box.Seal(nil, msg, &nonce, clientKey.B32(), s.privateKey.B32())
 
-	if err := conn.WriteByte(typeServerInfo); err != nil {
+	if err := typeServerInfo.Write(conn); err != nil {
 		return err
 	}
 	if _, err := conn.Write(nonce[:]); err != nil {
@@ -240,67 +223,67 @@ func (s *Server) sendServerInfo(conn *bufio.ReadWriter, clientKey [32]byte) erro
 	return conn.Flush()
 }
 
-func (s *Server) recvClientKey(conn *bufio.ReadWriter) (clientKey [32]byte, info *clientInfo, err error) {
+func (s *Server) recvClientKey(conn *bufio.ReadWriter) (clientKey key.Public, info *clientInfo, err error) {
 	if _, err := io.ReadFull(conn, clientKey[:]); err != nil {
-		return [32]byte{}, nil, err
+		return key.Public{}, nil, err
 	}
 	var nonce [24]byte
 	if _, err := io.ReadFull(conn, nonce[:]); err != nil {
-		return [32]byte{}, nil, fmt.Errorf("nonce: %v", err)
+		return key.Public{}, nil, fmt.Errorf("nonce: %v", err)
 	}
 	msgLen, err := readUint32(conn, oneMB)
 	if err != nil {
-		return [32]byte{}, nil, fmt.Errorf("msglen: %v", err)
+		return key.Public{}, nil, fmt.Errorf("msglen: %v", err)
 	}
 	msgbox := make([]byte, msgLen)
 	if _, err := io.ReadFull(conn, msgbox); err != nil {
-		return [32]byte{}, nil, fmt.Errorf("msgbox: %v", err)
+		return key.Public{}, nil, fmt.Errorf("msgbox: %v", err)
 	}
-	msg, ok := box.Open(nil, msgbox, &nonce, &clientKey, &s.privateKey)
+	msg, ok := box.Open(nil, msgbox, &nonce, (*[32]byte)(&clientKey), s.privateKey.B32())
 	if !ok {
-		return [32]byte{}, nil, fmt.Errorf("msgbox: cannot open len=%d with client key %x", msgLen, clientKey[:])
+		return key.Public{}, nil, fmt.Errorf("msgbox: cannot open len=%d with client key %x", msgLen, clientKey[:])
 	}
 	info = new(clientInfo)
 	if err := json.Unmarshal(msg, info); err != nil {
-		return [32]byte{}, nil, fmt.Errorf("msg: %v", err)
+		return key.Public{}, nil, fmt.Errorf("msg: %v", err)
 	}
 	return clientKey, info, nil
 }
 
-func (s *Server) sendPacket(conn *bufio.ReadWriter, srcKey [32]byte, contents []byte) error {
+func (s *Server) sendPacket(bw *bufio.Writer, srcKey key.Public, contents []byte) error {
-	if err := conn.WriteByte(typeRecvPacket); err != nil {
+	if err := typeRecvPacket.Write(bw); err != nil {
 		return err
 	}
-	if err := putUint32(conn.Writer, uint32(len(contents))); err != nil {
+	if err := putUint32(bw, uint32(len(contents))); err != nil {
 		return err
 	}
-	if _, err := conn.Write(contents); err != nil {
+	if _, err := bw.Write(contents); err != nil {
 		return err
 	}
-	return conn.Flush()
+	return bw.Flush()
 }
 
-func (s *Server) recvPacket(conn *bufio.ReadWriter) (dstKey [32]byte, contents []byte, err error) {
+func (s *Server) recvPacket(conn *bufio.ReadWriter) (dstKey key.Public, contents []byte, err error) {
 	if err := readType(conn.Reader, typeSendPacket); err != nil {
-		return [32]byte{}, nil, err
+		return key.Public{}, nil, err
 	}
 	if _, err := io.ReadFull(conn, dstKey[:]); err != nil {
-		return [32]byte{}, nil, err
+		return key.Public{}, nil, err
 	}
 	packetLen, err := readUint32(conn.Reader, oneMB)
 	if err != nil {
-		return [32]byte{}, nil, err
+		return key.Public{}, nil, err
 	}
 	contents = make([]byte, packetLen)
 	if _, err := io.ReadFull(conn, contents); err != nil {
-		return [32]byte{}, nil, err
+		return key.Public{}, nil, err
 	}
 	return dstKey, contents, nil
 }
 
 type client struct {
 	netConn net.Conn
-	key     [32]byte
+	key     key.Public
 	info    clientInfo
 
 	keepAliveTimer *time.Timer
@@ -311,7 +294,7 @@ type client struct {
 }
 
 func (c *client) keepAlive(ctx context.Context) error {
-	jitterMs, err := rand.Int(rand.Reader, big.NewInt(5000))
+	jitterMs, err := crand.Int(crand.Reader, big.NewInt(5000))
 	if err != nil {
 		panic(err)
 	}
@@ -329,7 +312,7 @@ func (c *client) keepAlive(ctx context.Context) error {
 			c.keepAliveTimer.Reset(keepAlive + jitter)
 		case <-c.keepAliveTimer.C:
 			c.mu.Lock()
-			err := c.conn.WriteByte(typeKeepAlive)
+			err := typeKeepAlive.Write(c.conn)
 			if err == nil {
 				err = c.conn.Flush()
 			}
@@ -349,33 +332,3 @@ type clientInfo struct {
 
 type serverInfo struct {
 }
-
-func readType(r *bufio.Reader, t uint8) error {
-	packetType, err := r.ReadByte()
-	if err != nil {
-		return err
-	}
-	if packetType != t {
-		return fmt.Errorf("bad packet type 0x%X, want 0x%X", packetType, t)
-	}
-	return nil
-}
-
-func putUint32(w io.Writer, v uint32) error {
-	var b [4]byte
-	bin.PutUint32(b[:], v)
-	_, err := w.Write(b[:])
-	return err
-}
-
-func readUint32(r io.Reader, maxVal uint32) (uint32, error) {
-	b := make([]byte, 4)
-	if _, err := io.ReadFull(r, b); err != nil {
-		return 0, err
-	}
-	val := bin.Uint32(b)
-	if val > maxVal {
-		return 0, fmt.Errorf("uint32 %d exceeds limit %d", val, maxVal)
-	}
-	return val, nil
-}

derp/derp_test.go

@@ -6,33 +6,31 @@
 
 import (
 	"bufio"
-	"crypto/rand"
+	crand "crypto/rand"
 	"net"
 	"testing"
 	"time"
 
-	"golang.org/x/crypto/curve25519"
+	"tailscale.com/types/key"
 )
 
 func TestSendRecv(t *testing.T) {
 	const numClients = 3
-	var serverPrivateKey [32]byte
+	var serverPrivateKey key.Private
-	if _, err := rand.Read(serverPrivateKey[:]); err != nil {
+	if _, err := crand.Read(serverPrivateKey[:]); err != nil {
 		t.Fatal(err)
 	}
-	var clientPrivateKeys [][32]byte
+	var clientPrivateKeys []key.Private
 	for i := 0; i < numClients; i++ {
-		var key [32]byte
+		var key key.Private
-		if _, err := rand.Read(key[:]); err != nil {
+		if _, err := crand.Read(key[:]); err != nil {
 			t.Fatal(err)
 		}
 		clientPrivateKeys = append(clientPrivateKeys, key)
 	}
-	var clientKeys [][32]byte
+	var clientKeys []key.Public
 	for _, privKey := range clientPrivateKeys {
-		var key [32]byte
+		clientKeys = append(clientKeys, privKey.Public())
-		curve25519.ScalarBaseMult(&key, &privKey)
-		clientKeys = append(clientKeys, key)
 	}
 
 	ln, err := net.Listen("tcp", ":0")

derp/derphttp/derphttp_client.go

@@ -23,6 +23,7 @@
 	"sync"
 
 	"tailscale.com/derp"
+	"tailscale.com/types/key"
 	"tailscale.com/types/logger"
 )
 
@@ -32,7 +33,7 @@
 // Recv will report the error and not retry, but subsequent calls to
 // Send/Recv will completely re-establish the connection.
 type Client struct {
-	privateKey [32]byte
+	privateKey key.Private
 	logf       logger.Logf
 	closed     chan struct{}
 	url        *url.URL
@@ -45,7 +46,7 @@ type Client struct {
 	client   *derp.Client
 }
 
-func NewClient(privateKey [32]byte, serverURL string, logf logger.Logf) (*Client, error) {
+func NewClient(privateKey key.Private, serverURL string, logf logger.Logf) (*Client, error) {
 	u, err := url.Parse(serverURL)
 	if err != nil {
 		return nil, fmt.Errorf("derphttp.NewClient: %v", err)
@@ -146,7 +147,7 @@ func (c *Client) connect(caller string) (client *derp.Client, err error) {
 	return c.client, nil
 }
 
-func (c *Client) Send(dstKey [32]byte, b []byte) error {
+func (c *Client) Send(dstKey key.Public, b []byte) error {
 	client, err := c.connect("derphttp.Client.Send")
 	if err != nil {
 		return err

derp/derphttp/derphttp_test.go

@@ -5,7 +5,7 @@
 package derphttp
 
 import (
-	"crypto/rand"
+	crand "crypto/rand"
 	"crypto/tls"
 	"net"
 	"net/http"
@@ -13,29 +13,27 @@
 	"testing"
 	"time"
 
-	"golang.org/x/crypto/curve25519"
 	"tailscale.com/derp"
+	"tailscale.com/types/key"
 )
 
 func TestSendRecv(t *testing.T) {
 	const numClients = 3
-	var serverPrivateKey [32]byte
+	var serverPrivateKey key.Private
-	if _, err := rand.Read(serverPrivateKey[:]); err != nil {
+	if _, err := crand.Read(serverPrivateKey[:]); err != nil {
 		t.Fatal(err)
 	}
-	var clientPrivateKeys [][32]byte
+	var clientPrivateKeys []key.Private
 	for i := 0; i < numClients; i++ {
-		var key [32]byte
+		var key key.Private
-		if _, err := rand.Read(key[:]); err != nil {
+		if _, err := crand.Read(key[:]); err != nil {
 			t.Fatal(err)
 		}
 		clientPrivateKeys = append(clientPrivateKeys, key)
 	}
-	var clientKeys [][32]byte
+	var clientKeys []key.Public
 	for _, privKey := range clientPrivateKeys {
-		var key [32]byte
+		clientKeys = append(clientKeys, privKey.Public())
-		curve25519.ScalarBaseMult(&key, &privKey)
-		clientKeys = append(clientKeys, key)
 	}
 
 	s := derp.NewServer(serverPrivateKey, t.Logf)

derp/doc.go

@@ -1,13 +0,0 @@
-// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// Package derp implements DERP, the Detour Encrypted Routing Protocol.
-//
-// DERP routes packets to clients using curve25519 keys as addresses.
-//
-// DERP is used by Tailscale nodes to proxy encrypted WireGuard
-// packets through the Tailscale cloud servers when a direct path
-// cannot be found or opened. DERP is a last resort. Both sides
-// between very aggressive NATs, firewalls, no IPv6, etc? Well, DERP.
-package derp

wgengine/magicsock/magicsock.go

@@ -22,6 +22,7 @@
 	"tailscale.com/derp/derphttp"
 	"tailscale.com/stun"
 	"tailscale.com/stunner"
+	"tailscale.com/types/key"
 )
 
 // A Conn routes UDP packets and actively manages a list of its endpoints.
@@ -446,12 +447,12 @@ func (c *Conn) ReceiveIPv6(buff []byte) (int, device.Endpoint, *net.UDPAddr, err
 	return 0, nil, nil, syscall.EAFNOSUPPORT
 }
 
-func (c *Conn) SetPrivateKey(privateKey [32]byte) error {
+func (c *Conn) SetPrivateKey(privateKey wgcfg.PrivateKey) error {
 	if c.derpServer == "" {
 		return nil
 	}
 
-	derp, err := derphttp.NewClient(privateKey, c.derpServer, log.Printf)
+	derp, err := derphttp.NewClient(key.Private(privateKey), c.derpServer, log.Printf)
 	if err != nil {
 		return err
 	}
@@ -528,7 +529,7 @@ func (c *Conn) LinkChange() {
 
 // AddrSet is a set of UDP addresses that implements wireguard/device.Endpoint.
 type AddrSet struct {
-	publicKey [32]byte      // peer public key used for DERP communication
+	publicKey key.Public    // peer public key used for DERP communication
 	addrs     []net.UDPAddr // ordered priority list provided by wgengine
 
 	mu       sync.Mutex   // guards roamAddr and curAddr