ethereumproject / ECIPs

The Ethereum Classic Improvement Proposal
55 stars 47 forks source link

ECIP1011 for Replay Attack Fix Using Change in the Signing Process #10

Open avtarsehra opened 7 years ago

avtarsehra commented 7 years ago

See the draft version of the ECIP1011 here:

https://github.com/avtarsehra/ECIPs/blob/master/ECIPs/ECIP1011.md

I beleive changing the signing process would be the simplest and cleanest method for fixing this. While future proofing is useful, I personally feel that a robust, secure, clean and quick fix right now that can be upgraded later would be the best option. This approach allows you to use a constant value, which can be made into a dynamic value at a later stage. But I think trying to implement a solution that can cover all future forks would be overkill, and would require reference to the blockchain thus minimising the possibilities for creating offline transactions.

elaineo commented 7 years ago

Future forks might not even need to worry about replay attacks, right? If we leave difficulty bombs behind...

splix commented 7 years ago

And fork 1920k is already behind, and it's a well known block. It's too late to introduce any fix for that particular block, it already have several fixes like SafeSplit contract and similar

avtarsehra commented 7 years ago

Yes I agree. This is why we could even utilise the approach suggested in the ECIP, but just use an arbitrary constant hash for the change. Maybe something that can be easily adjusted without hardcoding, so if a change is required in future it can be executed by users directly - even in the case of light clients.

avtarsehra commented 7 years ago

I had a detailed chat with @splix on this, around using a dynamic value for the appended value in the signing and validation process. The outcomes of this made sense so I have updated the ECIP based on these discussions.

marcusrbrown commented 7 years ago

The pros of this algorithm are pretty straightforward:

However it does not provide replay prevention within the same chain, or compatible chains (e.g a private chain based off of ETC).

Another issue is that the client will have to keep track of all previous and current block hashes in order to sync the blockchain from the beginning. This is especially true for light clients - how do you communicate when the hash change occurs, and how do light clients verify transactions at any given point in time? My proposal suffers from the same issue - dependency on a stable block number that can change. Once you have to maintain more than one block hash, you begin increasing the cost of verifying transactions to the receiver, since the receiver has to iterate over the current and previous hashes prior to recovery.

An alternative is to use a pool of block numbers that are frozen at an interval (128 or 256 blocks). The pool could exist inside of a contract. That solves the issue of having a constant, verifiable source of hashes to test against. That still doesn't solve the problem of the receiver having to compute the KEC() of the transactions + block hash for every hash in the pool.

BTW, I don't agree that we are up against the clock where we need to avoid future-proofing the algorithm against additional forks or cousin chains. We have the opportunity to solve this issue for the foreseeable future. We are close to having an elegant solution, let's spend a bit more time iterating on it.

avtarsehra commented 7 years ago

@igetgames with the new modification the following is not the case:

However it does not provide replay prevention within the same chain, or compatible chains (e.g a private chain based off of ETC). We have introduced a dynamic variable that is appended to transactions in the signing process. This dynamic variable is the block hash chosen based on a running lead time of 6 epochs (~24hours). But there is also an option for light clients to use a default variable of 0, so they don't need online access. This would be a compromise for building transactions completely offline.

In this update we included an approach where validation of a signed transaction can be done with a pool of 7 block header hashes from last 7 days.

I am not a big fan of relying on a contract for referencing protocol level parameters. I don't think I can give you a good answer to that, except that if had to write this up in the yellow paper spec (which I am planning to do as we go forward) that would just look very inelegant. But that is just me, and may not be everyone's pragmatic PoV.

Yes I agree with you, I don't think we should be driven to compromise on a a good solution, as you say we are close to finding something that would be really cool!

marcusrbrown commented 7 years ago

@avtarsehra and I are discussing this on Slack. We'll update this thread in a bit.

avtarsehra commented 7 years ago

I have made an update to the ECIP by formalising the math, so the appropriate reference block number can be calculated quickly using just the current block height.

Also I initially thought that a flaw in this approach is that delayed transactions would become invalid if they are delayed longer than the required validation period. But maybe this would become a feature, as this is true anyway if you delay a signed transaction and the account nonce changes in the meantime.

splix commented 7 years ago

To find a most recent reference block it's easier to use boolean logic. For slot of 1024 blocks, you can use 0b11111111111111111111110000000000 or 0xfffffc00 hex, and just have B_num && 0xfffffc00 (that's for 32 bits, of course)

