From 293431aaea55907357df7901158ff6688ef19db6 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Mon, 25 Oct 2021 16:41:30 -0700 Subject: [PATCH] control/noise: use key.Machine{Public,Private} as appropriate. Signed-off-by: David Anderson --- control/noise/conn.go | 4 +- control/noise/conn_test.go | 8 ++-- control/noise/handshake.go | 81 ++++++++++++++++----------------- control/noise/handshake_test.go | 24 +++++----- control/noise/interop_test.go | 30 ++++++------ control/noise/key.go | 26 ----------- control/noise/messages.go | 33 ++++++-------- types/key/machine.go | 39 ++++++++++++++++ 8 files changed, 126 insertions(+), 119 deletions(-) delete mode 100644 control/noise/key.go diff --git a/control/noise/conn.go b/control/noise/conn.go index 4dce79f80..6deda2ea4 100644 --- a/control/noise/conn.go +++ b/control/noise/conn.go @@ -42,7 +42,7 @@ type Conn struct { conn net.Conn version uint16 - peer key.Public + peer key.MachinePublic handshakeHash [blake2s.Size]byte rx rxState tx txState @@ -83,7 +83,7 @@ func (c *Conn) HandshakeHash() [blake2s.Size]byte { } // Peer returns the peer's long-term public key. -func (c *Conn) Peer() key.Public { +func (c *Conn) Peer() key.MachinePublic { return c.peer } diff --git a/control/noise/conn_test.go b/control/noise/conn_test.go index 170a44b34..e87e4e159 100644 --- a/control/noise/conn_test.go +++ b/control/noise/conn_test.go @@ -197,8 +197,8 @@ func TestConnStd(t *testing.T) { t.Skip("not all tests can pass on this Conn, see https://github.com/golang/go/issues/46977") nettest.TestConn(t, func() (c1 net.Conn, c2 net.Conn, stop func(), err error) { s1, s2 := tsnettest.NewConn("noise", 4096) - controlKey := key.NewPrivate() - machineKey := key.NewPrivate() + controlKey := key.NewMachine() + machineKey := key.NewMachine() serverErr := make(chan error, 1) go func() { var err error @@ -312,8 +312,8 @@ func (s *readSink) Total() int { func pairWithConns(t *testing.T, clientConn, serverConn net.Conn) (*Conn, *Conn) { var ( - controlKey = key.NewPrivate() - machineKey = key.NewPrivate() + controlKey = key.NewMachine() + machineKey = key.NewMachine() server *Conn serverErr = make(chan error, 1) ) diff --git a/control/noise/handshake.go b/control/noise/handshake.go index ea052a213..b8c9a5df6 100644 --- a/control/noise/handshake.go +++ b/control/noise/handshake.go @@ -14,6 +14,7 @@ "strconv" "time" + "go4.org/mem" "golang.org/x/crypto/blake2s" chp "golang.org/x/crypto/chacha20poly1305" "golang.org/x/crypto/curve25519" @@ -23,22 +24,21 @@ ) const ( - // protocolName is the name of the specific instantiation of the - // Noise protocol we're using. Each field is defined in the Noise - // spec, and shouldn't be changed unless we're switching to a - // different Noise protocol instance. + // protocolName is the name of the specific instantiation of Noise + // that the control protocol uses. This string's value is fixed by + // the Noise spec, and shouldn't be changed unless we're updating + // the control protocol to use a different Noise instance. protocolName = "Noise_IK_25519_ChaChaPoly_BLAKE2s" - // protocolVersion is the version of the Tailscale base - // protocol that Client will use when initiating a handshake. + // protocolVersion is the version of the control protocol that + // Client will use when initiating a handshake. protocolVersion uint16 = 1 // protocolVersionPrefix is the name portion of the protocol - // name+version string that gets mixed into the Noise handshake as - // a prologue. + // name+version string that gets mixed into the handshake as a + // prologue. // - // This mixing verifies that both clients agree that - // they're executing the Tailscale control protocol at a specific - // version that matches the advertised version in the cleartext - // packet header. + // This mixing verifies that both clients agree that they're + // executing the control protocol at a specific version that + // matches the advertised version in the cleartext packet header. protocolVersionPrefix = "Tailscale Control Protocol v" invalidNonce = ^uint64(0) ) @@ -49,12 +49,12 @@ func protocolVersionPrologue(version uint16) []byte { return strconv.AppendUint(ret, uint64(version), 10) } -// Client initiates a Noise client handshake, returning the resulting -// Noise connection. +// Client initiates a control client handshake, returning the resulting +// control connection. // // The context deadline, if any, covers the entire handshaking // process. Any preexisting Conn deadline is removed. -func Client(ctx context.Context, conn net.Conn, machineKey key.Private, controlKey key.Public) (*Conn, error) { +func Client(ctx context.Context, conn net.Conn, machineKey key.MachinePrivate, controlKey key.MachinePublic) (*Conn, error) { if deadline, ok := ctx.Deadline(); ok { if err := conn.SetDeadline(deadline); err != nil { return nil, fmt.Errorf("setting conn deadline: %w", err) @@ -72,20 +72,20 @@ func Client(ctx context.Context, conn net.Conn, machineKey key.Private, controlK // <- s // ... - s.MixHash(controlKey[:]) + s.MixHash(controlKey.UntypedBytes()) // -> e, es, s, ss init := mkInitiationMessage() - machineEphemeral := key.NewPrivate() + machineEphemeral := key.NewMachine() machineEphemeralPub := machineEphemeral.Public() - copy(init.EphemeralPub(), machineEphemeralPub[:]) - s.MixHash(machineEphemeralPub[:]) + copy(init.EphemeralPub(), machineEphemeralPub.UntypedBytes()) + s.MixHash(machineEphemeralPub.UntypedBytes()) cipher, err := s.MixDH(machineEphemeral, controlKey) if err != nil { return nil, fmt.Errorf("computing es: %w", err) } machineKeyPub := machineKey.Public() - s.EncryptAndHash(cipher, init.MachinePub(), machineKeyPub[:]) + s.EncryptAndHash(cipher, init.MachinePub(), machineKeyPub.UntypedBytes()) cipher, err = s.MixDH(machineKey, controlKey) if err != nil { return nil, fmt.Errorf("computing ss: %w", err) @@ -122,9 +122,8 @@ func Client(ctx context.Context, conn net.Conn, machineKey key.Private, controlK } // <- e, ee, se - var controlEphemeralPub key.Public - copy(controlEphemeralPub[:], resp.EphemeralPub()) - s.MixHash(controlEphemeralPub[:]) + controlEphemeralPub := key.MachinePublicFromRaw32(mem.B(resp.EphemeralPub())) + s.MixHash(controlEphemeralPub.UntypedBytes()) if _, err = s.MixDH(machineEphemeral, controlEphemeralPub); err != nil { return nil, fmt.Errorf("computing ee: %w", err) } @@ -156,12 +155,12 @@ func Client(ctx context.Context, conn net.Conn, machineKey key.Private, controlK return c, nil } -// Server initiates a Noise server handshake, returning the resulting -// Noise connection. +// Server initiates a control server handshake, returning the resulting +// control connection. // // The context deadline, if any, covers the entire handshaking // process. -func Server(ctx context.Context, conn net.Conn, controlKey key.Private) (*Conn, error) { +func Server(ctx context.Context, conn net.Conn, controlKey key.MachinePrivate) (*Conn, error) { if deadline, ok := ctx.Deadline(); ok { if err := conn.SetDeadline(deadline); err != nil { return nil, fmt.Errorf("setting conn deadline: %w", err) @@ -215,20 +214,20 @@ func Server(ctx context.Context, conn net.Conn, controlKey key.Private) (*Conn, // <- s // ... controlKeyPub := controlKey.Public() - s.MixHash(controlKeyPub[:]) + s.MixHash(controlKeyPub.UntypedBytes()) // -> e, es, s, ss - var machineEphemeralPub key.Public - copy(machineEphemeralPub[:], init.EphemeralPub()) - s.MixHash(machineEphemeralPub[:]) + machineEphemeralPub := key.MachinePublicFromRaw32(mem.B(init.EphemeralPub())) + s.MixHash(machineEphemeralPub.UntypedBytes()) cipher, err := s.MixDH(controlKey, machineEphemeralPub) if err != nil { return nil, fmt.Errorf("computing es: %w", err) } - var machineKey key.Public - if err := s.DecryptAndHash(cipher, machineKey[:], init.MachinePub()); err != nil { + var machineKeyBytes [32]byte + if err := s.DecryptAndHash(cipher, machineKeyBytes[:], init.MachinePub()); err != nil { return nil, fmt.Errorf("decrypting machine key: %w", err) } + machineKey := key.MachinePublicFromRaw32(mem.B(machineKeyBytes[:])) cipher, err = s.MixDH(controlKey, machineKey) if err != nil { return nil, fmt.Errorf("computing ss: %w", err) @@ -239,10 +238,10 @@ func Server(ctx context.Context, conn net.Conn, controlKey key.Private) (*Conn, // <- e, ee, se resp := mkResponseMessage() - controlEphemeral := key.NewPrivate() + controlEphemeral := key.NewMachine() controlEphemeralPub := controlEphemeral.Public() - copy(resp.EphemeralPub(), controlEphemeralPub[:]) - s.MixHash(controlEphemeralPub[:]) + copy(resp.EphemeralPub(), controlEphemeralPub.UntypedBytes()) + s.MixHash(controlEphemeralPub.UntypedBytes()) if _, err := s.MixDH(controlEphemeral, machineEphemeralPub); err != nil { return nil, fmt.Errorf("computing ee: %w", err) } @@ -276,14 +275,12 @@ func Server(ctx context.Context, conn net.Conn, controlKey key.Private) (*Conn, return c, nil } -// symmetricState is the SymmetricState object from the Noise protocol -// spec. It contains all the symmetric cipher state of an in-flight -// handshake. Field names match the variable names in the spec. +// symmetricState contains the state of an in-flight handshake. type symmetricState struct { finished bool - h [blake2s.Size]byte - ck [blake2s.Size]byte + h [blake2s.Size]byte // hash of currently-processed handshake state + ck [blake2s.Size]byte // chaining key used to construct session keys at the end of the handshake } func (s *symmetricState) checkFinished() { @@ -319,9 +316,9 @@ func (s *symmetricState) MixHash(data []byte) { // reduce the risk of error in the caller (e.g. invoking X25519 with // two private keys, or two public keys), and thus producing the wrong // calculation. -func (s *symmetricState) MixDH(priv key.Private, pub key.Public) (*singleUseCHP, error) { +func (s *symmetricState) MixDH(priv key.MachinePrivate, pub key.MachinePublic) (*singleUseCHP, error) { s.checkFinished() - keyData, err := curve25519.X25519(priv[:], pub[:]) + keyData, err := curve25519.X25519(priv.UntypedBytes(), pub.UntypedBytes()) if err != nil { return nil, fmt.Errorf("computing X25519: %w", err) } diff --git a/control/noise/handshake_test.go b/control/noise/handshake_test.go index 043dfd8aa..c60f48d89 100644 --- a/control/noise/handshake_test.go +++ b/control/noise/handshake_test.go @@ -19,8 +19,8 @@ func TestHandshake(t *testing.T) { var ( clientConn, serverConn = tsnettest.NewConn("noise", 128000) - serverKey = key.NewPrivate() - clientKey = key.NewPrivate() + serverKey = key.NewMachine() + clientKey = key.NewMachine() server *Conn serverErr = make(chan error, 1) ) @@ -71,8 +71,8 @@ func TestNoReuse(t *testing.T) { clientBuf, serverBuf bytes.Buffer clientConn = &readerConn{clientRaw, io.TeeReader(clientRaw, &clientBuf)} serverConn = &readerConn{serverRaw, io.TeeReader(serverRaw, &serverBuf)} - serverKey = key.NewPrivate() - clientKey = key.NewPrivate() + serverKey = key.NewMachine() + clientKey = key.NewMachine() server *Conn serverErr = make(chan error, 1) ) @@ -164,8 +164,8 @@ func TestTampering(t *testing.T) { var ( clientConn, serverRaw = tsnettest.NewConn("noise", 128000) serverConn = &readerConn{serverRaw, &tamperReader{serverRaw, i, 0}} - serverKey = key.NewPrivate() - clientKey = key.NewPrivate() + serverKey = key.NewMachine() + clientKey = key.NewMachine() serverErr = make(chan error, 1) ) go func() { @@ -192,8 +192,8 @@ func TestTampering(t *testing.T) { var ( clientRaw, serverConn = tsnettest.NewConn("noise", 128000) clientConn = &readerConn{clientRaw, &tamperReader{clientRaw, i, 0}} - serverKey = key.NewPrivate() - clientKey = key.NewPrivate() + serverKey = key.NewMachine() + clientKey = key.NewMachine() serverErr = make(chan error, 1) ) go func() { @@ -217,8 +217,8 @@ func TestTampering(t *testing.T) { var ( clientRaw, serverConn = tsnettest.NewConn("noise", 128000) clientConn = &readerConn{clientRaw, &tamperReader{clientRaw, 53 + i, 0}} - serverKey = key.NewPrivate() - clientKey = key.NewPrivate() + serverKey = key.NewMachine() + clientKey = key.NewMachine() serverErr = make(chan error, 1) ) go func() { @@ -258,8 +258,8 @@ func TestTampering(t *testing.T) { var ( clientConn, serverRaw = tsnettest.NewConn("noise", 128000) serverConn = &readerConn{serverRaw, &tamperReader{serverRaw, 101 + i, 0}} - serverKey = key.NewPrivate() - clientKey = key.NewPrivate() + serverKey = key.NewMachine() + clientKey = key.NewMachine() serverErr = make(chan error, 1) ) go func() { diff --git a/control/noise/interop_test.go b/control/noise/interop_test.go index d3ae83468..8c6f35342 100644 --- a/control/noise/interop_test.go +++ b/control/noise/interop_test.go @@ -20,8 +20,8 @@ func TestInteropClient(t *testing.T) { var ( s1, s2 = tsnettest.NewConn("noise", 128000) - controlKey = key.NewPrivate() - machineKey = key.NewPrivate() + controlKey = key.NewMachine() + machineKey = key.NewMachine() serverErr = make(chan error, 2) serverBytes = make(chan []byte, 1) c2s = "client>server" @@ -68,8 +68,8 @@ func TestInteropClient(t *testing.T) { func TestInteropServer(t *testing.T) { var ( s1, s2 = tsnettest.NewConn("noise", 128000) - controlKey = key.NewPrivate() - machineKey = key.NewPrivate() + controlKey = key.NewMachine() + machineKey = key.NewMachine() clientErr = make(chan error, 2) clientBytes = make(chan []byte, 1) c2s = "client>server" @@ -115,12 +115,13 @@ func TestInteropServer(t *testing.T) { // noiseExplorerClient uses the Noise Explorer implementation of Noise // IK to handshake as a Noise client on conn, transmit payload, and // read+return a payload from the peer. -func noiseExplorerClient(conn net.Conn, controlKey key.Public, machineKey key.Private, payload []byte) ([]byte, error) { - mk := keypair{ - private_key: machineKey, - public_key: machineKey.Public(), - } - session := InitSession(true, protocolVersionPrologue(protocolVersion), mk, controlKey) +func noiseExplorerClient(conn net.Conn, controlKey key.MachinePublic, machineKey key.MachinePrivate, payload []byte) ([]byte, error) { + var mk keypair + copy(mk.private_key[:], machineKey.UntypedBytes()) + copy(mk.public_key[:], machineKey.Public().UntypedBytes()) + var peerKey [32]byte + copy(peerKey[:], controlKey.UntypedBytes()) + session := InitSession(true, protocolVersionPrologue(protocolVersion), mk, peerKey) _, msg1 := SendMessage(&session, nil) var hdr [headerLen]byte @@ -185,11 +186,10 @@ func noiseExplorerClient(conn net.Conn, controlKey key.Public, machineKey key.Pr return p, nil } -func noiseExplorerServer(conn net.Conn, controlKey key.Private, wantMachineKey key.Public, payload []byte) ([]byte, error) { - mk := keypair{ - private_key: controlKey, - public_key: controlKey.Public(), - } +func noiseExplorerServer(conn net.Conn, controlKey key.MachinePrivate, wantMachineKey key.MachinePublic, payload []byte) ([]byte, error) { + var mk keypair + copy(mk.private_key[:], controlKey.UntypedBytes()) + copy(mk.public_key[:], controlKey.Public().UntypedBytes()) session := InitSession(false, protocolVersionPrologue(protocolVersion), mk, [32]byte{}) var buf [1024]byte diff --git a/control/noise/key.go b/control/noise/key.go deleted file mode 100644 index 6fe480c01..000000000 --- a/control/noise/key.go +++ /dev/null @@ -1,26 +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 noise - -// Note that these types are deliberately separate from the types/key -// package. That package defines generic curve25519 keys, without -// consideration for how those keys are used. We don't want to -// encourage mixing machine keys, node keys, and whatever else we -// might use curve25519 for. -// -// Furthermore, the implementation in types/key does some work that is -// unnecessary for machine keys, and results in a harder to follow -// implementation. In particular, machine keys do not need to be -// clamped per the curve25519 spec because they're only used with the -// X25519 operation, and the X25519 operation defines its own clamping -// and sanity checking logic. Thus, these keys must be used only with -// this Noise protocol implementation, and the easiest way to ensure -// that is a different type. - -// PrivateKey is a Tailscale machine private key. -type PrivateKey [32]byte - -// PublicKey is a Tailscale machine public key. -type PublicKey [32]byte diff --git a/control/noise/messages.go b/control/noise/messages.go index 381038915..1d9ddb37d 100644 --- a/control/noise/messages.go +++ b/control/noise/messages.go @@ -6,12 +6,12 @@ import "encoding/binary" -// The transport protocol is mostly Noise messages encapsulated in a -// small header describing the payload's type and length. The one -// place we deviate from pure Noise+header is that we also support -// sending an unauthenticated plaintext error as payload, to provide -// an explanation for a connection error that happens before the -// handshake completes. +// The control protocol wire format is mostly Noise messages +// encapsulated in a small header describing the payload's type and +// length. The one place we deviate from pure Noise+header is that we +// also support sending an unauthenticated plaintext error as payload, +// to provide an explanation for a connection error that happens +// before the handshake completes. // // All frames in our protocol have a 5-byte header: // @@ -31,9 +31,10 @@ // version numbers. At minimum, the version number must change // whenever any particulars of the Noise handshake change // (e.g. switching from Noise IK to Noise IKpsk1 or Noise XX), and -// when security-critical aspects of the "uppper" protocol within the -// Noise frames change (e.g. how further authentication data is bound -// to the underlying Noise session). +// when security-critical aspects of the "uppper" protocol (the one +// running inside the established base protocol session) change +// (e.g. how further authentication data is bound to the underlying +// session). // headerLen is the size of the header that gets prepended to Noise // messages. @@ -51,7 +52,7 @@ // hints only. They are not encrypted or authenticated, and so can // be seen and tampered with on the wire. msgTypeError = 3 - // msgTypeRecord frames carry a Noise transport message (i.e. "user data"). + // msgTypeRecord frames carry session data bytes. msgTypeRecord = 4 ) @@ -64,10 +65,8 @@ func hdrVersion(bs []byte) uint16 { return binary.LittleEndian.Uint16(bs[:2]) } func hdrType(bs []byte) byte { return bs[2] } func hdrLen(bs []byte) int { return int(binary.LittleEndian.Uint16(bs[3:5])) } -// initiationMessage is the Noise protocol message sent from a client -// machine to a control server. Aside from the message header, the -// values are as specified in the Noise specification for the IK -// handshake pattern. +// initiationMessage is the protocol message sent from a client +// machine to a control server. // // 5b: header (see headerLen for fields) // 32b: client ephemeral public key (cleartext) @@ -92,10 +91,8 @@ func (m *initiationMessage) EphemeralPub() []byte { return m[headerLen : headerL func (m *initiationMessage) MachinePub() []byte { return m[headerLen+32 : headerLen+32+48] } func (m *initiationMessage) Tag() []byte { return m[headerLen+32+48:] } -// responseMessage is the Noise protocol message sent from a control -// server to a client machine. Aside from the message header, the -// values are as specified in the Noise specification for the IK -// handshake pattern. +// responseMessage is the protocol message sent from a control server +// to a client machine. // // 5b: header (see headerLen for fields) // 32b: control ephemeral public key (cleartext) diff --git a/types/key/machine.go b/types/key/machine.go index ca294cde3..9687300f5 100644 --- a/types/key/machine.go +++ b/types/key/machine.go @@ -77,6 +77,19 @@ func (k *MachinePrivate) UnmarshalText(b []byte) error { return parseHex(k.k[:], mem.B(b), mem.S(machinePrivateHexPrefix)) } +// UntypedBytes returns k, encoded as an untyped 64-character hex +// string. +// +// Deprecated: this function is risky to use, because it produces +// serialized values that do not identify themselves as a +// MachinePrivate, allowing other code to potentially parse it back in +// as the wrong key type. For new uses that don't require this +// specific raw byte serialization, please use +// MarshalText/UnmarshalText. +func (k MachinePrivate) UntypedBytes() []byte { + return append([]byte(nil), k.k[:]...) +} + // SealTo wraps cleartext into a NaCl box (see // golang.org/x/crypto/nacl) to p, authenticated from k, using a // random nonce. @@ -112,6 +125,19 @@ type MachinePublic struct { k [32]byte } +// MachinePublicFromRaw32 parses a 32-byte raw value as a MachinePublic. +// +// This should be used only when deserializing a MachinePublic from a +// binary protocol. +func MachinePublicFromRaw32(raw mem.RO) MachinePublic { + if raw.Len() != 32 { + panic("input has wrong size") + } + var ret MachinePublic + raw.Copy(ret.k[:]) + return ret +} + // ParseMachinePublicUntyped parses an untyped 64-character hex value // as a MachinePublic. // @@ -153,6 +179,19 @@ func (k MachinePublic) UntypedHexString() string { return hex.EncodeToString(k.k[:]) } +// UntypedBytes returns k, encoded as an untyped 64-character hex +// string. +// +// Deprecated: this function is risky to use, because it produces +// serialized values that do not identify themselves as a +// MachinePublic, allowing other code to potentially parse it back in +// as the wrong key type. For new uses that don't require this +// specific raw byte serialization, please use +// MarshalText/UnmarshalText. +func (k MachinePublic) UntypedBytes() []byte { + return append([]byte(nil), k.k[:]...) +} + // String returns the output of MarshalText as a string. func (k MachinePublic) String() string { bs, err := k.MarshalText()