From bf8713fdcd2256c9501962a2b8257c87c9667d43 Mon Sep 17 00:00:00 2001 From: Billy Lynch Date: Mon, 2 May 2022 14:32:39 -0400 Subject: [PATCH] Validate tlog entry when verifying signature via public key. Previously, when signatures were validated via public key, we only checked for the existence of the tlog, but didn't actually validate the entry. This refactors the code to ensure both cert + public paths go through the same tlog entry validation. Signed-off-by: Billy Lynch --- .../cosign/rekor/mock/mock_rekor_client.go | 97 +++++++++++++++++++ .../pkg/cosign/rekor/mock_rekor_client.go | 64 ------------ internal/pkg/cosign/rekor/signer_test.go | 3 +- pkg/cosign/verify.go | 33 +++---- pkg/cosign/verify_test.go | 37 +++++++ 5 files changed, 151 insertions(+), 83 deletions(-) create mode 100644 internal/pkg/cosign/rekor/mock/mock_rekor_client.go delete mode 100644 internal/pkg/cosign/rekor/mock_rekor_client.go diff --git a/internal/pkg/cosign/rekor/mock/mock_rekor_client.go b/internal/pkg/cosign/rekor/mock/mock_rekor_client.go new file mode 100644 index 00000000000..f29c3b5eec0 --- /dev/null +++ b/internal/pkg/cosign/rekor/mock/mock_rekor_client.go @@ -0,0 +1,97 @@ +// Copyright 2022 The Sigstore Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package mock + +import ( + "encoding/base64" + "encoding/hex" + + "github.com/go-openapi/runtime" + "github.com/google/trillian/merkle/rfc6962" + "github.com/sigstore/rekor/pkg/generated/client/entries" + "github.com/sigstore/rekor/pkg/generated/models" +) + +var ( + lea = models.LogEntryAnon{ + Attestation: &models.LogEntryAnonAttestation{}, + Body: base64.StdEncoding.EncodeToString([]byte("asdf")), + IntegratedTime: new(int64), + LogID: new(string), + LogIndex: new(int64), + Verification: &models.LogEntryAnonVerification{ + InclusionProof: &models.InclusionProof{ + RootHash: new(string), + TreeSize: new(int64), + LogIndex: new(int64), + }, + }, + } + data = models.LogEntry{ + uuid(lea): lea, + } +) + +// uuid generates the UUID for the given LogEntry. +// This is effectively a reimplementation of +// pkg/cosign/tlog.go -> verifyUUID / ComputeLeafHash, but separated +// to avoid a circular dependency. +// TODO?: Perhaps we should refactor the tlog libraries into a separate +// package? +func uuid(e models.LogEntryAnon) string { + entryBytes, err := base64.StdEncoding.DecodeString(e.Body.(string)) + if err != nil { + panic(err) + } + return hex.EncodeToString(rfc6962.DefaultHasher.HashLeaf(entryBytes)) +} + +// EntriesClient is a client that implements entries.ClientService for Rekor +// To use: +// var mClient client.Rekor +// mClient.entries = &EntriesClient{} +type EntriesClient struct { + Entries models.LogEntry +} + +func (m *EntriesClient) CreateLogEntry(params *entries.CreateLogEntryParams, opts ...entries.ClientOption) (*entries.CreateLogEntryCreated, error) { + return &entries.CreateLogEntryCreated{ + ETag: "", + Location: "", + Payload: data, + }, nil +} + +func (m *EntriesClient) GetLogEntryByIndex(params *entries.GetLogEntryByIndexParams, opts ...entries.ClientOption) (*entries.GetLogEntryByIndexOK, error) { + return &entries.GetLogEntryByIndexOK{ + Payload: data, + }, nil +} + +func (m *EntriesClient) GetLogEntryByUUID(params *entries.GetLogEntryByUUIDParams, opts ...entries.ClientOption) (*entries.GetLogEntryByUUIDOK, error) { + return &entries.GetLogEntryByUUIDOK{ + Payload: data, + }, nil +} + +func (m *EntriesClient) SearchLogQuery(params *entries.SearchLogQueryParams, opts ...entries.ClientOption) (*entries.SearchLogQueryOK, error) { + return &entries.SearchLogQueryOK{ + Payload: []models.LogEntry{data}, + }, nil +} + +// TODO: Implement mock +func (m *EntriesClient) SetTransport(transport runtime.ClientTransport) { +} diff --git a/internal/pkg/cosign/rekor/mock_rekor_client.go b/internal/pkg/cosign/rekor/mock_rekor_client.go deleted file mode 100644 index 23facc2fb94..00000000000 --- a/internal/pkg/cosign/rekor/mock_rekor_client.go +++ /dev/null @@ -1,64 +0,0 @@ -// Copyright 2022 The Sigstore Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package rekor - -import ( - "github.com/go-openapi/runtime" - "github.com/sigstore/rekor/pkg/generated/client/entries" - "github.com/sigstore/rekor/pkg/generated/models" -) - -// Client that implements entries.ClientService for Rekor -// To use: -// var mClient client.Rekor -// mClient.entries = &MockEntriesClient{} -type MockEntriesClient struct { -} - -func (m *MockEntriesClient) CreateLogEntry(params *entries.CreateLogEntryParams, opts ...entries.ClientOption) (*entries.CreateLogEntryCreated, error) { - return &entries.CreateLogEntryCreated{ - ETag: "", - Location: "", - Payload: map[string]models.LogEntryAnon{ - "sdf": { - Attestation: &models.LogEntryAnonAttestation{}, - Body: nil, - IntegratedTime: new(int64), - LogID: new(string), - LogIndex: new(int64), - Verification: &models.LogEntryAnonVerification{}, - }, - }, - }, nil -} - -// TODO: Implement mock -func (m *MockEntriesClient) GetLogEntryByIndex(params *entries.GetLogEntryByIndexParams, opts ...entries.ClientOption) (*entries.GetLogEntryByIndexOK, error) { - return nil, nil -} - -// TODO: Implement mock -func (m *MockEntriesClient) GetLogEntryByUUID(params *entries.GetLogEntryByUUIDParams, opts ...entries.ClientOption) (*entries.GetLogEntryByUUIDOK, error) { - return nil, nil -} - -// TODO: Implement mock -func (m *MockEntriesClient) SearchLogQuery(params *entries.SearchLogQueryParams, opts ...entries.ClientOption) (*entries.SearchLogQueryOK, error) { - return nil, nil -} - -// TODO: Implement mock -func (m *MockEntriesClient) SetTransport(transport runtime.ClientTransport) { -} diff --git a/internal/pkg/cosign/rekor/signer_test.go b/internal/pkg/cosign/rekor/signer_test.go index 8c3e58e1db0..cb705718605 100644 --- a/internal/pkg/cosign/rekor/signer_test.go +++ b/internal/pkg/cosign/rekor/signer_test.go @@ -23,6 +23,7 @@ import ( "testing" "github.com/sigstore/cosign/internal/pkg/cosign/payload" + "github.com/sigstore/cosign/internal/pkg/cosign/rekor/mock" "github.com/sigstore/cosign/pkg/cosign" "github.com/sigstore/rekor/pkg/generated/client" "github.com/sigstore/sigstore/pkg/signature" @@ -47,7 +48,7 @@ func TestSigner(t *testing.T) { // Mock out Rekor client var mClient client.Rekor - mClient.Entries = &MockEntriesClient{} + mClient.Entries = &mock.EntriesClient{} testSigner := NewSigner(payloadSigner, &mClient) diff --git a/pkg/cosign/verify.go b/pkg/cosign/verify.go index e2e9d98a368..e3b63413dde 100644 --- a/pkg/cosign/verify.go +++ b/pkg/cosign/verify.go @@ -303,15 +303,7 @@ func tlogValidatePublicKey(ctx context.Context, rekorClient *client.Rekor, pub c if err != nil { return err } - b64sig, err := sig.Base64Signature() - if err != nil { - return err - } - payload, err := sig.Payload() - if err != nil { - return err - } - _, err = FindTlogEntry(ctx, rekorClient, b64sig, payload, pemBytes) + _, err = tlogValidateEntry(ctx, rekorClient, sig, pemBytes) return err } @@ -324,23 +316,28 @@ func tlogValidateCertificate(ctx context.Context, rekorClient *client.Rekor, sig if err != nil { return err } - b64sig, err := sig.Base64Signature() + e, err := tlogValidateEntry(ctx, rekorClient, sig, pemBytes) if err != nil { return err } - payload, err := sig.Payload() + // if we have a cert, we should check expiry + return CheckExpiry(cert, time.Unix(*e.IntegratedTime, 0)) +} + +func tlogValidateEntry(ctx context.Context, client *client.Rekor, sig oci.Signature, pem []byte) (*models.LogEntryAnon, error) { + b64sig, err := sig.Base64Signature() if err != nil { - return err + return nil, err } - e, err := FindTlogEntry(ctx, rekorClient, b64sig, payload, pemBytes) + payload, err := sig.Payload() if err != nil { - return err + return nil, err } - if err := VerifyTLogEntry(ctx, rekorClient, e); err != nil { - return err + e, err := FindTlogEntry(ctx, client, b64sig, payload, pem) + if err != nil { + return nil, err } - // if we have a cert, we should check expiry - return CheckExpiry(cert, time.Unix(*e.IntegratedTime, 0)) + return e, VerifyTLogEntry(ctx, client, e) } type fakeOCISignatures struct { diff --git a/pkg/cosign/verify_test.go b/pkg/cosign/verify_test.go index 2c38ccd0cc4..c8c2f7d5464 100644 --- a/pkg/cosign/verify_test.go +++ b/pkg/cosign/verify_test.go @@ -40,11 +40,13 @@ import ( "github.com/in-toto/in-toto-golang/in_toto" "github.com/pkg/errors" "github.com/secure-systems-lab/go-securesystemslib/dsse" + "github.com/sigstore/cosign/internal/pkg/cosign/rekor/mock" "github.com/sigstore/cosign/pkg/cosign/bundle" ctuf "github.com/sigstore/cosign/pkg/cosign/tuf" "github.com/sigstore/cosign/pkg/oci/static" "github.com/sigstore/cosign/pkg/types" "github.com/sigstore/cosign/test" + "github.com/sigstore/rekor/pkg/generated/client" rtypes "github.com/sigstore/rekor/pkg/types" "github.com/sigstore/sigstore/pkg/cryptoutils" "github.com/sigstore/sigstore/pkg/signature" @@ -345,6 +347,41 @@ func TestVerifyImageSignatureWithExistingSub(t *testing.T) { } } +// This test ensures that image signature validation fails properly if we are +// using a SigVerifier with Rekor. +// See https://github.com/sigstore/cosign/issues/1816 for more details. +func TestVerifyImageSignatureWithSigVerifierAndRekor(t *testing.T) { + sv, privKey, err := signature.NewDefaultECDSASignerVerifier() + if err != nil { + t.Fatalf("error generating verifier: %v", err) + } + + payload := []byte{1, 2, 3, 4} + h := sha256.Sum256(payload) + sig, _ := privKey.Sign(rand.Reader, h[:], crypto.SHA256) + ociSig, _ := static.NewSignature(payload, base64.StdEncoding.EncodeToString(sig)) + + // Add a fake rekor client - this makes it look like there's a matching + // tlog entry for the signature during validation (even though it does not + // match the underlying data / key) + mClient := new(client.Rekor) + mClient.Entries = &mock.EntriesClient{} + + if _, err := VerifyImageSignature(context.TODO(), ociSig, v1.Hash{}, &CheckOpts{ + SigVerifier: sv, + RekorClient: mClient, + }); err == nil || !strings.Contains(err.Error(), "verifying inclusion proof") { + // TODO(wlynch): This is a weak test, since this is really failing because + // there is no inclusion proof for the Rekor entry rather than failing to + // validate the Rekor public key itself. At the very least this ensures + // that we're hitting tlog validation during signature checking, + // but we should look into improving this once there is an in-memory + // Rekor client that is capable of performing inclusion proof validation + // in unit tests. + t.Fatal("expected error while verifying signature") + } +} + func TestValidateAndUnpackCertSuccess(t *testing.T) { subject := "email@email" oidcIssuer := "https://accounts.google.com"