From 2f00e5cad120ae333ed88f90dee5f0fcad90f4ed Mon Sep 17 00:00:00 2001 From: zelig Date: Fri, 23 Jan 2015 12:44:57 +0000 Subject: [PATCH] add correct authentication with HMAC --- p2p/crypto.go | 19 ++++++++++--------- p2p/crypto_test.go | 12 ++++++------ p2p/encryption.go | 18 +++++++----------- p2p/encryption_test.go | 4 ++-- p2p/peer.go | 3 +++ 5 files changed, 28 insertions(+), 28 deletions(-) diff --git a/p2p/crypto.go b/p2p/crypto.go index e7ad29ec39..e6cd82d80e 100644 --- a/p2p/crypto.go +++ b/p2p/crypto.go @@ -150,7 +150,7 @@ func (self *cryptoId) NewSession(r io.Reader, w io.Writer, remotePubKeyS []byte, } clogger.Debugf("receiver handshake (sent to %v):\n%v", hexkey(remotePubKeyS), hexkey(response)) } - return self.newSession(r, w, initNonce, recNonce, auth, randomPrivKey, remoteRandomPubKey) + return self.newSession(r, w, initiator, initNonce, recNonce, auth, randomPrivKey, remoteRandomPubKey) } /* @@ -357,7 +357,7 @@ func (self *cryptoId) completeHandshake(auth []byte) (respNonce []byte, remoteRa /* newSession is called after the handshake is completed. The arguments are values negotiated in the handshake and the return value is a new session : a new session Token to be remembered for the next time we connect with this peer. And a MsgReadWriter that implements an encrypted and authenticated connection with key material obtained from the crypto handshake key exchange */ -func (self *cryptoId) newSession(r io.Reader, w io.Writer, initNonce, respNonce, auth []byte, privKey *ecdsa.PrivateKey, remoteRandomPubKey *ecdsa.PublicKey) (sessionToken []byte, rw MsgReadWriter, err error) { +func (self *cryptoId) newSession(r io.Reader, w io.Writer, initiator bool, initNonce, respNonce, auth []byte, privKey *ecdsa.PrivateKey, remoteRandomPubKey *ecdsa.PublicKey) (sessionToken []byte, rw MsgReadWriter, err error) { // 3) Now we can trust ecdhe-random-pubk to derive new keys //ecdhe-shared-secret = ecdh.agree(ecdhe-random, remote-ecdhe-random-pubk) var dhSharedSecret []byte @@ -375,13 +375,14 @@ func (self *cryptoId) newSession(r io.Reader, w io.Writer, initNonce, respNonce, // mac-secret = crypto.Sha3(ecdhe-shared-secret || aes-secret) var macSecret = crypto.Sha3(append(dhSharedSecret, aesSecret...)) // # destroy ecdhe-shared-secret - // egress-mac = crypto.Sha3(mac-secret^nonce || auth) - var egressMac = crypto.Sha3(append(Xor(macSecret, respNonce), auth...)) - // # destroy nonce - // ingress-mac = crypto.Sha3(mac-secret^initiator-nonce || auth), - var ingressMac = crypto.Sha3(append(Xor(macSecret, initNonce), auth...)) - // # destroy remote-nonce - + var egressMac, ingressMac []byte + if initiator { + egressMac = Xor(macSecret, respNonce) + ingressMac = Xor(macSecret, initNonce) + } else { + egressMac = Xor(macSecret, initNonce) + ingressMac = Xor(macSecret, respNonce) + } clogger.Debugf("aes-secret: %v", hexkey(aesSecret)) clogger.Debugf("mac-secret: %v", hexkey(macSecret)) clogger.Debugf("egress-mac: %v", hexkey(egressMac)) diff --git a/p2p/crypto_test.go b/p2p/crypto_test.go index 2cb9c9bbe4..fd6b8aa290 100644 --- a/p2p/crypto_test.go +++ b/p2p/crypto_test.go @@ -109,12 +109,12 @@ func TestCryptoHandshake(t *testing.T) { conn0, conn1 := net.Pipe() // now both parties should have the same session parameters - initSessionToken, initRW, err := initiator.newSession(bufio.NewReader(conn0), conn0, initNonce, recNonce, auth, randomPrivKey, remoteRandomPubKey) + initSessionToken, initRW, err := initiator.newSession(bufio.NewReader(conn0), conn0, true, initNonce, recNonce, auth, randomPrivKey, remoteRandomPubKey) if err != nil { t.Errorf("%v", err) } - recSessionToken, recRW, err := receiver.newSession(bufio.NewReader(conn1), conn1, remoteInitNonce, remoteRecNonce, auth, remoteRandomPrivKey, remoteInitRandomPubKey) + recSessionToken, recRW, err := receiver.newSession(bufio.NewReader(conn1), conn1, false, remoteInitNonce, remoteRecNonce, auth, remoteRandomPrivKey, remoteInitRandomPubKey) if err != nil { t.Errorf("%v", err) } @@ -149,11 +149,11 @@ func TestCryptoHandshake(t *testing.T) { if !bytes.Equal(initSecretRW.macSecret, recSecretRW.macSecret) { t.Errorf("macSecrets do not match") } - if !bytes.Equal(initSecretRW.egressMac, recSecretRW.egressMac) { - t.Errorf("egressMacs do not match") + if !bytes.Equal(initSecretRW.egressMac, recSecretRW.ingressMac) { + t.Errorf("initiator's egressMac do not match receiver's ingressMac") } - if !bytes.Equal(initSecretRW.ingressMac, recSecretRW.ingressMac) { - t.Errorf("ingressMacs do not match") + if !bytes.Equal(initSecretRW.ingressMac, recSecretRW.egressMac) { + t.Errorf("initiator's inressMac do not match receiver's egressMac") } } diff --git a/p2p/encryption.go b/p2p/encryption.go index b0e6cfd501..66fc3c4277 100644 --- a/p2p/encryption.go +++ b/p2p/encryption.go @@ -7,11 +7,9 @@ import ( "crypto/hmac" "crypto/sha256" "encoding/binary" - "fmt" "hash" "io" - // "github.com/ethereum/go-ethereum/crypto/sha256" "github.com/ethereum/go-ethereum/ethutil" "github.com/ethereum/go-ethereum/rlp" ) @@ -82,11 +80,11 @@ func (self *CryptoMsgRW) WriteMsg(msg Msg) (err error) { copy(plaintext[listhdrLen:], code) msg.Payload.Read(plaintext[listhdrLen+codeLen:]) self.Encrypt(ciphertext, plaintext) - fmt.Printf("ENCRYPT:\npt: %v\nct: %v\n", hexkey(plaintext), hexkey(ciphertext)) if _, err = self.w.Write(ciphertext); err != nil { return } - if _, err = self.w.Write(self.egress.Sum(nil)); err != nil { + mac := self.egress.Sum(nil) + if _, err = self.w.Write(mac); err != nil { return } @@ -130,18 +128,16 @@ func (self *CryptoMsgRW) readPayload(size uint32) (r rlp.ByteReader, err error) ciphertext := make([]byte, size) self.r.Read(ciphertext) self.Decrypt(plaintext, ciphertext) - fmt.Printf("DECRYPT:\npt: %v\nct: %v\n", hexkey(plaintext), hexkey(ciphertext)) mac := make([]byte, 32) if _, err = self.r.Read(mac); err != nil { err = newPeerError(errRead, "%v", err) return } - // var expectedMac = self.ingress.Sum(nil) - // if !hmac.Equal(expectedMac, mac) { - // err = newPeerError(errAuthentication, "ingress incorrect") - // return - // } + var expectedMac = self.ingress.Sum(nil) + if !hmac.Equal(expectedMac, mac) { + err = newPeerError(errAuthentication, "ingress incorrect:\nexp %v\ngot %v\n", hexkey(expectedMac), hexkey(mac)) + return + } r = bytes.NewReader(plaintext) - // r = io.LimitReader(bytes.NewReader(plaintext), int64(size)) return } diff --git a/p2p/encryption_test.go b/p2p/encryption_test.go index 5128c95eda..20de23a23c 100644 --- a/p2p/encryption_test.go +++ b/p2p/encryption_test.go @@ -4,7 +4,6 @@ import ( "bufio" "bytes" "crypto/rand" - // "fmt" "io" "net" "testing" @@ -47,7 +46,8 @@ func TestEncryption(t *testing.T) { } messenger0 := NewMessenger(rw0) - rw1, err := NewCryptoMsgRW(bufio.NewReader(conn1), conn1, args[0], args[1], args[2], args[3]) + // note that args 3/2 swapped! ingress <-> egress MAC should reverse + rw1, err := NewCryptoMsgRW(bufio.NewReader(conn1), conn1, args[0], args[1], args[3], args[2]) if err != nil { return } diff --git a/p2p/peer.go b/p2p/peer.go index 7704c764b8..a9afe4322a 100644 --- a/p2p/peer.go +++ b/p2p/peer.go @@ -321,6 +321,8 @@ func (p *Peer) handleCryptoHandshake() (err error) { if crw, err = NewMsgRW(bufio.NewReader(p.conn), p.conn); err != nil { return } + p.Infof("insecure connection using no encryption/authentication") + case EthCrypto: // cryptoId is just created for the lifecycle of the handshake // it is survived by an encrypted readwriter @@ -350,6 +352,7 @@ func (p *Peer) handleCryptoHandshake() (err error) { p.Errorf("%v", err) } p.crw = NewMessenger(crw) + p.Infof("secure connection using %v", p.CryptoType) return }