Skip to content

feat(signing): sign with AWS KMS-held party keys - #303

Open
scolear wants to merge 5 commits into
mainfrom
feature/aws-kms-signer
Open

feat(signing): sign with AWS KMS-held party keys#303
scolear wants to merge 5 commits into
mainfrom
feature/aws-kms-signer

Conversation

@scolear

@scolear scolear commented Aug 4, 2026

Copy link
Copy Markdown
Member

What

Adds AwsKmsSigner, the first non-export signing backend behind the TransactionSigner trait from #291. When the party's Daml key carries a kms_key_id (KMS-backed participants), select_signer routes to it and decman signs the interactive-submission prepared hash through the AWS KMS Sign API. Exportable vault keys keep the export path unchanged.

Why

On a crypto.provider = kms participant the party's Daml key is created inside AWS KMS and cannot be exported. Canton has no API that signs a ledger transaction with a vault key — party signatures must come from the caller. So the party could be created (#268) but could not deploy its governance-core contract. This closes that gap: decman asks the KMS to sign, and the key never leaves it.

Design decision (pilot call): the party key stays in the node's KMS, and the operator grants decman kms:Sign — security posture is worth the one-time IAM setup. A decman-held local-key backend remains possible later behind the same trait.

Signature semantics — verified against Canton source, not assumed

  • Prepare returns the raw 32-byte digest (hash.unwrap, no multihash prefix); Execute recomputes and verifies those exact bytes.
  • Canton verifies ECDSA with SHA256withECDSA (message semantics — the verifier hashes the input again). The KMS call is therefore MessageType=RAW + ECDSA_SHA_256. Passing DIGEST would silently skip one hash pass and fail.
  • Canton accepts ECDSA signatures only in DER (supportedSignatureFormats = {Der} for EcDsaSha256); AWS KMS returns DER, passed through unchanged.
  • signed_by = the fingerprint of the party's registered signing key (Protocol usage), which fix: support KMS-backed nodes in party key generation (spec + fingerprint) #268 made correct for P-256.
  • No low-s/canonicality enforcement exists in Canton's ECDSA verify, so KMS output verifies as-is.

Each signature is also verified locally (via p256) against the registered public key before submission — wrong key or wrong semantics fails fast with context instead of an opaque ExecuteSubmission rejection.

Operator setup

docs/KMS_SIGNING.md (new) documents the one-time setup: AWS credentials for decman (IRSA on EKS) and kms:Sign on the party keys, plus the failure-mode table. Key discovery, algorithm selection, and format are automatic.

New dependencies

  • aws-config / aws-sdk-kms — the official AWS SDK; needed to call KMS Sign. rustls, no default features.
  • p256 (RustCrypto) — parses the SPKI public key and verifies each signature locally before submission.

CI note

.cargo/audit.toml gains an ignore for RUSTSEC-2026-0235 (rkyv): the crate enters the lockfile only through rust_decimal's disabled optional feature and is never compiled; graph-aware cargo-deny agrees. This keeps the lockfile-scanning Cargo Audit gate green and is unrelated to KMS.

How tested

  • Unit tests for the key-spec → algorithm mapping (EC-P256 supported; every other spec rejected with a clear error). 304 lib tests pass; clippy and fmt clean.
  • The review round removed the speculative P-384 arm: no P-384 party key can exist today (KMS nodes generate the P-256 default), and the arm skipped the local verification.
  • The KMS Sign call itself cannot be unit-tested without AWS; live validation is the next step: grant decman-6's role kms:Sign on the devnet party keys and deploy governance-core with the test-kms-2 party. The local verification step means a semantics error surfaces in decman logs, not as a ledger rejection.

Scope notes

  • Only AWS KMS. A key held by a different KMS driver (MPCH) also carries a kms_key_id; its Sign call fails with a clear KMS error until a dedicated backend exists (their client API is still an open question with MPCH).
  • Existing JCE parties are untouched — no kms_key_id, same export path as before.

Refs #264

A KMS-backed participant creates the party's Daml key inside AWS KMS,
where it cannot be exported, so the export-and-sign path fails and the
party cannot deploy its governance contract. Canton offers no API that
signs a ledger transaction with a vault key: party signatures must come
from the caller.

Add AwsKmsSigner, a TransactionSigner backend that calls the AWS KMS
Sign API with the key id the vault reports for the key. select_signer
now routes keys that carry a kms_key_id to it; exportable vault keys
keep the export path unchanged.

The signature semantics follow Canton's verify path exactly, confirmed
against source: the prepared-transaction hash is a raw 32-byte digest
signed as a message (KMS MessageType=RAW, ECDSA_SHA_256 — Canton
verifies with SHA256withECDSA, which hashes the input again), and the
signature stays in the DER encoding KMS returns (the only format Canton
accepts for ECDSA). Each signature is verified locally against the
registered public key before submission, so a wrong key or wrong
semantics fails here with context instead of at ExecuteSubmission.

The operator grants decman's role kms:Sign on the party keys; the new
docs/KMS_SIGNING.md covers the setup and failure modes.

