Skip to content

Commit

Permalink
refactors.
Browse files Browse the repository at this point in the history 
Signed-off-by: ianhundere <138915+ianhundere@users.noreply.github.com>

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 <colleenmurphy@google.com>

* 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 <colleenmurphy@google.com>

---------

Signed-off-by: Colleen Murphy <colleenmurphy@google.com>

adds copyright, changes vars to consts, and cert-chain file path, TSACertChainPath.

Signed-off-by: ianhundere <138915+ianhundere@users.noreply.github.com>
  • Loading branch information
@ianhundere
ianhundere committed May 31, 2024
1 parent 550dbf9 commit c1bef74
Show file tree
Hide file tree
Showing 7 changed files with 210 additions and 50 deletions.
46 changes: 30 additions & 16 deletions cmd/cosign/cli/verify/verify_attestation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package verify

import (
"context"
"crypto/x509"
"errors"
"flag"
"fmt"
Expand All @@ -28,7 +29,6 @@ import (
"github.com/sigstore/cosign/v2/cmd/cosign/cli/fulcio"
"github.com/sigstore/cosign/v2/cmd/cosign/cli/options"
"github.com/sigstore/cosign/v2/cmd/cosign/cli/rekor"
"github.com/sigstore/cosign/v2/internal/pkg/cosign/tsa"
"github.com/sigstore/cosign/v2/internal/ui"
"github.com/sigstore/cosign/v2/pkg/cosign"
"github.com/sigstore/cosign/v2/pkg/cosign/cue"
Expand All @@ -38,6 +38,7 @@ import (
"github.com/sigstore/cosign/v2/pkg/oci"
"github.com/sigstore/cosign/v2/pkg/policy"
sigs "github.com/sigstore/cosign/v2/pkg/signature"
"github.com/sigstore/sigstore/pkg/cryptoutils"
)

// VerifyAttestationCommand verifies a signature on a supplied container image
Expand Down Expand Up @@ -119,28 +120,41 @@ func (c *VerifyAttestationCommand) Exec(ctx context.Context, images []string) (e
}

if c.TSACertChainPath != "" {
_, err := os.Stat(c.TSACertChainPath)
leaves, intermediates, roots, err := cosign.GetTSACerts(ctx)
if err != nil {
return fmt.Errorf("unable to open timestamp certificate chain file '%s: %w", c.TSACertChainPath, err)
}
// TODO: Add support for TUF certificates.
pemBytes, err := os.ReadFile(filepath.Clean(c.TSACertChainPath))
if err != nil {
return fmt.Errorf("error reading certification chain path file: %w", err)
}

leaves, intermediates, roots, err := tsa.SplitPEMCertificateChain(pemBytes)
if err != nil {
return fmt.Errorf("error splitting certificates: %w", err)
return fmt.Errorf("unable to get TSA certificates: %w", err)
}
if len(leaves) > 1 {
return fmt.Errorf("certificate chain must contain at most one TSA certificate")
}
if len(leaves) == 1 {
co.TSACertificate = leaves[0]
certs, err := cryptoutils.UnmarshalCertificatesFromPEM(leaves[0])
if err != nil {
return fmt.Errorf("error parsing TSA certificate: %w", err)
}
if len(certs) != 1 {
return fmt.Errorf("expected a single TSA certificate, got %d", len(certs))
}
co.TSACertificate = certs[0]
}
var allIntermediateCerts []*x509.Certificate
for _, intermediate := range intermediates {
certs, err := cryptoutils.UnmarshalCertificatesFromPEM(intermediate)
if err != nil {
return fmt.Errorf("error parsing TSA intermediate certificates: %w", err)
}
allIntermediateCerts = append(allIntermediateCerts, certs...)
}
co.TSAIntermediateCertificates = allIntermediateCerts
var allRootCerts []*x509.Certificate
for _, root := range roots {
certs, err := cryptoutils.UnmarshalCertificatesFromPEM(root)
if err != nil {
return fmt.Errorf("error parsing TSA root certificates: %w", err)
}
allRootCerts = append(allRootCerts, certs...)
}
co.TSAIntermediateCertificates = intermediates
co.TSARootCertificates = roots
co.TSARootCertificates = allRootCerts
}
if !c.IgnoreTlog {
if c.RekorURL != "" {
Expand Down
44 changes: 28 additions & 16 deletions cmd/cosign/cli/verify/verify_blob.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import (
"github.com/sigstore/cosign/v2/cmd/cosign/cli/fulcio"
"github.com/sigstore/cosign/v2/cmd/cosign/cli/options"
"github.com/sigstore/cosign/v2/cmd/cosign/cli/rekor"
"github.com/sigstore/cosign/v2/internal/pkg/cosign/tsa"
"github.com/sigstore/cosign/v2/internal/ui"
"github.com/sigstore/cosign/v2/pkg/blob"
"github.com/sigstore/cosign/v2/pkg/cosign"
Expand Down Expand Up @@ -116,28 +115,41 @@ func (c *VerifyBlobCmd) Exec(ctx context.Context, blobRef string) error {
return fmt.Errorf("timestamp-certificate-chain is required to validate a RFC3161 timestamp")
}
if c.KeyOpts.TSACertChainPath != "" {
_, err := os.Stat(c.KeyOpts.TSACertChainPath)
leaves, intermediates, roots, err := cosign.GetTSACerts(ctx)
if err != nil {
return fmt.Errorf("unable to open timestamp certificate chain file '%s: %w", c.KeyOpts.TSACertChainPath, err)
}
// TODO: Add support for TUF certificates.
pemBytes, err := os.ReadFile(filepath.Clean(c.KeyOpts.TSACertChainPath))
if err != nil {
return fmt.Errorf("error reading certification chain path file: %w", err)
}

leaves, intermediates, roots, err := tsa.SplitPEMCertificateChain(pemBytes)
if err != nil {
return fmt.Errorf("error splitting certificates: %w", err)
return fmt.Errorf("unable to get TSA certificates: %w", err)
}
if len(leaves) > 1 {
return fmt.Errorf("certificate chain must contain at most one TSA certificate")
}
if len(leaves) == 1 {
co.TSACertificate = leaves[0]
certs, err := cryptoutils.UnmarshalCertificatesFromPEM(leaves[0])
if err != nil {
return fmt.Errorf("error parsing TSA certificate: %w", err)
}
if len(certs) != 1 {
return fmt.Errorf("expected a single TSA certificate, got %d", len(certs))
}
co.TSACertificate = certs[0]
}
var allIntermediateCerts []*x509.Certificate
for _, intermediate := range intermediates {
certs, err := cryptoutils.UnmarshalCertificatesFromPEM(intermediate)
if err != nil {
return fmt.Errorf("error parsing TSA intermediate certificates: %w", err)
}
allIntermediateCerts = append(allIntermediateCerts, certs...)
}
co.TSAIntermediateCertificates = allIntermediateCerts
var allRootCerts []*x509.Certificate
for _, root := range roots {
certs, err := cryptoutils.UnmarshalCertificatesFromPEM(root)
if err != nil {
return fmt.Errorf("error parsing TSA root certificates: %w", err)
}
allRootCerts = append(allRootCerts, certs...)
}
co.TSAIntermediateCertificates = intermediates
co.TSARootCertificates = roots
co.TSARootCertificates = allRootCerts
}

if !c.IgnoreTlog {
Expand Down
46 changes: 29 additions & 17 deletions cmd/cosign/cli/verify/verify_blob_attestation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import (
"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"
"github.com/sigstore/cosign/v2/pkg/cosign/bundle"
Expand Down Expand Up @@ -144,28 +143,41 @@ func (c *VerifyBlobAttestationCommand) Exec(ctx context.Context, artifactPath st
return fmt.Errorf("timestamp-cert-chain is required to validate a rfc3161 timestamp bundle")
}
if c.KeyOpts.TSACertChainPath != "" {
_, err := os.Stat(c.TSACertChainPath)
leaves, intermediates, roots, err := cosign.GetTSACerts(ctx)
if err != nil {
return fmt.Errorf("unable to open timestamp certificate chain file: %w", err)
}
// TODO: Add support for TUF certificates.
pemBytes, err := os.ReadFile(filepath.Clean(c.TSACertChainPath))
if err != nil {
return fmt.Errorf("error reading certification chain path file: %w", err)
}

leaves, intermediates, roots, err := tsa.SplitPEMCertificateChain(pemBytes)
if err != nil {
return fmt.Errorf("error splitting certificates: %w", err)
return fmt.Errorf("unable to get TSA certificates: %w", err)
}
if len(leaves) > 1 {
return fmt.Errorf("certificate chain must contain at most one TSA certificate")
}
if len(leaves) == 1 {
co.TSACertificate = leaves[0]
certs, err := cryptoutils.UnmarshalCertificatesFromPEM(leaves[0])
if err != nil {
return fmt.Errorf("error parsing TSA certificate: %w", err)
}
if len(certs) != 1 {
return fmt.Errorf("expected a single TSA certificate, got %d", len(certs))
}
co.TSACertificate = certs[0]
}
co.TSAIntermediateCertificates = intermediates
co.TSARootCertificates = roots
var allIntermediateCerts []*x509.Certificate
for _, intermediate := range intermediates {
certs, err := cryptoutils.UnmarshalCertificatesFromPEM(intermediate)
if err != nil {
return fmt.Errorf("error parsing TSA intermediate certificates: %w", err)
}
allIntermediateCerts = append(allIntermediateCerts, certs...)
}
co.TSAIntermediateCertificates = allIntermediateCerts
var allRootCerts []*x509.Certificate
for _, root := range roots {
certs, err := cryptoutils.UnmarshalCertificatesFromPEM(root)
if err != nil {
return fmt.Errorf("error parsing TSA root certificates: %w", err)
}
allRootCerts = append(allRootCerts, certs...)
}
co.TSARootCertificates = allRootCerts
}

if !c.IgnoreTlog {
Expand Down Expand Up @@ -356,4 +368,4 @@ func (c *VerifyBlobAttestationCommand) Exec(ctx context.Context, artifactPath st

fmt.Fprintln(os.Stderr, "Verified OK")
return nil
}
}
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ require (
github.com/secure-systems-lab/go-securesystemslib v0.8.0
github.com/sigstore/fulcio v1.4.5
github.com/sigstore/rekor v1.3.6
github.com/sigstore/sigstore v1.8.3
github.com/sigstore/sigstore v1.8.4
github.com/sigstore/sigstore/pkg/signature/kms/aws v1.8.3
github.com/sigstore/sigstore/pkg/signature/kms/azure v1.8.3
github.com/sigstore/sigstore/pkg/signature/kms/gcp v1.8.3
Expand Down
11 changes: 11 additions & 0 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,7 @@ github.com/go-quicktest/qt v1.101.0 h1:O1K29Txy5P2OK0dGo59b7b0LR6wKfIhttaAhHUyn7
github.com/go-quicktest/qt v1.101.0/go.mod h1:14Bz/f7NwaXPtdYEgzsx46kqSxVwTbzVZsDC26tQJow=
github.com/go-rod/rod v0.114.7 h1:h4pimzSOUnw7Eo41zdJA788XsawzHjJMyzCE3BrBww0=
github.com/go-rod/rod v0.114.7/go.mod h1:aiedSEFg5DwG/fnNbUOTPMTTWX3MRj6vIs/a684Mthw=
github.com/go-rod/rod v0.116.0 h1:ypRryjTys3EnqHskJ/TdgodFMvXV0EHvmy4bSkKZgHM=
github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0/go.mod h1:fyg7847qk6SyHyPtNmDHnmrv/HOrqktSC+C9fM+CJOE=
github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 h1:tfuBGBXKqDEevZMzYi5KSi8KkcZtzBcTgAUUtapy0OI=
github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572/go.mod h1:9Pwr4B2jHnOSGXyyzV8ROjYa2ojvAY6HCGYYfMoC3Ls=
Expand Down Expand Up @@ -607,6 +608,8 @@ github.com/sigstore/rekor v1.3.6 h1:QvpMMJVWAp69a3CHzdrLelqEqpTM3ByQRt5B5Kspbi8=
github.com/sigstore/rekor v1.3.6/go.mod h1:JDTSNNMdQ/PxdsS49DJkJ+pRJCO/83nbR5p3aZQteXc=
github.com/sigstore/sigstore v1.8.3 h1:G7LVXqL+ekgYtYdksBks9B38dPoIsbscjQJX/MGWkA4=
github.com/sigstore/sigstore v1.8.3/go.mod h1:mqbTEariiGA94cn6G3xnDiV6BD8eSLdL/eA7bvJ0fVs=
github.com/sigstore/sigstore v1.8.4 h1:g4ICNpiENFnWxjmBzBDWUn62rNFeny/P77HUC8da32w=
github.com/sigstore/sigstore v1.8.4/go.mod h1:1jIKtkTFEeISen7en+ZPWdDHazqhxco/+v9CNjc7oNg=
github.com/sigstore/sigstore/pkg/signature/kms/aws v1.8.3 h1:LTfPadUAo+PDRUbbdqbeSl2OuoFQwUFTnJ4stu+nwWw=
github.com/sigstore/sigstore/pkg/signature/kms/aws v1.8.3/go.mod h1:QV/Lxlxm0POyhfyBtIbTWxNeF18clMlkkyL9mu45y18=
github.com/sigstore/sigstore/pkg/signature/kms/azure v1.8.3 h1:xgbPRCr2npmmsuVVteJqi/ERw9+I13Wou7kq0Yk4D8g=
Expand Down Expand Up @@ -750,6 +753,8 @@ golang.org/x/crypto v0.6.0/go.mod h1:OFC/31mSvZgRz0V1QTNCzfAI1aIRzbiufJtkMIlEp58
golang.org/x/crypto v0.7.0/go.mod h1:pYwdfH91IfpZVANVyUOhSIPZaFoJGxTFbZhFTx+dXZU=
golang.org/x/crypto v0.10.0/go.mod h1:o4eNf7Ede1fv+hwOwZsTHl9EsPFO6q6ZvYR8vYfY45I=
golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU=
golang.org/x/crypto v0.22.0 h1:g1v0xeRhjcugydODzvb3mEM9SQ0HGp9s/nh3COQ/C30=
golang.org/x/crypto v0.22.0/go.mod h1:vr6Su+7cTlO45qkww3VDJlzDn0ctJvRgYbC2NvXHt+M=
golang.org/x/crypto v0.23.0 h1:dIJU/v2J8Mdglj/8rJ6UUOM3Zc9zLZxVZwwxMooUSAI=
golang.org/x/crypto v0.23.0/go.mod h1:CKFgDieR+mRhux2Lsu27y0fO304Db0wZe70UKqHu0v8=
golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
Expand Down Expand Up @@ -792,6 +797,8 @@ golang.org/x/net v0.11.0/go.mod h1:2L/ixqYpgIVXmeoSA/4Lu7BzTG4KIyPIryS4IsOd1oQ=
golang.org/x/net v0.25.0 h1:d/OCCoBEUq33pjydKrGQhw7IlUPI2Oylr+8qLx49kac=
golang.org/x/net v0.25.0/go.mod h1:JkAGAh7GEvH74S6FOH42FLoXpXbE/aqXSrIQjXgsiwM=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
golang.org/x/oauth2 v0.19.0 h1:9+E/EZBCbTLNrbN35fHv/a/d/mOBatymz1zbtQrXpIg=
golang.org/x/oauth2 v0.19.0/go.mod h1:vYi7skDa1x015PmRRYZ7+s1cWyPgrPiSYRe4rnsexc8=
golang.org/x/oauth2 v0.20.0 h1:4mQdhULixXKP1rwYBW0vAijoXnkTG0BLCDRzfe1idMo=
golang.org/x/oauth2 v0.20.0/go.mod h1:XYTD2NtWslqkgxebSiOHnXEap4TF09sJSc7H1sXbhtI=
golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
Expand Down Expand Up @@ -836,6 +843,8 @@ golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.9.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/sys v0.19.0 h1:q5f1RH2jigJ1MoAWp2KTp3gm5zAGFUTarQZ5U386+4o=
golang.org/x/sys v0.19.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/sys v0.20.0 h1:Od9JTbYCk261bKm4M/mw7AklTlFYIa0bIp9BgSm1S8Y=
golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
Expand All @@ -846,6 +855,8 @@ golang.org/x/term v0.6.0/go.mod h1:m6U89DPEgQRMq3DNkDClhWw02AUbt2daBVO4cn4Hv9U=
golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo=
golang.org/x/term v0.9.0/go.mod h1:M6DEAAIenWoTxdKrOltXcmDY3rSplQUkrvaDU5FcQyo=
golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk=
golang.org/x/term v0.19.0 h1:+ThwsDv+tYfnJFhF4L8jITxu1tdTWRTZpdsWgEgjL6Q=
golang.org/x/term v0.19.0/go.mod h1:2CuTdWZ7KHSQwUzKva0cbMg6q2DMI3Mmxp+gKJbskEk=
golang.org/x/term v0.20.0 h1:VnkxpohqXaOBYJtBmEppKUG6mXpi+4O6purfc2+sMhw=
golang.org/x/term v0.20.0/go.mod h1:8UkIAJTvZgivsXaD6/pH6U9ecQzZ45awqEOzuCvwpFY=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
Expand Down
7 changes: 7 additions & 0 deletions pkg/cosign/env/env.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ const (
VariableSigstoreRootFile Variable = "SIGSTORE_ROOT_FILE"
VariableSigstoreRekorPublicKey Variable = "SIGSTORE_REKOR_PUBLIC_KEY"
VariableSigstoreIDToken Variable = "SIGSTORE_ID_TOKEN" //nolint:gosec
VariableSigstoreTSACertificateFile Variable = "SIGSTORE_TSA_CERTIFICATE_FILE"

// Other external environment variables
VariableGitHubHost Variable = "GITHUB_HOST"
Expand Down Expand Up @@ -138,6 +139,12 @@ var (
Sensitive: false,
External: true,
},
VariableSigstoreTSACertificateFile: {
Description: "path to the TSA certificate file used by Sigstore",
Expects: "path to the TSA certificate file",
Sensitive: false,
External: true,
},

VariableGitHubHost: {
Description: "is URL of the GitHub Enterprise instance",
Expand Down
Loading

0 comments on commit c1bef74

Please sign in to comment.