satellite/overlay: don't insert new nodes if contact check failed

Currently, the satellite tracks connectivity information about all nodes
that have contacted it, even if we have never successfully contacted
the node back.

This behavior was leveraged during a security audit to create hundreds
of thousands of "junk nodes" in the nodes table on one satellite, which
affected performance of queries such as node selection.

With this change, we should no longer track information about nodes that
have never been successfully contacted.

Note that it will still be possible to cause the creation of "junk
node" entries in the db; the attacker just has to set up individual
publicly-routable IP+port pairs for each node as it is created, so it
can respond to a PingBack.

Change-Id: Ibb6da6cc908fd4fc85aae1ba00313ba2738409ab
This commit is contained in:
paul cannon 2022-08-23 17:21:33 -05:00 committed by Storj Robot
parent 5a5f609022
commit 07bbe7d340

View File

@ -560,6 +560,12 @@ func (service *Service) UpdateCheckIn(ctx context.Context, node NodeCheckInInfo,
}
if oldInfo == nil {
if !node.IsUp {
// this is a previously unknown node, and we couldn't pingback to verify that it even
// exists. Don't bother putting it in the db.
return nil
}
node.CountryCode, err = service.GeoIP.LookupISOCountryCode(node.LastIPPort)
if err != nil {
failureMeter.Mark(1)