Fetch verification targets by TUF custom metadata #1423

haydentherapper · 2022-02-08T00:10:52Z

Summary

Continuing #1273, this adds support for fetching a list of TUF targets by custom metadata. This will be used for verification using expired/rotated targets. All targets will be persisted in the TUF target metadata. Only one will be marked as active while the others are marked as expired.

The custom metadata on a target looks like looks like:

      "fulcio.crt.pem": {
        "custom": {
          "sigstore": {
            "status": "Active",
            "usage": "Fulcio"
          }
        },
        "hashes": {
          "sha512": "f2e33a6dc208cee1f51d33bbea675ab0f0ced269617497985f9a0680689ee7073e4b6f8fef64c91bda590d30c129b3070dddce824c05bc165ac9802f0705cab6"
        },
        "length": 740
      },

Possible statuses are active or expired, and possible usages are fulcio, rekor, and ctfe. unknown is also supported for each as a default value.

To test this, I:

Used the current TUF targets to sign and verify. These TUF targets do not contain custom metadata, so this tested the fallback.
Initialized a TUF repository with custom metadata for each target (based on the script from the BYO TUF blog post). I changed the filenames too so no fallback would happen. Initialized Cosign with the custom TUF metadata. Successfully signed and verified.

Next steps:

Use the Status of the custom metadata to inform users when verifying with an expired target
Remove the OCI timestamp code
After TUF Root v3 is published and bundled with Cosign, we can remove the code that falls back to targets by filename.

Ticket Link

Fixes #1342

Ref #1273

Release Note

TUF targets can now be referenced by custom metadata instead of filename

Multiple verification keys can be used to verify Fulcio certificates, Rekor entries, or Fulcio CT log SCTs

haydentherapper · 2022-02-08T00:12:43Z

cc @asraa @dlorenc @bobcallaway

dlorenc · 2022-02-08T19:15:33Z

These test failures look related to the PR, any idea?

haydentherapper · 2022-02-08T22:28:37Z

Reran tests, the unit tests succeeded (I saw the flaky "timestamp.json" off by two bytes), but the KinD e2e tests are failing. Investigating now

haydentherapper · 2022-02-08T22:59:58Z

It appears the tests collided with an ongoing distroless release. It pulled the latest image before signing was complete. No more errors!

dlorenc · 2022-02-10T02:57:21Z

Cc @znewman01 could you take a look?

znewman01 · 2022-02-10T14:42:18Z

could you take a look?

Yup, sometime in the next couple hours.

pkg/cosign/tuf/client.go

znewman01

LGTM

znewman01 · 2022-02-10T23:05:04Z

cmd/cosign/cli/fulcio/fulcioroots/fulcioroots_test.go

+
+import "testing"
+
+func TestGetFulcioRoots(t *testing.T) {


Pardon my unfamiliarity with the codebase here.

I don't really understand this test -- we make a TUF client using tuf.NewFromEnv() which AFAICT looks in ~/.sigstore/root by default. Is there some test fixture that sets TUF up for us?

You're correct, this just ends up relying on the embedded root. I want to refactor the tests in a later PR to use the test utility newTufCustomRepo or newTufRepo so we can generate an instance of the TUF repository that we can easily manipulate to test different edge cases. For now, this test just aims to increase code coverage and exercise Get.

pkg/cosign/tuf/client.go

znewman01 · 2022-02-10T23:34:50Z

pkg/cosign/tuf/client_test.go

+		t.Error(err)
+	}
+	if l := dirLen(t, tufRoot); l == 0 {
+		t.Errorf("expected filesystem writes, got %d entries", l)


I'm a little puzzled by "expected filesystem writes". Can you clarify?

It's a check to make sure Initialize wrote something to the tufRoot directory.

pkg/cosign/tuf/client_test.go

cmd/cosign/cli/fulcio/fulcioverifier/fulcioverifier.go

znewman01 · 2022-02-10T23:56:34Z

cmd/cosign/cli/fulcio/fulcioverifier/fulcioverifier.go

 		if err != nil {
 			return err
 		}
-		// Is there a reason why this must be ECDSA key?


Is there? :)

The code makes assumptions about all of the TUF targets. We should clearly document these assumptions in cosign/root-signing. We could add support for other keys if there was a need, and it might come up with BYO-TUF. I'd encourage others to use ECDSA, primarily because you can have much smaller keys and get the same security strength.

pkg/cosign/tuf/client_test.go

haydentherapper

Thanks for the review!

haydentherapper · 2022-02-11T05:57:57Z

cmd/cosign/cli/fulcio/fulcioroots/fulcioroots_test.go

