Replay protection wrong hash

[grarco]

Thanks to @Fraccaman, we found out that the new implementation of replay protection implemented in #1280 manages the wrapper tx correctly but fails to safely protect the inner tx. This is because we are currently taking the hash of the header which could be easily changed by modifying some less important fields (like the timestamp) to force multiple applications of the same wasm code and the associated data.

To fix it we should simply save in storage the hash of the data (which is already available in the header) together with the hash of the entire header (to protect the wrapper).

[grarco]

@murisi

[murisi]

@grarco @Fraccaman Nice catch! My bad, I hadn't thought enough about the interaction between replay protection and the new transaction structure.

You're solution seems to be the most straightforward, but I guess we need to account for an attacker maliciously replaying ExtraData sections, or maybe other sections? Maybe instead of saving just the hash of the data occurring in the header, we could save the target fields of each Signature section occurring in the transaction? Doing this would cover for the replay of any of the signed sections including ExtraData, Data, and the header.

[Fraccaman]

@murisi we could do that for sure, but I'm concern about the fact that we would include too many data into storage (replay protection is already taking a lot of space). Maybe some of those section dont need to be replay protected? For example, the Code section ?

[murisi]

@Fraccaman @grarco Agreed. We discussed elsewhere that maybe the Code section may not need to be signed. If this were the case, then there would be no Signature section for the Code section, and according to the above algorithm the Code section would not be replay protected. More generally I mean to say that only signed sections would be replay protected. But I guess even this might be undesirable because a Tx may have more than 2 signed sections (meaning more than 2 keys in replay protection storage).

What if we instead generate a single nonce for each transaction and include it in each transaction section? Network nodes would then put that nonce as a key into replay protection storage. This way: transaction sections are unreplayable, at most 2 entries are put into replay protection storage for a single transaction, and there would be no way for an attacker to make a transaction by recombining sections from other transactions (which may not have been played yet). This approach would bind all the sections of a transaction together and prevent even the signer of the wrapper transaction from, say, changing the code whilst leaving the data (signed by someone else) unchanged.

[grarco]

So, this could actually work. My concerns are:

1. The amount of work required to change the entire replay protection mechanism
2. Some issues/characteristics of this design that we encountered in the initial design phase, like:

• The need for reordering txs in prepare_proposal and process_proposal to guarantee the correct logic
• If the nonce is encrypted aswell we can only check the validity of it after decryption (next block) but by this time fees have already been paid
• the client must keep track of the txs' results to craft a new valid tx

I was thinking that we might save the current hash-based replay protection mechanism by adding another Section in the tx (which could be a new variant of the Section enum): this section should contain the hash produced from the concatenation of the lexicographically sorted hashes of all the non-signature sections in the tx (note that sorting is required cause, otherwise, simply rearranging the sections in the vec would be enough to create a new hash). This hash should be computed on the sections in plaintext. This single hash is the one we need to save in storage to protect the sections from a replay attack. This section must be kept in plaintext to allow evaluation in mempool, prepare_proposal and process_proposal. It also needs to be signed and the signature must be encrypted to prevent leakage of the public key.

Since we also save in storage the hash of the header, we'll be back at two hashes in storage which is what we used to have and seems to be fine in terms of space occupation.

When we receive the tx we can check whether any of these hashes is already in storage and reject the tx as a replay attempt. If instead it's valid, we then decrypt the sections in the following block. Here we must check (in VP) that the signature over this "cumulated" hash is valid and that the hash itself is correct given the sections of the transaction. We also need to check that the signature on the hash comes from the same public key that signed the Data and Code sections. This way, not even the wrapper's signer can tamper with the sections. The only way to do so would be to use the same exact sections (included the hash section) of a previously applied transaction, but this would be a replay attack and the check on the hash would prevent this tx from being included into a block

[grarco]

Actually, the commitment hash could be better place inside the Header while we keep its signature in one of the sections. This way we don't need to loop on the vector of sections to look for this hash, this also seems to play well with #1683