add gossip validation for dc, and data column quarantine strategy #6581
Conversation
| from std/strutils import join | ||
|
|
||
| const | ||
| MaxDataColumns = 3 * SLOTS_PER_EPOCH * NUMBER_OF_COLUMNS |
There was a problem hiding this comment.
Doubt regarding MaxDataColumns, I think we should bump this number up (?)
| quarantine.data_columns.del(oldest_column_key) | ||
| let block_root = | ||
| hash_tree_root(dataColumnSidecar.signed_block_header.message) | ||
| discard quarantine.data_columns.hasKeyOrPut( |
There was a problem hiding this comment.
hasKeyOrPut means that a (block_root, dataColumnSidecar.index) pair being inserted into the quarantine blocks new DataColumnSidecars for that same (block_root, dataColumnSidecar.index) pair blocks future matching keys.
Does this enable a denial of service attack? It seems like based on https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.6/specs/_features/eip7594/p2p-interface.md#data_column_sidecar_subnet_id there are three conditions relevant:
- [REJECT] The sidecar's
kzg_commitmentsfield inclusion proof is valid as verified byverify_data_column_sidecar_inclusion_proof(sidecar).- [REJECT] The sidecar's column data is valid as verified by
verify_data_column_sidecar_kzg_proofs(sidecar).- [IGNORE] The sidecar is the first sidecar for the tuple
(block_header.slot, block_header.proposer_index, sidecar.index)with valid header signature, sidecar inclusion proof, and kzg proof.
Taken together, the first two authenticate that the KZG blobs apply to the block (but, foreshadowing, not vice versa anywhere I can find) and that the columns map to the blobs, respectively.
But an attacker can create arbitrary fake blobs, tie them to the block_root they see via gossip, and then construct (valid) proofs, commitments, columns, and the rest from them and race to send the resulting columns so they're received sooner than the legitimate columns.
The gossip validation conditions have no cryptographic way to distinguish between the legitimacy of the two, and so allow it through to this quarantine codepath, where it then sits there and blocks legitimate columns' arrivals, because it's only keyed on (block_root, dataColumnSidecar.index), rather than the full-3-tuple-equivalent:
- [IGNORE] The sidecar is the first sidecar for the tuple
(block_header.slot, block_header.proposer_index, sidecar.index)with valid header signature, sidecar inclusion proof, and kzg proof.
which includes the proposer_index. Eventually, the attack-columns will get pushed out of the quarantine, and it might recover, but a persistent attacker can keep spamming them.
| quarantine: var DataColumnQuarantine, digest: Eth2Digest, | ||
| blck: deneb.SignedBeaconBlock | electra.SignedBeaconBlock): | ||
| seq[ref DataColumnSidecar] = | ||
| var r: seq[ref DataColumnSidecar] |
There was a problem hiding this comment.
Both seq[ref DataColumnSidecar] are exactly DataColumnSidecars:
No description provided.