2373596 && 0xfffffc00 == 2372608
2373995 && 0xfffffc00 == 2373632 == 2372608 + 1024
2373996 && 0xfffffc00 == 2373632
marcusrbrown commented 7 years ago

Also I initially thought that a flaw in this approach is that delayed transactions would become invalid if they are delayed longer than the required validation period. But maybe this would become a feature, as this is true anyway if you delay a signed transaction and the account nonce changes in the meantime.

You can postdate a transaction by giving it a higher nonce than the account nonce. The unsent transaction is still valid as long as the account nonce doesn't reach or exceed that value. This is the attack vector of replay attacks in the first place!

This ECIP adds an implicit expiry time to transactions via the 7 day validity period. This is a separate feature that does not address replay prevention. If we want to support expiring (non-offline) transactions then that should be in a separate specification. But that's debatable.

The epoch block mechanism described here doesn't provide replay protection at the actual time a fork occurs. If chain A forks into chain B, and clients on chain A do not upgrade or refuse to switch to the fork, then you have up to 7 days where transactions can be replayed from B onto A. This is because until the next epoch, both A and B have the same pool of block hashes from the current epoch counting backwards a week. The pool of block hashes won't start to differ on both chains until the next epoch occurs, up to a day later. However, transactions older than a day (up to 6 days) can still be replayed between B to A until 6 additional epochs occur on the fork block and the entire pool of block hashes no longer agree.

I can't sign off on a ECIP that leaves gaps at its highest level of security where replay attacks can still occur. Although it has seen many revisions, at its core it is designed to avoid increasing the size of transactions by a mere 32 bytes, with the tradeoff being increased computations on the verifier. With the additional field specifying a block hash, you could do a simple binary search of your pool of block hashes, which could consist of the most recent fork block AND a pool of the most recent block hashes (pick 128, 256, 1024 - whatever you can live with on the client). While this does add an implicit expiry to transactions as well, you can provide absolute protection against replays by including the hash of the most recent fork block. Clients that specify 0 as the block hash value are still susceptible to having their transaction replayed.

avtarsehra commented 7 years ago

Thanks for the great input.

Technically only the transactions executed in the first 24 hours (lag time of 6 epochs or 6144blocks) can be replayed, but this lag time can be reduced to 4 hours (1 epoch or 1024 blocks). The validation period is about how far back transactions will be allowed, as you say an implicit expiry time. Personally I thought this is of value, but agreed that should be discussed in separate ECIP.

Also, I think you may be too focused on the last part of the proposal, which is the dynamic signing variable. But Most of the ECIP is about the process itself, and we can easily just use a fixed variable or a variable from a fixed pool (as in your suggestion). So technically you would have immediate replay protection but with no transaction size increase.

I'm certainly not 100% against adding an extra field to the transaction structure, but I think it needs to be done in a thought through way. To be honest going through the discussions in the Ethereum thread where they discussed that I don't feel they really exhausted alternatives - it was a bit of group think going on there :-). But anyway, if after these discussions it turns out that adding an extra field is the optimal approach I wouldn't be against it.

marcusrbrown commented 7 years ago

Ok, I think we are almost there :)

I've pointed out the two issues I have with the dynamic variable, the first being "slippage" during the epoch, the second being the number of hashes you have to try per transaction when pulling from the pool. The transaction size increase would mitigate that second concern, but it's not a requirement to get this working with your method.

Instead let's just enforce that the signing variable is either one of:

  1. 0 (null hash) to support offline transactions
  2. The block hash of the most recent fork, T_a, where T_a is the hash of the Genesis block if no forks have occurred.

The rationale is that the client has to have the block number of the current and potentially pending fork block (or 0 if no fork is planned) in order to function properly across the entire blockchain. So if the only acceptable "online" T_a value is the most recent fork block, then there is no possibility of transactions being published on a fork where they don't belong. As soon as the fork happens, A (the non-fork) is using the previous block hash and its transactions cannot be replayed onto B (the fork). In the same vein, B is now signing transactions with the hash of the most recent fork block, so it cannot have transactions replayed onto A.

I think we should leave the transaction expiry as a separate feature so that we can solve replay attacks definitively.

avtarsehra commented 7 years ago

