From 5801b4fad3c772a47d4108093d3b7607cfb7339f Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 5 Sep 2022 15:59:34 +0200 Subject: [PATCH 01/18] Replay protection specs first draft --- documentation/specs/src/SUMMARY.md | 1 + .../src/base-ledger/replay-protection.md | 98 +++++++++++++++++++ 2 files changed, 99 insertions(+) create mode 100644 documentation/specs/src/base-ledger/replay-protection.md diff --git a/documentation/specs/src/SUMMARY.md b/documentation/specs/src/SUMMARY.md index b7537eb315..94c6390400 100644 --- a/documentation/specs/src/SUMMARY.md +++ b/documentation/specs/src/SUMMARY.md @@ -8,6 +8,7 @@ - [Default account](./base-ledger/default-account.md) - [Multisignature account](./base-ledger/multisignature.md) - [Fungible token](./base-ledger/fungible-token.md) + - [Replay protection](./base-ledger/replay-protection.md) - [Multi-asset shielded pool](./masp.md) - [Ledger integration](./masp/ledger-integration.md) - [Asset type](./masp/asset-type.md) diff --git a/documentation/specs/src/base-ledger/replay-protection.md b/documentation/specs/src/base-ledger/replay-protection.md new file mode 100644 index 0000000000..2b94c1e4f1 --- /dev/null +++ b/documentation/specs/src/base-ledger/replay-protection.md @@ -0,0 +1,98 @@ +# Replay Protection + +Namada supports a replay protection mechanism to prevent the execution of already processed transactions. + +## Tendermint + +[Tendermint]((https://docs.tendermint.com/v0.33/app-dev/app-development.html#replay-protection)) provides a first layer of protection against replay attacks in its mempool. The mechanism is based on a cache of previously seen transactions. This check though, like all the checks performed in `CheckTx`, is weak, since a malicious validator could always propose a block containing invalid transactions. There's therefore the need for a more robust replay protection mechanism implemented directly in the application. + +## WrapperTx + +`WrapperTx` is the only type of transaction currently accepted by the ledger. It must be protected from replay attacks because, if it wasn't, a malicious user could replay the transaction as is. Even if the inner transaction implemented replay protection or, for any reason, wasn't accepted, the signer of the wrapper would still pay for gas and fees, effectively suffering economic damage. + +To protect this transaction we can implement an in-protocol mechanism that requires us to expand the current definition of the struct to include a transaction counter: + +```rust +pub struct WrapperTx { + /// The fee to be payed for including the tx + pub fee: Fee, + /// Used to determine an implicit account of the fee payer + pub pk: common::PublicKey, + /// The epoch in which the tx is to be submitted. This determines + /// which decryption key will be used + pub epoch: Epoch, + /// Max amount of gas that can be used when executing the inner tx + pub gas_limit: GasLimit, + /// Transaction counter for replay-protection + pub tx_counter: u32, + /// the encrypted payload + pub inner_tx: EncryptedTx, + /// sha-2 hash of the inner transaction acting as a commitment + /// the contents of the encrypted payload + pub tx_hash: Hash, +} +``` + +In addition, we need a counter in the storage subspace of every implict address: + +``` +/$Address/tx_counter: u32 +``` + +In `process_proposal` we check that `tx_counter` field in `WrapperTx` is greater or equal to the value currently in storage; if this doesn't hold, the transaction is considered invalid. + +At last, in `finalize_block`, the protocol updates the counter key in storage, increasing its value by one. Now, if a malicious user tried to replay this transaction, the `tx_counter` in the struct would no longer be greater or equal to the one in storage and the transaction would be deemed invalid. + +Note that the address whose counter is involved in this process is the one signing the transaction (the same as the `pk` field of the struct). + +## InnerTx + +The `EncryptedTx` incapsulated inside `WrapperTx` should be protected too. That's because, if it wasn't, an attacker could extract the inner tx, rewrap it, and replay it.\ +To simplify this check we will perform it entirely in Wasm: the check of the counter will be carryed out by the validity predicates while the actual writing of the counter in storage will be done by the transactions themselves. + +To do so, the `SignedTxData` attached to the transaction will hold the current value of the counter in storage. The transaction will simply read the value from storage and increase its value by one. The VP will then check the validity of the signature and, if it's deemed valid, will proceed to checking if the pre value of the counter in storage was less or equal than the one contained in the transaction data and if the post value of the key in storage has been incremented by one. + +In the specific case of a transfer, since MASP already comes with replay protection as part of the Zcash design (see the [MASP specs](https://specs.namada.net/masp.html) and [Zcash protocol specs](https://zips.z.cash/protocol/protocol.pdf)), the counter in `SignedTxData` should be optional. + +An alternative implementation could place the protection for the inner tx in protocol, just like the wrapper one. In this case we would need to extend the `Tx` struct to hold an additional optional field (again, because of MASP) for the transaction counter and the address involved in replay protection, like so: + +```rust +pub struct Tx { + pub code: Vec, + pub data: Option>, + pub timestamp: DateTimeUtc, + pub tx_counter: Option<(Address, u32)> +} +``` + +The check would run in `process_proposal` and the update in `finalize_block`, just like for the wrapper transaction. The drawback of this implementation is that it implies the need for an hard fork in case of a modification of the replay protection mechanism. + +### VPs and Txs + +To implement replay protection for the inner transaction we will need to update all the VPs checking the transaction's signature to include the check on the transaction counter: at the moment the `vp_user` validity predicate is the only one to update. + +In addition, all the transactions involving `SignedTxData` should increment the counter. + +## Single counter + +The mechanisms to prevent replay attacks for both transactions (wrapper and inner) will share the same `tx_counter` key in storage. + +This could be an issue when the signer of the `WrapperTx` is the same of the `InnerTx` (default behaviour of the client): in this case, if both transactions expect the same counter in storage, the wrapper transaction will pass validation but the inner one will fail. To cope with this, the client will be responsible for producing a valid `WrapperTx` in which `tx_counter` will be set to the current value of the counter in storage, call it `storage_ctr`, while the inner transaction will have `storage_ctr + 1` in its data. + +## Storage Interface + +Replay protection will require interaction with the storage from both the protocol and Wasm. To do so we can take advantage of the `StorageRead` and `StorageWrite` traits to work with a single interface. + +## Batching + +The implementation proposed in this document doesn't support batching of multiple transactions from a same address in a single block. This is because the order in which transactions will be included in the block by the proposer is not guarateed. An out of order execution of multiple transaction would lead to the failure of some of them (in the worst case, the failure of all of them but the first one executed, in the best case, the failure of only the last transaction). This problem will be amplified by the introduction of Ferveo for DKG which will be able to reorder transactions. + +The Wasm implementation of replay protection can't cope with this problem because every wasm run (of either a transaction or a validity predicate) is only aware of its context, meaning the wasm bytecode and the serialized transaction data. The lack of awareness of the other transactions makes it impossible to develop a replay protection mechanism supporting batching in wasm. + +To address this issue there could be two ways: + +- Keep the proposed replay protection mechanism and implement a batching mechanism in the client to embed more than one transaction in a single `Tx` struct +- Implement replay protection in protocol for the inner transaction (as discolsed in section [InnerTx](#InnerTx)) + +Following the second option, the ledger would be able to analyze the validity of the counters, of all the transcations relating to a single address, against the value in storage at the beginning of the block. +Finally, it could increment the counter in storage a single time by the correct amount (given by the amount of transactions that were executed with success). From 9fd2bfe7f5a44185f5adc786f671ec340448c074 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 5 Sep 2022 18:13:18 +0200 Subject: [PATCH 02/18] Review batching --- documentation/specs/src/base-ledger/replay-protection.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/documentation/specs/src/base-ledger/replay-protection.md b/documentation/specs/src/base-ledger/replay-protection.md index 2b94c1e4f1..e976d29a08 100644 --- a/documentation/specs/src/base-ledger/replay-protection.md +++ b/documentation/specs/src/base-ledger/replay-protection.md @@ -91,8 +91,10 @@ The Wasm implementation of replay protection can't cope with this problem becaus To address this issue there could be two ways: -- Keep the proposed replay protection mechanism and implement a batching mechanism in the client to embed more than one transaction in a single `Tx` struct +- Keep the proposed replay protection in Wasm and implement a batching mechanism in both the client and the ledger to embed more than one transaction in a single `Tx` struct - Implement replay protection in protocol for the inner transaction (as discolsed in section [InnerTx](#InnerTx)) Following the second option, the ledger would be able to analyze the validity of the counters, of all the transcations relating to a single address, against the value in storage at the beginning of the block. Finally, it could increment the counter in storage a single time by the correct amount (given by the amount of transactions that were executed with success). + +The first option, though, seems to have more benefits. In addition to allowing batching, it's more flexible and it also enables the in-order execution of the transactions included in the batch, which may come in handy in certain cases. From 7b152671ab766ba1fd1284119db3c4c8a2dd8317 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 6 Sep 2022 11:38:02 +0200 Subject: [PATCH 03/18] Fixes typos, rephrase batching --- documentation/specs/src/base-ledger/replay-protection.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/documentation/specs/src/base-ledger/replay-protection.md b/documentation/specs/src/base-ledger/replay-protection.md index e976d29a08..8e9ddea036 100644 --- a/documentation/specs/src/base-ledger/replay-protection.md +++ b/documentation/specs/src/base-ledger/replay-protection.md @@ -85,7 +85,7 @@ Replay protection will require interaction with the storage from both the protoc ## Batching -The implementation proposed in this document doesn't support batching of multiple transactions from a same address in a single block. This is because the order in which transactions will be included in the block by the proposer is not guarateed. An out of order execution of multiple transaction would lead to the failure of some of them (in the worst case, the failure of all of them but the first one executed, in the best case, the failure of only the last transaction). This problem will be amplified by the introduction of Ferveo for DKG which will be able to reorder transactions. +The implementation proposed in this document doesn't support batching of multiple transactions from a same address in a single block. More specifically, the transactions will all succeed only if they are executed in the intended order, but the order in which transactions will be included in the block by the proposer is not guaranteed. An out of order execution of multiple transactions would lead to the failure of some of them (in the worst case, the failure of all of them but the first one executed, in the best case, the failure of only the last transaction). This problem will be amplified by the introduction of Ferveo for DKG which will be able to reorder transactions. The Wasm implementation of replay protection can't cope with this problem because every wasm run (of either a transaction or a validity predicate) is only aware of its context, meaning the wasm bytecode and the serialized transaction data. The lack of awareness of the other transactions makes it impossible to develop a replay protection mechanism supporting batching in wasm. @@ -94,7 +94,7 @@ To address this issue there could be two ways: - Keep the proposed replay protection in Wasm and implement a batching mechanism in both the client and the ledger to embed more than one transaction in a single `Tx` struct - Implement replay protection in protocol for the inner transaction (as discolsed in section [InnerTx](#InnerTx)) -Following the second option, the ledger would be able to analyze the validity of the counters, of all the transcations relating to a single address, against the value in storage at the beginning of the block. +Following the second option, the ledger would be able to analyze the validity of the counters, of all the transactions relating to a single address, against the value in storage at the beginning of the block. Finally, it could increment the counter in storage a single time by the correct amount (given by the amount of transactions that were executed with success). The first option, though, seems to have more benefits. In addition to allowing batching, it's more flexible and it also enables the in-order execution of the transactions included in the batch, which may come in handy in certain cases. From eb3a937a5113d840c147d6874383f1b01ff2c5f8 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Wed, 7 Sep 2022 13:08:19 +0200 Subject: [PATCH 04/18] Fixes `tx_counter` type, check logic and drawbacks --- .../src/base-ledger/replay-protection.md | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/documentation/specs/src/base-ledger/replay-protection.md b/documentation/specs/src/base-ledger/replay-protection.md index 8e9ddea036..cef66e549d 100644 --- a/documentation/specs/src/base-ledger/replay-protection.md +++ b/documentation/specs/src/base-ledger/replay-protection.md @@ -24,7 +24,7 @@ pub struct WrapperTx { /// Max amount of gas that can be used when executing the inner tx pub gas_limit: GasLimit, /// Transaction counter for replay-protection - pub tx_counter: u32, + pub tx_counter: u64, /// the encrypted payload pub inner_tx: EncryptedTx, /// sha-2 hash of the inner transaction acting as a commitment @@ -36,12 +36,12 @@ pub struct WrapperTx { In addition, we need a counter in the storage subspace of every implict address: ``` -/$Address/tx_counter: u32 +/$Address/tx_counter: u64 ``` -In `process_proposal` we check that `tx_counter` field in `WrapperTx` is greater or equal to the value currently in storage; if this doesn't hold, the transaction is considered invalid. +In `process_proposal` we check that `tx_counter` field in `WrapperTx` is equal to the value currently in storage; if this doesn't hold, the transaction is considered invalid. -At last, in `finalize_block`, the protocol updates the counter key in storage, increasing its value by one. Now, if a malicious user tried to replay this transaction, the `tx_counter` in the struct would no longer be greater or equal to the one in storage and the transaction would be deemed invalid. +At last, in `finalize_block`, the protocol updates the counter key in storage, increasing its value by one. Now, if a malicious user tried to replay this transaction, the `tx_counter` in the struct would no longer be equal to the one in storage and the transaction would be deemed invalid. Note that the address whose counter is involved in this process is the one signing the transaction (the same as the `pk` field of the struct). @@ -50,7 +50,7 @@ Note that the address whose counter is involved in this process is the one signi The `EncryptedTx` incapsulated inside `WrapperTx` should be protected too. That's because, if it wasn't, an attacker could extract the inner tx, rewrap it, and replay it.\ To simplify this check we will perform it entirely in Wasm: the check of the counter will be carryed out by the validity predicates while the actual writing of the counter in storage will be done by the transactions themselves. -To do so, the `SignedTxData` attached to the transaction will hold the current value of the counter in storage. The transaction will simply read the value from storage and increase its value by one. The VP will then check the validity of the signature and, if it's deemed valid, will proceed to checking if the pre value of the counter in storage was less or equal than the one contained in the transaction data and if the post value of the key in storage has been incremented by one. +To do so, the `SignedTxData` attached to the transaction will hold the current value of the counter in storage. The transaction will simply read the value from storage and increase its value by one. The VP will then check the validity of the signature and, if it's deemed valid, will proceed to checking if the pre value of the counter in storage was equal to the one contained in the transaction data and if the post value of the key in storage has been incremented by one. In the specific case of a transfer, since MASP already comes with replay protection as part of the Zcash design (see the [MASP specs](https://specs.namada.net/masp.html) and [Zcash protocol specs](https://zips.z.cash/protocol/protocol.pdf)), the counter in `SignedTxData` should be optional. @@ -61,11 +61,14 @@ pub struct Tx { pub code: Vec, pub data: Option>, pub timestamp: DateTimeUtc, - pub tx_counter: Option<(Address, u32)> + pub tx_counter: Option<(Address, u64)> } ``` -The check would run in `process_proposal` and the update in `finalize_block`, just like for the wrapper transaction. The drawback of this implementation is that it implies the need for an hard fork in case of a modification of the replay protection mechanism. +The check would run in `process_proposal` and the update in `finalize_block`, just like for the wrapper transaction. This implementation, though, shows two drawbacks: + +- it implies the need for an hard fork in case of a modification of the replay protection mechanism +- it's not clear who's the source of the inner transaction from the outside, as that depends on the specific code of the transaction itself. We could use specific whitelisted txs set to define when it requires a counter (would not work for future programmable transactions), but still, we have no way to define which address should be targeted for replay protection (**blocking issue**) ### VPs and Txs @@ -85,7 +88,7 @@ Replay protection will require interaction with the storage from both the protoc ## Batching -The implementation proposed in this document doesn't support batching of multiple transactions from a same address in a single block. More specifically, the transactions will all succeed only if they are executed in the intended order, but the order in which transactions will be included in the block by the proposer is not guaranteed. An out of order execution of multiple transactions would lead to the failure of some of them (in the worst case, the failure of all of them but the first one executed, in the best case, the failure of only the last transaction). This problem will be amplified by the introduction of Ferveo for DKG which will be able to reorder transactions. +The implementation proposed in this document doesn't support batching of multiple transactions from a same address in a single block. More specifically, the transactions will all succeed only if they are executed in the intended order, but the order in which transactions will be included in the block by the proposer is not guaranteed. An out of order execution of multiple transactions would lead to the failure of some of them (in the worst case, the failure of all of them but one, in the best case, the failure of only one). This problem will be amplified by the introduction of Ferveo for DKG which will be able to reorder transactions. The Wasm implementation of replay protection can't cope with this problem because every wasm run (of either a transaction or a validity predicate) is only aware of its context, meaning the wasm bytecode and the serialized transaction data. The lack of awareness of the other transactions makes it impossible to develop a replay protection mechanism supporting batching in wasm. @@ -95,6 +98,6 @@ To address this issue there could be two ways: - Implement replay protection in protocol for the inner transaction (as discolsed in section [InnerTx](#InnerTx)) Following the second option, the ledger would be able to analyze the validity of the counters, of all the transactions relating to a single address, against the value in storage at the beginning of the block. -Finally, it could increment the counter in storage a single time by the correct amount (given by the amount of transactions that were executed with success). +Finally, it could increment the counter in storage a single time by the correct amount (given by the amount of transactions that were executed with success). As already mentioned, though, this implementation suffers from some serious issues and seems to not be feasible. -The first option, though, seems to have more benefits. In addition to allowing batching, it's more flexible and it also enables the in-order execution of the transactions included in the batch, which may come in handy in certain cases. +The first option, instead, carries more benefits, since in addition to allowing batching, it's also more flexible. From d03f2bb1c835cc427e1e4b5d589d2a8217f3ec89 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Fri, 30 Sep 2022 18:27:33 +0200 Subject: [PATCH 05/18] Updates replay protection specs --- .../src/base-ledger/replay-protection.md | 258 +++++++++++++++--- 1 file changed, 220 insertions(+), 38 deletions(-) diff --git a/documentation/specs/src/base-ledger/replay-protection.md b/documentation/specs/src/base-ledger/replay-protection.md index cef66e549d..5cfb5d36fa 100644 --- a/documentation/specs/src/base-ledger/replay-protection.md +++ b/documentation/specs/src/base-ledger/replay-protection.md @@ -1,16 +1,24 @@ # Replay Protection -Namada supports a replay protection mechanism to prevent the execution of already processed transactions. +Replay protection is a mechanism to prevent _replay attacks_, which consist of a malicious user resubmitting an already executed transaction (also mentioned as tx in this document) to the ledger. -## Tendermint +A replay attack causes the state of the machine to deviate from the intended one (from the perspective of the parties involved in the original transaction) and causes economic damage to the fee payer of the original transaction, who finds himself paying more than once. Further economic damage is caused if the transaction involved the moving of value in some form (e.g. a transfer of tokens) with the sender being deprived of more value than intended. -[Tendermint]((https://docs.tendermint.com/v0.33/app-dev/app-development.html#replay-protection)) provides a first layer of protection against replay attacks in its mempool. The mechanism is based on a cache of previously seen transactions. This check though, like all the checks performed in `CheckTx`, is weak, since a malicious validator could always propose a block containing invalid transactions. There's therefore the need for a more robust replay protection mechanism implemented directly in the application. +Since the original transaction was already well formatted for the protocol's rules, the attacker doesn't need to rework it, making this attack relatively easy. -## WrapperTx +Of course, a replay attack makes sense only if the attacker differs from the _source_ of the original transaction, as a user will always be able to generate another semantically identical transaction to submit without the need to replay the same one. -`WrapperTx` is the only type of transaction currently accepted by the ledger. It must be protected from replay attacks because, if it wasn't, a malicious user could replay the transaction as is. Even if the inner transaction implemented replay protection or, for any reason, wasn't accepted, the signer of the wrapper would still pay for gas and fees, effectively suffering economic damage. +To prevent this scenario, Namada supports a replay protection mechanism to prevent the execution of already processed transactions. + +## Context -To protect this transaction we can implement an in-protocol mechanism that requires us to expand the current definition of the struct to include a transaction counter: +This section will illustrate the pre-existing context in which we are going to implement the replay protection mechanism. + +### Encryption-Authentication + +The current implementation of Namada is built on top of Tendermint which provides an encrypted and authenticated communication channel between every two nodes to prevent a _man-in-the-middle_ attack (see the detailed [spec](https://github.com/tendermint/tendermint/blob/29e5fbcc648510e4763bd0af0b461aed92c21f30/spec/p2p/peer.md)). + +The Namada protocol relies on this substrate to exchange transactions (messages) that will define the state transition of the ledger. More specifically, a transaction is composed of two parts: a `WrapperTx` and an inner `Tx` ```rust pub struct WrapperTx { @@ -23,81 +31,255 @@ pub struct WrapperTx { pub epoch: Epoch, /// Max amount of gas that can be used when executing the inner tx pub gas_limit: GasLimit, - /// Transaction counter for replay-protection - pub tx_counter: u64, /// the encrypted payload pub inner_tx: EncryptedTx, /// sha-2 hash of the inner transaction acting as a commitment /// the contents of the encrypted payload pub tx_hash: Hash, } + +pub struct Tx { + pub code: Vec, + pub data: Option>, + pub timestamp: DateTimeUtc, +} ``` -In addition, we need a counter in the storage subspace of every implict address: +The wrapper transaction is composed of some metadata, the encrypted inner transaction itself and the hash of this. The inner `Tx` transaction carries the Wasm code to be executed and the associated data. + +A transaction is constructed as follows: + +1. The struct `Tx` is produced +2. The hash of this transaction gets signed by the author, producing another `Tx` where the data field holds the concatenation of the original data and the signature (`SignedTxData`) +3. The produced transaction is encrypted and embedded in a `WrapperTx`. The encryption step is there for a future implementation of DKG (see [Ferveo](https://github.com/anoma/ferveo)) +4. Finally, the `WrapperTx` gets converted to a `Tx` struct, signed over its hash (same as step 2, relying on `SignedTxData`), and submitted to the network + +Note that the signer of the `WrapperTx` and that of the inner one don't need to coincide, but the signer of the wrapper will be charged with gas and fees. +In the execution steps: + +1. The `WrapperTx` signature is verified and, only if valid, the tx is processed +2. In the following height the proposer decrypts the inner tx, checks that the hash matches that of the `tx_hash` field and, if everything went well, includes the decrypted tx in the proposed block +3. The inner tx will then be executed by the Wasm runtime +4. After the execution, the affected validity predicates (also mentioned as VP in this document) will check the storage changes and (if relevant) the signature of the transaction: if the signature is not valid, the VP will deem the transaction invalid and the changes won't be applied to the storage + +The signature checks effectively prevent any tampering with the transaction data because that would cause the checks to fail and the transaction to be rejected. +For a more in-depth view, please refer to the [Namada execution spec](https://specs.namada.net/base-ledger/execution.html). + +### Tendermint replay protection + +The underlying consensus engine, [Tendermint](https://github.com/tendermint/tendermint/blob/29e5fbcc648510e4763bd0af0b461aed92c21f30/spec/abci/apps.md), provides a first layer of protection in its mempool which is based on a cache of previously seen transactions. This mechanism is actually aimed at preventing a block proposer from including an already processed transaction in the next block, which can happen when the transaction has been received late. Of course, this also acts as a countermeasure against intentional replay attacks. This check though, like all the checks performed in `CheckTx`, is weak, since a malicious validator could always propose a block containing invalid transactions. There's therefore the need for a more robust replay protection mechanism implemented directly in the application. + +## Implementation + +Namada replay protection consists of three parts: the in-Wasm solution for `EncryptedTx` (also called the `InnerTx`), an in-protocol mechanism for `WrapperTx` and a way to mitigate replay attacks in case of a fork. + +### InnerTx + +The actual Wasm code and data for the transaction are encapsulated inside a struct `Tx`, which gets encrypted as an `EncryptedTx` and wrapped inside a `WrapperTx` (see the [relative](#encryption-authentication) section). This inner transaction must be protected from replay attacks because it carries the actual semantics of the state transition. Moreover, even if the wrapper transaction was protected from replay attacks, an attacker could extract the inner transaction, rewrap it, and replay it. Note that for this attack to work, the attacker will need to sign the outer transaction himself and pay gas and fees for that, but this could still cause much greater damage to the parties involved in the inner transaction. + +We will implement the protection entirely in Wasm: the check of the counter will be carried out by the validity predicates while the actual writing of the counter in storage will be done by the transactions themselves. + +To do so, the `SignedTxData` attached to the transaction will hold the current value of the counter in storage: + +```rust +pub struct SignedTxData { + /// The original tx data bytes, if any + pub data: Option>, + /// The optional transaction counter for replay protection + pub tx_counter: Option, + /// The signature is produced on the tx data concatenated with the tx code + /// and the timestamp. + pub sig: common::Signature, +} +``` + +The counter must reside in `SignedTxData` and not in the data itself because this must be checked by the validity predicate which is not aware of the specific transaction that took place but only of the changes in the storage; therefore, the VP is not able to correctly deserialize the data of the transactions since it doesn't know what type of data the bytes represent. + +The counter will be signed as well to protect it from tampering and grant it the same guarantees explained at the [beginning](#encryption-authentication) of this document. + +The wasm transaction will simply read the value from storage and increase its value by one. The target key in storage will be the following: + +``` +/$Address/inner_tx_counter: u64 +``` + +The VP of the _source_ address will then check the validity of the signature and, if it's deemed valid, will proceed to check if the pre-value of the counter in storage was equal to the one contained in the `SignedTxData` struct and if the post-value of the key in storage has been incremented by one: if any of these conditions doesn't hold the VP will discard the transactions and prevent the changes from being applied to the storage. + +In the specific case of a transfer, since MASP already comes with replay protection as part of the Zcash design (see the [MASP specs](https://specs.namada.net/masp.html) and [Zcash protocol specs](https://zips.z.cash/protocol/protocol.pdf)), the counter in `SignedTxData` is not required and therefore should be optional. + +To implement replay protection for the inner transaction we will need to update all the VPs checking the transaction's signature to include the check on the transaction counter: at the moment the `vp_user` validity predicate is the only one to update. In addition, all the transactions involving `SignedTxData` should increment the counter. + +### WrapperTx + +`WrapperTx` is the only type of transaction currently accepted by the ledger. It must be protected from replay attacks because, if it wasn't, a malicious user could replay the transaction as is. Even if the inner transaction implemented replay protection (as explained in the [previous](#innertx) section) or, for any reason, wasn't accepted, the signer of the wrapper would still pay for gas and fees, effectively suffering economic damage. + +To protect this transaction we can implement an in-protocol mechanism. Since the wrapper transaction gets signed before being submitted to the network, we can leverage the `tx_counter` field of the `SignedTxData` already introduced for the inner tx. + +In addition, we need another counter in the storage subspace of every address: ``` -/$Address/tx_counter: u64 +/$Address/wrapper_tx_counter: u64 ``` -In `process_proposal` we check that `tx_counter` field in `WrapperTx` is equal to the value currently in storage; if this doesn't hold, the transaction is considered invalid. +where `$Address` is the one signing the transaction (the same implied by the `pk` field of the `WrapperTx` struct). -At last, in `finalize_block`, the protocol updates the counter key in storage, increasing its value by one. Now, if a malicious user tried to replay this transaction, the `tx_counter` in the struct would no longer be equal to the one in storage and the transaction would be deemed invalid. +The check will consist of a signature check first followed by a check on the counter that will make sure that the counter attached to the transaction matches the one in storage for the signing address. This will be done in the `process_proposal` function so that validators can decide whether the transaction is valid or not; if it's not, then they will discard the transaction and skip to the following one. -Note that the address whose counter is involved in this process is the one signing the transaction (the same as the `pk` field of the struct). +At last, in `finalize_block`, the ledger will update the counter key in storage, increasing its value by one. This will happen when the following conditions are met: -## InnerTx +- `process_proposal` has accepted the tx by validating its signature and transaction counter +- The tx was correctly applied in `finalize_block` (for `WrapperTx` this simply means inclusion in the block and gas accounting) -The `EncryptedTx` incapsulated inside `WrapperTx` should be protected too. That's because, if it wasn't, an attacker could extract the inner tx, rewrap it, and replay it.\ -To simplify this check we will perform it entirely in Wasm: the check of the counter will be carryed out by the validity predicates while the actual writing of the counter in storage will be done by the transactions themselves. +Now, if a malicious user tried to replay this transaction, the `tx_counter` in the struct would no longer be equal to the one in storage and the transaction would be deemed invalid. -To do so, the `SignedTxData` attached to the transaction will hold the current value of the counter in storage. The transaction will simply read the value from storage and increase its value by one. The VP will then check the validity of the signature and, if it's deemed valid, will proceed to checking if the pre value of the counter in storage was equal to the one contained in the transaction data and if the post value of the key in storage has been incremented by one. +### Forks -In the specific case of a transfer, since MASP already comes with replay protection as part of the Zcash design (see the [MASP specs](https://specs.namada.net/masp.html) and [Zcash protocol specs](https://zips.z.cash/protocol/protocol.pdf)), the counter in `SignedTxData` should be optional. +In the case of a fork, the transaction counters are not enough to prevent replay attacks. Transactions, in fact, could still be replayed on the other branch as long as their format is kept unchanged and the counters in storage match. -An alternative implementation could place the protection for the inner tx in protocol, just like the wrapper one. In this case we would need to extend the `Tx` struct to hold an additional optional field (again, because of MASP) for the transaction counter and the address involved in replay protection, like so: +To mitigate this problem, transactions will need to carry a `ChainId` identifier to tie them to a specific fork. This field needs to be added to the `Tx` struct so that it applies to both `WrapperTx` and `EncryptedTx` (for the same reason explained in [InnerTx](#InnerTx) about the double transaction counter): ```rust pub struct Tx { pub code: Vec, pub data: Option>, pub timestamp: DateTimeUtc, - pub tx_counter: Option<(Address, u64)> + pub chain_id: ChainId } ``` -The check would run in `process_proposal` and the update in `finalize_block`, just like for the wrapper transaction. This implementation, though, shows two drawbacks: +This new field will be signed just like the other ones and is therefore subject to the same guarantees explained in the [initial](#encrypted-authenticated-fixme-better-name-for-this-section) section. +The validity of this identifier will be checked in `process_proposal` for both the outer and inner tx: if a transaction carries an unexpected chain id, it won't be applied, meaning that the counter in storage won't be updated and no other modifications will be applied to storage. + +## Implementation details + +In this section we'll talk about some details of the replay protection mechanism that derive from the solution proposed in the previous section. + +### Storage counters + +Replay protection will require interaction with the storage from both the protocol and Wasm. To do so we can take advantage of the `StorageRead` and `StorageWrite` traits to work with a single interface. + +The proposed implementation requires two transaction counters in storage for every address, so that the storage subspace of a given address looks like the following: + +``` +/$Address/wrapper_tx_counter: u64 +/$Address/inner_tx_counter: u64 +``` + +An implementation requiring a single counter in storage has been taken into consideration and discarded because that would not support batching; see the [relative section](#single-counter-in-storage) for a more in-depth explanation. + +For both the wrapper and inner transaction, the increase of the counter in storage is an important step that must be correctly executed. More specifically, we want to increase the counter as soon as we verify that the signature, the chain id and the passed-in transaction counter are valid. The increase should happen immediately after the checks because of two reasons: + +- Prevent replay attack of a transaction in the same block +- Update the transaction counter even in case the transaction fails, to prevent a possible replay attack in the future (since a transaction invalid at state Sx could become valid at state Sn where `n > x`) + +For `WrapperTx`, the counter increase and fee accounting will per performed in `finalize_block` (as stated in the [relative](#wrappertx) section). + +For `InnerTx`, instead, the logic is not straightforward. The transaction code will be executed in a Wasm environment ([Wasmer](https://wasmer.io)) till it eventually completes or raises an exception. In case of success, the counter in storage will be updated correctly but, in case of failure, the protocol will discard all of the changes brought by the transactions to the write-ahead-log, including the updated transaction counter. This is a problem because the transaction could be successfully replayed in the future if it will become valid. + +The ideal solution would be to interrupt the execution of the Wasm code after the transaction counter (if any) has been increased. This would allow performing a first run of the involved VPs and, if all of them accept the changes, let the protocol commit these changes before any possible failure. After that, the protocol would resume the execution of the transaction from the previous interrupt point until completion or failure, after which a second pass of the VPs is initiated to validate the remaining state modifications. In case of a VP rejection after the counter increase there would be no need to resume execution and the transaction could be immediately deemed invalid so that the protocol could skip to the next tx to be executed. With this solution, the counter update would be committed to storage regardless of a failure of the transaction itself. + +Unfortunately, at the moment, Wasmer doesn't allow [yielding](https://github.com/wasmerio/wasmer/issues/1127) from the execution. For now, the responsibility will be up to the user to provide a valid inner transaction, and, in case of an invalid one, to take actions to prevent a possible replay attack: in essence, the user will be required to submit a new valid transaction to invalidate the counter of the previous one. + +### Batching and transaction ordering + +The proposed replay protection technique supports the execution of multiple transactions with the same address as _source_ in a single block. Actually, the presence of the transaction counters and the checks performed on them now impose a strict ordering on the execution sequence (which can be an added value for some use cases). The correct execution of more than one transaction per source address in the same block is preserved as long as: + +1. The wrapper transactions are inserted in the block with the correct ascending order +2. No hole is present in the counters' sequence +3. The counter of the first transaction included in the block matches the expected one in storage + +The conditions are enforced by the block proposer who has an interest in maximizing the amount of fees extracted by the proposed block. To support this incentive, we will charge gas and fees at the same moment in which we perform the counter increase explained in the [storage counters](#storage-counters) section: this way we can avoid charging fees and gas if the transaction is invalid (invalid signature, wrong counter or wrong chain id), effectively incentivizing the block proposer to include only valid transactions and correctly reorder them to maximize the fees (see the [block rejection](#block-rejection) section for an alternative solution that was discarded in favor of this). + +In case of a missing transaction causes a hole in the sequence of transaction counters, the block proposer will include in the block all the transactions up to the missing one and discard all the ones following that one, effectively preserving the correct ordering. + +Correctly ordering the transactions is not enough to guarantee the correct execution. As already mentioned in the [WrapperTx](#wrappertx) section, the block proposer and the validators also need to access the storage to check that the first transaction counter of a sequence is actually the expected one. + +The entire counter ordering is only done on the `WrapperTx`: if the inner counter is wrong then the inner transaction will fail and the signer of the corresponding wrapper will be charged with fees. This incentivizes submitters to produce valid transactions and discourages malicious user from rewrapping and resubmitting old transactions. + +### Mempool checks + +As a form of optimization to prevent mempool spamming, some of the checks that have been introduced in this document will also be brought to the `mempool_validate` function. Of course, we always refer to checks on the `WrapperTx` only. More specifically: + +- Check the `ChainId` field +- Check the signature of the transaction against the `pk` field of the `WrapperTx` +- Perform a limited check on the transaction counter + +Regarding the last point, `mempool_validate` will check if the counter in the transaction is `>=` than the one in storage for the address signing the `WrapperTx`. A complete check (checking for strict equality) is not feasible, as described in the [relative](#mempool-counter-validation) section. + +## Alternatives considered + +In this last section we list some possible solutions that were taken into consideration during the writing of this spec but were eventually discarded. + +### Mempool counter validation + +The idea of performing a complete validation of the transaction counters in the `mempool_validate` function was discarded because of a possible flaw. + +Suppose a client sends five transactions (counters from 1 to 5). The mempool of the next block proposer is not guaranteed to receive them in order: something on the network could shuffle the transactions up so that they arrive in the following order: 2-3-4-5-1. Now, since we validate every single transaction to be included in the mempool in the exact order in which we receive them, we would discard the first four transactions and only accept the last one, that with counter 1. Now the next block proposer might have the four discarded transactions in its mempool (since those were not added to the previous block and therefore not evicted from the other mempools, at least they shouldn't, see [block rejection](#block-rejection)) and could therefore include them in the following block. But still, a process that could have ended in a single block actually took two blocks. Moreover, there are two more issues: + +- The next block proposer might have the remaining transactions out of order in his mempool as well, effectively propagating the same issue down to the next block proposer +- The next block proposer might not have these transactions in his mempool at all + +Finally, transactions that are not allowed into the mempool don't get propagated to the other peers, making their inclusion in a block even harder. +It is instead better to avoid a complete filter on the transactions based on their order in the mempool: instead we are going to perform a simpler check and then let the block proposer rearrange them correctly when proposing the block. + +### In-protocol protection for InnerTx + +An alternative implementation could place the protection for the inner tx in protocol, just like the wrapper one, based on the transaction counter inside `SignedTxData`. The check would run in `process_proposal` and the update in `finalize_block`, just like for the wrapper transaction. This implementation, though, shows two drawbacks: - it implies the need for an hard fork in case of a modification of the replay protection mechanism - it's not clear who's the source of the inner transaction from the outside, as that depends on the specific code of the transaction itself. We could use specific whitelisted txs set to define when it requires a counter (would not work for future programmable transactions), but still, we have no way to define which address should be targeted for replay protection (**blocking issue**) -### VPs and Txs +### In-protocol counter increase for InnerTx + +In the [storage counter](#storage-counters) section we mentioned the issue of increasing the transaction counter for an inner tx even in case of failure. A possible solution that we took in consideration and discarded was to increase the counter from protocol in case of a failure. -To implement replay protection for the inner transaction we will need to update all the VPs checking the transaction's signature to include the check on the transaction counter: at the moment the `vp_user` validity predicate is the only one to update. +This is technically feasible since the protocol is aware of the keys modified by the transaction and also of the results of the validity predicates (useful in case the transaction updated more than one counter in storage). It is then possible to recover the value and reapply the change directly from protocol. This logic though, is quite dispersive, since it effectively splits the management of the counter for the `InnerTx` among Wasm and protocol, while our initial intent was to keep it completely in Wasm. -In addition, all the transactions involving `SignedTxData` should increment the counter. +### Single counter in storage -## Single counter +We can't use a single transaction counter in storage because this would prevent batching. -The mechanisms to prevent replay attacks for both transactions (wrapper and inner) will share the same `tx_counter` key in storage. +As an example, if a client (with a current counter in storage holding value 5) generates two transactions to be included in the same block, signing both the outer and the inner (default behavior of the client), it would need to generate the following transaction counters: -This could be an issue when the signer of the `WrapperTx` is the same of the `InnerTx` (default behaviour of the client): in this case, if both transactions expect the same counter in storage, the wrapper transaction will pass validation but the inner one will fail. To cope with this, the client will be responsible for producing a valid `WrapperTx` in which `tx_counter` will be set to the current value of the counter in storage, call it `storage_ctr`, while the inner transaction will have `storage_ctr + 1` in its data. +``` +[ + T1: (WrapperCtr: 5, InnerCtr: 6), + T2: (WrapperCtr: 7, InnerCtr: 8) +] +``` -## Storage Interface +Now, the current execution model of Namada includes the `WrapperTx` in a block first to then decrypt and execute the inner tx in the following block (respecting the committed order of the transactions). That would mean that the outer tx of `T1` would pass validation and immediately increase the counter to 6 to prevent a replay attack in the same block. Now, the outer tx of `T2` will be processed but it won't pass validation because it carries a counter with value 7 while the ledger expects 6. -Replay protection will require interaction with the storage from both the protocol and Wasm. To do so we can take advantage of the `StorageRead` and `StorageWrite` traits to work with a single interface. +To fix this, one could think to set the counters as follows: + +``` +[ + T1: (WrapperCtr: 5, InnerCtr: 7), + T2: (WrapperCtr: 6, InnerCtr: 8) +] +``` -## Batching +This way both the transactions will be considered valid and executed. The issue is that, if the second transaction is not included in the block (for any reason), than the first transaction (the only one remaining at this point) will fail. In fact, after the outer tx has correctly increased the counter in storage to value 6 the block will be accepted. In the next block the inner transaction will be decrypted and executed but this last step will fail since the counter in `SignedTxData` carries a value of 7 and the counter in storage has a value of 6. + +To cope with this there are two possible ways. The first one is that, instead of checking the exact value of the counter in storage and increasing its value by one, we could check that the transaction carries a counter `>=` than the one in storage and write this one (not increase) to storage. The problem with this is that it the lack of support for strict ordering of execution. + +The second option is to keep the usual increase strategy of the counter (increase by one and check for strict equality) and simply use two different counters in storage for each address. The transaction will then look like this: + +``` +[ + T1: (WrapperCtr: 5, InnerCtr: 5), + T2: (WrapperCtr: 6, InnerCtr: 6) +] +``` -The implementation proposed in this document doesn't support batching of multiple transactions from a same address in a single block. More specifically, the transactions will all succeed only if they are executed in the intended order, but the order in which transactions will be included in the block by the proposer is not guaranteed. An out of order execution of multiple transactions would lead to the failure of some of them (in the worst case, the failure of all of them but one, in the best case, the failure of only one). This problem will be amplified by the introduction of Ferveo for DKG which will be able to reorder transactions. +Since the order of inclusion of the `WrapperTxs` forces the same order of the execution for the inner ones, both transactions can be correctly executed and the correctness will be maintained even in case `T2` didn't make it to the block (note that the counter for an inner tx and the corresponding wrapper one don't need to coincide). -The Wasm implementation of replay protection can't cope with this problem because every wasm run (of either a transaction or a validity predicate) is only aware of its context, meaning the wasm bytecode and the serialized transaction data. The lack of awareness of the other transactions makes it impossible to develop a replay protection mechanism supporting batching in wasm. +### Block rejection -To address this issue there could be two ways: +The implementation proposed in this document has one flaw when it comes to discontinuous transactions. If, for example, for a given address, the counter in storage for the `WrapperTx` is 5 and the block proposer receives, in order, transactions 6, 5 and 8, the proposer will have an incentive to correctly order transactions 5 and 6 to gain the fees that he would otherwise lose. Transaction 8 will never be accepted by the validators no matter the ordering (since they will expect tx 7 which got lost): this effectively means that the block proposer has no incentive to include this transaction in the block because it would gain him no fees but, at the same time, he doesn't really have a disincentive to not include it, since in this case the validators will simply discard the invalid tx but accept the rest of the block granting the proposer his fees on all the other transactions. -- Keep the proposed replay protection in Wasm and implement a batching mechanism in both the client and the ledger to embed more than one transaction in a single `Tx` struct -- Implement replay protection in protocol for the inner transaction (as discolsed in section [InnerTx](#InnerTx)) +A similar scenario happens in the case of a single transaction that is not the expected one (e.g. tx 5 when 4 is expected), or for a different type of inconsistencies, like a wrong `ChainId` or an invalid signature. -Following the second option, the ledger would be able to analyze the validity of the counters, of all the transactions relating to a single address, against the value in storage at the beginning of the block. -Finally, it could increment the counter in storage a single time by the correct amount (given by the amount of transactions that were executed with success). As already mentioned, though, this implementation suffers from some serious issues and seems to not be feasible. +It is up to the block proposer then, whether to include or not these kinds of transactions: a malicious proposer could do so to spam the block without suffering any penalty. The lack of fees could be a strong enough measure to prevent proposers from applying this behavior, together with the fact that the only damage caused to the chain would be spamming the blocks. -The first option, instead, carries more benefits, since in addition to allowing batching, it's also more flexible. +If one wanted to completely prevent this scenario, the solution would be to reject the entire block: this way the proposer would have an incentive to behave correctly (by not including these transactions into the block) to gain the block fees. This would allow to shrink the size of the blocks in case of unfair block proposers but it would also cause the slow down of the block creation process, since after a block rejection a new Tendermint round has to be initiated. From 806c23d112eee39ac87fb7fe63e2c4b249d0e101 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 18 Oct 2022 12:33:14 +0200 Subject: [PATCH 06/18] Updates replay protection specs with hash strategy --- .../src/base-ledger/replay-protection.md | 298 +++++++++++++++--- 1 file changed, 258 insertions(+), 40 deletions(-) diff --git a/documentation/specs/src/base-ledger/replay-protection.md b/documentation/specs/src/base-ledger/replay-protection.md index 5cfb5d36fa..7ebeb847f4 100644 --- a/documentation/specs/src/base-ledger/replay-protection.md +++ b/documentation/specs/src/base-ledger/replay-protection.md @@ -71,12 +71,127 @@ The underlying consensus engine, [Tendermint](https://github.com/tendermint/tend ## Implementation -Namada replay protection consists of three parts: the in-Wasm solution for `EncryptedTx` (also called the `InnerTx`), an in-protocol mechanism for `WrapperTx` and a way to mitigate replay attacks in case of a fork. +Namada replay protection consists of three parts: the hash-based solution for both `EncryptedTx` (also called the `InnerTx`) and `WrapperTx`, a way to mitigate replay attacks in case of a fork and a concept of a lifetime for the transactions. -### InnerTx +### Hash register The actual Wasm code and data for the transaction are encapsulated inside a struct `Tx`, which gets encrypted as an `EncryptedTx` and wrapped inside a `WrapperTx` (see the [relative](#encryption-authentication) section). This inner transaction must be protected from replay attacks because it carries the actual semantics of the state transition. Moreover, even if the wrapper transaction was protected from replay attacks, an attacker could extract the inner transaction, rewrap it, and replay it. Note that for this attack to work, the attacker will need to sign the outer transaction himself and pay gas and fees for that, but this could still cause much greater damage to the parties involved in the inner transaction. +`WrapperTx` is the only type of transaction currently accepted by the ledger. It must be protected from replay attacks because, if it wasn't, a malicious user could replay the transaction as is. Even if the inner transaction implemented replay protection or, for any reason, wasn't accepted, the signer of the wrapper would still pay for gas and fees, effectively suffering economic damage. + +To prevent the replay of both these transactions we will rely on a set of already processed transactions' digests that will be kept in storage. These digests will be computed on the **signed** transactions. To support this, we'll need a subspace in storage headed by a `ReplayProtection` internal address: + +``` +/$ReplayProtectionAddress/$tx0_hash: None +/$ReplayProtectionAddress/$tx1_hash: None +/$ReplayProtectionAddress/$tx2_hash: None +... +``` + +The hashes will form the last part of the path to allow for a fast storage lookup. + +The consistency of the storage subspace is of critical importance for the correct working of the replay protection mechanism. To protect it, a validity predicate will check that no changes to this subspace are applied by any wasm transaction, as those should only be available from protocol. + +Both in `mempool_validation` and `process_proposal` we will perform a check (together with others, see the [relative](#wrapper-checks) section) on both the digests against the storage to check that neither of the transactions has already been executed: if this doesn't hold, the `WrapperTx` will not be included into the mempool/block respectively. If both checks pass then the transaction is included in the block and executed. In the `finalize_block` function we will add the transaction's hash to storage to prevent re-executions. We will first add the hash of the wrapper transaction. After that, in the following block, we deserialize the inner transaction, check the correct order of the transactions in the block and execute the tx: if it runs out of gas then we'll avoid storing its hash to allow rewrapping and executing the transaction, otherwise we'll add the hash in storage (both in case of success or failure of the tx). + +### Forks + +In the case of a fork, the transaction hash is not enough to prevent replay attacks. Transactions, in fact, could still be replayed on the other branch as long as their format is kept unchanged and the counters in storage match. + +To mitigate this problem, transactions will need to carry a `ChainId` identifier to tie them to a specific fork. This field needs to be added to the `Tx` struct so that it applies to both `WrapperTx` and `EncryptedTx`: + +```rust +pub struct Tx { + pub code: Vec, + pub data: Option>, + pub timestamp: DateTimeUtc, + pub chain_id: ChainId +} +``` + +This new field will be signed just like the other ones and is therefore subject to the same guarantees explained in the [initial](#encryption-authentication) section. The validity of this identifier will be checked in `process_proposal` for both the outer and inner tx: if a transaction carries an unexpected chain id, it won't be applied, meaning that no modifications will be applied to storage. + +### Transaction lifetime + +In general, a transaction is valid at the moment of submission, but after that, a series of external factors (ledger state, etc.) might change the mind of the submitter who's now not interested in the execution of the transaction anymore. + +We have to introduce the concept of a lifetime (or timeout) for the transactions: basically, the `Tx` struct will hold an extra field called `expiration` stating the maximum block height up until which the submitter is willing to see the transaction executed. After the specified block height, the transaction will be considered invalid and discarded regardless of all the other checks. + +By introducing this new field we are setting a new constraint in the transaction's contract, where the ledger will make sure to prevent the execution of the transaction after the deadline and, on the other side, the submitter commits himself to the result of the execution at least until its expiration. If the expiration is reached and the transaction has not been executed the submitter can decide to submit a new transaction if he's still interested in the changes carried by it. + +In our design, the `expiration` will hold until the transaction is executed: once it's executed, either in case of success or failure, the tx hash will be written to storage and the transaction will not be replayable. In essence, the transaction submitter commits himself to one of these three conditions: + +- Transaction is invalid regardless of the specific state +- Transaction is executed (either with success or not) and the transaction hash is saved in the storage +- Expiration block height has passed + +The first condition satisfied will invalidate further executions of the same tx. + +In anticipation of DKG implementation, the current struct `WrapperTx` holds a field `epoch` stating the epoch in which the tx should be executed. This is because Ferveo will produce a new public key each epoch, effectively limiting the lifetime of the transaction (see section 2.2.2 of the [documentation](https://eprint.iacr.org/2022/898.pdf)). Unfortunately, for replay protection, a resolution of 1 epoch (~ 1 day) is too low for the possible needs of the submitters, therefore we need the `expiration` field to hold a maximum `BlockHeight` to increase resolution down to a single block (~ 10 seconds). + +```rust +pub struct Tx { + pub code: Vec, + pub data: Option>, + pub timestamp: DateTimeUtc, + pub chain_id: ChainId, + /// Lifetime of the transaction, also determines which decryption key will be used + pub expiration: BlockHeight, +} + +pub struct WrapperTx { + /// The fee to be payed for including the tx + pub fee: Fee, + /// Used to determine an implicit account of the fee payer + pub pk: common::PublicKey, + /// Max amount of gas that can be used when executing the inner tx + pub gas_limit: GasLimit, + /// the encrypted payload + pub inner_tx: EncryptedTx, + /// sha-2 hash of the inner transaction acting as a commitment + /// the contents of the encrypted payload + pub tx_hash: Hash, +} +``` + +Since we now have more detailed information about the desired lifetime of the transaction, we can remove the `epoch` field and rely solely on `expiration`. Now, the producer of the inner transaction should make sure to set a sensible value for this field, in the sense that it should not span more than one epoch. If this happens, then the transaction will be correctly decrypted only in a subset of the desired lifetime (the one expecting the actual key used for the encryption), while, in the following epochs, the transaction will fail decryption and won't be executed. In essence, the `expiration` parameter can only restrict the implicit lifetime within the current epoch, it can not surpass it as that would make the transaction fail in the decryption phase. + +The subject encrypting the inner transaction will also be responsible for using the appropriate public key for encryption relative to the targeted block height. + +The wrapper transaction will match the `expiration` of the inner for correct execution. Note that we need this field also for the wrapper to anticipate the check at mempool/proposal evaluation time, but also to prevent someone from inserting a wrapper transaction after the corresponding inner has expired forcing the wrapper signer to pay for the fees. + +### Wrapper checks + +In `mempool_validation` and `process_proposal` we will perform some checks on the wrapper tx to validate it. These will involve: + +- Valid signature +- Enough funds to pay the fee +- Valid chainId +- Valid transaction hash +- Valid expiration + +These checks can all be done before executing the transactions themselves (the check on the gas cannot be done ahead of time). If any of these fails, the transaction should be considered invalid and the action to take will be one of the followings: + +1. If the checks fail on the signature, chainId, expiration or transaction hash, then this transaction will be forever invalid, regardless of the possible evolution of the ledger's state. There's no need to include the transaction in the block. Moreover, we **cannot** include this transaction in the block to charge a fee (as a sort of punishment) because these errors may not depend on the signer of the tx (could be due to malicious users or simply a delay in the tx inclusion in the block) +2. If the checks fail _only_ because of an insufficient balance, the wrapper should be kept in mempool for a future play in case the funds should become available +3. If all the checks pass validation we will include the transaction in the block to store the hash and charge the fee + +The `expiration` parameter also justifies step 2 of the previous bullet points which states that if the validity checks fail only because of an insufficient balance to pay for fees then the transaction should be kept in mempool for future execution. Without it, the transaction could be potentially executed at any future moment, possibly going against the mutated interests of the submitter. With the expiration parameter, now, the submitter commits himself to accept the execution of the transaction up to the specified block height: it's going to be his responsibility to provide a sensible value for this parameter. Given this constraint the transaction will be kept in memepool up until the expiration (since it would become invalid after that in any case), to prevent the mempool from increasing too much in size. + +This mechanism can also be applied to another scenario. Suppose a transaction was not propagated to the network by a node (or a group of colluding nodes). Now, this tx might be valid, but it doesn't get inserted into a block. Without an expiration, this tx can be replayed (better, applied, since it was never executed in the first place) at a future moment in time when the submitter might not be willing to execute it anymore. + +## Possible optimizations + +In this section we describe two alternative solutions that come with some optimizations. + +### Transaction counter + +Instead of relying on a hash (32 bytes) we could use a 64 bits (8 bytes) transaction counter as nonce for the wrapper and inner transactions. The advantage is that it would only use 25% of the storage space required by the proposed solution. On the other hand, the handling of the counter for the inner transaction will be performed entirely in wasm (transactions and VPs) making it a bit less efficient. + +**NOTE**: this solution requires the ability to [yield](https://github.com/wasmerio/wasmer/issues/1127) execution from Wasmer which is not implemented yet. + +#### InnerTx + We will implement the protection entirely in Wasm: the check of the counter will be carried out by the validity predicates while the actual writing of the counter in storage will be done by the transactions themselves. To do so, the `SignedTxData` attached to the transaction will hold the current value of the counter in storage: @@ -105,13 +220,11 @@ The wasm transaction will simply read the value from storage and increase its va The VP of the _source_ address will then check the validity of the signature and, if it's deemed valid, will proceed to check if the pre-value of the counter in storage was equal to the one contained in the `SignedTxData` struct and if the post-value of the key in storage has been incremented by one: if any of these conditions doesn't hold the VP will discard the transactions and prevent the changes from being applied to the storage. -In the specific case of a transfer, since MASP already comes with replay protection as part of the Zcash design (see the [MASP specs](https://specs.namada.net/masp.html) and [Zcash protocol specs](https://zips.z.cash/protocol/protocol.pdf)), the counter in `SignedTxData` is not required and therefore should be optional. +In the specific case of a shielded transfer, since MASP already comes with replay protection as part of the Zcash design (see the [MASP specs](https://specs.namada.net/masp.html) and [Zcash protocol specs](https://zips.z.cash/protocol/protocol.pdf)), the counter in `SignedTxData` is not required and therefore should be optional. To implement replay protection for the inner transaction we will need to update all the VPs checking the transaction's signature to include the check on the transaction counter: at the moment the `vp_user` validity predicate is the only one to update. In addition, all the transactions involving `SignedTxData` should increment the counter. -### WrapperTx - -`WrapperTx` is the only type of transaction currently accepted by the ledger. It must be protected from replay attacks because, if it wasn't, a malicious user could replay the transaction as is. Even if the inner transaction implemented replay protection (as explained in the [previous](#innertx) section) or, for any reason, wasn't accepted, the signer of the wrapper would still pay for gas and fees, effectively suffering economic damage. +#### WrapperTx To protect this transaction we can implement an in-protocol mechanism. Since the wrapper transaction gets signed before being submitted to the network, we can leverage the `tx_counter` field of the `SignedTxData` already introduced for the inner tx. @@ -132,33 +245,15 @@ At last, in `finalize_block`, the ledger will update the counter key in storage, Now, if a malicious user tried to replay this transaction, the `tx_counter` in the struct would no longer be equal to the one in storage and the transaction would be deemed invalid. -### Forks - -In the case of a fork, the transaction counters are not enough to prevent replay attacks. Transactions, in fact, could still be replayed on the other branch as long as their format is kept unchanged and the counters in storage match. +#### Implementation details -To mitigate this problem, transactions will need to carry a `ChainId` identifier to tie them to a specific fork. This field needs to be added to the `Tx` struct so that it applies to both `WrapperTx` and `EncryptedTx` (for the same reason explained in [InnerTx](#InnerTx) about the double transaction counter): +In this section we'll talk about some details of the replay protection mechanism that derive from the solution proposed in this section. -```rust -pub struct Tx { - pub code: Vec, - pub data: Option>, - pub timestamp: DateTimeUtc, - pub chain_id: ChainId -} -``` - -This new field will be signed just like the other ones and is therefore subject to the same guarantees explained in the [initial](#encrypted-authenticated-fixme-better-name-for-this-section) section. -The validity of this identifier will be checked in `process_proposal` for both the outer and inner tx: if a transaction carries an unexpected chain id, it won't be applied, meaning that the counter in storage won't be updated and no other modifications will be applied to storage. - -## Implementation details - -In this section we'll talk about some details of the replay protection mechanism that derive from the solution proposed in the previous section. - -### Storage counters +##### Storage counters Replay protection will require interaction with the storage from both the protocol and Wasm. To do so we can take advantage of the `StorageRead` and `StorageWrite` traits to work with a single interface. -The proposed implementation requires two transaction counters in storage for every address, so that the storage subspace of a given address looks like the following: +This implementation requires two transaction counters in storage for every address, so that the storage subspace of a given address looks like the following: ``` /$Address/wrapper_tx_counter: u64 @@ -167,7 +262,7 @@ The proposed implementation requires two transaction counters in storage for eve An implementation requiring a single counter in storage has been taken into consideration and discarded because that would not support batching; see the [relative section](#single-counter-in-storage) for a more in-depth explanation. -For both the wrapper and inner transaction, the increase of the counter in storage is an important step that must be correctly executed. More specifically, we want to increase the counter as soon as we verify that the signature, the chain id and the passed-in transaction counter are valid. The increase should happen immediately after the checks because of two reasons: +For both the wrapper and inner transaction, the increase of the counter in storage is an important step that must be correctly executed. First, the implementation will `panic!` in case of a counter overflow to prevent wrapping, since this would allow for the replay of previous transactions. Also, we want to increase the counter as soon as we verify that the signature, the chain id and the passed-in transaction counter are valid. The increase should happen immediately after the checks because of two reasons: - Prevent replay attack of a transaction in the same block - Update the transaction counter even in case the transaction fails, to prevent a possible replay attack in the future (since a transaction invalid at state Sx could become valid at state Sn where `n > x`) @@ -178,11 +273,13 @@ For `InnerTx`, instead, the logic is not straightforward. The transaction code w The ideal solution would be to interrupt the execution of the Wasm code after the transaction counter (if any) has been increased. This would allow performing a first run of the involved VPs and, if all of them accept the changes, let the protocol commit these changes before any possible failure. After that, the protocol would resume the execution of the transaction from the previous interrupt point until completion or failure, after which a second pass of the VPs is initiated to validate the remaining state modifications. In case of a VP rejection after the counter increase there would be no need to resume execution and the transaction could be immediately deemed invalid so that the protocol could skip to the next tx to be executed. With this solution, the counter update would be committed to storage regardless of a failure of the transaction itself. -Unfortunately, at the moment, Wasmer doesn't allow [yielding](https://github.com/wasmerio/wasmer/issues/1127) from the execution. For now, the responsibility will be up to the user to provide a valid inner transaction, and, in case of an invalid one, to take actions to prevent a possible replay attack: in essence, the user will be required to submit a new valid transaction to invalidate the counter of the previous one. +Unfortunately, at the moment, Wasmer doesn't allow [yielding](https://github.com/wasmerio/wasmer/issues/1127) from the execution. -### Batching and transaction ordering +In case the transaction went out of gas (given the `gas_limit` field of the wrapper), all the changes applied will be discarded from the WAL and will not affect the state of the storage. The inner transaction could then be rewrapped with a correct gas limit and replayed until the `expiration` block has been reached. -The proposed replay protection technique supports the execution of multiple transactions with the same address as _source_ in a single block. Actually, the presence of the transaction counters and the checks performed on them now impose a strict ordering on the execution sequence (which can be an added value for some use cases). The correct execution of more than one transaction per source address in the same block is preserved as long as: +##### Batching and transaction ordering + +This replay protection technique supports the execution of multiple transactions with the same address as _source_ in a single block. Actually, the presence of the transaction counters and the checks performed on them now impose a strict ordering on the execution sequence (which can be an added value for some use cases). The correct execution of more than one transaction per source address in the same block is preserved as long as: 1. The wrapper transactions are inserted in the block with the correct ascending order 2. No hole is present in the counters' sequence @@ -196,7 +293,7 @@ Correctly ordering the transactions is not enough to guarantee the correct execu The entire counter ordering is only done on the `WrapperTx`: if the inner counter is wrong then the inner transaction will fail and the signer of the corresponding wrapper will be charged with fees. This incentivizes submitters to produce valid transactions and discourages malicious user from rewrapping and resubmitting old transactions. -### Mempool checks +##### Mempool checks As a form of optimization to prevent mempool spamming, some of the checks that have been introduced in this document will also be brought to the `mempool_validate` function. Of course, we always refer to checks on the `WrapperTx` only. More specifically: @@ -206,11 +303,11 @@ As a form of optimization to prevent mempool spamming, some of the checks that h Regarding the last point, `mempool_validate` will check if the counter in the transaction is `>=` than the one in storage for the address signing the `WrapperTx`. A complete check (checking for strict equality) is not feasible, as described in the [relative](#mempool-counter-validation) section. -## Alternatives considered +#### Alternatives considered -In this last section we list some possible solutions that were taken into consideration during the writing of this spec but were eventually discarded. +In this section we list some possible solutions that were taken into consideration during the writing of this solution but were eventually discarded. -### Mempool counter validation +##### Mempool counter validation The idea of performing a complete validation of the transaction counters in the `mempool_validate` function was discarded because of a possible flaw. @@ -222,20 +319,20 @@ Suppose a client sends five transactions (counters from 1 to 5). The mempool of Finally, transactions that are not allowed into the mempool don't get propagated to the other peers, making their inclusion in a block even harder. It is instead better to avoid a complete filter on the transactions based on their order in the mempool: instead we are going to perform a simpler check and then let the block proposer rearrange them correctly when proposing the block. -### In-protocol protection for InnerTx +##### In-protocol protection for InnerTx An alternative implementation could place the protection for the inner tx in protocol, just like the wrapper one, based on the transaction counter inside `SignedTxData`. The check would run in `process_proposal` and the update in `finalize_block`, just like for the wrapper transaction. This implementation, though, shows two drawbacks: - it implies the need for an hard fork in case of a modification of the replay protection mechanism - it's not clear who's the source of the inner transaction from the outside, as that depends on the specific code of the transaction itself. We could use specific whitelisted txs set to define when it requires a counter (would not work for future programmable transactions), but still, we have no way to define which address should be targeted for replay protection (**blocking issue**) -### In-protocol counter increase for InnerTx +##### In-protocol counter increase for InnerTx In the [storage counter](#storage-counters) section we mentioned the issue of increasing the transaction counter for an inner tx even in case of failure. A possible solution that we took in consideration and discarded was to increase the counter from protocol in case of a failure. This is technically feasible since the protocol is aware of the keys modified by the transaction and also of the results of the validity predicates (useful in case the transaction updated more than one counter in storage). It is then possible to recover the value and reapply the change directly from protocol. This logic though, is quite dispersive, since it effectively splits the management of the counter for the `InnerTx` among Wasm and protocol, while our initial intent was to keep it completely in Wasm. -### Single counter in storage +##### Single counter in storage We can't use a single transaction counter in storage because this would prevent batching. @@ -274,7 +371,7 @@ The second option is to keep the usual increase strategy of the counter (increas Since the order of inclusion of the `WrapperTxs` forces the same order of the execution for the inner ones, both transactions can be correctly executed and the correctness will be maintained even in case `T2` didn't make it to the block (note that the counter for an inner tx and the corresponding wrapper one don't need to coincide). -### Block rejection +##### Block rejection The implementation proposed in this document has one flaw when it comes to discontinuous transactions. If, for example, for a given address, the counter in storage for the `WrapperTx` is 5 and the block proposer receives, in order, transactions 6, 5 and 8, the proposer will have an incentive to correctly order transactions 5 and 6 to gain the fees that he would otherwise lose. Transaction 8 will never be accepted by the validators no matter the ordering (since they will expect tx 7 which got lost): this effectively means that the block proposer has no incentive to include this transaction in the block because it would gain him no fees but, at the same time, he doesn't really have a disincentive to not include it, since in this case the validators will simply discard the invalid tx but accept the rest of the block granting the proposer his fees on all the other transactions. @@ -283,3 +380,124 @@ A similar scenario happens in the case of a single transaction that is not the e It is up to the block proposer then, whether to include or not these kinds of transactions: a malicious proposer could do so to spam the block without suffering any penalty. The lack of fees could be a strong enough measure to prevent proposers from applying this behavior, together with the fact that the only damage caused to the chain would be spamming the blocks. If one wanted to completely prevent this scenario, the solution would be to reject the entire block: this way the proposer would have an incentive to behave correctly (by not including these transactions into the block) to gain the block fees. This would allow to shrink the size of the blocks in case of unfair block proposers but it would also cause the slow down of the block creation process, since after a block rejection a new Tendermint round has to be initiated. + +### Wrapper-bound InnerTx + +The solution is to tie an `InnerTx` to the corresponding `WrapperTx`. By doing so, it becomes impossible to rewrap an inner transaction and, therefore, all the attacks related to this practice would be unfeasible. This mechanism requires eight times less space in storage (only a 64 bit counter for the wrapper tx) and only one check on the wrapper counter in protocol. As a con, it requires communication between the signer of the inner transaction and that of the wrapper during the transaction construction. + +To do so we will have to change the current definition of the two tx structs to the following: + +```rust +pub struct WrapperTx { + /// The fee to be payed for including the tx + pub fee: Fee, + /// Used to determine an implicit account of the fee payer + pub pk: common::PublicKey, + /// Max amount of gas that can be used when executing the inner tx + pub gas_limit: GasLimit, + /// Lifetime of the transaction, also determines which decryption key will be used + pub expiration: BlockHeight, + /// Chain identifier for replay protection + pub chain_id: ChainId, + /// Transaction counter for replay protection + pub tx_counter: u64, + /// the encrypted payload + pub inner_tx: EncryptedTx, +} + +pub struct Tx { + pub code: Vec, + pub data: Option>, + pub timestamp: DateTimeUtc, + pub wrapper_commit: Option, +} +``` + +The Wrapper transaction no longer holds the inner transaction hash while the inner one now holds a commit to the corresponding wrapper tx in the form of the hash of a `WrapperCommit` struct, defined as: + +```rust +pub struct WrapperCommit { + pub pk: common::PublicKey, + pub tx_counter: u64, + pub expiration: BlockHeight, + pub chain_id: ChainId, +} +``` + +The `pk-tx_counter` couple contained in this struct, uniquely identifies a single `WrapperTx` (since a valid tx_counter is unique given the address) so that the inner one is now bound to this specific wrapper. The remaining fields, `expiration` and `chain_id`, will tie these two values given their importance in terms of safety (see the [relative](#wrappertx-checks) section). Note that the `wrapper_commit` field must be optional because the `WrapperTx` struct itself gets converted to a `Tx` struct before submission but it doesn't need any commitment. + +Both the inner and wrapper tx get signed on their hash, as usual, to prevent tampering with data. When a wrapper gets processed by the ledger, we first check the validity of the signature, checking that none of the fields were modified: this means that the inner tx embedded within the wrapper is, in fact, the intended one. This last statement means that no external attacker has tampered data, but the tampering could still have been performed by the signer of the wrapper before signing the wrapper transaction. + +If this check (and others, explained later in the [checks](#wrappertx-checks) section) passes, then the inner tx gets decrypted in the following block proposal process. At this time we check that the order in which the inner txs are inserted in the block matches that of the corresponding wrapper txs in the previous block. To do so, we rely on an in-storage queue holding the hash of the `WrapperCommit` struct computed from the wrapper tx. From the inner tx we extract the `WrapperCommit` hash and check that it matches that in the queue: if they don't it means that the inner tx has been reordered or rewrapped and we reject the block. Note that, since we have already checked the wrapper at this point, the only way to rewrap the inner tx would be to also modify its commitment (need to change at least the `tx_counter` field), otherwise the checks on the wrapper would have spotted the inconsistency and rejected the tx. + +If this check passes then we can send the inner transaction to the wasm environment for execution: if the transaction is signed, then at least one VP will check its signature to spot possible tampering of the data (especially by the wrapper signer, since this specific case cannot be checked before this step) and, if this is the case, will reject this transaction and no storage modifications will be applied. + +In summary: + +- The `InnerTx` carries a unique identifier of the `WrapperTx` embedding it +- Both the inner and wrapper txs are signed on all of their data +- The signature check on the wrapper tx ensures that the inner transaction is the intended one and that this wrapper has not been used to wrap a different inner tx. It also verifies that no tampering happened with the inner transaction by a third party. Finally, it ensures that the public key is the one of the signer +- The check on the `WrapperCommit` ensures that the inner tx has not been reordered nor rewrapped (this last one is a non-exhaustive check, inner tx data could have been tampered with by the wrapper signer) +- The signature check of the inner tx performed in Vp grants that no data of the inner tx has been tampered with, effectively verifying the correctness of the previous check (`WrapperCommit`) + +This sequence of controls makes it no longer possible to rewrap an `InnerTx` which is now bound to its wrapper. This implies that replay protection is only needed on the `WrapperTx` since there's no way to extract the inner one, rewrap it and replay it. + +#### WrapperTx checks + +In `mempool_validation` and `process_proposal` we will perform some checks on the wrapper tx to validate it. These will involve: + +- Valid signature +- Enough funds to pay for the fee +- Valid chainId +- Valid transaction counter +- Valid expiration + +These checks can all be done before executing the transactions themselves. The check on the gas cannot be done ahead of time and we'll deal with it later. If any of these fails, the transaction should be considered invalid and the action to take will be one of the followings: + +1. If the checks fail on the signature, chainId, expiration or transaction counter, then this transaction will be forever invalid, regardless of the possible evolution of the ledger's state. There's no need to include the transaction in the block nor to increase the transaction counter. Moreover, we **cannot** include this transaction in the block to charge a fee (as a sort of punishment) because these errors may not depend on the signer of the tx (could be due to malicious users or simply a delay in the tx inclusion in the block) +2. If the checks fail _only_ because of an insufficient balance, the wrapper should be kept in mempool for a future play in case the funds should become available +3. If all the checks pass validation we will include the transaction in the block to increase the counter and charge the fee + +Note that, regarding point one, there's a distinction to be made about an invalid `tx_counter` which could be invalid because of being old or being in advance. To solve this last issue (counter greater than the expected one), we have to introduce the concept of a lifetime (or timeout) for the transactions: basically, the `WrapperTx` will hold an extra field called `expiration` stating the maximum block height up until which the submitter is willing to see the transaction executed. After the specified block height the transaction will be considered invalid and discarded regardless of all the other checks. This way, in case of a transaction with a counter greater than expected, it is sufficient to wait till after the expiration to submit more transactions, so that the counter in storage is not modified (kept invalid for the transaction under observation) and replaying that tx would result in a rejection. + +This actually generalizes to a more broad concept. In general, a transaction is valid at the moment of submission, but after that, a series of external factors (ledger state, etc.) might change the mind of the submitter who's now not interested in the execution of the transaction anymore. By introducing this new field we are introducing a new constraint in the transaction's contract, where the ledger will make sure to prevent the execution of the transaction after the deadline and, on the other side, the submitter commits himself to the result of the execution at least until its expiration. If the expiration is reached and the transaction has not been executed the submitter can decide to submit a new, identical transaction if he's still interested in the changes carried by it. + +In our design, the `expiration` will hold until the transaction is executed, once it's executed, either in case of success or failure, the `tx_counter` will be increased and the transaction will not be replayable. In essence, the transaction submitter commits himself to one of these three conditions: + +- Transaction is invalid regardless of the specific state +- Transaction is executed (either with success or not) and the transaction counter is increased +- Expiration block height has passed + +The first condition satisfied will invalidate further executions of the same tx. + +The `expiration` parameter also justifies step 2 of the previous bullet points which states that if the validity checks fail only because of an insufficient balance to pay for fees than the transaction should be kept in mempool for a future execution. Without it, the transaction could be potentially executed at any future moment (provided that the counter is still valid), possibily going against the mutated interests of the submitter. With the expiration parameter, now, the submitter commits himself to accepting the execution of the transaction up to the specified block height: it's going to be his responsibility to provide a sensible value for this parameter. Given this constraint the transaction will be kept in memepool up until the expiration (since it would become invalid after that in any case), to prevent the mempool from increasing too much in size. + +This mechanism can also be applied to another scenario. Suppose a transaction was not propagated to the network by a node (or a group of colluding nodes). Now, this tx might be valid, but it doesn't get inserted into a block. Without an expiration, if the submitter doesn't submit any other transaction (which gets included in a block to increase the transaction counter), this tx can be replayed (better, applied, since it was never executed in the first place) at a future moment in time when the submitter might not be willing to execute it any more. + +Since the signer of the wrapper may be different from the one of the inner we also need to include this `expiration` field in the `WrapperCommit` struct, to prevent the signer of the wrapper from setting a lifetime which is in conflict with the interests of the inner signer. Note that adding a separate lifetime for the wrapper alone (which would require two separate checks) doesn't carry any benefit: a wrapper with a lifetime greater than the inner would have no sense since the inner would fail. Restricting the lifetime would work but it also means that the wrapper could prevent a valid inner transaction from being executed. We will then keep a single `expiration` field specifying the wrapper tx max block height (the inner one will actually be executed one block later because of the execution mechanism of Namada). + +To prevent the signer of the wrapper from submitting the transaction to a different chain, the `ChainId` field should also be included in the commit. + +Finally, in case the transaction run out of gas (based on the provided `gas_limit` field of the wrapper) we don't need to take any action: by this time the transaction counter will have already been incremented and the tx is not replayable anymore. In theory, we don't even need to increment the counter since the only way this transaction could become valid is a change in the way gas is accounted, which might require a fork anyway, and consequently a change in the required `ChainId`. However, since we can't tell the gas consumption before the inner tx has been executed, we cannot anticipate this check. + +#### WrapperCommit + +The fields of `WrapperTx` not included in `WrapperCommit` are at the discretion of the `WrapperTx` producer. These fields are not included in the commit because of one of these two reasons: + +- They depend on the specific state of the wrapper signer and cannot be forced (like `fee`, since the wrapper signer must have enough funds to pay for those) +- They are not a threat (in terms of replay attacks) to the signer of the inner transaction in case of failure of the transaction + +In a certain way, the `WrapperCommit` not only binds an `InnerTx` no a wrapper, but effectively allows the inner to control the wrapper by requesting some specific parameters for its creation and bind these parameters among the two transactions: this allows us to apply the same constraints to both txs while performing the checks on the wrapper only. + +#### Transaction creation process + +To craft a transaction, the process will now be the following (optional steps are only required if the signer of the inner differs from that of the wrapper): + +- (**Optional**) the `InnerTx` constructor request, to the wrapper signer, his public key and the `tx_counter` to be used +- The `InnerTx` is constructed in its entirety with also the `wrapper_commit` field to define the constraints of the future wrapper +- The produced `Tx` struct get signed over all of its data (with `SignedTxData`) producing a new struct `Tx` +- (**Optional**) The inner tx produced is sent to the `WrapperTx` producer together with the `WrapperCommit` struct (required since the inner tx only holds the hash of it) +- The signer of the wrapper constructs a `WrapperTx` compliant with the `WrapperCommit` fields +- The produced `WrapperTx` gets signed over all of its fields + +Compared to a solution not binding the inner tx to the wrapper one, this solution requires the exchange of 3 messages (request `tx_counter`, receive `tx_counter`, send `InnerTx`) between the two signers (in case they differ), instead of one. However, it allows the signer of the inner to send the `InnerTx` to the wrapper signer already encrypted, guaranteeing a higher level of safety: only the `WrapperCommit` struct should be sent clear, but this doesn't reveal any sensitive information about the inner transaction itself. From 3fd7a3a13833708262f84c12d7bf84add6a55fcb Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 18 Oct 2022 13:07:21 +0200 Subject: [PATCH 07/18] Removes panic --- documentation/specs/src/base-ledger/replay-protection.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/specs/src/base-ledger/replay-protection.md b/documentation/specs/src/base-ledger/replay-protection.md index 7ebeb847f4..b2fb6e7f81 100644 --- a/documentation/specs/src/base-ledger/replay-protection.md +++ b/documentation/specs/src/base-ledger/replay-protection.md @@ -262,7 +262,7 @@ This implementation requires two transaction counters in storage for every addre An implementation requiring a single counter in storage has been taken into consideration and discarded because that would not support batching; see the [relative section](#single-counter-in-storage) for a more in-depth explanation. -For both the wrapper and inner transaction, the increase of the counter in storage is an important step that must be correctly executed. First, the implementation will `panic!` in case of a counter overflow to prevent wrapping, since this would allow for the replay of previous transactions. Also, we want to increase the counter as soon as we verify that the signature, the chain id and the passed-in transaction counter are valid. The increase should happen immediately after the checks because of two reasons: +For both the wrapper and inner transaction, the increase of the counter in storage is an important step that must be correctly executed. First, the implementation will return an error in case of a counter overflow to prevent wrapping, since this would allow for the replay of previous transactions. Also, we want to increase the counter as soon as we verify that the signature, the chain id and the passed-in transaction counter are valid. The increase should happen immediately after the checks because of two reasons: - Prevent replay attack of a transaction in the same block - Update the transaction counter even in case the transaction fails, to prevent a possible replay attack in the future (since a transaction invalid at state Sx could become valid at state Sn where `n > x`) From aba665bc81a4af69c77ed210b4e2d1950aca2e33 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 18 Oct 2022 14:24:43 +0200 Subject: [PATCH 08/18] Mentions strict ordering of txs --- documentation/specs/src/base-ledger/replay-protection.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/documentation/specs/src/base-ledger/replay-protection.md b/documentation/specs/src/base-ledger/replay-protection.md index b2fb6e7f81..ebfcda3f83 100644 --- a/documentation/specs/src/base-ledger/replay-protection.md +++ b/documentation/specs/src/base-ledger/replay-protection.md @@ -186,7 +186,7 @@ In this section we describe two alternative solutions that come with some optimi ### Transaction counter -Instead of relying on a hash (32 bytes) we could use a 64 bits (8 bytes) transaction counter as nonce for the wrapper and inner transactions. The advantage is that it would only use 25% of the storage space required by the proposed solution. On the other hand, the handling of the counter for the inner transaction will be performed entirely in wasm (transactions and VPs) making it a bit less efficient. +Instead of relying on a hash (32 bytes) we could use a 64 bits (8 bytes) transaction counter as nonce for the wrapper and inner transactions. The advantage is that it would only use 25% of the storage space required by the proposed solution. On the other hand, the handling of the counter for the inner transaction will be performed entirely in wasm (transactions and VPs) making it a bit less efficient. This solution also imposes a strict ordering on the transactions issued by a same address. **NOTE**: this solution requires the ability to [yield](https://github.com/wasmerio/wasmer/issues/1127) execution from Wasmer which is not implemented yet. @@ -383,7 +383,7 @@ If one wanted to completely prevent this scenario, the solution would be to reje ### Wrapper-bound InnerTx -The solution is to tie an `InnerTx` to the corresponding `WrapperTx`. By doing so, it becomes impossible to rewrap an inner transaction and, therefore, all the attacks related to this practice would be unfeasible. This mechanism requires eight times less space in storage (only a 64 bit counter for the wrapper tx) and only one check on the wrapper counter in protocol. As a con, it requires communication between the signer of the inner transaction and that of the wrapper during the transaction construction. +The solution is to tie an `InnerTx` to the corresponding `WrapperTx`. By doing so, it becomes impossible to rewrap an inner transaction and, therefore, all the attacks related to this practice would be unfeasible. This mechanism requires eight times less space in storage (only a 64 bit counter for the wrapper tx) and only one check on the wrapper counter in protocol. As a con, it requires communication between the signer of the inner transaction and that of the wrapper during the transaction construction. This solution also imposes a strict ordering on the wrapper transactions issued by a same address. To do so we will have to change the current definition of the two tx structs to the following: From 72e696cb1f719be72ddcc1c98930da2f4555fdac Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 18 Oct 2022 16:52:28 +0200 Subject: [PATCH 09/18] Fixes optimizations stats --- documentation/specs/src/base-ledger/replay-protection.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/documentation/specs/src/base-ledger/replay-protection.md b/documentation/specs/src/base-ledger/replay-protection.md index ebfcda3f83..dae2cce3ff 100644 --- a/documentation/specs/src/base-ledger/replay-protection.md +++ b/documentation/specs/src/base-ledger/replay-protection.md @@ -186,7 +186,7 @@ In this section we describe two alternative solutions that come with some optimi ### Transaction counter -Instead of relying on a hash (32 bytes) we could use a 64 bits (8 bytes) transaction counter as nonce for the wrapper and inner transactions. The advantage is that it would only use 25% of the storage space required by the proposed solution. On the other hand, the handling of the counter for the inner transaction will be performed entirely in wasm (transactions and VPs) making it a bit less efficient. This solution also imposes a strict ordering on the transactions issued by a same address. +Instead of relying on a hash (32 bytes) we could use a 64 bits (8 bytes) transaction counter as nonce for the wrapper and inner transactions. The advantage is that the space required would be much less since we only need two 8 bytes values in storage for every address which is signing transactions. On the other hand, the handling of the counter for the inner transaction will be performed entirely in wasm (transactions and VPs) making it a bit less efficient. This solution also imposes a strict ordering on the transactions issued by a same address. **NOTE**: this solution requires the ability to [yield](https://github.com/wasmerio/wasmer/issues/1127) execution from Wasmer which is not implemented yet. @@ -383,7 +383,7 @@ If one wanted to completely prevent this scenario, the solution would be to reje ### Wrapper-bound InnerTx -The solution is to tie an `InnerTx` to the corresponding `WrapperTx`. By doing so, it becomes impossible to rewrap an inner transaction and, therefore, all the attacks related to this practice would be unfeasible. This mechanism requires eight times less space in storage (only a 64 bit counter for the wrapper tx) and only one check on the wrapper counter in protocol. As a con, it requires communication between the signer of the inner transaction and that of the wrapper during the transaction construction. This solution also imposes a strict ordering on the wrapper transactions issued by a same address. +The solution is to tie an `InnerTx` to the corresponding `WrapperTx`. By doing so, it becomes impossible to rewrap an inner transaction and, therefore, all the attacks related to this practice would be unfeasible. This mechanism requires even less space in storage (only a 64 bit counter for every address signing wrapper transactions) and only one check on the wrapper counter in protocol. As a con, it requires communication between the signer of the inner transaction and that of the wrapper during the transaction construction. This solution also imposes a strict ordering on the wrapper transactions issued by a same address. To do so we will have to change the current definition of the two tx structs to the following: From 901c05c6739878a290e2ae21b6d2ae3c8d72a8f8 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 20 Oct 2022 17:29:24 +0200 Subject: [PATCH 10/18] Fixes internal docs references --- documentation/specs/src/base-ledger/replay-protection.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/documentation/specs/src/base-ledger/replay-protection.md b/documentation/specs/src/base-ledger/replay-protection.md index dae2cce3ff..28aa98bc72 100644 --- a/documentation/specs/src/base-ledger/replay-protection.md +++ b/documentation/specs/src/base-ledger/replay-protection.md @@ -63,7 +63,7 @@ In the execution steps: 4. After the execution, the affected validity predicates (also mentioned as VP in this document) will check the storage changes and (if relevant) the signature of the transaction: if the signature is not valid, the VP will deem the transaction invalid and the changes won't be applied to the storage The signature checks effectively prevent any tampering with the transaction data because that would cause the checks to fail and the transaction to be rejected. -For a more in-depth view, please refer to the [Namada execution spec](https://specs.namada.net/base-ledger/execution.html). +For a more in-depth view, please refer to the [Namada execution spec](./execution.md). ### Tendermint replay protection @@ -220,7 +220,7 @@ The wasm transaction will simply read the value from storage and increase its va The VP of the _source_ address will then check the validity of the signature and, if it's deemed valid, will proceed to check if the pre-value of the counter in storage was equal to the one contained in the `SignedTxData` struct and if the post-value of the key in storage has been incremented by one: if any of these conditions doesn't hold the VP will discard the transactions and prevent the changes from being applied to the storage. -In the specific case of a shielded transfer, since MASP already comes with replay protection as part of the Zcash design (see the [MASP specs](https://specs.namada.net/masp.html) and [Zcash protocol specs](https://zips.z.cash/protocol/protocol.pdf)), the counter in `SignedTxData` is not required and therefore should be optional. +In the specific case of a shielded transfer, since MASP already comes with replay protection as part of the Zcash design (see the [MASP specs](../masp.md) and [Zcash protocol specs](https://zips.z.cash/protocol/protocol.pdf)), the counter in `SignedTxData` is not required and therefore should be optional. To implement replay protection for the inner transaction we will need to update all the VPs checking the transaction's signature to include the check on the transaction counter: at the moment the `vp_user` validity predicate is the only one to update. In addition, all the transactions involving `SignedTxData` should increment the counter. From 42b313b5c196f2f72256062d8c86593ba1d43074 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 21 Nov 2022 12:05:48 +0100 Subject: [PATCH 11/18] Changes expiration block to expiration datetime --- .../src/base-ledger/replay-protection.md | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/documentation/specs/src/base-ledger/replay-protection.md b/documentation/specs/src/base-ledger/replay-protection.md index 28aa98bc72..24e44e9c59 100644 --- a/documentation/specs/src/base-ledger/replay-protection.md +++ b/documentation/specs/src/base-ledger/replay-protection.md @@ -115,7 +115,7 @@ This new field will be signed just like the other ones and is therefore subject In general, a transaction is valid at the moment of submission, but after that, a series of external factors (ledger state, etc.) might change the mind of the submitter who's now not interested in the execution of the transaction anymore. -We have to introduce the concept of a lifetime (or timeout) for the transactions: basically, the `Tx` struct will hold an extra field called `expiration` stating the maximum block height up until which the submitter is willing to see the transaction executed. After the specified block height, the transaction will be considered invalid and discarded regardless of all the other checks. +We have to introduce the concept of a lifetime (or timeout) for the transactions: basically, the `Tx` struct will hold an extra field called `expiration` stating the maximum `DateTimeUtc` up until which the submitter is willing to see the transaction executed. After the specified time, the transaction will be considered invalid and discarded regardless of all the other checks. By introducing this new field we are setting a new constraint in the transaction's contract, where the ledger will make sure to prevent the execution of the transaction after the deadline and, on the other side, the submitter commits himself to the result of the execution at least until its expiration. If the expiration is reached and the transaction has not been executed the submitter can decide to submit a new transaction if he's still interested in the changes carried by it. @@ -123,11 +123,11 @@ In our design, the `expiration` will hold until the transaction is executed: onc - Transaction is invalid regardless of the specific state - Transaction is executed (either with success or not) and the transaction hash is saved in the storage -- Expiration block height has passed +- Expiration time has passed The first condition satisfied will invalidate further executions of the same tx. -In anticipation of DKG implementation, the current struct `WrapperTx` holds a field `epoch` stating the epoch in which the tx should be executed. This is because Ferveo will produce a new public key each epoch, effectively limiting the lifetime of the transaction (see section 2.2.2 of the [documentation](https://eprint.iacr.org/2022/898.pdf)). Unfortunately, for replay protection, a resolution of 1 epoch (~ 1 day) is too low for the possible needs of the submitters, therefore we need the `expiration` field to hold a maximum `BlockHeight` to increase resolution down to a single block (~ 10 seconds). +In anticipation of DKG implementation, the current struct `WrapperTx` holds a field `epoch` stating the epoch in which the tx should be executed. This is because Ferveo will produce a new public key each epoch, effectively limiting the lifetime of the transaction (see section 2.2.2 of the [documentation](https://eprint.iacr.org/2022/898.pdf)). Unfortunately, for replay protection, a resolution of 1 epoch (~ 1 day) is too low for the possible needs of the submitters, therefore we need the `expiration` field to hold a maximum `DateTimeUtc` to increase resolution down to a single block (~ 10 seconds). ```rust pub struct Tx { @@ -136,7 +136,7 @@ pub struct Tx { pub timestamp: DateTimeUtc, pub chain_id: ChainId, /// Lifetime of the transaction, also determines which decryption key will be used - pub expiration: BlockHeight, + pub expiration: DateTimeUtc, } pub struct WrapperTx { @@ -156,7 +156,7 @@ pub struct WrapperTx { Since we now have more detailed information about the desired lifetime of the transaction, we can remove the `epoch` field and rely solely on `expiration`. Now, the producer of the inner transaction should make sure to set a sensible value for this field, in the sense that it should not span more than one epoch. If this happens, then the transaction will be correctly decrypted only in a subset of the desired lifetime (the one expecting the actual key used for the encryption), while, in the following epochs, the transaction will fail decryption and won't be executed. In essence, the `expiration` parameter can only restrict the implicit lifetime within the current epoch, it can not surpass it as that would make the transaction fail in the decryption phase. -The subject encrypting the inner transaction will also be responsible for using the appropriate public key for encryption relative to the targeted block height. +The subject encrypting the inner transaction will also be responsible for using the appropriate public key for encryption relative to the targeted time. The wrapper transaction will match the `expiration` of the inner for correct execution. Note that we need this field also for the wrapper to anticipate the check at mempool/proposal evaluation time, but also to prevent someone from inserting a wrapper transaction after the corresponding inner has expired forcing the wrapper signer to pay for the fees. @@ -176,7 +176,7 @@ These checks can all be done before executing the transactions themselves (the c 2. If the checks fail _only_ because of an insufficient balance, the wrapper should be kept in mempool for a future play in case the funds should become available 3. If all the checks pass validation we will include the transaction in the block to store the hash and charge the fee -The `expiration` parameter also justifies step 2 of the previous bullet points which states that if the validity checks fail only because of an insufficient balance to pay for fees then the transaction should be kept in mempool for future execution. Without it, the transaction could be potentially executed at any future moment, possibly going against the mutated interests of the submitter. With the expiration parameter, now, the submitter commits himself to accept the execution of the transaction up to the specified block height: it's going to be his responsibility to provide a sensible value for this parameter. Given this constraint the transaction will be kept in memepool up until the expiration (since it would become invalid after that in any case), to prevent the mempool from increasing too much in size. +The `expiration` parameter also justifies step 2 of the previous bullet points which states that if the validity checks fail only because of an insufficient balance to pay for fees then the transaction should be kept in mempool for future execution. Without it, the transaction could be potentially executed at any future moment, possibly going against the mutated interests of the submitter. With the expiration parameter, now, the submitter commits himself to accept the execution of the transaction up to the specified time: it's going to be his responsibility to provide a sensible value for this parameter. Given this constraint the transaction will be kept in memepool up until the expiration (since it would become invalid after that in any case), to prevent the mempool from increasing too much in size. This mechanism can also be applied to another scenario. Suppose a transaction was not propagated to the network by a node (or a group of colluding nodes). Now, this tx might be valid, but it doesn't get inserted into a block. Without an expiration, this tx can be replayed (better, applied, since it was never executed in the first place) at a future moment in time when the submitter might not be willing to execute it anymore. @@ -275,7 +275,7 @@ The ideal solution would be to interrupt the execution of the Wasm code after th Unfortunately, at the moment, Wasmer doesn't allow [yielding](https://github.com/wasmerio/wasmer/issues/1127) from the execution. -In case the transaction went out of gas (given the `gas_limit` field of the wrapper), all the changes applied will be discarded from the WAL and will not affect the state of the storage. The inner transaction could then be rewrapped with a correct gas limit and replayed until the `expiration` block has been reached. +In case the transaction went out of gas (given the `gas_limit` field of the wrapper), all the changes applied will be discarded from the WAL and will not affect the state of the storage. The inner transaction could then be rewrapped with a correct gas limit and replayed until the `expiration` time has been reached. ##### Batching and transaction ordering @@ -396,7 +396,7 @@ pub struct WrapperTx { /// Max amount of gas that can be used when executing the inner tx pub gas_limit: GasLimit, /// Lifetime of the transaction, also determines which decryption key will be used - pub expiration: BlockHeight, + pub expiration: DateTimeUtc, /// Chain identifier for replay protection pub chain_id: ChainId, /// Transaction counter for replay protection @@ -419,7 +419,7 @@ The Wrapper transaction no longer holds the inner transaction hash while the inn pub struct WrapperCommit { pub pk: common::PublicKey, pub tx_counter: u64, - pub expiration: BlockHeight, + pub expiration: DateTimeUtc, pub chain_id: ChainId, } ``` @@ -458,7 +458,7 @@ These checks can all be done before executing the transactions themselves. The c 2. If the checks fail _only_ because of an insufficient balance, the wrapper should be kept in mempool for a future play in case the funds should become available 3. If all the checks pass validation we will include the transaction in the block to increase the counter and charge the fee -Note that, regarding point one, there's a distinction to be made about an invalid `tx_counter` which could be invalid because of being old or being in advance. To solve this last issue (counter greater than the expected one), we have to introduce the concept of a lifetime (or timeout) for the transactions: basically, the `WrapperTx` will hold an extra field called `expiration` stating the maximum block height up until which the submitter is willing to see the transaction executed. After the specified block height the transaction will be considered invalid and discarded regardless of all the other checks. This way, in case of a transaction with a counter greater than expected, it is sufficient to wait till after the expiration to submit more transactions, so that the counter in storage is not modified (kept invalid for the transaction under observation) and replaying that tx would result in a rejection. +Note that, regarding point one, there's a distinction to be made about an invalid `tx_counter` which could be invalid because of being old or being in advance. To solve this last issue (counter greater than the expected one), we have to introduce the concept of a lifetime (or timeout) for the transactions: basically, the `WrapperTx` will hold an extra field called `expiration` stating the maximum time up until which the submitter is willing to see the transaction executed. After the specified time the transaction will be considered invalid and discarded regardless of all the other checks. This way, in case of a transaction with a counter greater than expected, it is sufficient to wait till after the expiration to submit more transactions, so that the counter in storage is not modified (kept invalid for the transaction under observation) and replaying that tx would result in a rejection. This actually generalizes to a more broad concept. In general, a transaction is valid at the moment of submission, but after that, a series of external factors (ledger state, etc.) might change the mind of the submitter who's now not interested in the execution of the transaction anymore. By introducing this new field we are introducing a new constraint in the transaction's contract, where the ledger will make sure to prevent the execution of the transaction after the deadline and, on the other side, the submitter commits himself to the result of the execution at least until its expiration. If the expiration is reached and the transaction has not been executed the submitter can decide to submit a new, identical transaction if he's still interested in the changes carried by it. @@ -466,15 +466,15 @@ In our design, the `expiration` will hold until the transaction is executed, onc - Transaction is invalid regardless of the specific state - Transaction is executed (either with success or not) and the transaction counter is increased -- Expiration block height has passed +- Expiration time has passed The first condition satisfied will invalidate further executions of the same tx. -The `expiration` parameter also justifies step 2 of the previous bullet points which states that if the validity checks fail only because of an insufficient balance to pay for fees than the transaction should be kept in mempool for a future execution. Without it, the transaction could be potentially executed at any future moment (provided that the counter is still valid), possibily going against the mutated interests of the submitter. With the expiration parameter, now, the submitter commits himself to accepting the execution of the transaction up to the specified block height: it's going to be his responsibility to provide a sensible value for this parameter. Given this constraint the transaction will be kept in memepool up until the expiration (since it would become invalid after that in any case), to prevent the mempool from increasing too much in size. +The `expiration` parameter also justifies step 2 of the previous bullet points which states that if the validity checks fail only because of an insufficient balance to pay for fees than the transaction should be kept in mempool for a future execution. Without it, the transaction could be potentially executed at any future moment (provided that the counter is still valid), possibily going against the mutated interests of the submitter. With the expiration parameter, now, the submitter commits himself to accepting the execution of the transaction up to the specified time: it's going to be his responsibility to provide a sensible value for this parameter. Given this constraint the transaction will be kept in memepool up until the expiration (since it would become invalid after that in any case), to prevent the mempool from increasing too much in size. This mechanism can also be applied to another scenario. Suppose a transaction was not propagated to the network by a node (or a group of colluding nodes). Now, this tx might be valid, but it doesn't get inserted into a block. Without an expiration, if the submitter doesn't submit any other transaction (which gets included in a block to increase the transaction counter), this tx can be replayed (better, applied, since it was never executed in the first place) at a future moment in time when the submitter might not be willing to execute it any more. -Since the signer of the wrapper may be different from the one of the inner we also need to include this `expiration` field in the `WrapperCommit` struct, to prevent the signer of the wrapper from setting a lifetime which is in conflict with the interests of the inner signer. Note that adding a separate lifetime for the wrapper alone (which would require two separate checks) doesn't carry any benefit: a wrapper with a lifetime greater than the inner would have no sense since the inner would fail. Restricting the lifetime would work but it also means that the wrapper could prevent a valid inner transaction from being executed. We will then keep a single `expiration` field specifying the wrapper tx max block height (the inner one will actually be executed one block later because of the execution mechanism of Namada). +Since the signer of the wrapper may be different from the one of the inner we also need to include this `expiration` field in the `WrapperCommit` struct, to prevent the signer of the wrapper from setting a lifetime which is in conflict with the interests of the inner signer. Note that adding a separate lifetime for the wrapper alone (which would require two separate checks) doesn't carry any benefit: a wrapper with a lifetime greater than the inner would have no sense since the inner would fail. Restricting the lifetime would work but it also means that the wrapper could prevent a valid inner transaction from being executed. We will then keep a single `expiration` field specifying the wrapper tx max time (the inner one will actually be executed one block later because of the execution mechanism of Namada). To prevent the signer of the wrapper from submitting the transaction to a different chain, the `ChainId` field should also be included in the commit. From 3a43f50410dd0a3ab23cfa734d5b3d2b4bbc24bf Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 21 Nov 2022 12:21:55 +0100 Subject: [PATCH 12/18] changelog: add #440 --- .changelog/unreleased/docs/440-replay-protection-specs.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/docs/440-replay-protection-specs.md diff --git a/.changelog/unreleased/docs/440-replay-protection-specs.md b/.changelog/unreleased/docs/440-replay-protection-specs.md new file mode 100644 index 0000000000..b05bb443b3 --- /dev/null +++ b/.changelog/unreleased/docs/440-replay-protection-specs.md @@ -0,0 +1,2 @@ +- Adds specs for replay protection + ([#440](https://github.com/anoma/namada/pull/440)) \ No newline at end of file From 36f4eb363b6cc5436be904e7248a64d35ce5ba70 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 25 Oct 2022 14:48:03 +0200 Subject: [PATCH 13/18] Adds multisignature specs --- .../specs/src/base-ledger/multisignature.md | 58 ++++++++++++++++++- 1 file changed, 56 insertions(+), 2 deletions(-) diff --git a/documentation/specs/src/base-ledger/multisignature.md b/documentation/specs/src/base-ledger/multisignature.md index a32b8da81e..aa70f5afc6 100644 --- a/documentation/specs/src/base-ledger/multisignature.md +++ b/documentation/specs/src/base-ledger/multisignature.md @@ -1,3 +1,57 @@ -## k-of-n multisignature +# k-of-n multisignature -The k-of-n multisignature validity predicate authorises transactions on the basis of k out of n parties approving them. \ No newline at end of file +The k-of-n multisignature validity predicate authorises transactions on the basis of k out of n parties approving them. This document targets the encrypted wasm transactions: at the moment there's no support for multisignature on wrapper or protocol transactions. + +## Protocol + +Namada transactions get signed before being delivered to the network. This signature is then checked by the VPs to determine the validity of the transaction. To support multisignature we need to extend the current `SignedTxData` struct to the following: + +```rust +pub enum Signature { + Sig(common::Signature), + MultiSig(Vec) +} + +pub struct SignedTxData { + /// The original tx data bytes, if any + pub data: Option>, + /// The signature is produced on the tx data concatenated with the tx code + /// and the timestamp. + pub sig: Signature, +} +``` + +This struct will now hold either a signature or multiple signatures over the data carried by the transaction. The different enum variants allow for a quick check of the correct signature type at validation time. + +## VPs + +To support multisig we provide a new `vp_multisig` wasm validity predicate that can be used instead of the usual `vp_user` for `implicit addresses` (see [spec](./default-account.md)). This new vp will be generic, it will allow for arbitrary actions on the account as long as the signatures are valid. + +Moreover, `established` and `internal` addresses may want a multi-signature scheme on top of their validation process. Among the internal ones, `PGF` will require multisignature for its council (see the [relative](../economics/public-goods-funding.md) spec). + +To support the validity checks, the VP will need to access two types of information: + +1. The multisig threshold +2. A list of valid signers' public keys + +This data defines the requirements of a valid transaction operating on the multisignature address and it will be written in storage when the account is first created: + +``` +/\$Address/multisig/threshold/: u8 +/\$Address/multisig/pubkeys/: Vec +``` + +To verify the correctness of the signatures, these VPs will proceed with a four-steps verification process: + +1. Check that the type of the signature is `MultiSig` +2. Check to have enough **unique** signatures for the given threshold +3. Validate the signatures +4. Check to have enough **valid** signatures for the given threshold + +Steps 1 and 2 allow to short-circuit the validation process and avoid unnecessary processing and storage access. The signatures will be validated against the list of predefined public keys: a signature will be rejected if it's not valid for any of these public keys. Step 4 will halt as soon as it retrieves enough valid signatures to match the threshold, meaning that the remaining signatures will not be verified. + +## Transaction construction + +To craft a multisigned transaction, the involved parties will need to coordinate. More specifically, the transaction will be constructed by one entity which will then distribute it to the signers and collect their signatures: note that the constructing party doesn't necessarily need to be one of the signers. Finally, these signatures will be inserted in the `SignedTxData` struct so that they can be encrypted, wrapped and submitted to the network. + +Namada does not provide a layer to support this process, so the involved parties will need to rely on an external communication mechanism. From 1f3b0be93c5463aa6cbdff0fa03570401ebaa7bc Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 22 Nov 2022 14:38:30 +0100 Subject: [PATCH 14/18] Updates multisig specs --- .../specs/src/base-ledger/multisignature.md | 65 +++++++++++++++---- 1 file changed, 52 insertions(+), 13 deletions(-) diff --git a/documentation/specs/src/base-ledger/multisignature.md b/documentation/specs/src/base-ledger/multisignature.md index aa70f5afc6..2ba2f78112 100644 --- a/documentation/specs/src/base-ledger/multisignature.md +++ b/documentation/specs/src/base-ledger/multisignature.md @@ -9,7 +9,7 @@ Namada transactions get signed before being delivered to the network. This signa ```rust pub enum Signature { Sig(common::Signature), - MultiSig(Vec) + MultiSig(Vec<(u8, common::Signature)) } pub struct SignedTxData { @@ -21,37 +21,76 @@ pub struct SignedTxData { } ``` -This struct will now hold either a signature or multiple signatures over the data carried by the transaction. The different enum variants allow for a quick check of the correct signature type at validation time. +The `MultiSig` variant holds a vector of tuples where the first element is an 8-bit integer and a the second one is a signature. The integer serves as an index to match a specific signature to one of the public keys in the list of accepted ones. This way, we can improve the verification algorithm and check each signature only against the public key at the provided index (linear in time complexity), without the need to cycle on all of them which would be $\mathcal{O}(n^2)$. ## VPs -To support multisig we provide a new `vp_multisig` wasm validity predicate that can be used instead of the usual `vp_user` for `implicit addresses` (see [spec](./default-account.md)). This new vp will be generic, it will allow for arbitrary actions on the account as long as the signatures are valid. +To support multisig we provide a new generic `vp_multisig` wasm validity predicate that will allow for arbitrary actions on the account as long as the signatures are valid. Note that these modifications can affect the multisig account parameters themselves. -Moreover, `established` and `internal` addresses may want a multi-signature scheme on top of their validation process. Among the internal ones, `PGF` will require multisignature for its council (see the [relative](../economics/public-goods-funding.md) spec). - -To support the validity checks, the VP will need to access two types of information: +To perform the validity checks, the VP will need to access two types of information: 1. The multisig threshold 2. A list of valid signers' public keys -This data defines the requirements of a valid transaction operating on the multisignature address and it will be written in storage when the account is first created: +This data defines the requirements of a valid transaction operating on the multisignature address and it will be written in storage when the account is created: ``` /\$Address/multisig/threshold/: u8 -/\$Address/multisig/pubkeys/: Vec +/\$Address/multisig/pubkeys/: LazyVec ``` -To verify the correctness of the signatures, these VPs will proceed with a four-steps verification process: +The `LazyVec` struct will split all of its element on different subkeys in storage so that we won't need to load the entire vector of public keys in memory for validation but just the ones pointed by the indexes in the `SignedTxData` struct. + +To verify the correctness of the signatures, these VPs will proceed with a three-steps verification process: 1. Check that the type of the signature is `MultiSig` 2. Check to have enough **unique** signatures for the given threshold -3. Validate the signatures -4. Check to have enough **valid** signatures for the given threshold +3. Check to have enough **valid** signatures for the given threshold + +Steps 1 and 2 allow to short-circuit the validation process and avoid unnecessary processing and storage access. Each signature will be validated **only** against the public key found in list at the specified index. Step 3 will halt as soon as it retrieves enough valid signatures to match the threshold, meaning that the remaining signatures will not be verified. + +In the transaction initiating the established address, the submitter will be able to provide a custom VP if the provided one doesn't fully satisfy the desired requirements. + +## Addresses + +The multisig vp introduced in the previous section is available for `established` addresses. To generate a multisig account we provide a new `tx_init_multisig_account` wasm transaction to be used instead of the already available `tx_init_account`. A multisig account can be created by anyone and the creator is responsible for providing the correct data, represented by the following struct: + +```rust +pub struct InitMultiSigAccount { + /// The VP code + pub vp_code: Vec, + /// Multisig threshold for k-of-n + pub threshold: u8, + /// Multisig signers' pubkeys + pub pubkeys: Vec +} +``` -Steps 1 and 2 allow to short-circuit the validation process and avoid unnecessary processing and storage access. The signatures will be validated against the list of predefined public keys: a signature will be rejected if it's not valid for any of these public keys. Step 4 will halt as soon as it retrieves enough valid signatures to match the threshold, meaning that the remaining signatures will not be verified. +Finally, the tx performs the following writes to storage: + +- The multisig vp +- The threshold +- The list of public keys of the signers + +No checks will be run on these parameters, meaning that the creator could provide wrong data: to perform validation on this data we would need an internal VP managing the creation of every multisig account. This VP, though, is not required since in case of an error the creator can simply submit a new transaction to generate the correct account. On the other side, the participants of a multisig account can refuse to sign transactions if they don't agree on the parameters defining the account itself. + +`Internal` addresses may want a multi-signature scheme on top of their validation process as well. Among the internal ones, `PGF` will require multisignature for its council (see the [relative](../economics/public-goods-funding.md) spec). The storage data necessary for the correct working of the multisig for an internal address are written in the genesis file: these keys can be later modified through governance. + +`Implicit` addresses are not generated by a transaction and therefore are not suitable for a multisignature scheme since there would be no way to properly construct them. More specifically, an implicit address doesn't allow for: + +- A custom, modifiable VP +- An initial transaction to be used as an initializer for the relevant data ## Transaction construction -To craft a multisigned transaction, the involved parties will need to coordinate. More specifically, the transaction will be constructed by one entity which will then distribute it to the signers and collect their signatures: note that the constructing party doesn't necessarily need to be one of the signers. Finally, these signatures will be inserted in the `SignedTxData` struct so that they can be encrypted, wrapped and submitted to the network. +To craft a multisigned transaction, the involved parties will need to coordinate. More specifically, the transaction will be constructed by one entity which will then distribute it to the signers and collect their signatures: note that the constructing party doesn't necessarily need to be one of the signers. Finally, these signatures will be inserted in the `SignedTxData` struct so that it can be encrypted, wrapped and submitted to the network. Namada does not provide a layer to support this process, so the involved parties will need to rely on an external communication mechanism. + +## Replay protection + +The [replay protection](./replay-protection.md) mechanism of Namada will prevent third-party malicious users from replaying a transaction having a multisig account as the source. This mechanism, though, is not enough to completely protect multisigned transactions: in this case, in fact, the threat could come from the members of the account themselves. + +With the current hash-based replay protection mechanism, once the transaction has been executed, its hash gets stored to prevent a replay. The issue, with a multisig transaction, is that the same transaction with a different set or amount of signatures would have a different hash, meaning that it can be replayed. + +There's no way to mitigate this scenario with the current implementation, so the members of a multisignature account will be responsible for behaving honestly towards each other and, possibly, punish malicious users by excluding them from the account. From 9f9f799651fc3cc1fa46ba4429150a070dc66683 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Wed, 23 Nov 2022 15:28:45 +0100 Subject: [PATCH 15/18] Updates multisig specs checks --- .../specs/src/base-ledger/multisignature.md | 36 ++++++++++++++----- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/documentation/specs/src/base-ledger/multisignature.md b/documentation/specs/src/base-ledger/multisignature.md index 2ba2f78112..0a274fd6e0 100644 --- a/documentation/specs/src/base-ledger/multisignature.md +++ b/documentation/specs/src/base-ledger/multisignature.md @@ -1,6 +1,6 @@ # k-of-n multisignature -The k-of-n multisignature validity predicate authorises transactions on the basis of k out of n parties approving them. This document targets the encrypted wasm transactions: at the moment there's no support for multisignature on wrapper or protocol transactions. +The k-of-n multisignature validity predicate authorizes transactions on the basis of k out of n parties approving them. This document targets the encrypted wasm transactions: at the moment there's no support for multisignature on wrapper or protocol transactions. ## Protocol @@ -21,7 +21,7 @@ pub struct SignedTxData { } ``` -The `MultiSig` variant holds a vector of tuples where the first element is an 8-bit integer and a the second one is a signature. The integer serves as an index to match a specific signature to one of the public keys in the list of accepted ones. This way, we can improve the verification algorithm and check each signature only against the public key at the provided index (linear in time complexity), without the need to cycle on all of them which would be $\mathcal{O}(n^2)$. +The `MultiSig` variant holds a vector of tuples where the first element is an 8-bit integer and the second one is a signature. The integer serves as an index to match a specific signature to one of the public keys in the list of accepted ones. This way, we can improve the verification algorithm and check each signature only against the public key at the provided index (linear in time complexity), without the need to cycle on all of them which would be $\mathcal{O}(n^2)$. ## VPs @@ -39,17 +39,17 @@ This data defines the requirements of a valid transaction operating on the multi /\$Address/multisig/pubkeys/: LazyVec ``` -The `LazyVec` struct will split all of its element on different subkeys in storage so that we won't need to load the entire vector of public keys in memory for validation but just the ones pointed by the indexes in the `SignedTxData` struct. +The `LazyVec` struct will split all of its elements on different subkeys in storage so that we won't need to load the entire vector of public keys in memory for validation but just the ones pointed by the indexes in the `SignedTxData` struct. -To verify the correctness of the signatures, these VPs will proceed with a three-steps verification process: +To verify the correctness of the signatures, these VPs will proceed with a three-step verification process: 1. Check that the type of the signature is `MultiSig` 2. Check to have enough **unique** signatures for the given threshold 3. Check to have enough **valid** signatures for the given threshold -Steps 1 and 2 allow to short-circuit the validation process and avoid unnecessary processing and storage access. Each signature will be validated **only** against the public key found in list at the specified index. Step 3 will halt as soon as it retrieves enough valid signatures to match the threshold, meaning that the remaining signatures will not be verified. +Steps 1 and 2 allow us to short-circuit the validation process and avoid unnecessary processing and storage access. Each signature will be validated **only** against the public key found in the list at the specified index. Step 3 will halt as soon as it retrieves enough valid signatures to match the threshold, meaning that the remaining signatures will not be verified. -In the transaction initiating the established address, the submitter will be able to provide a custom VP if the provided one doesn't fully satisfy the desired requirements. +In the transaction initiating the established address, the submitter will be able to provide a custom VP if the provided one doesn't impose the desired constraints. ## Addresses @@ -72,15 +72,33 @@ Finally, the tx performs the following writes to storage: - The threshold - The list of public keys of the signers -No checks will be run on these parameters, meaning that the creator could provide wrong data: to perform validation on this data we would need an internal VP managing the creation of every multisig account. This VP, though, is not required since in case of an error the creator can simply submit a new transaction to generate the correct account. On the other side, the participants of a multisig account can refuse to sign transactions if they don't agree on the parameters defining the account itself. - `Internal` addresses may want a multi-signature scheme on top of their validation process as well. Among the internal ones, `PGF` will require multisignature for its council (see the [relative](../economics/public-goods-funding.md) spec). The storage data necessary for the correct working of the multisig for an internal address are written in the genesis file: these keys can be later modified through governance. -`Implicit` addresses are not generated by a transaction and therefore are not suitable for a multisignature scheme since there would be no way to properly construct them. More specifically, an implicit address doesn't allow for: +`Implicit` addresses are not generated by a transaction and, therefore, are not suitable for a multisignature scheme since there would be no way to properly construct them. More specifically, an implicit address doesn't allow for: - A custom, modifiable VP - An initial transaction to be used as an initializer for the relevant data +## Multisig account init validation + +Since the VP of an established account does not get triggered at account creation, no checks will be run on the multisig parameters, meaning that the creator could provide wrong data. + +To perform validation at account creation time we could: + +1. Write in storage the addresses together with the public keys to trigger their VPs +2. Manually trigger the multisig VP even at creation time +3. Create an internal VP managing the creation of every multisig account + +All of these solutions would require the init transaction to become a multisigned one. + +Solution 1 actually exhibits a problem: in case the members of the account would like to exclude one of them from the account, the target account could refuse to sign the multisig transaction carrying this modification. At validation time, his private VP will be triggered and, since there's no signature matching his own public key in the transaction, it would reject it effectively preventing the multisig account to operate on itself even with enough signatures to match the threshold. This goes against the principle that a multisig account should be self-sufficient and controlled by its own VP and not those of its members. + +Solution 2 would perform just a partial check since the logic of the VP will revolve around the threshold. + +Finally, solution 3 would require an internal VP dedicated to the management of multisig addresses' parameters both at creation and modification time. This could implement a logic based on the threshold or a logic requiring a signature by all the members to initialize/modify a multisig account's parameters. The former effectively collapses to the VP of the account itself (making the internal VP redundant), while the latter has the same problem as solution 1. + +In the end, we don't implement any of these checks and will leave the responsibility to the signer of the transaction creating the address: in case of an error he can simply submit a new transaction to generate the correct account. On the other side, the participants of a multisig account can refuse to sign transactions if they don't agree on the parameters defining the account itself. + ## Transaction construction To craft a multisigned transaction, the involved parties will need to coordinate. More specifically, the transaction will be constructed by one entity which will then distribute it to the signers and collect their signatures: note that the constructing party doesn't necessarily need to be one of the signers. Finally, these signatures will be inserted in the `SignedTxData` struct so that it can be encrypted, wrapped and submitted to the network. From 4378fb02b54f52559010684f73572226a08eadf9 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 29 Nov 2022 15:01:00 +0100 Subject: [PATCH 16/18] Merges normal and multisig accounts into one. Misc updates --- .../specs/src/base-ledger/multisignature.md | 48 +++++++------------ 1 file changed, 18 insertions(+), 30 deletions(-) diff --git a/documentation/specs/src/base-ledger/multisignature.md b/documentation/specs/src/base-ledger/multisignature.md index 0a274fd6e0..7be36c156a 100644 --- a/documentation/specs/src/base-ledger/multisignature.md +++ b/documentation/specs/src/base-ledger/multisignature.md @@ -1,31 +1,28 @@ # k-of-n multisignature -The k-of-n multisignature validity predicate authorizes transactions on the basis of k out of n parties approving them. This document targets the encrypted wasm transactions: at the moment there's no support for multisignature on wrapper or protocol transactions. +The k-of-n multisignature validity predicate authorizes transactions on the basis of k out of n parties approving them. This document targets the encrypted wasm transactions: there won't be support for multisignature on wrapper or protocol transactions. ## Protocol -Namada transactions get signed before being delivered to the network. This signature is then checked by the VPs to determine the validity of the transaction. To support multisignature we need to extend the current `SignedTxData` struct to the following: +Namada transactions get signed before being delivered to the network. This signature is then checked by the VPs to determine the validity of the transaction. To support multisignature we need to modify the current `SignedTxData` struct to the following: ```rust -pub enum Signature { - Sig(common::Signature), - MultiSig(Vec<(u8, common::Signature)) -} - pub struct SignedTxData { /// The original tx data bytes, if any pub data: Option>, /// The signature is produced on the tx data concatenated with the tx code /// and the timestamp. - pub sig: Signature, + pub sig: Vec<(u8, common::Signature)>, } ``` -The `MultiSig` variant holds a vector of tuples where the first element is an 8-bit integer and the second one is a signature. The integer serves as an index to match a specific signature to one of the public keys in the list of accepted ones. This way, we can improve the verification algorithm and check each signature only against the public key at the provided index (linear in time complexity), without the need to cycle on all of them which would be $\mathcal{O}(n^2)$. +The `sig` field now holds a vector of tuples where the first element is an 8-bit integer and the second one is a signature. The integer serves as an index to match a specific signature to one of the public keys in the list of accepted ones. This way, we can improve the verification algorithm and check each signature only against the public key at the provided index (linear in time complexity), without the need to cycle on all of them which would be $\mathcal{O}(n^2)$. + +This means that non-multisig addresses will now be seen as 1-of-1 multisig accounts. ## VPs -To support multisig we provide a new generic `vp_multisig` wasm validity predicate that will allow for arbitrary actions on the account as long as the signatures are valid. Note that these modifications can affect the multisig account parameters themselves. +Since all the addresses will be multisig ones, we will keep using the already available `vp_user` as the default validity predicate. The only modification required is the signature check which must happen on a set of signatures instead of a single one. To perform the validity checks, the VP will need to access two types of information: @@ -35,33 +32,32 @@ To perform the validity checks, the VP will need to access two types of informat This data defines the requirements of a valid transaction operating on the multisignature address and it will be written in storage when the account is created: ``` -/\$Address/multisig/threshold/: u8 -/\$Address/multisig/pubkeys/: LazyVec +/\$Address/threshold/: u8 +/\$Address/pubkeys/: LazyVec ``` The `LazyVec` struct will split all of its elements on different subkeys in storage so that we won't need to load the entire vector of public keys in memory for validation but just the ones pointed by the indexes in the `SignedTxData` struct. -To verify the correctness of the signatures, these VPs will proceed with a three-step verification process: - -1. Check that the type of the signature is `MultiSig` -2. Check to have enough **unique** signatures for the given threshold -3. Check to have enough **valid** signatures for the given threshold +To verify the correctness of the signatures, this VP will proceed with a two-step verification process: -Steps 1 and 2 allow us to short-circuit the validation process and avoid unnecessary processing and storage access. Each signature will be validated **only** against the public key found in the list at the specified index. Step 3 will halt as soon as it retrieves enough valid signatures to match the threshold, meaning that the remaining signatures will not be verified. +1. Check to have enough **unique** signatures for the given threshold +2. Check to have enough **valid** signatures for the given threshold -In the transaction initiating the established address, the submitter will be able to provide a custom VP if the provided one doesn't impose the desired constraints. +Step 1 allows us to short-circuit the validation process and avoid unnecessary processing and storage access. Each signature will be validated **only** against the public key found in the list at the specified index. Step 2 will halt as soon as it retrieves enough valid signatures to match the threshold, meaning that the remaining signatures will not be verified. ## Addresses -The multisig vp introduced in the previous section is available for `established` addresses. To generate a multisig account we provide a new `tx_init_multisig_account` wasm transaction to be used instead of the already available `tx_init_account`. A multisig account can be created by anyone and the creator is responsible for providing the correct data, represented by the following struct: +The vp introduced in the previous section is available for `established` addresses. To generate a multisig account we need to modify the `InitAccount` struct to support multiple public keys and a threshold, as follows: ```rust -pub struct InitMultiSigAccount { +pub struct InitAccount { /// The VP code pub vp_code: Vec, /// Multisig threshold for k-of-n pub threshold: u8, - /// Multisig signers' pubkeys + /// Multisig signers' pubkeys to be written into the account's storage. This can be used + /// for signature verification of transactions for the newly created + /// account. pub pubkeys: Vec } ``` @@ -104,11 +100,3 @@ In the end, we don't implement any of these checks and will leave the responsibi To craft a multisigned transaction, the involved parties will need to coordinate. More specifically, the transaction will be constructed by one entity which will then distribute it to the signers and collect their signatures: note that the constructing party doesn't necessarily need to be one of the signers. Finally, these signatures will be inserted in the `SignedTxData` struct so that it can be encrypted, wrapped and submitted to the network. Namada does not provide a layer to support this process, so the involved parties will need to rely on an external communication mechanism. - -## Replay protection - -The [replay protection](./replay-protection.md) mechanism of Namada will prevent third-party malicious users from replaying a transaction having a multisig account as the source. This mechanism, though, is not enough to completely protect multisigned transactions: in this case, in fact, the threat could come from the members of the account themselves. - -With the current hash-based replay protection mechanism, once the transaction has been executed, its hash gets stored to prevent a replay. The issue, with a multisig transaction, is that the same transaction with a different set or amount of signatures would have a different hash, meaning that it can be replayed. - -There's no way to mitigate this scenario with the current implementation, so the members of a multisignature account will be responsible for behaving honestly towards each other and, possibly, punish malicious users by excluding them from the account. From f239661b849852d2a57744271d0c96c1154ce530 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 29 Nov 2022 15:34:04 +0100 Subject: [PATCH 17/18] Fixes hash on unsigned txs --- documentation/specs/src/base-ledger/replay-protection.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/specs/src/base-ledger/replay-protection.md b/documentation/specs/src/base-ledger/replay-protection.md index 24e44e9c59..1094460cad 100644 --- a/documentation/specs/src/base-ledger/replay-protection.md +++ b/documentation/specs/src/base-ledger/replay-protection.md @@ -79,7 +79,7 @@ The actual Wasm code and data for the transaction are encapsulated inside a stru `WrapperTx` is the only type of transaction currently accepted by the ledger. It must be protected from replay attacks because, if it wasn't, a malicious user could replay the transaction as is. Even if the inner transaction implemented replay protection or, for any reason, wasn't accepted, the signer of the wrapper would still pay for gas and fees, effectively suffering economic damage. -To prevent the replay of both these transactions we will rely on a set of already processed transactions' digests that will be kept in storage. These digests will be computed on the **signed** transactions. To support this, we'll need a subspace in storage headed by a `ReplayProtection` internal address: +To prevent the replay of both these transactions we will rely on a set of already processed transactions' digests that will be kept in storage. These digests will be computed on the **unsigned** transactions, to support replay protection even for [multisigned](multisignature.md) transactions: in this case, if hashes were taken from the signed transactions, a different set of signatures on the same tx would produce a different hash, effectively allowing for a replay. To support this, we'll need a subspace in storage headed by a `ReplayProtection` internal address: ``` /$ReplayProtectionAddress/$tx0_hash: None From 7e72eb85bab73f95f4aaf4b3dddcfd9a7c51bbd2 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 1 Dec 2022 14:55:43 +0100 Subject: [PATCH 18/18] changelog: add #680 --- .changelog/unreleased/docs/680-multisig-specs.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/docs/680-multisig-specs.md diff --git a/.changelog/unreleased/docs/680-multisig-specs.md b/.changelog/unreleased/docs/680-multisig-specs.md new file mode 100644 index 0000000000..8c2abf92ee --- /dev/null +++ b/.changelog/unreleased/docs/680-multisig-specs.md @@ -0,0 +1,2 @@ +- Adds specs for multisig accounts + ([#680](https://github.com/anoma/namada/pull/680)) \ No newline at end of file