Generate trusted_root.json in the TUF server #1235
Conversation
|
FWIW this is also related to #1001 - it addresses the part the TUF server to ensure it generates |
cmd/tuf/server/main.go
Outdated
| noK8s = flag.Bool("no-k8s", false, "Run in a non-k8s environment") | ||
| keysSecretName = flag.String("keyssecret", "", "Name of the secret to create for generated keys (keys won't be stored unless this is provided)") | ||
| noK8s = flag.Bool("no-k8s", false, "Run in a non-k8s environment") | ||
| metadataTargets = flag.Bool("metadata-targets", true, "Serve individual targets with custom Sigstore metadata") |
There was a problem hiding this comment.
Thanks for adding this flag - Can we add some more wording about this being deprecated in the future in favor of only using the trust root?
pkg/repo/repo.go
Outdated
|
|
||
| func getTargetUsage(name string) string { | ||
| for _, knownTargetType := range []string{FulcioTarget, RekorTarget, CTFETarget, TSATarget} { | ||
| if strings.Contains(name, strings.ToLower(knownTargetType)) { |
There was a problem hiding this comment.
Can we strings.ToLower(name) as well?
pkg/repo/repo.go
Outdated
| // * CreateRepoOptions.AddMetadataTargets: true | ||
| // * CreateRepoOptions.AddTrustedRoot: false | ||
| func CreateRepo(ctx context.Context, files map[string][]byte) (tuf.LocalStore, string, error) { | ||
| return CreateRepoWithOptions(ctx, files, CreateRepoOptions{AddMetadataTargets: true, AddTrustedRoot: false}) |
There was a problem hiding this comment.
Could we set both to True by default?
There was a problem hiding this comment.
My point here was to preserve backwards compatibility of what this function used to do before. If you think that is not an issue, I definitely can do that.
There was a problem hiding this comment.
I'm good with making the change - it adds a file, so it shouldn't be a change that causes compatibility issues.
| return CreateRepoWithOptions(ctx, files, CreateRepoOptions{AddMetadataTargets: true, AddTrustedRoot: false}) | ||
| } | ||
|
|
||
| func constructTrustedRoot(targets []TargetWithMetadata) (*TargetWithMetadata, error) { |
There was a problem hiding this comment.
Can we add a test for this?
There was a problem hiding this comment.
Sure, I will do that.
|
@haydentherapper I think I addressed all points from your review, except the one about perhaps the one about some docs - I can address that if you can advise where to put the docs. Thanks! |
|
Thanks, I left a comment about where we could put some docs! |
|
@haydentherapper thanks for the pointer, done! |
| now := time.Now() | ||
|
|
||
| // we sort the targets by Name, this results in intermediary certs being sorted correctly, | ||
| // as long as there is less than 10, which is ok to assume for the purposes of this code |
There was a problem hiding this comment.
Could you add a note about this in documentation?
There was a problem hiding this comment.
The README modification that I did says:
* `<target>_root.crt.pem`, e.g. `tsa_root.crt.pem`
* `<target>_intermediate_0.crt.pem`, e.g. `tsa_intermediate_0.crt.pem`
* `<target>_intermediate_1.crt.pem`, e.g. `tsa_intermediate_1.crt.pem`
* (more intermediates, but at most 10 intermediate certificates altogether)
* `<target>_leaf.crt.pem`, e.g. `tsa_leaf.crt.pem`
I don't see what's missing. Can you be more specific about what I should note?
There was a problem hiding this comment.
I missed the comment about 10 intermediates, all good!
pkg/repo/repo.go
Outdated
| certChain := []*x509.Certificate{} | ||
|
|
||
| // skip potential whitespace at end of file (8 is kinda random, but seems to work fine) | ||
| for len(rest) > 8 { |
There was a problem hiding this comment.
Could we use https://pkg.go.dev/strings#TrimSpace instead?
There was a problem hiding this comment.
Sounds good, I'll do that.
pkg/repo/repo.go
Outdated
| rest := bytes.TrimSpace(certChainPem) | ||
| certChain := []*x509.Certificate{} | ||
|
|
||
| // skip potential whitespace at end of file (8 is kinda random, but seems to work fine) |
There was a problem hiding this comment.
can remove outdated comment too
There was a problem hiding this comment.
Oh, sorry, I forgot about that.
Signed-off-by: Slavek Kabrda <bkabrda@redhat.com>
Signed-off-by: Slavek Kabrda <bkabrda@redhat.com>
Signed-off-by: Slavek Kabrda <bkabrda@redhat.com>
Co-authored-by: Hayden B <hblauzvern@google.com> Signed-off-by: Slavek Kabrda <bkabrda@redhat.com>
Signed-off-by: Slavek Kabrda <bkabrda@redhat.com>
Signed-off-by: Slavek Kabrda <bkabrda@redhat.com>
bkabrda
force-pushed
the
init-trusted-root-json
branch
from
f1e85bf
to
6be48a9
Compare
|
@haydentherapper rebased and addressed the last comment. Thank you for the great review! |
Summary
This PR replaces #1191 and is the last PR to address #1182 to make the TUF server useful for prod deployment - it allows for generating the
trusted_root.jsonfile and serving it as one of the TUF targets. It does the same as #1191, but uses the new functionality from sigstore-go.Release Note
The TUF server was improved to generate a
trusted_root.jsontarget 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.