I'm happy with your suggestion. Dynamic signing variable is a nice to have from my view, but I agree it needs more analysis and simulations to find the optimal parameters. I will continue working on it as I think it can be used in some way...

marcusrbrown commented 7 years ago

Would you be OK with going forward with client implementation using the most recent block hash? That value is trivially accessible. If you come up with another solution for the dynamic variable then it will be easy to plug in at that point.

avtarsehra commented 7 years ago

Yes definitely that would be ok. If it is a fixed value it doesn't matter which is used, so most recent block hash would be good. The other can be null (as you mentioned) for offline client support.

elaineo commented 7 years ago

I agree with @igetgames on the dynamic signing var -- this is a separate spec that changes the current functionality, and I don't see the rationale for it.

The general procedure of changing the signing process looks like a good solution to me.

avtarsehra commented 7 years ago

I'm good with that. I will remove the last section in the ECIP connected to the dynamic signing variable.

avtarsehra commented 7 years ago

@splix @elaineo @igetgames I have made the update as discussed above. Just for reference I have left the below part in, but in the main body included some of the points from above.

On a related note, we will need to get back to devs for parity etc and discuss timelines regarding parity changes. If everyone is ok with what we got we can share the ECIP for the diff bomb delay, and one for the replay fix. We will need to include our timelines i.e. when we planning to go live.

whatisgravity commented 7 years ago

This is to be stacked with the difficulty bomb so it will not cause unnecessary interruptions with older clients?

realcodywburns commented 7 years ago

light clients and hardware wallets shouldn't be affected by the change as block header hash can be 0 if the dynamic variable is used. Wallet addresses would be unaffected but will be unable to send any transactions with old clients after the switch. Enough time needs to be given to the exchanges and miners to upgrade their systems for a smooth transition. the clients can be released with the fix asap and then the trigger block can engage both changes at once globally.

marcusrbrown commented 7 years ago

light clients and hardware wallets shouldn't be affected by the change as block header hash can be 0 if the dynamic variable is used.

All clients are affected. Hashing an additional 32 bytes of '0' will make the transaction hash differ on clients that only hash the existing transaction structure.

Enough time needs to be given to the exchanges and miners to upgrade their systems for a smooth transition. the clients can be released with the fix asap and then the trigger block can engage both changes at once globally.

I would agree except that replay attacks are an ongoing and critical issue. The implementation can be written to trigger on a specific block sometime in November (giving enough time for Parity and other implementations to apply and test, and for announcements to be made), and future-proofed because we know the difficulty bomb delay will occur at 3000000. The code will simply check the block height to retrieve the most recent fork hash.

An advantage of fixing replay attacks sooner than later is that it will demonstrate that the Classic developers can coordinate critical fixes and roll them out in a timely fashion.

realcodywburns commented 7 years ago

This is very true. Is the plan to issue this as a eip as well? Losing ledger, trezor, and what ever else will be unfortunate. Also cew and cew-cx will need a patch

elaineo commented 7 years ago

@igetgames I don't think a November trigger is realistic. I would love to have the replay fixed before ETH, but would prefer to spend more time having our clients fully tested and getting it propagated to everyone with plenty of advance notice.

marcusrbrown commented 7 years ago

@realcodywburns I definitely think we should issue this as an EIP, but I would want @avtarsehra to sign off on it first.

@elaineo Understood.

realcodywburns commented 7 years ago

Is this PR ready to be merged?

elaineo commented 7 years ago

Looks like ETH will have a hard fork in about 2 weeks (block 2,642,462) to implement this: https://github.com/ethereum/EIPs/issues/155

avtarsehra commented 7 years ago

@elaineo yes that's great they are doing it. But would also be good for ETC to implement our own version, so if ETC forks we can prevent replay issues between ETC and other chains that may emerge. We are using this as a test for new dev's. @Splix and I were supposed to have a call with them today but unfortunately it had to be cancelled at the last minute. I will follow-up on that tomorrow and provide an update.

elaineo commented 7 years ago

do we want our own version? if the changes are compatible except for the CHAIN_ID, then it will be easier to pull upstream changes.

avtarsehra commented 7 years ago

Yes that is a good point. We were mainly using as a dev test as it utilises some key skills we wanted to identify.

splix commented 7 years ago

@avtarsehra maybe we should merge it with status Rejected? for history and reference