Support self signed certificate chain #989

jku · 2025-03-12T13:39:15Z

This makes fetch_tsa_certs.go support a self-signed certificate chain as well:

if --gcp-ca-parent is given, tool works like before (creates a chain with root, intermediate and leaf). If it is not given, the chain will contain a selfsigned root and leaf instead
--parent-validity=DAYS was added since we can no longer rely on CA to set this in the self signing case
--intermediate-kms-resource was renamed --parent-kms-resource so it works for both the intermediate case and self-signed case

Fixes #988

This change does not affect functionality yet (apart from slightly changing the error messages if --gcp-ca-parent is not provided): it's just refactoring to enable future work. Most importantly naming of certificates and keys in the fetchCertificateChain method is now consistent: * "leaf" refers to the actual timestamp signing key/cert * "parent" is the key/cert that signs "leaf": it may be a self-signed certicifate or an intermediate signed by CA * "root" is a real CA certificate that signs "parent" (if a CA is used) Signed-off-by: Jussi Kukkonen <[email protected]>

It is now possible to create a certificate chain that does not use a real CA but instead has a self-signed signing certificate as the parent of of the timestamp signing certificate. This is a little experimental (e.g. cert lifetime is just hard coded now). Signed-off-by: Jussi Kukkonen <[email protected]>

This is useful mostly for the self signed case Signed-off-by: Jussi Kukkonen <[email protected]>

codecov · 2025-03-12T13:43:05Z

Codecov Report

Attention: Patch coverage is 0% with 115 lines in your changes missing coverage. Please review.

Project coverage is 44.13%. Comparing base (6fd19b0) to head (206b419).
Report is 317 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/fetch-tsa-certs/fetch_tsa_certs.go	0.00%	115 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #989      +/-   ##
==========================================
- Coverage   52.85%   44.13%   -8.73%     
==========================================
  Files          20       55      +35     
  Lines        1209     3707    +2498     
==========================================
+ Hits          639     1636     +997     
- Misses        509     1931    +1422     
- Partials       61      140      +79

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

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jku · 2025-03-12T14:05:30Z

CC @haydentherapper

cmd/fetch-tsa-certs/fetch_tsa_certs.go

These are needed to get BasicConstraints in the cert. Signed-off-by: Jussi Kukkonen <[email protected]>

jku · 2025-03-12T16:30:29Z

This is what the self-signed CA looks like now:

Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            54:56:2b:03:7e:a7:05:19:7d:79:08:66:3b:c4:d1:bc:bb:25:2c:bf
        Signature Algorithm: ecdsa-with-SHA256
        Issuer: O=sigstore.dev, CN=sigstore-tsa-selfsigned
        Validity
            Not Before: Mar 12 16:25:08 2025 GMT
            Not After : Mar 12 16:25:08 2045 GMT
        Subject: O=sigstore.dev, CN=sigstore-tsa-selfsigned
        Subject Public Key Info:
            Public Key Algorithm: id-ecPublicKey
                Public-Key: (256 bit)
                pub:
                    04:b9:b8:a9:57:b8:c5:e4:cd:4c:7f:82:00:f2:7a:
                    33:0d:c6:f9:9a:70:55:97:35:49:ae:2c:71:d4:0b:
                    ea:21:14:35:05:07:85:1e:7f:bd:fc:39:fc:87:66:
                    55:9a:c3:92:0e:37:bd:b0:b1:a3:60:cb:df:67:f5:
                    46:79:8d:df:ad
                ASN1 OID: prime256v1
                NIST CURVE: P-256
        X509v3 extensions:
            X509v3 Key Usage: critical
                Certificate Sign, CRL Sign
            X509v3 Basic Constraints: critical
                CA:TRUE, pathlen:0
            X509v3 Subject Key Identifier: 
                23:16:7F:90:75:59:C2:0B:BB:71:82:25:04:B1:CA:D4:7A:C6:0A:14
    Signature Algorithm: ecdsa-with-SHA256
    Signature Value:
        30:45:02:20:25:1d:ab:e5:e7:3a:9c:0a:43:36:c6:b6:7d:90:
        a2:aa:c7:65:e2:36:21:0d:6c:c2:a3:c9:ca:30:76:29:c0:a7:
        02:21:00:b8:d2:bb:42:ee:4a:97:44:3f:b3:53:b8:84:31:0a:
        b8:a2:6e:ef:8f:bc:79:e3:af:2c:c5:c9:05:8f:39:ec:d4

$ openssl verify -verbose -CAfile self-signed leaf
leaf: OK
$ openssl verify -verbose -CAfile self-signed self-signed
self-signed: OK

cmd/fetch-tsa-certs/fetch_tsa_certs.go

haydentherapper · 2025-03-12T18:44:36Z

cmd/fetch-tsa-certs/fetch_tsa_certs.go

