Skip to content

WIP: AWS KMS code signing#274

Draft
lf- wants to merge 2 commits intoindygreg:mainfrom
MercuryTechnologies:jade/push-rxryprnrpxsw
Draft

WIP: AWS KMS code signing#274
lf- wants to merge 2 commits intoindygreg:mainfrom
MercuryTechnologies:jade/push-rxryprnrpxsw

Conversation

@lf-
Copy link

@lf- lf- commented Jan 28, 2026

I found a bug in the docs in cryptography-rs, which I think actually is a bug in basically every extant provider: indygreg/cryptography-rs#68 We also don't use the sign() function afaik, so I suspect it was never noticed.

This appears to produce correct looking results in Apparency. I've tested it with a self signed cert from rcodesign which I imported the private key of into KMS.

The motivation for this feature rather than PKCS11 is that 1. PKCS11 is a pain in the ass (especially with AWS KMS where there is an unofficial PKCS11 but it's not packaged in nixpkgs and it's just . a lot of complexity) and 2. AWS CloudHSM is really really expensive ($1/hour!!), so KMS makes more sense to use in practice ($1/month/key to Not Have Stealable Keys is awesome actually).

TODOs:

  • Figure out testing. Integration testing this is probably going to suck, but it is possible. Try: https://github.com/nsmithuk/local-kms

    I don't think there is that much code to unit test here and we would probably have to introduce a mocking layer over the KMS API (which seems to likely defeat the purpose).

  • Cleanup pass on the code.

    • DRY up some common functionality between PKCS11 and AWS KMS. I assume this is going to be reused by most implementations of cloud signing.
  • Remove default-on aws-kms feature.

  • Docs

@lf-
Copy link
Author

lf- commented Jan 28, 2026

I would like advice on how to write an integration test that makes sense: If I do depend on local-kms for integration testing, how do we obtain that test-only Go dependency? What do we do about when the executable isn't there? Presumably somehow fail the test in CI but mark as skipped locally? idk. Edit: I've cooked up the beginnings of a test fixture (it is not especially good code. yet.) to be able to test against a local-kms server; I don't know how to integrate it with the main test suite yet.

Also, I found the API for certificates to be probably confusing, on further thought: you, more or less, if you want to support CSRs, need to be able to not have a certificate in your Key object. Yet the x509_certificate::signing::Sign trait, in its use in rcodesign, has signature_algorithm() frequently incorrectly implemented as asking the certificate inside for some details.

I think the general conclusion I have with this API is that the API is probably okay, but the signature_algorithm and key_algorithm being a certificate field is too tempting for people to put certificates in their key objects, which they should not, lest CSRs be broken. I'm going to revise this PR to not put a certificate in the Key as I have the key metadata and can make correct choices for all of these without it. done! that's an unambiguous code improvement and the private key is now just a private key.

Another one: I am not sure if the signing_key in SigningSettings is correct to be a global (and particularly a pair of (Key, Certificate)). It's conceivable to me that one might have multiple identities such as when signing .pkg files, which use different certs from the executables within. That depends on further refactors anyhow, though.


To move forward, I need advice on how to write tests for this.

@lf- lf- force-pushed the jade/push-rxryprnrpxsw branch 4 times, most recently from 32e2b08 to 3abf93f Compare January 29, 2026 23:09
lf- added 2 commits February 2, 2026 15:45
I found a bug in the docs in cryptography-rs, which I think actually is
a bug in basically every extant provider: indygreg/cryptography-rs#68
We also don't use the sign() function afaik, so I suspect it was never
noticed.

TODOs:
- Figure out testing. Integration testing this is probably going to
  suck, but it is possible. Try: https://github.com/nsmithuk/local-kms

  I don't think there is that much *code* to unit test here and we would
  probably have to introduce a mocking layer over the KMS API (which
  seems to likely defeat the purpose).
- Cleanup pass on the code.
  - DRY up some common functionality between PKCS11 and AWS KMS. I
    assume this is going to be reused by most implementations of cloud
    signing.
- Remove default-on aws-kms feature.
This requires a copy of the go binary from
https://github.com/nsmithuk/local-kms, which is trivial to either build
or download.

I don't know how to integrate this into the overall test suite yet, so
I'm pushing what I have.
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.

1 participant