Guard against non-expected parties during init observation. #295
Conversation
…eters." & Revert "Add pub key hashes to list of parties in on-chain head parameters.". After discussing the next steps, we realized that passing the pub key hashes on-chain and checking the PTs does not actually provide any extra security guarantees and only makes the on-chain code more complicated. In the end, this is something we can only truly handle off-chain, durign the observation of an init transaction. It is the observer who knows the configuration it is expecting, and that can decide whether some observation is valid or not. On-chain, there isn't much we can do since, anyone crafting the init transaction may also change the redeemer, parameters or anything really. The participants of a head are BY DEFINITION the keys identified by the PT. Now, those participants may or may not reflect a known configuration of a node, but this is decided off-chain exclusively.
Use it for catching errors on an illed-formed init tx.
… reason. Whoopsie...
Indeed... mutating outputs isn't caught by our guard because we only check minted values. Which is this however sufficient? (a) The ledger rules ensure that any minted value is actually properly distributed in outputs (transaction ins and outs must balance each other) (b) Our on-chain validator does ensure that the right number of assets are minted, in the right quantity, and that assets are distributed across the right number of outputs.
| @@ -158,6 +159,7 @@ withDirectChain tracer networkId iocp socketPath keyPair party cardanoKeys callb | |||
| SomeOnChainHeadState $ | |||
| idleOnChainHeadState | |||
| networkId | |||
| (cardanoKeys \\ [verificationKey wallet]) | |||
There was a problem hiding this comment.
suggestion: Perhaps worthwhile to detail the comment on line 151 so that this line makes more within the context?
There was a problem hiding this comment.
Or just pass only peers' keys?
| (event, observation) <- observeInitTx networkId ownParty tx | ||
| observeTx tx OnChainHeadState{networkId, peerVerificationKeys, ownParty, ownVerificationKey} = do | ||
| let allVerificationKeys = ownVerificationKey : peerVerificationKeys | ||
| (event, observation) <- observeInitTx networkId allVerificationKeys ownParty tx |
There was a problem hiding this comment.
Seems like we are not very consistent in what we store or pass as arguments to functions: Sometimes we pass all keys including ours, sometimes only our peers'...
There was a problem hiding this comment.
Yes, but this is very much context dependent... I started with all keys in the State, but, since the state also stored the ownVerificationKey, there was redundancy in the state which is room for hazards.
Within the observation function however, we care not about this distinction and all keys can be treated the same.. Not very consistent but,... I don't know, still preferable?
Unit Test Results 5 files ±0 71 suites ±0 7m 16s ⏱️ -18s Results for commit 7814721. ± Comparison against base commit 51546e9. This pull request removes 1 and adds 2 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
KtorZ
changed the title
📍 Add mutation on initial output address Pass list of PKH to on-chain state and verify PTs minting using that #243
📍 Add pub key hashes to list of parties in on-chain head parameters.
📍 Verify PTs match their respective pubkey hashes in head parameters.
📍 Revert "Verify PTs match their respective pubkey hashes in head parameters."
& Revert "Add pub key hashes to list of parties in on-chain head parameters.".
After discussing the next steps, we realized that passing the pub key
hashes on-chain and checking the PTs does not actually provide any
extra security guarantees and only makes the on-chain code more
complicated.
In the end, this is something we can only truly handle off-chain,
durign the observation of an init transaction. It is the observer who
knows the configuration it is expecting, and that can decide whether
some observation is valid or not.
On-chain, there isn't much we can do since, anyone crafting the init
transaction may also change the redeemer, parameters or anything
really. The participants of a head are BY DEFINITION the keys
identified by the PT. Now, those participants may or may not reflect a
known configuration of a node, but this is decided off-chain
exclusively.
📍 Extract observe init mutations from init mutations.
📍 Define new mutation properties for testing off-chain code observation.
Use it for catching errors on an ill-formed init tx.
📍 Fix output selection for init mutation: make tests fail for the right reason.
Whoopsie...
📍 Add Cardano credentials verification during init observation.
📍 Tweak observe-init mutation to mutate minted values instead of outputs.
Indeed... mutating outputs isn't caught by our guard because we only
check minted values. Which is this however sufficient?
(a) The ledger rules ensure that any minted value is actually properly
distributed in outputs (transaction ins and outs must balance each
other)
(b) Our on-chain validator does ensure that the right number of assets
are minted, in the right quantity, and that assets are distributed
across the right number of outputs.