diff --git a/pkg/commands/compute/deploy.go b/pkg/commands/compute/deploy.go index 506f67c21..6d907e82d 100644 --- a/pkg/commands/compute/deploy.go +++ b/pkg/commands/compute/deploy.go @@ -2,7 +2,6 @@ package compute import ( "bytes" - "crypto/sha512" "errors" "fmt" "io" @@ -10,7 +9,6 @@ import ( "net/http" "os" "path/filepath" - "sort" "strings" "time" @@ -92,7 +90,7 @@ func NewDeployCommand(parent cmd.Registerer, g *global.Data, m manifest.Data) *D // Exec implements the command interface. func (c *DeployCommand) Exec(in io.Reader, out io.Writer) (err error) { - fnActivateTrial, source, serviceID, pkgPath, hashSum, err := setupDeploy(c, out) + fnActivateTrial, source, serviceID, pkgPath, filesHash, err := setupDeploy(c, out) if err != nil { return err } @@ -150,7 +148,7 @@ func (c *DeployCommand) Exec(in io.Reader, out io.Writer) (err error) { } cont, err = processPackage( - c, hashSum, pkgPath, serviceID, serviceVersion.Number, spinner, out, + c, filesHash, pkgPath, serviceID, serviceVersion.Number, spinner, out, ) if err != nil { return err @@ -217,7 +215,7 @@ func validStatusCodeRange(status int) bool { func setupDeploy(c *DeployCommand, out io.Writer) ( fnActivateTrial activator, source manifest.Source, - serviceID, pkgPath, hashSum string, + serviceID, pkgPath, filesHash string, err error, ) { defaultActivator := func(customerID string) error { return nil } @@ -235,7 +233,7 @@ func setupDeploy(c *DeployCommand, out io.Writer) ( cmd.DisplayServiceID(serviceID, flag, source, out) } - pkgPath, hashSum, err = validatePackage(c.Manifest, c.Package, c.Globals.Verbose(), c.Globals.ErrLog, out) + pkgPath, filesHash, err = validatePackage(c.Manifest, c.Package, c.Globals.Verbose(), c.Globals.ErrLog, out) if err != nil { return defaultActivator, source, serviceID, "", "", err } @@ -243,7 +241,7 @@ func setupDeploy(c *DeployCommand, out io.Writer) ( endpoint, _ := c.Globals.Endpoint() fnActivateTrial = preconfigureActivateTrial(endpoint, token, c.Globals.HTTPClient) - return fnActivateTrial, source, serviceID, pkgPath, hashSum, err + return fnActivateTrial, source, serviceID, pkgPath, filesHash, err } // validatePackage short-circuits the deploy command if the user hasn't first @@ -257,21 +255,21 @@ func validatePackage( verbose bool, errLog fsterr.LogInterface, out io.Writer, -) (pkgPath, hashSum string, err error) { +) (pkgPath, filesHash string, err error) { err = data.File.ReadError() if err != nil { if packageFlag == "" { if errors.Is(err, os.ErrNotExist) { err = fsterr.ErrReadingManifest } - return pkgPath, hashSum, err + return pkgPath, filesHash, err } // NOTE: Before returning the manifest read error, we'll attempt to read // the manifest from within the given package archive. err := readManifestFromPackageArchive(&data, packageFlag, verbose, out) if err != nil { - return pkgPath, hashSum, err + return pkgPath, filesHash, err } } @@ -281,7 +279,7 @@ func validatePackage( errLog.AddWithContext(err, map[string]any{ "Package path": packageFlag, }) - return pkgPath, hashSum, err + return pkgPath, filesHash, err } pkgSize, err := packageSize(pkgPath) @@ -289,14 +287,14 @@ func validatePackage( errLog.AddWithContext(err, map[string]any{ "Package path": pkgPath, }) - return pkgPath, hashSum, fsterr.RemediationError{ + return pkgPath, filesHash, fsterr.RemediationError{ Inner: fmt.Errorf("error reading package size: %w", err), Remediation: "Run `fastly compute build` to produce a Compute@Edge package, alternatively use the --package flag to reference a package outside of the current project.", } } if pkgSize > MaxPackageSize { - return pkgPath, hashSum, fsterr.RemediationError{ + return pkgPath, filesHash, fsterr.RemediationError{ Inner: fmt.Errorf("package size is too large (%d bytes)", pkgSize), Remediation: fsterr.PackageSizeRemediation, } @@ -319,15 +317,15 @@ func validatePackage( "Package path": pkgPath, "Package size": pkgSize, }) - return pkgPath, hashSum, err + return pkgPath, filesHash, err } - hashSum, err = getHashSum(contents) + filesHash, err = getFilesHash(pkgPath) if err != nil { return pkgPath, "", err } - return pkgPath, hashSum, nil + return pkgPath, filesHash, nil } // readManifestFromPackageArchive extracts the manifest file from the given @@ -782,16 +780,16 @@ func checkServiceID(serviceID string, client api.Interface) error { return nil } -// pkgCompare compares the local package hashsum against the existing service +// pkgCompare compares the local package files hash against the existing service // package version and exits early with message if identical. -func pkgCompare(client api.Interface, serviceID string, version int, hashSum string, out io.Writer) (bool, error) { +func pkgCompare(client api.Interface, serviceID string, version int, filesHash string, out io.Writer) (bool, error) { p, err := client.GetPackage(&fastly.GetPackageInput{ ServiceID: serviceID, ServiceVersion: version, }) if err == nil { - if hashSum == p.Metadata.HashSum { + if filesHash == p.Metadata.FilesHash { text.Info(out, "Skipping package deployment, local and service version are identical. (service %v, version %v) ", serviceID, version) return false, nil } @@ -800,22 +798,6 @@ func pkgCompare(client api.Interface, serviceID string, version int, hashSum str return true, nil } -// getHashSum creates a SHA 512 hash from the given file contents in a specific order. -func getHashSum(contents map[string]*bytes.Buffer) (hash string, err error) { - h := sha512.New() - keys := make([]string, 0, len(contents)) - for k := range contents { - keys = append(keys, k) - } - sort.Strings(keys) - for _, fname := range keys { - if _, err := io.Copy(h, contents[fname]); err != nil { - return "", err - } - } - return fmt.Sprintf("%x", h.Sum(nil)), nil -} - // pkgUpload uploads the package to the specified service and version. func pkgUpload(spinner text.Spinner, client api.Interface, serviceID string, version int, path string) error { err := spinner.Start() @@ -1126,12 +1108,12 @@ func processSetupCreation( func processPackage( c *DeployCommand, - hashSum, pkgPath, serviceID string, + filesHash, pkgPath, serviceID string, serviceVersion int, spinner text.Spinner, out io.Writer, ) (cont bool, err error) { - cont, err = pkgCompare(c.Globals.APIClient, serviceID, serviceVersion, hashSum, out) + cont, err = pkgCompare(c.Globals.APIClient, serviceID, serviceVersion, filesHash, out) if err != nil { c.Globals.ErrLog.AddWithContext(err, map[string]any{ "Package path": pkgPath, diff --git a/pkg/commands/compute/deploy_test.go b/pkg/commands/compute/deploy_test.go index 880943fd0..a218ac54c 100644 --- a/pkg/commands/compute/deploy_test.go +++ b/pkg/commands/compute/deploy_test.go @@ -1970,7 +1970,8 @@ func getPackageIdentical(i *fastly.GetPackageInput) (*fastly.Package, error) { ServiceID: i.ServiceID, ServiceVersion: i.ServiceVersion, Metadata: fastly.PackageMetadata{ - HashSum: "bf634ccf8be5c8417cf562466ece47ea61056ddeb07273a3d861e8ad757ed3577bc182006d04093c301467cadfd2b1805eedebd1e7cfa0404c723680f2dbc01e", + FilesHash: "d8786807216a37608ecd0bc2357c86f883faad89043141f0a147f2c186ce0212333d31229399c131539205908f5cf0884ea64552782544ff9b27416cd5b996b2", + HashSum: "bf634ccf8be5c8417cf562466ece47ea61056ddeb07273a3d861e8ad757ed3577bc182006d04093c301467cadfd2b1805eedebd1e7cfa0404c723680f2dbc01e", }, }, nil } diff --git a/pkg/commands/compute/hashfiles.go b/pkg/commands/compute/hashfiles.go index 180a8ce6a..12698d178 100644 --- a/pkg/commands/compute/hashfiles.go +++ b/pkg/commands/compute/hashfiles.go @@ -3,16 +3,14 @@ package compute import ( "archive/tar" "bytes" - "compress/gzip" "crypto/sha512" - "errors" "fmt" "io" - "os" "path/filepath" "sort" "github.com/kennygrant/sanitize" + "github.com/mholt/archiver/v3" "github.com/fastly/cli/pkg/cmd" "github.com/fastly/cli/pkg/global" @@ -68,27 +66,9 @@ func (c *HashFilesCommand) Exec(in io.Reader, out io.Writer) (err error) { } } - var r io.Reader - // G304 (CWE-22): Potential file inclusion via variable - // #nosec - r, err = os.Open(pkg) + hash, err := getFilesHash(pkg) if err != nil { - return fmt.Errorf("failed to open package '%s': %w", pkg, err) - } - - zr, err := gzip.NewReader(r) - if err != nil { - return fmt.Errorf("failed to create a gzip reader: %w", err) - } - - files, err := c.ReadFilesFromPackage(tar.NewReader(zr)) - if err != nil { - return fmt.Errorf("failed to read files within the package: %w", err) - } - - hash, err := c.GetFilesHash(files) - if err != nil { - return fmt.Errorf("failed to generate hash from package files: %w", err) + return err } text.Output(out, hash) @@ -104,48 +84,21 @@ func (c *HashFilesCommand) Build(in io.Reader, out io.Writer) error { return c.buildCmd.Exec(in, output) } -// ReadFilesFromPackage reads all files within the provided package tar and -// generates a map data structure where the key is the filename and the value is -// the file contents. -func (c *HashFilesCommand) ReadFilesFromPackage(tr *tar.Reader) (map[string]*bytes.Buffer, error) { - // Store the content of every file within the package. +// getFilesHash returns a hash of all the files in the package in sorted filename order. +func getFilesHash(pkgPath string) (string, error) { contents := make(map[string]*bytes.Buffer) - - // Track overall package size. - var pkgSize int64 - - for { - hdr, err := tr.Next() - if err == io.EOF { - break - } - if err != nil { - return nil, err - } - - // Avoids G110: Potential DoS vulnerability via decompression bomb (gosec). - pkgSize += hdr.Size - if pkgSize > MaxPackageSize { - return nil, errors.New("package size exceeded 100MB limit") - } - - if hdr.Typeflag != tar.TypeReg { - continue - } - - contents[hdr.Name] = &bytes.Buffer{} - - _, err = io.CopyN(contents[hdr.Name], tr, hdr.Size) - if err != nil { - return nil, err + if err := validate(pkgPath, func(f archiver.File) error { + // This is safe to do - we already verified it in validate(). + filename := f.Header.(*tar.Header).Name + contents[filename] = &bytes.Buffer{} + if _, err := io.Copy(contents[filename], f); err != nil { + return fmt.Errorf("error reading %s: %w", filename, err) } + return nil + }); err != nil { + return "", err } - return contents, nil -} - -// GetFilesHash returns a hash of all the filecontent in sorted filename order. -func (c *HashFilesCommand) GetFilesHash(contents map[string]*bytes.Buffer) (string, error) { keys := make([]string, 0, len(contents)) for k := range contents { keys = append(keys, k) @@ -154,7 +107,7 @@ func (c *HashFilesCommand) GetFilesHash(contents map[string]*bytes.Buffer) (stri h := sha512.New() for _, fname := range keys { if _, err := io.Copy(h, contents[fname]); err != nil { - return "", err + return "", fmt.Errorf("failed to generate hash from package files: %w", err) } } return fmt.Sprintf("%x", h.Sum(nil)), nil diff --git a/pkg/commands/compute/hashsum.go b/pkg/commands/compute/hashsum.go index bf4fde346..9cdfaae4b 100644 --- a/pkg/commands/compute/hashsum.go +++ b/pkg/commands/compute/hashsum.go @@ -1,8 +1,10 @@ package compute import ( + "crypto/sha512" "fmt" "io" + "os" "github.com/fastly/cli/pkg/cmd" fsterr "github.com/fastly/cli/pkg/errors" @@ -48,7 +50,7 @@ func (c *HashsumCommand) Exec(in io.Reader, out io.Writer) (err error) { } } - _, hashSum, err := validatePackage(c.Manifest, c.Package, c.Globals.Verbose(), c.Globals.ErrLog, out) + pkgPath, _, err := validatePackage(c.Manifest, c.Package, c.Globals.Verbose(), c.Globals.ErrLog, out) if err != nil { var skipBuildMsg string if c.SkipBuild { @@ -64,6 +66,11 @@ func (c *HashsumCommand) Exec(in io.Reader, out io.Writer) (err error) { text.Break(out) } + hashSum, err := getHashSum(pkgPath) + if err != nil { + return err + } + text.Output(out, hashSum) return nil } @@ -76,3 +83,27 @@ func (c *HashsumCommand) Build(in io.Reader, out io.Writer) error { } return c.buildCmd.Exec(in, output) } + +// getHashSum returns a hash of the package. +func getHashSum(pkg string) (string, error) { + // gosec flagged this: + // G304 (CWE-22): Potential file inclusion via variable + // Disabling as we trust the source of the filepath variable. + /* #nosec */ + f, err := os.Open(pkg) + if err != nil { + return "", err + } + + h := sha512.New() + if _, err := io.Copy(h, f); err != nil { + f.Close() + return "", err + } + + if err = f.Close(); err != nil { + return "", err + } + + return fmt.Sprintf("%x", h.Sum(nil)), nil +} diff --git a/pkg/commands/compute/validate.go b/pkg/commands/compute/validate.go index edeaed6a6..0e473be4c 100644 --- a/pkg/commands/compute/validate.go +++ b/pkg/commands/compute/validate.go @@ -1,6 +1,7 @@ package compute import ( + "archive/tar" "fmt" "io" "os" @@ -81,19 +82,22 @@ type FileValidator func(archiver.File) error // tar.Read(). // // NOTE: This function is also called by the `deploy` command. -func validate(path string, fileValidator FileValidator) error { +func validate(path string, fileValidator FileValidator) (err error) { file, err := os.Open(filepath.Clean(path)) if err != nil { return fmt.Errorf("error reading package: %w", err) } defer file.Close() // #nosec G307 - tar := archiver.NewTarGz() - err = tar.Open(file, 0) + tr := archiver.NewTarGz() + err = tr.Open(file, 0) if err != nil { return fmt.Errorf("error unarchiving package: %w", err) } - defer tar.Close() + defer tr.Close() + + // Track overall package size + var pkgSize int64 files := map[string]bool{ "fastly.toml": false, @@ -101,7 +105,7 @@ func validate(path string, fileValidator FileValidator) error { } for { - f, err := tar.Read() + f, err := tr.Read() if err == io.EOF { break } @@ -109,6 +113,19 @@ func validate(path string, fileValidator FileValidator) error { return fmt.Errorf("error reading package: %w", err) } + // Avoids G110: Potential DoS vulnerability via decompression bomb (gosec). + pkgSize += f.Size() + if pkgSize > MaxPackageSize { + f.Close() + return fmt.Errorf("package size exceeded 100MB limit") + } + + header, ok := f.Header.(*tar.Header) + if !ok || header.Typeflag != tar.TypeReg { + f.Close() + continue + } + for k := range files { if k == f.Name() { files[k] = true