Re registering attack protection #27
Conversation
liorrutenberg
requested review from
mtabasco,
arielssv,
moshe-blox and
alonmuroch
|
Nonce should be removed from the register function, cause the node can validate it |
| @@ -0,0 +1,95 @@ | |||
| | Author | Title | Category | Status | | |||
| |-----------------------------| ------------------------------- | -------- | ------------------- | | |||
| | Lior Rutenberg (@lior-blox) | SSV Register Cluster copied shares attack | Core | open-for-discussion | | |||
There was a problem hiding this comment.
category should be contracts + core
| In both scenarios, a validator would be operating on the SSV network under the control of a harmful user instead of the rightful validator owner. | ||
|
|
||
| #### Front-Running | ||
| In this scenario, a malicious actor could "front-run" a genuine user trying to register a validator. The registration would appear as legitimate. To keep the attack ongoing, the attacker would cover the validator's fees. |
There was a problem hiding this comment.
"an honest" instead of genuine
|
|
||
| The BLS signature's authenticity will be confirmed by the SSV nodes, which will use the validator's public key and ensure that the nonce value is greater than the previous one. Instead of being on a validator level, the nonce will be on an account level to prevent scenarios where a validator is deregistered and re-registered by the same user. | ||
|
|
||
| The decision regarding whether the nonce should be stored on the contract or solely on the SSV nodes still needs to be made. |
There was a problem hiding this comment.
the SIP is not leaving open questions, let's decided where the nonce is stored and update the SIP
There was a problem hiding this comment.
I removed this.
we clarified it already in the node spec part
|
|
||
| To make any changes (such as removing or changing the cluster), the genuine user would need to exit the validator. | ||
|
|
||
| #### Re-registering attack |
There was a problem hiding this comment.
add also removing from a different cluster section explaining it
There was a problem hiding this comment.
Added in contract section
There was a problem hiding this comment.
If you want to use crypto lingo, then you can call it a "replay attack" or registration replay.
Just a nit
| 2. Altering the validator map keys within the contract to include the address along with the public key. | ||
| 3. Implementing signature verification within the SSV nodes during the validator registration process. | ||
|
|
||
| > Please be aware: A new contract deployment will be necessary to modify the validator map. |
There was a problem hiding this comment.
what is irrelevant, the note?
| > Please be aware: A new contract deployment will be necessary to modify the validator map. | ||
| > Furthermore, the uniqueness of a public key will be associated with each individual address, rather than being unique across the entire contract. | ||
|
|
||
| ### SSV Keys |
There was a problem hiding this comment.
what do you mean by SSV keys? cli changes do not belong to an SIP
|
@lior-blox update? |
| | [4](./sips/change_operator.md) | Change operators set | open-for-discussion | | ||
| | [5](./sips/ecies_share_encryption.md) | ECIES Share Encryption | open-for-discussion | | ||
| | [6](./sips/re_registering_attack_protection.md) | Register Cluster copied shares attack | open-for-discussion | |
There was a problem hiding this comment.
it's 9 not 6, @lior-blox please make changes
| ``` | ||
| To: | ||
| ```solidity | ||
| struct Validator { |
There was a problem hiding this comment.
@lior-blox should I push a commit with the current implementation code?
/// @notice Maps each validator's public key to its hashed representation of: operator Ids used by the validator and active / inactive flag (uses LSB)
mapping(bytes32 => bytes32) validatorPKs;
// example of setting the mapping
bytes32 hashedPk = keccak256(abi.encodePacked(publicKey, msg.sender));
validatorPKs[hashedPk] = bytes32(uint256(keccak256(abi.encodePacked(operatorIds))) | uint256(0x01)); // set LSB to 1
There was a problem hiding this comment.
@lior-blox I left a comment about including the current implementation solidity code in the example section. thanks
No description provided.