Merge pull request #16109 from karalabe/p2p-bond-check

p2p/discover: validate bond against lastpong, not db presence
This commit is contained in:
Péter Szilágyi 2018-02-17 18:47:44 +02:00 committed by GitHub
commit 9fd76e33af
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 26 additions and 25 deletions

View File

@ -257,7 +257,7 @@ func (db *nodeDB) expireNodes() error {
} }
// Skip the node if not expired yet (and not self) // Skip the node if not expired yet (and not self)
if !bytes.Equal(id[:], db.self[:]) { if !bytes.Equal(id[:], db.self[:]) {
if seen := db.lastPong(id); seen.After(threshold) { if seen := db.bondTime(id); seen.After(threshold) {
continue continue
} }
} }
@ -278,13 +278,18 @@ func (db *nodeDB) updateLastPing(id NodeID, instance time.Time) error {
return db.storeInt64(makeKey(id, nodeDBDiscoverPing), instance.Unix()) return db.storeInt64(makeKey(id, nodeDBDiscoverPing), instance.Unix())
} }
// lastPong retrieves the time of the last successful contact from remote node. // bondTime retrieves the time of the last successful pong from remote node.
func (db *nodeDB) lastPong(id NodeID) time.Time { func (db *nodeDB) bondTime(id NodeID) time.Time {
return time.Unix(db.fetchInt64(makeKey(id, nodeDBDiscoverPong)), 0) return time.Unix(db.fetchInt64(makeKey(id, nodeDBDiscoverPong)), 0)
} }
// updateLastPong updates the last time a remote node successfully contacted. // hasBond reports whether the given node is considered bonded.
func (db *nodeDB) updateLastPong(id NodeID, instance time.Time) error { func (db *nodeDB) hasBond(id NodeID) bool {
return time.Since(db.bondTime(id)) < nodeDBNodeExpiration
}
// updateBondTime updates the last pong time of a node.
func (db *nodeDB) updateBondTime(id NodeID, instance time.Time) error {
return db.storeInt64(makeKey(id, nodeDBDiscoverPong), instance.Unix()) return db.storeInt64(makeKey(id, nodeDBDiscoverPong), instance.Unix())
} }
@ -327,7 +332,7 @@ seek:
if n.ID == db.self { if n.ID == db.self {
continue seek continue seek
} }
if now.Sub(db.lastPong(n.ID)) > maxAge { if now.Sub(db.bondTime(n.ID)) > maxAge {
continue seek continue seek
} }
for i := range nodes { for i := range nodes {

View File

@ -125,13 +125,13 @@ func TestNodeDBFetchStore(t *testing.T) {
t.Errorf("ping: value mismatch: have %v, want %v", stored, inst) t.Errorf("ping: value mismatch: have %v, want %v", stored, inst)
} }
// Check fetch/store operations on a node pong object // Check fetch/store operations on a node pong object
if stored := db.lastPong(node.ID); stored.Unix() != 0 { if stored := db.bondTime(node.ID); stored.Unix() != 0 {
t.Errorf("pong: non-existing object: %v", stored) t.Errorf("pong: non-existing object: %v", stored)
} }
if err := db.updateLastPong(node.ID, inst); err != nil { if err := db.updateBondTime(node.ID, inst); err != nil {
t.Errorf("pong: failed to update: %v", err) t.Errorf("pong: failed to update: %v", err)
} }
if stored := db.lastPong(node.ID); stored.Unix() != inst.Unix() { if stored := db.bondTime(node.ID); stored.Unix() != inst.Unix() {
t.Errorf("pong: value mismatch: have %v, want %v", stored, inst) t.Errorf("pong: value mismatch: have %v, want %v", stored, inst)
} }
// Check fetch/store operations on a node findnode-failure object // Check fetch/store operations on a node findnode-failure object
@ -224,8 +224,8 @@ func TestNodeDBSeedQuery(t *testing.T) {
if err := db.updateNode(seed.node); err != nil { if err := db.updateNode(seed.node); err != nil {
t.Fatalf("node %d: failed to insert: %v", i, err) t.Fatalf("node %d: failed to insert: %v", i, err)
} }
if err := db.updateLastPong(seed.node.ID, seed.pong); err != nil { if err := db.updateBondTime(seed.node.ID, seed.pong); err != nil {
t.Fatalf("node %d: failed to insert lastPong: %v", i, err) t.Fatalf("node %d: failed to insert bondTime: %v", i, err)
} }
} }
@ -332,8 +332,8 @@ func TestNodeDBExpiration(t *testing.T) {
if err := db.updateNode(seed.node); err != nil { if err := db.updateNode(seed.node); err != nil {
t.Fatalf("node %d: failed to insert: %v", i, err) t.Fatalf("node %d: failed to insert: %v", i, err)
} }
if err := db.updateLastPong(seed.node.ID, seed.pong); err != nil { if err := db.updateBondTime(seed.node.ID, seed.pong); err != nil {
t.Fatalf("node %d: failed to update pong: %v", i, err) t.Fatalf("node %d: failed to update bondTime: %v", i, err)
} }
} }
// Expire some of them, and check the rest // Expire some of them, and check the rest
@ -365,8 +365,8 @@ func TestNodeDBSelfExpiration(t *testing.T) {
if err := db.updateNode(seed.node); err != nil { if err := db.updateNode(seed.node); err != nil {
t.Fatalf("node %d: failed to insert: %v", i, err) t.Fatalf("node %d: failed to insert: %v", i, err)
} }
if err := db.updateLastPong(seed.node.ID, seed.pong); err != nil { if err := db.updateBondTime(seed.node.ID, seed.pong); err != nil {
t.Fatalf("node %d: failed to update pong: %v", i, err) t.Fatalf("node %d: failed to update bondTime: %v", i, err)
} }
} }
// Expire the nodes and make sure self has been evacuated too // Expire the nodes and make sure self has been evacuated too

