Bug: Investigate issue in TUF root upgrades

[asraa]

Description

Investigate buggy behavior with consistent snapshots in the sigstore TUF root upgrade path:

Verifying with an upgraded TUF root that enables consistent snapshots causes a bug in a fresh environment. Explicitly calling cosign initialize and then verifying avoids the bug.

Reproduced by replacing the default remote store to the brokenv3 (consistent snapshot) branch of root-signing:

asraa@asraa1:~/git/cosign$ go build -o cosign ./cmd/cosign
asraa@asraa1:~/git/cosign$ COSIGN_EXPERIMENTAL=1 ./cosign verify gcr.io/distroless/base:debug 
panic: error getting targets

goroutine 1 [running]:
github.com/sigstore/cosign/cmd/cosign/cli/fulcio/fulcioroots.Get.func1()
	/home/asraa/git/cosign/cmd/cosign/cli/fulcio/fulcioroots/fulcioroots.go:69 +0x74
sync.(*Once).doSlow(0xc00084ade0?, 0x1d?)
	/usr/local/go/src/sync/once.go:68 +0xc2
sync.(*Once).Do(...)
	/usr/local/go/src/sync/once.go:59
github.com/sigstore/cosign/cmd/cosign/cli/fulcio/fulcioroots.Get()
	/home/asraa/git/cosign/cmd/cosign/cli/fulcio/fulcioroots/fulcioroots.go:65 +0x31
github.com/sigstore/cosign/cmd/cosign/cli/fulcio.GetRoots(...)
	/home/asraa/git/cosign/cmd/cosign/cli/fulcio/fulcio.go:174
github.com/sigstore/cosign/cmd/cosign/cli/verify.(*VerifyCommand).Exec(0xc0005b9c30, {0x2e23cd8, 0xc000054108}, {0xc000876a90, 0x1, 0x6bc9a5?})
	/home/asraa/git/cosign/cmd/cosign/cli/verify/verify.go:113 +0x3f5
github.com/sigstore/cosign/cmd/cosign/cli.Verify.func1(0xc0007d5b80, {0xc000876a90, 0x1, 0x1})
	/home/asraa/git/cosign/cmd/cosign/cli/verify.go:115 +0x265
github.com/spf13/cobra.(*Command).execute(0xc0007d5b80, {0xc000876a70, 0x1, 0x1})
	/home/asraa/go/pkg/mod/github.com/spf13/cobra@v1.4.0/command.go:856 +0x67c
github.com/spf13/cobra.(*Command).ExecuteC(0xc000908500)
	/home/asraa/go/pkg/mod/github.com/spf13/cobra@v1.4.0/command.go:974 +0x3b4
github.com/spf13/cobra.(*Command).Execute(...)
	/home/asraa/go/pkg/mod/github.com/spf13/cobra@v1.4.0/command.go:902
main.main()
	/home/asraa/git/cosign/cmd/cosign/main.go:51 +0x4c
asraa@asraa1:~/git/cosign$ ls $HOME/.sigstore
root
asraa@asraa1:~/git/cosign$ rm -r $HOME/.sigstore
asraa@asraa1:~/git/cosign$ ./cosign initialize
Root status: 
 {
	"local": "/home/asraa/.sigstore/root",
	"remote": "test-sigstore-root",
	"metadata": {
		"root.json": {
			"version": 3,
			"len": 5285,
			"expiration": "10 Nov 22 02:19 UTC",
			"error": ""
		},
		"snapshot.json": {
			"version": 29,
			"len": 1975,
			"expiration": "31 May 22 17:41 UTC",
			"error": ""
		},
		"targets.json": {
			"version": 3,
			"len": 6088,
			"expiration": "10 Nov 22 02:19 UTC",
			"error": ""
		},
		"timestamp.json": {
			"version": 29,
			"len": 717,
			"expiration": "24 May 22 17:41 UTC",
			"error": ""
		}
	},
	"targets": [
		"fulcio_v1.crt.pem",
		"rekor.0.pub",
		"rekor.pub",
		"artifact.pub",
		"ctfe.pub",
		"fulcio.crt.pem",
		"fulcio_intermediate_v1.crt.pem"
	]
}
asraa@asraa1:~/git/cosign$ COSIGN_EXPERIMENTAL=1 ./cosign verify gcr.io/distroless/base:debug 

Verification for gcr.io/distroless/base:debug --
[success]

cc @haydentherapper

Questions:

• Where is GetTargets pulling targets from in each case?
• What target names are being pulled in each case?

[haydentherapper]

When I did a cursory investigation, I added a lot more logging to print out the target names - Probably worth adding that in to Cosign.

[asraa]

Notes:

It's not a consistent snapshot problem. It's a problem that we introduced a new target fulcio_intermediate_v1.crt.pem. In the embedded workflow, we are still trying to retrieve a target from the embedded file system, and it doesn't exist! It doesn't pick up from the disk cache, even though it writes the new target to the disk cache

cosign/pkg/cosign/tuf/client.go

Line 596 in 03e66aa

e.setImpl = &diskCache{base: cachedTargetsDir(rootCacheDir())}

I think I need to clean up the targetImpl/setImpl, not sure I understand how the functionality is being separated

cosign/pkg/cosign/tuf/client.go

Lines 592 to 597 in 03e66aa

	e := &embedded{}
	if noCache() {
	e.setImpl = &memoryCache{}
	} else {
	e.setImpl = &diskCache{base: cachedTargetsDir(rootCacheDir())}
	}

The embedded impl has a disk cache for the setter, but gets targets from the embedded repository, not the disk cache. cc @dlorenc

OTOH when we do an initialize, then we have a targetImpl on diskcache with a getter that actually gets the updated targets in the disk cache

cosign/pkg/cosign/tuf/client.go

Line 567 in 03e66aa

fp := filepath.Join(f.base, p)

So this problem is just a problem in our cached/updated target getters.

[asraa]

One line fix is to switch to a diskCache if there's a need for an update in the embedded start workflow

[haydentherapper]

Is the only purpose of the embedded cache to start the workflow then? What happens with a purely in-memory initialize+update?

[asraa]

In-memory initialize is kinda a no-op -- in-memory only makes sense during a verification.

But yes, embedded is only for the start root -- if there's any updates it's a newFileImpl(), which is either a disk cache, or in-memory impl if you don't have caching enabled.

So my fix I think is to make it clear that embedded is a "starting state", and it eventually moves to a file impl on an update.

[asraa]

On top of this memoryCache doesn't have a Getter. So I'm just going to clean up the targetImpl and setImpls