Skip to content

Commit

Permalink
TUN-8630: Check checksum of downloaded binary to compare to current f…
Browse files Browse the repository at this point in the history
…or 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.
  • Loading branch information
DevinCarr committed Sep 11, 2024
1 parent a57fc25 commit 2484df1
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 31 deletions.
32 changes: 31 additions & 1 deletion cmd/cloudflared/cliutil/build_info.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package cliutil

import (
"crypto/sha256"
"fmt"
"io"
"os"
"runtime"

"github.com/rs/zerolog"
Expand All @@ -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 {
Expand All @@ -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())
}
Expand All @@ -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
}
2 changes: 1 addition & 1 deletion cmd/cloudflared/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
12 changes: 7 additions & 5 deletions cmd/cloudflared/updater/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -31,7 +32,7 @@ const (
)

var (
version string
buildInfo *cliutil.BuildInfo
BuiltForPackageManager = ""
)

Expand Down Expand Up @@ -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) {
Expand All @@ -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.
Expand Down Expand Up @@ -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...")
Expand Down
6 changes: 6 additions & 0 deletions cmd/cloudflared/updater/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
5 changes: 5 additions & 0 deletions cmd/cloudflared/updater/workers_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package updater
import (
"encoding/json"
"errors"
"fmt"
"net/http"
"runtime"
)
Expand Down Expand Up @@ -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
Expand Down
47 changes: 23 additions & 24 deletions cmd/cloudflared/updater/workers_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package updater
import (
"archive/tar"
"compress/gzip"
"crypto/sha256"
"errors"
"fmt"
"io"
Expand All @@ -16,6 +15,10 @@ import (
"strings"
"text/template"
"time"

"github.com/getsentry/sentry-go"

"github.com/cloudflare/cloudflared/cmd/cloudflared/cliutil"
)

const (
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 2484df1

Please sign in to comment.