Update SigningConfig to specify API version and validity periods by Hayden-IO · Pull Request #539 · sigstore/protobuf-specs

Hayden-IO · 2025-02-14T00:39:01Z

In order to faciliate clients gracefully handling breaking API changes, the SigningConfig will now include API versions for each of the service URLs so that clients can determine what services they are compatible with. Additionally, we've included validity periods which will be used to faciliate Rekor log sharding, when we spin up new log shards and distribute new key material.

Fixes #474

Summary

Release Note

Documentation

Hayden-IO · 2025-02-14T00:42:48Z

A few things to note:

~~I kept the old url fields because I'm not sure if that will break sigstore-python, which supports ClientTrustConfig.~~ Edit: The old fields have been removed.
That also means we need a new media type version, which I added.
I've included comments for the selection process. For CA and OIDC URLs, the client must select one URL that matches its criteria. ~~For Rekor and TSA URLs, the client must select all URLs that match its criteria. If this is confusing, lemme know and we can think about grouping things differently.~~ Edit: The Rekor and TSA URLs are now selected based on an explicitly provided selector.
~~Naming is hard because we've already used the abbreviated names I would have used for the pluralized fields....so the names are a bit verbose.~~ Edit: No longer an issue with removal of old fields.

kommendorkapten

Looks good!

cmurphy · 2025-02-14T18:44:52Z

protos/sigstore_trustroot.proto

+        // Clients must select only Service with the highest API version
+        // that the client is compatible with and that is within its
+        // validity period. Clients should select the first Service
+        // that meets this requirement.


If the sort order is the same as for TrustedRoot

protobuf-specs/protos/sigstore_trustroot.proto

Lines 108 to 109 in e943ce1

// All the listed instances SHOULD be sorted by the 'valid_for' in ascending

// order, that is, the oldest instance first. Only the last instance is

then this could result in a client choosing a service that is valid but about to expire, when a later service in the list would be valid for longer.

Good call out. I've added the following:

// All listed Services SHOULD be sorted by the `valid_for` window in // descending order, with the newest instance first.

With the newest instance first, this should limit how far into the list clients need to search to find a valid instance. Typically a client will just select the first from the list. With a sharding, a client will select the second (old shard) until the first in the list (new shard) becomes valid. For API changes, a client may have to search further into the list, but major API changes will be infrequent.

cmurphy · 2025-02-14T18:45:43Z

protos/sigstore_trustroot.proto

+        // These URL **MUST** be the "base" URLs for the transparency logs,
+        // which clients should construct appropriate API endpoints on top of.
+        //
+        // Clients must select ALL Services with the highest API version


This seems like a big change, as clients don't currently upload entries to multiple logs at once, and may not always want to. Could this be changed to ANY?

Originally, I was going to say that since this is for signing, I would expect that a user provide all the logs that they want to contact. Thinking more on this, Certificate Transparency has a similar concept with log lists, and doesn't require its clients to write to all logs, only a subset.

I'm proposing now that we include a configuration for how clients should select Rekor and TSA instances (we don't need a configuration for Fulcio and the OIDC provider because the bundle only supports one certificate). The config will specify a selection of ANY, ALL, or EXACT, with another field count when EXACT is specified (like CAs writing to 2 logs based on browser requirements).

We need to decide on what we as the PGI operator will set. I think ANY is fine since I expect we'll only ship one log for now.

Thanks for this idea!

cmurphy · 2025-02-14T18:46:25Z

protos/sigstore_trustroot.proto

+        // that the client is compatible with and that is within its
+        // validity period. Clients should select the first Service
+        // that meets this requirement.
+        repeated Service certificate_authority_urls = 6;


I wish the new field names were more meaningfully different from the old names but I don't have a great suggestion.

With the removal of the old fields, I've renamed them to the abbreviated service names

jku · 2025-02-18T13:52:16Z

protos/sigstore_trustroot.proto

