Kevin/public/master/updates (#488)

* update e ref

* tele/tsh install script sanitization (#5)

* implement input sanitization for telekubeInstallScript

* Remove bash based instructions which appear to no longer be used

* Expand comments around semver sanity check

* Revert "Remove bash based instructions which appear to no longer be used"

This reverts commit eaf40a0ed1959fed1411bd8171566a093dbc5921.

* validate server instructions

* Address review feedback

* initial implementation of tar path sanitization (#6)

* initial implementation of tar path sanitization

* address review feedback

* add nosniff to injectable endpoints (#10)

* Kevin/master/4125 disable basic authentication (#11)

* disable username/password BasicAuth against api

* prevent 2fa bypass with basic auth

* address review comments

* Do not leak sensitive information via referral  header (#9)

* apply acls to apikeys endpoints

* address review feedback

* update e ref

Gopkg.toml

@@ -32,8 +32,7 @@ ignored = [
 
 [[override]]
   name = "github.com/gravitational/trace"
-  #version = "1.1.6"
-  revision = "f30095ced5ff011085d26f160468dcc477607730"
+  version = "1.1.7"
 
 [[override]]
   name = "github.com/mitchellh/go-ps"

lib/app/handler/handler.go

@@ -38,6 +38,7 @@ import (
 	"github.com/gravitational/gravity/lib/pack"
 	"github.com/gravitational/gravity/lib/storage"
 	"github.com/gravitational/gravity/lib/users"
+	"github.com/gravitational/gravity/lib/utils"
 
 	"github.com/coreos/go-semver/semver"
 	"github.com/gravitational/form"
@@ -839,11 +840,25 @@ func (h *WebHandler) telekubeInstallScript(w http.ResponseWriter, r *http.Reques
 		ver = constants.LatestVersion
 	}
 
+	// Security: sanitize semver input
+	semver, err := semver.NewVersion(ver)
+	if err != nil {
+		return trace.Wrap(err)
+	}
+	err = utils.SanitizeSemver(*semver)
+	if err != nil {
+		return trace.Wrap(err)
+	}
+
 	tfVersion, err := getTerraformVersion(ver, h.Packages)
 	if err != nil {
 		return trace.Wrap(err)
 	}
 
+	w.Header().Set("Content-Type", "text/plain")
+	w.Header().Set("X-Content-Type-Options", "nosniff")
+	w.WriteHeader(http.StatusOK)
+
 	err = telekubeInstallScriptTemplate.Execute(w, map[string]string{
 		"version":   ver,
 		"tfVersion": tfVersion,

lib/app/handler/handler_test.go

@@ -24,6 +24,9 @@ import (
 	"testing"
 	"time"
 
+	"github.com/julienschmidt/httprouter"
+	"github.com/stretchr/testify/assert"
+
 	"github.com/gravitational/gravity/lib/app"
 	"github.com/gravitational/gravity/lib/app/client"
 	"github.com/gravitational/gravity/lib/app/docker"
@@ -214,3 +217,15 @@ func (r *HandlerSuite) TestFetchChart(c *C) {
 func (r *HandlerSuite) TestFetchIndexFile(c *C) {
 	r.suite.FetchIndexFile(c)
 }
+
+// TestTelekubeInstallScriptChecksSemverSanity ensures that the install script generator is checking the semver for
+// malicious input
+func TestTelekubeInstallScriptChecksSemverSanity(t *testing.T) {
+	h := &WebHandler{}
+	assert.Error(t, h.telekubeInstallScript(nil, nil, httprouter.Params{
+		httprouter.Param{
+			Key:   "version",
+			Value: "1.0.1-aaa$(touch grav)",
+		},
+	}), "validate that telekubeInstallScript throws error on bad input")
+}

lib/archive/archive.go

@@ -95,7 +95,6 @@ func Unpack(path string) (unpackedDir string, err error) {
 // The resulting files and directories are created using the current user context.
 func Extract(r io.Reader, dir string) error {
 	tarball := tar.NewReader(r)
-
 	for {
 		header, err := tarball.Next()
 		if err == io.EOF {
@@ -104,6 +103,12 @@ func Extract(r io.Reader, dir string) error {
 			return trace.Wrap(err)
 		}
 
+		// Security: ensure tar doesn't refer to file paths outside the directory
+		err = SanitizeTarPath(header, dir)
+		if err != nil {
+			return trace.Wrap(err)
+		}
+
 		if err := extractFile(tarball, header, dir); err != nil {
 			return trace.Wrap(err)
 		}
@@ -323,6 +328,24 @@ func extractFile(tarball *tar.Reader, header *tar.Header, dir string) error {
 	return nil
 }
 
+// SanitizeTarPath checks that the tar header paths resolve to a subdirectory path, and don't contain file paths or
+// links that could escape the tar file (e.g. ../../etc/passwrd)
+func SanitizeTarPath(header *tar.Header, dir string) error {
+	// Security: sanitize that all tar paths resolve to within the destination directory
+	destPath := filepath.Join(dir, header.Name)
+	if !strings.HasPrefix(destPath, filepath.Clean(dir)+string(os.PathSeparator)) {
+		return trace.BadParameter("%s: illegal file path", header.Name)
+	}
+	// Security: Ensure link destinations resolve to within the destination directory
+	if header.Linkname != "" {
+		linkPath := filepath.Join(dir, header.Linkname)
+		if !strings.HasPrefix(linkPath, filepath.Clean(dir)+string(os.PathSeparator)) {
+			return trace.BadParameter("%s: illegal link path", header.Linkname)
+		}
+	}
+	return nil
+}
+
 func writeFile(path string, r io.Reader, mode os.FileMode) error {
 	err := withDir(path, func() error {
 		out, err := os.Create(path)

lib/archive/archive_test.go

@@ -25,6 +25,8 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+
 	"github.com/gravitational/gravity/lib/defaults"
 
 	"github.com/docker/docker/pkg/archive"
@@ -229,3 +231,101 @@ type file struct {
 	data  []byte
 	isDir bool
 }
+
+func TestSanitizeTarPath(t *testing.T) {
+	cases := []struct {
+		header      *tar.Header
+		expectError bool
+	}{
+		// File path is within destination directory
+		{
+			header: &tar.Header{
+				Name: "test1.txt",
+			},
+			expectError: false,
+		},
+		{
+			header: &tar.Header{
+				Name: "./dir/test2.txt",
+			},
+			expectError: false,
+		},
+		{
+			header: &tar.Header{
+				Name: "/dir/../dir2/test3.txt",
+			},
+			expectError: false,
+		},
+		{
+			header: &tar.Header{
+				Name: "./dir/test4.txt",
+			},
+			expectError: false,
+		},
+		// Linkname path is within destination directory
+		{
+			header: &tar.Header{
+				Name:     "test5.txt",
+				Linkname: "test5.txt",
+			},
+			expectError: false,
+		},
+		{
+			header: &tar.Header{
+				Name:     "test6.txt",
+				Linkname: "./dir/test6.txt",
+			},
+			expectError: false,
+		},
+		{
+			header: &tar.Header{
+				Name:     "test7.txt",
+				Linkname: "/dir/../dir2/test7.txt",
+			},
+			expectError: false,
+		},
+		{
+			header: &tar.Header{
+				Name:     "test8.txt",
+				Linkname: "./dir/test8.txt",
+			},
+			expectError: false,
+		},
+		// Name will be outside destination directory
+		{
+			header: &tar.Header{
+				Name: "../test9.txt",
+			},
+			expectError: true,
+		},
+		{
+			header: &tar.Header{
+				Name: "./test/../../test10.txt",
+			},
+			expectError: true,
+		},
+		// Linkname points outside destination directory
+		{
+			header: &tar.Header{
+				Name:     "test11.txt",
+				Linkname: "../test11.txt",
+			},
+			expectError: true,
+		},
+		{
+			header: &tar.Header{
+				Name:     "test12.txt",
+				Linkname: "./test/../../test12.txt",
+			},
+			expectError: true,
+		},
+	}
+
+	for _, tt := range cases {
+		if tt.expectError {
+			assert.Error(t, SanitizeTarPath(tt.header, "/tmp"), "Name: %v LinkName: %v", tt.header.Name, tt.header.Linkname)
+		} else {
+			assert.NoError(t, SanitizeTarPath(tt.header, "/tmp"), "Name: %v LinkName: %v", tt.header.Name, tt.header.Linkname)
+		}
+	}
+}

lib/ops/operatoracl.go

@@ -263,7 +263,7 @@ func (o *OperatorACL) DeleteAPIKey(ctx context.Context, userEmail, token string)
 	if err := o.currentUserActions(userEmail, teleservices.VerbUpdate); err != nil {
 		return trace.Wrap(err)
 	}
-	return o.operator.DeleteAPIKey(ctx, o.username, token)
+	return o.operator.DeleteAPIKey(ctx, userEmail, token)
 }
 
 func (o *OperatorACL) CreateInstallToken(req NewInstallTokenRequest) (*storage.InstallToken, error) {