From 79d8288f0a259dc17ff40cd22bd122a74d7d8e14 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 9 Mar 2021 12:53:02 -0800 Subject: [PATCH] wgengine/magicsock, derp, derp/derphttp: respond to DERP server->client pings No server support yet, but we want Tailscale 1.6 clients to be able to respond to them when the server can do it. Updates #1310 Signed-off-by: Brad Fitzpatrick --- derp/derp.go | 6 +++- derp/derp_client.go | 31 +++++++++++++++-- derp/derp_test.go | 58 ++++++++++++++++++++++++++++++++ derp/derphttp/derphttp_client.go | 23 +++++++++++++ wgengine/magicsock/magicsock.go | 9 +++++ 5 files changed, 124 insertions(+), 3 deletions(-) diff --git a/derp/derp.go b/derp/derp.go index 7bebb0c25..2774e2b3d 100644 --- a/derp/derp.go +++ b/derp/derp.go @@ -59,7 +59,8 @@ Login: * server sends frameServerInfo Steady state: -* server occasionally sends frameKeepAlive +* server occasionally sends frameKeepAlive (or framePing) +* client responds to any framePing with a framePong * client sends frameSendPacket * server then sends frameRecvPacket to recipient */ @@ -97,6 +98,9 @@ const ( // connection. (To be used for cluster load balancing // purposes, when clients end up on a non-ideal node) frameClosePeer = frameType(0x11) // 32B pub key of peer to close. + + framePing = frameType(0x12) // 8 byte ping payload, to be echoed back in framePong + framePong = frameType(0x13) // 8 byte payload, the contents of the ping being replied to ) var bin = binary.BigEndian diff --git a/derp/derp_client.go b/derp/derp_client.go index a77c150ab..c152611a1 100644 --- a/derp/derp_client.go +++ b/derp/derp_client.go @@ -238,6 +238,18 @@ func (c *Client) ForwardPacket(srcKey, dstKey key.Public, pkt []byte) (err error func (c *Client) writeTimeoutFired() { c.nc.Close() } +func (c *Client) SendPong(data [8]byte) error { + c.wmu.Lock() + defer c.wmu.Unlock() + if err := writeFrameHeader(c.bw, framePong, 8); err != nil { + return err + } + if _, err := c.bw.Write(data[:]); err != nil { + return err + } + return c.bw.Flush() +} + // NotePreferred sends a packet that tells the server whether this // client is the user's preferred server. This is only used in the // server for stats. @@ -319,6 +331,12 @@ type ServerInfoMessage struct{} func (ServerInfoMessage) msg() {} +// PingMessage is a request from a client or server to reply to the +// other side with a PongMessage with the given payload. +type PingMessage [8]byte + +func (PingMessage) msg() {} + // Recv reads a message from the DERP server. // // The returned message may alias memory owned by the Client; it @@ -397,8 +415,8 @@ func (c *Client) recvTimeout(timeout time.Duration) (m ReceivedMessage, err erro // TODO: add the results of parseServerInfo to ServerInfoMessage if we ever need it. return ServerInfoMessage{}, nil case frameKeepAlive: - // TODO: eventually we'll have server->client pings that - // require ack pongs. + // A one-way keep-alive message that doesn't require an acknowledgement. + // This predated framePing/framePong. continue case framePeerGone: if n < keyLen { @@ -427,6 +445,15 @@ func (c *Client) recvTimeout(timeout time.Duration) (m ReceivedMessage, err erro copy(rp.Source[:], b[:keyLen]) rp.Data = b[keyLen:n] return rp, nil + + case framePing: + var pm PingMessage + if n < 8 { + c.logf("[unexpected] dropping short ping frame") + continue + } + copy(pm[:], b[:]) + return pm, nil } } } diff --git a/derp/derp_test.go b/derp/derp_test.go index 87d456a03..b3aea308b 100644 --- a/derp/derp_test.go +++ b/derp/derp_test.go @@ -6,6 +6,7 @@ package derp import ( "bufio" + "bytes" "context" crand "crypto/rand" "crypto/x509" @@ -791,6 +792,63 @@ func TestMetaCert(t *testing.T) { } } +type dummyNetConn struct { + net.Conn +} + +func (dummyNetConn) SetReadDeadline(time.Time) error { return nil } + +func TestClientRecv(t *testing.T) { + tests := []struct { + name string + input []byte + want interface{} + }{ + { + name: "ping", + input: []byte{ + byte(framePing), 0, 0, 0, 8, + 1, 2, 3, 4, 5, 6, 7, 8, + }, + want: PingMessage{1, 2, 3, 4, 5, 6, 7, 8}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &Client{ + nc: dummyNetConn{}, + br: bufio.NewReader(bytes.NewReader(tt.input)), + logf: t.Logf, + } + got, err := c.Recv() + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("got %#v; want %#v", got, tt.want) + } + }) + } +} + +func TestClientSendPong(t *testing.T) { + var buf bytes.Buffer + c := &Client{ + bw: bufio.NewWriter(&buf), + } + if err := c.SendPong([8]byte{1, 2, 3, 4, 5, 6, 7, 8}); err != nil { + t.Fatal(err) + } + want := []byte{ + byte(framePong), 0, 0, 0, 8, + 1, 2, 3, 4, 5, 6, 7, 8, + } + if !bytes.Equal(buf.Bytes(), want) { + t.Errorf("unexpected output\nwrote: % 02x\n want: % 02x", buf.Bytes(), want) + } + +} + 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 0c523f07e..28fdef059 100644 --- a/derp/derphttp/derphttp_client.go +++ b/derp/derphttp/derphttp_client.go @@ -642,6 +642,29 @@ func (c *Client) ForwardPacket(from, to key.Public, b []byte) error { return err } +// SendPong sends a reply to a ping, with the ping's provided +// challenge/identifier data. +// +// Unlike other send methods, SendPong makes no attempt to connect or +// reconnect to the peer. It's best effort. If there's a connection +// problem, the server will choose to hang up on us if we're not +// replying. +func (c *Client) SendPong(data [8]byte) error { + c.mu.Lock() + if c.closed { + c.mu.Unlock() + return ErrClientClosed + } + if c.client == nil { + c.mu.Unlock() + return errors.New("not connected") + } + dc := c.client + c.mu.Unlock() + + return dc.SendPong(data) +} + // NotePreferred notes whether this Client is the caller's preferred // (home) DERP node. It's only used for stats. func (c *Client) NotePreferred(v bool) { diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 17dd73618..81ad54e49 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -1465,6 +1465,15 @@ func (c *Conn) runDerpReader(ctx context.Context, derpFakeAddr netaddr.IPPort, d peerPresent[m.Source] = true c.addDerpPeerRoute(m.Source, regionID, dc) } + case derp.PingMessage: + // Best effort reply to the ping. + pingData := [8]byte(m) + go func() { + if err := dc.SendPong(pingData); err != nil { + c.logf("magicsock: derp-%d SendPong error: %v", regionID, err) + } + }() + continue default: // Ignore. continue