Skip to content
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

Add functionality to generate trusted_root.json by the TUF server #1191

Closed
wants to merge 1 commit into from

Conversation

bkabrda
Copy link
Contributor

@bkabrda bkabrda commented Jul 24, 2024

Summary

This PR is the first of a series to address #1182. It adds functionality to the TUF server to generate a trusted_root.json as a target.

There is some overlap with the trtool project and in the future, ideally we would make a library to pull out the common functionality and share it.

Release Note

The TUF server now generates a trusted_root.json target from the supplied files.

Documentation

I don't believe that the TUF server is even documented anywhere, so I don't think documentation change is required, but please let me know if I'm wrong about that.

Signed-off-by: Slavek Kabrda <bkabrda@redhat.com>
Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

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

This is great, thank you!

Just left a few comments. One other one is that I think this is useful as a library, so can we add it straight to sigstore-go?

return CreateRepoWithMetadata(ctx, targets)
}

func constructTrustedRoot(targets []TargetWithMetadata) (*TargetWithMetadata, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move this to sigstore-go? There is some overlap with sigstore/cosign#3794, and I'd prefer to have this centrally located right off the bat.

@cmurphy FYI, since you were asking about utilities for trust root generation as well.

return 0, errors.New("unsupported elliptic curve")
case *rsa.PublicKey:
/*
NOTE: It is not possible to recognize padding from just the public key alone;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd just omit this commented out block, almost all Sigstore clients can't handle RSA-PSS keys.

switch getTargetUsage(target.Name) {
case FulcioTarget:
switch {
case strings.Contains(target.Name, "leaf"):
Copy link
Contributor

Choose a reason for hiding this comment

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

There should be no leaves in the trust root for Fulcio, it should only be intermediates and roots. The leaf would be the code signing certificate.

rest := certChainPem
certChain := v1Common.X509CertificateChain{Certificates: []*v1Common.X509Certificate{}}

// skip potential whitespace at end of file (8 is kinda random, but seems to work fine)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this is necessary, I believe whitespace will be handled by pem.Decode. Would be good to test for that.

@bkabrda
Copy link
Contributor Author

bkabrda commented Jul 26, 2024

Hi, thanks for the feedback! I can certainly try to add this to sigstore-go. I'll leave this PR open and when the sigstore-go work is merged, I'll update it to use that as a library from here.

@bkabrda
Copy link
Contributor Author

bkabrda commented Jul 26, 2024

Oh, I didn't realize that sigstore-go already has so much related code to this in https://github.com/sigstore/sigstore-go/tree/main/pkg/root, this actually makes so much sense... I don't know why I didn't think of looking there first. Thanks again for pointing me in the right direction!

@bkabrda
Copy link
Contributor Author

bkabrda commented Jul 30, 2024

Hi 👋 I created sigstore/sigstore-go#247. I have a local modification of this PR that uses the functionality from there and works fine. I'll push it once the sigstore-go PR is merged (ideally we would also release that if possible, not sure if/how I could help with that).

@bkabrda
Copy link
Contributor Author

bkabrda commented Aug 22, 2024

This PR is conflicting on multiple files and will need a significant rewrite to utilize sigstore/sigstore-go#247 anyway. I think it's going to be easier if I close it and open a new one instead.

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.

2 participants