+        // MUST be application/vnd.dev.sigstore.signingconfig.v0.1+json or
+        // application/vnd.dev.sigstore.signingconfig.v0.2+json


This seems a bit unclear: after this PR this message documents v0.2 only but that's not said anywhere. Do we allow v0.1 to be used in future as well?

Bundle uses this language:

// MUST be application/vnd.dev.sigstore.bundle.v0.3+json when encoded as JSON. // Clients must to be able to accept media type using the previously defined formats: // [list follows]

Of course situation here is a bit different: we should document

what we expect the instance to serve

I'd like to say "MUST serve v0.2" but obviously there's a migration period here

what we expect clients to handle

MUST handle v0.2, SHOULD handle v0.1

Updated to serve only v0.2, and say clients can optionally handle v0.1. In practice, I assume all will drop support.

jku · 2025-02-18T13:56:35Z

protos/sigstore_trustroot.proto

+        // application/vnd.dev.sigstore.signingconfig.v0.2+json
        string media_type = 5;

+        // Deprecated: Use certificate_authority_urls


Can you explain the way these deprecations are useful (compared to just removing the fields):

if a service does not want to support the new fields, I would expect them to just keep serving signingconfig v0.1?

if a client supports signingvonfig v0.2 then they need to support the new fields so won't need these

Maybe I'm missing a case?

is the idea that clients have an easier time supporting v0.2 if they can just update version number and keep doing what they are doing now?

I was aiming to concurrently support both to avoid a breaking change, but you're right, this isn't necessary.

Also, only sigstore-python and -go support signing configs and I'm fairly certain no one is using them for the Go client, so making this breaking change is likely to have no impact.

jku · 2025-02-18T14:06:33Z

I guess one option to make the transition to new SigningConfig as easy as possible is to not replace the old one but just offer a new one:

a new TUF artifact signing_config2.json is added alongside the existing signing_config.json
the clients can implement support on their schedule: instead of parsing signing_config.json they start download/parse signing_config2.json
at some point in future when enough clients have transitioned, signing_config.json artifact is removed from TUF repository

This would be a way to avoid a flag day for SigningConfig

jku · 2025-02-18T14:08:36Z

I kept the old url fields because I'm not sure if that will break sigstore-python, which supports ClientTrustConfig.

pretty sure sigstore-python will not parse the new data without code changes since the signingconfig version number changes

jku · 2025-02-18T17:24:28Z

I guess one option to make the transition to new SigningConfig as easy as possible is to not replace the old one but just offer a new one:

I'll have a better look at this again this week as I feel there is something to this idea -- we may be mistakenly combining the two problem cases (backwards incompatible service migration and plain log instance rotation), the right solutions to each might actually be different

jku · 2025-02-25T14:14:33Z

Okay, I couldn't actually make experiments with sigstore-python because of an ongoing protobuf-specs incompatibility, but I have tried to fully understand this and here's my current thinking:

On versioning SigningConfig itself

technically this PR makes only "compatible" changes to SigningConfig so we could update signing_config.json in root-signing without major issues. root-signing artifacts should be considered API and changes should avoid flag days when possible so this seems good... however:
the moment we switch on a rekor v2 log and add it to the config in root-signing, this compatibility is shown to be fake: some clients seem to be compatible with the new signingconfig but that's only because they rely on the deprecated fields that won't contain the new log

Let's consider this approach:

this pr actually makes incompatible SigningConfig changes (in other words the deprecated fields are removed)
we add a new artifact into root-signing: "signing_config_N.json" (where N is the version), the old config remains as "signing_config.json"
at this point clients can be updated to handle the new artifact (if we have already added the new artifact to both root-signing-staging and root-signing, they can remove the old signingconfig handling at the same time)
at some point in future we fully deprecate old signing_config by removing the old artifact from root-signing

I think that might be best of both worlds: avoids a flag day but still makes it clear the API actually changes (and allows us to see from CDN logs how much the old signing_config.json is still used). The counter argument could be that only sigstore-python is using signing_config.json so there's no point in considering this a real API change... but in that case I guess we can just remove the deprecated fields and not care about compatibility? comments @haydentherapper?

