Fix network sharding eclipse attack #1201
Conversation
iulianpascalau
added
type:bug
cmd/node/config/config.toml
Outdated
| @@ -227,16 +227,29 @@ | |||
| Size = 75000 | |||
| Type = "LRU" | |||
|
|
|||
| [PublicKeyShardId] | |||
| #PublicKeyShardId represents the main cache used to map Elrond block signing public keys to their associated | |||
There was a problem hiding this comment.
PublicKeyShardId -> PublicKeyPeerID
The long explanation of the logic behind this parameter is unnecessary here, please keep it simple
| } | ||
|
|
||
| func (pq *pidQueue) pop() p2p.PeerID { | ||
| evicted := pq.data[0] |
There was a problem hiding this comment.
there is no len check, this could give index out of bounds
There was a problem hiding this comment.
in order to keep this unexported struct as simple as possible, the check has been introduced before calling pop (UpdatePeerIdPublicKey function, L179)
| @@ -76,6 +90,7 @@ func (psm *PeerShardMapper) byIDWithNodesCoordinator(pid p2p.PeerID) (shardId ui | |||
|
|
|||
| pkBuff, ok := pkObj.([]byte) | |||
| if !ok { | |||
| psm.peerIdPk.Remove([]byte(pid)) | |||
There was a problem hiding this comment.
why is this getter doing a remove?
please don't change values in getters.
this change is also done without acquiring the mutex which will lead to race conditions.
There was a problem hiding this comment.
removed, added log.Warn as it is a programming error
| @@ -99,6 +114,7 @@ func (psm *PeerShardMapper) byIDSearchingPkInFallbackCache(pkBuff []byte) (shard | |||
|
|
|||
| shard, ok := shardObj.(uint32) | |||
| if !ok { | |||
| psm.fallbackPkShard.Remove(pkBuff) | |||
There was a problem hiding this comment.
same as previous comment
There was a problem hiding this comment.
removed, added log.Warn as it is a programming error
| @@ -113,15 +129,86 @@ func (psm *PeerShardMapper) byIDSearchingPidInFallbackCache(pid p2p.PeerID) (sha | |||
|
|
|||
| shard, ok := shardObj.(uint32) | |||
| if !ok { | |||
| psm.fallbackPidShard.Remove([]byte(pid)) | |||
There was a problem hiding this comment.
same as previous comment, please do not remove on getter
There was a problem hiding this comment.
removed, added log.Warn as it is a programming error
| } | ||
|
|
||
| idxPid := pq.indexOf(pid) | ||
| if idxPid > -1 { |
There was a problem hiding this comment.
please create a constant for -1 e.g idxNotFound and check against that ( != idxNotFound ).
you can also use maxint and make the type unsigned.
There was a problem hiding this comment.
done, added const
…at should not happen
No description provided.