Check that announced IP address is not an internal IP (ethereum#746)

* more helpful unauth msg, tweaks to loglevels

* Better unauthorized log msg, tweak log levels

* Check external proxy enode is not a private ip

* added urlv4 test with a docker ip

cmd/utils/flags.go

@@ -1283,6 +1283,11 @@ func SetProxyConfig(ctx *cli.Context, nodeCfg *node.Config, ethCfg *eth.Config)
 			if ethCfg.Istanbul.ProxyExternalFacingNode, err = enode.ParseV4(proxyEnodeURLPair[1]); err != nil {
 				Fatalf("Proxy external facing enodeURL (%s) invalid with err: %v", proxyEnodeURLPair[1], err)
 			}
+
+			// Check that external IP is not a private IP address.
+			if ethCfg.Istanbul.ProxyExternalFacingNode.IsPrivateIP() {
+				Fatalf("Proxy external facing enodeURL (%s) cannot be private IP.", proxyEnodeURLPair[1])
+			}
 		}
 
 		if !ctx.GlobalBool(NoDiscoverFlag.Name) {

consensus/istanbul/backend/engine.go

@@ -59,7 +59,7 @@ var (
 	// that is not part of the local blockchain.
 	errUnknownBlock = errors.New("unknown block")
 	// errUnauthorized is returned if a header is signed by a non authorized entity.
-	errUnauthorized = errors.New("unauthorized")
+	errUnauthorized = errors.New("not an elected validator")
 	// errInvalidDifficulty is returned if the difficulty of a block is not 1
 	errInvalidDifficulty = errors.New("invalid difficulty")
 	// errInvalidExtraDataFormat is returned when the extra data format is incorrect

consensus/istanbul/backend/handler.go

@@ -71,7 +71,11 @@ func (sb *Backend) HandleMsg(addr common.Address, msg p2p.Msg, peer consensus.Pe
 
 		var data []byte
 		if err := msg.Decode(&data); err != nil {
-			sb.logger.Error("Failed to decode message payload", "msg", msg)
+			if err == errUnauthorized {
+				sb.logger.Debug("Failed to decode message payload", "err", err)
+			} else {
+				sb.logger.Error("Failed to decode message payload", "err", err)
+			}
 			return true, errDecodeFailed
 		}
 
@@ -130,7 +134,7 @@ func (sb *Backend) handleConsensusMsg(peer consensus.Peer, payload []byte) error
 		}
 
 		// Need to forward the message to the proxied validator
-		sb.logger.Debug("Forwarding consensus message to proxied validator")
+		sb.logger.Trace("Forwarding consensus message to proxied validator")
 		if sb.proxiedPeer != nil {
 			go sb.proxiedPeer.Send(istanbulConsensusMsg, payload)
 		}
@@ -223,11 +227,8 @@ func (sb *Backend) NewChainHead(newBlock *types.Block) {
 		// Output whether this validator was or wasn't elected for the
 		// new epoch's validator set
 		if sb.coreStarted {
-			if _, val := valset.GetByAddress(sb.ValidatorAddress()); val != nil {
-				sb.logger.Info("Validators Election Results: Node IN ValidatorSet")
-			} else {
-				sb.logger.Info("Validators Election Results: Node OUT ValidatorSet")
-			}
+			_, val := valset.GetByAddress(sb.ValidatorAddress())
+			sb.logger.Info("Validator Election Results", "address", sb.ValidatorAddress(), "elected", (val != nil))
 
 			sb.newEpochCh <- struct{}{}
 		}

consensus/istanbul/core/core.go

@@ -386,7 +386,7 @@ func (c *core) waitForDesiredRound(r *big.Int) error {
 
 	// Don't wait for an older round
 	if c.current.DesiredRound().Cmp(r) >= 0 {
-		logger.Debug("New desired round not greater than current desired round")
+		logger.Trace("New desired round not greater than current desired round")
 		return nil
 	}
 

consensus/istanbul/core/roundchange.go

@@ -226,10 +226,10 @@ func (c *core) handleRoundChange(msg *istanbul.Message) error {
 	// On f+1 round changes we send a round change and wait for the next round if we haven't done so already
 	// On quorum round change messages we go to the next round immediately.
 	if quorumRound != nil && quorumRound.Cmp(c.current.DesiredRound()) >= 0 {
-		logger.Trace("Got quorum round change messages, starting new round.")
+		logger.Debug("Got quorum round change messages, starting new round.")
 		return c.startNewRound(quorumRound)
 	} else if ffRound != nil {
-		logger.Trace("Got f+1 round change messages, sending own round change message and waiting for next round.")
+		logger.Debug("Got f+1 round change messages, sending own round change message and waiting for next round.")
 		c.waitForDesiredRound(ffRound)
 	}
 

consensus/istanbul/errors.go

@@ -21,7 +21,7 @@ import "errors"
 var (
 	// ErrUnauthorizedAddress is returned when given address cannot be found in
 	// current validator set.
-	ErrUnauthorizedAddress = errors.New("unauthorized address")
+	ErrUnauthorizedAddress = errors.New("not an elected validator")
 	// ErrInvalidSigner is returned if a message's signature does not correspond to the address in msg.Address
 	ErrInvalidSigner = errors.New("signed by incorrect validator")
 	// ErrStoppedEngine is returned if the engine is stopped

eth/handler.go

@@ -288,7 +288,7 @@ func (pm *ProtocolManager) handle(p *peer) error {
 	if pm.peers.Len() >= pm.maxPeers && !(p.Peer.Info().Network.Trusted || p.Peer.Info().Network.Static) && p.Peer.Server != pm.proxyServer {
 		return p2p.DiscTooManyPeers
 	}
-	p.Log().Debug("Ethereum peer connected", "name", p.Name())
+	p.Log().Info("Ethereum peer connected", "name", p.Name())
 
 	// Execute the Ethereum handshake
 	var (
@@ -298,9 +298,8 @@ func (pm *ProtocolManager) handle(p *peer) error {
 		number  = head.Number.Uint64()
 		td      = pm.blockchain.GetTd(hash, number)
 	)
-	p.Log().Info("Ethereum handshake HASH", "hash", genesis.Hash(), "genesis", genesis)
 	if err := p.Handshake(pm.networkID, td, hash, genesis.Hash()); err != nil {
-		p.Log().Debug("Ethereum handshake failed", "err", err)
+		p.Log().Info("Ethereum handshake failed", "err", err)
 		return err
 	}
 	if rw, ok := p.rw.(*meteredMsgReadWriter); ok {

p2p/enode/urlv4.go

@@ -185,6 +185,20 @@ func (n *Node) v4URL() string {
 	return u.String()
 }
 
+var (
+	block24bit = net.IPNet{net.IPv4(10, 0, 0, 0), net.IPv4Mask(255, 0, 0, 0)}
+	block20bit = net.IPNet{net.IPv4(172, 16, 0, 0), net.IPv4Mask(255, 240, 0, 0)}
+	block16bit = net.IPNet{net.IPv4(192, 168, 0, 0), net.IPv4Mask(255, 255, 0, 0)}
+)
+
+// Returns true if the ip is a loopback or private ip, not generally accessible from the internet.
+func (n *Node) IsPrivateIP() bool {
+	return (!n.IP().IsGlobalUnicast() ||
+		block24bit.Contains(n.IP()) ||
+		block20bit.Contains(n.IP()) ||
+		block16bit.Contains(n.IP()))
+}
+
 // PubkeyToIDV4 derives the v4 node address from the given public key.
 func PubkeyToIDV4(key *ecdsa.PublicKey) ID {
 	e := make([]byte, 64)

p2p/enode/urlv4_test.go

@@ -28,9 +28,10 @@ import (
 )
 
 var parseNodeTests = []struct {
-	rawurl     string
-	wantError  string
-	wantResult *Node
+	rawurl      string
+	wantError   string
+	wantResult  *Node
+	wantPrivate bool
 }{
 	{
 		rawurl:    "http://foobar",
@@ -62,6 +63,17 @@ var parseNodeTests = []struct {
 			52150,
 			52150,
 		),
+		wantPrivate: true,
+	},
+	{
+		rawurl: "enode://1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439@5.1.1.1:52150",
+		wantResult: NewV4(
+			hexPubkey("1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"),
+			net.IP{0x5, 0x1, 0x1, 0x1},
+			52150,
+			52150,
+		),
+		wantPrivate: false,
 	},
 	{
 		rawurl: "enode://1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439@[::]:52150",
@@ -71,6 +83,17 @@ var parseNodeTests = []struct {
 			52150,
 			52150,
 		),
+		wantPrivate: true,
+	},
+	{
+		rawurl: "enode://1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439@172.17.0.3:52150",
+		wantResult: NewV4(
+			hexPubkey("1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439"),
+			net.ParseIP("172.17.0.3"),
+			52150,
+			52150,
+		),
+		wantPrivate: true,
 	},
 	{
 		rawurl: "enode://1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439@[2001:db8:3c4d:15::abcd:ef12]:52150",
@@ -89,6 +112,7 @@ var parseNodeTests = []struct {
 			52150,
 			22334,
 		),
+		wantPrivate: true,
 	},
 	// Incomplete nodes with no address.
 	{
@@ -148,6 +172,9 @@ func TestParseNode(t *testing.T) {
 			if !reflect.DeepEqual(n, test.wantResult) {
 				t.Errorf("test %q:\n  result mismatch:\ngot:  %#v\nwant: %#v", test.rawurl, n, test.wantResult)
 			}
+			if !n.Incomplete() && n.IsPrivateIP() != test.wantPrivate {
+				t.Errorf("test %q:\n  isPrivate mismatch:\nfor  %#v\ngot:  %#v\nwant: %#v", test.rawurl, n, n.IsPrivateIP(), test.wantPrivate)
+			}
 		}
 	}
 }