feat: add offline bundle signature verification #457

asraa · 2023-01-27T20:03:05Z

Signed-off-by: Asra Ali asraa@google.com

Adds cryptographic bundle verification following the Sigstore client spec. I expect this will be replaced by a direct call to another library in the long term
Removed a test case that was no longer relevant. That test was checking our code against the Rekor server update that returned a new return response. Now all code tests the new server response anyway.
Pipes out trusted root info outside the verification calls
Updates cosign to v2

Signed-off-by: Asra Ali <asraa@google.com>

laurentsimon

Looks great!

verifiers/internal/gha/bundle.go

laurentsimon · 2023-01-27T23:00:59Z

verifiers/internal/gha/provenance_test.go

+	}{
+		{
+			name: "valid",
+			path: "./testdata/bundle/attestation.intoto.sigstore",


we'll add invalid / adversarial ones inn the future?

Just added one or two that were easily to manipulate without creating a test Rekor signer. Filed a follow-up issue
#461

We'll be generating a suite from sigstore-go pretty soon, so we can close this out either by borrowing cases there or moving the code to call a verification func there that's tested.

laurentsimon · 2023-01-27T23:03:04Z

verifiers/internal/gha/rekor.go

@@ -67,13 +67,17 @@ func verifyTlogEntryByUUID(ctx context.Context, rekorClient *client.Rekor, entry
 			return nil, errors.New("expected matching UUID")
 		}
 		// Validate the entry response.
-		return verifyTlogEntry(ctx, rekorClient, entry)
+		return verifyTlogEntry(ctx, entry, true)


iiuc, we'll perform an online verification by default to be consistent with our existing code. And we'll provide an option to turn it off in the futuer, e.g. rekor-log-offline?

Right now the existing code still validates the inclusion proof (which requires online).

I haven't configured augmenting the offline verification with an online proof yet, this code just checks verification of the offline material. I'll file an issue though to augment with an option to check inclusion online as well.

laurentsimon · 2023-01-27T23:14:06Z

qq: does the verification also verifies the SCT (ie the fulcio CA cert was recorded in the TLS CT log)?
Are the leaf cert also appended to the PKI CT logs? Or only CA certs are?

somewhat related: something we need to do is unable this for the TLS connection as well #458

Signed-off-by: Asra Ali <asraa@google.com>

asraa · 2023-02-01T17:36:42Z

qq: does the verification also verifies the SCT (ie the fulcio CA cert was recorded in the TLS CT log)?

Yes! By default now the check is on

Are the leaf cert also appended to the PKI CT logs? Or only CA certs are?

Hmm not sure I totally understand this. The leaf cert is in the CT log.

Signed-off-by: Asra Ali <asraa@google.com>

laurentsimon · 2023-02-01T18:20:51Z

qq: does the verification also verifies the SCT (ie the fulcio CA cert was recorded in the TLS CT log)?

Yes! By default now the check is on

+1

Are the leaf cert also appended to the PKI CT logs? Or only CA certs are?

Hmm not sure I totally understand this. The leaf cert is in the CT log.

Are CA certs (ie Fulcio cert) in CT log as well? Do we care?

How about rekor cert (which is a leaf cert)? Do we care?

Which are verified?

asraa · 2023-02-01T19:17:18Z

Are CA certs (ie Fulcio cert) in CT log as well? Do we care?

Oh, yes the CA cert is in the CT log. SCT verification being enforced now (regenerating test cases, this is why they are failing) checks the offline promise of that. We don't check them online, but neither does normal CT. So this part is now enforced/verified.

How about rekor cert (which is a leaf cert)? Do we care?

This I do not understand. Rekor signs with a public key that is retrieved from the TUF root and does not have a leaf cert.

laurentsimon · 2023-02-01T19:23:35Z

Are CA certs (ie Fulcio cert) in CT log as well? Do we care?

Oh, yes the CA cert is in the CT log. SCT verification being enforced now (regenerating test cases, this is why they are failing) checks the offline promise of that. We don't check them online, but neither does normal CT. So this part is now enforced/verified.

How about rekor cert (which is a leaf cert)? Do we care?

This I do not understand. Rekor signs with a public key that is retrieved from the TUF root and does not have a leaf cert.

OK. I thought maybe there was a cert issued as well. Ignore my comment then.

Signed-off-by: Asra Ali <asraa@google.com>

laurentsimon · 2023-02-02T00:12:23Z

verifiers/internal/gha/builder.go

@@ -17,6 +17,9 @@ var (
 	trustedBuilderRepository = "slsa-framework/slsa-github-generator"
 	e2eTestRepository        = "slsa-framework/example-package"
 	certOidcIssuer           = "https://token.actions.githubusercontent.com"
+	// This is used in cosign's CheckOpts for validating the certificate. We
+	// do specific builder verification after this.
+	certSubjectRegexp = "https://github.com/*"


that will also be necessary for npm provenance verification (the one without the trusted builder)

do we intend on verifying non-L3 provenance though?

but yes - it's just a nit that cosign v2 requires some validation on it so I had to add it even though we do a more detail check on it later.

laurentsimon · 2023-02-02T21:53:17Z

LGTM to merge.

Co-authored-by: Ian Lewis <ianlewis@google.com>

feat: add bundle signature verification

86d7670

Signed-off-by: Asra Ali <asraa@google.com>

asraa requested review from ianlewis and laurentsimon as code owners January 27, 2023 20:03

asraa changed the title ~~feat: add bundle signature verification~~ [DRAFT] feat: add bundle signature verification Jan 27, 2023

laurentsimon approved these changes Jan 27, 2023

View reviewed changes

update

c6d8249

Signed-off-by: Asra Ali <asraa@google.com>

asraa changed the title ~~[DRAFT] feat: add bundle signature verification~~ feat: add offline bundle signature verification Feb 1, 2023

lint

3808563

Signed-off-by: Asra Ali <asraa@google.com>

asraa added 3 commits February 1, 2023 15:19

regenerate testcases to include SCT

d86907d

Signed-off-by: Asra Ali <asraa@google.com>

fix golang

a66da57

Signed-off-by: Asra Ali <asraa@google.com>

Merge branch 'main' into bundle-sig-verification

ca125c6

asraa requested a review from laurentsimon February 2, 2023 00:05

laurentsimon reviewed Feb 2, 2023

View reviewed changes

asraa merged commit 362bd1a into slsa-framework:main Feb 3, 2023

ramonpetgrave64 pushed a commit to ramonpetgrave64/slsa-verifier that referenced this pull request Apr 18, 2024

Re-enable pre-submits (slsa-framework#457)

a1dc421

Co-authored-by: Ian Lewis <ianlewis@google.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add offline bundle signature verification #457

feat: add offline bundle signature verification #457

asraa commented Jan 27, 2023 •

edited

Loading

laurentsimon left a comment

laurentsimon Jan 27, 2023

asraa Feb 1, 2023

laurentsimon Jan 27, 2023

asraa Feb 1, 2023

laurentsimon commented Jan 27, 2023 •

edited

Loading

asraa commented Feb 1, 2023

laurentsimon commented Feb 1, 2023 •

edited

Loading

asraa commented Feb 1, 2023

laurentsimon commented Feb 1, 2023

laurentsimon Feb 2, 2023

asraa Feb 2, 2023

laurentsimon commented Feb 2, 2023

feat: add offline bundle signature verification #457

feat: add offline bundle signature verification #457

Conversation

asraa commented Jan 27, 2023 • edited Loading

laurentsimon left a comment

Choose a reason for hiding this comment

laurentsimon Jan 27, 2023

Choose a reason for hiding this comment

asraa Feb 1, 2023

Choose a reason for hiding this comment

laurentsimon Jan 27, 2023

Choose a reason for hiding this comment

asraa Feb 1, 2023

Choose a reason for hiding this comment

laurentsimon commented Jan 27, 2023 • edited Loading

asraa commented Feb 1, 2023

laurentsimon commented Feb 1, 2023 • edited Loading

asraa commented Feb 1, 2023

laurentsimon commented Feb 1, 2023

laurentsimon Feb 2, 2023

Choose a reason for hiding this comment

asraa Feb 2, 2023

Choose a reason for hiding this comment

laurentsimon commented Feb 2, 2023

asraa commented Jan 27, 2023 •

edited

Loading

laurentsimon commented Jan 27, 2023 •

edited

Loading

laurentsimon commented Feb 1, 2023 •

edited

Loading