Refs #264
@socket-security

socket-security Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedcargo/​aws-config@​1.10.19910093100100
Addedcargo/​aws-sdk-kms@​1.114.010010093100100
Addedcargo/​p256@​0.13.210010093100100

View full report

scolear added 2 commits August 4, 2026 15:48
The "rustls" feature on aws-config/aws-sdk-kms selects the legacy
connector built on rustls 0.21, which pins rustls-webpki 0.101 and
its three open advisories (RUSTSEC-2026-0098/0099/0104). Switch to
"default-https-client", the current connector on rustls 0.23; the
vulnerable crates leave the lockfile entirely.

cargo-audit also flags rkyv 0.7 (RUSTSEC-2026-0235), which enters the
lockfile only through rust_decimal's disabled optional feature and is
never compiled. Ignore it in .cargo/audit.toml with the reasoning,
following the existing convention there.
Apply the persona-review round:

- Drop the P-384 arm. No P-384 party key can exist (KMS nodes generate
  the P-256 default), the arm skipped the local pre-submission
  verification, and its Canton format rule was never verified. The
  backend now rejects anything but EC-P256 with a clear error, and
  sign() reads straight through with no conditional path.
- Split the key-policy and IAM-policy snippets in docs/KMS_SIGNING.md.
  One snippet served both, and following the IAM route with
  Resource:"*" would grant signing with every key in the account,
  including the namespace key. The IAM variant now names explicit key
  ARNs and warns against the wildcard; a concrete ListMyKeys command
  covers key discovery.
- Update the signing module doc that still called vault export the
  only backend.
- Restore alphabetical order in the workspace Cargo.toml and drop the
  redundant SigningError::Other wrappers in favor of the #[from]
  conversion.
@scolear

scolear commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

[personas] Review summary — Gate B, round 1 (fixes in bc054c3)

Three persona lenses reviewed this PR: security, PM-against-spec, pragmatic. Ten findings. Six fixed in bc054c3, two filed as issues, one resolved by removal, one needed no change. Nothing is parked for a tiebreaker. One adjudication: the pragmatic lens wanted the P-384 arm deleted (blocking); the PM lens wanted it kept (nit). The panel deleted it — no P-384 key can exist today, the arm skipped the local verification, and its Canton format rule was never verified. A clean rejection is safer than an unverified path.

# Persona Finding Category Severity Disposition
1 security One policy snippet served both key-policy and IAM-policy routes; the IAM reading grants kms:Sign on every key in the account security blocking fixed — split snippets, explicit ARNs, wildcard warning (docs/KMS_SIGNING.md)
2 security P-384 skips local verification, doc overclaims security nit fixed by removal — the P-384 arm is gone
3 security kms_key_id from vault metadata is trusted verbatim (cross-account ARN shape) security nit filed#304
4 security Key ids and fingerprints in logs security nit no change — identifiers, not secrets
5 pm IAM-policy route may dead-end if the key policy does not delegate to IAM functionality nit fixed — doc names the key-policy edit as the reliable route
6 pm Key discovery names no concrete tool functionality nit fixed — grpcurl ListMyKeys command added
7 pm PR body overclaims P-384 support cosmetic nit fixed-w-mods — arm removed, body corrected
8 pm audit.toml ignore is silent scope in the PR body cosmetic nit fixed — CI note added to the body
9 pragmatic mod.rs doc still called vault export the only backend cosmetic blocking fixed
10 pragmatic P-384 arm speculative; Cargo.toml ordering; redundant error wrappers functionality blocking + nits fixed — arm deleted, ordering restored, wrappers dropped

By category: security 4 (1 blocking, fixed; 1 removed; 1 filed; 1 no-change) · functionality 3 (all fixed) · cosmetic 3 (all fixed).

Pre-existing behavior outside this PR, filed: #305 (vault_export submits a signature that failed local verification).

The PM lens confirms requirement coverage end-to-end: the execute step copies signatures verbatim (no Ed25519 assumption), JCE parties are untouched, and the docs match the code. The live devnet run (IAM grant + governance-core deployment) remains the close condition for #264 — green CI does not stand in for it.

Next: Copilot review loop (Gate C) runs on this improved code; a final targeted persona re-check follows if Copilot changes anything. The merge decision stays with the human gate.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds an AWS KMS–backed signing backend to decman so parties whose Daml keys are non-exportable (KMS-backed participants) can still sign interactive-submission prepared hashes without extracting private material from the participant vault.

Changes:

  • Introduces AwsKmsSigner implementing TransactionSigner, calling AWS KMS Sign and locally verifying signatures with p256 before submission.
  • Updates signer selection (select_signer) to route to KMS signing when the party key metadata includes kms_key_id.
  • Adds AWS SDK + p256 dependencies, plus operator documentation and a Cargo Audit ignore entry for a lockfile-only advisory.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
