From fbffdcf3d25b287121d9b83bc272a674b66e6f97 Mon Sep 17 00:00:00 2001 From: Kevin Nisbet Date: Fri, 5 Jul 2019 15:57:27 -0400 Subject: [PATCH] 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 --- Gopkg.lock | 146 +++++++++--------- lib/app/handler/handler.go | 15 ++ lib/app/handler/handler_test.go | 15 ++ lib/archive/archive.go | 25 ++- lib/archive/archive_test.go | 100 ++++++++++++ lib/ops/operatoracl.go | 2 +- lib/ops/opshandler/opshandler.go | 27 ++-- lib/ops/opsservice/service.go | 37 +++++ lib/ops/opsservice/service_test.go | 116 ++++++++++++++ lib/pack/webpack/webpack.go | 2 +- lib/users/usersservice/usersservice.go | 37 +++-- lib/users/usersservice/usersservice_test.go | 11 +- lib/utils/semver.go | 39 +++++ lib/utils/semver_test.go | 87 +++++++++++ .../app/components/user/googleAuthLogo.jsx | 4 +- .../app/components/user/userInviteReset.jsx | 2 +- web/src/index.ejs | 9 +- 17 files changed, 560 insertions(+), 114 deletions(-) create mode 100644 lib/ops/opsservice/service_test.go create mode 100644 lib/utils/semver.go create mode 100644 lib/utils/semver_test.go diff --git a/Gopkg.lock b/Gopkg.lock index 7607d2989c..765662bfd5 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -15,7 +15,7 @@ name = "github.com/Azure/go-ansiterm" packages = [ ".", - "winterm", + "winterm" ] pruneopts = "UT" revision = "d6e3b3328b783f23731bc4d058875b0371ff8109" @@ -29,7 +29,7 @@ "autorest/azure", "autorest/date", "logger", - "version", + "version" ] pruneopts = "UT" revision = "ea233b6412b0421a65dc6160e16c893364664a95" @@ -103,7 +103,7 @@ name = "github.com/alecthomas/template" packages = [ ".", - "parse", + "parse" ] pruneopts = "UT" revision = "b867cc6ab45cece8143cfcc6fc9c77cf3f2c23c0" @@ -191,7 +191,7 @@ "service/s3/s3manager", "service/sqs", "service/ssm", - "service/sts", + "service/sts" ] pruneopts = "UT" revision = "fefacb6d45b233a9e20097a176c1c46e85a782d3" @@ -249,7 +249,7 @@ packages = [ ".", "qr", - "utils", + "utils" ] pruneopts = "UT" revision = "fe0f26ff6d26693948ee8189aa064ee8c54141fa" @@ -277,7 +277,7 @@ "gettext", "gettext/mo", "gettext/plural", - "gettext/po", + "gettext/po" ] pruneopts = "UT" revision = "bf70f2a70fb1b1f36d90d671a72795984eab0fcb" @@ -300,7 +300,7 @@ "log", "ocsp/config", "signer", - "signer/local", + "signer/local" ] pruneopts = "UT" revision = "4b8305b36ad0a046fc683298642e048c291a4333" @@ -336,7 +336,7 @@ "pkg/tlsutil", "pkg/transport", "pkg/types", - "version", + "version" ] pruneopts = "UT" revision = "27fc7e2296f506182f58ce846e48f36b34fe6842" @@ -350,7 +350,7 @@ "jose", "key", "oauth2", - "oidc", + "oidc" ] pruneopts = "UT" revision = "e51edf2e47e65e5708600d4da6fca1388ee437b4" @@ -379,7 +379,7 @@ packages = [ "health", "httputil", - "timeutil", + "timeutil" ] pruneopts = "UT" revision = "1914e367e85eaf0c25d495b48e060dfe6190f8d0" @@ -439,7 +439,7 @@ "registry/storage/driver/filesystem", "registry/storage/driver/middleware", "uuid", - "version", + "version" ] pruneopts = "UT" revision = "edc3ab29cdff8694dd6feb85cfeb4b5f1b38ed9c" @@ -460,7 +460,7 @@ "pkg/random", "pkg/system", "pkg/term", - "pkg/term/windows", + "pkg/term/windows" ] pruneopts = "UT" revision = "092cba3727bb9b4a2f0e922cd6c0f93ea270e363" @@ -487,7 +487,7 @@ name = "github.com/docker/spdystream" packages = [ ".", - "spdy", + "spdy" ] pruneopts = "UT" revision = "6480d4af844c189cf5dd913db24ddd339d3a4f85" @@ -555,7 +555,7 @@ "external/github.com/opencontainers/runc/libcontainer/user", "external/golang.org/x/net/context", "external/golang.org/x/sys/unix", - "testing", + "testing" ] pruneopts = "UT" revision = "9184fe7a2d7c920b8cdd30faa64fe025f609ef20" @@ -565,7 +565,7 @@ name = "github.com/garyburd/redigo" packages = [ "internal", - "redis", + "redis" ] pruneopts = "UT" revision = "6ece6e0a09f28cc399b21550cbf37ab39ba63cce" @@ -628,7 +628,7 @@ "syntax/ast", "syntax/lexer", "util/runes", - "util/strings", + "util/strings" ] pruneopts = "UT" revision = "5ccd90ef52e1e632236f7326478d4faa74f99438" @@ -650,7 +650,7 @@ "proto", "protoc-gen-gogo/descriptor", "sortkeys", - "types", + "types" ] pruneopts = "UT" revision = "7d68e886eac4f7e34d0d82241a6273d6c304c5cf" @@ -681,7 +681,7 @@ "ptypes/any", "ptypes/duration", "ptypes/empty", - "ptypes/timestamp", + "ptypes/timestamp" ] pruneopts = "UT" revision = "b4deda0973fb4c70b50d226b1af49f3da59f5265" @@ -704,7 +704,7 @@ "asn1", "client", "x509", - "x509/pkix", + "x509/pkix" ] pruneopts = "UT" revision = "99d8352410cb54aacb8f34b261352c1a4f573453" @@ -731,7 +731,7 @@ packages = [ "OpenAPIv2", "compiler", - "extensions", + "extensions" ] pruneopts = "UT" revision = "ee43cbb60db7bd22502942cccbc39059117352ab" @@ -748,7 +748,7 @@ "openstack/identity/v2/tokens", "openstack/identity/v3/tokens", "openstack/utils", - "pagination", + "pagination" ] pruneopts = "UT" revision = "d3bcea3cf97e0f06b9e272e9c4a1b5909e7db216" @@ -776,7 +776,7 @@ ".", "cstrings", "jsonschema", - "schema", + "schema" ] pruneopts = "UT" revision = "c3428bd84c23f0cfcc759f2ef12632be5ff5d95d" @@ -786,7 +786,7 @@ name = "github.com/gravitational/coordinate" packages = [ "config", - "leader", + "leader" ] pruneopts = "UT" revision = "d3d3609ffa9eb7f9b30f90ed1463389feeeff971" @@ -821,7 +821,7 @@ packages = [ ".", "authority", - "constants", + "constants" ] pruneopts = "UT" revision = "f3111b1818ceae70bad37b64a252ed196fd078d4" @@ -834,7 +834,7 @@ "connlimit", "forward", "ratelimit", - "utils", + "utils" ] pruneopts = "UT" revision = "e4a7e35311e6c4da4cf9e73c6a3d8cf523937211" @@ -863,7 +863,7 @@ "agent/proto/agentpb", "lib/kubernetes", "monitoring", - "utils", + "utils" ] pruneopts = "UT" revision = "57869406f0ebe0d80989e632df28052a7a252ed9" @@ -877,7 +877,7 @@ "ratelimiter", "util", "watch", - "winfile", + "winfile" ] pruneopts = "UT" revision = "f50ab22a2454512d7bb89dc1d675a07e2c26b059" @@ -937,7 +937,7 @@ "lib/utils/parse", "lib/utils/proxy", "lib/web", - "lib/web/ui", + "lib/web/ui" ] pruneopts = "UT" revision = "8d626fcff656a0fca570e9b0dd919b8e4e4709eb" @@ -971,7 +971,7 @@ name = "github.com/gregjones/httpcache" packages = [ ".", - "diskcache", + "diskcache" ] pruneopts = "UT" revision = "9cad4c3443a7200dd6400aef47183728de563a38" @@ -998,7 +998,7 @@ name = "github.com/hashicorp/go-getter" packages = [ ".", - "helper/url", + "helper/url" ] pruneopts = "UT" revision = "4bda8fa99001c61db3cad96b421d4c12a81f256d" @@ -1056,7 +1056,7 @@ name = "github.com/hashicorp/golang-lru" packages = [ ".", - "simplelru", + "simplelru" ] pruneopts = "UT" revision = "20f1fb78b0740ba8c3cb143a61e86ba5c8669768" @@ -1074,7 +1074,7 @@ "hcl/token", "json/parser", "json/scanner", - "json/token", + "json/token" ] pruneopts = "UT" revision = "8cb6e5b959231cc1119e43259c4a608f9c51a241" @@ -1090,7 +1090,7 @@ "hcl/hclsyntax", "hcl/json", "hcldec", - "hclparse", + "hclparse" ] pruneopts = "UT" revision = "ed8144cda141c1f06f8ab122baff783b645cfe52" @@ -1103,7 +1103,7 @@ ".", "ast", "parser", - "scanner", + "scanner" ] pruneopts = "UT" revision = "fa9f258a92500514cc8e9c67020487709df92432" @@ -1133,7 +1133,7 @@ "svchost/disco", "terraform", "tfdiags", - "version", + "version" ] pruneopts = "UT" revision = "41e50bd32a8825a84535e353c3674af8ce799161" @@ -1229,7 +1229,7 @@ name = "github.com/mailgun/lemma" packages = [ "random", - "secret", + "secret" ] pruneopts = "UT" revision = "e8b0cd607f5855f9a4a33f8ae5d033178f559964" @@ -1270,7 +1270,7 @@ packages = [ "buffer", "jlexer", - "jwriter", + "jwriter" ] pruneopts = "UT" revision = "60711f1a8329503b04e1c88535f419d0bb440bff" @@ -1312,7 +1312,7 @@ packages = [ "gf256", "qr", - "qr/coding", + "qr/coding" ] pruneopts = "UT" revision = "90f07065088deccf50b28eb37c93dad3078c0f3c" @@ -1448,7 +1448,7 @@ name = "github.com/opencontainers/runc" packages = [ "libcontainer/system", - "libcontainer/user", + "libcontainer/user" ] pruneopts = "UT" revision = "baf6536d6259209c3edfa2b22237af82942d3dfa" @@ -1493,7 +1493,7 @@ ".", "cmd", "cmd/install", - "match", + "match" ] pruneopts = "UT" revision = "dcda3199365ca2a5f24aea4c42aa56f6a197d117" @@ -1505,7 +1505,7 @@ packages = [ ".", "hotp", - "totp", + "totp" ] pruneopts = "UT" revision = "54653902c20e47f3417541d35435cb6d6162e28a" @@ -1533,7 +1533,7 @@ packages = [ "expfmt", "internal/bitbucket.org/ww/goautoneg", - "model", + "model" ] pruneopts = "UT" revision = "61f87aac8082fa8c3c5655c7608d7478d46ac2ad" @@ -1544,7 +1544,7 @@ name = "github.com/prometheus/procfs" packages = [ ".", - "xfs", + "xfs" ] pruneopts = "UT" revision = "e645f4e5aaa8506fc71d6edbc5c4ff02c04c46f2" @@ -1554,7 +1554,7 @@ name = "github.com/russellhaering/gosaml2" packages = [ ".", - "types", + "types" ] pruneopts = "UT" revision = "8908227c114abe0b63b1f0606abae72d11bf632a" @@ -1566,7 +1566,7 @@ packages = [ ".", "etreeutils", - "types", + "types" ] pruneopts = "UT" revision = "605161228693b2efadce55323c9c661a40c5fbaa" @@ -1586,7 +1586,7 @@ "decoders", "formats", "loader", - "mediatypes", + "mediatypes" ] pruneopts = "UT" revision = "1471bfd74cff003b1e329d46a4de4487e88cc47b" @@ -1613,7 +1613,7 @@ name = "github.com/sirupsen/logrus" packages = [ ".", - "hooks/syslog", + "hooks/syslog" ] pruneopts = "UT" revision = "dcdb95d728dbb6fe4b9fbe6b3b9d1fb268e68036" @@ -1642,7 +1642,7 @@ packages = [ ".", "sha256", - "sha512", + "sha512" ] pruneopts = "UT" revision = "51ad44105773cafcbe91927f70ac68e1bf78f8b4" @@ -1677,7 +1677,7 @@ ".", "internal/hash", "internal/xlog", - "lzma", + "lzma" ] pruneopts = "UT" revision = "0c6b41e72360850ca4f98dc341fd999726ea007f" @@ -1688,7 +1688,7 @@ name = "github.com/vulcand/oxy" packages = [ "forward", - "utils", + "utils" ] pruneopts = "UT" revision = "40720199a16cdfb3854ff58ad91d290a7dd56de0" @@ -1698,7 +1698,7 @@ name = "github.com/vulcand/predicate" packages = [ ".", - "builder", + "builder" ] pruneopts = "UT" revision = "8fbfb3ab0e94276b6b58bec378600829adc7a203" @@ -1745,7 +1745,7 @@ "cty/function/stdlib", "cty/gocty", "cty/json", - "cty/set", + "cty/set" ] pruneopts = "UT" revision = "c2393a5d54f2fe015be2ddb1fc55431f749387de" @@ -1779,7 +1779,7 @@ "scrypt", "ssh", "ssh/agent", - "ssh/terminal", + "ssh/terminal" ] pruneopts = "UT" revision = "e84da0312774c21d64ee2317962ef669b27ffb41" @@ -1800,7 +1800,7 @@ "internal/timeseries", "nettest", "trace", - "websocket", + "websocket" ] pruneopts = "UT" revision = "9b4f9f5ad5197c79fd623a3638e70d8b26cef344" @@ -1814,7 +1814,7 @@ "google", "internal", "jws", - "jwt", + "jwt" ] pruneopts = "UT" revision = "c57b0facaced709681d9f90397429b9430a74754" @@ -1825,7 +1825,7 @@ name = "golang.org/x/sys" packages = [ "unix", - "windows", + "windows" ] pruneopts = "UT" revision = "95b1ffbd15a57cc5abb3f04402b9e8ec0016a52c" @@ -1849,7 +1849,7 @@ "unicode/cldr", "unicode/norm", "unicode/rangetable", - "width", + "width" ] pruneopts = "UT" revision = "fd889fe3a20f4878f5f47672fd3ca5b86db005e2" @@ -1875,7 +1875,7 @@ "internal/modules", "internal/remote_api", "internal/urlfetch", - "urlfetch", + "urlfetch" ] pruneopts = "UT" revision = "ae0ab99deb4dc413a2b4bd6c8bdd0eb67f1e4d06" @@ -1920,7 +1920,7 @@ "resolver/passthrough", "stats", "status", - "tap", + "tap" ] pruneopts = "UT" revision = "2e463a05d100327ca47ac218281906921038fd95" @@ -1956,7 +1956,7 @@ name = "gopkg.in/mgo.v2" packages = [ "bson", - "internal/json", + "internal/json" ] pruneopts = "UT" revision = "3f83fa5005286a7fe593b055f0d7771a7dce4655" @@ -1968,7 +1968,7 @@ ".", "cipher", "json", - "jwt", + "jwt" ] pruneopts = "UT" revision = "f61ac651e23224d43848a8dc387dee016d7eacf9" @@ -2027,7 +2027,7 @@ "settings/v1alpha1", "storage/v1", "storage/v1alpha1", - "storage/v1beta1", + "storage/v1beta1" ] pruneopts = "UT" revision = "74b699b93c15473932b89e3d1818ba8282f3b5ab" @@ -2042,7 +2042,7 @@ "pkg/client/clientset/clientset", "pkg/client/clientset/clientset/scheme", "pkg/client/clientset/clientset/typed/apiextensions/v1beta1", - "pkg/features", + "pkg/features" ] pruneopts = "UT" revision = "d4288ab6494571219e781fa423db363247635942" @@ -2103,7 +2103,7 @@ "pkg/watch", "third_party/forked/golang/json", "third_party/forked/golang/netutil", - "third_party/forked/golang/reflect", + "third_party/forked/golang/reflect" ] pruneopts = "UT" revision = "572dfc7bdfcb4531361a17d27b92851f59acf0dc" @@ -2117,7 +2117,7 @@ "pkg/authentication/serviceaccount", "pkg/authentication/user", "pkg/features", - "pkg/util/feature", + "pkg/util/feature" ] pruneopts = "UT" revision = "26bc712632e1faf9efc6b63f712325c760ad1ebe" @@ -2129,7 +2129,7 @@ packages = [ "pkg/genericclioptions", "pkg/genericclioptions/printers", - "pkg/genericclioptions/resource", + "pkg/genericclioptions/resource" ] pruneopts = "UT" revision = "491c94071cfae3b539753adf65705d8a333fb92b" @@ -2220,7 +2220,7 @@ "util/homedir", "util/integer", "util/jsonpath", - "util/retry", + "util/retry" ] pruneopts = "UT" revision = "e64494209f554a6723674bd494d69445fb76a1d4" @@ -2256,7 +2256,7 @@ "pkg/timeconv", "pkg/tlsutil", "pkg/urlutil", - "pkg/version", + "pkg/version" ] pruneopts = "UT" revision = "02a47c7249b1fc6d8fd3b94e6b4babf9d818144e" @@ -2276,7 +2276,7 @@ packages = [ "pkg/apis/apiregistration", "pkg/apis/apiregistration/v1", - "pkg/apis/apiregistration/v1beta1", + "pkg/apis/apiregistration/v1beta1" ] pruneopts = "UT" revision = "ef29109bb10bac567941babe2babf01baa39542b" @@ -2287,7 +2287,7 @@ name = "k8s.io/kube-openapi" packages = [ "pkg/util/proto", - "pkg/util/proto/validation", + "pkg/util/proto/validation" ] pruneopts = "UT" revision = "0317810137be915b9cf888946c6e115c1bfac693" @@ -2396,7 +2396,7 @@ "pkg/util/node", "pkg/util/parsers", "pkg/util/taints", - "pkg/version", + "pkg/version" ] pruneopts = "UT" revision = "eec55b9ba98609a46fee712359c7b5b365bdd920" @@ -2407,7 +2407,7 @@ name = "k8s.io/utils" packages = [ "exec", - "pointer", + "pointer" ] pruneopts = "UT" revision = "0d26856f57b32ec3398579285e5c8a2bfe8c5243" @@ -2626,7 +2626,7 @@ "k8s.io/helm/pkg/timeconv", "k8s.io/helm/pkg/version", "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1", - "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1beta1", + "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1beta1" ] solver-name = "gps-cdcl" solver-version = 1 diff --git a/lib/app/handler/handler.go b/lib/app/handler/handler.go index ecda3e6ee6..48ce3b9704 100644 --- a/lib/app/handler/handler.go +++ b/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, diff --git a/lib/app/handler/handler_test.go b/lib/app/handler/handler_test.go index 554de43248..05b81a6a7a 100644 --- a/lib/app/handler/handler_test.go +++ b/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") +} diff --git a/lib/archive/archive.go b/lib/archive/archive.go index 8d3cddb2f2..a9ed1e457e 100644 --- a/lib/archive/archive.go +++ b/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) diff --git a/lib/archive/archive_test.go b/lib/archive/archive_test.go index 49aeeb800e..159d22420b 100644 --- a/lib/archive/archive_test.go +++ b/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) + } + } +} diff --git a/lib/ops/operatoracl.go b/lib/ops/operatoracl.go index e936b022c9..3c7087c604 100644 --- a/lib/ops/operatoracl.go +++ b/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) { diff --git a/lib/ops/opshandler/opshandler.go b/lib/ops/opshandler/opshandler.go index 25a4827751..b09db1a6fa 100644 --- a/lib/ops/opshandler/opshandler.go +++ b/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) } diff --git a/lib/ops/opsservice/service.go b/lib/ops/opsservice/service.go index 86658f8845..fbcf1e2718 100644 --- a/lib/ops/opsservice/service.go +++ b/lib/ops/opsservice/service.go @@ -20,9 +20,11 @@ import ( "context" "fmt" "io" + "net" "net/url" "os" "path/filepath" + "regexp" "sync" "time" @@ -703,6 +705,11 @@ func (o *Operator) GetLocalSite() (*ops.Site, error) { // params are optional URL query parameters that can specify additional // configuration attributes. func (o *Operator) GetSiteInstructions(tokenID string, serverProfile string, params url.Values) (string, error) { + err := validateGetSiteInstructions(tokenID, serverProfile, params) + if err != nil { + return "", trace.Wrap(err) + } + token, err := o.backend().GetProvisioningToken(tokenID) if err != nil { return "", trace.Wrap(err) @@ -728,6 +735,36 @@ func (o *Operator) GetSiteInstructions(tokenID string, serverProfile string, par return instructions, nil } +var instructionsAllowedRePattern = `^([a-zA-Z0-9\-\.\_]*)$` +var instructionsAllowedRe = regexp.MustCompile(instructionsAllowedRePattern) + +// validateGetSiteInstructions validates user provided input into bash script generation for install instructions +// TODO(knisbet) we're going to try and eliminate the bash script generation based on user input, but as a +// workaround make sure we sanitize any inputs to the current function +func validateGetSiteInstructions(tokenID string, serverProfile string, params url.Values) error { + advertiseAddr := params.Get(schema.AdvertiseAddr) + if len(advertiseAddr) > 0 && net.ParseIP(advertiseAddr) == nil { + return trace.Wrap(trace.BadParameter("advertise_addr does not appear to be a valid IP address")). + AddField("advertise_addr", advertiseAddr) + } + + if !instructionsAllowedRe.Match([]byte(tokenID)) { + return trace.Wrap(trace.BadParameter("Token format validation failed")).AddFields(map[string]interface{}{ + "token": tokenID, + "allowRegex": instructionsAllowedRePattern, + }) + } + + if !instructionsAllowedRe.Match([]byte(serverProfile)) { + return trace.Wrap(trace.BadParameter("ServerProfile format validation failed")).AddFields(map[string]interface{}{ + "server_profile": serverProfile, + "allowRegex": instructionsAllowedRePattern, + }) + } + + return nil +} + // SignTLSKey signs X509 Public Key with X509 certificate authority of this site func (o *Operator) SignTLSKey(req ops.TLSSignRequest) (*ops.TLSSignResponse, error) { st, err := o.openSite(req.SiteKey()) diff --git a/lib/ops/opsservice/service_test.go b/lib/ops/opsservice/service_test.go new file mode 100644 index 0000000000..e38c48819b --- /dev/null +++ b/lib/ops/opsservice/service_test.go @@ -0,0 +1,116 @@ +/* +Copyright 2018 Gravitational, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package opsservice + +import ( + "net/url" + "testing" + + "github.com/gravitational/gravity/lib/schema" + "github.com/stretchr/testify/assert" +) + +func TestGetSiteInstructionsSanitization(t *testing.T) { + cases := []struct { + token string + serverProfile string + params url.Values + expectError bool + }{ + // Normal Inputs + { + token: "token1", + serverProfile: "profile1", + params: map[string][]string{ + schema.AdvertiseAddr: {"1.1.1.1"}, + }, + }, + { + token: "token-2", + serverProfile: "profile-2", + params: map[string][]string{ + schema.AdvertiseAddr: {"1.1.1.1"}, + }, + }, + { + token: "token3_underscore", + serverProfile: "profile3_underscore", + params: map[string][]string{ + schema.AdvertiseAddr: {"255.255.255.255"}, + }, + }, + // Malicious Inputs + { + expectError: true, + token: "token4;echo", + serverProfile: "profile4", + params: map[string][]string{ + schema.AdvertiseAddr: {"1.1.1.1"}, + }, + }, + { + expectError: true, + token: "token5$(touch grav)", + serverProfile: "profile5", + params: map[string][]string{ + schema.AdvertiseAddr: {"1.1.1.1"}, + }, + }, + { + expectError: true, + token: "token6", + serverProfile: "profile6;echo", + params: map[string][]string{ + schema.AdvertiseAddr: {"1.1.1.1"}, + }, + }, + { + expectError: true, + token: "token7", + serverProfile: "profile7;$(touch grav)", + params: map[string][]string{ + schema.AdvertiseAddr: {"1.1.1.1"}, + }, + }, + { + expectError: true, + token: "token8", + serverProfile: "profile8", + params: map[string][]string{ + schema.AdvertiseAddr: {"1.1.1.1;echo"}, + }, + }, + { + expectError: true, + token: "token9", + serverProfile: "profile9", + params: map[string][]string{ + schema.AdvertiseAddr: {"1.1.1.1$(touch grav)"}, + }, + }, + } + + for _, tt := range cases { + err := validateGetSiteInstructions(tt.token, tt.serverProfile, tt.params) + if tt.expectError { + assert.Error(t, err, tt) + } else { + assert.NoError(t, err, tt) + } + + } +} diff --git a/lib/pack/webpack/webpack.go b/lib/pack/webpack/webpack.go index 74a44bbc24..fc45c70202 100644 --- a/lib/pack/webpack/webpack.go +++ b/lib/pack/webpack/webpack.go @@ -301,7 +301,7 @@ func (s *Server) needsAuth(fn authHandle) httprouter.Handle { if !hasCookie { user, checker, err = s.cfg.Users.AuthenticateUser(*authCreds) if err != nil { - log.Debugf("authenticate error: %v", err) + log.Debugf("authenticate error: %v", trace.DebugReport(err)) // we hide the error from the remote user to avoid giving any hints trace.WriteError( w, trace.AccessDenied("bad username or password")) diff --git a/lib/users/usersservice/usersservice.go b/lib/users/usersservice/usersservice.go index d175807204..6703055d20 100644 --- a/lib/users/usersservice/usersservice.go +++ b/lib/users/usersservice/usersservice.go @@ -351,23 +351,25 @@ func (c *UsersService) AuthenticateUserBasicAuth(username, password string) (sto return nil, trace.BadParameter("unexpected user type %T", i) } + if err = c.checkCanUseBasicAuth(user); err != nil { + return nil, trace.Wrap(err) + } + switch user.GetType() { case storage.AgentUser: // check the provided password against agent api keys (it may have a few) - match := false keys, err := c.backend.GetAPIKeys(user.GetName()) if err != nil { return nil, trace.Wrap(err) } for _, k := range keys { if subtle.ConstantTimeCompare([]byte(k.Token), []byte(password)) == 1 { - match = true + return user, nil } } - if !match { - return nil, trace.AccessDenied("bad agent api key") - } - return user, nil + + return nil, trace.AccessDenied("bad agent api key") + case storage.AdminUser, storage.RegularUser: keys, err := c.backend.GetAPIKeys(user.GetName()) if err != nil { @@ -378,15 +380,32 @@ func (c *UsersService) AuthenticateUserBasicAuth(username, password string) (sto return user, nil } } - if err := bcrypt.CompareHashAndPassword([]byte(user.GetPassword()), []byte(password)); err != nil { - return nil, trace.AccessDenied("bad user password") + + if err := bcrypt.CompareHashAndPassword([]byte(user.GetPassword()), []byte(password)); err == nil { + return user, nil } - return user, nil + + return nil, trace.AccessDenied("bad user or password") default: return nil, trace.AccessDenied("unsupported user type: %v", user.GetType()) } } +func (c *UsersService) checkCanUseBasicAuth(user storage.User) error { + // don't allow users with TOTP/HOTP tokens set to use Basic Auth + totp, err := c.GetTOTP(user.GetName()) + if err != nil && !trace.IsNotFound(err) { + return trace.Wrap(err) + } + if len(totp) != 0 { + return trace.AccessDenied("basic auth not available") + } + if len(user.GetHOTP()) != 0 { + return trace.AccessDenied("basic auth not available") + } + return nil +} + // AuthenticateUserBearerAuth is used to authenticate site agent users // that connect using provisioning tokens or API keys func (c *UsersService) AuthenticateUserBearerAuth(token string) (storage.User, error) { diff --git a/lib/users/usersservice/usersservice_test.go b/lib/users/usersservice/usersservice_test.go index 462225527c..3ea0aca923 100644 --- a/lib/users/usersservice/usersservice_test.go +++ b/lib/users/usersservice/usersservice_test.go @@ -286,19 +286,12 @@ func (s *UsersSuite) TestPasswordRecovery(c *C) { Username: email, Password: "password2", }) - c.Assert(err, IsNil, Commentf( - "should be able to login with the new password")) + c.Assert(err, NotNil, Commentf( + "Basic auth shouln't work with OTP token set")) resetReq.Password = users.Password("password2") _, err = s.suite.Users.ResetUserWithToken(resetReq) c.Assert(err, NotNil, Commentf("should not be able to reuse password reset token")) - - _, _, err = s.suite.Users.AuthenticateUser(httplib.AuthCreds{ - Type: httplib.AuthBasic, - Username: email, - Password: "password3", - }) - c.Assert(err, NotNil, Commentf("password shouldn't have been changed")) } // TestPasswordRecovery verifies that: diff --git a/lib/utils/semver.go b/lib/utils/semver.go new file mode 100644 index 0000000000..98b87585da --- /dev/null +++ b/lib/utils/semver.go @@ -0,0 +1,39 @@ +/* +Copyright 2019 Gravitational, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package utils + +import ( + "regexp" + + "github.com/coreos/go-semver/semver" + "github.com/gravitational/trace" +) + +var semverRePattern = `^([a-zA-Z0-9\-\.]*)$` +var semverRe = regexp.MustCompile(semverRePattern) + +// SanitizeSemver validates semver pre-release/metadata fields are alphanumeric characters dash and dot as per +// https://semver.org/#semantic-versioning-specification-semver +func SanitizeSemver(ver semver.Version) error { + if !semverRe.Match([]byte(ver.PreRelease)) { + return trace.BadParameter("Semver pre-release failed validation.") + } + if !semverRe.Match([]byte(ver.Metadata)) { + return trace.BadParameter("Semver metadata failed validation.") + } + return nil +} diff --git a/lib/utils/semver_test.go b/lib/utils/semver_test.go new file mode 100644 index 0000000000..e6d851a72b --- /dev/null +++ b/lib/utils/semver_test.go @@ -0,0 +1,87 @@ +/* +Copyright 2019 Gravitational, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package utils + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/coreos/go-semver/semver" +) + +func TestSemverSanity(t *testing.T) { + cases := []struct { + version semver.Version + shouldErr bool + }{ + // + // Normal Inputs + // + { + version: *semver.New("0.0.0"), + shouldErr: false, + }, + { + version: *semver.New("0.0.0-alpha.1"), + shouldErr: false, + }, + { + version: *semver.New("0.0.0-alpha.0"), + shouldErr: false, + }, + { + version: *semver.New("99.0.0-alpha.106"), + shouldErr: false, + }, + { + version: *semver.New("0.0.0+some-text"), + shouldErr: false, + }, + { + version: *semver.New("0.0.0-alpha.55+some-text-Plus-Uppercase"), + shouldErr: false, + }, + // + // Malicious Inputs + // + { + version: *semver.New("0.0.0+;echo"), + shouldErr: true, + }, + { + version: *semver.New("0.0.0-;echo"), + shouldErr: true, + }, + { + version: *semver.New("1.0.1-aaa$(touch grav)"), + shouldErr: true, + }, + { + version: *semver.New("1.0.1+aaa$(touch grav)"), + shouldErr: true, + }, + } + + for _, tt := range cases { + if tt.shouldErr { + assert.Error(t, SanitizeSemver(tt.version), tt.version.String()) + } else { + assert.NoError(t, SanitizeSemver(tt.version), tt.version.String()) + } + } +} diff --git a/web/src/app/components/user/googleAuthLogo.jsx b/web/src/app/components/user/googleAuthLogo.jsx index 6837bae655..d8525a1515 100644 --- a/web/src/app/components/user/googleAuthLogo.jsx +++ b/web/src/app/components/user/googleAuthLogo.jsx @@ -20,8 +20,8 @@ const GoogleAuthInfo = () => (
Google Authenticator -
Download Google Authenticator on your phone to access your two factor token
+
Download Google Authenticator on your phone to access your two factor token
); - + export default GoogleAuthInfo; diff --git a/web/src/app/components/user/userInviteReset.jsx b/web/src/app/components/user/userInviteReset.jsx index 5dc0063882..e644b3db91 100644 --- a/web/src/app/components/user/userInviteReset.jsx +++ b/web/src/app/components/user/userInviteReset.jsx @@ -165,7 +165,7 @@ const InputFormFooter = ({ auth2faType }) => { return (
Click - here + here to learn more about U2F 2-Step Verification.
diff --git a/web/src/index.ejs b/web/src/index.ejs index e78581ec60..ceda18e45a 100644 --- a/web/src/index.ejs +++ b/web/src/index.ejs @@ -3,13 +3,14 @@ - - - + + + + <%= htmlWebpackPlugin.options.title %> -
+
\ No newline at end of file