-
Notifications
You must be signed in to change notification settings - Fork 547
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
Feature: adds rekor support for cosign attach command #2994
Feature: adds rekor support for cosign attach command #2994
Conversation
c82d820
to
2cdab46
Compare
Signed-off-by: Mritunjay <mritunjaysharma394@gmail.com>
2cdab46
to
a6d00db
Compare
} | ||
defer sv.Close() | ||
|
||
bundle, err := uploadToTlog(ctx, sv, rekorURL, func(r *client.Rekor, b []byte) (*models.LogEntryAnon, 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.
A bundle should already exist, correct? This command, like attach signature, should take an existing bundle and attach it to the container metadata.
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.
Thanks for the review, Yes even I felt the same initially but I was skeptical and had little less idea being new to the code base on how we will get rekor Bundle struct back? What should the user pass as a flag? Should rekor-url be passed (like it is passed with attest command?) or should we straight up use sig.Bundle()
to see if it exists or not and if it does then write/mutate signature using withBundle() opts ?
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.
A rekor bundle is usually in the format:
{"Bundle":{"SignedEntryTimestamp":"MEUCIG...yoIY=","Payload":{"body":"...","integratedTime":1643917737,"logIndex":1,"logID":"4d2e4...97291"}
My only doubt is how will we allow the user to pass it to be attached? Or it hasn't has to be passed at all? And we can leverage sig.Bundle()
because it already exists but was not attached in containers manifest (sounds contradicting to me as I am unsure about the passing thing)
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 would accept a path to a file with the bundle. sig.Bundle
is where it's stored, and if it's set, it will be attached to the container.
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.
Got it so we will read that file and unmarshal it in the form of *bundle.RekorBundle
and do something like this, right?:
b, err := rekorBytes(rekorRef)
if err != nil {
return err
}
remoteOpts, err := regOpts.ClientOpts(ctx)
if err != nil {
return err
}
dstRef, err := ociremote.RekorTag(ref, remoteOpts...) //will have to create new func for RekorTag in ociremote
if err != nil {
return err
}
//rough pseudo code only to tell the logic
var bundle *bundle.RekorBundle
json.Unmarshal(b, bundle)
img, err := static.NewFile(b, static.WithBundle(bundle))
if err != nil {
return err
}
I would take a look at cosign/cmd/cosign/cli/attach/sig.go Line 34 in 92df24c
|
Thanks a lot @haydentherapper, I tried to took my inspiration from there only but I was confused on how we will take this bundle? As []bytes (would be difficult for user to pass directly if they want to) or as struct or as url? Or we don't need to take it at all and just use |
Codecov Report
@@ Coverage Diff @@
## main #2994 +/- ##
==========================================
- Coverage 30.25% 30.13% -0.12%
==========================================
Files 151 151
Lines 9473 9509 +36
==========================================
Hits 2866 2866
- Misses 6162 6198 +36
Partials 445 445
|
@mritunjaysharma394 Sorry for the delayed response. Are you interested in completing this PR? See https://github.com/sigstore/cosign/pull/3001/files for an example. You should already have an existing bundle on disk (for example, output by https://github.com/sigstore/cosign/blob/main/cmd/cosign/cli/sign/sign_blob.go#L123-L139), and simply call cosign/pkg/oci/mutate/options.go Line 81 in 731adeb
cosign attach signature like in PR 3001.
|
One thing to note - "bundle" is not the same as "rekor bundle" or "rekor response" confusingly. A bundle will include the signature, certificate, and rekor response. I would recommend we implement a way to output only the rekor response (see #3110), which is JSON marshalled https://github.com/sigstore/cosign/blob/main/pkg/cosign/bundle/rekor.go#L21, and attach that. |
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
This PR was closed because it has been stalled for 10 days with no activity. |
Summary
This PR adds support for Rekor bundles to be attached in containers. Currently
cosign attach
supports attaching signatures, but not Rekor bundles.Fixes #2904
/kind feature
Release Note
cosign attach
got added with the support of attaching Rekor Bundles to the container.Documentation
Yes, it requires an update. Updated docs here with
make docgen
for now.