Add package for Tessera integration #62

cmurphy · 2025-02-25T00:55:52Z

Summary

This change adds the workflow to instantiate the tessera storage driver
add an entry to the log. It copies most of the sigstore/rekor/pkg/signer
package to enable signing checkpoints with an on-disk key file or a KMS
provider.

The TransparencyLogEntry structure uses signed integers for the LogIndex
and TreeSize values, while tessera mostly returns unsigned integers for
these values. This change incorporates a conversion mechanism. It would
probably be better to update the protobuf-specs definition to use
unsigned integers.

Release Note

Documentation

Partial #8
Fixes #9

codecov · 2025-02-25T23:22:49Z

Codecov Report

Attention: Patch coverage is 34.72222% with 188 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@83a03c9). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
pkg/tessera/tessera.go	39.75%	40 Missing and 10 partials ⚠️
pkg/signer/tink/tink.go	51.19%	31 Missing and 10 partials ⚠️
pkg/signer/tink.go	23.25%	31 Missing and 2 partials ⚠️
pkg/tessera/note.go	0.00%	30 Missing ⚠️
pkg/tessera/safeint.go	38.88%	10 Missing and 1 partial ⚠️
pkg/signer/signer.go	0.00%	10 Missing ⚠️
pkg/tessera/gcp.go	0.00%	10 Missing ⚠️
pkg/signer/file.go	70.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main      #62   +/-   ##
=======================================
  Coverage        ?   20.83%           
=======================================
  Files           ?       15           
  Lines           ?      480           
  Branches        ?        0           
=======================================
  Hits            ?      100           
  Misses          ?      356           
  Partials        ?       24

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jku · 2025-02-27T08:17:57Z

The TransparencyLogEntry structure uses signed integers for the LogIndex and TreeSize values, while tessera mostly returns unsigned integers for these values. This change incorporates a conversion mechanism. It would probably be better to update the protobuf-specs definition to use unsigned integers.

I guess the protobuf change could be problematic (unless unsignedness is also guaranteed in rekor v1): TransparencyLogEntry is used in places like Bundle as well so should remain compatible, right?

loosebazooka · 2025-02-27T16:58:51Z

pkg/signer/file.go

