Skip to content

accounts: honor configured hash algorithm on Google KMS signer - #233

Draft
vishalchangrani wants to merge 1 commit into
mainfrom
vishal/kms-honor-configured-hash-algo
Draft

vishalchangrani wants to merge 1 commit into
mainfrom
vishal/kms-honor-configured-hash-algo

Conversation

@vishalchangrani

Copy link
Copy Markdown
Contributor

Summary

  • KMSKey.Signer() now passes the account's configured HashAlgo() to the Google KMS signer instead of silently letting the KMS key's native algorithm (SHA2_256) win.
  • Lets flow-cli sign for Flow accounts whose public key is on-chain-registered with SHA3_256 — previously the signer produced a signature over a SHA2_256 digest and envelope verification failed with signature is not valid.
  • Depends on onflow/flow-go-sdk#1048, which introduces SignerForKeyWithHashAlgo and pre-hashes client-side so the KMS AsymmetricSign Digest field carries the SHA3_256 hash as-is.

Tracks the bug reported in flow-cli: onflow/flow-cli#2367

Test plan

  • go test ./accounts/... passes locally against the SDK branch.
  • Merge is blocked on flow-go-sdk#1048 landing and a flow-go-sdk release bump — this PR will not compile against the currently pinned flow-go-sdk v1.10.9.
  • Manual end-to-end verification against a mainnet Flow account registered with ECDSA_P256 / SHA3_256 backed by GCP KMS (reproduces the failure in the linked flow-cli issue).

Google KMS keys expose a native hash algorithm (SHA2_256 for the
EC_SIGN_P256_SHA256 and EC_SIGN_SECP256K1_SHA256 key types), but Flow
accounts can be registered with either SHA2_256 or SHA3_256 as the on-chain
hash algorithm. Passing the account's HashAlgo() straight through to the
KMS signer lets flow-cli sign for accounts whose public key is registered
with SHA3_256; the SDK Signer pre-hashes the message client-side and
submits the digest, so KMS signs it as-is without re-hashing.

Requires onflow/flow-go-sdk#1048.

Issue: onflow/flow-cli#2367

@jribbink jribbink left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think this changes behavior for existing google-kms keys that don't specify a hashAlgorithm. The signer has effectively assumed SHA2_256 until now, but with this change the default becomes SHA3_256, as it is for other key types when unspecified, so those would start failing. Would it make sense to keep SHA2_256 as the default for google-kms when it's unspecified, so existing setups keep working?

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

Successfully merging this pull request may close these issues.

2 participants