Ensure all signatures returned by beforeSign hooks are valid #25
Conversation
There was a problem hiding this comment.
To spark a discussion on these changes before I implement them - this change is intended to catch situations where multiple plugins implement beforeSign hooks that mutate and sign the transaction.
Given a Session with 2x plugins, both which mutate and sign:
- Plugin 1 mutates and prepends an action, then signs. The transaction has X+1 actions.
- Plugin 2 mutates and prepends an action, then signs. The transaction has X+2 actions now.
The signature from Plugin 1 is now invalid, since Plugin 2 mutated the transaction.
| @@ -62,6 +63,7 @@ export interface SessionOptions { | |||
| permissionLevel: PermissionLevelType | string | |||
| transactPlugins?: AbstractTransactPlugin[] | |||
| transactPluginsOptions?: TransactPluginsOptions | |||
| validatePluginSignatures?: boolean | |||
There was a problem hiding this comment.
Add an option to turn on and off this functionality.
| @@ -198,6 +204,7 @@ export class Session { | |||
| // Create response template to this transact call | |||
| const result: TransactResult = { | |||
| chain: this.chain, | |||
| keys: [], | |||
There was a problem hiding this comment.
Store an array of all the public keys used to sign the request to provide in the response and for use during signature validation after the beforeSign hooks complete.
| }) | ||
|
|
||
| // Merge newly discovered keys into the TransactResult. | ||
| result.keys = [...result.keys, ...recoveredKeys] |
There was a problem hiding this comment.
When signatures are returned by beforeSign hooks, recover the public key from the signature using the current digest of the transaction.
| } | ||
| } | ||
|
|
||
| // Validate all the signatures returned by the plugins against the current request | ||
| if (willValidatePluginSignatures) { | ||
| this.validateBeforeSignSignatures(context, request, result) |
There was a problem hiding this comment.
Once the transaction is finalized (after all beforeSign hooks), and before the wallet signs the transaction, validate all of the signatures provided against the transaction the user is about to sign.
This will throw an error if any of the signatures provided by the plugins are invalid (since the transaction would fail upon broadcast anyways).
| ) | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
This method takes the current request and all public keys recovered from the signatures provided by the beforeSign hooks. It recovers the digest from the transaction, and then for each signature, ensures that the signature is valid for the digest and the key recovered from the current digest/signature combo matches what was originally used.
This can be disabled with the `validatePluginSignatures` option, it is enabled by default to ensure that multiple mutations in the `beforeSign` hooks don't invalidate any signatures they may create.
aaroncox
force-pushed
the
mutation-signature-check
branch
from
e3bd626
to
48b0fdb
Compare
This can be disabled with the
validatePluginSignaturesoption, it is enabled by default to ensure that multiple mutations in thebeforeSignhooks don't invalidate any signatures they may create.