From 2484df1f81e58165b26f82519cfcd8561d1acd37 Mon Sep 17 00:00:00 2001 From: Devin Carr Date: Wed, 11 Sep 2024 16:00:00 -0700 Subject: [PATCH] TUN-8630: Check checksum of downloaded binary to compare to current for auto-updating In the rare case that the updater downloads the same binary (validated via checksum) we want to make sure that the updater does not attempt to upgrade and restart the cloudflared process. The binaries are equivalent and this would provide no value. However, we are covering this case because there was an errant deployment of cloudflared that reported itself as an older version and was then stuck in an infinite loop attempting to upgrade to the latest version which didn't exist. By making sure that the binary is different ensures that the upgrade will be attempted and cloudflared will be restarted to run the new version. This change only affects cloudflared tunnels running with default settings or `--no-autoupdate=false` which allows cloudflared to auto-update itself in-place. Most distributions that handle package management at the operating system level are not affected by this change. --- cmd/cloudflared/cliutil/build_info.go | 32 ++++++++++++++- cmd/cloudflared/main.go | 2 +- cmd/cloudflared/updater/update.go | 12 +++--- cmd/cloudflared/updater/update_test.go | 6 +++ cmd/cloudflared/updater/workers_service.go | 5 +++ cmd/cloudflared/updater/workers_update.go | 47 +++++++++++----------- 6 files changed, 73 insertions(+), 31 deletions(-) diff --git a/cmd/cloudflared/cliutil/build_info.go b/cmd/cloudflared/cliutil/build_info.go index fff4febf..78ef775a 100644 --- a/cmd/cloudflared/cliutil/build_info.go +++ b/cmd/cloudflared/cliutil/build_info.go @@ -1,7 +1,10 @@ package cliutil import ( + "crypto/sha256" "fmt" + "io" + "os" "runtime" "github.com/rs/zerolog" @@ -13,6 +16,7 @@ type BuildInfo struct { GoArch string `json:"go_arch"` BuildType string `json:"build_type"` CloudflaredVersion string `json:"cloudflared_version"` + Checksum string `json:"checksum"` } func GetBuildInfo(buildType, version string) *BuildInfo { @@ -22,11 +26,12 @@ func GetBuildInfo(buildType, version string) *BuildInfo { GoArch: runtime.GOARCH, BuildType: buildType, CloudflaredVersion: version, + Checksum: currentBinaryChecksum(), } } func (bi *BuildInfo) Log(log *zerolog.Logger) { - log.Info().Msgf("Version %s", bi.CloudflaredVersion) + log.Info().Msgf("Version %s (Checksum %s)", bi.CloudflaredVersion, bi.Checksum) if bi.BuildType != "" { log.Info().Msgf("Built%s", bi.GetBuildTypeMsg()) } @@ -51,3 +56,28 @@ func (bi *BuildInfo) GetBuildTypeMsg() string { func (bi *BuildInfo) UserAgent() string { return fmt.Sprintf("cloudflared/%s", bi.CloudflaredVersion) } + +// FileChecksum opens a file and returns the SHA256 checksum. +func FileChecksum(filePath string) (string, error) { + f, err := os.Open(filePath) + if err != nil { + return "", err + } + defer f.Close() + + h := sha256.New() + if _, err := io.Copy(h, f); err != nil { + return "", err + } + + return fmt.Sprintf("%x", h.Sum(nil)), nil +} + +func currentBinaryChecksum() string { + currentPath, err := os.Executable() + if err != nil { + return "" + } + sum, _ := FileChecksum(currentPath) + return sum +} diff --git a/cmd/cloudflared/main.go b/cmd/cloudflared/main.go index 61d2c41d..b0b93cf8 100644 --- a/cmd/cloudflared/main.go +++ b/cmd/cloudflared/main.go @@ -91,7 +91,7 @@ func main() { tunnel.Init(bInfo, graceShutdownC) // we need this to support the tunnel sub command... access.Init(graceShutdownC, Version) - updater.Init(Version) + updater.Init(bInfo) tracing.Init(Version) token.Init(Version) tail.Init(bInfo) diff --git a/cmd/cloudflared/updater/update.go b/cmd/cloudflared/updater/update.go index 07b382f5..439b129a 100644 --- a/cmd/cloudflared/updater/update.go +++ b/cmd/cloudflared/updater/update.go @@ -14,6 +14,7 @@ import ( "github.com/urfave/cli/v2" "golang.org/x/term" + "github.com/cloudflare/cloudflared/cmd/cloudflared/cliutil" "github.com/cloudflare/cloudflared/config" "github.com/cloudflare/cloudflared/logger" ) @@ -31,7 +32,7 @@ const ( ) var ( - version string + buildInfo *cliutil.BuildInfo BuiltForPackageManager = "" ) @@ -81,8 +82,8 @@ func (uo *UpdateOutcome) noUpdate() bool { return uo.Error == nil && uo.Updated == false } -func Init(v string) { - version = v +func Init(info *cliutil.BuildInfo) { + buildInfo = info } func CheckForUpdate(options updateOptions) (CheckResult, error) { @@ -100,11 +101,12 @@ func CheckForUpdate(options updateOptions) (CheckResult, error) { cfdPath = encodeWindowsPath(cfdPath) } - s := NewWorkersService(version, url, cfdPath, Options{IsBeta: options.isBeta, + s := NewWorkersService(buildInfo.CloudflaredVersion, url, cfdPath, Options{IsBeta: options.isBeta, IsForced: options.isForced, RequestedVersion: options.intendedVersion}) return s.Check() } + func encodeWindowsPath(path string) string { // We do this because Windows allows spaces in directories such as // Program Files but does not allow these directories to be spaced in batch files. @@ -237,7 +239,7 @@ func (a *AutoUpdater) Run(ctx context.Context) error { for { updateOutcome := loggedUpdate(a.log, updateOptions{updateDisabled: !a.configurable.enabled}) if updateOutcome.Updated { - Init(updateOutcome.Version) + buildInfo.CloudflaredVersion = updateOutcome.Version if IsSysV() { // SysV doesn't have a mechanism to keep service alive, we have to restart the process a.log.Info().Msg("Restarting service managed by SysV...") diff --git a/cmd/cloudflared/updater/update_test.go b/cmd/cloudflared/updater/update_test.go index f977b96e..3159f7ab 100644 --- a/cmd/cloudflared/updater/update_test.go +++ b/cmd/cloudflared/updater/update_test.go @@ -9,8 +9,14 @@ import ( "github.com/rs/zerolog" "github.com/stretchr/testify/assert" "github.com/urfave/cli/v2" + + "github.com/cloudflare/cloudflared/cmd/cloudflared/cliutil" ) +func init() { + Init(cliutil.GetBuildInfo("TEST", "TEST")) +} + func TestDisabledAutoUpdater(t *testing.T) { listeners := &gracenet.Net{} log := zerolog.Nop() diff --git a/cmd/cloudflared/updater/workers_service.go b/cmd/cloudflared/updater/workers_service.go index 4b52571c..b5883f1f 100644 --- a/cmd/cloudflared/updater/workers_service.go +++ b/cmd/cloudflared/updater/workers_service.go @@ -3,6 +3,7 @@ package updater import ( "encoding/json" "errors" + "fmt" "net/http" "runtime" ) @@ -79,6 +80,10 @@ func (s *WorkersService) Check() (CheckResult, error) { } defer resp.Body.Close() + if resp.StatusCode != 200 { + return nil, fmt.Errorf("unable to check for update: %d", resp.StatusCode) + } + var v VersionResponse if err := json.NewDecoder(resp.Body).Decode(&v); err != nil { return nil, err diff --git a/cmd/cloudflared/updater/workers_update.go b/cmd/cloudflared/updater/workers_update.go index b2d451a9..800fa4fe 100644 --- a/cmd/cloudflared/updater/workers_update.go +++ b/cmd/cloudflared/updater/workers_update.go @@ -3,7 +3,6 @@ package updater import ( "archive/tar" "compress/gzip" - "crypto/sha256" "errors" "fmt" "io" @@ -16,6 +15,10 @@ import ( "strings" "text/template" "time" + + "github.com/getsentry/sentry-go" + + "github.com/cloudflare/cloudflared/cmd/cloudflared/cliutil" ) const ( @@ -86,8 +89,25 @@ func (v *WorkersVersion) Apply() error { return err } - // check that the file is what is expected - if err := isValidChecksum(v.checksum, newFilePath); err != nil { + downloadSum, err := cliutil.FileChecksum(newFilePath) + if err != nil { + return err + } + + // Check that the file downloaded matches what is expected. + if v.checksum != downloadSum { + return errors.New("checksum validation failed") + } + + // Check if the currently running version has the same checksum + if downloadSum == buildInfo.Checksum { + // Currently running binary matches the downloaded binary so we have no reason to update. This is + // typically unexpected, as such we emit a sentry event. + localHub := sentry.CurrentHub().Clone() + err := errors.New("checksum validation matches currently running process") + localHub.CaptureException(err) + // Make sure to cleanup the new downloaded file since we aren't upgrading versions. + os.Remove(newFilePath) return err } @@ -189,27 +209,6 @@ func isCompressedFile(urlstring string) bool { return strings.HasSuffix(u.Path, ".tgz") } -// checks if the checksum in the json response matches the checksum of the file download -func isValidChecksum(checksum, filePath string) error { - f, err := os.Open(filePath) - if err != nil { - return err - } - defer f.Close() - - h := sha256.New() - if _, err := io.Copy(h, f); err != nil { - return err - } - - hash := fmt.Sprintf("%x", h.Sum(nil)) - - if checksum != hash { - return errors.New("checksum validation failed") - } - return nil -} - // writeBatchFile writes a batch file out to disk // see the dicussion on why it has to be done this way func writeBatchFile(targetPath string, newPath string, oldPath string) error {