From 5f484433ad56d3ab9a4e5c60af79e18e63fbb7c6 Mon Sep 17 00:00:00 2001 From: Hayden Blauzvern Date: Thu, 8 Aug 2024 22:47:21 +0000 Subject: [PATCH] Fix crash with missing checkpoint It was assumed that if an inclusion proof is present, then a checkpoint would be present too. Some bundles exist where the checkpoint was not present, particularly for v0.1. This is a bug, so we should err out, but we shouldn't crash if the checkpoint is missing. This also adds bundle validation when constructing a bundle from disk, as it was only validating when constructed from proto. Signed-off-by: Hayden Blauzvern --- pkg/bundle/bundle.go | 7 +- pkg/bundle/bundle_test.go | 154 ++++++++++++++++++++++++++++++++++++-- pkg/tlog/entry.go | 7 ++ 3 files changed, 158 insertions(+), 10 deletions(-) diff --git a/pkg/bundle/bundle.go b/pkg/bundle/bundle.go index fbf53e6c..cadf05e8 100644 --- a/pkg/bundle/bundle.go +++ b/pkg/bundle/bundle.go @@ -61,8 +61,7 @@ func NewBundle(pbundle *protobundle.Bundle) (*Bundle, error) { hasInclusionProof: false, } - err := bundle.validate() - if err != nil { + if err := bundle.validate(); err != nil { return nil, err } @@ -208,6 +207,10 @@ func LoadJSONFromPath(path string) (*Bundle, error) { return nil, err } + if err := bundle.validate(); err != nil { + return nil, err + } + return &bundle, nil } diff --git a/pkg/bundle/bundle_test.go b/pkg/bundle/bundle_test.go index 0e3630bc..746030f1 100644 --- a/pkg/bundle/bundle_test.go +++ b/pkg/bundle/bundle_test.go @@ -25,7 +25,6 @@ import ( protobundle "github.com/sigstore/protobuf-specs/gen/pb-go/bundle/v1" protocommon "github.com/sigstore/protobuf-specs/gen/pb-go/common/v1" - v1 "github.com/sigstore/protobuf-specs/gen/pb-go/common/v1" rekorv1 "github.com/sigstore/protobuf-specs/gen/pb-go/rekor/v1" _ "github.com/sigstore/rekor/pkg/types/hashedrekord" "github.com/stretchr/testify/require" @@ -280,7 +279,79 @@ func Test_validate(t *testing.T) { }, }, Content: &protobundle.VerificationMaterial_PublicKey{ - PublicKey: &v1.PublicKeyIdentifier{}, + PublicKey: &protocommon.PublicKeyIdentifier{}, + }, + }, + Content: &protobundle.Bundle_MessageSignature{}, + }, + }, + }, + { + name: "v0.1 with inclusion promise & proof without checkpoint", + pb: Bundle{ + Bundle: &protobundle.Bundle{ + MediaType: "application/vnd.dev.sigstore.bundle+json;version=0.1", + VerificationMaterial: &protobundle.VerificationMaterial{ + TlogEntries: []*rekorv1.TransparencyLogEntry{ + { + LogIndex: 42, + LogId: &protocommon.LogId{ + KeyId: []byte("deadbeef"), + }, + KindVersion: &rekorv1.KindVersion{ + Kind: "hashedrekord", + Version: "0.0.1", + }, + IntegratedTime: 1, + CanonicalizedBody: canonicalTlogBody, + InclusionProof: &rekorv1.InclusionProof{ + LogIndex: 42, + RootHash: []byte("b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c"), + }, + InclusionPromise: &rekorv1.InclusionPromise{ + SignedEntryTimestamp: []byte("1"), + }, + }, + }, + Content: &protobundle.VerificationMaterial_PublicKey{ + PublicKey: &protocommon.PublicKeyIdentifier{}, + }, + }, + Content: &protobundle.Bundle_MessageSignature{}, + }, + }, + wantErr: true, + }, + { + name: "v0.1 with inclusion proof & promise", + pb: Bundle{ + Bundle: &protobundle.Bundle{ + MediaType: "application/vnd.dev.sigstore.bundle+json;version=0.1", + VerificationMaterial: &protobundle.VerificationMaterial{ + TlogEntries: []*rekorv1.TransparencyLogEntry{ + { + LogIndex: 42, + LogId: &protocommon.LogId{ + KeyId: []byte("deadbeef"), + }, + KindVersion: &rekorv1.KindVersion{ + Kind: "hashedrekord", + Version: "0.0.1", + }, + IntegratedTime: 1, + CanonicalizedBody: canonicalTlogBody, + InclusionProof: &rekorv1.InclusionProof{ + LogIndex: 42, + RootHash: []byte("b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c"), + Checkpoint: &rekorv1.Checkpoint{Envelope: "checkpoint"}, + }, + InclusionPromise: &rekorv1.InclusionPromise{ + SignedEntryTimestamp: []byte("1"), + }, + }, + }, + Content: &protobundle.VerificationMaterial_PublicKey{ + PublicKey: &protocommon.PublicKeyIdentifier{}, }, }, Content: &protobundle.Bundle_MessageSignature{}, @@ -308,7 +379,7 @@ func Test_validate(t *testing.T) { }, }, Content: &protobundle.VerificationMaterial_PublicKey{ - PublicKey: &v1.PublicKeyIdentifier{}, + PublicKey: &protocommon.PublicKeyIdentifier{}, }, }, Content: &protobundle.Bundle_MessageSignature{}, @@ -317,7 +388,40 @@ func Test_validate(t *testing.T) { wantErr: true, }, { - name: "v0.2 with inclusion proof", + name: "v0.2 with inclusion proof without checkpoint", + pb: Bundle{ + Bundle: &protobundle.Bundle{ + MediaType: "application/vnd.dev.sigstore.bundle+json;version=0.2", + VerificationMaterial: &protobundle.VerificationMaterial{ + TlogEntries: []*rekorv1.TransparencyLogEntry{ + { + LogIndex: 42, + LogId: &protocommon.LogId{ + KeyId: []byte("deadbeef"), + }, + KindVersion: &rekorv1.KindVersion{ + Kind: "hashedrekord", + Version: "0.0.1", + }, + IntegratedTime: 1, + CanonicalizedBody: canonicalTlogBody, + InclusionProof: &rekorv1.InclusionProof{ + LogIndex: 42, + RootHash: []byte("b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c"), + }, + }, + }, + Content: &protobundle.VerificationMaterial_PublicKey{ + PublicKey: &protocommon.PublicKeyIdentifier{}, + }, + }, + Content: &protobundle.Bundle_MessageSignature{}, + }, + }, + wantErr: true, + }, + { + name: "v0.2 with inclusion proof without empty checkpoint", pb: Bundle{ Bundle: &protobundle.Bundle{ MediaType: "application/vnd.dev.sigstore.bundle+json;version=0.2", @@ -342,7 +446,41 @@ func Test_validate(t *testing.T) { }, }, Content: &protobundle.VerificationMaterial_PublicKey{ - PublicKey: &v1.PublicKeyIdentifier{}, + PublicKey: &protocommon.PublicKeyIdentifier{}, + }, + }, + Content: &protobundle.Bundle_MessageSignature{}, + }, + }, + wantErr: true, + }, + { + name: "v0.2 with inclusion proof", + pb: Bundle{ + Bundle: &protobundle.Bundle{ + MediaType: "application/vnd.dev.sigstore.bundle+json;version=0.2", + VerificationMaterial: &protobundle.VerificationMaterial{ + TlogEntries: []*rekorv1.TransparencyLogEntry{ + { + LogIndex: 42, + LogId: &protocommon.LogId{ + KeyId: []byte("deadbeef"), + }, + KindVersion: &rekorv1.KindVersion{ + Kind: "hashedrekord", + Version: "0.0.1", + }, + IntegratedTime: 1, + CanonicalizedBody: canonicalTlogBody, + InclusionProof: &rekorv1.InclusionProof{ + LogIndex: 42, + RootHash: []byte("b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c"), + Checkpoint: &rekorv1.Checkpoint{Envelope: "checkpoint"}, + }, + }, + }, + Content: &protobundle.VerificationMaterial_PublicKey{ + PublicKey: &protocommon.PublicKeyIdentifier{}, }, }, Content: &protobundle.Bundle_MessageSignature{}, @@ -370,7 +508,7 @@ func Test_validate(t *testing.T) { InclusionProof: &rekorv1.InclusionProof{ LogIndex: 42, RootHash: []byte("b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c"), - Checkpoint: &rekorv1.Checkpoint{}, + Checkpoint: &rekorv1.Checkpoint{Envelope: "checkpoint"}, }, }, }, @@ -404,7 +542,7 @@ func Test_validate(t *testing.T) { InclusionProof: &rekorv1.InclusionProof{ LogIndex: 42, RootHash: []byte("b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c"), - Checkpoint: &rekorv1.Checkpoint{}, + Checkpoint: &rekorv1.Checkpoint{Envelope: "checkpoint"}, }, }, }, @@ -818,7 +956,7 @@ func Test_BundleValidation(t *testing.T) { Content: &protobundle.Bundle_DsseEnvelope{}, VerificationMaterial: &protobundle.VerificationMaterial{ Content: &protobundle.VerificationMaterial_PublicKey{ - PublicKey: &v1.PublicKeyIdentifier{}, + PublicKey: &protocommon.PublicKeyIdentifier{}, }, TimestampVerificationData: &protobundle.TimestampVerificationData{}, }, diff --git a/pkg/tlog/entry.go b/pkg/tlog/entry.go index b1ba68a4..939e25f0 100644 --- a/pkg/tlog/entry.go +++ b/pkg/tlog/entry.go @@ -124,6 +124,13 @@ func ParseEntry(protoEntry *v1.TransparencyLogEntry) (entry *Entry, err error) { rootHash := hex.EncodeToString(protoEntry.InclusionProof.RootHash) + if protoEntry.InclusionProof.Checkpoint == nil { + return nil, fmt.Errorf("inclusion proof missing required checkpoint") + } + if protoEntry.InclusionProof.Checkpoint.Envelope == "" { + return nil, fmt.Errorf("inclusion proof checkpoint empty") + } + inclusionProof = &models.InclusionProof{ LogIndex: swag.Int64(protoEntry.InclusionProof.LogIndex), RootHash: &rootHash,