From e057384438d8755fb9d3d05710f60f54b95a4d34 Mon Sep 17 00:00:00 2001 From: lukechampine Date: Tue, 18 Feb 2025 13:12:54 -0500 Subject: [PATCH 1/2] v2: Improve signature and nonce handling --- mux.go | 8 ++++---- spec_v2.md | 19 ++++++++++--------- v2/handshake.go | 30 +++++++++++++++++++++++------- v2/mux.go | 12 ++++++------ v2/mux_test.go | 4 ++-- 5 files changed, 45 insertions(+), 28 deletions(-) diff --git a/mux.go b/mux.go index cdef530..1d66496 100644 --- a/mux.go +++ b/mux.go @@ -51,7 +51,7 @@ func (m *Mux) DialStream() *Stream { func Dial(conn net.Conn, theirKey ed25519.PublicKey) (*Mux, error) { // exchange versions var theirVersion [1]byte - if _, err := conn.Write([]byte{2}); err != nil { + if _, err := conn.Write([]byte{3}); err != nil { return nil, fmt.Errorf("could not write our version: %w", err) } else if _, err := io.ReadFull(conn, theirVersion[:]); err != nil { return nil, fmt.Errorf("could not read peer version: %w", err) @@ -62,7 +62,7 @@ func Dial(conn net.Conn, theirKey ed25519.PublicKey) (*Mux, error) { m, err := muxv1.Dial(conn, theirKey) return &Mux{m1: m}, err } - m, err := muxv2.Dial(conn, theirKey) + m, err := muxv2.Dial(conn, theirKey, theirVersion[0]) return &Mux{m2: m}, err } @@ -72,7 +72,7 @@ func Accept(conn net.Conn, ourKey ed25519.PrivateKey) (*Mux, error) { var theirVersion [1]byte if _, err := io.ReadFull(conn, theirVersion[:]); err != nil { return nil, fmt.Errorf("could not read peer version: %w", err) - } else if _, err := conn.Write([]byte{2}); err != nil { + } else if _, err := conn.Write([]byte{3}); err != nil { return nil, fmt.Errorf("could not write our version: %w", err) } else if theirVersion[0] == 0 { return nil, errors.New("peer sent invalid version") @@ -81,7 +81,7 @@ func Accept(conn net.Conn, ourKey ed25519.PrivateKey) (*Mux, error) { m, err := muxv1.Accept(conn, ourKey) return &Mux{m1: m}, err } - m, err := muxv2.Accept(conn, ourKey) + m, err := muxv2.Accept(conn, ourKey, theirVersion[0]) return &Mux{m2: m}, err } diff --git a/spec_v2.md b/spec_v2.md index c04ed08..e0431a7 100644 --- a/spec_v2.md +++ b/spec_v2.md @@ -33,18 +33,18 @@ The *dialing peer* generates an X25519 keypair and sends: | Length | Type | Description | |--------|--------|---------------| -| 0 | uint8 | Version | +| 1 | uint8 | Version | | 32 | []byte | X25519 pubkey | -The current version is 2. +The current version is 3. -The *accepting* peer derives the shared X25519 secret, hashes it with BLAKE2b -for use as a ChaCha20-Poly1305 key, hashes that key *again* to derive the -initial nonce value, and responds with: +The *accepting* peer generates an X25519 keypair, derives the shared X25519 +secret, hashes it with BLAKE2b for use as a ChaCha20-Poly1305 key, hashes that +key *again* to derive the initial nonce value, and responds with: | Length | Type | Description | |--------|--------|--------------------| -| 0 | uint8 | Version | +| 1 | uint8 | Version | | 32 | []byte | X25519 pubkey | | 64 | []byte | Ed25519 signature | | 24 | | Encrypted settings | @@ -60,7 +60,7 @@ The settings are: | 4 | uint32 | Max timeout | 120000-7200000 | Settings are encrypted in the same manner as [Packets](#packets): a ciphertext -(8 bytes in this case) followed by a 16-byte tag. +(8 bytes in this case) followed by a 16-byte authentication tag. Peers agree upon settings by choosing the minimum of the two packet sizes and the maximum of the two timeouts. The timeout is an integer number of @@ -116,8 +116,9 @@ must not be split across packet boundaries. (In other words, the maximum size of a frame's payload is `n - (4 + 2 + 2)`.) A separate nonce is tracked for both the dialing and accepting peer, incremented -after each use. The initial value for both nonces is the first 12 bytes of -BLAKE2b(BLAKE2b(shared secret)). To increment a nonce, interpret its leading 8 +after each use. The initial value for the nonces is the first 12 bytes of +BLAKE2b(BLAKE2b(shared secret)), but the most significant bit of the accepting +peer's nonce is flipped. To increment a nonce, interpret its least-significant 8 bytes as a 64-bit unsigned integer. ### Covert Frames diff --git a/v2/handshake.go b/v2/handshake.go index e552165..0179e7a 100644 --- a/v2/handshake.go +++ b/v2/handshake.go @@ -128,7 +128,7 @@ func mergeSettings(ours, theirs connSettings) (connSettings, error) { return merged, nil } -func initiateHandshake(conn net.Conn, theirKey ed25519.PublicKey, ourSettings connSettings) (*seqCipher, connSettings, error) { +func initiateHandshake(conn net.Conn, theirKey ed25519.PublicKey, theirVersion uint8, ourSettings connSettings) (*seqCipher, connSettings, error) { xsk, xpk := generateX25519KeyPair() // write pubkey @@ -142,12 +142,16 @@ func initiateHandshake(conn net.Conn, theirKey ed25519.PublicKey, ourSettings co return nil, connSettings{}, fmt.Errorf("could not read handshake response: %w", err) } - // verify signature + // verify signature and derive shared cipher var rxpk [32]byte copy(rxpk[:], buf[:32]) + msg := append(xpk[:], rxpk[:]...) + if theirVersion == 2 { + sigHash := blake2b.Sum256(msg) + msg = sigHash[:] + } sig := buf[32:][:64] - sigHash := blake2b.Sum256(append(xpk[:], rxpk[:]...)) - if !ed25519.Verify(theirKey, sigHash[:], sig) { + if !ed25519.Verify(theirKey, msg, sig) { return nil, connSettings{}, errors.New("invalid signature") } @@ -156,6 +160,10 @@ func initiateHandshake(conn net.Conn, theirKey ed25519.PublicKey, ourSettings co if err != nil { return nil, connSettings{}, fmt.Errorf("failed to derive shared cipher: %w", err) } + if theirVersion > 2 { + // flip the most significant bit of their nonce + cipher.theirNonce[len(cipher.theirNonce)-1] ^= 0x80 + } // decrypt settings var mergedSettings connSettings @@ -175,7 +183,7 @@ func initiateHandshake(conn net.Conn, theirKey ed25519.PublicKey, ourSettings co return cipher, mergedSettings, nil } -func acceptHandshake(conn net.Conn, ourKey ed25519.PrivateKey, ourSettings connSettings) (*seqCipher, connSettings, error) { +func acceptHandshake(conn net.Conn, ourKey ed25519.PrivateKey, theirVersion uint8, ourSettings connSettings) (*seqCipher, connSettings, error) { xsk, xpk := generateX25519KeyPair() // read pubkey @@ -191,10 +199,18 @@ func acceptHandshake(conn net.Conn, ourKey ed25519.PrivateKey, ourSettings connS if err != nil { return nil, connSettings{}, fmt.Errorf("failed to derive shared cipher: %w", err) } + if theirVersion > 2 { + // flip the most significant bit of our nonce + cipher.ourNonce[len(cipher.ourNonce)-1] ^= 0x80 + } // write pubkey, signature, and settings - sigHash := blake2b.Sum256(append(rxpk[:], xpk[:]...)) - sig := ed25519.Sign(ourKey, sigHash[:]) + msg := append(rxpk[:], xpk[:]...) + if theirVersion == 2 { + sigHash := blake2b.Sum256(msg) + msg = sigHash[:] + } + sig := ed25519.Sign(ourKey, msg) copy(buf, xpk[:]) copy(buf[32:], sig) encodeConnSettings(buf[32+64:], ourSettings) diff --git a/v2/mux.go b/v2/mux.go index 542a4ef..66dbe40 100644 --- a/v2/mux.go +++ b/v2/mux.go @@ -383,8 +383,8 @@ func newMux(conn net.Conn, cipher *seqCipher, settings connSettings) *Mux { } // Dial initiates a mux protocol handshake on the provided conn. -func Dial(conn net.Conn, theirKey ed25519.PublicKey) (*Mux, error) { - cipher, settings, err := initiateHandshake(conn, theirKey, defaultConnSettings) +func Dial(conn net.Conn, theirKey ed25519.PublicKey, theirVersion uint8) (*Mux, error) { + cipher, settings, err := initiateHandshake(conn, theirKey, theirVersion, defaultConnSettings) if err != nil { return nil, fmt.Errorf("handshake failed: %w", err) } @@ -392,8 +392,8 @@ func Dial(conn net.Conn, theirKey ed25519.PublicKey) (*Mux, error) { } // Accept reciprocates a mux protocol handshake on the provided conn. -func Accept(conn net.Conn, ourKey ed25519.PrivateKey) (*Mux, error) { - cipher, settings, err := acceptHandshake(conn, ourKey, defaultConnSettings) +func Accept(conn net.Conn, ourKey ed25519.PrivateKey, theirVersion uint8) (*Mux, error) { + cipher, settings, err := acceptHandshake(conn, ourKey, theirVersion, defaultConnSettings) if err != nil { return nil, fmt.Errorf("handshake failed: %w", err) } @@ -408,12 +408,12 @@ var anonPubkey = anonPrivkey.Public().(ed25519.PublicKey) // DialAnonymous initiates a mux protocol handshake to a party without a // pre-established identity. The counterparty must reciprocate the handshake with // AcceptAnonymous. -func DialAnonymous(conn net.Conn) (*Mux, error) { return Dial(conn, anonPubkey) } +func DialAnonymous(conn net.Conn) (*Mux, error) { return Dial(conn, anonPubkey, 3) } // AcceptAnonymous reciprocates a mux protocol handshake without a // pre-established identity. The counterparty must initiate the handshake with // DialAnonymous. -func AcceptAnonymous(conn net.Conn) (*Mux, error) { return Accept(conn, anonPrivkey) } +func AcceptAnonymous(conn net.Conn) (*Mux, error) { return Accept(conn, anonPrivkey, 3) } // A Stream is a duplex connection multiplexed over a net.Conn. It implements // the net.Conn interface. diff --git a/v2/mux_test.go b/v2/mux_test.go index 03dc110..3d3a560 100644 --- a/v2/mux_test.go +++ b/v2/mux_test.go @@ -87,7 +87,7 @@ func TestMux(t *testing.T) { if err != nil { return err } - m, err := Accept(conn, serverKey) + m, err := Accept(conn, serverKey, 3) if err != nil { return err } @@ -111,7 +111,7 @@ func TestMux(t *testing.T) { if err != nil { t.Fatal(err) } - m, err := Dial(conn, serverKey.Public().(ed25519.PublicKey)) + m, err := Dial(conn, serverKey.Public().(ed25519.PublicKey), 3) if err != nil { t.Fatal(err) } From 405761859e3e7b0094fb0168786ae1785268921f Mon Sep 17 00:00:00 2001 From: lukechampine Date: Wed, 19 Feb 2025 11:00:13 -0500 Subject: [PATCH 2/2] v2: Remove nonce derivation and hash pubkeys into shared secret --- spec_v2.md | 16 +++++------ v2/handshake.go | 71 ++++++++++++++++++++++++++----------------------- v2/mux_test.go | 61 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 42 deletions(-) diff --git a/spec_v2.md b/spec_v2.md index e0431a7..2426f75 100644 --- a/spec_v2.md +++ b/spec_v2.md @@ -23,7 +23,7 @@ when a "final" frame is sent, or (forcibly) when the connection is closed. The session is encrypted and authenticated: the dialer must know their peer's Ed25519 public key, which is used to sign the handshake and thereby derive a shared secret. This secret is then used to encrypt each frame with -ChaCha20-Poly1305. +ChaCha20-Poly1305, incrementing the nonce after each packet. All integers in this spec are little-endian. @@ -39,8 +39,9 @@ The *dialing peer* generates an X25519 keypair and sends: The current version is 3. The *accepting* peer generates an X25519 keypair, derives the shared X25519 -secret, hashes it with BLAKE2b for use as a ChaCha20-Poly1305 key, hashes that -key *again* to derive the initial nonce value, and responds with: +secret, and computes the ChaCha20-Poly1305 key as `BLAKE2b(secret | k1 | k2)`, +where `k1` is `k2` are the dialing and accepting X25519 pubkeys. It initialies +its nonce to `0`, and responds with: | Length | Type | Description | |--------|--------|--------------------| @@ -49,8 +50,8 @@ key *again* to derive the initial nonce value, and responds with: | 64 | []byte | Ed25519 signature | | 24 | | Encrypted settings | -Finally, the dialing peer derives the shared secret and responds with its own -encrypted settings. +Finally, the dialing peer derives the same ChaCha20-Poly1305 key, initializes +its nonce to `1<<95`, and responds with its own encrypted settings. The settings are: @@ -116,9 +117,8 @@ must not be split across packet boundaries. (In other words, the maximum size of a frame's payload is `n - (4 + 2 + 2)`.) A separate nonce is tracked for both the dialing and accepting peer, incremented -after each use. The initial value for the nonces is the first 12 bytes of -BLAKE2b(BLAKE2b(shared secret)), but the most significant bit of the accepting -peer's nonce is flipped. To increment a nonce, interpret its least-significant 8 +after each use. The initial nonce value is `0` for the dialing peer and `1<<95` +for the accepting peer. To increment a nonce, interpret its least-significant 8 bytes as a 64-bit unsigned integer. ### Covert Frames diff --git a/v2/handshake.go b/v2/handshake.go index 0179e7a..d680205 100644 --- a/v2/handshake.go +++ b/v2/handshake.go @@ -44,34 +44,6 @@ func (c *seqCipher) decryptInPlace(buf []byte) ([]byte, error) { return plaintext, err } -func deriveSharedCipher(xsk, xpk [32]byte) (*seqCipher, error) { - // NOTE: an error is only possible here if xpk is a "low-order point." - // Basically, if the other party chooses one of these points as their public - // key, then the resulting "secret" can be derived by anyone who observes - // the handshake, effectively rendering the protocol unencrypted. This would - // be a strange thing to do; the other party can decrypt the messages - // anyway, so if they want to make the messages public, nothing can stop - // them from doing so. Consequently, some people (notably djb himself) will - // tell you not to bother checking for low-order points at all. But why - // would we want to talk to a peer that's behaving weirdly? - secret, err := curve25519.X25519(xsk[:], xpk[:]) - if err != nil { - return nil, err - } - key := blake2b.Sum256(secret) - c, err := chacha20poly1305.New(key[:]) - if err != nil { - return nil, err - } - // hash the key again to get the initial nonce value - nonce := blake2b.Sum256(key[:]) - return &seqCipher{ - aead: c, - ourNonce: *(*[chachaPoly1305NonceSize]byte)(nonce[:]), - theirNonce: *(*[chachaPoly1305NonceSize]byte)(nonce[:]), - }, nil -} - type connSettings struct { PacketSize int MaxTimeout time.Duration @@ -156,12 +128,31 @@ func initiateHandshake(conn net.Conn, theirKey ed25519.PublicKey, theirVersion u } // derive shared cipher - cipher, err := deriveSharedCipher(xsk, rxpk) + secret, err := curve25519.X25519(xsk[:], rxpk[:]) if err != nil { + // NOTE: an error is only possible here if xpk is a "low-order point." + // Basically, if the other party chooses one of these points as their public + // key, then the resulting "secret" can be derived by anyone who observes + // the handshake, effectively rendering the protocol unencrypted. This would + // be a strange thing to do; the other party can decrypt the messages + // anyway, so if they want to make the messages public, nothing can stop + // them from doing so. Consequently, some people (notably djb himself) will + // tell you not to bother checking for low-order points at all. But why + // would we want to talk to a peer that's behaving weirdly? return nil, connSettings{}, fmt.Errorf("failed to derive shared cipher: %w", err) } - if theirVersion > 2 { - // flip the most significant bit of their nonce + var cipher *seqCipher + if theirVersion <= 2 { + key := blake2b.Sum256(secret) + aead, _ := chacha20poly1305.New(key[:]) // no error possible + cipher = &seqCipher{aead: aead} + nonce := blake2b.Sum256(key[:]) + copy(cipher.ourNonce[:], nonce[:]) + copy(cipher.theirNonce[:], nonce[:]) + } else { + key := blake2b.Sum256(append(append(secret, xpk[:]...), rxpk[:]...)) + aead, _ := chacha20poly1305.New(key[:]) // no error possible + cipher = &seqCipher{aead: aead} cipher.theirNonce[len(cipher.theirNonce)-1] ^= 0x80 } @@ -195,12 +186,24 @@ func acceptHandshake(conn net.Conn, ourKey ed25519.PrivateKey, theirVersion uint // derive shared cipher var rxpk [32]byte copy(rxpk[:], buf[:32]) - cipher, err := deriveSharedCipher(xsk, rxpk) + + // derive shared cipher + secret, err := curve25519.X25519(xsk[:], rxpk[:]) if err != nil { return nil, connSettings{}, fmt.Errorf("failed to derive shared cipher: %w", err) } - if theirVersion > 2 { - // flip the most significant bit of our nonce + var cipher *seqCipher + if theirVersion <= 2 { + key := blake2b.Sum256(secret) + aead, _ := chacha20poly1305.New(key[:]) + cipher = &seqCipher{aead: aead} + nonce := blake2b.Sum256(key[:]) + copy(cipher.ourNonce[:], nonce[:]) + copy(cipher.theirNonce[:], nonce[:]) + } else { + key := blake2b.Sum256(append(append(secret, rxpk[:]...), xpk[:]...)) + aead, _ := chacha20poly1305.New(key[:]) + cipher = &seqCipher{aead: aead} cipher.ourNonce[len(cipher.ourNonce)-1] ^= 0x80 } diff --git a/v2/mux_test.go b/v2/mux_test.go index 3d3a560..7709083 100644 --- a/v2/mux_test.go +++ b/v2/mux_test.go @@ -546,6 +546,67 @@ func TestCovertStream(t *testing.T) { } } +func TestCompatV2(t *testing.T) { + serverKey := ed25519.NewKeyFromSeed(frand.Bytes(ed25519.SeedSize)) + l, err := net.Listen("tcp", ":0") + if err != nil { + t.Fatal(err) + } + serverCh := make(chan error, 1) + go func() { + serverCh <- func() error { + conn, err := l.Accept() + if err != nil { + return err + } + m, err := Accept(conn, serverKey, 2) + if err != nil { + return err + } + defer m.Close() + s, err := m.AcceptStream() + if err != nil { + return err + } + defer s.Close() + buf := make([]byte, 100) + if n, err := s.Read(buf); err != nil { + return err + } else if _, err := fmt.Fprintf(s, "hello, %s!", buf[:n]); err != nil { + return err + } + return s.Close() + }() + }() + + conn, err := net.Dial("tcp", l.Addr().String()) + if err != nil { + t.Fatal(err) + } + m, err := Dial(conn, serverKey.Public().(ed25519.PublicKey), 2) + if err != nil { + t.Fatal(err) + } + defer m.Close() + s := m.DialStream() + defer s.Close() + buf := make([]byte, 100) + if _, err := s.Write([]byte("world")); err != nil { + t.Fatal(err) + } else if n, err := io.ReadFull(s, buf[:13]); err != nil { + t.Fatal(err) + } else if string(buf[:n]) != "hello, world!" { + t.Fatal("bad hello:", string(buf[:n])) + } + if err := s.Close(); err != nil && err != ErrPeerClosedConn { + t.Fatal(err) + } + + if err := <-serverCh; err != nil && err != ErrPeerClosedStream { + t.Fatal(err) + } +} + func BenchmarkMux(b *testing.B) { for _, numStreams := range []int{1, 2, 10, 100, 500, 1000} { b.Run(fmt.Sprint(numStreams), func(b *testing.B) {