The MakeSecretConnection function has a bug which allows a third party to MITM the connection and decrypt/modify data in a way which both sides think the long-term key of either side is the same.

This is due to a lack of validation that the ephemeral or long-term keys are not of low-order.

1. In shareEphPub the ephemeral keys are transmitted to each other.
2. Then in computeDHSecret a shared secret is derived from the two ephemeral keys using curve25519.ScalarMult
3. Using that secret the send and receive keys, and a challenge, are derived using deriveSecretAndChallenge
4. The challenge and long-term public key is then signed and transmitted to the other party using

The problem stems from computeDHSecret not validating the ephemeral public key, if the connection is intercepted and an ephemeral key consisting of all zeros is injected then the secret from computeDHSecret will be the same for both parties for every handshake with any key. That is - the two parties, because of the MITM injecting a null ephemeral key in place of the remotes key, will then both derive the same secret which is also known to the MITM actor.

The derived recvSecret, sendSecret and challenge is now also known by the MITM actor, and will be the same for every connection handshake that is intercepted. The authSigMsg by the long-term keys of either side will also be the same for every intercepted connection, and because the send/recv secrets are known the MITM actor can replay the auth signature message to the other party to impersonate any public key that it's previously observed.

So:

• Full active intercept with decrypt/re-encrypt
• Impersonation of any peer using signature re-play

This means SecretConnection isn't secure if connections are intercepted / redirected.

And any peers identity can be impersonated at the connection-level (as in, it'll show that peer in the remote peers list), unless the long-term public key of the peer is validated elsewhere (e.g. expecting a signature from the remote peers nodeInfo.ID() cannot be forged, only re-played during the handshake).

Affected functions:

• tendermint/p2p/conn/secret_connection.go

  Line 52 in 6643c5d

  func MakeSecretConnection(conn io.ReadWriteCloser, locPrivKey crypto.PrivKey) (*SecretConnection, error) {

• tendermint/p2p/conn/secret_connection.go

  Line 209 in 6643c5d

  func shareEphPubKey(conn io.ReadWriteCloser, locEphPub *[32]byte) (remEphPub *[32]byte, err error) {

• tendermint/p2p/conn/secret_connection.go

  Line 272 in 6643c5d

  func computeDHSecret(remPubKey, locPrivKey *[32]byte) (shrKey *[32]byte) {

• tendermint/p2p/conn/secret_connection.go

  Line 241 in 6643c5d

  func deriveSecretAndChallenge(dhSecret *[32]byte, locIsLeast bool) (recvSecret, sendSecret *[aeadKeySize]byte, challenge *[32]byte) {

Example code:

package main

import (
	"fmt"
	crand "crypto/rand"
	"golang.org/x/crypto/curve25519"
	"golang.org/x/crypto/nacl/box"
)


func genEphKeys() (ephPub, ephPriv *[32]byte) {
	var err error
	ephPub, ephPriv, err = box.GenerateKey(crand.Reader)
	if err != nil {
		panic("Could not generate ephemeral keypairs")
	}
	return
}

func computeDHSecret(remPubKey, locPrivKey *[32]byte) (shrKey *[32]byte) {
	shrKey = new([32]byte)
	curve25519.ScalarMult(shrKey, locPrivKey, remPubKey)
	return
}

func main() {
	//remPubKey, _ := genEphKeys()
	var remPubKey [32]byte
	_, locPrivKey := genEphKeys()

	shrKey := computeDHSecret(&remPubKey, locPrivKey)

	fmt.Println("remPubKey =", remPubKey)
	fmt.Println("locPrivKey =", locPrivKey)
	fmt.Println("shrKey =", shrKey)
}

Suggested fix:

Make a reader specifically for public keys, perform low-order point validation on the keys after reading.