Added AWS KMS signing#339
Conversation
brianjohnson5972
force-pushed
the
feature/aws-kms-signing
branch
from
efb24ba to
ec7cc07
Compare
| - Building and submitting outbound envelopes to external chains | ||
| - Responding to challenges if disputed | ||
|
|
||
| Signing keys for outbound submissions can be held locally (`KEY:`), in a `kiod` daemon (`KIOD:`), or in AWS KMS (`KMS:` — secp256k1 asymmetric keys). See `plugins/signature_provider_manager_plugin/KMS_SIGNING_DESIGN.md` for the KMS spec format and operational notes. |
There was a problem hiding this comment.
I don't see the KMS_SIGNING_DESIGN.md file in this PR.
There was a problem hiding this comment.
yeah, I'll pull that out and replace it with the README under the test directory.
| chain::plugin_config_exception, | ||
| "KMS ARN tail \"{}\" has empty key/alias name", tail); | ||
|
|
||
| return kms_key_ref{region, tail}; |
There was a problem hiding this comment.
This strips full KMS ARNs down to key/ or alias/ before calling KMS. AWS documents valid KeyId values as key ID, key ARN, alias name, or alias ARN, not key/. This means key ARNs will fail at first GetPublicKey/Sign, and alias ARNs lose their account identity and may resolve to a same-named alias in the caller’s account. Keep the full ARN as key_id for ARN specs, or strip only key ARNs to the bare UUID while preserving alias ARN/account semantics. Source: AWS KMS Sign docs, KeyId forms: https://docs.aws.amazon.com/cli/latest/reference/kms/sign.html
| // discovering it deep into production on the first sign. | ||
| { | ||
| std::scoped_lock lock(_signing_providers_mutex); | ||
| _kms_startup_probes.push_back(std::move(kms.warm_up)); |
There was a problem hiding this comment.
This records the KMS warm-up probe before the provider is inserted at :400. If set_provider() then rejects a duplicate name/public key, the failed provider’s probe remains in _kms_startup_probes and plugin_startup() can later probe or fail on a KMS key that was never registered. Defer adding the probe until after successful insertion, or roll it back on failure.
|
|
||
| // Carry the keccak digest into the sha256-typed closure argument unchanged. | ||
| const auto digest_bytes = digest.to_char_span(); | ||
| const signature sig = p.sign(sha256(digest_bytes.data(), digest_bytes.size())); |
There was a problem hiding this comment.
This should not be using sha256 as a bucket of bits. Just use a bucket of bits or add a sign_keccak256.
| sig.get<em::signature_shim>().recover_eth(digest))); | ||
| FC_ASSERT(recovered == p.public_key, | ||
| "remote signer produced a signature that does not recover to the " | ||
| "provider's public key — the signer's key or digest framing is wrong"); |
There was a problem hiding this comment.
No need to recover the key on every sign. Just have a test that verifies we can recover they key from a AWS signature.
| digest.to_uint8_span().data(), | ||
| digest.to_uint8_span().size()}); | ||
| req.SetMessageType(Aws::KMS::Model::MessageType::DIGEST); | ||
| req.SetSigningAlgorithm(Aws::KMS::Model::SigningAlgorithmSpec::ECDSA_SHA_256); |
There was a problem hiding this comment.
The AWS interface is rather strange here. They should have a ECDSA_KECCAK_256 but apparently they don't. But we shouldn't duplicate that strangeness. Make the comment above a bit more clear that we are only using the provided AWS interface and the apparently normal workaround by using ECDSA_SHA_256 as a way to say here is a 256 bit hash of data, don't rehash it.
| wallet_plugin_headers) | ||
| http_client_plugin | ||
| wallet_plugin_headers | ||
| aws-cpp-sdk-kms) |
There was a problem hiding this comment.
I would prefer this not to be linked into every nodeop instance. Can we factor the kms_signature_provider into its own plugin and only link it into the non-nodeop executables.
3 participants
What the feature does
Adds a third signature-provider backend, KMS:, to signature_provider_manager_plugin, alongside the existing KEY: (raw private key) and KIOD: (key daemon) forms. With KMS:, the signing key lives in AWS
KMS and never appears on the host or in process memory — nodeop/cranker calls KMSClient::Sign over the wire for each signature.
The first concrete consumer is the Ethereum cranker, replacing a hot KEY:0x… literal in the operator's config with an IAM-gated KMS key (CloudTrail-audited, rotatable without redeploy).
Spec grammar
--signature-provider eth-01,ethereum,ethereum,0x045a87…,KMS:
is either a full ARN (arn:aws:kms:::(key|alias)/) or the shorthand :. Region is mandatory — no silent fallback to AWS_REGION, so
misconfiguration fails loud.
Implementation (6 commits, ~1,480 lines)
New plugin-private TU — src/kms_signature_provider.{hpp,cpp}:
against the pinned public key; high-S is normalised to low-S (EIP-2).
Plugin hook — signature_provider_manager_plugin.cpp gets a KMS branch in create_provider_from_spec (returns std::nullopt for private_key, same shape as KIOD) and extended help text.
Build wiring — aws-sdk-cpp[kms] (v1.11.665, default-features:false) added to vcpkg.json; plugin CMakeLists.txt links aws-cpp-sdk-kms. The boringssl-custom collision with aws-c-cal/s2n (both declare
openssl) was resolved via overlay ports in wire-vcpkg-registry; baseline bumped to c739f08.
Scope boundaries
Tests
test_kms_signature_provider.cpp (461 lines) — fully offline suite: parser cases, DER round-trips, low-S flipping, v-recovery, per-region client caching, plus an env-gated
(KMS_LIVE_SPEC/KMS_LIVE_PUBKEY) live round-trip. test_create_provider_specs.cpp adds two end-to-end cases proving the plugin routes KMS: specs through the parser and rejects Solana KMS specs at parse
time.