-
Notifications
You must be signed in to change notification settings - Fork 57
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
Conversation
Signed-off-by: Slavek Kabrda <[email protected]>
There was a problem hiding this 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) { |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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"): |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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.
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. |
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! |
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). |
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. |
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.