+func fetchCertificateChain(ctx context.Context, root, parentKMSKey, leafKMSKey, tinkKeysetPath, tinkKmsKey string,
 	client *privateca.CertificateAuthorityClient) ([]*x509.Certificate, error) {
-	intermediateKMSSigner, err := kms.Get(ctx, intermediateKMSKey, crypto.SHA256)
+	parentKMSSigner, err := kms.Get(ctx, parentKMSKey, crypto.SHA256)


Can we make the hash configurable via a flag? I believe this will err out if the hash func doesn't match what the KMS provider supports, e.g. ecdsa-p384 needs sha-384. We can default to sha256.

I added a few options. The method call is a bit unwieldy now (with so many options) but I resisted urge to do more cleanup in the same PR

Scratch that: I tried to do this by just providing other values to kms.Get() and the signature algorithm in the certificate does not change (206b419)...

This makes sense when you consider that the (GCP) KMS signing key has the hash function baked in: I don't think you can change it like this

I'm not quite sure why kms.Get() has a hash function as an argument?

In any case implementing this may not be the 15 min job I imagined: I would rather not include this somewhat unrelated change in the PR. Let's file an issue if the feature is needed.

Sigh, sorry for making you go down this rabbit hole. Only the hashivault KMS provider uses the hash. I thought all did to determine which hash to use when computing the digest to sign...oh well. Crypto agility is fun.

cmd/fetch-tsa-certs/fetch_tsa_certs.go

haydentherapper · 2025-03-12T18:49:43Z

Root cert LGTM!

Signed-off-by: Jussi Kukkonen <[email protected]>

This changes one option that existed before this PR: "--intermediate-kms-resource" is now "--parent-kms-resource". The idea is that now this works in boths cases: * parent is used as intermediate if a CA is provided * parent is used as a self signed root if a CA is not provided This also sorts the options the same way everywhere: leaf options, then parent options, then CA root options, finally output options. Signed-off-by: Jussi Kukkonen <[email protected]>

Signed-off-by: Jussi Kukkonen <[email protected]>

jku added 3 commits March 12, 2025 15:41

Add --intermediate-validity flag

8fbdbc8

This is useful mostly for the self signed case Signed-off-by: Jussi Kukkonen <[email protected]>

jku force-pushed the support-self-signed-chain branch from 703833a to 8fbdbc8 Compare March 12, 2025 13:41

jku commented Mar 12, 2025

View reviewed changes

cmd/fetch-tsa-certs/fetch_tsa_certs.go Outdated Show resolved Hide resolved

fetch_tsa_certs: set additional certificate arguments

8db16a1

These are needed to get BasicConstraints in the cert. Signed-off-by: Jussi Kukkonen <[email protected]>

jku force-pushed the support-self-signed-chain branch from fa8b71d to 8db16a1 Compare March 12, 2025 16:28

haydentherapper reviewed Mar 12, 2025

View reviewed changes

jku added 4 commits March 13, 2025 12:19

README: Rewrite examples to include self-signed certificate

c850bfd

Signed-off-by: Jussi Kukkonen <[email protected]>

fetch-tsa-certs: Use validity argument for self-signed cert

dc5400c

Signed-off-by: Jussi Kukkonen <[email protected]>

fetch-tsa-certs: Uncapitalize local symbol

4669b9f

Signed-off-by: Jussi Kukkonen <[email protected]>

jku force-pushed the support-self-signed-chain branch 3 times, most recently from 206b419 to 4669b9f Compare March 13, 2025 11:48

jku marked this pull request as ready for review March 13, 2025 11:58

jku requested a review from a team as a code owner March 13, 2025 11:58

jku mentioned this pull request Mar 14, 2025

Setup TSA in staging sigstore/rekor-tiles#65

Closed

haydentherapper approved these changes Mar 14, 2025

View reviewed changes

haydentherapper merged commit ddb0233 into sigstore:main Mar 14, 2025
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Support self signed certificate chain #989

Support self signed certificate chain #989

Uh oh!

jku commented Mar 12, 2025 •

edited

Loading

Uh oh!

codecov bot commented Mar 12, 2025 •

edited

Loading

Uh oh!

jku commented Mar 12, 2025

Uh oh!

Uh oh!

jku commented Mar 12, 2025

Uh oh!

Uh oh!

haydentherapper Mar 12, 2025

Uh oh!

jku Mar 13, 2025

Uh oh!

jku Mar 13, 2025 •

edited

Loading

Uh oh!

haydentherapper Mar 14, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

haydentherapper commented Mar 12, 2025

Uh oh!

Uh oh!

Uh oh!

Support self signed certificate chain #989

Support self signed certificate chain #989

Uh oh!

Conversation

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

Uh oh!

Uh oh!

codecov bot commented Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jku commented Mar 12, 2025

Uh oh!

Uh oh!

jku commented Mar 12, 2025

Uh oh!

Uh oh!

haydentherapper Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

jku Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

jku Mar 13, 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!

haydentherapper Mar 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

haydentherapper commented Mar 12, 2025

Uh oh!

Uh oh!

Uh oh!

jku commented Mar 12, 2025 •

edited

Loading

codecov bot commented Mar 12, 2025 •

edited

Loading

jku Mar 13, 2025 •

edited

Loading