Skip to content

Conversation

@ArkadiuszNitkaSWI
Copy link
Contributor

Description of the changes being introduced by the pull request:

Problem

AWS KMS Signer fails when signing messages larger than 4KB due to using MessageType="RAW", which has a hard 4096-byte limit in the AWS KMS Sign API.

Changes Made

Modified AWSSigner.sign() method in _aws_signer.py:

  • Added local hash computation using hashlib.new(hash_algorithm)
  • Changed MessageType from "RAW" to "DIGEST"
  • Send only the computed digest to AWS KMS instead of the full payload

Benefits

  • Removes 4KB payload size limitation
  • Aligns with GCP and Azure signer implementations
  • Follows AWS KMS best practices for larger messages
  • Maintains backward compatibility

Fixes #1025

@jku
Copy link
Collaborator

jku commented Sep 22, 2025

Looks good at first glance, thanks.

Have you ran this on actual AWS already? (I don't think that's a requirement for merging, just curious as our aws tests only run on localstack)

Copy link
Collaborator

@jku jku left a comment

Choose a reason for hiding this comment

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

Nice. I think this makes a lot of sense (would make sense even if AWS did not have the limit).

Looks good to merge to me -- I left one comment but it's a code comment nitpick, feel free to leave it as is if you don't feel like fixing it.

@ianhundere any comments as original author?

@ArkadiuszNitkaSWI
Copy link
Contributor Author

Thank you for the review. I updates the docstrings. Yes, I tested it with a real AWS KMS key.
I also tested it with an RSTUF worker instance, because I’m using AWS KMS as an online key (I patched this signer library). It worked.

@ArkadiuszNitkaSWI
Copy link
Contributor Author

Hey @jku, I can't merge this. If you think it's ready, could you please merge it for me? Also, could you let me know when it can be released?

@jku
Copy link
Collaborator

jku commented Sep 23, 2025

sure, was just leaving some time for @lukpueh or ian to chime in if they wanted to.

This seems uncontroversial though so I can merge, let's say, tomorrow.

We could do a release for this, it's been a few months since the last one.

@lukpueh
Copy link
Member

lukpueh commented Sep 23, 2025

Have you ran this on actual AWS already? (I don't think that's a requirement for merging, just curious as our aws tests only run on localstack)

We actually run AWS tests with localstack as part of CI: https://github.com/secure-systems-lab/securesystemslib/actions/runs/17919157997/job/51027942600?pr=1026

So this should be fine.

@ianhundere
Copy link
Contributor

lgtm 🙌🏼

@jku jku merged commit f683509 into secure-systems-lab:main Sep 23, 2025
17 checks passed
@jku
Copy link
Collaborator

jku commented Sep 23, 2025

Thanks!

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.

AWS KMS Signer fails on messages larger than 4KB due to MessageType="RAW"

4 participants