diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 37413640c..626768293 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -32,6 +32,9 @@ program](https://hackerone.com/tendermint). - Go API - [libs/pubsub] [\#4070](https://github.com/tendermint/tendermint/pull/4070) `Query#(Matches|Conditions)` returns an error. +- P2P Protocol + - [p2p] [\3668](https://github.com/tendermint/tendermint/pull/3668) Make `SecretConnection` non-malleable + ### FEATURES: ### IMPROVEMENTS: diff --git a/p2p/conn/secret_connection.go b/p2p/conn/secret_connection.go index 6dfa02e53..9d6825df8 100644 --- a/p2p/conn/secret_connection.go +++ b/p2p/conn/secret_connection.go @@ -77,24 +77,40 @@ 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) { - locPubKey := locPrivKey.PubKey() + var ( + hash = sha256.New + hdkfState = new([32]byte) + locPubKey = locPrivKey.PubKey() + ) // Generate ephemeral keys for perfect forward secrecy. locEphPub, locEphPriv := genEphKeys() // Write local ephemeral pubkey and receive one too. - // NOTE: every 32-byte string is accepted as a Curve25519 public key - // (see DJB's Curve25519 paper: http://cr.yp.to/ecdh/curve25519-20060209.pdf) + // NOTE: every 32-byte string is accepted as a Curve25519 public key (see + // DJB's Curve25519 paper: http://cr.yp.to/ecdh/curve25519-20060209.pdf) remEphPub, err := shareEphPubKey(conn, locEphPub) if err != nil { return nil, err } // Sort by lexical order. - loEphPub, _ := sort32(locEphPub, remEphPub) + loEphPub, hiEphPub := sort32(locEphPub, remEphPub) + + hkdfInit := hkdf.New(hash, []byte("INIT_HANDSHAKE"), nil, []byte("TENDERMINT_SECRET_CONNECTION_TRANSCRIPT_HASH")) + if _, err := io.ReadFull(hkdfInit, hdkfState[:]); err != nil { + return nil, err + } + + hkdfLoEphPub := hkdf.New(hash, loEphPub[:], hdkfState[:], []byte("TENDERMINT_SECRET_CONNECTION_TRANSCRIPT_HASH")) + if _, err := io.ReadFull(hkdfLoEphPub, hdkfState[:]); err != nil { + return nil, err + } + + hkdfHiEphPub := hkdf.New(hash, hiEphPub[:], hdkfState[:], []byte("TENDERMINT_SECRET_CONNECTION_TRANSCRIPT_HASH")) - // Check if the local ephemeral public key - // was the least, lexicographically sorted. + // Check if the local ephemeral public key was the least, lexicographically + // sorted. locIsLeast := bytes.Equal(locEphPub[:], loEphPub[:]) // Compute common diffie hellman secret using X25519. @@ -102,9 +118,19 @@ func MakeSecretConnection(conn io.ReadWriteCloser, locPrivKey crypto.PrivKey) (* if err != nil { return nil, err } + if _, err := io.ReadFull(hkdfHiEphPub, hdkfState[:]); err != nil { + return nil, err + } + + hkdfSecret := hkdf.New(hash, dhSecret[:], hdkfState[:], []byte("TENDERMINT_SECRET_CONNECTION_TRANSCRIPT_HASH")) + if _, err := io.ReadFull(hkdfSecret, hdkfState[:]); err != nil { + return nil, err + } - // generate the secret used for receiving, sending, challenge via hkdf-sha2 on dhSecret - recvSecret, sendSecret, challenge := deriveSecretAndChallenge(dhSecret, locIsLeast) + // 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(hdkfState, locIsLeast) sendAead, err := chacha20poly1305.New(sendSecret[:]) if err != nil { @@ -114,7 +140,7 @@ func MakeSecretConnection(conn io.ReadWriteCloser, locPrivKey crypto.PrivKey) (* if err != nil { return nil, errors.New("invalid receive SecretConnection Key") } - // Construct SecretConnection. + sc := &SecretConnection{ conn: conn, recvBuffer: nil, @@ -134,11 +160,9 @@ func MakeSecretConnection(conn io.ReadWriteCloser, locPrivKey crypto.PrivKey) (* } remPubKey, remSignature := authSigMsg.Key, authSigMsg.Sig - if _, ok := remPubKey.(ed25519.PubKeyEd25519); !ok { return nil, errors.Errorf("expected ed25519 pubkey, got %T", remPubKey) } - if !remPubKey.VerifyBytes(challenge[:], remSignature) { return nil, errors.New("challenge verification failed") } @@ -285,9 +309,7 @@ func shareEphPubKey(conn io.ReadWriter, locEphPub *[32]byte) (remEphPub *[32]byt if err2 != nil { return nil, err2, true // abort } - if hasSmallOrder(_remEphPub) { - return nil, ErrSmallOrderRemotePubKey, true - } + return _remEphPub, nil, false }, ) @@ -303,52 +325,6 @@ func shareEphPubKey(conn io.ReadWriter, locEphPub *[32]byte) (remEphPub *[32]byt 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, diff --git a/p2p/conn/secret_connection_test.go b/p2p/conn/secret_connection_test.go index 0438ec259..e0d5f4fac 100644 --- a/p2p/conn/secret_connection_test.go +++ b/p2p/conn/secret_connection_test.go @@ -101,51 +101,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 { - remLowOrderPubKey := remLowOrderPubKey - _, _ = 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 { - remLowOrderPubKey := remLowOrderPubKey - 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)