Skip to content

Refactor sign command #2388

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.

Sign up for GitHub

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

Merged
priyawadhwa merged 1 commit into from
Oct 26, 2022
Merged

Refactor sign command #2388

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions cmd/cosign/cli/sign.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,7 @@ race conditions or (worse) malicious tampering.
OIDCProvider: o.OIDC.Provider,
SkipConfirmation: o.SkipConfirmation,
}
annotationsMap, err := o.AnnotationsMap()
if err != nil {
return err
}
if err := sign.SignCmd(ro, ko, o.Registry, annotationsMap.Annotations, args, o.Cert, o.CertChain, o.Upload,
o.OutputSignature, o.OutputCertificate, o.PayloadPath, o.Force, o.Recursive, o.Attachment, o.NoTlogUpload); err != nil {
if err := sign.SignCmd(ro, ko, *o, args); err != nil {
if o.Attachment == "" {
return fmt.Errorf("signing %v: %w", args, err)
}
Expand Down
31 changes: 17 additions & 14 deletions cmd/cosign/cli/sign/sign.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,7 @@ func ParseOCIReference(refStr string, out io.Writer, opts ...name.Option) (name.
}

// nolint
func SignCmd(ro *options.RootOptions, ko options.KeyOpts, regOpts options.RegistryOptions, annotations map[string]interface{},
imgs []string, certPath string, certChainPath string, upload bool, outputSignature, outputCertificate string,
payloadPath string, force bool, recursive bool, attachment string, noTlogUpload bool) error {
func SignCmd(ro *options.RootOptions, ko options.KeyOpts, signOpts options.SignOptions, imgs []string) error {
if options.EnableExperimental() {
if options.NOf(ko.KeyRef, ko.Sk) > 1 {
return &options.KeyParseError{}
Expand All @@ -139,48 +137,53 @@ func SignCmd(ro *options.RootOptions, ko options.KeyOpts, regOpts options.Regist
ctx, cancel := context.WithTimeout(context.Background(), ro.Timeout)
defer cancel()

sv, err := SignerFromKeyOpts(ctx, certPath, certChainPath, ko)
sv, err := SignerFromKeyOpts(ctx, signOpts.Cert, signOpts.CertChain, ko)
if err != nil {
return fmt.Errorf("getting signer: %w", err)
}
defer sv.Close()
dd := cremote.NewDupeDetector(sv)

var staticPayload []byte
if payloadPath != "" {
fmt.Fprintln(os.Stderr, "Using payload from:", payloadPath)
staticPayload, err = os.ReadFile(filepath.Clean(payloadPath))
if signOpts.PayloadPath != "" {
fmt.Fprintln(os.Stderr, "Using payload from:", signOpts.PayloadPath)
staticPayload, err = os.ReadFile(filepath.Clean(signOpts.PayloadPath))
if err != nil {
return fmt.Errorf("payload from file: %w", err)
}
}

// Set up an ErrDone consideration to return along "success" paths
var ErrDone error
if !recursive {
if !signOpts.Recursive {
ErrDone = mutate.ErrSkipChildren
}

regOpts := signOpts.Registry
opts, err := regOpts.ClientOpts(ctx)
if err != nil {
return fmt.Errorf("constructing client options: %w", err)
}
am, err := signOpts.AnnotationsMap()
if err != nil {
return fmt.Errorf("getting annotations: %w", err)
}
annotations := am.Annotations
for _, inputImg := range imgs {
ref, err := ParseOCIReference(inputImg, os.Stderr, regOpts.NameOptions()...)
if err != nil {
return err
}
ref, err = GetAttachedImageRef(ref, attachment, opts...)
ref, err = GetAttachedImageRef(ref, signOpts.Attachment, opts...)
if err != nil {
return fmt.Errorf("unable to resolve attachment %s for image %s", attachment, inputImg)
return fmt.Errorf("unable to resolve attachment %s for image %s", signOpts.Attachment, inputImg)
}

if digest, ok := ref.(name.Digest); ok && !recursive {
if digest, ok := ref.(name.Digest); ok && !signOpts.Recursive {
se, err := ociremote.SignedEntity(ref, opts...)
if err != nil {
return fmt.Errorf("accessing image: %w", err)
}
err = signDigest(ctx, digest, staticPayload, ko, regOpts, annotations, upload, outputSignature, outputCertificate, force, recursive, noTlogUpload, dd, sv, se)
err = signDigest(ctx, digest, staticPayload, ko, regOpts, annotations, signOpts.Upload, signOpts.OutputSignature, signOpts.OutputCertificate, signOpts.Force, signOpts.Recursive, signOpts.NoTlogUpload, dd, sv, se)
if err != nil {
return fmt.Errorf("signing digest: %w", err)
}
Expand All @@ -200,7 +203,7 @@ func SignCmd(ro *options.RootOptions, ko options.KeyOpts, regOpts options.Regist
}
digest := ref.Context().Digest(d.String())

err = signDigest(ctx, digest, staticPayload, ko, regOpts, annotations, upload, outputSignature, outputCertificate, force, recursive, noTlogUpload, dd, sv, se)
err = signDigest(ctx, digest, staticPayload, ko, regOpts, annotations, signOpts.Upload, signOpts.OutputSignature, signOpts.OutputCertificate, signOpts.Force, signOpts.Recursive, signOpts.NoTlogUpload, dd, sv, se)
if err != nil {
return fmt.Errorf("signing digest: %w", err)
}
Expand Down
3 changes: 2 additions & 1 deletion cmd/cosign/cli/sign/sign_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ func TestSignCmdLocalKeyAndSk(t *testing.T) {
Sk: true,
},
} {
err := SignCmd(ro, ko, options.RegistryOptions{}, nil, nil, "", "", false, "", "", "", false, false, "", false)
so := options.SignOptions{}
err := SignCmd(ro, ko, so, nil)
if (errors.Is(err, &options.KeyParseError{}) == false) {
t.Fatal("expected KeyParseError")
}
Expand Down
87 changes: 66 additions & 21 deletions test/e2e_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,10 @@ func TestSignVerify(t *testing.T) {

// Now sign the image
ko := options.KeyOpts{KeyRef: privKeyPath, PassFunc: passFunc}
must(sign.SignCmd(ro, ko, options.RegistryOptions{}, nil, []string{imgName}, "", "", true, "", "", "", false, false, "", false), t)
so := options.SignOptions{
Upload: true,
}
must(sign.SignCmd(ro, ko, so, []string{imgName}), t)

// Now verify and download should work!
must(verify(pubKeyPath, imgName, true, nil, ""), t)
Expand All @@ -141,9 +144,11 @@ func TestSignVerify(t *testing.T) {
// Look for a specific annotation
mustErr(verify(pubKeyPath, imgName, true, map[string]interface{}{"foo": "bar"}, ""), t)

so.AnnotationOptions = options.AnnotationOptions{
Annotations: []string{"foo=bar"},
}
// Sign the image with an annotation
annotations := map[string]interface{}{"foo": "bar"}
must(sign.SignCmd(ro, ko, options.RegistryOptions{}, annotations, []string{imgName}, "", "", true, "", "", "", false, false, "", false), t)
must(sign.SignCmd(ro, ko, so, []string{imgName}), t)

// It should match this time.
must(verify(pubKeyPath, imgName, true, map[string]interface{}{"foo": "bar"}, ""), t)
Expand All @@ -167,7 +172,10 @@ func TestSignVerifyClean(t *testing.T) {

// Now sign the image
ko := options.KeyOpts{KeyRef: privKeyPath, PassFunc: passFunc}
must(sign.SignCmd(ro, ko, options.RegistryOptions{}, nil, []string{imgName}, "", "", true, "", "", "", false, false, "", false), t)
so := options.SignOptions{
Upload: true,
}
must(sign.SignCmd(ro, ko, so, []string{imgName}), t)

// Now verify and download should work!
must(verify(pubKeyPath, imgName, true, nil, ""), t)
Expand Down Expand Up @@ -196,7 +204,10 @@ func TestImportSignVerifyClean(t *testing.T) {

// Now sign the image
ko := options.KeyOpts{KeyRef: privKeyPath, PassFunc: passFunc}
must(sign.SignCmd(ro, ko, options.RegistryOptions{}, nil, []string{imgName}, "", "", true, "", "", "", false, false, "", false), t)
so := options.SignOptions{
Upload: true,
}
must(sign.SignCmd(ro, ko, so, []string{imgName}), t)

// Now verify and download should work!
must(verify(pubKeyPath, imgName, true, nil, ""), t)
Expand Down Expand Up @@ -453,9 +464,12 @@ func TestRekorBundle(t *testing.T) {
PassFunc: passFunc,
RekorURL: rekorURL,
}
so := options.SignOptions{
Upload: true,
}

// Sign the image
must(sign.SignCmd(ro, ko, options.RegistryOptions{}, nil, []string{imgName}, "", "", true, "", "", "", false, false, "", false), t)
must(sign.SignCmd(ro, ko, so, []string{imgName}), t)
// Make sure verify works
must(verify(pubKeyPath, imgName, true, nil, ""), t)

Expand Down Expand Up @@ -485,14 +499,17 @@ func TestDuplicateSign(t *testing.T) {

// Now sign the image
ko := options.KeyOpts{KeyRef: privKeyPath, PassFunc: passFunc}
must(sign.SignCmd(ro, ko, options.RegistryOptions{}, nil, []string{imgName}, "", "", true, "", "", "", false, false, "", false), t)
so := options.SignOptions{
Upload: true,
}
must(sign.SignCmd(ro, ko, so, []string{imgName}), t)

// Now verify and download should work!
must(verify(pubKeyPath, imgName, true, nil, ""), t)
must(download.SignatureCmd(ctx, options.RegistryOptions{}, imgName), t)

// Signing again should work just fine...
must(sign.SignCmd(ro, ko, options.RegistryOptions{}, nil, []string{imgName}, "", "", true, "", "", "", false, false, "", false), t)
must(sign.SignCmd(ro, ko, so, []string{imgName}), t)

se, err := ociremote.SignedEntity(ref, ociremote.WithRemoteOptions(registryClientOpts(ctx)...))
must(err, t)
Expand Down Expand Up @@ -588,14 +605,17 @@ func TestMultipleSignatures(t *testing.T) {

// Now sign the image with one key
ko := options.KeyOpts{KeyRef: priv1, PassFunc: passFunc}
must(sign.SignCmd(ro, ko, options.RegistryOptions{}, nil, []string{imgName}, "", "", true, "", "", "", false, false, "", false), t)
so := options.SignOptions{
Upload: true,
}
must(sign.SignCmd(ro, ko, so, []string{imgName}), t)
// Now verify should work with that one, but not the other
must(verify(pub1, imgName, true, nil, ""), t)
mustErr(verify(pub2, imgName, true, nil, ""), t)

// Now sign with the other key too
ko.KeyRef = priv2
must(sign.SignCmd(ro, ko, options.RegistryOptions{}, nil, []string{imgName}, "", "", true, "", "", "", false, false, "", false), t)
must(sign.SignCmd(ro, ko, so, []string{imgName}), t)

// Now verify should work with both
must(verify(pub1, imgName, true, nil, ""), t)
Expand Down Expand Up @@ -977,7 +997,10 @@ func TestSaveLoad(t *testing.T) {
ctx := context.Background()
// Now sign the image and verify it
ko := options.KeyOpts{KeyRef: privKeyPath, PassFunc: passFunc}
must(sign.SignCmd(ro, ko, options.RegistryOptions{}, nil, []string{imgName}, "", "", true, "", "", "", false, false, "", false), t)
so := options.SignOptions{
Upload: true,
}
must(sign.SignCmd(ro, ko, so, []string{imgName}), t)
must(verify(pubKeyPath, imgName, true, nil, ""), t)

// save the image to a temp dir
Expand Down Expand Up @@ -1010,7 +1033,10 @@ func TestSaveLoadAttestation(t *testing.T) {
ctx := context.Background()
// Now sign the image and verify it
ko := options.KeyOpts{KeyRef: privKeyPath, PassFunc: passFunc}
must(sign.SignCmd(ro, ko, options.RegistryOptions{}, nil, []string{imgName}, "", "", true, "", "", "", false, false, "", false), t)
so := options.SignOptions{
Upload: true,
}
must(sign.SignCmd(ro, ko, so, []string{imgName}), t)
must(verify(pubKeyPath, imgName, true, nil, ""), t)

// now, append an attestation to the image
Expand Down Expand Up @@ -1104,7 +1130,11 @@ func TestAttachSBOM(t *testing.T) {

// Now sign the sbom with one key
ko1 := options.KeyOpts{KeyRef: privKeyPath1, PassFunc: passFunc}
must(sign.SignCmd(ro, ko1, options.RegistryOptions{}, nil, []string{imgName}, "", "", true, "", "", "", false, false, "sbom", false), t)
so := options.SignOptions{
Upload: true,
Attachment: "sbom",
}
must(sign.SignCmd(ro, ko1, so, []string{imgName}), t)

// Now verify should work with that one, but not the other
must(verify(pubKeyPath1, imgName, true, nil, "sbom"), t)
Expand Down Expand Up @@ -1141,7 +1171,10 @@ func TestTlog(t *testing.T) {
PassFunc: passFunc,
RekorURL: rekorURL,
}
must(sign.SignCmd(ro, ko, options.RegistryOptions{}, nil, []string{imgName}, "", "", true, "", "", "", false, false, "", false), t)
so := options.SignOptions{
Upload: true,
}
must(sign.SignCmd(ro, ko, so, []string{imgName}), t)

// Now verify should work!
must(verify(pubKeyPath, imgName, true, nil, ""), t)
Expand All @@ -1153,7 +1186,7 @@ func TestTlog(t *testing.T) {
mustErr(verify(pubKeyPath, imgName, true, nil, ""), t)

// Sign again with the tlog env var on
must(sign.SignCmd(ro, ko, options.RegistryOptions{}, nil, []string{imgName}, "", "", true, "", "", "", false, false, "", false), t)
must(sign.SignCmd(ro, ko, so, []string{imgName}), t)
// And now verify works!
must(verify(pubKeyPath, imgName, true, nil, ""), t)
}
Expand All @@ -1179,7 +1212,10 @@ func TestNoTlog(t *testing.T) {
PassFunc: passFunc,
RekorURL: rekorURL,
}
must(sign.SignCmd(ro, ko, options.RegistryOptions{}, nil, []string{imgName}, "", "", true, "", "", "", false, false, "", false), t)
so := options.SignOptions{
Upload: true,
}
must(sign.SignCmd(ro, ko, so, []string{imgName}), t)

// Now verify should work!
must(verify(pubKeyPath, imgName, true, nil, ""), t)
Expand All @@ -1191,7 +1227,10 @@ func TestNoTlog(t *testing.T) {
mustErr(verify(pubKeyPath, imgName, true, nil, ""), t)

// Sign again with the tlog env var on with option to not upload tlog
must(sign.SignCmd(ro, ko, options.RegistryOptions{}, nil, []string{imgName}, "", "", true, "", "", "", false, false, "", true), t)
so = options.SignOptions{
NoTlogUpload: true,
}
must(sign.SignCmd(ro, ko, so, []string{imgName}), t)
// And verify it still fails.
mustErr(verify(pubKeyPath, imgName, true, nil, ""), t)
}
Expand Down Expand Up @@ -1353,9 +1392,11 @@ func TestInvalidBundle(t *testing.T) {
defer setenv(t, env.VariableExperimental.String(), "1")()
remoteOpts := ociremote.WithRemoteOptions(registryClientOpts(ctx)...)
ko := options.KeyOpts{KeyRef: privKeyPath, PassFunc: passFunc, RekorURL: rekorURL}
regOpts := options.RegistryOptions{}

must(sign.SignCmd(ro, ko, regOpts, nil, []string{img1}, "", "", true, "", "", "", true, false, "", true), t)
so := options.SignOptions{
Upload: true,
Force: true,
}
must(sign.SignCmd(ro, ko, so, []string{img1}), t)
// verify image1
must(verify(pubKeyPath, img1, true, nil, ""), t)
// extract the bundle from image1
Expand All @@ -1380,7 +1421,11 @@ func TestInvalidBundle(t *testing.T) {
img2 := path.Join(regName, "unrelated")
imgRef2, _, cleanup := mkimage(t, img2)
defer cleanup()
must(sign.SignCmd(ro, ko, regOpts, nil, []string{img2}, "", "", true, "", "", "", false, false, "", true), t)
so = options.SignOptions{
Upload: true,
NoTlogUpload: true,
}
must(sign.SignCmd(ro, ko, so, []string{img2}), t)
must(verify(pubKeyPath, img2, true, nil, ""), t)

si2, err := ociremote.SignedEntity(imgRef2, remoteOpts)
Expand Down