+
+import "testing"
+
+func TestGetFulcioRoots(t *testing.T) {


You're correct, this just ends up relying on the embedded root. I want to refactor the tests in a later PR to use the test utility newTufCustomRepo or newTufRepo so we can generate an instance of the TUF repository that we can easily manipulate to test different edge cases. For now, this test just aims to increase code coverage and exercise Get.

haydentherapper · 2022-02-11T06:01:31Z

cmd/cosign/cli/fulcio/fulcioverifier/fulcioverifier.go

 		if err != nil {
 			return err
 		}
-		// Is there a reason why this must be ECDSA key?


The code makes assumptions about all of the TUF targets. We should clearly document these assumptions in cosign/root-signing. We could add support for other keys if there was a need, and it might come up with BYO-TUF. I'd encourage others to use ECDSA, primarily because you can have much smaller keys and get the same security strength.

pkg/cosign/tuf/client_test.go

haydentherapper · 2022-02-11T06:11:18Z

pkg/cosign/tuf/client_test.go

+		t.Error(err)
+	}
+	if l := dirLen(t, tufRoot); l == 0 {
+		t.Errorf("expected filesystem writes, got %d entries", l)


It's a check to make sure Initialize wrote something to the tufRoot directory.

pkg/cosign/tuf/client_test.go

Signed-off-by: Hayden Blauzvern <[email protected]>

This uses GetTargetsByMeta to read the targets using the custom metadata, or fallback to the old targets by filename. Signed-off-by: Hayden Blauzvern <[email protected]>

Signed-off-by: Hayden Blauzvern <[email protected]>

* Add TUF client method for fetching by metadata Signed-off-by: Hayden Blauzvern <[email protected]> * Fetch verification targets by TUF custom metadata This uses GetTargetsByMeta to read the targets using the custom metadata, or fallback to the old targets by filename. Signed-off-by: Hayden Blauzvern <[email protected]> * Resolve PR comments, linter, and update tests Signed-off-by: Hayden Blauzvern <[email protected]>

haydentherapper force-pushed the verify-old branch from cb78b01 to aad1f53 Compare February 8, 2022 00:38

asraa mentioned this pull request Feb 8, 2022

Use right fulcio certificate when verifying bundles sigstore/sigstore-rs#32

Closed

haydentherapper force-pushed the verify-old branch from 9d29d19 to 10987bd Compare February 8, 2022 21:52

haydentherapper force-pushed the verify-old branch from bd2fb54 to e8d3e91 Compare February 9, 2022 00:44

asraa suggested changes Feb 10, 2022

View reviewed changes

pkg/cosign/tuf/client.go Show resolved Hide resolved

pkg/cosign/tuf/client.go Outdated Show resolved Hide resolved

asraa suggested changes Feb 10, 2022

View reviewed changes

pkg/cosign/tuf/client.go Outdated Show resolved Hide resolved

pkg/cosign/tuf/client.go Show resolved Hide resolved

haydentherapper force-pushed the verify-old branch from 1c0bdc9 to a911047 Compare February 10, 2022 23:28

haydentherapper requested a review from asraa February 10, 2022 23:43

znewman01 approved these changes Feb 11, 2022

View reviewed changes

haydentherapper commented Feb 11, 2022

View reviewed changes

haydentherapper added 3 commits February 11, 2022 06:14

Add TUF client method for fetching by metadata

55c9578

Signed-off-by: Hayden Blauzvern <[email protected]>

Fetch verification targets by TUF custom metadata

d93726b

This uses GetTargetsByMeta to read the targets using the custom metadata, or fallback to the old targets by filename. Signed-off-by: Hayden Blauzvern <[email protected]>

Resolve PR comments, linter, and update tests

4a8f560

Signed-off-by: Hayden Blauzvern <[email protected]>

haydentherapper force-pushed the verify-old branch from 2b118ec to 4a8f560 Compare February 11, 2022 06:16

asraa approved these changes Feb 11, 2022

View reviewed changes

dlorenc merged commit 0e76059 into sigstore:main Feb 11, 2022

github-actions bot added this to the v1.6.0 milestone Feb 11, 2022

dlorenc mentioned this pull request Feb 21, 2022

TUF: Using custom meta for target identification #1291

Closed

asraa mentioned this pull request Jul 14, 2022

publish for asraa sigstore/root-signing#323

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fetch verification targets by TUF custom metadata #1423

Fetch verification targets by TUF custom metadata #1423

haydentherapper commented Feb 8, 2022

haydentherapper commented Feb 8, 2022

dlorenc commented Feb 8, 2022

haydentherapper commented Feb 8, 2022

haydentherapper commented Feb 8, 2022

dlorenc commented Feb 10, 2022

znewman01 commented Feb 10, 2022

znewman01 left a comment

znewman01 Feb 10, 2022

haydentherapper Feb 11, 2022

znewman01 Feb 10, 2022

haydentherapper Feb 11, 2022

znewman01 Feb 10, 2022

haydentherapper Feb 11, 2022

haydentherapper left a comment

haydentherapper Feb 11, 2022

haydentherapper Feb 11, 2022

haydentherapper Feb 11, 2022

Fetch verification targets by TUF custom metadata #1423

Fetch verification targets by TUF custom metadata #1423

Conversation

haydentherapper commented Feb 8, 2022

Summary

Ticket Link

Release Note

haydentherapper commented Feb 8, 2022

dlorenc commented Feb 8, 2022

haydentherapper commented Feb 8, 2022

haydentherapper commented Feb 8, 2022

dlorenc commented Feb 10, 2022

znewman01 commented Feb 10, 2022

znewman01 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

haydentherapper left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment