diff --git a/cmd/launcher/main.go b/cmd/launcher/main.go index 329b16c8b..78da1f61b 100644 --- a/cmd/launcher/main.go +++ b/cmd/launcher/main.go @@ -42,23 +42,30 @@ func main() { // launcher is _also_ called when we're checking update // validity (with autoupdate.checkExecutable). This is // somewhat awkward as we end up with extra call layers. - newerBinary, err := autoupdate.FindNewestSelf(ctx) - if err != nil { - logutil.Fatal(logger, err, "checking for updated version") - } + // + // Allow a caller to set `LAUNCHER_SKIP_UPDATES` as a way to + // skip exec'ing an update. This helps prevent launcher from + // fork-bombing itself. This is an ENV, because there's no + // good way to pass it through the flags. + if !env.Bool("LAUNCHER_SKIP_UPDATES", false) { + newerBinary, err := autoupdate.FindNewestSelf(ctx) + if err != nil { + logutil.Fatal(logger, err, "checking for updated version") + } - if newerBinary != "" { - level.Debug(logger).Log( - "msg", "preparing to exec new binary", - "oldVersion", version.Version().Version, - "newBinary", newerBinary, - ) - if err := execwrapper.Exec(ctx, newerBinary, os.Args, os.Environ()); err != nil { - logutil.Fatal(logger, err, "exec") + if newerBinary != "" { + level.Debug(logger).Log( + "msg", "preparing to exec new binary", + "oldVersion", version.Version().Version, + "newBinary", newerBinary, + ) + if err := execwrapper.Exec(ctx, newerBinary, os.Args, os.Environ()); err != nil { + logutil.Fatal(logger, err, "exec") + } + panic("how") + } else { + level.Debug(logger).Log("msg", "Nothing new") } - panic("how") - } else { - level.Info(logger).Log("msg", "Nothing new") } // if the launcher is being ran with a positional argument, handle that diff --git a/go.mod b/go.mod index 400489f85..3b8d45a5e 100644 --- a/go.mod +++ b/go.mod @@ -59,7 +59,7 @@ require ( github.com/onsi/gomega v1.4.3 // indirect github.com/opencontainers/go-digest v1.0.0-rc1 // indirect github.com/opencontainers/image-spec v1.0.1 // indirect - github.com/peterbourgon/ff v1.1.0 + github.com/peterbourgon/ff v1.7.0 github.com/pkg/errors v0.8.1 github.com/prometheus/client_golang v0.9.2 // indirect github.com/serenize/snaker v0.0.0-20171204205717-a683aaf2d516 diff --git a/go.sum b/go.sum index cdece1905..d62489248 100644 --- a/go.sum +++ b/go.sum @@ -178,6 +178,7 @@ github.com/miekg/pkcs11 v0.0.0-20180208123018-5f6e0d0dad6f h1:8MAK/u+dE11/n8VIHQ github.com/miekg/pkcs11 v0.0.0-20180208123018-5f6e0d0dad6f/go.mod h1:WCBAbTOdfhHhz7YXujeZMF7owC4tPb1naKFsgfUISjo= github.com/mitchellh/go-ps v0.0.0-20170309133038-4fdf99ab2936 h1:kw1v0NlnN+GZcU8Ma8CLF2Zzgjfx95gs3/GN3vYAPpo= github.com/mitchellh/go-ps v0.0.0-20170309133038-4fdf99ab2936/go.mod h1:r1VsdOzOPt1ZSrGZWFoNhsAedKnEd6r9Np1+5blZCWk= +github.com/mitchellh/go-wordwrap v1.0.0/go.mod h1:ZXFpozHsX6DPmq2I0TCekCxypsnAUbP2oI0UX1GXzOo= github.com/mitchellh/mapstructure v1.0.0 h1:vVpGvMXJPqSDh2VYHF7gsfQj8Ncx+Xw5Y1KHeTRY+7I= github.com/mitchellh/mapstructure v1.0.0/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= github.com/mixer/clock v0.0.0-20170901150240-b08e6b4da7ea h1:eWwD4TJbXXLj8Ay2KQ1F3lh1YtgpcWZL3ZZN2sKvSDg= @@ -200,8 +201,11 @@ github.com/opencontainers/image-spec v1.0.1 h1:JMemWkRwHx4Zj+fVxWoMCFm/8sYGGrUVo github.com/opencontainers/image-spec v1.0.1/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0= github.com/pelletier/go-toml v1.2.0 h1:T5zMGML61Wp+FlcbWjRDT7yAxhJNAiPPLOFECq181zc= github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic= +github.com/pelletier/go-toml v1.6.0/go.mod h1:5N711Q9dKgbdkxHL+MEfF31hpT7l0S0s/t2kKREewys= github.com/peterbourgon/ff v1.1.0 h1:mz0/c1gtUwz/MsyJs3GwPjFSTFlD4UEwKEPw5Z+0cm8= github.com/peterbourgon/ff v1.1.0/go.mod h1:P2CDTJbjip+iAWS2jnkp6uKBX2Bcvc/K4p5j47CTRv0= +github.com/peterbourgon/ff v1.7.0 h1:hknvTgsh90jNBIjPq7xeq32Y9AmSbpXvjrFW4sJwW+A= +github.com/peterbourgon/ff v1.7.0/go.mod h1:/KKxnU5cBj4w21jEMj4Rway/kslRP6XAOHh7CH8AyAM= github.com/pkg/errors v0.8.0 h1:WdK/asTD0HN+q6hsWO3/vpuAkAr+tw6aNJNDFFf0+qw= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= @@ -324,6 +328,7 @@ golang.org/x/tools v0.0.0-20190506145303-2d16b83fe98c/go.mod h1:RgjU9mgBXZiqYHBn golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= golang.org/x/tools v0.0.0-20190606124116-d0a3d012864b/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= golang.org/x/tools v0.0.0-20190628153133-6cdbf07be9d0/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= +golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/api v0.4.0/go.mod h1:8k5glujaEP+g9n7WNsDg8QP6cUVNI86fCNMcbazEtwE= google.golang.org/api v0.7.0 h1:9sdfJOzWlkqPltHAuzT2Cp+yrBeY1KRVYgms8soxMwM= google.golang.org/api v0.7.0/go.mod h1:WtwebWUNSVBH/HAw79HIFXZNqEvBhG+Ra+ax0hx3E3M= @@ -364,6 +369,8 @@ gopkg.in/yaml.v2 v2.2.1 h1:mUhvW9EsL+naU5Q3cakzfE91YhliOondGd6ZrsDBHQE= gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v2 v2.2.4 h1:/eiJrUcujPVeJ3xlSWaiNi3uSVmDGBK1pDHUHAnao1I= +gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= diff --git a/pkg/autoupdate/findnew.go b/pkg/autoupdate/findnew.go index f0d2aaa79..b98812ca9 100644 --- a/pkg/autoupdate/findnew.go +++ b/pkg/autoupdate/findnew.go @@ -32,6 +32,7 @@ type newestSettings struct { deleteCorrupt bool skipFullBinaryPathCheck bool buildTimestamp string + runningExecutable string } type newestOption func(*newestSettings) @@ -67,6 +68,15 @@ func SkipFullBinaryPathCheck() newestOption { } } +// withRunningExectuable sets the current executable. This is because +// we never need to run an executable check against ourselves. (And +// doing so will trigger a fork bomb) +func withRunningExectuable(exe string) newestOption { + return func(no *newestSettings) { + no.runningExecutable = exe + } +} + // FindNewestSelf invokes `FindNewest` with the running binary path, // as determined by os.Executable. However, if the current running // version is the same as the newest on disk, it will return empty string. @@ -82,7 +92,7 @@ func FindNewestSelf(ctx context.Context, opts ...newestOption) (string, error) { return "", errors.New("can't find newest empty string") } - opts = append(opts, SkipFullBinaryPathCheck()) + opts = append(opts, SkipFullBinaryPathCheck(), withRunningExectuable(exPath)) newest := FindNewest(ctx, exPath, opts...) @@ -183,18 +193,25 @@ func FindNewest(ctx context.Context, fullBinaryPath string, opts ...newestOption } } - // Sanity check that the executable is executable. Also remove the update if appropriate - if err := checkExecutable(ctx, file, "--version"); err != nil { - if newestSettings.deleteCorrupt { - level.Error(logger).Log("msg", "not executable. Removing", "binary", file, "reason", err) - if err := os.RemoveAll(basedir); err != nil { - level.Error(logger).Log("msg", "error deleting broken update dir", "dir", basedir, "err", err) + // If the file is _not_ the running executable, sanity + // check that executions work. If the exec fails, + // there's clearly an issue and we should remove it. + if newestSettings.runningExecutable != file { + if err := checkExecutable(ctx, file, "--version"); err != nil { + if newestSettings.deleteCorrupt { + level.Error(logger).Log("msg", "not executable. Removing", "binary", file, "reason", err) + if err := os.RemoveAll(basedir); err != nil { + level.Error(logger).Log("msg", "error deleting broken update dir", "dir", basedir, "err", err) + } + } else { + level.Error(logger).Log("msg", "not executable. Skipping", "binary", file, "reason", err) } - } else { - level.Error(logger).Log("msg", "not executable. Skipping", "binary", file, "reason", err) - } - continue + continue + } + } else { + // This logging is mostly here to make test coverage of the conditional clear + level.Debug(logger).Log("msg", "Skipping checkExecutable against self", "file", file) } // We always want to increment the foundCount, since it's what triggers deletion. @@ -268,10 +285,22 @@ func checkExecutable(ctx context.Context, potentialBinary string, args ...string return err } + // If we can determine that the requested executable is + // ourself, don't try to exec. It's needless, and a potential + // fork bomb. Ignore errors, either we get an answer or we don't. + selfPath, _ := os.Executable() + if filepath.Clean(selfPath) == filepath.Clean(potentialBinary) { + return nil + } + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() cmd := exec.CommandContext(ctx, potentialBinary, args...) + + // Set env, this should prevent launcher for fork-bombing + cmd.Env = append(cmd.Env, "LAUNCHER_SKIP_UPDATES=TRUE") + execErr := cmd.Run() if ctx.Err() != nil { diff --git a/pkg/autoupdate/findnew_test.go b/pkg/autoupdate/findnew_test.go index 096cc1788..8c7631262 100644 --- a/pkg/autoupdate/findnew_test.go +++ b/pkg/autoupdate/findnew_test.go @@ -47,12 +47,14 @@ func TestFindNewestSelf(t *testing.T) { require.Empty(t, newest, "No correct binaries, should be empty") } - expectedNewest := filepath.Join(updatesDir, "3", filepath.Base(binaryPath)) - - require.NoError(t, copyFile(expectedNewest, binaryPath, false), "copy executable") - require.NoError(t, os.Chmod(expectedNewest, 0755), "chmod") + for _, n := range []string{"2", "3"} { + updatedBinaryPath := filepath.Join(updatesDir, n, filepath.Base(binaryPath)) + require.NoError(t, copyFile(updatedBinaryPath, binaryPath, false), "copy executable") + require.NoError(t, os.Chmod(updatedBinaryPath, 0755), "chmod") + } { + expectedNewest := filepath.Join(updatesDir, "3", filepath.Base(binaryPath)) newest, err := FindNewestSelf(ctx) require.NoError(t, err) require.Equal(t, expectedNewest, newest, "Should find newer binary") @@ -345,6 +347,13 @@ func copyFile(dstPath, srcPath string, truncate bool) error { func TestCheckExecutable(t *testing.T) { t.Parallel() + + // We need to run this from a temp dir, else the early return + // from matching os.Executable bypasses the point of this. + tmpDir, binaryName, cleanupFunc := setupTestDir(t, executableUpdates) + defer cleanupFunc() + targetExe := filepath.Join(tmpDir, binaryName) + var tests = []struct { testName string expectedErr bool @@ -369,9 +378,16 @@ func TestCheckExecutable(t *testing.T) { for _, tt := range tests { t.Run(tt.testName, func(t *testing.T) { - err := checkExecutable(context.TODO(), os.Args[0], "-test.run=TestHelperProcess", "--", tt.testName) + err := checkExecutable(context.TODO(), targetExe, "-test.run=TestHelperProcess", "--", tt.testName) if tt.expectedErr { require.Error(t, err, tt.testName) + + // As a bonus, we can test that if we call os.Args[0], + // we still don't get an error. This is because we + // trigger the match against os.Executable and don't + // invoked. This is here, and not a dedicated test, + // because we ensure the same test arguments. + require.NoError(t, checkExecutable(context.TODO(), os.Args[0], "-test.run=TestHelperProcess", "--", tt.testName), "calling self with %s", tt.testName) } else { require.NoError(t, err, tt.testName) }