Changing the detached SCT to the correct format

[haydentherapper]

Description

See sigstore/sigstore-python#24 - There was an issue around unmarshalling because the detached SCT is in an unexpected format.

Given that Fulcio 0.4 will break the Cosign client already, I'd like to just go ahead and fix the header. We'll need to hold off on Fulcio 0.4 and release Cosign 1.8 (with a change to expect a different struct for the detached SCT from Fulcio) first.

@dlorenc @cpanato FYI - Does this seem reasonable?

[haydentherapper]

Another option - When we switch the production instance to return embedded SCTs, any integrators need to support those, instead of detached SCTs. We can then fix detached SCTs later.

[haydentherapper]

Given that we're in the middle of releasing Fulcio 0.4 and Cosign 1.8, and given that the SCT format will change anyways once we roll out the new config changes for the production instance of Fulcio, let's not make any additional changes right now.

Once the production changes are rolled out, I'll update the detached SCT to be the right format (since at that point, no one will be impacted).

[di]

I'm not sure the current format is "wrong", it's just misleading to call it a signature. RFC 5246, section 4.7 calls this a 'digitally-signed element', and I think it could be useful to have additional means to verify the signature algorithm and hash algorithm.

Given that we're currently writing clients against this API, and since I'm guessing this wouldn't result in a version bump of the API, this would have the effect of breaking SCT verification for all existing clients, no?

[woodruffw]

I'm not sure the current format is "wrong", it's just misleading to call it a signature. RFC 5246, section 4.7 calls this a 'digitally-signed element', and I think it could be useful to have additional means to verify the signature algorithm and hash algorithm.

This is my thinking as well (although I'm admittedly less familiar with the ecosystem here). If I understand correctly, we do need some of the metadata in the DigitallySigned payload if Fulcio ever begins to do/support signatures that aren't ECDSA + SHA256.

[haydentherapper]

I'm referring to the subtle differences between the structs:

For the detached SCT, it's marshalled into a AddChainResponse:

type AddChainResponse struct {
	SCTVersion Version `json:"sct_version"` // SCT structure version
	ID         []byte  `json:"id"`          // Log ID
	Timestamp  uint64  `json:"timestamp"`   // Timestamp of issuance
	Extensions string  `json:"extensions"`  // Holder for any CT extensions
	Signature  []byte  `json:"signature"`   // Log signature for this SCT
}

Whereas the SignedCertificateTimestamp, what will be embedded in the certificate, contains the digitally signed type:

type SignedCertificateTimestamp struct {
	SCTVersion Version `tls:"maxval:255"`
	LogID      LogID
	Timestamp  uint64
	Extensions CTExtensions    `tls:"minlen:0,maxlen:65535"`
	Signature  DigitallySigned // Signature over TLS-encoded CertificateTimestamp
}

I think a client should be able to assume that the same struct is present in the embedded certificate and the header. I don't want to force clients to switch between these types, like what we currently do in cosign.

Given that we're currently writing clients against this API, and since I'm guessing this wouldn't result in a version bump of the API, this would have the effect of breaking SCT verification for all existing clients, no?

We're already going to be breaking clients when we switch to embedded SCTs, we're not going to send the header too. A warning is being added to the new release of Cosign, and we'll give the Python and Java clients a heads up before we switch (hopefully this week?).

[dlorenc]

Is this still an issue? We did the switch to embedded already.

[haydentherapper]

We did, so this doesn't affect production. This would be for modifying the response for self-hosted instances. While this isn't a production breaking change, it's a breaking change for self-hosted instances, so I assume we'd want it to be done before 1.0.

I could also be convinced that this isn't really a bug, but we're going to have to live with this indefinitely post-1.0 (or at least until 2.0)

[dlorenc]

To make sure I understand, it's still a "bug" if you run with detached SCTs, but we don't run with those in production anymore so that doesn't affect us, but it might affect someone else running with detached SCTs?

[haydentherapper]

That is correct.

If we do nothing, then existing clients will still work, it's just a quirk in the response format that we'd need to document for new clients. If we make this change, then we will need to update all clients.

[dlorenc]

Can we just deprecate/delete that response? I'm not sure anyone is using it or still cares.

[haydentherapper]

It's still needed for GCP CA Service unfortunately, since that doesn't support precertificates/embedded SCTs.

[dlorenc]

Ah, when we use that directly?

Then I think I vote for just not touching this and documenting the issue/quirk.

[haydentherapper]

#718