On Service.major_api_version

It's really hard to say if this will actually be a mechanism that works in cases that are not this specific upgrade (rekor v1->v2) but I can't figure out anything smarter. LGTM

Hayden-IO · 2025-03-11T23:19:36Z

@jku Thanks for the review and suggestions. I've updated this PR to remove the old fields. This is cleaner, it likely doesn't affect any clients or users, and for all the reasons you've stated, this should be fine if we version the signing config.

Given that no clients are using the public-good-instance-provided signing config at the moment, I would suggest we simply update the existing signing config. I like this idea for versioning configs and so we should document this to use this in the future for the next time we make a breaking change that removes fields.

In order to faciliate clients gracefully handling breaking API changes, the SigningConfig will now include API versions for each of the service URLs so that clients can determine what services they are compatible with. Additionally, we've included validity periods which will be used to faciliate Rekor log sharding, when we spin up new log shards and distribute new key material. Fixes sigstore#474 Signed-off-by: Hayden B <8418760+haydentherapper@users.noreply.github.com>

protos/sigstore_trustroot.proto

kommendorkapten

Nice work!

Signed-off-by: Hayden B <8418760+haydentherapper@users.noreply.github.com>

kommendorkapten

Thank you!

Hayden-IO mentioned this pull request Feb 14, 2025

SigningConfig proto to have start dates? #474

Closed

Hayden-IO requested review from jku, kommendorkapten, loosebazooka and woodruffw February 14, 2025 00:43

kommendorkapten previously approved these changes Feb 14, 2025

View reviewed changes

cmurphy reviewed Feb 14, 2025

View reviewed changes

jku reviewed Feb 18, 2025

View reviewed changes

Hayden-IO dismissed kommendorkapten’s stale review via c2768a6 March 11, 2025 23:26

Hayden-IO force-pushed the sc-updates branch from dbd11fe to c2768a6 Compare March 11, 2025 23:26

Hayden-IO force-pushed the sc-updates branch from c2768a6 to da14d87 Compare March 11, 2025 23:32

Hayden-IO commented Mar 12, 2025

View reviewed changes

protos/sigstore_trustroot.proto Show resolved Hide resolved

kommendorkapten previously approved these changes Mar 12, 2025

View reviewed changes

Add comments for service selector enums

6efede5

Signed-off-by: Hayden B <8418760+haydentherapper@users.noreply.github.com>

Hayden-IO dismissed kommendorkapten’s stale review via 6efede5 March 12, 2025 17:53

kommendorkapten approved these changes Mar 13, 2025

View reviewed changes

Hayden-IO merged commit 4752ba2 into sigstore:main Mar 13, 2025
22 checks passed

Hayden-IO deleted the sc-updates branch March 13, 2025 18:10

	// All the listed instances SHOULD be sorted by the 'valid_for' in ascending
	// order, that is, the oldest instance first. Only the last instance is

		// MUST be application/vnd.dev.sigstore.signingconfig.v0.1+json or
		// application/vnd.dev.sigstore.signingconfig.v0.2+json

Conversation

Hayden-IO commented Feb 14, 2025

Summary

Release Note

Documentation

Uh oh!

Hayden-IO commented Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kommendorkapten left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jku Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jku commented Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jku commented Feb 18, 2025

Uh oh!

jku commented Feb 18, 2025

Uh oh!

jku commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

On versioning SigningConfig itself

On Service.major_api_version

Uh oh!

Hayden-IO commented Mar 11, 2025

Uh oh!

Uh oh!

kommendorkapten left a comment

Choose a reason for hiding this comment

Uh oh!

kommendorkapten left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Hayden-IO commented Feb 14, 2025 •

edited

Loading

jku Feb 18, 2025 •

edited

Loading

jku commented Feb 18, 2025 •

edited

Loading

jku commented Feb 25, 2025 •

edited

Loading