Kevin/public/5.5.x/updates (#485)

* bump e ref (#17)

* Use noreferrer attribute in login components

* Kevin/5.5.x/security backport (#12)

* [5.5] fix restart regression (#442)

*  * Fix a regression with rolling update with restart not configured to
happen on the target node.
 * Configure resource create/remove operations to log similar to install
operation - both to syslog and a log file.

* Fix plan tests with restart step taking the ExecServer

* Workaround HTTP_PROXY issue with etcdctl (#446)

* Workaround HTTP_PROXY issue with etcdctl

* Update planet to release

* bump kubernetes for second fix CVE-2019-1002101 (#459)

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

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

* disable username/password BasicAuth against api

* prevent 2fa bypass with basic auth

* address review comments

* add nosniff to injectable endpoints (#10)

* initial implementation of tar path sanitization (#6)

* initial implementation of tar path sanitization

* address review feedback

* 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

* bump submodule pointer

* apply acls to apikeys endpoints

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

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

lib/ops/opshandler/opshandler.go

@@ -301,6 +301,7 @@ func (h *WebHandler) getSiteInstructions(w http.ResponseWriter, r *http.Request,
 		return
 	}
 	w.Header().Set("Content-Type", "text/plain")
+	w.Header().Set("X-Content-Type-Options", "nosniff")
 	w.WriteHeader(http.StatusOK)
 	w.Write([]byte(instructions))
 }
@@ -330,7 +331,7 @@ Success response:
   }]
 
 */
-func (h *WebHandler) getApps(w http.ResponseWriter, r *http.Request, p httprouter.Params, ctx *HandlerContext) error {
+func (h *WebHandler) getApps(w http.ResponseWriter, r *http.Request, p httprouter.Params, context *HandlerContext) error {
 	repositories, err := h.cfg.Packages.GetRepositories()
 	if err != nil {
 		return trace.Wrap(err)
@@ -354,7 +355,7 @@ func (h *WebHandler) getApps(w http.ResponseWriter, r *http.Request, p httproute
 
     POST /portal/v1/accounts/:account_id/sites/:site_domain/usertokens/resets
 */
-func (h *WebHandler) resetUser(w http.ResponseWriter, r *http.Request, p httprouter.Params, ctx *HandlerContext) error {
+func (h *WebHandler) resetUser(w http.ResponseWriter, r *http.Request, p httprouter.Params, context *HandlerContext) error {
 	data, err := ioutil.ReadAll(r.Body)
 	if err != nil {
 		return trace.Wrap(err)
@@ -364,7 +365,7 @@ func (h *WebHandler) resetUser(w http.ResponseWriter, r *http.Request, p httprou
 		return trace.BadParameter(err.Error())
 	}
 
-	userToken, err := ctx.Identity.CreateResetToken(
+	userToken, err := context.Identity.CreateResetToken(
 		fmt.Sprintf("https://%v", h.cfg.PublicAdvertiseAddr.String()),
 		req.Name,
 		req.TTL)
@@ -424,7 +425,7 @@ Success response:
   }
 
 */
-func (h *WebHandler) createAccount(w http.ResponseWriter, r *http.Request, p httprouter.Params, ctx *HandlerContext) error {
+func (h *WebHandler) createAccount(w http.ResponseWriter, r *http.Request, p httprouter.Params, context *HandlerContext) error {
 	data, err := ioutil.ReadAll(r.Body)
 	if err != nil {
 		return trace.Wrap(err)
@@ -433,7 +434,7 @@ func (h *WebHandler) createAccount(w http.ResponseWriter, r *http.Request, p htt
 	if err := json.Unmarshal(data, &req); err != nil {
 		return trace.BadParameter(err.Error())
 	}
-	account, err := ctx.Operator.CreateAccount(req)
+	account, err := context.Operator.CreateAccount(req)
 	if err != nil {
 		return trace.Wrap(err)
 	}
@@ -559,7 +560,7 @@ func (h *WebHandler) createUser(w http.ResponseWriter, r *http.Request, p httpro
      "message": "user jenkins deleted"
    }
 */
-func (h *WebHandler) deleteLocalUser(w http.ResponseWriter, r *http.Request, p httprouter.Params, ctx *HandlerContext) error {
+func (h *WebHandler) deleteLocalUser(w http.ResponseWriter, r *http.Request, p httprouter.Params, context *HandlerContext) error {
 	name := p[0].Value
 	err := h.cfg.Users.DeleteUser(name)
 	if err != nil {
@@ -580,7 +581,7 @@ func (h *WebHandler) deleteLocalUser(w http.ResponseWriter, r *http.Request, p h
    }
 
 */
-func (h *WebHandler) createAPIKey(w http.ResponseWriter, r *http.Request, p httprouter.Params, ctx *HandlerContext) error {
+func (h *WebHandler) createAPIKey(w http.ResponseWriter, r *http.Request, p httprouter.Params, context *HandlerContext) error {
 	d := json.NewDecoder(r.Body)
 	var req ops.NewAPIKeyRequest
 	if err := d.Decode(&req); err != nil {
@@ -608,9 +609,9 @@ func (h *WebHandler) createAPIKey(w http.ResponseWriter, r *http.Request, p http
      ...
    ]
 */
-func (h *WebHandler) getAPIKeys(w http.ResponseWriter, r *http.Request, p httprouter.Params, ctx *HandlerContext) error {
+func (h *WebHandler) getAPIKeys(w http.ResponseWriter, r *http.Request, p httprouter.Params, context *HandlerContext) error {
 	userEmail := p.ByName("user_email")
-	keys, err := h.cfg.Operator.GetAPIKeys(userEmail)
+	keys, err := context.Operator.GetAPIKeys(userEmail)
 	if err != nil {
 		return trace.Wrap(err)
 	}
@@ -620,7 +621,7 @@ func (h *WebHandler) getAPIKeys(w http.ResponseWriter, r *http.Request, p httpro
 
 /* deleteAPIKey deletes an api key
 
-   DELETE /portal/v1/users/:user_email/apikeys/:api_key
+   DELETE /portal/v1/apikeys/user/:user_email/:api_key
 
    Success response:
 
@@ -753,7 +754,7 @@ func (h *WebHandler) getTrustedClusterToken(w http.ResponseWriter, r *http.Reque
 	}
   }
 */
-func (h *WebHandler) createSite(w http.ResponseWriter, r *http.Request, p httprouter.Params, ctx *HandlerContext) error {
+func (h *WebHandler) createSite(w http.ResponseWriter, r *http.Request, p httprouter.Params, context *HandlerContext) error {
 	data, err := ioutil.ReadAll(r.Body)
 	if err != nil {
 		return trace.Wrap(err)
@@ -779,7 +780,7 @@ func (h *WebHandler) createSite(w http.ResponseWriter, r *http.Request, p httpro
 		return trace.Wrap(err)
 	}
 
-	site, err := ctx.Operator.CreateSite(req)
+	site, err := context.Operator.CreateSite(req)
 	if err != nil {
 		return trace.Wrap(err)
 	}
@@ -2121,7 +2122,7 @@ func (h *WebHandler) getAppInstaller(w http.ResponseWriter, r *http.Request, p h
 	}
 	installerReq.AccountID = accountID
 
-	reader, err := h.cfg.Operator.GetAppInstaller(installerReq)
+	reader, err := context.Operator.GetAppInstaller(installerReq)
 	if err != nil {
 		return trace.Wrap(err)
 	}