@@ -0,0 +1,43 @@
+/*
+Copyright The Rekor Authors.


It's weird addlicense isn't catching this, but I think the other files are Copyright [YYYY] The Sigstore Authors

This was copied without modification from rekor v1 and I'm not sure what the rules are for changing the copyright. I think it wasn't caught by the linter because it's not by "The Sigstore Authors".

loosebazooka · 2025-02-27T20:55:58Z

pkg/signer/file.go

+	signature.SignerVerifier
+}
+
+func NewFile(keyPath, keyPass string) (*File, error) {


I find this naming confusing.

something like signer.FromFile(...) (or maybe, signer.NewFromFile) sounds nicer to me, but I'm still getting back into go conventions from Java land.

Yeah it's just copied from rekor https://github.com/sigstore/rekor/blob/main/pkg/signer/file.go

loosebazooka · 2025-02-27T21:04:24Z

pkg/signer/signer.go

+		}):
+		return kms.Get(ctx, signer, crypto.SHA256)
+	default:
+		return NewFile(signer, pass)


could this result in some confusing error messages? Where we don't find an expected matching KMS provider but then the default is invoked and fails. Then our error message is something about the NewFile constructor failing?

Yeah that is strange, this was also copypasted from rekor but probably worth fixing.

Actually no, I think this is by design. If it isn't a known KMS schema, there is no other way to interpret it except as a file path. It's also not really possible to definitively look at a string and determine if it's a file path, it could start with '/' or not, it could even just be '-' to indicate stdin (https://github.com/smallstep/crypto/blob/84f676bc58c42233dd7679c8530122f63ad8e740/internal/utils/io.go#L23). This is working in rekor v1, I think I'll leave it as is.

My rekor checkout was old and didn't include the new Tink provider, so I'll update that part.

loosebazooka · 2025-02-27T21:11:11Z

pkg/tessera/tessera.go

+	readTileFn client.TileFetcherFunc
+}
+
+func NewStorage(ctx context.Context, origin, bucket, spanner, keyPath, keyPass string) (*Storage, error) {


Does it make sense for this be named according to the backend type? How would we add other backend types?

Could bucket and spanner be abstracted interfaces for leaf storage and metadata storage?

I think it still makes sense for this to be Storage and NewStorage because the object is going to work the same no matter what the backend is. I'll change this to accept a tessera.Driver instead of a bucket and spanner string so the caller just has to create and pass in the driver, and it will work the same.

loosebazooka · 2025-02-27T21:14:01Z

pkg/tessera/tessera.go

+	readTileFn client.TileFetcherFunc
+}
+
+func NewStorage(ctx context.Context, origin, bucket, spanner, keyPath, keyPass string) (*Storage, error) {


Similarly, instead of accepting keyPath and keyPass, could it just accept the signer already instantiated? (then the code can be explicit about local key or kms)

cmurphy · 2025-02-27T21:45:00Z

@jku

TransparencyLogEntry is used in places like Bundle as well so should remain compatible, right?

Good point, it would probably be a minefield to change it.

jku

looks fine to me (although the tessera integration itself I can't competently evaluate), left a few questions.

WRT unit testing, can we leverage existing tests in sigstore/rekor or is the implementation so different that this makes no sense?

jku · 2025-02-27T08:36:35Z

pkg/signer/signer.go

+	default:
+		return NewFile(signer, pass)


this is existing code that's being moved so not asking for changes, just making sure I understand: the signer string is prefixed with the signer type but file signers have no prefix at all, it's just a path?

I see you already discussed this in another thread.

jku · 2025-02-27T08:47:18Z

pkg/tessera/note.go

+// Name returns the server name associated with the key.
+func (n *noteSigner) Name() string {
+	return n.name


this may be a noob golang question but are getters like this useful or expected?

There's definitely a cognitive load to them (while reading this I'm now keeping track of both noteSigner.name and notesigner.Name()).

I'll add some comments explaining. These methods are required so that noteSigner can implement https://pkg.go.dev/golang.org/x/mod/sumdb/note#Signer which Tessera requires for signing checkpoints.

jku · 2025-02-27T08:54:49Z

pkg/tessera/note.go

+// isValidName reports whether name is valid.
+// It must be non-empty and not have any Unicode spaces or pluses.


there's no context for name usage yet so ... is there some rekor/tessera specific criteria here or where does "validity" come from?

Here's the context https://github.com/C2SP/C2SP/blob/main/tlog-checkpoint.md#note-text I'll add a comment

jku · 2025-02-27T09:22:35Z

pkg/tessera/safeint.go

+// safeInt64 holds equivalent int64 and uint64 integers.
+type safeInt64 struct {
+	u uint64
+	i int64
+}


Can you (or one of the tessera folks) expand on why this works -- are checkpoint indices guaranteed to be below MaxInt64?

It's not a guarantee by Tessera. It's an assumption in Rekor v1 and only because Rekor v1 uses signed ints. I'm using it here for compatibility with the TransparencyLogEntry definition, which uses signed ints. The operator would need to be aware of this limitation and start a new tree before the size reaches MaxInt64.

loosebazooka

lgtm, will wait on jku's comments to be resolved.

loosebazooka · 2025-02-28T15:37:32Z

pkg/signer/signer.go

+)
+
+// New returns a Signer for the given KMS provider, Tink, or a private key file on disk.
+func New(ctx context.Context, signer, pass, tinkKEKURI, tinkKeysetPath string) (signature.Signer, error) {


I'm still a little confused by this New. There's too many optional parameters in this method, its a bit funky. Is this just so we pass CLI options directly into the new?

Anyway, since we're just copying things over, we should probably leave as is, I guess I can go back in later and mess with it.

Basically yeah:

https://github.com/sigstore/rekor/blob/c820fcaf3afdc91f0acf6824d55c1ac7df249df1/pkg/api/api.go#L173-L178
https://github.com/sigstore/rekor/blob/c820fcaf3afdc91f0acf6824d55c1ac7df249df1/pkg/sharding/ranges.go#L149-L150

It got more complicated when tink was added in sigstore/rekor#2228

cmurphy

WRT unit testing, can we leverage existing tests in sigstore/rekor or is the implementation so different that this makes no sense?

For this part, there's no equivalent in sigstore/rekor because there's no Tessera, and Rekor doesn't even directly use the TLE in its API responses. Once I actually start writing the HTTP handlers I may be able to pull more in from rekor v1.

cmurphy · 2025-02-28T17:20:07Z

pkg/signer/signer.go

+)
+
+// New returns a Signer for the given KMS provider, Tink, or a private key file on disk.
+func New(ctx context.Context, signer, pass, tinkKEKURI, tinkKeysetPath string) (signature.Signer, error) {


Basically yeah:

https://github.com/sigstore/rekor/blob/c820fcaf3afdc91f0acf6824d55c1ac7df249df1/pkg/api/api.go#L173-L178
https://github.com/sigstore/rekor/blob/c820fcaf3afdc91f0acf6824d55c1ac7df249df1/pkg/sharding/ranges.go#L149-L150

It got more complicated when tink was added in sigstore/rekor#2228

cmurphy · 2025-02-28T17:22:22Z

pkg/tessera/note.go

+// Name returns the server name associated with the key.
+func (n *noteSigner) Name() string {
+	return n.name


I'll add some comments explaining. These methods are required so that noteSigner can implement https://pkg.go.dev/golang.org/x/mod/sumdb/note#Signer which Tessera requires for signing checkpoints.

cmurphy · 2025-02-28T17:25:06Z

pkg/tessera/note.go

+// isValidName reports whether name is valid.
+// It must be non-empty and not have any Unicode spaces or pluses.


Here's the context https://github.com/C2SP/C2SP/blob/main/tlog-checkpoint.md#note-text I'll add a comment

cmurphy · 2025-02-28T17:28:55Z

pkg/tessera/safeint.go

+// safeInt64 holds equivalent int64 and uint64 integers.
+type safeInt64 struct {
+	u uint64
+	i int64
+}


It's not a guarantee by Tessera. It's an assumption in Rekor v1 and only because Rekor v1 uses signed ints. I'm using it here for compatibility with the TransparencyLogEntry definition, which uses signed ints. The operator would need to be aware of this limitation and start a new tree before the size reaches MaxInt64.

This change adds the workflow to instantiate the tessera storage driver add an entry to the log. It copies most of the sigstore/rekor/pkg/signer package to enable signing checkpoints with an on-disk key file or a KMS provider. The TransparencyLogEntry structure uses signed integers for the LogIndex and TreeSize values, while tessera mostly returns unsigned integers for these values. This change incorporates a conversion mechanism. It would probably be better to update the protobuf-specs definition to use unsigned integers. Signed-off-by: Colleen Murphy <[email protected]>

cmurphy · 2025-02-28T17:52:44Z

Here's the script I've been using to test this, if it's of interest https://gist.github.com/cmurphy/0ae387aceba197ce0490fa75d4e2ffd6

It also shows why I wanted to put pkg/signer in this PR even though pkg/tessera isn't directly using it.

haydentherapper · 2025-03-03T05:09:19Z

pkg/signer/tink_test.go

+	"strings"
+	"testing"
+
+	tinkUtils "github.com/sigstore/rekor/pkg/signer/tink"


Update the import to this package?

haydentherapper · 2025-03-03T06:59:55Z

pkg/tessera/note.go

+// noteSigner uses an arbitrary sigstore signer to implement golang.org/x/mod/sumdb/note.Signer,
+// which is used in Tessera to sign checkpoints in the signed notes format
+// (https://github.com/C2SP/C2SP/blob/main/signed-note.md).
+type noteSigner struct {


Should we call this an ECDSA note signer? From C2SP, the key ID calculation is for the 0x02 signature type.

This is sigstore/rekor#2062.

We can use Tessera's note signer if the key is ed25519, and this signer if the key is ecdsa.

If it's RSA, we need to define our own key ID since that is not defined by C2SP. I'd suggest SHA256(key name || 0xA || 0xFF || "RSA-PKCS#1v1.5" || public key)[:4]. 0xFF comes from the spec, and we include an additional identifier recommended by the spec.

cmurphy force-pushed the tessera branch from 1456694 to ef1f5a3 Compare February 25, 2025 23:20

cmurphy force-pushed the tessera branch 3 times, most recently from fe79d30 to 0d767f8 Compare February 27, 2025 00:35

cmurphy changed the title ~~WIP: Add package for Tessera integration~~ Add package for Tessera integration Feb 27, 2025

cmurphy marked this pull request as ready for review February 27, 2025 00:39

cmurphy requested review from a team as code owners February 27, 2025 00:39

loosebazooka reviewed Feb 27, 2025

View reviewed changes

cmurphy force-pushed the tessera branch from 0d767f8 to 75fa63e Compare February 27, 2025 22:22

jku reviewed Feb 28, 2025

View reviewed changes

loosebazooka reviewed Feb 28, 2025

View reviewed changes

cmurphy commented Feb 28, 2025

View reviewed changes

cmurphy force-pushed the tessera branch from 75fa63e to 068198d Compare February 28, 2025 17:47

cmurphy force-pushed the tessera branch from 068198d to 31452da Compare February 28, 2025 17:51

haydentherapper reviewed Mar 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add package for Tessera integration #62

Add package for Tessera integration #62

cmurphy commented Feb 25, 2025 •

edited

Loading

codecov bot commented Feb 25, 2025 •

edited

Loading

jku commented Feb 27, 2025 •

edited

Loading

loosebazooka Feb 27, 2025

cmurphy Feb 27, 2025

loosebazooka Feb 27, 2025

cmurphy Feb 27, 2025

loosebazooka Feb 27, 2025

cmurphy Feb 27, 2025

cmurphy Feb 27, 2025

loosebazooka Feb 27, 2025

cmurphy Feb 27, 2025

loosebazooka Feb 27, 2025

cmurphy Feb 27, 2025

cmurphy commented Feb 27, 2025

jku left a comment

jku Feb 27, 2025

jku Feb 28, 2025

jku Feb 27, 2025

cmurphy Feb 28, 2025

jku Feb 27, 2025

cmurphy Feb 28, 2025

jku Feb 27, 2025

cmurphy Feb 28, 2025

loosebazooka left a comment

loosebazooka Feb 28, 2025

cmurphy Feb 28, 2025

cmurphy left a comment

cmurphy Feb 28, 2025

cmurphy Feb 28, 2025

cmurphy Feb 28, 2025

cmurphy Feb 28, 2025

cmurphy commented Feb 28, 2025

haydentherapper Mar 3, 2025

haydentherapper Mar 3, 2025

		// isValidName reports whether name is valid.
		// It must be non-empty and not have any Unicode spaces or pluses.

Add package for Tessera integration #62

Are you sure you want to change the base?

Add package for Tessera integration #62

Conversation

cmurphy commented Feb 25, 2025 • edited Loading

Summary

Release Note

Documentation

codecov bot commented Feb 25, 2025 • edited Loading

Codecov Report

jku commented Feb 27, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmurphy commented Feb 27, 2025

jku left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loosebazooka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmurphy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmurphy commented Feb 28, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmurphy commented Feb 25, 2025 •

edited

Loading

codecov bot commented Feb 25, 2025 •

edited

Loading

jku commented Feb 27, 2025 •

edited

Loading