Skip to content

Commit

Permalink
Fix crash with missing checkpoint
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@haydentherapper
haydentherapper committed Aug 8, 2024
1 parent 004c425 commit 5f48443
Show file tree
Hide file tree
Showing 3 changed files with 158 additions and 10 deletions.
7 changes: 5 additions & 2 deletions pkg/bundle/bundle.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down
154 changes: 146 additions & 8 deletions pkg/bundle/bundle_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{},
Expand Down Expand Up @@ -308,7 +379,7 @@ func Test_validate(t *testing.T) {
},
},
Content: &protobundle.VerificationMaterial_PublicKey{
PublicKey: &v1.PublicKeyIdentifier{},
PublicKey: &protocommon.PublicKeyIdentifier{},
},
},
Content: &protobundle.Bundle_MessageSignature{},
Expand All @@ -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",
Expand All @@ -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{},
Expand Down Expand Up @@ -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"},
},
},
},
Expand Down Expand Up @@ -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"},
},
},
},
Expand Down Expand Up @@ -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{},
},
Expand Down
7 changes: 7 additions & 0 deletions pkg/tlog/entry.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 5f48443

Please sign in to comment.