From 936dd0f3bc19457c8496af00b181f0a8a2f18d6f Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 26 Feb 2015 22:30:34 +0000 Subject: [PATCH 01/13] p2p: add basic RLPx frame I/O --- p2p/rlpx.go | 129 +++++++++++++++++++++++++++++++++++++++++++++++ p2p/rlpx_test.go | 123 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 252 insertions(+) create mode 100644 p2p/rlpx.go create mode 100644 p2p/rlpx_test.go diff --git a/p2p/rlpx.go b/p2p/rlpx.go new file mode 100644 index 000000000..9fd1aed1f --- /dev/null +++ b/p2p/rlpx.go @@ -0,0 +1,129 @@ +package p2p + +import ( + "bytes" + "crypto/aes" + "crypto/cipher" + "crypto/hmac" + "errors" + "hash" + "io" + + "github.com/ethereum/go-ethereum/rlp" +) + +var ( + zeroHeader = []byte{0xC2, 0x80, 0x80} + zero16 = make([]byte, 16) +) + +type rlpxFrameRW struct { + conn io.ReadWriter + + macCipher cipher.Block + egressMAC hash.Hash + ingressMAC hash.Hash +} + +func newRlpxFrameRW(conn io.ReadWriter, macSecret []byte, egressMAC, ingressMAC hash.Hash) *rlpxFrameRW { + cipher, err := aes.NewCipher(macSecret) + if err != nil { + panic("invalid macSecret: " + err.Error()) + } + return &rlpxFrameRW{conn: conn, macCipher: cipher, egressMAC: egressMAC, ingressMAC: ingressMAC} +} + +func (rw *rlpxFrameRW) WriteMsg(msg Msg) error { + ptype, _ := rlp.EncodeToBytes(msg.Code) + + // write header + headbuf := make([]byte, 32) + fsize := uint32(len(ptype)) + msg.Size + putInt24(fsize, headbuf) // TODO: check overflow + copy(headbuf[3:], zeroHeader) + copy(headbuf[16:], updateHeaderMAC(rw.egressMAC, rw.macCipher, headbuf[:16])) + if _, err := rw.conn.Write(headbuf); err != nil { + return err + } + + // write frame, updating the egress MAC while writing to conn. + tee := io.MultiWriter(rw.conn, rw.egressMAC) + if _, err := tee.Write(ptype); err != nil { + return err + } + if _, err := io.Copy(tee, msg.Payload); err != nil { + return err + } + if padding := fsize % 16; padding > 0 { + if _, err := tee.Write(zero16[:16-padding]); err != nil { + return err + } + } + + // write packet-mac. egress MAC is up to date because + // frame content was written to it as well. + _, err := rw.conn.Write(rw.egressMAC.Sum(nil)) + return err +} + +func (rw *rlpxFrameRW) ReadMsg() (msg Msg, err error) { + // read the header + headbuf := make([]byte, 32) + if _, err := io.ReadFull(rw.conn, headbuf); err != nil { + return msg, err + } + fsize := readInt24(headbuf) + // ignore protocol type for now + shouldMAC := updateHeaderMAC(rw.ingressMAC, rw.macCipher, headbuf[:16]) + if !hmac.Equal(shouldMAC[:16], headbuf[16:]) { + return msg, errors.New("bad header MAC") + } + + // read the frame content + framebuf := make([]byte, fsize) + if _, err := io.ReadFull(rw.conn, framebuf); err != nil { + return msg, err + } + rw.ingressMAC.Write(framebuf) + if padding := fsize % 16; padding > 0 { + if _, err := io.CopyN(rw.ingressMAC, rw.conn, int64(16-padding)); err != nil { + return msg, err + } + } + // read and validate frame MAC. we can re-use headbuf for that. + if _, err := io.ReadFull(rw.conn, headbuf); err != nil { + return msg, err + } + if !hmac.Equal(rw.ingressMAC.Sum(nil), headbuf) { + return msg, errors.New("bad frame MAC") + } + + // decode message code + content := bytes.NewReader(framebuf) + if err := rlp.Decode(content, &msg.Code); err != nil { + return msg, err + } + msg.Size = uint32(content.Len()) + msg.Payload = content + return msg, nil +} + +func updateHeaderMAC(mac hash.Hash, block cipher.Block, header []byte) []byte { + aesbuf := make([]byte, aes.BlockSize) + block.Encrypt(aesbuf, mac.Sum(nil)) + for i := range aesbuf { + aesbuf[i] ^= header[i] + } + mac.Write(aesbuf) + return mac.Sum(nil) +} + +func readInt24(b []byte) uint32 { + return uint32(b[2]) | uint32(b[1])<<8 | uint32(b[0])<<16 +} + +func putInt24(v uint32, b []byte) { + b[0] = byte(v >> 16) + b[1] = byte(v >> 8) + b[2] = byte(v) +} diff --git a/p2p/rlpx_test.go b/p2p/rlpx_test.go new file mode 100644 index 000000000..380d9aba6 --- /dev/null +++ b/p2p/rlpx_test.go @@ -0,0 +1,123 @@ +package p2p + +import ( + "bytes" + "crypto/rand" + "encoding/hex" + "fmt" + "io/ioutil" + "strings" + "testing" + + "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/crypto/sha3" + "github.com/ethereum/go-ethereum/rlp" +) + +func TestRlpxFrameFake(t *testing.T) { + buf := new(bytes.Buffer) + secret := crypto.Sha3() + hash := fakeHash([]byte{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}) + rw := newRlpxFrameRW(buf, secret, hash, hash) + + golden := unhex(` +000006C2808000000000000000000000 +01010101010101010101010101010101 +08C40102030400000000000000000000 +01010101010101010101010101010101 +01010101010101010101010101010101 +`) + + // Check WriteMsg. This puts a message into the buffer. + if err := EncodeMsg(rw, 8, []interface{}{1, 2, 3, 4}); err != nil { + t.Fatalf("WriteMsg error: %v", err) + } + written := buf.Bytes() + if !bytes.Equal(written, golden) { + t.Fatalf("output mismatch:\n got: %x\n want: %x", written, golden) + } + + // Check ReadMsg. It reads the message encoded by WriteMsg, which + // is equivalent to the golden message above. + msg, err := rw.ReadMsg() + if err != nil { + t.Fatalf("ReadMsg error: %v", err) + } + if msg.Size != 5 { + t.Errorf("msg size mismatch: got %d, want %d", msg.Size, 5) + } + if msg.Code != 8 { + t.Errorf("msg code mismatch: got %d, want %d", msg.Code, 8) + } + payload, _ := ioutil.ReadAll(msg.Payload) + wantPayload := unhex("C401020304") + if !bytes.Equal(payload, wantPayload) { + t.Errorf("msg payload mismatch:\ngot %x\nwant %x", payload, wantPayload) + } +} + +type fakeHash []byte + +func (fakeHash) Write(p []byte) (int, error) { return len(p), nil } +func (fakeHash) Reset() {} +func (fakeHash) BlockSize() int { return 0 } + +func (h fakeHash) Size() int { return len(h) } +func (h fakeHash) Sum(b []byte) []byte { return append(b, h...) } + +func unhex(str string) []byte { + b, err := hex.DecodeString(strings.Replace(str, "\n", "", -1)) + if err != nil { + panic(fmt.Sprintf("invalid hex string: %q", str)) + } + return b +} + +func TestRlpxFrameRW(t *testing.T) { + var ( + macSecret = make([]byte, 16) + egressMACinit = make([]byte, 32) + ingressMACinit = make([]byte, 32) + ) + for _, s := range [][]byte{macSecret, egressMACinit, ingressMACinit} { + rand.Read(s) + } + + conn := new(bytes.Buffer) + + em1 := sha3.NewKeccak256() + em1.Write(egressMACinit) + im1 := sha3.NewKeccak256() + im1.Write(ingressMACinit) + rw1 := newRlpxFrameRW(conn, macSecret, em1, im1) + + em2 := sha3.NewKeccak256() + em2.Write(ingressMACinit) + im2 := sha3.NewKeccak256() + im2.Write(egressMACinit) + rw2 := newRlpxFrameRW(conn, macSecret, em2, im2) + + // send some messages + for i := 0; i < 10; i++ { + // write message into conn buffer + wmsg := []interface{}{"foo", "bar", strings.Repeat("test", i)} + err := EncodeMsg(rw1, uint64(i), wmsg) + if err != nil { + t.Fatalf("WriteMsg error (i=%d): %v", i, err) + } + + // read message that rw1 just wrote + msg, err := rw2.ReadMsg() + if err != nil { + t.Fatalf("ReadMsg error (i=%d): %v", i, err) + } + if msg.Code != uint64(i) { + t.Fatalf("msg code mismatch: got %d, want %d", msg.Code, i) + } + payload, _ := ioutil.ReadAll(msg.Payload) + wantPayload, _ := rlp.EncodeToBytes(wmsg) + if !bytes.Equal(payload, wantPayload) { + t.Fatalf("msg payload mismatch:\ngot %x\nwant %x", payload, wantPayload) + } + } +} From 51e01cceca81bc5e82896815754b7c33bb6e6005 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 27 Feb 2015 02:09:53 +0000 Subject: [PATCH 02/13] p2p: encrypted and authenticated RLPx frame I/O --- p2p/handshake.go | 157 +++++++++++++++++++++++++++--------------- p2p/handshake_test.go | 100 ++++++--------------------- p2p/rlpx.go | 66 +++++++++++++----- p2p/rlpx_test.go | 44 +++++++----- 4 files changed, 196 insertions(+), 171 deletions(-) diff --git a/p2p/handshake.go b/p2p/handshake.go index 614711eaf..17f572dea 100644 --- a/p2p/handshake.go +++ b/p2p/handshake.go @@ -5,12 +5,14 @@ import ( "crypto/rand" "errors" "fmt" + "hash" "io" "net" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/crypto/ecies" "github.com/ethereum/go-ethereum/crypto/secp256k1" + "github.com/ethereum/go-ethereum/crypto/sha3" "github.com/ethereum/go-ethereum/p2p/discover" "github.com/ethereum/go-ethereum/rlp" ) @@ -38,13 +40,23 @@ func newConn(fd net.Conn, hs *protoHandshake) *conn { return &conn{newFrameRW(fd, msgWriteTimeout), hs} } -// encHandshake represents information about the remote end -// of a connection that is negotiated during the encryption handshake. +// encHandshake contains the state of the encryption handshake. type encHandshake struct { - ID discover.NodeID - IngressMAC []byte - EgressMAC []byte - Token []byte + remoteID discover.NodeID + initiator bool + initNonce, respNonce []byte + dhSharedSecret []byte + randomPrivKey *ecdsa.PrivateKey + remoteRandomPub *ecdsa.PublicKey +} + +// secrets represents the connection secrets +// which are negotiated during the encryption handshake. +type secrets struct { + RemoteID discover.NodeID + AES, MAC []byte + EgressMAC, IngressMAC hash.Hash + Token []byte } // protoHandshake is the RLP structure of the protocol handshake. @@ -56,6 +68,34 @@ type protoHandshake struct { ID discover.NodeID } +// secrets is called after the handshake is completed. +// It extracts the connection secrets from the handshake values. +func (h *encHandshake) secrets(auth, authResp []byte) secrets { + sharedSecret := crypto.Sha3(h.dhSharedSecret, crypto.Sha3(h.respNonce, h.initNonce)) + aesSecret := crypto.Sha3(h.dhSharedSecret, sharedSecret) + s := secrets{ + RemoteID: h.remoteID, + AES: aesSecret, + MAC: crypto.Sha3(h.dhSharedSecret, aesSecret), + Token: crypto.Sha3(sharedSecret), + } + + // setup sha3 instances for the MACs + mac1 := sha3.NewKeccak256() + mac1.Write(xor(s.MAC, h.respNonce)) + mac1.Write(auth) + mac2 := sha3.NewKeccak256() + mac2.Write(xor(s.MAC, h.initNonce)) + mac2.Write(authResp) + if h.initiator { + s.EgressMAC, s.IngressMAC = mac1, mac2 + } else { + s.EgressMAC, s.IngressMAC = mac2, mac1 + } + + return s +} + // setupConn starts a protocol session on the given connection. // It runs the encryption handshake and the protocol handshake. // If dial is non-nil, the connection the local node is the initiator. @@ -68,36 +108,47 @@ func setupConn(fd net.Conn, prv *ecdsa.PrivateKey, our *protoHandshake, dial *di } func setupInboundConn(fd net.Conn, prv *ecdsa.PrivateKey, our *protoHandshake) (*conn, error) { - // var remotePubkey []byte - // sessionToken, remotePubkey, err = inboundEncHandshake(fd, prv, nil) - // copy(remoteID[:], remotePubkey) + secrets, err := inboundEncHandshake(fd, prv, nil) + if err != nil { + return nil, fmt.Errorf("encryption handshake failed: %v", err) + } - rw := newFrameRW(fd, msgWriteTimeout) - rhs, err := readProtocolHandshake(rw, our) + // Run the protocol handshake using authenticated messages. + // TODO: move buffering setup here (out of newFrameRW) + phsrw := newRlpxFrameRW(fd, secrets) + rhs, err := readProtocolHandshake(phsrw, our) if err != nil { return nil, err } - if err := writeProtocolHandshake(rw, our); err != nil { + if err := writeProtocolHandshake(phsrw, our); err != nil { return nil, fmt.Errorf("protocol write error: %v", err) } + + rw := newFrameRW(fd, msgWriteTimeout) return &conn{rw, rhs}, nil } func setupOutboundConn(fd net.Conn, prv *ecdsa.PrivateKey, our *protoHandshake, dial *discover.Node) (*conn, error) { - // remoteID = dial.ID - // sessionToken, err = outboundEncHandshake(fd, prv, remoteID[:], nil) + secrets, err := outboundEncHandshake(fd, prv, dial.ID[:], nil) + if err != nil { + return nil, fmt.Errorf("encryption handshake failed: %v", err) + } - rw := newFrameRW(fd, msgWriteTimeout) - if err := writeProtocolHandshake(rw, our); err != nil { + // Run the protocol handshake using authenticated messages. + // TODO: move buffering setup here (out of newFrameRW) + phsrw := newRlpxFrameRW(fd, secrets) + if err := writeProtocolHandshake(phsrw, our); err != nil { return nil, fmt.Errorf("protocol write error: %v", err) } - rhs, err := readProtocolHandshake(rw, our) + rhs, err := readProtocolHandshake(phsrw, our) if err != nil { return nil, fmt.Errorf("protocol handshake read error: %v", err) } if rhs.ID != dial.ID { return nil, errors.New("dialed node id mismatch") } + + rw := newFrameRW(fd, msgWriteTimeout) return &conn{rw, rhs}, nil } @@ -107,43 +158,48 @@ func setupOutboundConn(fd net.Conn, prv *ecdsa.PrivateKey, our *protoHandshake, // privateKey is the local client's private key // remotePublicKey is the remote peer's node ID // sessionToken is the token from a previous session with this node. -func outboundEncHandshake(conn io.ReadWriter, prvKey *ecdsa.PrivateKey, remotePublicKey []byte, sessionToken []byte) ( - newSessionToken []byte, - err error, -) { +func outboundEncHandshake(conn io.ReadWriter, prvKey *ecdsa.PrivateKey, remotePublicKey []byte, sessionToken []byte) (s secrets, err error) { auth, initNonce, randomPrivKey, err := authMsg(prvKey, remotePublicKey, sessionToken) if err != nil { - return nil, err + return s, err } if _, err = conn.Write(auth); err != nil { - return nil, err + return s, err } response := make([]byte, rHSLen) if _, err = io.ReadFull(conn, response); err != nil { - return nil, err + return s, err } recNonce, remoteRandomPubKey, _, err := completeHandshake(response, prvKey) if err != nil { - return nil, err + return s, err } - return newSession(initNonce, recNonce, randomPrivKey, remoteRandomPubKey) + h := &encHandshake{ + initiator: true, + initNonce: initNonce, + respNonce: recNonce, + randomPrivKey: randomPrivKey, + remoteRandomPub: remoteRandomPubKey, + } + copy(h.remoteID[:], remotePublicKey) + return h.secrets(auth, response), nil } // authMsg creates the initiator handshake. +// TODO: change all the names func authMsg(prvKey *ecdsa.PrivateKey, remotePubKeyS, sessionToken []byte) ( auth, initNonce []byte, randomPrvKey *ecdsa.PrivateKey, err error, ) { - // session init, common to both parties remotePubKey, err := importPublicKey(remotePubKeyS) if err != nil { return } - var tokenFlag byte // = 0x00 + var tokenFlag byte if sessionToken == nil { // no session token found means we need to generate shared secret. // ecies shared secret is used as initial session token for new peers @@ -151,14 +207,13 @@ func authMsg(prvKey *ecdsa.PrivateKey, remotePubKeyS, sessionToken []byte) ( if sessionToken, err = ecies.ImportECDSA(prvKey).GenerateShared(ecies.ImportECDSAPublic(remotePubKey), sskLen, sskLen); err != nil { return } - // tokenFlag = 0x00 // redundant } else { // for known peers, we use stored token from the previous session tokenFlag = 0x01 } - //E(remote-pubk, S(ecdhe-random, ecdh-shared-secret^nonce) || H(ecdhe-random-pubk) || pubk || nonce || 0x0) - // E(remote-pubk, S(ecdhe-random, token^nonce) || H(ecdhe-random-pubk) || pubk || nonce || 0x1) + //E(remote-pubk, S(ecdhe-random, sha3(ecdh-shared-secret^nonce)) || H(ecdhe-random-pubk) || pubk || nonce || 0x0) + // E(remote-pubk, S(ecdhe-random, sha3(token^nonce)) || H(ecdhe-random-pubk) || pubk || nonce || 0x1) // allocate msgLen long message, var msg []byte = make([]byte, authMsgLen) initNonce = msg[authMsgLen-shaLen-1 : authMsgLen-1] @@ -242,27 +297,32 @@ func completeHandshake(auth []byte, prvKey *ecdsa.PrivateKey) ( // // privateKey is the local client's private key // sessionToken is the token from a previous session with this node. -func inboundEncHandshake(conn io.ReadWriter, prvKey *ecdsa.PrivateKey, sessionToken []byte) ( - token, remotePubKey []byte, - err error, -) { +func inboundEncHandshake(conn io.ReadWriter, prvKey *ecdsa.PrivateKey, sessionToken []byte) (s secrets, err error) { // we are listening connection. we are responders in the // handshake. Extract info from the authentication. The initiator // starts by sending us a handshake that we need to respond to. so // we read auth message first, then respond. auth := make([]byte, iHSLen) if _, err := io.ReadFull(conn, auth); err != nil { - return nil, nil, err + return s, err } response, recNonce, initNonce, remotePubKey, randomPrivKey, remoteRandomPubKey, err := authResp(auth, sessionToken, prvKey) if err != nil { - return nil, nil, err + return s, err } if _, err = conn.Write(response); err != nil { - return nil, nil, err + return s, err } - token, err = newSession(initNonce, recNonce, randomPrivKey, remoteRandomPubKey) - return token, remotePubKey, err + + h := &encHandshake{ + initiator: false, + initNonce: initNonce, + respNonce: recNonce, + randomPrivKey: randomPrivKey, + remoteRandomPub: remoteRandomPubKey, + } + copy(h.remoteID[:], remotePubKey) + return h.secrets(auth, response), err } // authResp is called by peer if it accepted (but not @@ -349,23 +409,6 @@ func authResp(auth, sessionToken []byte, prvKey *ecdsa.PrivateKey) ( return } -// newSession is called after the handshake is completed. The -// arguments are values negotiated in the handshake. The return value -// is a new session Token to be remembered for the next time we -// connect with this peer. -func newSession(initNonce, respNonce []byte, privKey *ecdsa.PrivateKey, remoteRandomPubKey *ecdsa.PublicKey) ([]byte, error) { - // 3) Now we can trust ecdhe-random-pubk to derive new keys - //ecdhe-shared-secret = ecdh.agree(ecdhe-random, remote-ecdhe-random-pubk) - pubKey := ecies.ImportECDSAPublic(remoteRandomPubKey) - dhSharedSecret, err := ecies.ImportECDSA(privKey).GenerateShared(pubKey, sskLen, sskLen) - if err != nil { - return nil, err - } - sharedSecret := crypto.Sha3(dhSharedSecret, crypto.Sha3(respNonce, initNonce)) - sessionToken := crypto.Sha3(sharedSecret) - return sessionToken, nil -} - // importPublicKey unmarshals 512 bit public keys. func importPublicKey(pubKey []byte) (pubKeyEC *ecdsa.PublicKey, err error) { var pubKey65 []byte diff --git a/p2p/handshake_test.go b/p2p/handshake_test.go index 06c6a6932..66e610d17 100644 --- a/p2p/handshake_test.go +++ b/p2p/handshake_test.go @@ -2,8 +2,6 @@ package p2p import ( "bytes" - "crypto/ecdsa" - "crypto/rand" "net" "reflect" "testing" @@ -69,102 +67,46 @@ func TestSharedSecret(t *testing.T) { } } -func TestCryptoHandshake(t *testing.T) { - testCryptoHandshake(newkey(), newkey(), nil, t) -} - -func TestCryptoHandshakeWithToken(t *testing.T) { - sessionToken := make([]byte, shaLen) - rand.Read(sessionToken) - testCryptoHandshake(newkey(), newkey(), sessionToken, t) -} - -func testCryptoHandshake(prv0, prv1 *ecdsa.PrivateKey, sessionToken []byte, t *testing.T) { - var err error - // pub0 := &prv0.PublicKey - pub1 := &prv1.PublicKey - - // pub0s := crypto.FromECDSAPub(pub0) - pub1s := crypto.FromECDSAPub(pub1) - - // simulate handshake by feeding output to input - // initiator sends handshake 'auth' - auth, initNonce, randomPrivKey, err := authMsg(prv0, pub1s, sessionToken) - if err != nil { - t.Errorf("%v", err) - } - // t.Logf("-> %v", hexkey(auth)) - - // receiver reads auth and responds with response - response, remoteRecNonce, remoteInitNonce, _, remoteRandomPrivKey, remoteInitRandomPubKey, err := authResp(auth, sessionToken, prv1) - if err != nil { - t.Errorf("%v", err) - } - // t.Logf("<- %v\n", hexkey(response)) - - // initiator reads receiver's response and the key exchange completes - recNonce, remoteRandomPubKey, _, err := completeHandshake(response, prv0) - if err != nil { - t.Errorf("completeHandshake error: %v", err) - } - - // now both parties should have the same session parameters - initSessionToken, err := newSession(initNonce, recNonce, randomPrivKey, remoteRandomPubKey) - if err != nil { - t.Errorf("newSession error: %v", err) - } - - recSessionToken, err := newSession(remoteInitNonce, remoteRecNonce, remoteRandomPrivKey, remoteInitRandomPubKey) - if err != nil { - t.Errorf("newSession error: %v", err) - } - - // fmt.Printf("\nauth (%v) %x\n\nresp (%v) %x\n\n", len(auth), auth, len(response), response) - - // fmt.Printf("\nauth %x\ninitNonce %x\nresponse%x\nremoteRecNonce %x\nremoteInitNonce %x\nremoteRandomPubKey %x\nrecNonce %x\nremoteInitRandomPubKey %x\ninitSessionToken %x\n\n", auth, initNonce, response, remoteRecNonce, remoteInitNonce, remoteRandomPubKey, recNonce, remoteInitRandomPubKey, initSessionToken) - - if !bytes.Equal(initNonce, remoteInitNonce) { - t.Errorf("nonces do not match") - } - if !bytes.Equal(recNonce, remoteRecNonce) { - t.Errorf("receiver nonces do not match") - } - if !bytes.Equal(initSessionToken, recSessionToken) { - t.Errorf("session tokens do not match") - } -} - func TestEncHandshake(t *testing.T) { defer testlog(t).detach() prv0, _ := crypto.GenerateKey() prv1, _ := crypto.GenerateKey() - pub0s, _ := exportPublicKey(&prv0.PublicKey) - pub1s, _ := exportPublicKey(&prv1.PublicKey) rw0, rw1 := net.Pipe() - tokens := make(chan []byte) + secrets := make(chan secrets) go func() { - token, err := outboundEncHandshake(rw0, prv0, pub1s, nil) + pub1s, _ := exportPublicKey(&prv1.PublicKey) + s, err := outboundEncHandshake(rw0, prv0, pub1s, nil) if err != nil { t.Errorf("outbound side error: %v", err) } - tokens <- token + id1 := discover.PubkeyID(&prv1.PublicKey) + if s.RemoteID != id1 { + t.Errorf("outbound side remote ID mismatch") + } + secrets <- s }() go func() { - token, remotePubkey, err := inboundEncHandshake(rw1, prv1, nil) + s, err := inboundEncHandshake(rw1, prv1, nil) if err != nil { t.Errorf("inbound side error: %v", err) } - if !bytes.Equal(remotePubkey, pub0s) { - t.Errorf("inbound side returned wrong remote pubkey\n got: %x\n want: %x", remotePubkey, pub0s) + id0 := discover.PubkeyID(&prv0.PublicKey) + if s.RemoteID != id0 { + t.Errorf("inbound side remote ID mismatch") } - tokens <- token + secrets <- s }() - t1, t2 := <-tokens, <-tokens - if !bytes.Equal(t1, t2) { - t.Error("session token mismatch") + // get computed secrets from both sides + t1, t2 := <-secrets, <-secrets + // don't compare remote node IDs + t1.RemoteID, t2.RemoteID = discover.NodeID{}, discover.NodeID{} + // flip MACs on one of them so they compare equal + t1.EgressMAC, t1.IngressMAC = t1.IngressMAC, t1.EgressMAC + if !reflect.DeepEqual(t1, t2) { + t.Errorf("secrets mismatch:\n t1: %#v\n t2: %#v", t1, t2) } } diff --git a/p2p/rlpx.go b/p2p/rlpx.go index 9fd1aed1f..761dc2ed9 100644 --- a/p2p/rlpx.go +++ b/p2p/rlpx.go @@ -13,24 +13,44 @@ import ( ) var ( + // this is used in place of actual frame header data. + // TODO: replace this when Msg contains the protocol type code. zeroHeader = []byte{0xC2, 0x80, 0x80} - zero16 = make([]byte, 16) + + // sixteen zero bytes + zero16 = make([]byte, 16) ) type rlpxFrameRW struct { conn io.ReadWriter + enc cipher.Stream + dec cipher.Stream macCipher cipher.Block egressMAC hash.Hash ingressMAC hash.Hash } -func newRlpxFrameRW(conn io.ReadWriter, macSecret []byte, egressMAC, ingressMAC hash.Hash) *rlpxFrameRW { - cipher, err := aes.NewCipher(macSecret) +func newRlpxFrameRW(conn io.ReadWriter, s secrets) *rlpxFrameRW { + macc, err := aes.NewCipher(s.MAC) if err != nil { - panic("invalid macSecret: " + err.Error()) + panic("invalid MAC secret: " + err.Error()) + } + encc, err := aes.NewCipher(s.AES) + if err != nil { + panic("invalid AES secret: " + err.Error()) + } + // we use an all-zeroes IV for AES because the key used + // for encryption is ephemeral. + iv := make([]byte, encc.BlockSize()) + return &rlpxFrameRW{ + conn: conn, + enc: cipher.NewCTR(encc, iv), + dec: cipher.NewCTR(encc, iv), + macCipher: macc, + egressMAC: s.EgressMAC, + ingressMAC: s.IngressMAC, } - return &rlpxFrameRW{conn: conn, macCipher: cipher, egressMAC: egressMAC, ingressMAC: ingressMAC} } func (rw *rlpxFrameRW) WriteMsg(msg Msg) error { @@ -41,13 +61,14 @@ func (rw *rlpxFrameRW) WriteMsg(msg Msg) error { fsize := uint32(len(ptype)) + msg.Size putInt24(fsize, headbuf) // TODO: check overflow copy(headbuf[3:], zeroHeader) + rw.enc.XORKeyStream(headbuf[:16], headbuf[:16]) // first half is now encrypted copy(headbuf[16:], updateHeaderMAC(rw.egressMAC, rw.macCipher, headbuf[:16])) if _, err := rw.conn.Write(headbuf); err != nil { return err } - // write frame, updating the egress MAC while writing to conn. - tee := io.MultiWriter(rw.conn, rw.egressMAC) + // write encrypted frame, updating the egress MAC while writing to conn. + tee := cipher.StreamWriter{S: rw.enc, W: io.MultiWriter(rw.conn, rw.egressMAC)} if _, err := tee.Write(ptype); err != nil { return err } @@ -62,7 +83,8 @@ func (rw *rlpxFrameRW) WriteMsg(msg Msg) error { // write packet-mac. egress MAC is up to date because // frame content was written to it as well. - _, err := rw.conn.Write(rw.egressMAC.Sum(nil)) + mac := updateHeaderMAC(rw.egressMAC, rw.macCipher, rw.egressMAC.Sum(nil)) + _, err := rw.conn.Write(mac) return err } @@ -72,34 +94,40 @@ func (rw *rlpxFrameRW) ReadMsg() (msg Msg, err error) { if _, err := io.ReadFull(rw.conn, headbuf); err != nil { return msg, err } - fsize := readInt24(headbuf) - // ignore protocol type for now + // verify header mac shouldMAC := updateHeaderMAC(rw.ingressMAC, rw.macCipher, headbuf[:16]) if !hmac.Equal(shouldMAC[:16], headbuf[16:]) { return msg, errors.New("bad header MAC") } + rw.dec.XORKeyStream(headbuf[:16], headbuf[:16]) // first half is now decrypted + fsize := readInt24(headbuf) + // ignore protocol type for now // read the frame content - framebuf := make([]byte, fsize) + var rsize = fsize // frame size rounded up to 16 byte boundary + if padding := fsize % 16; padding > 0 { + rsize += 16 - padding + } + framebuf := make([]byte, rsize) if _, err := io.ReadFull(rw.conn, framebuf); err != nil { return msg, err } - rw.ingressMAC.Write(framebuf) - if padding := fsize % 16; padding > 0 { - if _, err := io.CopyN(rw.ingressMAC, rw.conn, int64(16-padding)); err != nil { - return msg, err - } - } + // read and validate frame MAC. we can re-use headbuf for that. + rw.ingressMAC.Write(framebuf) if _, err := io.ReadFull(rw.conn, headbuf); err != nil { return msg, err } - if !hmac.Equal(rw.ingressMAC.Sum(nil), headbuf) { + shouldMAC = updateHeaderMAC(rw.ingressMAC, rw.macCipher, rw.ingressMAC.Sum(nil)) + if !hmac.Equal(shouldMAC, headbuf) { return msg, errors.New("bad frame MAC") } + // decrypt frame content + rw.dec.XORKeyStream(framebuf, framebuf) + // decode message code - content := bytes.NewReader(framebuf) + content := bytes.NewReader(framebuf[:fsize]) if err := rlp.Decode(content, &msg.Code); err != nil { return msg, err } diff --git a/p2p/rlpx_test.go b/p2p/rlpx_test.go index 380d9aba6..b3c2adf8d 100644 --- a/p2p/rlpx_test.go +++ b/p2p/rlpx_test.go @@ -16,14 +16,18 @@ import ( func TestRlpxFrameFake(t *testing.T) { buf := new(bytes.Buffer) - secret := crypto.Sha3() hash := fakeHash([]byte{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}) - rw := newRlpxFrameRW(buf, secret, hash, hash) + rw := newRlpxFrameRW(buf, secrets{ + AES: crypto.Sha3(), + MAC: crypto.Sha3(), + IngressMAC: hash, + EgressMAC: hash, + }) golden := unhex(` -000006C2808000000000000000000000 +00828ddae471818bb0bfa6b551d1cb42 01010101010101010101010101010101 -08C40102030400000000000000000000 +ba628a4ba590cb43f7848f41c4382885 01010101010101010101010101010101 01010101010101010101010101010101 `) @@ -75,27 +79,35 @@ func unhex(str string) []byte { func TestRlpxFrameRW(t *testing.T) { var ( + aesSecret = make([]byte, 16) macSecret = make([]byte, 16) egressMACinit = make([]byte, 32) ingressMACinit = make([]byte, 32) ) - for _, s := range [][]byte{macSecret, egressMACinit, ingressMACinit} { + for _, s := range [][]byte{aesSecret, macSecret, egressMACinit, ingressMACinit} { rand.Read(s) } - conn := new(bytes.Buffer) - em1 := sha3.NewKeccak256() - em1.Write(egressMACinit) - im1 := sha3.NewKeccak256() - im1.Write(ingressMACinit) - rw1 := newRlpxFrameRW(conn, macSecret, em1, im1) + s1 := secrets{ + AES: aesSecret, + MAC: macSecret, + EgressMAC: sha3.NewKeccak256(), + IngressMAC: sha3.NewKeccak256(), + } + s1.EgressMAC.Write(egressMACinit) + s1.IngressMAC.Write(ingressMACinit) + rw1 := newRlpxFrameRW(conn, s1) - em2 := sha3.NewKeccak256() - em2.Write(ingressMACinit) - im2 := sha3.NewKeccak256() - im2.Write(egressMACinit) - rw2 := newRlpxFrameRW(conn, macSecret, em2, im2) + s2 := secrets{ + AES: aesSecret, + MAC: macSecret, + EgressMAC: sha3.NewKeccak256(), + IngressMAC: sha3.NewKeccak256(), + } + s2.EgressMAC.Write(ingressMACinit) + s2.IngressMAC.Write(egressMACinit) + rw2 := newRlpxFrameRW(conn, s2) // send some messages for i := 0; i < 10; i++ { From 736e632215d49dd7bc61126f78dda4bad12768ea Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 27 Feb 2015 03:06:55 +0000 Subject: [PATCH 03/13] p2p: use RLPx frames for messaging --- p2p/handshake.go | 27 ++++++++++----------------- p2p/message.go | 19 +++++++++++++++++++ p2p/peer.go | 21 ++++++++++++--------- p2p/peer_test.go | 36 +++++++++++++++++++----------------- p2p/server.go | 7 ++++--- p2p/server_test.go | 13 +++++++++---- 6 files changed, 73 insertions(+), 50 deletions(-) diff --git a/p2p/handshake.go b/p2p/handshake.go index 17f572dea..10ef970dc 100644 --- a/p2p/handshake.go +++ b/p2p/handshake.go @@ -32,14 +32,10 @@ const ( ) type conn struct { - *frameRW + MsgReadWriter *protoHandshake } -func newConn(fd net.Conn, hs *protoHandshake) *conn { - return &conn{newFrameRW(fd, msgWriteTimeout), hs} -} - // encHandshake contains the state of the encryption handshake. type encHandshake struct { remoteID discover.NodeID @@ -115,17 +111,16 @@ func setupInboundConn(fd net.Conn, prv *ecdsa.PrivateKey, our *protoHandshake) ( // Run the protocol handshake using authenticated messages. // TODO: move buffering setup here (out of newFrameRW) - phsrw := newRlpxFrameRW(fd, secrets) - rhs, err := readProtocolHandshake(phsrw, our) + rw := newRlpxFrameRW(fd, secrets) + rhs, err := readProtocolHandshake(rw, our) if err != nil { return nil, err } - if err := writeProtocolHandshake(phsrw, our); err != nil { + // TODO: validate that handshake node ID matches + if err := writeProtocolHandshake(rw, our); err != nil { return nil, fmt.Errorf("protocol write error: %v", err) } - - rw := newFrameRW(fd, msgWriteTimeout) - return &conn{rw, rhs}, nil + return &conn{&lockedRW{wrapped: rw}, rhs}, nil } func setupOutboundConn(fd net.Conn, prv *ecdsa.PrivateKey, our *protoHandshake, dial *discover.Node) (*conn, error) { @@ -136,20 +131,18 @@ func setupOutboundConn(fd net.Conn, prv *ecdsa.PrivateKey, our *protoHandshake, // Run the protocol handshake using authenticated messages. // TODO: move buffering setup here (out of newFrameRW) - phsrw := newRlpxFrameRW(fd, secrets) - if err := writeProtocolHandshake(phsrw, our); err != nil { + rw := newRlpxFrameRW(fd, secrets) + if err := writeProtocolHandshake(rw, our); err != nil { return nil, fmt.Errorf("protocol write error: %v", err) } - rhs, err := readProtocolHandshake(phsrw, our) + rhs, err := readProtocolHandshake(rw, our) if err != nil { return nil, fmt.Errorf("protocol handshake read error: %v", err) } if rhs.ID != dial.ID { return nil, errors.New("dialed node id mismatch") } - - rw := newFrameRW(fd, msgWriteTimeout) - return &conn{rw, rhs}, nil + return &conn{&lockedRW{wrapped: rw}, rhs}, nil } // outboundEncHandshake negotiates a session token on conn. diff --git a/p2p/message.go b/p2p/message.go index 7adad4b09..d61faad13 100644 --- a/p2p/message.go +++ b/p2p/message.go @@ -119,6 +119,25 @@ func EncodeMsg(w MsgWriter, code uint64, data ...interface{}) error { return w.WriteMsg(NewMsg(code, data...)) } +// lockedRW wraps a MsgReadWriter with locks around +// ReadMsg and WriteMsg. +type lockedRW struct { + rmu, wmu sync.Mutex + wrapped MsgReadWriter +} + +func (rw *lockedRW) ReadMsg() (Msg, error) { + rw.rmu.Lock() + defer rw.rmu.Unlock() + return rw.wrapped.ReadMsg() +} + +func (rw *lockedRW) WriteMsg(msg Msg) error { + rw.wmu.Lock() + defer rw.wmu.Unlock() + return rw.wrapped.WriteMsg(msg) +} + // frameRW is a MsgReadWriter that reads and writes devp2p message frames. // As required by the interface, ReadMsg and WriteMsg can be called from // multiple goroutines. diff --git a/p2p/peer.go b/p2p/peer.go index fb027c834..4982c4612 100644 --- a/p2p/peer.go +++ b/p2p/peer.go @@ -40,6 +40,7 @@ type Peer struct { // Use them to display messages related to the peer. *logger.Logger + conn net.Conn rw *conn running map[string]*protoRW @@ -52,8 +53,9 @@ type Peer struct { // NewPeer returns a peer for testing purposes. func NewPeer(id discover.NodeID, name string, caps []Cap) *Peer { pipe, _ := net.Pipe() - conn := newConn(pipe, &protoHandshake{ID: id, Name: name, Caps: caps}) - peer := newPeer(conn, nil) + msgpipe, _ := MsgPipe() + conn := &conn{msgpipe, &protoHandshake{ID: id, Name: name, Caps: caps}} + peer := newPeer(pipe, conn, nil) close(peer.closed) // ensures Disconnect doesn't block return peer } @@ -76,12 +78,12 @@ func (p *Peer) Caps() []Cap { // RemoteAddr returns the remote address of the network connection. func (p *Peer) RemoteAddr() net.Addr { - return p.rw.RemoteAddr() + return p.conn.RemoteAddr() } // LocalAddr returns the local address of the network connection. func (p *Peer) LocalAddr() net.Addr { - return p.rw.LocalAddr() + return p.conn.LocalAddr() } // Disconnect terminates the peer connection with the given reason. @@ -98,10 +100,11 @@ func (p *Peer) String() string { return fmt.Sprintf("Peer %.8x %v", p.rw.ID[:], p.RemoteAddr()) } -func newPeer(conn *conn, protocols []Protocol) *Peer { - logtag := fmt.Sprintf("Peer %.8x %v", conn.ID[:], conn.RemoteAddr()) +func newPeer(fd net.Conn, conn *conn, protocols []Protocol) *Peer { + logtag := fmt.Sprintf("Peer %.8x %v", conn.ID[:], fd.RemoteAddr()) p := &Peer{ Logger: logger.NewLogger(logtag), + conn: fd, rw: conn, running: matchProtocols(protocols, conn.Caps, conn), disc: make(chan DiscReason), @@ -138,7 +141,7 @@ loop: // We rely on protocols to abort if there is a write error. It // might be more robust to handle them here as well. p.DebugDetailf("Read error: %v\n", err) - p.rw.Close() + p.conn.Close() return DiscNetworkError case err := <-p.protoErr: reason = discReasonForError(err) @@ -161,14 +164,14 @@ func (p *Peer) politeDisconnect(reason DiscReason) { EncodeMsg(p.rw, discMsg, uint(reason)) // Wait for the other side to close the connection. // Discard any data that they send until then. - io.Copy(ioutil.Discard, p.rw) + io.Copy(ioutil.Discard, p.conn) close(done) }() select { case <-done: case <-time.After(disconnectGracePeriod): } - p.rw.Close() + p.conn.Close() } func (p *Peer) readLoop() error { diff --git a/p2p/peer_test.go b/p2p/peer_test.go index a1260adbd..1ba43bed5 100644 --- a/p2p/peer_test.go +++ b/p2p/peer_test.go @@ -3,6 +3,7 @@ package p2p import ( "bytes" "fmt" + "io" "io/ioutil" "net" "reflect" @@ -29,8 +30,8 @@ var discard = Protocol{ }, } -func testPeer(protos []Protocol) (*conn, *Peer, <-chan DiscReason) { - fd1, fd2 := net.Pipe() +func testPeer(protos []Protocol) (io.Closer, *conn, *Peer, <-chan DiscReason) { + fd1, _ := net.Pipe() hs1 := &protoHandshake{ID: randomID(), Version: baseProtocolVersion} hs2 := &protoHandshake{ID: randomID(), Version: baseProtocolVersion} for _, p := range protos { @@ -38,11 +39,12 @@ func testPeer(protos []Protocol) (*conn, *Peer, <-chan DiscReason) { hs2.Caps = append(hs2.Caps, p.cap()) } - peer := newPeer(newConn(fd1, hs1), protos) + p1, p2 := MsgPipe() + peer := newPeer(fd1, &conn{p1, hs1}, protos) errc := make(chan DiscReason, 1) go func() { errc <- peer.run() }() - return newConn(fd2, hs2), peer, errc + return p1, &conn{p2, hs2}, peer, errc } func TestPeerProtoReadMsg(t *testing.T) { @@ -67,8 +69,8 @@ func TestPeerProtoReadMsg(t *testing.T) { }, } - rw, _, errc := testPeer([]Protocol{proto}) - defer rw.Close() + closer, rw, _, errc := testPeer([]Protocol{proto}) + defer closer.Close() EncodeMsg(rw, baseProtocolLength+2, 1) EncodeMsg(rw, baseProtocolLength+3, 2) @@ -105,8 +107,8 @@ func TestPeerProtoReadLargeMsg(t *testing.T) { }, } - rw, _, errc := testPeer([]Protocol{proto}) - defer rw.Close() + closer, rw, _, errc := testPeer([]Protocol{proto}) + defer closer.Close() EncodeMsg(rw, 18, make([]byte, msgsize)) select { @@ -134,8 +136,8 @@ func TestPeerProtoEncodeMsg(t *testing.T) { return nil }, } - rw, _, _ := testPeer([]Protocol{proto}) - defer rw.Close() + closer, rw, _, _ := testPeer([]Protocol{proto}) + defer closer.Close() if err := expectMsg(rw, 17, []string{"foo", "bar"}); err != nil { t.Error(err) @@ -145,8 +147,8 @@ func TestPeerProtoEncodeMsg(t *testing.T) { func TestPeerWriteForBroadcast(t *testing.T) { defer testlog(t).detach() - rw, peer, peerErr := testPeer([]Protocol{discard}) - defer rw.Close() + closer, rw, peer, peerErr := testPeer([]Protocol{discard}) + defer closer.Close() // test write errors if err := peer.writeProtoMsg("b", NewMsg(3)); err == nil { @@ -181,8 +183,8 @@ func TestPeerWriteForBroadcast(t *testing.T) { func TestPeerPing(t *testing.T) { defer testlog(t).detach() - rw, _, _ := testPeer(nil) - defer rw.Close() + closer, rw, _, _ := testPeer(nil) + defer closer.Close() if err := EncodeMsg(rw, pingMsg); err != nil { t.Fatal(err) } @@ -194,15 +196,15 @@ func TestPeerPing(t *testing.T) { func TestPeerDisconnect(t *testing.T) { defer testlog(t).detach() - rw, _, disc := testPeer(nil) - defer rw.Close() + closer, rw, _, disc := testPeer(nil) + defer closer.Close() if err := EncodeMsg(rw, discMsg, DiscQuitting); err != nil { t.Fatal(err) } if err := expectMsg(rw, discMsg, []interface{}{DiscRequested}); err != nil { t.Error(err) } - rw.Close() // make test end faster + closer.Close() // make test end faster if reason := <-disc; reason != DiscRequested { t.Errorf("run returned wrong reason: got %v, want %v", reason, DiscRequested) } diff --git a/p2p/server.go b/p2p/server.go index 3ea2538d1..e53e832aa 100644 --- a/p2p/server.go +++ b/p2p/server.go @@ -358,14 +358,15 @@ func (srv *Server) findPeers() { func (srv *Server) startPeer(fd net.Conn, dest *discover.Node) { // TODO: handle/store session token - fd.SetDeadline(time.Now().Add(handshakeTimeout)) + // TODO: reenable deadlines + // fd.SetDeadline(time.Now().Add(handshakeTimeout)) conn, err := srv.setupFunc(fd, srv.PrivateKey, srv.ourHandshake, dest) if err != nil { fd.Close() srvlog.Debugf("Handshake with %v failed: %v", fd.RemoteAddr(), err) return } - p := newPeer(conn, srv.Protocols) + p := newPeer(fd, conn, srv.Protocols) if ok, reason := srv.addPeer(conn.ID, p); !ok { srvlog.DebugDetailf("Not adding %v (%v)\n", p, reason) p.politeDisconnect(reason) @@ -375,7 +376,7 @@ func (srv *Server) startPeer(fd net.Conn, dest *discover.Node) { srvlog.Debugf("Added %v\n", p) srvjslog.LogJson(&logger.P2PConnected{ RemoteId: fmt.Sprintf("%x", conn.ID[:]), - RemoteAddress: conn.RemoteAddr().String(), + RemoteAddress: fd.RemoteAddr().String(), RemoteVersionString: conn.Name, NumConnections: srv.PeerCount(), }) diff --git a/p2p/server_test.go b/p2p/server_test.go index c109fffb9..c348f5a9a 100644 --- a/p2p/server_test.go +++ b/p2p/server_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/crypto/sha3" "github.com/ethereum/go-ethereum/p2p/discover" ) @@ -23,8 +24,14 @@ func startTestServer(t *testing.T, pf newPeerHook) *Server { newPeerHook: pf, setupFunc: func(fd net.Conn, prv *ecdsa.PrivateKey, our *protoHandshake, dial *discover.Node) (*conn, error) { id := randomID() + rw := newRlpxFrameRW(fd, secrets{ + MAC: zero16, + AES: zero16, + IngressMAC: sha3.NewKeccak256(), + EgressMAC: sha3.NewKeccak256(), + }) return &conn{ - frameRW: newFrameRW(fd, msgWriteTimeout), + MsgReadWriter: rw, protoHandshake: &protoHandshake{ID: id, Version: baseProtocolVersion}, }, nil }, @@ -143,9 +150,7 @@ func TestServerBroadcast(t *testing.T) { // broadcast one message srv.Broadcast("discard", 0, "foo") - goldbuf := new(bytes.Buffer) - writeMsg(goldbuf, NewMsg(16, "foo")) - golden := goldbuf.Bytes() + golden := unhex("66e94e166f0a2c3b884cfa59ca34") // check that the message has been written everywhere for i, conn := range conns { From d084aed5e9df5d06812332ed03d3ea55e3ddf819 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 27 Feb 2015 03:09:25 +0000 Subject: [PATCH 04/13] p2p: delete frameRW --- p2p/message.go | 138 -------------------------------------------- p2p/message_test.go | 46 --------------- 2 files changed, 184 deletions(-) diff --git a/p2p/message.go b/p2p/message.go index d61faad13..2ef84f99d 100644 --- a/p2p/message.go +++ b/p2p/message.go @@ -1,15 +1,11 @@ package p2p import ( - "bufio" "bytes" - "encoding/binary" "errors" "fmt" "io" "io/ioutil" - "math/big" - "net" "sync" "sync/atomic" "time" @@ -138,140 +134,6 @@ func (rw *lockedRW) WriteMsg(msg Msg) error { return rw.wrapped.WriteMsg(msg) } -// frameRW is a MsgReadWriter that reads and writes devp2p message frames. -// As required by the interface, ReadMsg and WriteMsg can be called from -// multiple goroutines. -type frameRW struct { - net.Conn // make Conn methods available. be careful. - bufconn *bufio.ReadWriter - - // this channel is used to 'lend' bufconn to a caller of ReadMsg - // until the message payload has been consumed. the channel - // receives a value when EOF is reached on the payload, unblocking - // a pending call to ReadMsg. - rsync chan struct{} - - // this mutex guards writes to bufconn. - writeMu sync.Mutex -} - -func newFrameRW(conn net.Conn, timeout time.Duration) *frameRW { - rsync := make(chan struct{}, 1) - rsync <- struct{}{} - return &frameRW{ - Conn: conn, - bufconn: bufio.NewReadWriter(bufio.NewReader(conn), bufio.NewWriter(conn)), - rsync: rsync, - } -} - -var magicToken = []byte{34, 64, 8, 145} - -func (rw *frameRW) WriteMsg(msg Msg) error { - rw.writeMu.Lock() - defer rw.writeMu.Unlock() - rw.SetWriteDeadline(time.Now().Add(msgWriteTimeout)) - if err := writeMsg(rw.bufconn, msg); err != nil { - return err - } - return rw.bufconn.Flush() -} - -func writeMsg(w io.Writer, msg Msg) error { - // TODO: handle case when Size + len(code) + len(listhdr) overflows uint32 - code := ethutil.Encode(uint32(msg.Code)) - listhdr := makeListHeader(msg.Size + uint32(len(code))) - payloadLen := uint32(len(listhdr)) + uint32(len(code)) + msg.Size - - start := make([]byte, 8) - copy(start, magicToken) - binary.BigEndian.PutUint32(start[4:], payloadLen) - - for _, b := range [][]byte{start, listhdr, code} { - if _, err := w.Write(b); err != nil { - return err - } - } - _, err := io.CopyN(w, msg.Payload, int64(msg.Size)) - return err -} - -func makeListHeader(length uint32) []byte { - if length < 56 { - return []byte{byte(length + 0xc0)} - } - enc := big.NewInt(int64(length)).Bytes() - lenb := byte(len(enc)) + 0xf7 - return append([]byte{lenb}, enc...) -} - -func (rw *frameRW) ReadMsg() (msg Msg, err error) { - <-rw.rsync // wait until bufconn is ours - - rw.SetReadDeadline(time.Now().Add(frameReadTimeout)) - - // read magic and payload size - start := make([]byte, 8) - if _, err = io.ReadFull(rw.bufconn, start); err != nil { - return msg, err - } - if !bytes.HasPrefix(start, magicToken) { - return msg, fmt.Errorf("bad magic token %x", start[:4]) - } - size := binary.BigEndian.Uint32(start[4:]) - - // decode start of RLP message to get the message code - posr := &postrack{rw.bufconn, 0} - s := rlp.NewStream(posr) - if _, err := s.List(); err != nil { - return msg, err - } - msg.Code, err = s.Uint() - if err != nil { - return msg, err - } - msg.Size = size - posr.p - - rw.SetReadDeadline(time.Now().Add(payloadReadTimeout)) - - if msg.Size <= wholePayloadSize { - // msg is small, read all of it and move on to the next message. - pbuf := make([]byte, msg.Size) - if _, err := io.ReadFull(rw.bufconn, pbuf); err != nil { - return msg, err - } - rw.rsync <- struct{}{} // bufconn is available again - msg.Payload = bytes.NewReader(pbuf) - } else { - // lend bufconn to the caller until it has - // consumed the payload. eofSignal will send a value - // on rw.rsync when EOF is reached. - pr := &eofSignal{rw.bufconn, msg.Size, rw.rsync} - msg.Payload = pr - } - return msg, nil -} - -// postrack wraps an rlp.ByteReader with a position counter. -type postrack struct { - r rlp.ByteReader - p uint32 -} - -func (r *postrack) Read(buf []byte) (int, error) { - n, err := r.r.Read(buf) - r.p += uint32(n) - return n, err -} - -func (r *postrack) ReadByte() (byte, error) { - b, err := r.r.ReadByte() - if err == nil { - r.p++ - } - return b, err -} - // eofSignal wraps a reader with eof signaling. the eof channel is // closed when the wrapped reader returns an error or when count bytes // have been read. diff --git a/p2p/message_test.go b/p2p/message_test.go index 4b94ebb5f..1757cbe7a 100644 --- a/p2p/message_test.go +++ b/p2p/message_test.go @@ -25,52 +25,6 @@ func TestNewMsg(t *testing.T) { } } -// func TestEncodeDecodeMsg(t *testing.T) { -// msg := NewMsg(3, 1, "000") -// buf := new(bytes.Buffer) -// if err := writeMsg(buf, msg); err != nil { -// t.Fatalf("encodeMsg error: %v", err) -// } -// // t.Logf("encoded: %x", buf.Bytes()) - -// decmsg, err := readMsg(buf) -// if err != nil { -// t.Fatalf("readMsg error: %v", err) -// } -// if decmsg.Code != 3 { -// t.Errorf("incorrect code %d, want %d", decmsg.Code, 3) -// } -// if decmsg.Size != 5 { -// t.Errorf("incorrect size %d, want %d", decmsg.Size, 5) -// } - -// var data struct { -// I uint -// S string -// } -// if err := decmsg.Decode(&data); err != nil { -// t.Fatalf("Decode error: %v", err) -// } -// if data.I != 1 { -// t.Errorf("incorrect data.I: got %v, expected %d", data.I, 1) -// } -// if data.S != "000" { -// t.Errorf("incorrect data.S: got %q, expected %q", data.S, "000") -// } -// } - -// func TestDecodeRealMsg(t *testing.T) { -// data := ethutil.Hex2Bytes("2240089100000080f87e8002b5457468657265756d282b2b292f5065657220536572766572204f6e652f76302e372e382f52656c656173652f4c696e75782f672b2bc082765fb84086dd80b7aefd6a6d2e3b93f4f300a86bfb6ef7bdc97cb03f793db6bb") -// msg, err := readMsg(bytes.NewReader(data)) -// if err != nil { -// t.Fatalf("unexpected error: %v", err) -// } - -// if msg.Code != 0 { -// t.Errorf("incorrect code %d, want %d", msg.Code, 0) -// } -// } - func ExampleMsgPipe() { rw1, rw2 := MsgPipe() go func() { From d344054e5a2844241bf0e4f64ccfc4d2ad259718 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 27 Feb 2015 14:08:57 +0100 Subject: [PATCH 05/13] p2p: make RLPx frame MAC 16 bytes as defined in the spec --- p2p/rlpx.go | 31 +++++++++++++++++++------------ p2p/rlpx_test.go | 1 - 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/p2p/rlpx.go b/p2p/rlpx.go index 761dc2ed9..a041bb314 100644 --- a/p2p/rlpx.go +++ b/p2p/rlpx.go @@ -62,12 +62,15 @@ func (rw *rlpxFrameRW) WriteMsg(msg Msg) error { putInt24(fsize, headbuf) // TODO: check overflow copy(headbuf[3:], zeroHeader) rw.enc.XORKeyStream(headbuf[:16], headbuf[:16]) // first half is now encrypted - copy(headbuf[16:], updateHeaderMAC(rw.egressMAC, rw.macCipher, headbuf[:16])) + + // write header MAC + copy(headbuf[16:], updateMAC(rw.egressMAC, rw.macCipher, headbuf[:16])) if _, err := rw.conn.Write(headbuf); err != nil { return err } - // write encrypted frame, updating the egress MAC while writing to conn. + // write encrypted frame, updating the egress MAC hash with + // the data written to conn. tee := cipher.StreamWriter{S: rw.enc, W: io.MultiWriter(rw.conn, rw.egressMAC)} if _, err := tee.Write(ptype); err != nil { return err @@ -81,9 +84,10 @@ func (rw *rlpxFrameRW) WriteMsg(msg Msg) error { } } - // write packet-mac. egress MAC is up to date because + // write frame MAC. egress MAC hash is up to date because // frame content was written to it as well. - mac := updateHeaderMAC(rw.egressMAC, rw.macCipher, rw.egressMAC.Sum(nil)) + fmacseed := rw.egressMAC.Sum(nil) + mac := updateMAC(rw.egressMAC, rw.macCipher, fmacseed) _, err := rw.conn.Write(mac) return err } @@ -95,8 +99,8 @@ func (rw *rlpxFrameRW) ReadMsg() (msg Msg, err error) { return msg, err } // verify header mac - shouldMAC := updateHeaderMAC(rw.ingressMAC, rw.macCipher, headbuf[:16]) - if !hmac.Equal(shouldMAC[:16], headbuf[16:]) { + shouldMAC := updateMAC(rw.ingressMAC, rw.macCipher, headbuf[:16]) + if !hmac.Equal(shouldMAC, headbuf[16:]) { return msg, errors.New("bad header MAC") } rw.dec.XORKeyStream(headbuf[:16], headbuf[:16]) // first half is now decrypted @@ -115,11 +119,12 @@ func (rw *rlpxFrameRW) ReadMsg() (msg Msg, err error) { // read and validate frame MAC. we can re-use headbuf for that. rw.ingressMAC.Write(framebuf) - if _, err := io.ReadFull(rw.conn, headbuf); err != nil { + fmacseed := rw.ingressMAC.Sum(nil) + if _, err := io.ReadFull(rw.conn, headbuf[:16]); err != nil { return msg, err } - shouldMAC = updateHeaderMAC(rw.ingressMAC, rw.macCipher, rw.ingressMAC.Sum(nil)) - if !hmac.Equal(shouldMAC, headbuf) { + shouldMAC = updateMAC(rw.ingressMAC, rw.macCipher, fmacseed) + if !hmac.Equal(shouldMAC, headbuf[:16]) { return msg, errors.New("bad frame MAC") } @@ -136,14 +141,16 @@ func (rw *rlpxFrameRW) ReadMsg() (msg Msg, err error) { return msg, nil } -func updateHeaderMAC(mac hash.Hash, block cipher.Block, header []byte) []byte { +// updateMAC reseeds the given hash with encrypted seed. +// it returns the first 16 bytes of the hash sum after seeding. +func updateMAC(mac hash.Hash, block cipher.Block, seed []byte) []byte { aesbuf := make([]byte, aes.BlockSize) block.Encrypt(aesbuf, mac.Sum(nil)) for i := range aesbuf { - aesbuf[i] ^= header[i] + aesbuf[i] ^= seed[i] } mac.Write(aesbuf) - return mac.Sum(nil) + return mac.Sum(nil)[:16] } func readInt24(b []byte) uint32 { diff --git a/p2p/rlpx_test.go b/p2p/rlpx_test.go index b3c2adf8d..077dd1309 100644 --- a/p2p/rlpx_test.go +++ b/p2p/rlpx_test.go @@ -29,7 +29,6 @@ func TestRlpxFrameFake(t *testing.T) { 01010101010101010101010101010101 ba628a4ba590cb43f7848f41c4382885 01010101010101010101010101010101 -01010101010101010101010101010101 `) // Check WriteMsg. This puts a message into the buffer. From 2c505efd1ec03dc28ebabbb54253f3eb9e719be5 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Mon, 2 Mar 2015 15:26:24 +0100 Subject: [PATCH 06/13] p2p/discover: add NodeID.Pubkey --- p2p/discover/node.go | 15 +++++++++++++++ p2p/discover/node_test.go | 18 ++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/p2p/discover/node.go b/p2p/discover/node.go index c6d2e9766..de2588258 100644 --- a/p2p/discover/node.go +++ b/p2p/discover/node.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "io" + "math/big" "math/rand" "net" "net/url" @@ -14,6 +15,7 @@ import ( "strings" "time" + "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/crypto/secp256k1" "github.com/ethereum/go-ethereum/rlp" ) @@ -187,6 +189,19 @@ func PubkeyID(pub *ecdsa.PublicKey) NodeID { return id } +// Pubkey returns the public key represented by the node ID. +// It returns an error if the ID is not a point on the curve. +func (id NodeID) Pubkey() (*ecdsa.PublicKey, error) { + p := &ecdsa.PublicKey{Curve: crypto.S256(), X: new(big.Int), Y: new(big.Int)} + half := len(id) / 2 + p.X.SetBytes(id[:half]) + p.Y.SetBytes(id[half:]) + if !p.Curve.IsOnCurve(p.X, p.Y) { + return nil, errors.New("not a point on the S256 curve") + } + return p, nil +} + // recoverNodeID computes the public key used to sign the // given hash from the signature. func recoverNodeID(hash, sig []byte) (id NodeID, err error) { diff --git a/p2p/discover/node_test.go b/p2p/discover/node_test.go index ae82ae4f1..60b01b6ca 100644 --- a/p2p/discover/node_test.go +++ b/p2p/discover/node_test.go @@ -133,6 +133,24 @@ func TestNodeID_recover(t *testing.T) { if pub != recpub { t.Errorf("recovered wrong pubkey:\ngot: %v\nwant: %v", recpub, pub) } + + ecdsa, err := pub.Pubkey() + if err != nil { + t.Errorf("Pubkey error: %v", err) + } + if !reflect.DeepEqual(ecdsa, &prv.PublicKey) { + t.Errorf("Pubkey mismatch:\n got: %#v\n want: %#v", ecdsa, &prv.PublicKey) + } +} + +func TestNodeID_pubkeyBad(t *testing.T) { + ecdsa, err := NodeID{}.Pubkey() + if err == nil { + t.Error("expected error for zero ID") + } + if ecdsa != nil { + t.Error("expected nil result") + } } func TestNodeID_distcmp(t *testing.T) { From 7d39fd66782dee01f9534bed3cbe22c97c8d610f Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Mon, 2 Mar 2015 15:26:44 +0100 Subject: [PATCH 07/13] p2p: make encryption handshake code easier to follow This mostly changes how information is passed around. Instead of using many function parameters and return values, put the entire state in a struct and pass that. This also adds back derivation of ecdhe-shared-secret. I deleted it by accident in a previous refactoring. --- p2p/handshake.go | 461 +++++++++++++++++++----------------------- p2p/handshake_test.go | 129 ++++++------ 2 files changed, 280 insertions(+), 310 deletions(-) diff --git a/p2p/handshake.go b/p2p/handshake.go index 10ef970dc..a56de968d 100644 --- a/p2p/handshake.go +++ b/p2p/handshake.go @@ -2,6 +2,7 @@ package p2p import ( "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" "errors" "fmt" @@ -26,26 +27,26 @@ const ( authMsgLen = sigLen + shaLen + pubLen + shaLen + 1 authRespLen = pubLen + shaLen + 1 - eciesBytes = 65 + 16 + 32 - iHSLen = authMsgLen + eciesBytes // size of the final ECIES payload sent as initiator's handshake - rHSLen = authRespLen + eciesBytes // size of the final ECIES payload sent as receiver's handshake + eciesBytes = 65 + 16 + 32 + encAuthMsgLen = authMsgLen + eciesBytes // size of the final ECIES payload sent as initiator's handshake + encAuthRespLen = authRespLen + eciesBytes // size of the final ECIES payload sent as receiver's handshake ) +// conn represents a remote connection after encryption handshake +// and protocol handshake have completed. +// +// The MsgReadWriter is usually layered as follows: +// +// lockedRW (thread-safety for ReadMsg, WriteMsg) +// rlpxFrameRW (message encoding, encryption, authentication) +// bufio.ReadWriter (buffering) +// net.Conn (network I/O) +// type conn struct { MsgReadWriter *protoHandshake } -// encHandshake contains the state of the encryption handshake. -type encHandshake struct { - remoteID discover.NodeID - initiator bool - initNonce, respNonce []byte - dhSharedSecret []byte - randomPrivKey *ecdsa.PrivateKey - remoteRandomPub *ecdsa.PublicKey -} - // secrets represents the connection secrets // which are negotiated during the encryption handshake. type secrets struct { @@ -64,34 +65,6 @@ type protoHandshake struct { ID discover.NodeID } -// secrets is called after the handshake is completed. -// It extracts the connection secrets from the handshake values. -func (h *encHandshake) secrets(auth, authResp []byte) secrets { - sharedSecret := crypto.Sha3(h.dhSharedSecret, crypto.Sha3(h.respNonce, h.initNonce)) - aesSecret := crypto.Sha3(h.dhSharedSecret, sharedSecret) - s := secrets{ - RemoteID: h.remoteID, - AES: aesSecret, - MAC: crypto.Sha3(h.dhSharedSecret, aesSecret), - Token: crypto.Sha3(sharedSecret), - } - - // setup sha3 instances for the MACs - mac1 := sha3.NewKeccak256() - mac1.Write(xor(s.MAC, h.respNonce)) - mac1.Write(auth) - mac2 := sha3.NewKeccak256() - mac2.Write(xor(s.MAC, h.initNonce)) - mac2.Write(authResp) - if h.initiator { - s.EgressMAC, s.IngressMAC = mac1, mac2 - } else { - s.EgressMAC, s.IngressMAC = mac2, mac1 - } - - return s -} - // setupConn starts a protocol session on the given connection. // It runs the encryption handshake and the protocol handshake. // If dial is non-nil, the connection the local node is the initiator. @@ -104,7 +77,7 @@ func setupConn(fd net.Conn, prv *ecdsa.PrivateKey, our *protoHandshake, dial *di } func setupInboundConn(fd net.Conn, prv *ecdsa.PrivateKey, our *protoHandshake) (*conn, error) { - secrets, err := inboundEncHandshake(fd, prv, nil) + secrets, err := receiverEncHandshake(fd, prv, nil) if err != nil { return nil, fmt.Errorf("encryption handshake failed: %v", err) } @@ -124,7 +97,7 @@ func setupInboundConn(fd net.Conn, prv *ecdsa.PrivateKey, our *protoHandshake) ( } func setupOutboundConn(fd net.Conn, prv *ecdsa.PrivateKey, our *protoHandshake, dial *discover.Node) (*conn, error) { - secrets, err := outboundEncHandshake(fd, prv, dial.ID[:], nil) + secrets, err := initiatorEncHandshake(fd, prv, dial.ID, nil) if err != nil { return nil, fmt.Errorf("encryption handshake failed: %v", err) } @@ -145,14 +118,66 @@ func setupOutboundConn(fd net.Conn, prv *ecdsa.PrivateKey, our *protoHandshake, return &conn{&lockedRW{wrapped: rw}, rhs}, nil } -// outboundEncHandshake negotiates a session token on conn. +// encHandshake contains the state of the encryption handshake. +type encHandshake struct { + initiator bool + remoteID discover.NodeID + + remotePub *ecies.PublicKey // remote-pubk + initNonce, respNonce []byte // nonce + randomPrivKey *ecies.PrivateKey // ecdhe-random + remoteRandomPub *ecies.PublicKey // ecdhe-random-pubk +} + +// secrets is called after the handshake is completed. +// It extracts the connection secrets from the handshake values. +func (h *encHandshake) secrets(auth, authResp []byte) (secrets, error) { + ecdheSecret, err := h.randomPrivKey.GenerateShared(h.remoteRandomPub, sskLen, sskLen) + if err != nil { + return secrets{}, err + } + + // derive base secrets from ephemeral key agreement + sharedSecret := crypto.Sha3(ecdheSecret, crypto.Sha3(h.respNonce, h.initNonce)) + aesSecret := crypto.Sha3(ecdheSecret, sharedSecret) + s := secrets{ + RemoteID: h.remoteID, + AES: aesSecret, + MAC: crypto.Sha3(ecdheSecret, aesSecret), + Token: crypto.Sha3(sharedSecret), + } + + // setup sha3 instances for the MACs + mac1 := sha3.NewKeccak256() + mac1.Write(xor(s.MAC, h.respNonce)) + mac1.Write(auth) + mac2 := sha3.NewKeccak256() + mac2.Write(xor(s.MAC, h.initNonce)) + mac2.Write(authResp) + if h.initiator { + s.EgressMAC, s.IngressMAC = mac1, mac2 + } else { + s.EgressMAC, s.IngressMAC = mac2, mac1 + } + + return s, nil +} + +func (h *encHandshake) ecdhShared(prv *ecdsa.PrivateKey) ([]byte, error) { + return ecies.ImportECDSA(prv).GenerateShared(h.remotePub, sskLen, sskLen) +} + +// initiatorEncHandshake negotiates a session token on conn. // it should be called on the dialing side of the connection. // -// privateKey is the local client's private key -// remotePublicKey is the remote peer's node ID -// sessionToken is the token from a previous session with this node. -func outboundEncHandshake(conn io.ReadWriter, prvKey *ecdsa.PrivateKey, remotePublicKey []byte, sessionToken []byte) (s secrets, err error) { - auth, initNonce, randomPrivKey, err := authMsg(prvKey, remotePublicKey, sessionToken) +// prv is the local client's private key. +// token is the token from a previous session with this node. +func initiatorEncHandshake(conn io.ReadWriter, prv *ecdsa.PrivateKey, remoteID discover.NodeID, token []byte) (s secrets, err error) { + h, err := newInitiatorHandshake(remoteID) + if err != nil { + return s, err + } + auth, err := h.authMsg(prv, token) if err != nil { return s, err } @@ -160,250 +185,189 @@ func outboundEncHandshake(conn io.ReadWriter, prvKey *ecdsa.PrivateKey, remotePu return s, err } - response := make([]byte, rHSLen) + response := make([]byte, encAuthRespLen) if _, err = io.ReadFull(conn, response); err != nil { return s, err } - recNonce, remoteRandomPubKey, _, err := completeHandshake(response, prvKey) - if err != nil { + if err := h.decodeAuthResp(response, prv); err != nil { return s, err } - - h := &encHandshake{ - initiator: true, - initNonce: initNonce, - respNonce: recNonce, - randomPrivKey: randomPrivKey, - remoteRandomPub: remoteRandomPubKey, - } - copy(h.remoteID[:], remotePublicKey) - return h.secrets(auth, response), nil + return h.secrets(auth, response) } -// authMsg creates the initiator handshake. -// TODO: change all the names -func authMsg(prvKey *ecdsa.PrivateKey, remotePubKeyS, sessionToken []byte) ( - auth, initNonce []byte, - randomPrvKey *ecdsa.PrivateKey, - err error, -) { - remotePubKey, err := importPublicKey(remotePubKeyS) - if err != nil { - return +func newInitiatorHandshake(remoteID discover.NodeID) (*encHandshake, error) { + // generate random initiator nonce + n := make([]byte, shaLen) + if _, err := rand.Read(n); err != nil { + return nil, err } + // generate random keypair to use for signing + randpriv, err := ecies.GenerateKey(rand.Reader, crypto.S256(), nil) + if err != nil { + return nil, err + } + rpub, err := remoteID.Pubkey() + if err != nil { + return nil, fmt.Errorf("bad remoteID: %v", err) + } + h := &encHandshake{ + initiator: true, + remoteID: remoteID, + remotePub: ecies.ImportECDSAPublic(rpub), + initNonce: n, + randomPrivKey: randpriv, + } + return h, nil +} +// authMsg creates an encrypted initiator handshake message. +func (h *encHandshake) authMsg(prv *ecdsa.PrivateKey, token []byte) ([]byte, error) { var tokenFlag byte - if sessionToken == nil { + if token == nil { // no session token found means we need to generate shared secret. // ecies shared secret is used as initial session token for new peers // generate shared key from prv and remote pubkey - if sessionToken, err = ecies.ImportECDSA(prvKey).GenerateShared(ecies.ImportECDSAPublic(remotePubKey), sskLen, sskLen); err != nil { - return + var err error + if token, err = h.ecdhShared(prv); err != nil { + return nil, err } } else { // for known peers, we use stored token from the previous session tokenFlag = 0x01 } - //E(remote-pubk, S(ecdhe-random, sha3(ecdh-shared-secret^nonce)) || H(ecdhe-random-pubk) || pubk || nonce || 0x0) - // E(remote-pubk, S(ecdhe-random, sha3(token^nonce)) || H(ecdhe-random-pubk) || pubk || nonce || 0x1) - // allocate msgLen long message, - var msg []byte = make([]byte, authMsgLen) - initNonce = msg[authMsgLen-shaLen-1 : authMsgLen-1] - if _, err = rand.Read(initNonce); err != nil { - return - } - // create known message - // ecdh-shared-secret^nonce for new peers - // token^nonce for old peers - var sharedSecret = xor(sessionToken, initNonce) - - // generate random keypair to use for signing - if randomPrvKey, err = crypto.GenerateKey(); err != nil { - return - } - // sign shared secret (message known to both parties): shared-secret - var signature []byte - // signature = sign(ecdhe-random, shared-secret) - // uses secp256k1.Sign - if signature, err = crypto.Sign(sharedSecret, randomPrvKey); err != nil { - return + // sign known message: + // ecdh-shared-secret^nonce for new peers + // token^nonce for old peers + signed := xor(token, h.initNonce) + signature, err := crypto.Sign(signed, h.randomPrivKey.ExportECDSA()) + if err != nil { + return nil, err } - // message - // signed-shared-secret || H(ecdhe-random-pubk) || pubk || nonce || 0x0 - copy(msg, signature) // copy signed-shared-secret - // H(ecdhe-random-pubk) - var randomPubKey64 []byte - if randomPubKey64, err = exportPublicKey(&randomPrvKey.PublicKey); err != nil { - return - } - var pubKey64 []byte - if pubKey64, err = exportPublicKey(&prvKey.PublicKey); err != nil { - return - } - copy(msg[sigLen:sigLen+shaLen], crypto.Sha3(randomPubKey64)) - // pubkey copied to the correct segment. - copy(msg[sigLen+shaLen:sigLen+shaLen+pubLen], pubKey64) - // nonce is already in the slice - // stick tokenFlag byte to the end - msg[authMsgLen-1] = tokenFlag + // encode auth message + // signature || sha3(ecdhe-random-pubk) || pubk || nonce || token-flag + msg := make([]byte, authMsgLen) + n := copy(msg, signature) + n += copy(msg[n:], crypto.Sha3(exportPubkey(&h.randomPrivKey.PublicKey))) + n += copy(msg[n:], crypto.FromECDSAPub(&prv.PublicKey)[1:]) + n += copy(msg[n:], h.initNonce) + msg[n] = tokenFlag - // encrypt using remote-pubk - // auth = eciesEncrypt(remote-pubk, msg) - if auth, err = crypto.Encrypt(remotePubKey, msg); err != nil { - return - } - return + // encrypt auth message using remote-pubk + return ecies.Encrypt(rand.Reader, h.remotePub, msg, nil, nil) } -// completeHandshake is called when the initiator receives an -// authentication response (aka receiver handshake). It completes the -// handshake by reading off parameters the remote peer provides needed -// to set up the secure session. -func completeHandshake(auth []byte, prvKey *ecdsa.PrivateKey) ( - respNonce []byte, - remoteRandomPubKey *ecdsa.PublicKey, - tokenFlag bool, - err error, -) { - var msg []byte - // they prove that msg is meant for me, - // I prove I possess private key if i can read it - if msg, err = crypto.Decrypt(prvKey, auth); err != nil { - return +// decodeAuthResp decode an encrypted authentication response message. +func (h *encHandshake) decodeAuthResp(auth []byte, prv *ecdsa.PrivateKey) error { + msg, err := crypto.Decrypt(prv, auth) + if err != nil { + return fmt.Errorf("could not decrypt auth response (%v)", err) } - - respNonce = msg[pubLen : pubLen+shaLen] - var remoteRandomPubKeyS = msg[:pubLen] - if remoteRandomPubKey, err = importPublicKey(remoteRandomPubKeyS); err != nil { - return + h.respNonce = msg[pubLen : pubLen+shaLen] + h.remoteRandomPub, err = importPublicKey(msg[:pubLen]) + if err != nil { + return err } - if msg[authRespLen-1] == 0x01 { - tokenFlag = true - } - return + // ignore token flag for now + return nil } -// inboundEncHandshake negotiates a session token on conn. +// receiverEncHandshake negotiates a session token on conn. // it should be called on the listening side of the connection. // -// privateKey is the local client's private key -// sessionToken is the token from a previous session with this node. -func inboundEncHandshake(conn io.ReadWriter, prvKey *ecdsa.PrivateKey, sessionToken []byte) (s secrets, err error) { - // we are listening connection. we are responders in the - // handshake. Extract info from the authentication. The initiator - // starts by sending us a handshake that we need to respond to. so - // we read auth message first, then respond. - auth := make([]byte, iHSLen) +// prv is the local client's private key. +// token is the token from a previous session with this node. +func receiverEncHandshake(conn io.ReadWriter, prv *ecdsa.PrivateKey, token []byte) (s secrets, err error) { + // read remote auth sent by initiator. + auth := make([]byte, encAuthMsgLen) if _, err := io.ReadFull(conn, auth); err != nil { return s, err } - response, recNonce, initNonce, remotePubKey, randomPrivKey, remoteRandomPubKey, err := authResp(auth, sessionToken, prvKey) + h, err := decodeAuthMsg(prv, token, auth) if err != nil { return s, err } - if _, err = conn.Write(response); err != nil { + + // send auth response + resp, err := h.authResp(prv, token) + if err != nil { + return s, err + } + if _, err = conn.Write(resp); err != nil { return s, err } - h := &encHandshake{ - initiator: false, - initNonce: initNonce, - respNonce: recNonce, - randomPrivKey: randomPrivKey, - remoteRandomPub: remoteRandomPubKey, - } - copy(h.remoteID[:], remotePubKey) - return h.secrets(auth, response), err + return h.secrets(auth, resp) } -// authResp is called by peer if it accepted (but not -// initiated) the connection from the remote. It is passed the initiator -// handshake received and the session token belonging to the -// remote initiator. -// -// The first return value is the authentication response (aka receiver -// handshake) that is to be sent to the remote initiator. -func authResp(auth, sessionToken []byte, prvKey *ecdsa.PrivateKey) ( - authResp, respNonce, initNonce, remotePubKeyS []byte, - randomPrivKey *ecdsa.PrivateKey, - remoteRandomPubKey *ecdsa.PublicKey, - err error, -) { - // they prove that msg is meant for me, - // I prove I possess private key if i can read it - msg, err := crypto.Decrypt(prvKey, auth) - if err != nil { - return - } - - remotePubKeyS = msg[sigLen+shaLen : sigLen+shaLen+pubLen] - remotePubKey, _ := importPublicKey(remotePubKeyS) - - var tokenFlag byte - if sessionToken == nil { - // no session token found means we need to generate shared secret. - // ecies shared secret is used as initial session token for new peers - // generate shared key from prv and remote pubkey - if sessionToken, err = ecies.ImportECDSA(prvKey).GenerateShared(ecies.ImportECDSAPublic(remotePubKey), sskLen, sskLen); err != nil { - return - } - // tokenFlag = 0x00 // redundant - } else { - // for known peers, we use stored token from the previous session - tokenFlag = 0x01 - } - - // the initiator nonce is read off the end of the message - initNonce = msg[authMsgLen-shaLen-1 : authMsgLen-1] - // I prove that i own prv key (to derive shared secret, and read - // nonce off encrypted msg) and that I own shared secret they - // prove they own the private key belonging to ecdhe-random-pubk - // we can now reconstruct the signed message and recover the peers - // pubkey - var signedMsg = xor(sessionToken, initNonce) - var remoteRandomPubKeyS []byte - if remoteRandomPubKeyS, err = secp256k1.RecoverPubkey(signedMsg, msg[:sigLen]); err != nil { - return - } - // convert to ECDSA standard - if remoteRandomPubKey, err = importPublicKey(remoteRandomPubKeyS); err != nil { - return - } - - // now we find ourselves a long task too, fill it random - var resp = make([]byte, authRespLen) - // generate shaLen long nonce - respNonce = resp[pubLen : pubLen+shaLen] - if _, err = rand.Read(respNonce); err != nil { - return - } +func decodeAuthMsg(prv *ecdsa.PrivateKey, token []byte, auth []byte) (*encHandshake, error) { + var err error + h := new(encHandshake) // generate random keypair for session - if randomPrivKey, err = crypto.GenerateKey(); err != nil { - return + h.randomPrivKey, err = ecies.GenerateKey(rand.Reader, crypto.S256(), nil) + if err != nil { + return nil, err } + // generate random nonce + h.respNonce = make([]byte, shaLen) + if _, err = rand.Read(h.respNonce); err != nil { + return nil, err + } + + msg, err := crypto.Decrypt(prv, auth) + if err != nil { + return nil, fmt.Errorf("could not decrypt auth message (%v)", err) + } + + // decode message parameters + // signature || sha3(ecdhe-random-pubk) || pubk || nonce || token-flag + h.initNonce = msg[authMsgLen-shaLen-1 : authMsgLen-1] + copy(h.remoteID[:], msg[sigLen+shaLen:sigLen+shaLen+pubLen]) + rpub, err := h.remoteID.Pubkey() + if err != nil { + return nil, fmt.Errorf("bad remoteID: %#v", err) + } + h.remotePub = ecies.ImportECDSAPublic(rpub) + + // recover remote random pubkey from signed message. + if token == nil { + // TODO: it is an error if the initiator has a token and we don't. check that. + + // no session token means we need to generate shared secret. + // ecies shared secret is used as initial session token for new peers. + // generate shared key from prv and remote pubkey. + if token, err = h.ecdhShared(prv); err != nil { + return nil, err + } + } + signedMsg := xor(token, h.initNonce) + remoteRandomPub, err := secp256k1.RecoverPubkey(signedMsg, msg[:sigLen]) + if err != nil { + return nil, err + } + h.remoteRandomPub, _ = importPublicKey(remoteRandomPub) + return h, nil +} + +// authResp generates the encrypted authentication response message. +func (h *encHandshake) authResp(prv *ecdsa.PrivateKey, token []byte) ([]byte, error) { // responder auth message // E(remote-pubk, ecdhe-random-pubk || nonce || 0x0) - var randomPubKeyS []byte - if randomPubKeyS, err = exportPublicKey(&randomPrivKey.PublicKey); err != nil { - return + resp := make([]byte, authRespLen) + n := copy(resp, exportPubkey(&h.randomPrivKey.PublicKey)) + n += copy(resp[n:], h.respNonce) + if token == nil { + resp[n] = 0 + } else { + resp[n] = 1 } - copy(resp[:pubLen], randomPubKeyS) - // nonce is already in the slice - resp[authRespLen-1] = tokenFlag - // encrypt using remote-pubk - // auth = eciesEncrypt(remote-pubk, msg) - // why not encrypt with ecdhe-random-remote - if authResp, err = crypto.Encrypt(remotePubKey, resp); err != nil { - return - } - return + return ecies.Encrypt(rand.Reader, h.remotePub, resp, nil, nil) } // importPublicKey unmarshals 512 bit public keys. -func importPublicKey(pubKey []byte) (pubKeyEC *ecdsa.PublicKey, err error) { +func importPublicKey(pubKey []byte) (*ecies.PublicKey, error) { var pubKey65 []byte switch len(pubKey) { case 64: @@ -414,14 +378,15 @@ func importPublicKey(pubKey []byte) (pubKeyEC *ecdsa.PublicKey, err error) { default: return nil, fmt.Errorf("invalid public key length %v (expect 64/65)", len(pubKey)) } - return crypto.ToECDSAPub(pubKey65), nil + // TODO: fewer pointless conversions + return ecies.ImportECDSAPublic(crypto.ToECDSAPub(pubKey65)), nil } -func exportPublicKey(pubKeyEC *ecdsa.PublicKey) (pubKey []byte, err error) { - if pubKeyEC == nil { - return nil, fmt.Errorf("no ECDSA public key given") +func exportPubkey(pub *ecies.PublicKey) []byte { + if pub == nil { + panic("nil pubkey") } - return crypto.FromECDSAPub(pubKeyEC)[1:], nil + return elliptic.Marshal(pub.Curve, pub.X, pub.Y)[1:] } func xor(one, other []byte) (xor []byte) { diff --git a/p2p/handshake_test.go b/p2p/handshake_test.go index 66e610d17..19423bb82 100644 --- a/p2p/handshake_test.go +++ b/p2p/handshake_test.go @@ -2,51 +2,18 @@ package p2p import ( "bytes" + "crypto/rand" + "fmt" "net" "reflect" "testing" + "time" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/crypto/ecies" "github.com/ethereum/go-ethereum/p2p/discover" ) -func TestPublicKeyEncoding(t *testing.T) { - prv0, _ := crypto.GenerateKey() // = ecdsa.GenerateKey(crypto.S256(), rand.Reader) - pub0 := &prv0.PublicKey - pub0s := crypto.FromECDSAPub(pub0) - pub1, err := importPublicKey(pub0s) - if err != nil { - t.Errorf("%v", err) - } - eciesPub1 := ecies.ImportECDSAPublic(pub1) - if eciesPub1 == nil { - t.Errorf("invalid ecdsa public key") - } - pub1s, err := exportPublicKey(pub1) - if err != nil { - t.Errorf("%v", err) - } - if len(pub1s) != 64 { - t.Errorf("wrong length expect 64, got", len(pub1s)) - } - pub2, err := importPublicKey(pub1s) - if err != nil { - t.Errorf("%v", err) - } - pub2s, err := exportPublicKey(pub2) - if err != nil { - t.Errorf("%v", err) - } - if !bytes.Equal(pub1s, pub2s) { - t.Errorf("exports dont match") - } - pub2sEC := crypto.FromECDSAPub(pub2) - if !bytes.Equal(pub0s, pub2sEC) { - t.Errorf("exports dont match") - } -} - func TestSharedSecret(t *testing.T) { prv0, _ := crypto.GenerateKey() // = ecdsa.GenerateKey(crypto.S256(), rand.Reader) pub0 := &prv0.PublicKey @@ -68,46 +35,84 @@ func TestSharedSecret(t *testing.T) { } func TestEncHandshake(t *testing.T) { - defer testlog(t).detach() + for i := 0; i < 20; i++ { + start := time.Now() + if err := testEncHandshake(nil); err != nil { + t.Fatalf("i=%d %v", i, err) + } + t.Logf("(without token) %d %v\n", i+1, time.Since(start)) + } - prv0, _ := crypto.GenerateKey() - prv1, _ := crypto.GenerateKey() - rw0, rw1 := net.Pipe() - secrets := make(chan secrets) + for i := 0; i < 20; i++ { + tok := make([]byte, shaLen) + rand.Reader.Read(tok) + start := time.Now() + if err := testEncHandshake(tok); err != nil { + t.Fatalf("i=%d %v", i, err) + } + t.Logf("(with token) %d %v\n", i+1, time.Since(start)) + } +} + +func testEncHandshake(token []byte) error { + type result struct { + side string + s secrets + err error + } + var ( + prv0, _ = crypto.GenerateKey() + prv1, _ = crypto.GenerateKey() + rw0, rw1 = net.Pipe() + output = make(chan result) + ) go func() { - pub1s, _ := exportPublicKey(&prv1.PublicKey) - s, err := outboundEncHandshake(rw0, prv0, pub1s, nil) - if err != nil { - t.Errorf("outbound side error: %v", err) + r := result{side: "initiator"} + defer func() { output <- r }() + + pub1s := discover.PubkeyID(&prv1.PublicKey) + r.s, r.err = initiatorEncHandshake(rw0, prv0, pub1s, token) + if r.err != nil { + return } id1 := discover.PubkeyID(&prv1.PublicKey) - if s.RemoteID != id1 { - t.Errorf("outbound side remote ID mismatch") + if r.s.RemoteID != id1 { + r.err = fmt.Errorf("remote ID mismatch: got %v, want: %v", r.s.RemoteID, id1) } - secrets <- s }() go func() { - s, err := inboundEncHandshake(rw1, prv1, nil) - if err != nil { - t.Errorf("inbound side error: %v", err) + r := result{side: "receiver"} + defer func() { output <- r }() + + r.s, r.err = receiverEncHandshake(rw1, prv1, token) + if r.err != nil { + return } id0 := discover.PubkeyID(&prv0.PublicKey) - if s.RemoteID != id0 { - t.Errorf("inbound side remote ID mismatch") + if r.s.RemoteID != id0 { + r.err = fmt.Errorf("remote ID mismatch: got %v, want: %v", r.s.RemoteID, id0) } - secrets <- s }() - // get computed secrets from both sides - t1, t2 := <-secrets, <-secrets - // don't compare remote node IDs - t1.RemoteID, t2.RemoteID = discover.NodeID{}, discover.NodeID{} - // flip MACs on one of them so they compare equal - t1.EgressMAC, t1.IngressMAC = t1.IngressMAC, t1.EgressMAC - if !reflect.DeepEqual(t1, t2) { - t.Errorf("secrets mismatch:\n t1: %#v\n t2: %#v", t1, t2) + // wait for results from both sides + r1, r2 := <-output, <-output + + if r1.err != nil { + return fmt.Errorf("%s side error: %v", r1.side, r1.err) } + if r2.err != nil { + return fmt.Errorf("%s side error: %v", r2.side, r2.err) + } + + // don't compare remote node IDs + r1.s.RemoteID, r2.s.RemoteID = discover.NodeID{}, discover.NodeID{} + // flip MACs on one of them so they compare equal + r1.s.EgressMAC, r1.s.IngressMAC = r1.s.IngressMAC, r1.s.EgressMAC + if !reflect.DeepEqual(r1.s, r2.s) { + return fmt.Errorf("secrets mismatch:\n t1: %#v\n t2: %#v", r1.s, r2.s) + } + return nil } func TestSetupConn(t *testing.T) { From 21649100b1ed64c9bd73c547360dd6db9b5218fb Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 4 Mar 2015 12:02:08 +0100 Subject: [PATCH 08/13] p2p: verify protocol handshake node ID --- p2p/handshake.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/p2p/handshake.go b/p2p/handshake.go index a56de968d..3ad25bae4 100644 --- a/p2p/handshake.go +++ b/p2p/handshake.go @@ -89,6 +89,9 @@ func setupInboundConn(fd net.Conn, prv *ecdsa.PrivateKey, our *protoHandshake) ( if err != nil { return nil, err } + if rhs.ID != secrets.RemoteID { + return nil, errors.New("node ID in protocol handshake does not match encryption handshake") + } // TODO: validate that handshake node ID matches if err := writeProtocolHandshake(rw, our); err != nil { return nil, fmt.Errorf("protocol write error: %v", err) From 7964f30dcbdde00b2960ef6e98320e0a0f9300e2 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 4 Mar 2015 12:03:43 +0100 Subject: [PATCH 09/13] p2p: msg.Payload contains list data With RLPx frames, the message code is contained in the frame and is no longer part of the encoded data. EncodeMsg, Msg.Decode have been updated to match. Code that decodes RLP directly from Msg.Payload will need to change. --- p2p/message.go | 18 +++--------------- p2p/message_test.go | 16 +++++++++++++--- p2p/peer.go | 4 ++-- p2p/peer_test.go | 43 ++----------------------------------------- p2p/rlpx_test.go | 14 ++------------ p2p/server.go | 3 ++- p2p/server_test.go | 2 +- 7 files changed, 25 insertions(+), 75 deletions(-) diff --git a/p2p/message.go b/p2p/message.go index 2ef84f99d..04b9e71f3 100644 --- a/p2p/message.go +++ b/p2p/message.go @@ -51,19 +51,8 @@ type Msg struct { // NewMsg creates an RLP-encoded message with the given code. func NewMsg(code uint64, params ...interface{}) Msg { - buf := new(bytes.Buffer) - for _, p := range params { - buf.Write(ethutil.Encode(p)) - } - return Msg{Code: code, Size: uint32(buf.Len()), Payload: buf} -} - -func encodePayload(params ...interface{}) []byte { - buf := new(bytes.Buffer) - for _, p := range params { - buf.Write(ethutil.Encode(p)) - } - return buf.Bytes() + p := bytes.NewReader(ethutil.Encode(params)) + return Msg{Code: code, Size: uint32(p.Len()), Payload: p} } // Decode parse the RLP content of a message into @@ -71,8 +60,7 @@ func encodePayload(params ...interface{}) []byte { // // For the decoding rules, please see package rlp. func (msg Msg) Decode(val interface{}) error { - s := rlp.NewListStream(msg.Payload, uint64(msg.Size)) - if err := s.Decode(val); err != nil { + if err := rlp.Decode(msg.Payload, val); err != nil { return newPeerError(errInvalidMsg, "(code %#x) (size %d) %v", msg.Code, msg.Size, err) } return nil diff --git a/p2p/message_test.go b/p2p/message_test.go index 1757cbe7a..31ed61d87 100644 --- a/p2p/message_test.go +++ b/p2p/message_test.go @@ -2,10 +2,12 @@ package p2p import ( "bytes" + "encoding/hex" "fmt" "io" "io/ioutil" "runtime" + "strings" "testing" "time" ) @@ -15,11 +17,11 @@ func TestNewMsg(t *testing.T) { if msg.Code != 3 { t.Errorf("incorrect code %d, want %d", msg.Code) } - if msg.Size != 5 { - t.Errorf("incorrect size %d, want %d", msg.Size, 5) + expect := unhex("c50183303030") + if msg.Size != uint32(len(expect)) { + t.Errorf("incorrect size %d, want %d", msg.Size, len(expect)) } pl, _ := ioutil.ReadAll(msg.Payload) - expect := []byte{0x01, 0x83, 0x30, 0x30, 0x30} if !bytes.Equal(pl, expect) { t.Errorf("incorrect payload content, got %x, want %x", pl, expect) } @@ -139,3 +141,11 @@ func TestEOFSignal(t *testing.T) { default: } } + +func unhex(str string) []byte { + b, err := hex.DecodeString(strings.Replace(str, "\n", "", -1)) + if err != nil { + panic(fmt.Sprintf("invalid hex string: %q", str)) + } + return b +} diff --git a/p2p/peer.go b/p2p/peer.go index 4982c4612..025be4ba9 100644 --- a/p2p/peer.go +++ b/p2p/peer.go @@ -193,12 +193,12 @@ func (p *Peer) handle(msg Msg) error { msg.Discard() go EncodeMsg(p.rw, pongMsg) case msg.Code == discMsg: - var reason DiscReason + var reason [1]DiscReason // no need to discard or for error checking, we'll close the // connection after this. rlp.Decode(msg.Payload, &reason) p.Disconnect(DiscRequested) - return discRequestedError(reason) + return discRequestedError(reason[0]) case msg.Code < baseProtocolLength: // ignore other base protocol messages return msg.Discard() diff --git a/p2p/peer_test.go b/p2p/peer_test.go index 1ba43bed5..cc9f1f0cd 100644 --- a/p2p/peer_test.go +++ b/p2p/peer_test.go @@ -85,41 +85,6 @@ func TestPeerProtoReadMsg(t *testing.T) { } } -func TestPeerProtoReadLargeMsg(t *testing.T) { - defer testlog(t).detach() - - msgsize := uint32(10 * 1024 * 1024) - done := make(chan struct{}) - proto := Protocol{ - Name: "a", - Length: 5, - Run: func(peer *Peer, rw MsgReadWriter) error { - msg, err := rw.ReadMsg() - if err != nil { - t.Errorf("read error: %v", err) - } - if msg.Size != msgsize+4 { - t.Errorf("incorrect msg.Size, got %d, expected %d", msg.Size, msgsize) - } - msg.Discard() - close(done) - return nil - }, - } - - closer, rw, _, errc := testPeer([]Protocol{proto}) - defer closer.Close() - - EncodeMsg(rw, 18, make([]byte, msgsize)) - select { - case <-done: - case err := <-errc: - t.Errorf("peer returned: %v", err) - case <-time.After(2 * time.Second): - t.Errorf("receive timeout") - } -} - func TestPeerProtoEncodeMsg(t *testing.T) { defer testlog(t).detach() @@ -246,13 +211,9 @@ func expectMsg(r MsgReader, code uint64, content interface{}) error { if err != nil { panic("content encode error: " + err.Error()) } - // skip over list header in encoded value. this is temporary. - contentEncR := bytes.NewReader(contentEnc) - if k, _, err := rlp.NewStream(contentEncR).Kind(); k != rlp.List || err != nil { - panic("content must encode as RLP list") + if int(msg.Size) != len(contentEnc) { + return fmt.Errorf("message size mismatch: got %d, want %d", msg.Size, len(contentEnc)) } - contentEnc = contentEnc[len(contentEnc)-contentEncR.Len():] - actualContent, err := ioutil.ReadAll(msg.Payload) if err != nil { return err diff --git a/p2p/rlpx_test.go b/p2p/rlpx_test.go index 077dd1309..49354c7ed 100644 --- a/p2p/rlpx_test.go +++ b/p2p/rlpx_test.go @@ -3,8 +3,6 @@ package p2p import ( "bytes" "crypto/rand" - "encoding/hex" - "fmt" "io/ioutil" "strings" "testing" @@ -32,7 +30,7 @@ ba628a4ba590cb43f7848f41c4382885 `) // Check WriteMsg. This puts a message into the buffer. - if err := EncodeMsg(rw, 8, []interface{}{1, 2, 3, 4}); err != nil { + if err := EncodeMsg(rw, 8, 1, 2, 3, 4); err != nil { t.Fatalf("WriteMsg error: %v", err) } written := buf.Bytes() @@ -68,14 +66,6 @@ func (fakeHash) BlockSize() int { return 0 } func (h fakeHash) Size() int { return len(h) } func (h fakeHash) Sum(b []byte) []byte { return append(b, h...) } -func unhex(str string) []byte { - b, err := hex.DecodeString(strings.Replace(str, "\n", "", -1)) - if err != nil { - panic(fmt.Sprintf("invalid hex string: %q", str)) - } - return b -} - func TestRlpxFrameRW(t *testing.T) { var ( aesSecret = make([]byte, 16) @@ -112,7 +102,7 @@ func TestRlpxFrameRW(t *testing.T) { for i := 0; i < 10; i++ { // write message into conn buffer wmsg := []interface{}{"foo", "bar", strings.Repeat("test", i)} - err := EncodeMsg(rw1, uint64(i), wmsg) + err := EncodeMsg(rw1, uint64(i), wmsg...) if err != nil { t.Fatalf("WriteMsg error (i=%d): %v", i, err) } diff --git a/p2p/server.go b/p2p/server.go index e53e832aa..67d5514b4 100644 --- a/p2p/server.go +++ b/p2p/server.go @@ -10,6 +10,7 @@ import ( "sync" "time" + "github.com/ethereum/go-ethereum/ethutil" "github.com/ethereum/go-ethereum/logger" "github.com/ethereum/go-ethereum/p2p/discover" "github.com/ethereum/go-ethereum/p2p/nat" @@ -135,7 +136,7 @@ func (srv *Server) SuggestPeer(n *discover.Node) { func (srv *Server) Broadcast(protocol string, code uint64, data ...interface{}) { var payload []byte if data != nil { - payload = encodePayload(data...) + payload = ethutil.Encode(data) } srv.lock.RLock() defer srv.lock.RUnlock() diff --git a/p2p/server_test.go b/p2p/server_test.go index c348f5a9a..30447050c 100644 --- a/p2p/server_test.go +++ b/p2p/server_test.go @@ -150,7 +150,7 @@ func TestServerBroadcast(t *testing.T) { // broadcast one message srv.Broadcast("discard", 0, "foo") - golden := unhex("66e94e166f0a2c3b884cfa59ca34") + golden := unhex("66e94d166f0a2c3b884cfa59ca34") // check that the message has been written everywhere for i, conn := range conns { From 6e7e5d5fd56a9a6f73e51239ed6648d76db9650d Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 4 Mar 2015 13:12:50 +0100 Subject: [PATCH 10/13] eth, whisper: fix msg.Payload reads --- eth/protocol.go | 40 +++++++++++++++++++++++----------------- whisper/peer.go | 27 ++++++++++----------------- 2 files changed, 33 insertions(+), 34 deletions(-) diff --git a/eth/protocol.go b/eth/protocol.go index 663af43fe..b86f33614 100644 --- a/eth/protocol.go +++ b/eth/protocol.go @@ -3,7 +3,6 @@ package eth import ( "bytes" "fmt" - "io" "math/big" "github.com/ethereum/go-ethereum/core/types" @@ -188,33 +187,37 @@ func (self *ethProtocol) handle() error { case BlockHashesMsg: msgStream := rlp.NewStream(msg.Payload) - var err error - var i int - - iter := func() (hash []byte, ok bool) { - hash, err = msgStream.Bytes() - if err == nil { - i++ - ok = true - } else { - if err != io.EOF { - self.protoError(ErrDecode, "msg %v: after %v hashes : %v", msg, i, err) - } - } - return + if _, err := msgStream.List(); err != nil { + return err } + var i int + iter := func() (hash []byte, ok bool) { + hash, err := msgStream.Bytes() + if err == rlp.EOL { + return nil, false + } else if err != nil { + self.protoError(ErrDecode, "msg %v: after %v hashes : %v", msg, i, err) + return nil, false + } + i++ + return hash, true + } self.blockPool.AddBlockHashes(iter, self.id) case GetBlocksMsg: msgStream := rlp.NewStream(msg.Payload) + if _, err := msgStream.List(); err != nil { + return err + } + var blocks []interface{} var i int for { i++ var hash []byte if err := msgStream.Decode(&hash); err != nil { - if err == io.EOF { + if err == rlp.EOL { break } else { return self.protoError(ErrDecode, "msg %v: %v", msg, err) @@ -232,10 +235,13 @@ func (self *ethProtocol) handle() error { case BlocksMsg: msgStream := rlp.NewStream(msg.Payload) + if _, err := msgStream.List(); err != nil { + return err + } for { var block types.Block if err := msgStream.Decode(&block); err != nil { - if err == io.EOF { + if err == rlp.EOL { break } else { return self.protoError(ErrDecode, "msg %v: %v", msg, err) diff --git a/whisper/peer.go b/whisper/peer.go index 332ddd22a..66cfec88c 100644 --- a/whisper/peer.go +++ b/whisper/peer.go @@ -2,10 +2,10 @@ package whisper import ( "fmt" - "io/ioutil" "time" "github.com/ethereum/go-ethereum/p2p" + "github.com/ethereum/go-ethereum/rlp" "gopkg.in/fatih/set.v0" ) @@ -77,8 +77,7 @@ func (self *peer) broadcast(envelopes []*Envelope) error { } if i > 0 { - msg := p2p.NewMsg(envelopesMsg, envs[:i]...) - if err := self.ws.WriteMsg(msg); err != nil { + if err := p2p.EncodeMsg(self.ws, envelopesMsg, envs[:i]...); err != nil { return err } self.peer.DebugDetailln("broadcasted", i, "message(s)") @@ -93,34 +92,28 @@ func (self *peer) addKnown(envelope *Envelope) { func (self *peer) handleStatus() error { ws := self.ws - if err := ws.WriteMsg(self.statusMsg()); err != nil { return err } - msg, err := ws.ReadMsg() if err != nil { return err } - if msg.Code != statusMsg { return fmt.Errorf("peer send %x before status msg", msg.Code) } - - data, err := ioutil.ReadAll(msg.Payload) + s := rlp.NewStream(msg.Payload) + if _, err := s.List(); err != nil { + return fmt.Errorf("bad status message: %v", err) + } + pv, err := s.Uint() if err != nil { - return err + return fmt.Errorf("bad status message: %v", err) } - - if len(data) == 0 { - return fmt.Errorf("malformed status. data len = 0") - } - - if pv := data[0]; pv != protocolVersion { + if pv != protocolVersion { return fmt.Errorf("protocol version mismatch %d != %d", pv, protocolVersion) } - - return nil + return msg.Discard() // ignore anything after protocol version } func (self *peer) statusMsg() p2p.Msg { From 22659a7feaf4e939a33762c3f83b43d8bec757db Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 4 Mar 2015 16:27:37 +0100 Subject: [PATCH 11/13] p2p: restore read/write timeouts They got lost in the transition to rlpxFrameRW. --- p2p/handshake.go | 8 +++----- p2p/message.go | 40 ++++++++++++---------------------------- p2p/peer.go | 3 ++- p2p/rlpx.go | 5 +++++ p2p/server.go | 18 +++++++++++++++--- 5 files changed, 37 insertions(+), 37 deletions(-) diff --git a/p2p/handshake.go b/p2p/handshake.go index 3ad25bae4..7fc497517 100644 --- a/p2p/handshake.go +++ b/p2p/handshake.go @@ -37,7 +37,7 @@ const ( // // The MsgReadWriter is usually layered as follows: // -// lockedRW (thread-safety for ReadMsg, WriteMsg) +// netWrapper (I/O timeouts, thread-safe ReadMsg, WriteMsg) // rlpxFrameRW (message encoding, encryption, authentication) // bufio.ReadWriter (buffering) // net.Conn (network I/O) @@ -83,7 +83,6 @@ func setupInboundConn(fd net.Conn, prv *ecdsa.PrivateKey, our *protoHandshake) ( } // Run the protocol handshake using authenticated messages. - // TODO: move buffering setup here (out of newFrameRW) rw := newRlpxFrameRW(fd, secrets) rhs, err := readProtocolHandshake(rw, our) if err != nil { @@ -96,7 +95,7 @@ func setupInboundConn(fd net.Conn, prv *ecdsa.PrivateKey, our *protoHandshake) ( if err := writeProtocolHandshake(rw, our); err != nil { return nil, fmt.Errorf("protocol write error: %v", err) } - return &conn{&lockedRW{wrapped: rw}, rhs}, nil + return &conn{rw, rhs}, nil } func setupOutboundConn(fd net.Conn, prv *ecdsa.PrivateKey, our *protoHandshake, dial *discover.Node) (*conn, error) { @@ -106,7 +105,6 @@ func setupOutboundConn(fd net.Conn, prv *ecdsa.PrivateKey, our *protoHandshake, } // Run the protocol handshake using authenticated messages. - // TODO: move buffering setup here (out of newFrameRW) rw := newRlpxFrameRW(fd, secrets) if err := writeProtocolHandshake(rw, our); err != nil { return nil, fmt.Errorf("protocol write error: %v", err) @@ -118,7 +116,7 @@ func setupOutboundConn(fd net.Conn, prv *ecdsa.PrivateKey, our *protoHandshake, if rhs.ID != dial.ID { return nil, errors.New("dialed node id mismatch") } - return &conn{&lockedRW{wrapped: rw}, rhs}, nil + return &conn{rw, rhs}, nil } // encHandshake contains the state of the encryption handshake. diff --git a/p2p/message.go b/p2p/message.go index 04b9e71f3..f88c31d1d 100644 --- a/p2p/message.go +++ b/p2p/message.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "io/ioutil" + "net" "sync" "sync/atomic" "time" @@ -14,28 +15,6 @@ import ( "github.com/ethereum/go-ethereum/rlp" ) -// parameters for frameRW -const ( - // maximum time allowed for reading a message header. - // this is effectively the amount of time a connection can be idle. - frameReadTimeout = 1 * time.Minute - - // maximum time allowed for reading the payload data of a message. - // this is shorter than (and distinct from) frameReadTimeout because - // the connection is not considered idle while a message is transferred. - // this also limits the payload size of messages to how much the connection - // can transfer within the timeout. - payloadReadTimeout = 5 * time.Second - - // maximum amount of time allowed for writing a complete message. - msgWriteTimeout = 5 * time.Second - - // messages smaller than this many bytes will be read at - // once before passing them to a protocol. this increases - // concurrency in the processing. - wholePayloadSize = 64 * 1024 -) - // Msg defines the structure of a p2p message. // // Note that a Msg can only be sent once since the Payload reader is @@ -103,22 +82,27 @@ func EncodeMsg(w MsgWriter, code uint64, data ...interface{}) error { return w.WriteMsg(NewMsg(code, data...)) } -// lockedRW wraps a MsgReadWriter with locks around -// ReadMsg and WriteMsg. -type lockedRW struct { +// netWrapper wrapsa MsgReadWriter with locks around +// ReadMsg/WriteMsg and applies read/write deadlines. +type netWrapper struct { rmu, wmu sync.Mutex - wrapped MsgReadWriter + + rtimeout, wtimeout time.Duration + conn net.Conn + wrapped MsgReadWriter } -func (rw *lockedRW) ReadMsg() (Msg, error) { +func (rw *netWrapper) ReadMsg() (Msg, error) { rw.rmu.Lock() defer rw.rmu.Unlock() + rw.conn.SetReadDeadline(time.Now().Add(rw.rtimeout)) return rw.wrapped.ReadMsg() } -func (rw *lockedRW) WriteMsg(msg Msg) error { +func (rw *netWrapper) WriteMsg(msg Msg) error { rw.wmu.Lock() defer rw.wmu.Unlock() + rw.conn.SetWriteDeadline(time.Now().Add(rw.wtimeout)) return rw.wrapped.WriteMsg(msg) } diff --git a/p2p/peer.go b/p2p/peer.go index 025be4ba9..c2c83abfc 100644 --- a/p2p/peer.go +++ b/p2p/peer.go @@ -20,8 +20,8 @@ const ( baseProtocolLength = uint64(16) baseProtocolMaxMsgSize = 10 * 1024 * 1024 - disconnectGracePeriod = 2 * time.Second pingInterval = 15 * time.Second + disconnectGracePeriod = 2 * time.Second ) const ( @@ -176,6 +176,7 @@ func (p *Peer) politeDisconnect(reason DiscReason) { func (p *Peer) readLoop() error { for { + p.conn.SetDeadline(time.Now().Add(frameReadTimeout)) msg, err := p.rw.ReadMsg() if err != nil { return err diff --git a/p2p/rlpx.go b/p2p/rlpx.go index a041bb314..166bbb5e6 100644 --- a/p2p/rlpx.go +++ b/p2p/rlpx.go @@ -21,6 +21,11 @@ var ( zero16 = make([]byte, 16) ) +// rlpxFrameRW implements a simplified version of RLPx framing. +// chunked messages are not supported and all headers are equal to +// zeroHeader. +// +// rlpxFrameRW is not safe for concurrent use from multiple goroutines. type rlpxFrameRW struct { conn io.ReadWriter enc cipher.Stream diff --git a/p2p/server.go b/p2p/server.go index 67d5514b4..8f99bc33d 100644 --- a/p2p/server.go +++ b/p2p/server.go @@ -17,9 +17,17 @@ import ( ) const ( - handshakeTimeout = 5 * time.Second defaultDialTimeout = 10 * time.Second refreshPeersInterval = 30 * time.Second + + // total timeout for encryption handshake and protocol + // handshake in both directions. + handshakeTimeout = 5 * time.Second + // maximum time allowed for reading a complete message. + // this is effectively the amount of time a connection can be idle. + frameReadTimeout = 1 * time.Minute + // maximum amount of time allowed for writing a complete message. + frameWriteTimeout = 5 * time.Second ) var srvlog = logger.NewLogger("P2P Server") @@ -359,14 +367,18 @@ func (srv *Server) findPeers() { func (srv *Server) startPeer(fd net.Conn, dest *discover.Node) { // TODO: handle/store session token - // TODO: reenable deadlines - // fd.SetDeadline(time.Now().Add(handshakeTimeout)) + fd.SetDeadline(time.Now().Add(handshakeTimeout)) conn, err := srv.setupFunc(fd, srv.PrivateKey, srv.ourHandshake, dest) if err != nil { fd.Close() srvlog.Debugf("Handshake with %v failed: %v", fd.RemoteAddr(), err) return } + + conn.MsgReadWriter = &netWrapper{ + wrapped: conn.MsgReadWriter, + conn: fd, rtimeout: frameReadTimeout, wtimeout: frameWriteTimeout, + } p := newPeer(fd, conn, srv.Protocols) if ok, reason := srv.addPeer(conn.ID, p); !ok { srvlog.DebugDetailf("Not adding %v (%v)\n", p, reason) From 429828cd9205a8db0024652fd9da96cfbdaeae86 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 4 Mar 2015 16:39:04 +0100 Subject: [PATCH 12/13] p2p: reject messages that cannot be written as simple RLPx frames Until chunked frames are implemented we cannot send messages with a size overflowing uint24. --- p2p/rlpx.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/p2p/rlpx.go b/p2p/rlpx.go index 166bbb5e6..6b533e275 100644 --- a/p2p/rlpx.go +++ b/p2p/rlpx.go @@ -19,6 +19,8 @@ var ( // sixteen zero bytes zero16 = make([]byte, 16) + + maxUint24 = ^uint32(0) >> 8 ) // rlpxFrameRW implements a simplified version of RLPx framing. @@ -64,6 +66,9 @@ func (rw *rlpxFrameRW) WriteMsg(msg Msg) error { // write header headbuf := make([]byte, 32) fsize := uint32(len(ptype)) + msg.Size + if fsize > maxUint24 { + return errors.New("message size overflows uint24") + } putInt24(fsize, headbuf) // TODO: check overflow copy(headbuf[3:], zeroHeader) rw.enc.XORKeyStream(headbuf[:16], headbuf[:16]) // first half is now encrypted From 215c763d53fc8e06e8c9807875eacaccf3ef45fa Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 4 Mar 2015 16:54:36 +0100 Subject: [PATCH 13/13] eth, p2p: delete p2p.Blacklist It is unused and untested right now. We can bring it back later if required. --- eth/backend.go | 9 +------- p2p/server.go | 59 -------------------------------------------------- 2 files changed, 1 insertion(+), 67 deletions(-) diff --git a/eth/backend.go b/eth/backend.go index 5cd7b308f..8b76e0ca3 100644 --- a/eth/backend.go +++ b/eth/backend.go @@ -108,11 +108,9 @@ func (cfg *Config) nodeKey() (*ecdsa.PrivateKey, error) { type Ethereum struct { // Channel for shutting down the ethereum shutdownChan chan bool - quit chan bool // DB interface - db ethutil.Database - blacklist p2p.Blacklist + db ethutil.Database //*** SERVICES *** // State manager for processing new blocks and managing the over all states @@ -170,10 +168,8 @@ func New(config *Config) (*Ethereum, error) { eth := &Ethereum{ shutdownChan: make(chan bool), - quit: make(chan bool), db: db, keyManager: keyManager, - blacklist: p2p.NewBlacklist(), eventMux: &event.TypeMux{}, logger: ethlogger, } @@ -206,7 +202,6 @@ func New(config *Config) (*Ethereum, error) { Name: config.Name, MaxPeers: config.MaxPeers, Protocols: protocols, - Blacklist: eth.blacklist, NAT: config.NAT, NoDial: !config.Dial, BootstrapNodes: config.parseBootNodes(), @@ -280,8 +275,6 @@ func (s *Ethereum) Stop() { // Close the database defer s.db.Close() - close(s.quit) - s.txSub.Unsubscribe() // quits txBroadcastLoop s.blockSub.Unsubscribe() // quits blockBroadcastLoop diff --git a/p2p/server.go b/p2p/server.go index 8f99bc33d..34000cb4c 100644 --- a/p2p/server.go +++ b/p2p/server.go @@ -66,10 +66,6 @@ type Server struct { // each peer. Protocols []Protocol - // If Blacklist is set to a non-nil value, the given Blacklist - // is used to verify peer connections. - Blacklist Blacklist - // If ListenAddr is set to a non-nil address, the server // will listen for incoming connections. // @@ -183,9 +179,6 @@ func (srv *Server) Start() (err error) { if srv.setupFunc == nil { srv.setupFunc = setupConn } - if srv.Blacklist == nil { - srv.Blacklist = NewBlacklist() - } // node table ntab, err := discover.ListenUDP(srv.PrivateKey, srv.ListenAddr, srv.NAT) @@ -417,8 +410,6 @@ func (srv *Server) addPeer(id discover.NodeID, p *Peer) (bool, DiscReason) { return false, DiscTooManyPeers case srv.peers[id] != nil: return false, DiscAlreadyConnected - case srv.Blacklist.Exists(id[:]): - return false, DiscUselessPeer case id == srv.ntab.Self(): return false, DiscSelf } @@ -432,53 +423,3 @@ func (srv *Server) removePeer(p *Peer) { srv.lock.Unlock() srv.peerWG.Done() } - -type Blacklist interface { - Get([]byte) (bool, error) - Put([]byte) error - Delete([]byte) error - Exists(pubkey []byte) (ok bool) -} - -type BlacklistMap struct { - blacklist map[string]bool - lock sync.RWMutex -} - -func NewBlacklist() *BlacklistMap { - return &BlacklistMap{ - blacklist: make(map[string]bool), - } -} - -func (self *BlacklistMap) Get(pubkey []byte) (bool, error) { - self.lock.RLock() - defer self.lock.RUnlock() - v, ok := self.blacklist[string(pubkey)] - var err error - if !ok { - err = fmt.Errorf("not found") - } - return v, err -} - -func (self *BlacklistMap) Exists(pubkey []byte) (ok bool) { - self.lock.RLock() - defer self.lock.RUnlock() - _, ok = self.blacklist[string(pubkey)] - return -} - -func (self *BlacklistMap) Put(pubkey []byte) error { - self.lock.Lock() - defer self.lock.Unlock() - self.blacklist[string(pubkey)] = true - return nil -} - -func (self *BlacklistMap) Delete(pubkey []byte) error { - self.lock.Lock() - defer self.lock.Unlock() - delete(self.blacklist, string(pubkey)) - return nil -}