-
Notifications
You must be signed in to change notification settings - Fork 137
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Tink signing backend #645
Conversation
Codecov Report
@@ Coverage Diff @@
## main #645 +/- ##
==========================================
- Coverage 62.02% 61.85% -0.18%
==========================================
Files 29 30 +1
Lines 1646 1717 +71
==========================================
+ Hits 1021 1062 +41
- Misses 552 571 +19
- Partials 73 84 +11
Continue to review full report at Codecov.
|
f291e2e
to
4302340
Compare
I'm not sure I follow this part - are we saying that we'd be storing an encrypted private key locally, then decrypting that private key to use for signing? The encryption/decryption would happen in KMS, but then the signing would happen with an in-memory key? |
Yep! It's similar to the file CA implementation, but instead of a password-protected signing key, it's a KMS-encrypted signing key. All signing would happen with an in-memory key. There's only one call to KMS, just to decrypt the key (unlike the current implementation which signs all certs with a KMS key) Here's the end to end flow:
|
Ah ok, thanks. Makes sense. The part that threw me off was around auditing access to the key - I guess it's technically correct that we can audit decryptions of the key, but if it were to leak we don't have any magic there. |
Yea, if the key leaked from memory somehow, there's nothing that can be done, but at rest, if the encrypted key is compromised, it should be infeasible to decrypt it without the KMS key/service account credentials. |
263ffd6
to
3fda04f
Compare
Tested with:
edit docker-compose to add necessary flags test with curl |
@bobcallaway @dlorenc Bumping PR, thanks! Just a note, the only untested function is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks!
This adds support for using encrypted Tink keysets to load a signer. There are two main benefits from this work: We can leverage this instead of KMS if we need to support a higher QPS, and Tink keysets use strong secure defaults. Keysets can be encrypted with AESGCM, do not rely on a KDF, cannot be brute-forced, and access to the key can be audited through cloud audit logs. Tink does not provide a method to extract the signing key from the keyset intentionally, so I wrote a helper library to reach into the key handle proto to construct a crypto.Signer. Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
Summary
This adds support for using encrypted Tink keysets to load a signer.
There are two main benefits from this work: We can leverage this instead
of KMS if we need to support a higher QPS, and Tink keysets use strong
secure defaults. Keysets can be encrypted with AESGCM, do not rely on a
KDF, cannot be brute-forced, and access to the key can be audited
through cloud audit logs.
Tink does not provide a method to extract the signing key from the
keyset intentionally, so I wrote a helper library to reach into the key
handle proto to construct a crypto.Signer.
Signed-off-by: Hayden Blauzvern hblauzvern@google.com
Ticket Link
Fixes #646
Release Note