View File

@ -455,7 +455,7 @@ func (tab *Table) loadSeedNodes(bond bool) {
} }
for i := range seeds { for i := range seeds {
seed := seeds[i] seed := seeds[i]
age := log.Lazy{Fn: func() interface{} { return time.Since(tab.db.lastPong(seed.ID)) }} age := log.Lazy{Fn: func() interface{} { return time.Since(tab.db.bondTime(seed.ID)) }}
log.Debug("Found seed node in database", "id", seed.ID, "addr", seed.addr(), "age", age) log.Debug("Found seed node in database", "id", seed.ID, "addr", seed.addr(), "age", age)
tab.add(seed) tab.add(seed)
} }
@ -596,7 +596,7 @@ func (tab *Table) bond(pinged bool, id NodeID, addr *net.UDPAddr, tcpPort uint16
} }
// Start bonding if we haven't seen this node for a while or if it failed findnode too often. // Start bonding if we haven't seen this node for a while or if it failed findnode too often.
node, fails := tab.db.node(id), tab.db.findFails(id) node, fails := tab.db.node(id), tab.db.findFails(id)
age := time.Since(tab.db.lastPong(id)) age := time.Since(tab.db.bondTime(id))
var result error var result error
if fails > 0 || age > nodeDBNodeExpiration { if fails > 0 || age > nodeDBNodeExpiration {
log.Trace("Starting bonding ping/pong", "id", id, "known", node != nil, "failcount", fails, "age", age) log.Trace("Starting bonding ping/pong", "id", id, "known", node != nil, "failcount", fails, "age", age)
@ -663,7 +663,7 @@ func (tab *Table) ping(id NodeID, addr *net.UDPAddr) error {
if err := tab.net.ping(id, addr); err != nil { if err := tab.net.ping(id, addr); err != nil {
return err return err
} }
tab.db.updateLastPong(id, time.Now()) tab.db.updateBondTime(id, time.Now())
return nil return nil
} }

View File

@ -613,7 +613,7 @@ func (req *findnode) handle(t *udp, from *net.UDPAddr, fromID NodeID, mac []byte
if expired(req.Expiration) { if expired(req.Expiration) {
return errExpired return errExpired
} }
if t.db.node(fromID) == nil { if !t.db.hasBond(fromID) {
// No bond exists, we don't process the packet. This prevents // No bond exists, we don't process the packet. This prevents
// an attack vector where the discovery protocol could be used // an attack vector where the discovery protocol could be used
// to amplify traffic in a DDOS attack. A malicious actor // to amplify traffic in a DDOS attack. A malicious actor

View File

@ -247,12 +247,8 @@ func TestUDP_findnode(t *testing.T) {
// ensure there's a bond with the test node, // ensure there's a bond with the test node,
// findnode won't be accepted otherwise. // findnode won't be accepted otherwise.
test.table.db.updateNode(NewNode( test.table.db.updateBondTime(PubkeyID(&test.remotekey.PublicKey), time.Now())
PubkeyID(&test.remotekey.PublicKey),
test.remoteaddr.IP,
uint16(test.remoteaddr.Port),
99,
))
// check that closest neighbors are returned. // check that closest neighbors are returned.
test.packetIn(nil, findnodePacket, &findnode{Target: testTarget, Expiration: futureExp}) test.packetIn(nil, findnodePacket, &findnode{Target: testTarget, Expiration: futureExp})
expected := test.table.closest(targetHash, bucketSize) expected := test.table.closest(targetHash, bucketSize)