Skip to content

Add KMS signer support for AWS, GCP, Azure, and file-based backends#597

Open
SequeI wants to merge 2 commits into
sigstore:mainfrom
SequeI:FeatureKMS
Open

Add KMS signer support for AWS, GCP, Azure, and file-based backends#597
SequeI wants to merge 2 commits into
sigstore:mainfrom
SequeI:FeatureKMS

Conversation

@SequeI
Copy link
Copy Markdown
Contributor

@SequeI SequeI commented Jan 6, 2026

Summary

This commit adds support for signing models using Key Management Service (KMS) providers through KMS URIs. The implementation follows the same pattern as the existing PKCS11 signer, providing a consistent interface for raw signer instantiations. Only available provider for now is AWS.

  • AWS KMS: kms://aws/<key-id/key-arn>?region=<region>

Closes #203

Moved some testing resources and the check EC curve functions to not duplicate

❯ hatch run model_signing sign kms-key bert-base-uncased  \
  --kms_uri "kms://aws/arn:aws:kms:<>" \
  --signature model.sig
  
Found credentials in shared credentials file: ~/.aws/credentials
Signing succeeded

❯ hatch run model_signing verify key bert-base-uncased  \
  --public-key /aws-kms-pubkey.pem \
  --signature model.sig
  
Verification succeeded
Checklist
  • All commits are signed-off, using DCO
  • All new code has docstrings and type annotations
  • All new code is covered by tests. Aim for at least 90% coverage. CI is configured to highlight lines not covered by tests.
  • Public facing changes are paired with documentation changes
  • Release note has been added to CHANGELOG.md if needed

@SequeI SequeI requested review from a team as code owners January 6, 2026 11:52
@Hayden-IO
Copy link
Copy Markdown
Collaborator

Quick drive-by comment - Are there concrete use cases for the chosen KMS providers, or is this to mirror what's available in the Go libraries? We've had some challenges supporting Azure and AWS because the public infrastructure doesn't use these, and we have no test infrastructure to verify functionality. If no one is asking for these, I'd limit which KMS providers are supported.

@mihaimaruseac
Copy link
Copy Markdown
Member

mihaimaruseac commented Mar 30, 2026

+1, let's only add the support here when there is a significant need for this.

It's for the same reason why #148 is still open, but because we haven't yet seen an actual demand to hash models while they are on remote blob storage we did not spend cycles to implement it.

@mihaimaruseac mihaimaruseac added the discusion pending Label for PRs and Issues that we should discuss at a Model Signing SIG Meeting label Mar 30, 2026
@mihaimaruseac
Copy link
Copy Markdown
Member

Confirmed that there is a usecase for this

@SequeI
Copy link
Copy Markdown
Contributor Author

SequeI commented Apr 9, 2026

@mihaimaruseac Which provider is the use case for?

IMO, I can revise this PR to add support for that specific provider first, then when need arises we can increase KMS provider options.

@mihaimaruseac
Copy link
Copy Markdown
Member

Yesterday at the OMS meeting @ralphbean said that there is a need for this PR

@Hayden-IO
Copy link
Copy Markdown
Collaborator

But for all KMS providers? If you don't have a good way to test support across all of these ecosystems, I would advise against proactively adding all of them.

@mihaimaruseac
Copy link
Copy Markdown
Member

Oh, that's true. Waiting on @ralphbean to suggest which of the providers is important

@ralphbean
Copy link
Copy Markdown

I only have a concrete use case for the AWS one.

Add support for signing models using AWS KMS through KMS URIs.
The URI format is: kms://aws/<key-id-or-arn>?region=<region>

Install with `pip install model-signing[kms]` to enable this functionality.

Signed-off-by: SequeI <asiek@redhat.com>
Signed-off-by: SequeI <asiek@redhat.com>
@SequeI
Copy link
Copy Markdown
Contributor Author

SequeI commented Apr 23, 2026

Reworked PR for AWS provider only, pipelines failing due to get dangerous oidc token action, not related to PR. We should fix/rework this sometime if possible, I have notice a lot of failures from this action

@ralphbean Let me know if this is what you had in mind, I tested via full ARN and worked fine.

if len(arn_parts) != 6 or arn_parts[5].split("/")[0] != "key":
raise ValueError(
"AWS KMS ARN must have format: "
"arn:aws:kms:<region>:<account-id>:key/<key-id>"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The ARN validation here rejects valid AWS KMS ARNs:

  1. Keys in aws-cn (China) and aws-us-gov (GovCloud) partitions — e.g. arn:aws-cn:kms:... — are rejected because only arn:aws:kms: prefix is checked.
  2. Alias-based ARNs (arn:aws:kms:region:account:alias/my-alias) are rejected because only key/ resource type is accepted.
  3. The elif "/" in key_id branch also rejects alias ARNs that don't match the arn:aws:kms: prefix.

Consider relaxing the validation here — AWS KMS will validate the key ID/ARN authoritatively when the API call is made, so client-side validation that rejects valid inputs does more harm than good.


def get_public_key(self) -> ec.EllipticCurvePublicKey:
return self._public_key

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The ternary chain here silently maps any unrecognized curve to ECDSA_SHA_512. Consider using a dict lookup instead — it's more readable, explicitly lists the supported mappings, and raises KeyError naturally for unrecognized curves:

_CURVE_TO_ALGORITHM = {
    "secp256r1": "ECDSA_SHA_256",
    "secp384r1": "ECDSA_SHA_384",
    "secp521r1": "ECDSA_SHA_512",
}
# then: SigningAlgorithm=_CURVE_TO_ALGORITHM[self._public_key.curve.name]

_parse_kms_uri("kms://aws/key-id/extra")

with pytest.raises(ValueError, match="AWS KMS ARN must have format"):
_parse_kms_uri("kms://aws/arn:aws:kms:invalid")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The tests here only cover URI parsing. There are no tests for the actual signing path — no mocked boto3 tests for Signer instantiation, sign(), or round-trip sign-and-verify.

Every other signer in this project (key, certificate, sigstore, pkcs11) has sign-and-verify round-trip tests in tests/api_test.py. Consider adding at minimum a unit test with mocked boto3 that verifies the Signer can produce a valid Signature object that can be verified with the existing EC key Verifier.

KeyId=self._key_id,
Message=digest,
MessageType="DIGEST",
SigningAlgorithm=(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

After retrieving the public key, consider also checking response["KeyUsage"] to verify the key is configured for signing (SIGN_VERIFY). AWS KMS keys can have ENCRYPT_DECRYPT usage — if one of those is passed here, the sign() call will fail later with an opaque AWS API error. An early check like:

if response.get("KeyUsage") \!= "SIGN_VERIFY":
    raise ValueError(
        f"KMS key must have KeyUsage=SIGN_VERIFY, got {response.get('KeyUsage')}"
    )

would give a clear, actionable error message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

discusion pending Label for PRs and Issues that we should discuss at a Model Signing SIG Meeting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Custom raw signers

4 participants