Update p2p/conn/secret_connection.go

Co-Authored-By: Bot from GolangCI <42910462+golangcibot@users.noreply.github.com>

p2p/conn/secret_connection.go

@@ -73,6 +73,11 @@ type SecretConnection struct {
 // Caller should call conn.Close()
 // See docs/sts-final.pdf for more information.
 func MakeSecretConnection(conn io.ReadWriteCloser, locPrivKey crypto.PrivKey) (*SecretConnection, error) {
+
+	hash := sha256.New
+	hdkf_state := new([32]byte)
+	hkdfInit := hkdf.New(hash, []byte("INIT_HANDSHAKE"), nil, []byte("TENDERMINT_SECRET_CONNECTION_TRANSCRIPT_HASH"))
+
 	locPubKey := locPrivKey.PubKey()
 
 	// Generate ephemeral keys for perfect forward secrecy.
@@ -87,7 +92,19 @@ func MakeSecretConnection(conn io.ReadWriteCloser, locPrivKey crypto.PrivKey) (*
 	}
 
 	// Sort by lexical order.
-	loEphPub, _ := sort32(locEphPub, remEphPub)
+	loEphPub, hiEphPub := sort32(locEphPub, remEphPub)
+
+	if _, err := io.ReadFull(hkdfInit, hdkf_state[:]); err != nil {
+		return nil, err
+	}
+
+	hkdfLoEphPub := hkdf.New(hash, loEphPub[:], hdkf_state[:], []byte("TENDERMINT_SECRET_CONNECTION_TRANSCRIPT_HASH"))
+
+	if _, err := io.ReadFull(hkdfLoEphPub, hdkf_state[:]); err != nil {
+		return nil, err
+	}
+
+	hkdfHiEphPub := hkdf.New(hash, hiEphPub[:], hdkf_state[:], []byte("TENDERMINT_SECRET_CONNECTION_TRANSCRIPT_HASH"))
 
 	// Check if the local ephemeral public key
 	// was the least, lexicographically sorted.
@@ -98,9 +115,19 @@ func MakeSecretConnection(conn io.ReadWriteCloser, locPrivKey crypto.PrivKey) (*
 	if err != nil {
 		return nil, err
 	}
+	if _, err := io.ReadFull(hkdfHiEphPub, hdkf_state[:]); err != nil {
+		return nil, err
+	}
 
-	// generate the secret used for receiving, sending, challenge via hkdf-sha2 on dhSecret
-	recvSecret, sendSecret, challenge := deriveSecretAndChallenge(dhSecret, locIsLeast)
+	hkdfSecret := hkdf.New(hash, dhSecret[:], hdkf_state[:], []byte("TENDERMINT_SECRET_CONNECTION_TRANSCRIPT_HASH"))
+
+	if _, err := io.ReadFull(hkdfSecret, hdkf_state[:]); err != nil {
+		return nil, err
+	}
+
+	// generate the secret used for receiving, sending, challenge via HKDF-SHA2
+	// on the transcript state (which itself also uses HKDF-SHA2 to derive a key from the dhSecret)
+	recvSecret, sendSecret, challenge := deriveSecretAndChallenge(hdkf_state, locIsLeast)
 
 	// Construct SecretConnection.
 	sc := &SecretConnection{
@@ -264,9 +291,7 @@ func shareEphPubKey(conn io.ReadWriteCloser, locEphPub *[32]byte) (remEphPub *[3
 			if err2 != nil {
 				return nil, err2, true // abort
 			}
-			if hasSmallOrder(_remEphPub) {
-				return nil, ErrSmallOrderRemotePubKey, true
-			}
+
 			return _remEphPub, nil, false
 		},
 	)
@@ -282,52 +307,6 @@ func shareEphPubKey(conn io.ReadWriteCloser, locEphPub *[32]byte) (remEphPub *[3
 	return &_remEphPub, nil
 }
 
-// use the samne blacklist as lib sodium (see https://eprint.iacr.org/2017/806.pdf for reference):
-// https://github.com/jedisct1/libsodium/blob/536ed00d2c5e0c65ac01e29141d69a30455f2038/src/libsodium/crypto_scalarmult/curve25519/ref10/x25519_ref10.c#L11-L17
-var blacklist = [][32]byte{
-	// 0 (order 4)
-	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
-	// 1 (order 1)
-	{0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
-	// 325606250916557431795983626356110631294008115727848805560023387167927233504
-	//   (order 8)
-	{0xe0, 0xeb, 0x7a, 0x7c, 0x3b, 0x41, 0xb8, 0xae, 0x16, 0x56, 0xe3,
-		0xfa, 0xf1, 0x9f, 0xc4, 0x6a, 0xda, 0x09, 0x8d, 0xeb, 0x9c, 0x32,
-		0xb1, 0xfd, 0x86, 0x62, 0x05, 0x16, 0x5f, 0x49, 0xb8, 0x00},
-	// 39382357235489614581723060781553021112529911719440698176882885853963445705823
-	//    (order 8)
-	{0x5f, 0x9c, 0x95, 0xbc, 0xa3, 0x50, 0x8c, 0x24, 0xb1, 0xd0, 0xb1,
-		0x55, 0x9c, 0x83, 0xef, 0x5b, 0x04, 0x44, 0x5c, 0xc4, 0x58, 0x1c,
-		0x8e, 0x86, 0xd8, 0x22, 0x4e, 0xdd, 0xd0, 0x9f, 0x11, 0x57},
-	// p-1 (order 2)
-	{0xec, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-		0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-		0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f},
-	// p (=0, order 4)
-	{0xed, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-		0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-		0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f},
-	// p+1 (=1, order 1)
-	{0xee, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-		0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-		0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f},
-}
-
-func hasSmallOrder(pubKey [32]byte) bool {
-	isSmallOrderPoint := false
-	for _, bl := range blacklist {
-		if subtle.ConstantTimeCompare(pubKey[:], bl[:]) == 1 {
-			isSmallOrderPoint = true
-			break
-		}
-	}
-	return isSmallOrderPoint
-}
-
 func deriveSecretAndChallenge(dhSecret *[32]byte, locIsLeast bool) (recvSecret, sendSecret *[aeadKeySize]byte, challenge *[32]byte) {
 	hash := sha256.New
 	hkdf := hkdf.New(hash, dhSecret[:], nil, []byte("TENDERMINT_SECRET_CONNECTION_KEY_AND_CHALLENGE_GEN"))

p2p/conn/secret_connection_test.go

@@ -100,49 +100,6 @@ func TestSecretConnectionHandshake(t *testing.T) {
 	}
 }
 
-// Test that shareEphPubKey rejects lower order public keys based on an
-// (incomplete) blacklist.
-func TestShareLowOrderPubkey(t *testing.T) {
-	var fooConn, barConn = makeKVStoreConnPair()
-	defer fooConn.Close()
-	defer barConn.Close()
-	locEphPub, _ := genEphKeys()
-
-	// all blacklisted low order points:
-	for _, remLowOrderPubKey := range blacklist {
-		_, _ = cmn.Parallel(
-			func(_ int) (val interface{}, err error, abort bool) {
-				_, err = shareEphPubKey(fooConn, locEphPub)
-
-				require.Error(t, err)
-				require.Equal(t, err, ErrSmallOrderRemotePubKey)
-
-				return nil, nil, false
-			},
-			func(_ int) (val interface{}, err error, abort bool) {
-				readRemKey, err := shareEphPubKey(barConn, &remLowOrderPubKey)
-
-				require.NoError(t, err)
-				require.Equal(t, locEphPub, readRemKey)
-
-				return nil, nil, false
-			})
-	}
-}
-
-// Test that additionally that the Diffie-Hellman shared secret is non-zero.
-// The shared secret would be zero for lower order pub-keys (but tested against the blacklist only).
-func TestComputeDHFailsOnLowOrder(t *testing.T) {
-	_, locPrivKey := genEphKeys()
-	for _, remLowOrderPubKey := range blacklist {
-		shared, err := computeDHSecret(&remLowOrderPubKey, locPrivKey)
-		assert.Error(t, err)
-
-		assert.Equal(t, err, ErrSharedSecretIsZero)
-		assert.Empty(t, shared)
-	}
-}
-
 func TestConcurrentWrite(t *testing.T) {
 	fooSecConn, barSecConn := makeSecretConnPair(t)
 	fooWriteText := cmn.RandStr(dataMaxSize)