docs/KMS_SIGNING.md New operator guide for IAM + runtime setup and troubleshooting for KMS-backed party signing.
crates/decman/src/signing/signer.rs Routes signing backend selection to AWS KMS when kms_key_id is present.
crates/decman/src/signing/mod.rs Registers the new aws_kms signing module and updates module-level docs.
crates/decman/src/signing/error.rs Adds a typed SigningError::Kms variant for KMS signing failures.
crates/decman/src/signing/aws_kms.rs New AWS KMS signing backend implementation + key-spec→algorithm mapping tests.
crates/decman/Cargo.toml Adds aws-config, aws-sdk-kms, and p256 to the decman crate dependencies.
Cargo.toml Adds workspace dependency definitions for AWS SDK crates and p256 with selected features.
Cargo.lock Locks new transitive dependencies from the AWS SDK and RustCrypto additions.
.cargo/audit.toml Ignores a specific RustSec advisory deemed unreachable due to disabled features.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread docs/KMS_SIGNING.md Outdated
The guide referred to the field as kms_key_id in one place and
kmsKeyId in another. State once that grpcurl's JSON output renders
the proto field kms_key_id as kmsKeyId, so operators can match the
two.
@scolear

scolear commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

[copilot-loop] Review summary — Gate C, round 1 (converged)

Copilot reviewed the post-persona code (bc054c3) and generated one comment.

# Finding Category Disposition
1 docs/KMS_SIGNING.md uses both kms_key_id and kmsKeyId for the same field cosmetic fixed in 1fb6ff0 — the doc now states grpcurl renders the proto field as kmsKeyId

By category: cosmetic 1 (fixed). No security or functionality findings.

The round converged in one pass. Copilot changed no code itself, and the fix is one documentation sentence, so the terminal persona re-check has no code delta to review. Gate B and Gate C are both complete; the merge decision stays with the human gate. The live devnet validation (IAM grant + governance-core deployment) remains the close condition for #264.

@scolear
scolear marked this pull request as ready for review August 6, 2026 12:11
@scolear
scolear requested review from a team and sosaucily August 6, 2026 12:11
@schronck

schronck commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Went through the whole diff plus the call site in sign.rs and the generated crypto protos. Crypto looks right and the operator doc is better than most of ours. One thing I'd fix before merge, the rest can be follow-ups.

Checked a couple of the claims rather than trusting them. SignatureFormat::Der = 2 is documented as the ECDSA format in the generated v30 protos, so that's correct. And the AccessDenied case really does reach the logs, which I was suspicious of because SdkError's own Display is just "service error", but #[source] keeps the chain and both call sites log with {e:#} (workflow/mod.rs:557 and :209). So the failure table in the doc holds.

aws-config feature set drops SSO

Cargo.toml:15 turns off default features and doesn't put sso or credentials-process back. Upstream default is ["default-https-client", "rt-tokio", "credentials-process", "sso"]. Lockfile confirms it: no aws-sdk-sso, no aws-sdk-ssooidc.

IRSA is unaffected, web identity is core and aws-sdk-sts is still there, so the devnet run will work. But anyone running decman locally with AWS_PROFILE after aws sso login gets a credentials error, and that's how we auth to everything else. It also contradicts step 1 of the doc ("decman uses the default AWS credential chain"), and the escape hatch further down that tells the operator to run the workflow once and read the key ARN out of the AccessDeniedException assumes a local run works.

One line to add sso back, probably credentials-process too. Or keep the smaller dep surface and say in the doc that only env vars, static keys, IRSA and IMDS resolve. I'd add them back.

Region

aws_kms.rs:50 takes whatever the chain gives it. If nothing resolves a region the SDK fails with a ConstructionFailure, not the "dispatch failure" the doc's table maps it to. And if the region doesn't match the keys you get NotFoundException with no hint why.

Cheap version: bail in new() when config.region() is None and name AWS_REGION. Better version: when kms_key_id is a full ARN, pull the region out of it and override, which kills the mismatch class entirely. That one needs the client built in sign() where the key is in hand.

Tests cover the easy half

The two tests are on algorithms_for, which is a five line match. The part actually doing work, SPKI parse then DER verify against the hash (aws_kms.rs:90 and :132), has nothing on it, and it's testable offline with a generated P-256 key. Pull it out as verify_locally(spki_der, hash, sig_der) and that's three real tests: good sig passes, wrong key fails, garbage DER fails. Since the KMS call itself can't be unit tested, it's the only place tests buy anything before the devnet run.

Smaller stuff

The local verify pins the KMS call to the semantics we assume, but it can't check that assumption against Canton. Both sides hash once, so if the Canton reading is wrong the signature still passes locally and gets rejected at ExecuteSubmission. Worth keeping in mind during the live run: a green local verify isn't evidence the semantics are right. The comment at aws_kms.rs:85 reads a bit stronger than that.

signer.rs:87 and the doc's Scope section name MPCH as the other driver that reports a kms_key_id. Canton's GCP KMS driver does too and would route here. Worth a sentence.

CI note in the PR body is stale. The rkyv ignore went into main on its own in #308 and this branch just merged it in, so it isn't in the diff anymore.

Sign calls are sequential, one round trip per hash. Fine at current counts, just noting it.

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.

4 participants