Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Attack] Cross-chain signature replay #3

Open
bddap opened this issue Mar 6, 2020 · 9 comments
Open

[Attack] Cross-chain signature replay #3

bddap opened this issue Mar 6, 2020 · 9 comments

Comments

@bddap
Copy link
Contributor

bddap commented Mar 6, 2020

Many planed signable types, like Revoke and DIDRemoval include a field:
last_updated_in_block: BlockNumber,

The field is intended to mitigate replay attacks. AFAIK the mitigation works, but only in the context of a single chain.

In a multi chain environment, e.g. testnet+mainnet, the user may (unwisely) use the same public DID key on both chains. In that case, a signature posted to testnet may be replayed on mainnet and counted valid.

@bddap
Copy link
Contributor Author

bddap commented Mar 6, 2020

On simple mitigation, if necessary, would be to use BlockHash in place of BlockNumber.

@lovesh
Copy link
Member

lovesh commented Mar 6, 2020

Good point. But how would BlockHash be available during block execution. It should only be available after extrinsics of the block have been processed (and state changes applied). Or would blockhash be just the hash of an included exstrinsics?
Btw, the attack is less likely as you should have created/updated the DID (or other objects) in the same block number in both chains (its an == check and not >)

@bddap
Copy link
Contributor Author

bddap commented Mar 6, 2020

My understanding is that last_updated_in_block refers to the last block during which an update occurred, not the current block.
Similarly, BlockHash would refer to the last block during which an update occurred, not the current block.

@bddap
Copy link
Contributor Author

bddap commented Mar 6, 2020

Oh, scratch that. I see the issue. During block execution, we only have the hash for the previous block, not the current one.

@bddap
Copy link
Contributor Author

bddap commented Mar 11, 2020

Ok, new idea. This one is stolen from substrate.

The CheckGenesis SignedExtension prevents cross-chain signature replay (but not for hard-forks). We maybe implement something similar.

@lovesh
Copy link
Member

lovesh commented Mar 12, 2020

Can you elaborate how?
I think you are saying to include the genesis hash in the payload by DID. I can see how that we can implement Encode for the StateChange enum and the while encoding (serializing) before signing, you (the node as well as the SDK) include the genesis hash. I see how the SDK can using api.genesisHash but don't see how to do it for the node

@bddap
Copy link
Contributor Author

bddap commented Mar 12, 2020

I'm assuming the runtime somehow has read access to the genesis hash. The CheckGenesis extension must be getting it from somewhere.

@lovesh
Copy link
Member

lovesh commented Mar 12, 2020

I see it would somehow know but how exactly, i don't know. Got some pointers in Riot chat,
https://github.com/paritytech/substrate/blob/877d79bcebe7728d502c33227c2e4dea0766e22d/bin/node/runtime/src/lib.rs#L553 and https://github.com/paritytech/substrate/blob/877d79bcebe7728d502c33227c2e4dea0766e22d/bin/node/runtime/src/lib.rs#L674 is where CheckGenesis is used within the runtime
But i have to try them

@lovesh
Copy link
Member

lovesh commented Mar 13, 2020

You can get the genesis hash with <system::Module<T>>::block_hash(T::BlockNumber::from(0)). See here

let g = <system::Module<T>>::block_hash(T::BlockNumber::from(0));
. I am printing (storing since io_print didn't work) the blockhash here
println!("StateChange::hash {:?}", DIDModule::id(&did));
. Run with SKIP_WASM_BUILD=1 flag to print

olegnn added a commit that referenced this issue Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants