diff --git a/crypto/multisig/threshold_pubkey.go b/crypto/multisig/threshold_pubkey.go index 234d420f1..36e2dc2dd 100644 --- a/crypto/multisig/threshold_pubkey.go +++ b/crypto/multisig/threshold_pubkey.go @@ -21,6 +21,11 @@ func NewPubKeyMultisigThreshold(k int, pubkeys []crypto.PubKey) crypto.PubKey { if len(pubkeys) < k { panic("threshold k of n multisignature: len(pubkeys) < k") } + for _, pubkey := range pubkeys { + if pubkey == nil { + panic("nil pubkey") + } + } return PubKeyMultisigThreshold{uint(k), pubkeys} } diff --git a/p2p/conn/secret_connection.go b/p2p/conn/secret_connection.go index f57e589e6..088c81252 100644 --- a/p2p/conn/secret_connection.go +++ b/p2p/conn/secret_connection.go @@ -7,21 +7,22 @@ import ( "crypto/sha256" "crypto/subtle" "encoding/binary" - "errors" "io" "math" "net" "sync" "time" + pool "github.com/libp2p/go-buffer-pool" + "github.com/pkg/errors" "golang.org/x/crypto/chacha20poly1305" "golang.org/x/crypto/curve25519" + "golang.org/x/crypto/hkdf" "golang.org/x/crypto/nacl/box" - pool "github.com/libp2p/go-buffer-pool" "github.com/tendermint/tendermint/crypto" + "github.com/tendermint/tendermint/crypto/ed25519" cmn "github.com/tendermint/tendermint/libs/common" - "golang.org/x/crypto/hkdf" ) // 4 + 1024 == 1028 total frame size @@ -107,11 +108,11 @@ func MakeSecretConnection(conn io.ReadWriteCloser, locPrivKey crypto.PrivKey) (* sendAead, err := chacha20poly1305.New(sendSecret[:]) if err != nil { - return nil, errors.New("Invalid send SecretConnection Key") + return nil, errors.New("invalid send SecretConnection Key") } recvAead, err := chacha20poly1305.New(recvSecret[:]) if err != nil { - return nil, errors.New("Invalid receive SecretConnection Key") + return nil, errors.New("invalid receive SecretConnection Key") } // Construct SecretConnection. sc := &SecretConnection{ @@ -134,12 +135,12 @@ func MakeSecretConnection(conn io.ReadWriteCloser, locPrivKey crypto.PrivKey) (* remPubKey, remSignature := authSigMsg.Key, authSigMsg.Sig - if remPubKey == nil { - return nil, errors.New("peer sent a nil public key") + 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") + return nil, errors.New("challenge verification failed") } // We've authorized. @@ -222,7 +223,7 @@ func (sc *SecretConnection) Read(data []byte) (n int, err error) { defer pool.Put(frame) _, err = sc.recvAead.Open(frame[:0], sc.recvNonce[:], sealedFrame, nil) if err != nil { - return n, errors.New("Failed to decrypt SecretConnection") + return n, errors.New("failed to decrypt SecretConnection") } incrNonce(sc.recvNonce) // end decryption diff --git a/p2p/conn/secret_connection_test.go b/p2p/conn/secret_connection_test.go index 743fcf4c4..0438ec259 100644 --- a/p2p/conn/secret_connection_test.go +++ b/p2p/conn/secret_connection_test.go @@ -16,7 +16,9 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/ed25519" + "github.com/tendermint/tendermint/crypto/secp256k1" cmn "github.com/tendermint/tendermint/libs/common" ) @@ -365,6 +367,51 @@ func TestDeriveSecretsAndChallengeGolden(t *testing.T) { } } +type privKeyWithNilPubKey struct { + orig crypto.PrivKey +} + +func (pk privKeyWithNilPubKey) Bytes() []byte { return pk.orig.Bytes() } +func (pk privKeyWithNilPubKey) Sign(msg []byte) ([]byte, error) { return pk.orig.Sign(msg) } +func (pk privKeyWithNilPubKey) PubKey() crypto.PubKey { return nil } +func (pk privKeyWithNilPubKey) Equals(pk2 crypto.PrivKey) bool { return pk.orig.Equals(pk2) } + +func TestNilPubkey(t *testing.T) { + var fooConn, barConn = makeKVStoreConnPair() + var fooPrvKey = ed25519.GenPrivKey() + var barPrvKey = privKeyWithNilPubKey{ed25519.GenPrivKey()} + + go func() { + _, err := MakeSecretConnection(barConn, barPrvKey) + assert.NoError(t, err) + }() + + assert.NotPanics(t, func() { + _, err := MakeSecretConnection(fooConn, fooPrvKey) + if assert.Error(t, err) { + assert.Equal(t, "expected ed25519 pubkey, got ", err.Error()) + } + }) +} + +func TestNonEd25519Pubkey(t *testing.T) { + var fooConn, barConn = makeKVStoreConnPair() + var fooPrvKey = ed25519.GenPrivKey() + var barPrvKey = secp256k1.GenPrivKey() + + go func() { + _, err := MakeSecretConnection(barConn, barPrvKey) + assert.NoError(t, err) + }() + + assert.NotPanics(t, func() { + _, err := MakeSecretConnection(fooConn, fooPrvKey) + if assert.Error(t, err) { + assert.Equal(t, "expected ed25519 pubkey, got secp256k1.PubKeySecp256k1", err.Error()) + } + }) +} + // Creates the data for a test vector file. // The file format is: // Hex(diffie_hellman_secret), loc_is_least, Hex(recvSecret), Hex(sendSecret), Hex(challenge) diff --git a/p2p/transport.go b/p2p/transport.go index 8d6ea236e..95c646ac0 100644 --- a/p2p/transport.go +++ b/p2p/transport.go @@ -6,6 +6,8 @@ import ( "net" "time" + "github.com/pkg/errors" + "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/p2p/conn" ) @@ -270,6 +272,23 @@ func (mt *MultiplexTransport) acceptPeers() { // // [0] https://en.wikipedia.org/wiki/Head-of-line_blocking go func(c net.Conn) { + defer func() { + if r := recover(); r != nil { + err := ErrRejected{ + conn: c, + err: errors.Errorf("recovered from panic: %v", r), + isAuthFailure: true, + } + select { + case mt.acceptc <- accept{err: err}: + case <-mt.closec: + // Give up if the transport was closed. + _ = c.Close() + return + } + } + }() + var ( nodeInfo NodeInfo secretConn *conn.SecretConnection