Skip to content
This repository has been archived by the owner on Mar 5, 2024. It is now read-only.

TargetRandState Concern #1310

Closed
anthonyra opened this issue Apr 22, 2022 · 2 comments
Closed

TargetRandState Concern #1310

anthonyra opened this issue Apr 22, 2022 · 2 comments

Comments

@anthonyra
Copy link

anthonyra commented Apr 22, 2022

{Path, StartP} = get_path(POCVersion, Challenger, BlockTime, Entropy, Keys, Vars, OldLedger, Ledger, StartFT),

To check poc validity, we use the PoCOnionKeyHash which is first created with a validators heartbeat. This is submitted and selected by the CG. The originating validator then sees this OnionKeyHash (public key hash) being selected for PoC from the CG. It then generates the PoC. Later when checking if the PoC was valid, the PoCOnionKeyHash is used to validate ZoneRandState but the Keys supplied are never checked to match those for the submitted PoCOnionKeyHash. Which the private key is used to form the TargetRandState.

Without checking the PoCOnionKeyHash to the a re-hashed public key from the Keys, you could submit different Keys thus resulting in a deterministic TargetRandState for the PoC (potentially).

I feel that there should be a check between the PoCOnionKeyHash in the txn and the Keys submitted via the secret.

Secret = ?MODULE:secret(Txn),
Keys = libp2p_crypto:keys_from_bin(Secret),
#{public := OnionCompactKey, secret := {ecc_compact, POCPrivKey}} = Keys,
OnionHash = crypto:hash(sha256, libp2p_crypto:pubkey_to_bin(OnionCompactKey)),
case POCOnionKeyHash == OnionHash of
     true ->
             case blockchain_ledger_poc_v3:verify(PoC, Challenger, BlockHash) of
                    ...
                    {Path, StartP} = get_path(POCVersion, Challenger, BlockTime, Entropy, POCPrivKey, Vars, OldLedger, Ledger, StartFT),
             end;
     false ->
             {error, invalid_poc}
end
@andymck
Copy link
Contributor

andymck commented Apr 25, 2022

@anthonyra thanks for this. Validating the presented PocOnionKeyHash against the public key hash is good for correctness. Your suggestion also highlighted a wider weakness in that the txn is not verifying the key pair presented are actually a valid pair. I will address both in a PR shortly.

@andymck
Copy link
Contributor

andymck commented Apr 25, 2022

@anthonyra changes related to this merged here: #1312

@andymck andymck closed this as completed Apr 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants