Skip to content

Add support for Tink keyset signer #282

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

haydentherapper
Copy link

This PR adds support for in-memory signing using a Tink keyset. The keyset is encrypted with a key-encryption-key stored in GCP KMS. The key is decrypted on startup and loaded into memory. This uses a utility to unpack the keyset into a crypto.Signer so that it can be used to sign certificates. This also validates that the key is an ECDSA P-256 key as per RFC 6962, since Tink supports many key types.

@haydentherapper
Copy link
Author

@phbnf Let me know if you want to chat more about this! This would help us to deploy static-ct without managing keys in multiple ways, as we're switching over all services to use Tink keysets.

cmd/gcp/tink.go Outdated
if err != nil {
return nil, err
}
signer, _, err := tinkUtils.KeyHandleToSigner(kh)
Copy link
Author

Choose a reason for hiding this comment

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

Actively making a breaking change to this API in sigstore/sigstore#2069, to drop the unused return value.

Copy link
Author

Choose a reason for hiding this comment

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

For more info, this is needed because Tink doesn't provide an API to transform a key handle or keyset into a crypto.Signer.

Copy link
Collaborator

Choose a reason for hiding this comment

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

[I'll leave the PR as a whole for Phil to comment on when he's back as he may already have plans for how to go about this, but this bit caught my eye so I thought I'd pop a drive-by comment on :) 🛻 ]

I suspect we'd probably prefer to copy the contents of tinkutils (which seems relatively small, and self-contained) over into an internal/tink directory rather than add an additional dep on sigstore.

Separately, I was wondering whether it might be possible to achieve something similar without cracking open the keys but wrapping at a higher level, e.g.:

<waving of hands>

type tinkSigner struct {
	sign func([]byte) ([]byte, error)
	pubK crypto.PublicKey
}

func (s *tinkSigner) Public() crypto.PublicKey {
	return s.pubK
}

func (s *tinkSigner) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) {
        // According to the docs, it seems like it's optional to actually use rand.
        // Should it check opts.Hash against whatever kh was configured with though?
	return s.sign(digest)
}

var _ crypto.Signer = &tinkSigner{}

// KeyHandleToSigner converts a key handle to the crypto.Signer interface.
func KeyHandleToSigner(kh *keyset.Handle) (crypto.Signer, error) {
	signer, err := signature.NewSigner(kh)
	if err != nil {
		return nil, err
	}
	pubH, err := kh.Public()
	if err != nil {
		return nil, err
	}
	verifier, err := signature.NewVerifier(pubH)
	if err != nil {
		return nil, err
	}
	s := &tinkSigner{
		sign: signer.Sign,
		pubK: verifier,
	}
	return s, nil
}

Copy link
Author

Choose a reason for hiding this comment

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

I can copy in the code, I was actually thinking about doing that originally. It's a small amount of code and shouldn't need active maintenance. It was also just updated by someone from the Tink development team as well.

The issue with that code is that Tink's signing interface takes in the data to sign, whereas crypto.Signer's Sign takes in a digest. I did find an issue discussing the idea of a prehash API, at which point then we could simplify to this proposal.

@phbnf
Copy link
Collaborator

phbnf commented May 9, 2025

I'm back! Thanks for sending the PR! My original plan was to keep things as bare bone as possible for Alpha, and then eventually add other mechanisms driven by demand.

Given CT load, it would be super expensive to sign every SCT with KMS, so the TesseraCT binary will do the signing, and the private key will live in memory. Taken this into account, I'm thinking of (1) also generating the keys from a small util binary running on GCP using Go's standard library, and to store it in secret manager directly, (2) relying on Secret Manager to encrypt/decrypt keys. This is the simplest reasonable approach I could come up with for Alpha in order to get the ball rolling. Would that allow you to get started and experiment?

It's far (far) from being perfect, and I can think of a bunch of reasons why someone running TesseraCT wouldn't want (1), nor (2), nor.. want to use Tink, and would come up with another mechanism. At which point, we'd en up having to maintain a lot of mechanism in this repo, which I'd like to avoid. If the scheme in the previous paragraph would be enough to get started, then I'd rather stick to it to keep things as simple and flexible as possible for Alpha, and then improve this later. What do you think? :)

@haydentherapper
Copy link
Author

Our usage of Tink is roughly the same as how you're using GCP Secrets! For some background, we aren't using the Tink library for signing and verification (particularly because the crypto.x509 functions need a crypto.Signer that signs a digest, rather than how Tink's signer takes in the preimage), we just use Tink as a way to store an encrypted signing key. In GCP, we initialize a KEK (key encryption key) that encrypts a signing key. On service startup, the KEK is used to decrypt the signing key and load it into memory.

In terms of threat model, a full compromise of the GCP project would result in the private key being leaked either with GCP Secrets or using the Tink KEK to decrypt the key. One benefit of Tink over GCP Secrets is that the key is never stored in the clear, which is why I had added support for Tink-encrypted signers awhile ago.

If we need to use GCP Secrets, we can do so, it just requires us creating additional configuration to load an externally managed secret into a Kubernetes pod.

Selfishly, I'd like to see Tink supported in static-ct so that our deployment for rekor-tiles and the Fulcio CT log can be identical. We can reuse all of our Terraform and Kubernetes configuration, particularly for this to create the KEK and mount the encrypted key in a config map.

This PR adds support for in-memory signing using a Tink keyset. The
keyset is encrypted with a key-encryption-key stored in GCP KMS. The key
is decrypted on startup and loaded into memory. This uses a utility to
unpack the keyset into a crypto.Signer so that it can be used to sign
certificates. This also validates that the key is an ECDSA P-256 key as
per RFC 6962, since Tink supports many key types.

Signed-off-by: Hayden B <8418760+haydentherapper@users.noreply.github.com>
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.

3 participants