From e4159912560d611ee23ba187ceb14c0de1ff3d82 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 18 Aug 2020 15:32:32 -0700 Subject: [PATCH] derp, derp/derphttp: remove one RTT from DERP setup * advertise server's DERP public key following its ServerHello * have client look for that DEPR public key in the response PeerCertificates * let client advertise it's going into a "fast start" mode if it finds it * modify server to support that fast start mode, just not sending the HTTP response header Cuts down another round trip, bringing the latency of being able to write our first DERP frame from SF to Bangalore from ~725ms (3 RTT) to ~481ms (2 RTT: TCP and TLS). Fixes #693 Signed-off-by: Brad Fitzpatrick --- cmd/derper/derper.go | 10 ++++ derp/derp.go | 4 +- derp/derp_client.go | 19 +++++-- derp/derp_server.go | 50 ++++++++++++++++- derp/derp_test.go | 20 +++++++ derp/derphttp/derphttp_client.go | 94 ++++++++++++++++++++++++++------ derp/derphttp/derphttp_server.go | 24 +++++++- 7 files changed, 193 insertions(+), 28 deletions(-) diff --git a/cmd/derper/derper.go b/cmd/derper/derper.go index 66a00d759..838b27028 100644 --- a/cmd/derper/derper.go +++ b/cmd/derper/derper.go @@ -7,6 +7,7 @@ package main // import "tailscale.com/cmd/derper" import ( "context" + "crypto/tls" "encoding/json" "errors" "expvar" @@ -185,6 +186,15 @@ func main() { certManager.Email = "security@tailscale.com" } httpsrv.TLSConfig = certManager.TLSConfig() + letsEncryptGetCert := httpsrv.TLSConfig.GetCertificate + httpsrv.TLSConfig.GetCertificate = func(hi *tls.ClientHelloInfo) (*tls.Certificate, error) { + cert, err := letsEncryptGetCert(hi) + if err != nil { + return nil, err + } + cert.Certificate = append(cert.Certificate, s.MetaCert()) + return cert, nil + } go func() { err := http.ListenAndServe(":80", certManager.HTTPHandler(tsweb.Port80Handler{Main: mux})) if err != nil { diff --git a/derp/derp.go b/derp/derp.go index b5fadede1..7bebb0c25 100644 --- a/derp/derp.go +++ b/derp/derp.go @@ -39,10 +39,10 @@ const ( keepAlive = 60 * time.Second ) -// protocolVersion is bumped whenever there's a wire-incompatible change. +// ProtocolVersion is bumped whenever there's a wire-incompatible change. // * version 1 (zero on wire): consistent box headers, in use by employee dev nodes a bit // * version 2: received packets have src addrs in frameRecvPacket at beginning -const protocolVersion = 2 +const ProtocolVersion = 2 // frameType is the one byte frame type at the beginning of the frame // header. The second field is a big-endian uint32 describing the diff --git a/derp/derp_client.go b/derp/derp_client.go index aca67fef6..a77c150ab 100644 --- a/derp/derp_client.go +++ b/derp/derp_client.go @@ -48,7 +48,8 @@ func (f clientOptFunc) update(o *clientOpt) { f(o) } // clientOpt are the options passed to newClient. type clientOpt struct { - MeshKey string + MeshKey string + ServerPub key.Public } // MeshKey returns a ClientOpt to pass to the DERP server during connect to get @@ -57,6 +58,12 @@ type clientOpt struct { // An empty key means to not use a mesh key. func MeshKey(key string) ClientOpt { return clientOptFunc(func(o *clientOpt) { o.MeshKey = key }) } +// ServerPublicKey returns a ClientOpt to declare that the server's DERP public key is known. +// If key is the zero value, the returned ClientOpt is a no-op. +func ServerPublicKey(key key.Public) ClientOpt { + return clientOptFunc(func(o *clientOpt) { o.ServerPub = key }) +} + func NewClient(privateKey key.Private, nc Conn, brw *bufio.ReadWriter, logf logger.Logf, opts ...ClientOpt) (*Client, error) { var opt clientOpt for _, o := range opts { @@ -78,8 +85,12 @@ func newClient(privateKey key.Private, nc Conn, brw *bufio.ReadWriter, logf logg bw: brw.Writer, meshKey: opt.MeshKey, } - if err := c.recvServerKey(); err != nil { - return nil, fmt.Errorf("derp.Client: failed to receive server key: %v", err) + if opt.ServerPub.IsZero() { + if err := c.recvServerKey(); err != nil { + return nil, fmt.Errorf("derp.Client: failed to receive server key: %v", err) + } + } else { + c.serverKey = opt.ServerPub } if err := c.sendClientKey(); err != nil { return nil, fmt.Errorf("derp.Client: failed to send client key: %v", err) @@ -144,7 +155,7 @@ func (c *Client) sendClientKey() error { return err } msg, err := json.Marshal(clientInfo{ - Version: protocolVersion, + Version: ProtocolVersion, MeshKey: c.meshKey, }) if err != nil { diff --git a/derp/derp_server.go b/derp/derp_server.go index 7c6394e50..cdbe08e3c 100644 --- a/derp/derp_server.go +++ b/derp/derp_server.go @@ -9,7 +9,10 @@ package derp import ( "bufio" "context" + "crypto/ed25519" crand "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" "encoding/json" "errors" "expvar" @@ -17,6 +20,7 @@ import ( "io" "io/ioutil" "log" + "math/big" "math/rand" "os" "runtime" @@ -84,8 +88,10 @@ type Server struct { memSys0 uint64 // runtime.MemStats.Sys at start (or early-ish) meshKey string limitedLogf logger.Logf + metaCert []byte // the encoded x509 cert to send after LetsEncrypt cert+intermediate // Counters: + _ [pad32bit]byte packetsSent, bytesSent expvar.Int packetsRecv, bytesRecv expvar.Int packetsRecvByKind metrics.LabelMap @@ -177,6 +183,7 @@ func NewServer(privateKey key.Private, logf logger.Logf) *Server { watchers: map[*sclient]bool{}, sentTo: map[key.Public]map[key.Public]int64{}, } + s.initMetacert() s.packetsRecvDisco = s.packetsRecvByKind.Get("disco") s.packetsRecvOther = s.packetsRecvByKind.Get("other") s.packetsDroppedUnknown = s.packetsDroppedReason.Get("unknown_dest") @@ -270,6 +277,47 @@ func (s *Server) Accept(nc Conn, brw *bufio.ReadWriter, remoteAddr string) { } } +// initMetacert initialized s.metaCert with a self-signed x509 cert +// encoding this server's public key and protocol version. cmd/derper +// then sends this after the Let's Encrypt leaf + intermediate certs +// after the ServerHello (encrypted in TLS 1.3, not that it matters +// much). +// +// Then the client can save a round trip getting that and can start +// speaking DERP right away. (We don't use ALPN because that's sent in +// the clear and we're being paranoid to not look too weird to any +// middleboxes, given that DERP is an ultimate fallback path). But +// since the post-ServerHello certs are encrypted we can have the +// client also use them as a signal to be able to start speaking DERP +// right away, starting with its identity proof, encrypted to the +// server's public key. +// +// This RTT optimization fails where there's a corp-mandated +// TLS proxy with corp-mandated root certs on employee machines and +// and TLS proxy cleans up unnecessary certs. In that case we just fall +// back to the extra RTT. +func (s *Server) initMetacert() { + pub, priv, err := ed25519.GenerateKey(crand.Reader) + if err != nil { + log.Fatal(err) + } + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(ProtocolVersion), + Subject: pkix.Name{ + CommonName: fmt.Sprintf("derpkey%x", s.publicKey[:]), + }, + } + cert, err := x509.CreateCertificate(crand.Reader, tmpl, tmpl, pub, priv) + if err != nil { + log.Fatalf("CreateCertificate: %v", err) + } + s.metaCert = cert +} + +// MetaCert returns the server metadata cert that can be sent by the +// TLS server to let the client skip a round trip during start-up. +func (s *Server) MetaCert() []byte { return s.metaCert } + // registerClient notes that client c is now authenticated and ready for packets. // If c's public key was already connected with a different connection, the prior one is closed. func (s *Server) registerClient(c *sclient) { @@ -720,7 +768,7 @@ func (s *Server) sendServerInfo(bw *bufio.Writer, clientKey key.Public) error { if _, err := crand.Read(nonce[:]); err != nil { return err } - msg, err := json.Marshal(serverInfo{Version: protocolVersion}) + msg, err := json.Marshal(serverInfo{Version: ProtocolVersion}) if err != nil { return err } diff --git a/derp/derp_test.go b/derp/derp_test.go index d9e6580ff..87d456a03 100644 --- a/derp/derp_test.go +++ b/derp/derp_test.go @@ -8,12 +8,14 @@ import ( "bufio" "context" crand "crypto/rand" + "crypto/x509" "encoding/json" "errors" "expvar" "fmt" "io" "io/ioutil" + "log" "net" "reflect" "sync" @@ -771,6 +773,24 @@ func TestForwarderRegistration(t *testing.T) { }) } +func TestMetaCert(t *testing.T) { + priv := newPrivateKey(t) + pub := priv.Public() + s := NewServer(priv, t.Logf) + + certBytes := s.MetaCert() + cert, err := x509.ParseCertificate(certBytes) + if err != nil { + log.Fatal(err) + } + if fmt.Sprint(cert.SerialNumber) != fmt.Sprint(ProtocolVersion) { + t.Errorf("serial = %v; want %v", cert.SerialNumber, ProtocolVersion) + } + if g, w := cert.Subject.CommonName, fmt.Sprintf("derpkey%x", pub[:]); g != w { + t.Errorf("CommonName = %q; want %q", g, w) + } +} + func BenchmarkSendRecv(b *testing.B) { for _, size := range []int{10, 100, 1000, 10000} { b.Run(fmt.Sprintf("msgsize=%d", size), func(b *testing.B) { benchmarkSendRecvSize(b, size) }) diff --git a/derp/derphttp/derphttp_client.go b/derp/derphttp/derphttp_client.go index f9c52f1ce..d902a4074 100644 --- a/derp/derphttp/derphttp_client.go +++ b/derp/derphttp/derphttp_client.go @@ -14,6 +14,7 @@ import ( "bufio" "context" "crypto/tls" + "crypto/x509" "errors" "fmt" "io" @@ -23,9 +24,11 @@ import ( "net/http" "net/url" "os" + "strings" "sync" "time" + "go4.org/mem" "inet.af/netaddr" "tailscale.com/derp" "tailscale.com/net/dnscache" @@ -255,14 +258,41 @@ func (c *Client) connect(ctx context.Context, caller string) (client *derp.Clien } }() - var httpConn net.Conn // a TCP conn or a TLS conn; what we speak HTTP to + var httpConn net.Conn // a TCP conn or a TLS conn; what we speak HTTP to + var serverPub key.Public // or zero if unknown (if not using TLS or TLS middlebox eats it) + var serverProtoVersion int if c.useHTTPS() { - httpConn = c.tlsClient(tcpConn, node) + tlsConn := c.tlsClient(tcpConn, node) + httpConn = tlsConn + + // Force a handshake now (instead of waiting for it to + // be done implicitly on read/write) so we can check + // the ConnectionState. + if err := tlsConn.Handshake(); err != nil { + return nil, 0, err + } + + // We expect to be using TLS 1.3 to our own servers, and only + // starting at TLS 1.3 are the server's returned certificates + // encrypted, so only look for and use our "meta cert" if we're + // using TLS 1.3. If we're not using TLS 1.3, it might be a user + // running cmd/derper themselves with a different configuration, + // in which case we can avoid this fast-start optimization. + // (If a corporate proxy is MITM'ing TLS 1.3 connections with + // corp-mandated TLS root certs than all bets are off anyway.) + // Note that we're not specifically concerned about TLS downgrade + // attacks. TLS handles that fine: + // https://blog.gypsyengineer.com/en/security/how-does-tls-1-3-protect-against-downgrade-attacks.html + connState := tlsConn.ConnectionState() + if connState.Version >= tls.VersionTLS13 { + serverPub, serverProtoVersion = parseMetaCert(connState.PeerCertificates) + } } else { httpConn = tcpConn } brw := bufio.NewReadWriter(bufio.NewReader(httpConn), bufio.NewWriter(httpConn)) + var derpClient *derp.Client req, err := http.NewRequest("GET", c.urlString(node), nil) if err != nil { @@ -271,24 +301,39 @@ func (c *Client) connect(ctx context.Context, caller string) (client *derp.Clien req.Header.Set("Upgrade", "DERP") req.Header.Set("Connection", "Upgrade") - if err := req.Write(brw); err != nil { - return nil, 0, err - } - if err := brw.Flush(); err != nil { - return nil, 0, err - } + if !serverPub.IsZero() && serverProtoVersion != 0 { + // parseMetaCert found the server's public key (no TLS + // middlebox was in the way), so skip the HTTP upgrade + // exchange. See https://github.com/tailscale/tailscale/issues/693 + // for an overview. We still send the HTTP request + // just to get routed into the server's HTTP Handler so it + // can Hijack the request, but we signal with a special header + // that we don't want to deal with its HTTP response. + req.Header.Set(fastStartHeader, "1") // suppresses the server's HTTP response + if err := req.Write(brw); err != nil { + return nil, 0, err + } + // No need to flush the HTTP request. the derp.Client's initial + // client auth frame will flush it. + } else { + if err := req.Write(brw); err != nil { + return nil, 0, err + } + if err := brw.Flush(); err != nil { + return nil, 0, err + } - resp, err := http.ReadResponse(brw.Reader, req) - if err != nil { - return nil, 0, err - } - if resp.StatusCode != http.StatusSwitchingProtocols { - b, _ := ioutil.ReadAll(resp.Body) - resp.Body.Close() - return nil, 0, fmt.Errorf("GET failed: %v: %s", err, b) + resp, err := http.ReadResponse(brw.Reader, req) + if err != nil { + return nil, 0, err + } + if resp.StatusCode != http.StatusSwitchingProtocols { + b, _ := ioutil.ReadAll(resp.Body) + resp.Body.Close() + return nil, 0, fmt.Errorf("GET failed: %v: %s", err, b) + } } - - derpClient, err := derp.NewClient(c.privateKey, httpConn, brw, c.logf, derp.MeshKey(c.MeshKey)) + derpClient, err = derp.NewClient(c.privateKey, httpConn, brw, c.logf, derp.MeshKey(c.MeshKey), derp.ServerPublicKey(serverPub)) if err != nil { return nil, 0, err } @@ -701,3 +746,16 @@ func (c *Client) closeForReconnect(brokenClient *derp.Client) { } var ErrClientClosed = errors.New("derphttp.Client closed") + +func parseMetaCert(certs []*x509.Certificate) (serverPub key.Public, serverProtoVersion int) { + for _, cert := range certs { + if cn := cert.Subject.CommonName; strings.HasPrefix(cn, "derpkey") { + var err error + serverPub, err = key.NewPublicFromHexMem(mem.S(strings.TrimPrefix(cn, "derpkey"))) + if err == nil && cert.SerialNumber.BitLen() <= 8 { // supports up to version 255 + return serverPub, int(cert.SerialNumber.Int64()) + } + } + } + return key.Public{}, 0 +} diff --git a/derp/derphttp/derphttp_server.go b/derp/derphttp/derphttp_server.go index a2260010e..6a6452b32 100644 --- a/derp/derphttp/derphttp_server.go +++ b/derp/derphttp/derphttp_server.go @@ -5,33 +5,51 @@ package derphttp import ( + "fmt" "log" "net/http" "tailscale.com/derp" ) +// fastStartHeader is the header (with value "1") that signals to the HTTP +// server that the DERP HTTP client does not want the HTTP 101 response +// headers and it will begin writing & reading the DERP protocol immediately +// following its HTTP request. +const fastStartHeader = "Derp-Fast-Start" + func Handler(s *derp.Server) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if p := r.Header.Get("Upgrade"); p != "WebSocket" && p != "DERP" { http.Error(w, "DERP requires connection upgrade", http.StatusUpgradeRequired) return } - w.Header().Set("Upgrade", "DERP") - w.Header().Set("Connection", "Upgrade") - w.WriteHeader(http.StatusSwitchingProtocols) + fastStart := r.Header.Get(fastStartHeader) == "1" h, ok := w.(http.Hijacker) if !ok { http.Error(w, "HTTP does not support general TCP support", 500) return } + netConn, conn, err := h.Hijack() if err != nil { log.Printf("Hijack failed: %v", err) http.Error(w, "HTTP does not support general TCP support", 500) return } + + if !fastStart { + pubKey := s.PublicKey() + fmt.Fprintf(conn, "HTTP/1.1 101 Switching Protocols\r\n"+ + "Upgrade: DERP\r\n"+ + "Connection: Upgrade\r\n"+ + "Derp-Version: %v\r\n"+ + "Derp-Public-Key: %x\r\n\r\n", + derp.ProtocolVersion, + pubKey[:]) + } + s.Accept(netConn, conn, netConn.RemoteAddr().String()) }) }