From 37231bba0aa95307f33f23b59d29969bffdd5d13 Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Wed, 4 Sep 2024 14:14:14 -0400 Subject: [PATCH] Add additional validation for nil elements in Bundles Signed-off-by: Cody Soyland --- pkg/bundle/bundle.go | 30 +++++++-- pkg/bundle/bundle_test.go | 138 +++++++++++++++++++++++++++++++++++++- 2 files changed, 162 insertions(+), 6 deletions(-) diff --git a/pkg/bundle/bundle.go b/pkg/bundle/bundle.go index f6f93ed3..4ce5df86 100644 --- a/pkg/bundle/bundle.go +++ b/pkg/bundle/bundle.go @@ -38,7 +38,7 @@ var ErrUnsupportedMediaType = fmt.Errorf("%w: unsupported media type", ErrValida var ErrMissingVerificationMaterial = fmt.Errorf("%w: missing verification material", ErrValidation) var ErrUnimplemented = errors.New("unimplemented") var ErrInvalidAttestation = fmt.Errorf("%w: invalid attestation", ErrValidation) -var ErrMissingEnvelope = fmt.Errorf("%w: missing envelope", ErrInvalidAttestation) +var ErrMissingEnvelope = fmt.Errorf("%w: missing valid envelope", ErrInvalidAttestation) var ErrDecodingJSON = fmt.Errorf("%w: decoding json", ErrInvalidAttestation) var ErrDecodingB64 = fmt.Errorf("%w: decoding base64", ErrInvalidAttestation) @@ -196,7 +196,7 @@ func validateBundle(b *protobundle.Bundle) error { switch b.VerificationMaterial.Content.(type) { case *protobundle.VerificationMaterial_PublicKey, *protobundle.VerificationMaterial_Certificate, *protobundle.VerificationMaterial_X509CertificateChain: default: - return fmt.Errorf("invalid verififcation material content: verification material must be one of public key, x509 certificate and x509 certificate chain") + return fmt.Errorf("invalid verification material content: verification material must be one of public key, x509 certificate and x509 certificate chain") } return nil @@ -245,8 +245,11 @@ func (b *Bundle) VerificationContent() (verify.VerificationContent, error) { switch content := b.VerificationMaterial.GetContent().(type) { case *protobundle.VerificationMaterial_X509CertificateChain: + if content.X509CertificateChain == nil { + return nil, ErrMissingVerificationMaterial + } certs := content.X509CertificateChain.GetCertificates() - if len(certs) == 0 { + if len(certs) == 0 || certs[0].RawBytes == nil { return nil, ErrMissingVerificationMaterial } parsedCert, err := x509.ParseCertificate(certs[0].RawBytes) @@ -258,6 +261,9 @@ func (b *Bundle) VerificationContent() (verify.VerificationContent, error) { } return cert, nil case *protobundle.VerificationMaterial_Certificate: + if content.Certificate == nil || content.Certificate.RawBytes == nil { + return nil, ErrMissingVerificationMaterial + } parsedCert, err := x509.ParseCertificate(content.Certificate.RawBytes) if err != nil { return nil, ErrValidationError(err) @@ -267,6 +273,9 @@ func (b *Bundle) VerificationContent() (verify.VerificationContent, error) { } return cert, nil case *protobundle.VerificationMaterial_PublicKey: + if content.PublicKey == nil { + return nil, ErrMissingVerificationMaterial + } pk := &PublicKey{ hint: content.PublicKey.Hint, } @@ -318,6 +327,9 @@ func (b *Bundle) SignatureContent() (verify.SignatureContent, error) { } return envelope, nil case *protobundle.Bundle_MessageSignature: + if content.MessageSignature == nil || content.MessageSignature.MessageDigest == nil { + return nil, ErrMissingVerificationMaterial + } return NewMessageSignature( content.MessageSignature.MessageDigest.Digest, protocommon.HashAlgorithm_name[int32(content.MessageSignature.MessageDigest.Algorithm)], @@ -372,11 +384,21 @@ func (b *Bundle) MinVersion(expectVersion string) bool { } func parseEnvelope(input *protodsse.Envelope) (*Envelope, error) { + if input == nil { + return nil, ErrMissingEnvelope + } output := &dsse.Envelope{} - output.Payload = base64.StdEncoding.EncodeToString([]byte(input.GetPayload())) + payload := input.GetPayload() + if payload == nil { + return nil, ErrMissingEnvelope + } + output.Payload = base64.StdEncoding.EncodeToString([]byte(payload)) output.PayloadType = string(input.GetPayloadType()) output.Signatures = make([]dsse.Signature, len(input.GetSignatures())) for i, sig := range input.GetSignatures() { + if sig == nil { + return nil, ErrMissingEnvelope + } output.Signatures[i].KeyID = sig.GetKeyid() output.Signatures[i].Sig = base64.StdEncoding.EncodeToString(sig.GetSig()) } diff --git a/pkg/bundle/bundle_test.go b/pkg/bundle/bundle_test.go index 1d8d74eb..9f551902 100644 --- a/pkg/bundle/bundle_test.go +++ b/pkg/bundle/bundle_test.go @@ -25,6 +25,7 @@ import ( protobundle "github.com/sigstore/protobuf-specs/gen/pb-go/bundle/v1" protocommon "github.com/sigstore/protobuf-specs/gen/pb-go/common/v1" + protodsse "github.com/sigstore/protobuf-specs/gen/pb-go/dsse" rekorv1 "github.com/sigstore/protobuf-specs/gen/pb-go/rekor/v1" _ "github.com/sigstore/rekor/pkg/types/hashedrekord" "github.com/stretchr/testify/require" @@ -669,6 +670,53 @@ func TestVerificationContent(t *testing.T) { }, wantErr: true, }, + { + name: "certificate chain with nil bytes", + pb: Bundle{ + Bundle: &protobundle.Bundle{ + VerificationMaterial: &protobundle.VerificationMaterial{ + Content: &protobundle.VerificationMaterial_X509CertificateChain{ + X509CertificateChain: &protocommon.X509CertificateChain{ + Certificates: []*protocommon.X509Certificate{ + { + RawBytes: nil, + }, + }, + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "certificate chain with nil cert", + pb: Bundle{ + Bundle: &protobundle.Bundle{ + VerificationMaterial: &protobundle.VerificationMaterial{ + Content: &protobundle.VerificationMaterial_X509CertificateChain{ + X509CertificateChain: &protocommon.X509CertificateChain{ + Certificates: nil, + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "certificate chain with nil chain", + pb: Bundle{ + Bundle: &protobundle.Bundle{ + VerificationMaterial: &protobundle.VerificationMaterial{ + Content: &protobundle.VerificationMaterial_X509CertificateChain{ + X509CertificateChain: nil, + }, + }, + }, + }, + wantErr: true, + }, { name: "certificate", pb: Bundle{ @@ -699,6 +747,36 @@ func TestVerificationContent(t *testing.T) { }, wantErr: true, }, + { + name: "certificate with nil bytes", + pb: Bundle{ + Bundle: &protobundle.Bundle{ + VerificationMaterial: &protobundle.VerificationMaterial{ + Content: &protobundle.VerificationMaterial_Certificate{ + Certificate: &protocommon.X509Certificate{ + RawBytes: nil, + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "empty certificate", + pb: Bundle{ + Bundle: &protobundle.Bundle{ + VerificationMaterial: &protobundle.VerificationMaterial{ + Content: &protobundle.VerificationMaterial_Certificate{ + Certificate: &protocommon.X509Certificate{ + RawBytes: nil, + }, + }, + }, + }, + }, + wantErr: true, + }, { name: "public key", pb: Bundle{ @@ -712,6 +790,19 @@ func TestVerificationContent(t *testing.T) { }, wantPublicKey: true, }, + { + name: "nil public key", + pb: Bundle{ + Bundle: &protobundle.Bundle{ + VerificationMaterial: &protobundle.VerificationMaterial{ + Content: &protobundle.VerificationMaterial_PublicKey{ + PublicKey: nil, + }, + }, + }, + }, + wantErr: true, + }, } for _, tt := range tests { tt := tt @@ -742,16 +833,50 @@ func TestSignatureContent(t *testing.T) { pb Bundle wantEnvelope bool wantSignature bool + wantErr bool }{ { name: "dsse envelope", pb: Bundle{ Bundle: &protobundle.Bundle{ - Content: &protobundle.Bundle_DsseEnvelope{}, + Content: &protobundle.Bundle_DsseEnvelope{ + DsseEnvelope: &protodsse.Envelope{ + Payload: []byte{}, + Signatures: []*protodsse.Signature{{Sig: []byte{}, Keyid: ""}}, + }, + }, }, }, wantEnvelope: true, }, + { + name: "dsse envelope with nil signature", + pb: Bundle{ + Bundle: &protobundle.Bundle{ + Content: &protobundle.Bundle_DsseEnvelope{ + DsseEnvelope: &protodsse.Envelope{ + Payload: []byte{}, + Signatures: []*protodsse.Signature{nil}, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "dsse envelope with nil payload", + pb: Bundle{ + Bundle: &protobundle.Bundle{ + Content: &protobundle.Bundle_DsseEnvelope{ + DsseEnvelope: &protodsse.Envelope{ + Payload: nil, + Signatures: []*protodsse.Signature{{Sig: []byte{}, Keyid: ""}}, + }, + }, + }, + }, + wantErr: true, + }, { name: "message signature", pb: Bundle{ @@ -770,6 +895,10 @@ func TestSignatureContent(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { got, gotErr := tt.pb.SignatureContent() + if tt.wantErr { + require.Error(t, gotErr) + return + } require.NoError(t, gotErr) if tt.wantEnvelope { require.NotNil(t, got.EnvelopeContent()) @@ -794,7 +923,12 @@ func TestEnvelope(t *testing.T) { name: "dsse envelope", pb: Bundle{ Bundle: &protobundle.Bundle{ - Content: &protobundle.Bundle_DsseEnvelope{}, + Content: &protobundle.Bundle_DsseEnvelope{ + DsseEnvelope: &protodsse.Envelope{ + Payload: []byte{}, + Signatures: []*protodsse.Signature{{Sig: []byte{}, Keyid: ""}}, + }, + }, }, }, },