From 313cc26966bd4e6213f257dd3fbfca716f40ea5f Mon Sep 17 00:00:00 2001 From: Marco Dinis Date: Tue, 25 Jun 2024 16:04:53 +0100 Subject: [PATCH] [v15] Script oneoff: add optional command prefix (sudo) (#43465) * Script oneoff: add optional command prefix (sudo) We are converting the installer script used for Server Auto Discover to use go instead of shell script. As an example, in EC2 Auto Discover, the script runs as `ssm-user` which has access to using `sudo`. This script is currently using `sudo` to change system wide configurations (adding repos, installing packages, create file locks, ....). In order to convert this script into go code, we must also run with elevated privileges. This PR changes the `oneoff` script to optionally run with a prefix. Only `sudo` can be used as a command prefix. * use t.cleanup and fix sudo usage when testing --- lib/web/scripts/oneoff/oneoff.go | 25 ++++++ lib/web/scripts/oneoff/oneoff.sh | 2 +- lib/web/scripts/oneoff/oneoff_test.go | 107 ++++++++++++++++++++++---- 3 files changed, 119 insertions(+), 15 deletions(-) diff --git a/lib/web/scripts/oneoff/oneoff.go b/lib/web/scripts/oneoff/oneoff.go index ac3cc4836a2b4..8eda144d5dc13 100644 --- a/lib/web/scripts/oneoff/oneoff.go +++ b/lib/web/scripts/oneoff/oneoff.go @@ -41,8 +41,14 @@ const ( // binMktemp is the default binary name for creating temporary directories. binMktemp = "mktemp" + + // PrefixSUDO is a Teleport Command Prefix that executes with higher privileges + // Use with caution. + PrefixSUDO = "sudo" ) +var allowedCommandPrefix = []string{PrefixSUDO} + var ( //go:embed oneoff.sh oneoffScript string @@ -53,6 +59,13 @@ var ( // OneOffScriptParams contains the required params to create a script that downloads and executes teleport binary. type OneOffScriptParams struct { + // TeleportCommandPrefix is a prefix command to use when calling teleport command. + // Acceptable values are: "sudo" + TeleportCommandPrefix string + // binSudo contains the location for the sudo binary. + // Used for testing. + binSudo string + // TeleportArgs is the arguments to pass to the teleport binary. // Eg, 'version' TeleportArgs string @@ -96,6 +109,10 @@ func (p *OneOffScriptParams) CheckAndSetDefaults() error { p.BinMktemp = binMktemp } + if p.binSudo == "" { + p.binSudo = "sudo" + } + if p.CDNBaseURL == "" { p.CDNBaseURL = teleportCDNLocation } @@ -118,6 +135,14 @@ func (p *OneOffScriptParams) CheckAndSetDefaults() error { p.SuccessMessage = "Completed successfully." } + switch p.TeleportCommandPrefix { + case PrefixSUDO: + p.TeleportCommandPrefix = p.binSudo + case "": + default: + return trace.BadParameter("invalid command prefix %q, only %v are supported", p.TeleportCommandPrefix, allowedCommandPrefix) + } + return nil } diff --git a/lib/web/scripts/oneoff/oneoff.sh b/lib/web/scripts/oneoff/oneoff.sh index 1b1549a937e7f..912e4d6ab3368 100644 --- a/lib/web/scripts/oneoff/oneoff.sh +++ b/lib/web/scripts/oneoff/oneoff.sh @@ -45,7 +45,7 @@ main() { mkdir -p ${tempDir}/bin mv ${tempDir}/${teleportFlavor}/teleport ${tempDir}/bin/teleport echo "> ${tempDir}/bin/teleport ${teleportArgs} $@" - ${tempDir}/bin/teleport ${teleportArgs} $@ && echo $successMessage + {{.TeleportCommandPrefix}} ${tempDir}/bin/teleport ${teleportArgs} $@ && echo $successMessage } main $@ diff --git a/lib/web/scripts/oneoff/oneoff_test.go b/lib/web/scripts/oneoff/oneoff_test.go index 5de287bb92b40..963f7d2392f1d 100644 --- a/lib/web/scripts/oneoff/oneoff_test.go +++ b/lib/web/scripts/oneoff/oneoff_test.go @@ -48,15 +48,21 @@ func TestOneOffScript(t *testing.T) { unameMock, err := bintest.NewMock("uname") require.NoError(t, err) - defer func() { + t.Cleanup(func() { assert.NoError(t, unameMock.Close()) - }() + }) mktempMock, err := bintest.NewMock("mktemp") require.NoError(t, err) - defer func() { + t.Cleanup(func() { assert.NoError(t, mktempMock.Close()) - }() + }) + + sudoMock, err := bintest.NewMock("sudo") + require.NoError(t, err) + t.Cleanup(func() { + assert.NoError(t, sudoMock.Close()) + }) script, err := BuildScript(OneOffScriptParams{ BinUname: unameMock.Path, @@ -75,9 +81,9 @@ func TestOneOffScript(t *testing.T) { teleportMock, err := bintest.NewMock(testWorkingDir + "/bin/teleport") require.NoError(t, err) - defer func() { + t.Cleanup(func() { assert.NoError(t, teleportMock.Close()) - }() + }) teleportBinTarball, err := utils.CompressTarGzArchive([]string{"teleport/teleport"}, singleFileFS{file: teleportMock.Path}) require.NoError(t, err) @@ -86,7 +92,7 @@ func TestOneOffScript(t *testing.T) { assert.Equal(t, "/teleport-v13.1.0-linux-amd64-bin.tar.gz", req.URL.Path) http.ServeContent(w, req, "teleport-v13.1.0-linux-amd64-bin.tar.gz", time.Now(), bytes.NewReader(teleportBinTarball.Bytes())) })) - defer func() { testServer.Close() }() + t.Cleanup(func() { testServer.Close() }) script, err := BuildScript(OneOffScriptParams{ BinUname: unameMock.Path, @@ -124,6 +130,65 @@ func TestOneOffScript(t *testing.T) { require.NoDirExists(t, testWorkingDir) }) + t.Run("command with prefix can be executed", func(t *testing.T) { + // set up + testWorkingDir := t.TempDir() + require.NoError(t, os.Mkdir(testWorkingDir+"/bin/", 0o755)) + scriptLocation := testWorkingDir + "/" + scriptName + + teleportMock, err := bintest.NewMock(testWorkingDir + "/bin/teleport") + require.NoError(t, err) + t.Cleanup(func() { + assert.NoError(t, teleportMock.Close()) + }) + + teleportBinTarball, err := utils.CompressTarGzArchive([]string{"teleport/teleport"}, singleFileFS{file: teleportMock.Path}) + require.NoError(t, err) + + testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + assert.Equal(t, "/teleport-v13.1.0-linux-amd64-bin.tar.gz", req.URL.Path) + http.ServeContent(w, req, "teleport-v13.1.0-linux-amd64-bin.tar.gz", time.Now(), bytes.NewReader(teleportBinTarball.Bytes())) + })) + t.Cleanup(func() { testServer.Close() }) + + script, err := BuildScript(OneOffScriptParams{ + BinUname: unameMock.Path, + BinMktemp: mktempMock.Path, + CDNBaseURL: testServer.URL, + TeleportVersion: "v13.1.0", + TeleportArgs: "version", + SuccessMessage: "Test was a success.", + TeleportCommandPrefix: "sudo", + binSudo: sudoMock.Path, + }) + require.NoError(t, err) + + unameMock.Expect("-s").AndWriteToStdout("Linux") + unameMock.Expect("-m").AndWriteToStdout("x86_64") + mktempMock.Expect("-d", "-p", homeDir).AndWriteToStdout(testWorkingDir) + sudoMock.Expect(teleportMock.Path, "version").AndWriteToStdout(teleportVersionOutput) + + err = os.WriteFile(scriptLocation, []byte(script), 0700) + require.NoError(t, err) + + // execute script + out, err := exec.Command("sh", scriptLocation).CombinedOutput() + + // validate + require.NoError(t, err, string(out)) + + require.True(t, unameMock.Check(t)) + require.True(t, mktempMock.Check(t)) + require.True(t, teleportMock.Check(t)) + + require.Contains(t, string(out), "teleport version") + require.Contains(t, string(out), teleportVersionOutput) + require.Contains(t, string(out), "Test was a success.") + + // Script should remove the temporary directory. + require.NoDirExists(t, testWorkingDir) + }) + t.Run("command can be executed with extra arguments", func(t *testing.T) { teleportHelpStart := "Use teleport start --config teleport.yaml" // set up @@ -133,9 +198,9 @@ func TestOneOffScript(t *testing.T) { teleportMock, err := bintest.NewMock(testWorkingDir + "/bin/teleport") require.NoError(t, err) - defer func() { + t.Cleanup(func() { assert.NoError(t, teleportMock.Close()) - }() + }) teleportBinTarball, err := utils.CompressTarGzArchive([]string{"teleport/teleport"}, singleFileFS{file: teleportMock.Path}) require.NoError(t, err) @@ -144,7 +209,7 @@ func TestOneOffScript(t *testing.T) { assert.Equal(t, "/teleport-v13.1.0-linux-amd64-bin.tar.gz", req.URL.Path) http.ServeContent(w, req, "teleport-v13.1.0-linux-amd64-bin.tar.gz", time.Now(), bytes.NewReader(teleportBinTarball.Bytes())) })) - defer func() { testServer.Close() }() + t.Cleanup(func() { testServer.Close() }) script, err := BuildScript(OneOffScriptParams{ BinUname: unameMock.Path, @@ -165,7 +230,7 @@ func TestOneOffScript(t *testing.T) { require.NoError(t, err) // execute script - out, err := exec.Command("bash", scriptLocation, "start").CombinedOutput() + out, err := exec.Command("sh", scriptLocation, "start").CombinedOutput() // validate require.NoError(t, err, string(out)) @@ -235,6 +300,20 @@ func TestOneOffScript(t *testing.T) { require.True(t, trace.IsBadParameter(err), "expected BadParameter, got %+v", err) }) + t.Run("invalid command prefix should return an error", func(t *testing.T) { + _, err := BuildScript(OneOffScriptParams{ + BinUname: unameMock.Path, + BinMktemp: mktempMock.Path, + CDNBaseURL: "dummyURL", + TeleportVersion: "v13.1.0", + TeleportArgs: "version", + SuccessMessage: "Test was a success.", + TeleportFlavor: "teleport", + TeleportCommandPrefix: "rm -rf thing", + }) + require.True(t, trace.IsBadParameter(err), "expected BadParameter, got %+v", err) + }) + t.Run("if enterprise build, it uses the enterprise package name", func(t *testing.T) { // set up testWorkingDir := t.TempDir() @@ -243,9 +322,9 @@ func TestOneOffScript(t *testing.T) { teleportMock, err := bintest.NewMock(testWorkingDir + "/bin/teleport") require.NoError(t, err) - defer func() { + t.Cleanup(func() { assert.NoError(t, teleportMock.Close()) - }() + }) modules.SetTestModules(t, &modules.TestModules{ TestBuildType: modules.BuildEnterprise, @@ -257,7 +336,7 @@ func TestOneOffScript(t *testing.T) { assert.Equal(t, "/teleport-ent-v13.1.0-linux-amd64-bin.tar.gz", req.URL.Path) http.ServeContent(w, req, "teleport-ent-v13.1.0-linux-amd64-bin.tar.gz", time.Now(), bytes.NewReader(teleportBinTarball.Bytes())) })) - defer func() { testServer.Close() }() + t.Cleanup(func() { testServer.Close() }) script, err := BuildScript(OneOffScriptParams{ BinUname: unameMock.Path,