From 629f5f8fa672973503edde75f84dcd984637629e Mon Sep 17 00:00:00 2001 From: Hayden B Date: Wed, 10 Apr 2024 14:57:21 -0700 Subject: [PATCH] Fixes for GHSA-88jx-383q-w4qc and GHSA-95pr-fxf5-86gv (#3661) * Merge pull request from GHSA-95pr-fxf5-86gv An Image may come from an untrusted source and contain an unknown number of signatures in the .sig manifest. A common pattern in cosign is to use the number of signatures as the capacity for a new slice. But this means the size of the slice is based on an unvalidated external input and could result in cosign running out of memory. This change adds validation for certain implementations of the oci.Signatures Get() method to limit the number of image descriptors returned. This way, callers can rely on the returned slice of signatures being a reasonable size to process safely. The limit is set to 1000, which is a generous size based on the practical restrictions that container registries set for image manifest size and approximations of memory allocations for signature layers. Signed-off-by: Colleen Murphy * Merge pull request from GHSA-88jx-383q-w4qc When downloading an attestation or SBOM from an external source, check its size before reading it into memory. This protects the host from potentially reading a maliciously large attachment into memory and exhausting the system. SBOMs can vary widely in size, and there could be legitimate SBOMs of up to 700MB. However, reading a 700MB SBOM into memory would easily bring down a small cloud VM. Moreover, most SBOMs are not going to be that large. This change sets a reasonable default of 128MiB, and allows overriding the default by setting the environment variable `COSIGN_MAX_ATTACHMENT_SIZE`. Signed-off-by: Colleen Murphy --------- Signed-off-by: Colleen Murphy --- .../cli/verify/verify_blob_attestation.go | 9 ++ .../verify/verify_blob_attestation_test.go | 12 ++ go.mod | 1 + internal/pkg/cosign/payload/size/errors.go | 31 +++++ internal/pkg/cosign/payload/size/size.go | 38 ++++++ internal/pkg/cosign/payload/size/size_test.go | 110 ++++++++++++++++++ pkg/cosign/env/env.go | 6 + pkg/oci/errors.go | 31 +++++ pkg/oci/internal/signature/layer.go | 9 ++ pkg/oci/internal/signature/layer_test.go | 52 +++++++++ pkg/oci/layout/signatures.go | 8 +- pkg/oci/layout/signatures_test.go | 62 ++++++++++ pkg/oci/mutate/signatures.go | 6 + pkg/oci/mutate/signatures_test.go | 63 ++++++++++ pkg/oci/remote/remote.go | 10 ++ pkg/oci/remote/remote_test.go | 70 +++++++++++ pkg/oci/remote/signatures.go | 6 + pkg/oci/remote/signatures_test.go | 22 ++++ pkg/oci/signature/layer.go | 9 ++ pkg/oci/signature/layer_test.go | 52 +++++++++ pkg/oci/static/file.go | 9 ++ pkg/oci/static/file_test.go | 48 +++++++- 22 files changed, 657 insertions(+), 7 deletions(-) create mode 100644 internal/pkg/cosign/payload/size/errors.go create mode 100644 internal/pkg/cosign/payload/size/size.go create mode 100644 internal/pkg/cosign/payload/size/size_test.go create mode 100644 pkg/oci/errors.go create mode 100644 pkg/oci/layout/signatures_test.go diff --git a/cmd/cosign/cli/verify/verify_blob_attestation.go b/cmd/cosign/cli/verify/verify_blob_attestation.go index 0d9c9b77a61..63983eb4c2d 100644 --- a/cmd/cosign/cli/verify/verify_blob_attestation.go +++ b/cmd/cosign/cli/verify/verify_blob_attestation.go @@ -34,6 +34,7 @@ import ( "github.com/sigstore/cosign/v2/cmd/cosign/cli/options" "github.com/sigstore/cosign/v2/cmd/cosign/cli/rekor" internal "github.com/sigstore/cosign/v2/internal/pkg/cosign" + payloadsize "github.com/sigstore/cosign/v2/internal/pkg/cosign/payload/size" "github.com/sigstore/cosign/v2/internal/pkg/cosign/tsa" "github.com/sigstore/cosign/v2/pkg/blob" "github.com/sigstore/cosign/v2/pkg/cosign" @@ -117,6 +118,14 @@ func (c *VerifyBlobAttestationCommand) Exec(ctx context.Context, artifactPath st return err } defer f.Close() + fileInfo, err := f.Stat() + if err != nil { + return err + } + err = payloadsize.CheckSize(uint64(fileInfo.Size())) + if err != nil { + return err + } payload = internal.NewHashReader(f, sha256.New()) if _, err := io.ReadAll(&payload); err != nil { diff --git a/cmd/cosign/cli/verify/verify_blob_attestation_test.go b/cmd/cosign/cli/verify/verify_blob_attestation_test.go index 35c9940d86d..2d87efeb451 100644 --- a/cmd/cosign/cli/verify/verify_blob_attestation_test.go +++ b/cmd/cosign/cli/verify/verify_blob_attestation_test.go @@ -32,6 +32,7 @@ gZPFIp557+TOoDxf14FODWc+sIPETk0OgCplAk60doVXbCv33IU4rXZHrg== const ( blobContents = "some-payload" anotherBlobContents = "another-blob" + hugeBlobContents = "hugepayloadhugepayloadhugepayloadhugepayloadhugepayloadhugepayloadhugepayloadhugepayloadhugepayloadhugepayloadhugepayloadhugepayloadhugepayload" blobSLSAProvenanceSignature = "eyJwYXlsb2FkVHlwZSI6ImFwcGxpY2F0aW9uL3ZuZC5pbi10b3RvK2pzb24iLCJwYXlsb2FkIjoiZXlKZmRIbHdaU0k2SW1oMGRIQnpPaTh2YVc0dGRHOTBieTVwYnk5VGRHRjBaVzFsYm5RdmRqQXVNU0lzSW5CeVpXUnBZMkYwWlZSNWNHVWlPaUpvZEhSd2N6b3ZMM05zYzJFdVpHVjJMM0J5YjNabGJtRnVZMlV2ZGpBdU1pSXNJbk4xWW1wbFkzUWlPbHQ3SW01aGJXVWlPaUppYkc5aUlpd2laR2xuWlhOMElqcDdJbk5vWVRJMU5pSTZJalkxT0RjNE1XTmtOR1ZrT1dKallUWXdaR0ZqWkRBNVpqZGlZamt4TkdKaU5URTFNREpsT0dJMVpEWXhPV1kxTjJZek9XRXhaRFkxTWpVNU5tTmpNalFpZlgxZExDSndjbVZrYVdOaGRHVWlPbnNpWW5WcGJHUmxjaUk2ZXlKcFpDSTZJaklpZlN3aVluVnBiR1JVZVhCbElqb2llQ0lzSW1sdWRtOWpZWFJwYjI0aU9uc2lZMjl1Wm1sblUyOTFjbU5sSWpwN2ZYMTlmUT09Iiwic2lnbmF0dXJlcyI6W3sia2V5aWQiOiIiLCJzaWciOiJNRVVDSUE4S2pacWtydDkwZnpCb2pTd3d0ajNCcWI0MUU2cnV4UWs5N1RMbnB6ZFlBaUVBek9Bak9Uenl2VEhxYnBGREFuNnpocmc2RVp2N2t4SzVmYVJvVkdZTWgyYz0ifV19" dssePredicateEmptySubject = "eyJwYXlsb2FkVHlwZSI6ImFwcGxpY2F0aW9uL3ZuZC5pbi10b3RvK2pzb24iLCJwYXlsb2FkIjoiZXlKZmRIbHdaU0k2SW1oMGRIQnpPaTh2YVc0dGRHOTBieTVwYnk5VGRHRjBaVzFsYm5RdmRqQXVNU0lzSW5CeVpXUnBZMkYwWlZSNWNHVWlPaUpvZEhSd2N6b3ZMM05zYzJFdVpHVjJMM0J5YjNabGJtRnVZMlV2ZGpBdU1pSXNJbk4xWW1wbFkzUWlPbHRkTENKd2NtVmthV05oZEdVaU9uc2lZblZwYkdSbGNpSTZleUpwWkNJNklqSWlmU3dpWW5WcGJHUlVlWEJsSWpvaWVDSXNJbWx1ZG05allYUnBiMjRpT25zaVkyOXVabWxuVTI5MWNtTmxJanA3ZlgxOWZRPT0iLCJzaWduYXR1cmVzIjpbeyJrZXlpZCI6IiIsInNpZyI6Ik1FWUNJUUNrTEV2NkhZZ0svZDdUK0N3NTdXbkZGaHFUTC9WalAyVDA5Q2t1dk1nbDRnSWhBT1hBM0lhWWg1M1FscVk1eVU4cWZxRXJma2tGajlEakZnaWovUTQ2NnJSViJ9XX0=" dssePredicateMissingSha256 = "eyJwYXlsb2FkVHlwZSI6ImFwcGxpY2F0aW9uL3ZuZC5pbi10b3RvK2pzb24iLCJwYXlsb2FkIjoiZXlKZmRIbHdaU0k2SW1oMGRIQnpPaTh2YVc0dGRHOTBieTVwYnk5VGRHRjBaVzFsYm5RdmRqQXVNU0lzSW5CeVpXUnBZMkYwWlZSNWNHVWlPaUpvZEhSd2N6b3ZMM05zYzJFdVpHVjJMM0J5YjNabGJtRnVZMlV2ZGpBdU1pSXNJbk4xWW1wbFkzUWlPbHQ3SW01aGJXVWlPaUppYkc5aUlpd2laR2xuWlhOMElqcDdmWDFkTENKd2NtVmthV05oZEdVaU9uc2lZblZwYkdSbGNpSTZleUpwWkNJNklqSWlmU3dpWW5WcGJHUlVlWEJsSWpvaWVDSXNJbWx1ZG05allYUnBiMjRpT25zaVkyOXVabWxuVTI5MWNtTmxJanA3ZlgxOWZRPT0iLCJzaWduYXR1cmVzIjpbeyJrZXlpZCI6IiIsInNpZyI6Ik1FVUNJQysvM2M4RFo1TGFZTEx6SFZGejE3ZmxHUENlZXVNZ2tIKy8wa2s1cFFLUEFpRUFqTStyYnBBRlJybDdpV0I2Vm9BYVZPZ3U3NjRRM0JKdHI1bHk4VEFHczNrPSJ9XX0=" @@ -46,6 +47,7 @@ func TestVerifyBlobAttestation(t *testing.T) { blobPath := writeBlobFile(t, td, blobContents, "blob") anotherBlobPath := writeBlobFile(t, td, anotherBlobContents, "other-blob") + hugeBlobPath := writeBlobFile(t, td, hugeBlobContents, "huge-blob") keyRef := writeBlobFile(t, td, pubkey, "cosign.pub") tests := []struct { @@ -53,6 +55,7 @@ func TestVerifyBlobAttestation(t *testing.T) { blobPath string signature string predicateType string + env map[string]string shouldErr bool }{ { @@ -98,11 +101,20 @@ func TestVerifyBlobAttestation(t *testing.T) { signature: dssePredicateMultipleSubjectsInvalid, blobPath: blobPath, shouldErr: true, + }, { + description: "override file size limit", + signature: blobSLSAProvenanceSignature, + blobPath: hugeBlobPath, + env: map[string]string{"COSIGN_MAX_ATTACHMENT_SIZE": "128"}, + shouldErr: true, }, } for _, test := range tests { t.Run(test.description, func(t *testing.T) { + for k, v := range test.env { + t.Setenv(k, v) + } decodedSig, err := base64.StdEncoding.DecodeString(test.signature) if err != nil { t.Fatal(err) diff --git a/go.mod b/go.mod index 05cfbe83c75..8dcf04001dd 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/cyberphone/json-canonicalization v0.0.0-20231011164504-785e29786b46 github.com/depcheck-test/depcheck-test v0.0.0-20220607135614-199033aaa936 github.com/digitorus/timestamp v0.0.0-20231217203849-220c5c2851b7 + github.com/dustin/go-humanize v1.0.1 github.com/go-openapi/runtime v0.28.0 github.com/go-openapi/strfmt v0.23.0 github.com/go-openapi/swag v0.23.0 diff --git a/internal/pkg/cosign/payload/size/errors.go b/internal/pkg/cosign/payload/size/errors.go new file mode 100644 index 00000000000..5a7e055989d --- /dev/null +++ b/internal/pkg/cosign/payload/size/errors.go @@ -0,0 +1,31 @@ +// Copyright 2024 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 payload + +import "fmt" + +// MaxLayerSizeExceeded is an error indicating that the layer is too big to read into memory and cosign should abort processing it. +type MaxLayerSizeExceeded struct { + value uint64 + maximum uint64 +} + +func NewMaxLayerSizeExceeded(value, maximum uint64) *MaxLayerSizeExceeded { + return &MaxLayerSizeExceeded{value, maximum} +} + +func (e *MaxLayerSizeExceeded) Error() string { + return fmt.Sprintf("size of layer (%d) exceeded the limit (%d)", e.value, e.maximum) +} diff --git a/internal/pkg/cosign/payload/size/size.go b/internal/pkg/cosign/payload/size/size.go new file mode 100644 index 00000000000..f867477c732 --- /dev/null +++ b/internal/pkg/cosign/payload/size/size.go @@ -0,0 +1,38 @@ +// Copyright 2024 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 payload + +import ( + "github.com/dustin/go-humanize" + "github.com/sigstore/cosign/v2/pkg/cosign/env" +) + +const defaultMaxSize = uint64(134217728) // 128MiB + +func CheckSize(size uint64) error { + maxSize := defaultMaxSize + maxSizeOverride, exists := env.LookupEnv(env.VariableMaxAttachmentSize) + if exists { + var err error + maxSize, err = humanize.ParseBytes(maxSizeOverride) + if err != nil { + maxSize = defaultMaxSize + } + } + if size > maxSize { + return NewMaxLayerSizeExceeded(size, maxSize) + } + return nil +} diff --git a/internal/pkg/cosign/payload/size/size_test.go b/internal/pkg/cosign/payload/size/size_test.go new file mode 100644 index 00000000000..7feba4024cd --- /dev/null +++ b/internal/pkg/cosign/payload/size/size_test.go @@ -0,0 +1,110 @@ +// Copyright 2024 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 payload + +import ( + "testing" +) + +func TestCheckSize(t *testing.T) { + tests := []struct { + name string + input uint64 + setting string + wantErr bool + }{ + { + name: "size is within default limit", + input: 1000, + wantErr: false, + }, + { + name: "size exceeds default limit", + input: 200000000, + wantErr: true, + }, + { + name: "size is within overridden limit (bytes)", + input: 1000, + setting: "1024", + wantErr: false, + }, + { + name: "size is exceeds overridden limit (bytes)", + input: 2000, + setting: "1024", + wantErr: true, + }, + { + name: "size is within overridden limit (megabytes, short form)", + input: 1999999, + setting: "2M", + wantErr: false, + }, + { + name: "size exceeds overridden limit (megabytes, short form)", + input: 2000001, + setting: "2M", + wantErr: true, + }, + { + name: "size is within overridden limit (megabytes, long form)", + input: 1999999, + setting: "2MB", + wantErr: false, + }, + { + name: "size exceeds overridden limit (megabytes, long form)", + input: 2000001, + setting: "2MB", + wantErr: true, + }, + { + name: "size is within overridden limit (mebibytes)", + input: 2097151, + setting: "2MiB", + wantErr: false, + }, + { + name: "size exceeds overridden limit (mebibytes)", + input: 2097153, + setting: "2MiB", + wantErr: true, + }, + { + name: "size is negative results in default", + input: 5121, + setting: "-5KiB", + wantErr: false, + }, + { + name: "invalid setting results in default", + input: 5121, + setting: "five kilobytes", + wantErr: false, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if test.setting != "" { + t.Setenv("COSIGN_MAX_ATTACHMENT_SIZE", test.setting) + } + got := CheckSize(test.input) + if (got != nil) != (test.wantErr) { + t.Errorf("CheckSize() = %v, expected %v", got, test.wantErr) + } + }) + } +} diff --git a/pkg/cosign/env/env.go b/pkg/cosign/env/env.go index 5c26d4f5168..a2960e08143 100644 --- a/pkg/cosign/env/env.go +++ b/pkg/cosign/env/env.go @@ -51,6 +51,7 @@ const ( VariablePKCS11ModulePath Variable = "COSIGN_PKCS11_MODULE_PATH" VariablePKCS11IgnoreCertificate Variable = "COSIGN_PKCS11_IGNORE_CERTIFICATE" VariableRepository Variable = "COSIGN_REPOSITORY" + VariableMaxAttachmentSize Variable = "COSIGN_MAX_ATTACHMENT_SIZE" // Sigstore environment variables VariableSigstoreCTLogPublicKeyFile Variable = "SIGSTORE_CT_LOG_PUBLIC_KEY_FILE" @@ -113,6 +114,11 @@ var ( Expects: "string with a repository", Sensitive: false, }, + VariableMaxAttachmentSize: { + Description: "maximum attachment size to download (default 128MiB)", + Expects: "human-readable unit of memory, e.g. 5120, 20K, 3M, 45MiB, 1GB", + Sensitive: false, + }, VariableSigstoreCTLogPublicKeyFile: { Description: "overrides what is used to validate the SCT coming back from Fulcio", diff --git a/pkg/oci/errors.go b/pkg/oci/errors.go new file mode 100644 index 00000000000..aa0c2985b0b --- /dev/null +++ b/pkg/oci/errors.go @@ -0,0 +1,31 @@ +// Copyright 2024 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 oci + +import "fmt" + +// MaxLayersExceeded is an error indicating that the artifact has too many layers and cosign should abort processing it. +type MaxLayersExceeded struct { + value int64 + maximum int64 +} + +func NewMaxLayersExceeded(value, maximum int64) *MaxLayersExceeded { + return &MaxLayersExceeded{value, maximum} +} + +func (e *MaxLayersExceeded) Error() string { + return fmt.Sprintf("number of layers (%d) exceeded the limit (%d)", e.value, e.maximum) +} diff --git a/pkg/oci/internal/signature/layer.go b/pkg/oci/internal/signature/layer.go index 176fe3cefdc..d92af61c30a 100644 --- a/pkg/oci/internal/signature/layer.go +++ b/pkg/oci/internal/signature/layer.go @@ -24,6 +24,7 @@ import ( "strings" v1 "github.com/google/go-containerregistry/pkg/v1" + payloadsize "github.com/sigstore/cosign/v2/internal/pkg/cosign/payload/size" "github.com/sigstore/cosign/v2/pkg/cosign/bundle" "github.com/sigstore/cosign/v2/pkg/oci" "github.com/sigstore/sigstore/pkg/cryptoutils" @@ -58,6 +59,14 @@ func (s *sigLayer) Annotations() (map[string]string, error) { // Payload implements oci.Signature func (s *sigLayer) Payload() ([]byte, error) { + size, err := s.Layer.Size() + if err != nil { + return nil, err + } + err = payloadsize.CheckSize(uint64(size)) + if err != nil { + return nil, err + } // Compressed is a misnomer here, we just want the raw bytes from the registry. r, err := s.Layer.Compressed() if err != nil { diff --git a/pkg/oci/internal/signature/layer_test.go b/pkg/oci/internal/signature/layer_test.go index e6700366500..d3895f9c42a 100644 --- a/pkg/oci/internal/signature/layer_test.go +++ b/pkg/oci/internal/signature/layer_test.go @@ -20,6 +20,8 @@ import ( "encoding/base64" "errors" "fmt" + "io" + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -50,6 +52,7 @@ func TestSignature(t *testing.T) { tests := []struct { name string l *sigLayer + env map[string]string wantPayloadErr error wantSig string wantSigErr error @@ -222,10 +225,39 @@ Hr/+CxFvaJWmpYqNkLDGRU+9orzh5hI2RrcuaQ== }, wantSig: "blah", wantChain: 1, + }, { + name: "payload size exceeds default limit", + l: &sigLayer{ + Layer: &mockLayer{size: 134217728 + 42}, // 128MB + 42 bytes + }, + wantPayloadErr: errors.New("size of layer (134217770) exceeded the limit (134217728)"), + }, { + name: "payload size exceeds overridden limit", + l: &sigLayer{ + Layer: &mockLayer{size: 1000000000 + 42}, // 1GB + 42 bytes + }, + env: map[string]string{"COSIGN_MAX_ATTACHMENT_SIZE": "1GB"}, + wantPayloadErr: errors.New("size of layer (1000000042) exceeded the limit (1000000000)"), + }, { + name: "payload size is within overridden limit", + l: &sigLayer{ + Layer: layer, + desc: v1.Descriptor{ + Digest: digest, + Annotations: map[string]string{ + sigkey: "blah", + }, + }, + }, + env: map[string]string{"COSIGN_MAX_ATTACHMENT_SIZE": "5KB"}, + wantSig: "blah", }} for _, test := range tests { t.Run(test.name, func(t *testing.T) { + for k, v := range test.env { + t.Setenv(k, v) + } b, err := test.l.Payload() switch { case (err != nil) != (test.wantPayloadErr != nil): @@ -239,6 +271,9 @@ Hr/+CxFvaJWmpYqNkLDGRU+9orzh5hI2RrcuaQ== t.Errorf("v1.SHA256() = %v, wanted %v", got, want) } } + if err != nil { + return + } switch got, err := test.l.Base64Signature(); { case (err != nil) != (test.wantSigErr != nil): @@ -453,3 +488,20 @@ func TestSignatureWithTSAAnnotation(t *testing.T) { }) } } + +type mockLayer struct { + size int64 +} + +func (m *mockLayer) Size() (int64, error) { + return m.size, nil +} + +func (m *mockLayer) Compressed() (io.ReadCloser, error) { + return io.NopCloser(strings.NewReader("data")), nil +} + +func (m *mockLayer) Digest() (v1.Hash, error) { panic("not implemented") } +func (m *mockLayer) DiffID() (v1.Hash, error) { panic("not implemented") } +func (m *mockLayer) Uncompressed() (io.ReadCloser, error) { panic("not implemented") } +func (m *mockLayer) MediaType() (types.MediaType, error) { panic("not implemented") } diff --git a/pkg/oci/layout/signatures.go b/pkg/oci/layout/signatures.go index c9f24866cf2..1b0d4c02334 100644 --- a/pkg/oci/layout/signatures.go +++ b/pkg/oci/layout/signatures.go @@ -21,6 +21,8 @@ import ( "github.com/sigstore/cosign/v2/pkg/oci/internal/signature" ) +const maxLayers = 1000 + type sigs struct { v1.Image } @@ -33,7 +35,11 @@ func (s *sigs) Get() ([]oci.Signature, error) { if err != nil { return nil, err } - signatures := make([]oci.Signature, 0, len(manifest.Layers)) + numLayers := int64(len(manifest.Layers)) + if numLayers > maxLayers { + return nil, oci.NewMaxLayersExceeded(numLayers, maxLayers) + } + signatures := make([]oci.Signature, 0, numLayers) for _, desc := range manifest.Layers { l, err := s.Image.LayerByDigest(desc.Digest) if err != nil { diff --git a/pkg/oci/layout/signatures_test.go b/pkg/oci/layout/signatures_test.go new file mode 100644 index 00000000000..6df93cebd6b --- /dev/null +++ b/pkg/oci/layout/signatures_test.go @@ -0,0 +1,62 @@ +// Copyright 2024 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 layout + +import ( + "errors" + "testing" + + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/fake" +) + +func TestGet(t *testing.T) { + tests := []struct { + name string + layers int + wantError error + }{ + { + name: "within limit", + layers: 23, + wantError: nil, + }, + { + name: "exceeds limit", + layers: 4242, + wantError: errors.New("number of layers (4242) exceeded the limit (1000)"), + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + s := sigs{ + Image: &fake.FakeImage{ + ManifestStub: func() (*v1.Manifest, error) { + return &v1.Manifest{ + Layers: make([]v1.Descriptor, test.layers), + }, nil + }, + }, + } + _, err := s.Get() + if test.wantError != nil && test.wantError.Error() != err.Error() { + t.Fatalf("Get() = %v, wanted %v", err, test.wantError) + } + if test.wantError == nil && err != nil { + t.Fatalf("Get() = %v, wanted %v", err, test.wantError) + } + }) + } +} diff --git a/pkg/oci/mutate/signatures.go b/pkg/oci/mutate/signatures.go index 4f3bd98fa1f..2a7c077ec2a 100644 --- a/pkg/oci/mutate/signatures.go +++ b/pkg/oci/mutate/signatures.go @@ -23,6 +23,8 @@ import ( "github.com/sigstore/cosign/v2/pkg/oci" ) +const maxLayers = 1000 + // AppendSignatures produces a new oci.Signatures with the provided signatures // appended to the provided base signatures. func AppendSignatures(base oci.Signatures, recordCreationTimestamp bool, sigs ...oci.Signature) (oci.Signatures, error) { @@ -106,5 +108,9 @@ func (sa *sigAppender) Get() ([]oci.Signature, error) { if err != nil { return nil, err } + sumLayers := int64(len(sl) + len(sa.sigs)) + if sumLayers > maxLayers { + return nil, oci.NewMaxLayersExceeded(sumLayers, maxLayers) + } return append(sl, sa.sigs...), nil } diff --git a/pkg/oci/mutate/signatures_test.go b/pkg/oci/mutate/signatures_test.go index b5f2b10c934..195780e3d90 100644 --- a/pkg/oci/mutate/signatures_test.go +++ b/pkg/oci/mutate/signatures_test.go @@ -16,8 +16,11 @@ package mutate import ( + "errors" "testing" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/sigstore/cosign/v2/pkg/oci" "github.com/sigstore/cosign/v2/pkg/oci/empty" "github.com/sigstore/cosign/v2/pkg/oci/static" ) @@ -83,3 +86,63 @@ func TestAppendSignatures(t *testing.T) { t.Errorf("Date of Signature was Zero") } } + +func TestGet(t *testing.T) { + tests := []struct { + name string + baseLayers int + appendLayers int + wantError error + }{ + { + name: "within limit", + baseLayers: 1, + appendLayers: 1, + wantError: nil, + }, + { + name: "base exceeds limit", + baseLayers: 2000, + appendLayers: 1, + wantError: errors.New("number of layers (2001) exceeded the limit (1000)"), + }, + { + name: "append exceeds limit", + baseLayers: 1, + appendLayers: 1300, + wantError: errors.New("number of layers (1301) exceeded the limit (1000)"), + }, + { + name: "sum exceeds limit", + baseLayers: 666, + appendLayers: 666, + wantError: errors.New("number of layers (1332) exceeded the limit (1000)"), + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + sa := sigAppender{ + base: &mockOCISignatures{ + signatures: make([]oci.Signature, test.baseLayers), + }, + sigs: make([]oci.Signature, test.appendLayers), + } + _, err := sa.Get() + if test.wantError != nil && test.wantError.Error() != err.Error() { + t.Fatalf("Get() = %v, wanted %v", err, test.wantError) + } + if test.wantError == nil && err != nil { + t.Fatalf("Get() = %v, wanted %v", err, test.wantError) + } + }) + } +} + +type mockOCISignatures struct { + v1.Image + signatures []oci.Signature +} + +func (m *mockOCISignatures) Get() ([]oci.Signature, error) { + return m.signatures, nil +} diff --git a/pkg/oci/remote/remote.go b/pkg/oci/remote/remote.go index 7827407ce3a..eab4e1f9b01 100644 --- a/pkg/oci/remote/remote.go +++ b/pkg/oci/remote/remote.go @@ -26,6 +26,7 @@ import ( "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/google/go-containerregistry/pkg/v1/remote/transport" "github.com/google/go-containerregistry/pkg/v1/types" + payloadsize "github.com/sigstore/cosign/v2/internal/pkg/cosign/payload/size" ociexperimental "github.com/sigstore/cosign/v2/internal/pkg/oci/remote" "github.com/sigstore/cosign/v2/pkg/oci" ) @@ -226,6 +227,15 @@ func (f *attached) FileMediaType() (types.MediaType, error) { // Payload implements oci.File func (f *attached) Payload() ([]byte, error) { + size, err := f.layer.Size() + if err != nil { + return nil, err + } + err = payloadsize.CheckSize(uint64(size)) + if err != nil { + return nil, err + } + // remote layers are believed to be stored // compressed, but we don't compress attachments // so use "Compressed" to access the raw byte diff --git a/pkg/oci/remote/remote_test.go b/pkg/oci/remote/remote_test.go index 90449ae7130..1d21792d573 100644 --- a/pkg/oci/remote/remote_test.go +++ b/pkg/oci/remote/remote_test.go @@ -17,11 +17,14 @@ package remote import ( "errors" + "io" + "strings" "testing" "github.com/google/go-containerregistry/pkg/name" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/types" ) func TestTagMethods(t *testing.T) { @@ -203,3 +206,70 @@ func TestDockercontentDigest(t *testing.T) { }) } } + +func TestPayload(t *testing.T) { + tests := []struct { + name string + size int64 + env map[string]string + wantError error + }{ + { + name: "within default limit", + size: 1000, + wantError: nil, + }, + { + name: "excceds default limit", + size: 1073741824, + wantError: errors.New("size of layer (1073741824) exceeded the limit (134217728)"), + }, + { + name: "exceeds overridden limit", + size: 5120, + env: map[string]string{"COSIGN_MAX_ATTACHMENT_SIZE": "1KB"}, + wantError: errors.New("size of layer (5120) exceeded the limit (1000)"), + }, + { + name: "within overridden limit", + size: 5120, + env: map[string]string{"COSIGN_MAX_ATTACHMENT_SIZE": "10KB"}, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + for k, v := range test.env { + t.Setenv(k, v) + } + a := attached{ + layer: &mockLayer{ + size: test.size, + }, + } + _, err := a.Payload() + if test.wantError != nil && test.wantError.Error() != err.Error() { + t.Fatalf("Payload() = %v, wanted %v", err, test.wantError) + } + if test.wantError == nil && err != nil { + t.Fatalf("Payload() = %v, wanted %v", err, test.wantError) + } + }) + } +} + +type mockLayer struct { + size int64 +} + +func (m *mockLayer) Compressed() (io.ReadCloser, error) { + return io.NopCloser(strings.NewReader("test payload")), nil +} + +func (m *mockLayer) Size() (int64, error) { + return m.size, nil +} + +func (m *mockLayer) Digest() (v1.Hash, error) { panic("not implemented") } +func (m *mockLayer) DiffID() (v1.Hash, error) { panic("not implemented") } +func (m *mockLayer) Uncompressed() (io.ReadCloser, error) { panic("not implemented") } +func (m *mockLayer) MediaType() (types.MediaType, error) { panic("not implemented") } diff --git a/pkg/oci/remote/signatures.go b/pkg/oci/remote/signatures.go index 635b5e9e07c..f8a53e0f8ea 100644 --- a/pkg/oci/remote/signatures.go +++ b/pkg/oci/remote/signatures.go @@ -27,6 +27,8 @@ import ( "github.com/sigstore/cosign/v2/pkg/oci/internal/signature" ) +const maxLayers = 1000 + // Signatures fetches the signatures image represented by the named reference. // If the tag is not found, this returns an empty oci.Signatures. func Signatures(ref name.Reference, opts ...Option) (oci.Signatures, error) { @@ -58,6 +60,10 @@ func (s *sigs) Get() ([]oci.Signature, error) { if err != nil { return nil, err } + numLayers := int64(len(m.Layers)) + if numLayers > maxLayers { + return nil, oci.NewMaxLayersExceeded(numLayers, maxLayers) + } signatures := make([]oci.Signature, 0, len(m.Layers)) for _, desc := range m.Layers { layer, err := s.Image.LayerByDigest(desc.Digest) diff --git a/pkg/oci/remote/signatures_test.go b/pkg/oci/remote/signatures_test.go index 91e7c7a05ea..c2367003361 100644 --- a/pkg/oci/remote/signatures_test.go +++ b/pkg/oci/remote/signatures_test.go @@ -22,6 +22,7 @@ import ( "github.com/google/go-containerregistry/pkg/name" v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/fake" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/google/go-containerregistry/pkg/v1/remote/transport" ) @@ -75,4 +76,25 @@ func TestSignaturesErrors(t *testing.T) { t.Fatalf("Signatures() = %v, wanted %v", err, want) } }) + + t.Run("too many layers", func(t *testing.T) { + remoteImage = func(_ name.Reference, _ ...remote.Option) (v1.Image, error) { + return &fake.FakeImage{ + ManifestStub: func() (*v1.Manifest, error) { + return &v1.Manifest{ + Layers: make([]v1.Descriptor, 10000), + }, nil + }, + }, nil + } + sigs, err := Signatures(name.MustParseReference("gcr.io/distroless/static:sha256-deadbeef.sig")) + if err != nil { + t.Fatalf("Signatures() = %v", err) + } + want := errors.New("number of layers (10000) exceeded the limit (1000)") + _, err = sigs.Get() + if err == nil || want.Error() != err.Error() { + t.Fatalf("Get() = %v", err) + } + }) } diff --git a/pkg/oci/signature/layer.go b/pkg/oci/signature/layer.go index 176fe3cefdc..d92af61c30a 100644 --- a/pkg/oci/signature/layer.go +++ b/pkg/oci/signature/layer.go @@ -24,6 +24,7 @@ import ( "strings" v1 "github.com/google/go-containerregistry/pkg/v1" + payloadsize "github.com/sigstore/cosign/v2/internal/pkg/cosign/payload/size" "github.com/sigstore/cosign/v2/pkg/cosign/bundle" "github.com/sigstore/cosign/v2/pkg/oci" "github.com/sigstore/sigstore/pkg/cryptoutils" @@ -58,6 +59,14 @@ func (s *sigLayer) Annotations() (map[string]string, error) { // Payload implements oci.Signature func (s *sigLayer) Payload() ([]byte, error) { + size, err := s.Layer.Size() + if err != nil { + return nil, err + } + err = payloadsize.CheckSize(uint64(size)) + if err != nil { + return nil, err + } // Compressed is a misnomer here, we just want the raw bytes from the registry. r, err := s.Layer.Compressed() if err != nil { diff --git a/pkg/oci/signature/layer_test.go b/pkg/oci/signature/layer_test.go index 9a610d35ca9..e88157d2150 100644 --- a/pkg/oci/signature/layer_test.go +++ b/pkg/oci/signature/layer_test.go @@ -20,6 +20,8 @@ import ( "encoding/base64" "errors" "fmt" + "io" + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -292,6 +294,7 @@ func TestSignatureWithTSAAnnotation(t *testing.T) { tests := []struct { name string l *sigLayer + env map[string]string wantPayloadErr error wantSig string wantSigErr error @@ -397,10 +400,39 @@ func TestSignatureWithTSAAnnotation(t *testing.T) { wantBundle: &bundle.RFC3161Timestamp{ SignedRFC3161Timestamp: mustDecode("MEUCIQClUkUqZNf+6dxBc/pxq22JIluTB7Kmip1G0FIF5E0C1wIgLqXm+IM3JYW/P/qjMZSXW+J8bt5EOqNfe3R+0A9ooFE="), }, + }, { + name: "payload size exceeds default limit", + l: &sigLayer{ + Layer: &mockLayer{size: 134217728 + 42}, // 128MiB + 42 bytes + }, + wantPayloadErr: errors.New("size of layer (134217770) exceeded the limit (134217728)"), + }, { + name: "payload size exceeds overridden limit", + l: &sigLayer{ + Layer: &mockLayer{size: 1000000000 + 42}, // 1GB + 42 bytes + }, + env: map[string]string{"COSIGN_MAX_ATTACHMENT_SIZE": "1GB"}, + wantPayloadErr: errors.New("size of layer (1000000042) exceeded the limit (1000000000)"), + }, { + name: "payload size is within overridden limit", + l: &sigLayer{ + Layer: layer, + desc: v1.Descriptor{ + Digest: digest, + Annotations: map[string]string{ + sigkey: "blah", + }, + }, + }, + env: map[string]string{"COSIGN_MAX_ATTACHMENT_SIZE": "5KB"}, + wantSig: "blah", }} for _, test := range tests { t.Run(test.name, func(t *testing.T) { + for k, v := range test.env { + t.Setenv(k, v) + } b, err := test.l.Payload() switch { case (err != nil) != (test.wantPayloadErr != nil): @@ -414,6 +446,9 @@ func TestSignatureWithTSAAnnotation(t *testing.T) { t.Errorf("v1.SHA256() = %v, wanted %v", got, want) } } + if err != nil { + return + } switch got, err := test.l.Base64Signature(); { case (err != nil) != (test.wantSigErr != nil): @@ -453,3 +488,20 @@ func TestSignatureWithTSAAnnotation(t *testing.T) { }) } } + +type mockLayer struct { + size int64 +} + +func (m *mockLayer) Size() (int64, error) { + return m.size, nil +} + +func (m *mockLayer) Compressed() (io.ReadCloser, error) { + return io.NopCloser(strings.NewReader("data")), nil +} + +func (m *mockLayer) Digest() (v1.Hash, error) { panic("not implemented") } +func (m *mockLayer) DiffID() (v1.Hash, error) { panic("not implemented") } +func (m *mockLayer) Uncompressed() (io.ReadCloser, error) { panic("not implemented") } +func (m *mockLayer) MediaType() (types.MediaType, error) { panic("not implemented") } diff --git a/pkg/oci/static/file.go b/pkg/oci/static/file.go index c3435be4d59..18ec65c3af8 100644 --- a/pkg/oci/static/file.go +++ b/pkg/oci/static/file.go @@ -22,6 +22,7 @@ import ( "github.com/google/go-containerregistry/pkg/v1/empty" "github.com/google/go-containerregistry/pkg/v1/mutate" "github.com/google/go-containerregistry/pkg/v1/types" + payloadsize "github.com/sigstore/cosign/v2/internal/pkg/cosign/payload/size" "github.com/sigstore/cosign/v2/internal/pkg/now" "github.com/sigstore/cosign/v2/pkg/oci" "github.com/sigstore/cosign/v2/pkg/oci/signed" @@ -82,6 +83,14 @@ func (f *file) FileMediaType() (types.MediaType, error) { // Payload implements oci.File func (f *file) Payload() ([]byte, error) { + size, err := f.layer.Size() + if err != nil { + return nil, err + } + err = payloadsize.CheckSize(uint64(size)) + if err != nil { + return nil, err + } rc, err := f.layer.Uncompressed() if err != nil { return nil, err diff --git a/pkg/oci/static/file_test.go b/pkg/oci/static/file_test.go index 0866f25deda..226335b4af8 100644 --- a/pkg/oci/static/file_test.go +++ b/pkg/oci/static/file_test.go @@ -16,6 +16,7 @@ package static import ( + "errors" "io" "strings" "testing" @@ -27,7 +28,7 @@ import ( func TestNewFile(t *testing.T) { payload := "this is the content!" - file, err := NewFile([]byte(payload), WithLayerMediaType("foo"), WithAnnotations(map[string]string{"foo": "bar"})) + f, err := NewFile([]byte(payload), WithLayerMediaType("foo"), WithAnnotations(map[string]string{"foo": "bar"})) if err != nil { t.Fatalf("NewFile() = %v", err) } @@ -38,7 +39,7 @@ func TestNewFile(t *testing.T) { t.Fatalf("NewFile() = %v", err) } - layers, err := file.Layers() + layers, err := f.Layers() if err != nil { t.Fatalf("Layers() = %v", err) } else if got, want := len(layers), 1; got != want { @@ -59,7 +60,7 @@ func TestNewFile(t *testing.T) { t.Run("check media type", func(t *testing.T) { wantMT := types.MediaType("foo") - gotMT, err := file.FileMediaType() + gotMT, err := f.FileMediaType() if err != nil { t.Fatalf("MediaType() = %v", err) } @@ -118,7 +119,7 @@ func TestNewFile(t *testing.T) { t.Errorf("Uncompressed() = %s, wanted %s", got, want) } - gotPayload, err := file.Payload() + gotPayload, err := f.Payload() if err != nil { t.Fatalf("Payload() = %v", err) } @@ -128,7 +129,7 @@ func TestNewFile(t *testing.T) { }) t.Run("check date", func(t *testing.T) { - fileCfg, err := file.ConfigFile() + fileCfg, err := f.ConfigFile() if err != nil { t.Fatalf("ConfigFile() = %v", err) } @@ -145,7 +146,7 @@ func TestNewFile(t *testing.T) { }) t.Run("check annotations", func(t *testing.T) { - m, err := file.Manifest() + m, err := f.Manifest() if err != nil { t.Fatalf("Manifest() = %v", err) } @@ -154,4 +155,39 @@ func TestNewFile(t *testing.T) { t.Errorf("Annotations = %s, wanted %s", got, want) } }) + + t.Run("huge file payload", func(t *testing.T) { + // default limit + f := file{ + layer: &mockLayer{200000000}, + } + want := errors.New("size of layer (200000000) exceeded the limit (134217728)") + _, err = f.Payload() + if err == nil || want.Error() != err.Error() { + t.Errorf("Payload() = %v, wanted %v", err, want) + } + // override limit + t.Setenv("COSIGN_MAX_ATTACHMENT_SIZE", "512MiB") + _, err = f.Payload() + if err != nil { + t.Errorf("Payload() = %v, wanted nil", err) + } + }) } + +type mockLayer struct { + size int64 +} + +func (m *mockLayer) Size() (int64, error) { + return m.size, nil +} + +func (m *mockLayer) Uncompressed() (io.ReadCloser, error) { + return io.NopCloser(strings.NewReader("data")), nil +} + +func (m *mockLayer) Digest() (v1.Hash, error) { panic("not implemented") } +func (m *mockLayer) DiffID() (v1.Hash, error) { panic("not implemented") } +func (m *mockLayer) Compressed() (io.ReadCloser, error) { panic("not implemented") } +func (m *mockLayer) MediaType() (types.MediaType, error) { panic("not implemented") }