Ensure we resolve tags once.

As I've been refactoring things, I noticed a handful of places where a single command potentially used a `name.Tag` multiple times for remote access.  There is an exceedingly small window here for a race to happen where the first tag access points to one image, and the subsequent access points to another.

I've already fixed a handful of these, and this change fixes one more (in `sget` IIRC), but also tries to add some defensive logic in a few places where we were already doing the right thing.

The defensive logic I added is to clobber `ref` with `digest` after resolving the reference:
```go
       digest, err := ociremote.ResolveDigest(ref, regOpts.ClientOpts(ctx)...)
       if err != nil {
               return err
       }
       // Overwrite "ref" with a digest to avoid a race where we use a tag
       // multiple times, and it potentially points to different things at
       // each access.
       ref = digest
```

This ensures that regardless of which reference is used below resolution, we always get what we resolved at this point.

There are some other superficial cleanups lumped in, which I noticed as I skimmed the code.

Signed-off-by: Matt Moore <[email protected]>

cmd/cosign/cli/attach/sbom.go

@@ -86,11 +86,8 @@ func SBOMCmd(ctx context.Context, regOpts options.RegistryOpts, sbomRef, sbomTyp
 	}
 
 	fmt.Fprintf(os.Stderr, "Uploading SBOM file for [%s] to [%s] with mediaType [%s].\n", ref.Name(), dstRef.Name(), sbomType)
-	if _, err := cremote.UploadFile(b, dstRef, types.MediaType(sbomType), types.OCIConfigJSON, remoteOpts...); err != nil {
-		return err
-	}
-
-	return nil
+	_, err = cremote.UploadFile(b, dstRef, types.MediaType(sbomType), types.OCIConfigJSON, remoteOpts...)
+	return err
 }
 
 func sbomBytes(sbomRef string) ([]byte, error) {

cmd/cosign/cli/attach/sig.go

@@ -69,11 +69,14 @@ func SignatureCmd(ctx context.Context, regOpts options.RegistryOpts, sigRef, pay
 	if err != nil {
 		return err
 	}
-
 	digest, err := ociremote.ResolveDigest(ref, regOpts.ClientOpts(ctx)...)
 	if err != nil {
 		return err
 	}
+	// Overwrite "ref" with a digest to avoid a race where we use a tag
+	// multiple times, and it potentially points to different things at
+	// each access.
+	ref = digest
 
 	var payload []byte
 	if payloadRef == "" {

cmd/cosign/cli/attest.go

@@ -154,6 +154,10 @@ func AttestCmd(ctx context.Context, ko sign.KeyOpts, regOpts options.RegistryOpt
 		return err
 	}
 	h, _ := v1.NewHash(digest.Identifier())
+	// Overwrite "ref" with a digest to avoid a race where we use a tag
+	// multiple times, and it potentially points to different things at
+	// each access.
+	ref = digest
 
 	sv, err := sign.SignerFromKeyOpts(ctx, certPath, ko)
 	if err != nil {

cmd/cosign/cli/generate/generate.go

@@ -73,11 +73,14 @@ func GenerateCmd(ctx context.Context, regOpts options.RegistryOpts, imageRef str
 	if err != nil {
 		return err
 	}
-
 	digest, err := ociremote.ResolveDigest(ref, regOpts.ClientOpts(ctx)...)
 	if err != nil {
 		return err
 	}
+	// Overwrite "ref" with a digest to avoid a race where we use a tag
+	// multiple times, and it potentially points to different things at
+	// each access.
+	ref = digest
 
 	json, err := (&payload.Cosign{Image: digest, Annotations: annotations}).MarshalJSON()
 	if err != nil {

pkg/cosign/kubernetes/webhook/validation.go

@@ -49,6 +49,7 @@ func valid(ctx context.Context, img string, keys []*ecdsa.PublicKey) bool {
 }
 
 func validSignatures(ctx context.Context, img string, key *ecdsa.PublicKey) ([]oci.Signature, error) {
+	// TODO(mattmoor): take the name.Reference as the param?
 	ref, err := name.ParseReference(img)
 	if err != nil {
 		return nil, err

pkg/sget/sget.go

@@ -52,12 +52,6 @@ func (sg *SecureGet) Do(ctx context.Context) error {
 		return err
 	}
 
-	if _, ok := ref.(name.Tag); ok {
-		if sg.KeyRef == "" && !options.EnableExperimental() {
-			return errors.New("public key must be specified when fetching by tag, you must fetch by digest or supply a public key")
-		}
-	}
-
 	opts := []remote.Option{
 		remote.WithAuthFromKeychain(authn.DefaultKeychain),
 		remote.WithContext(ctx),
@@ -67,6 +61,19 @@ func (sg *SecureGet) Do(ctx context.Context) error {
 		ClaimVerifier:      cosign.SimpleClaimVerifier,
 		RegistryClientOpts: []ociremote.Option{ociremote.WithRemoteOptions(opts...)},
 	}
+	if _, ok := ref.(name.Tag); ok {
+		if sg.KeyRef == "" && !options.EnableExperimental() {
+			return errors.New("public key must be specified when fetching by tag, you must fetch by digest or supply a public key")
+		}
+	}
+	// Overwrite "ref" with a digest to avoid a race where we verify the tag,
+	// and then access the file through the tag.  This has a race where we
+	// might download content that isn't what we verified.
+	ref, err = ociremote.ResolveDigest(ref, co.RegistryClientOpts...)
+	if err != nil {
+		return err
+	}
+
 	if sg.KeyRef != "" {
 		pub, err := sigs.LoadPublicKey(ctx, sg.KeyRef)
 		if err != nil {
@@ -104,8 +111,5 @@ func (sg *SecureGet) Do(ctx context.Context) error {
 	}
 
 	_, err = io.Copy(sg.Out, rc)
-	if err != nil {
-		return err
-	}
-	return nil
+	return err
 }