Merge branch 'release-1.1.12' into release-1.1.12+ent

.changelog/12036.txt

@@ -0,0 +1,3 @@
+```release-note:security
+Fix race condition in use of go-getter that could cause a client agent to download the wrong artifact into the wrong destination. [CVE-2022-24686](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-24686)
+```

.changelog/12037.txt

@@ -0,0 +1,3 @@
+```release-note:security
+Resolve symlinks to prevent unauthorized access to files outside the allocation directory. [CVE-2022-24683](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-24683)
+```

.changelog/12038.txt

@@ -0,0 +1,3 @@
+```release-note:security
+Add ACL requirement and HCL validation to the job parse API endpoint to prevent excessive CPU usage. [CVE-2022-24685](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-24685)
+```

.changelog/12039.txt

@@ -0,0 +1,3 @@
+```release-note:security
+Prevent panic in spread iterator during allocation stop. [CVE-2022-24684](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-24684)
+```

CHANGELOG.md

@@ -1,3 +1,12 @@
+## 1.1.12 (February 9, 2022)
+
+SECURITY:
+
+* Add ACL requirement and HCL validation to the job parse API endpoint to prevent excessive CPU usage. [CVE-2022-24685](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-24685) [[GH-12038](https://github.com/hashicorp/nomad/issues/12038)]
+* Fix race condition in use of go-getter that could cause a client agent to download the wrong artifact into the wrong destination. [CVE-2022-24686](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-24686) [[GH-12036](https://github.com/hashicorp/nomad/issues/12036)]
+* Prevent panic in spread iterator during allocation stop. [CVE-2022-24684](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-24684) [[GH-12039](https://github.com/hashicorp/nomad/issues/12039)]
+* Resolve symlinks to prevent unauthorized access to files outside the allocation directory. [CVE-2022-24683](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-24683) [[GH-12037](https://github.com/hashicorp/nomad/issues/12037)]
+
 ## 1.1.11 (February 1, 2022)
 
 BUG FIXES:

acl/policy.go

@@ -26,6 +26,7 @@ const (
 
 	NamespaceCapabilityDeny                 = "deny"
 	NamespaceCapabilityListJobs             = "list-jobs"
+	NamespaceCapabilityParseJob             = "parse-job"
 	NamespaceCapabilityReadJob              = "read-job"
 	NamespaceCapabilitySubmitJob            = "submit-job"
 	NamespaceCapabilityDispatchJob          = "dispatch-job"
@@ -146,7 +147,7 @@ func (p *PluginPolicy) isValid() bool {
 // isNamespaceCapabilityValid ensures the given capability is valid for a namespace policy
 func isNamespaceCapabilityValid(cap string) bool {
 	switch cap {
-	case NamespaceCapabilityDeny, NamespaceCapabilityListJobs, NamespaceCapabilityReadJob,
+	case NamespaceCapabilityDeny, NamespaceCapabilityParseJob, NamespaceCapabilityListJobs, NamespaceCapabilityReadJob,
 		NamespaceCapabilitySubmitJob, NamespaceCapabilityDispatchJob, NamespaceCapabilityReadLogs,
 		NamespaceCapabilityReadFS, NamespaceCapabilityAllocLifecycle,
 		NamespaceCapabilityAllocExec, NamespaceCapabilityAllocNodeExec,
@@ -166,6 +167,7 @@ func isNamespaceCapabilityValid(cap string) bool {
 func expandNamespacePolicy(policy string) []string {
 	read := []string{
 		NamespaceCapabilityListJobs,
+		NamespaceCapabilityParseJob,
 		NamespaceCapabilityReadJob,
 		NamespaceCapabilityCSIListVolume,
 		NamespaceCapabilityCSIReadVolume,

acl/policy_test.go

@@ -29,6 +29,7 @@ func TestParse(t *testing.T) {
 						Policy: PolicyRead,
 						Capabilities: []string{
 							NamespaceCapabilityListJobs,
+							NamespaceCapabilityParseJob,
 							NamespaceCapabilityReadJob,
 							NamespaceCapabilityCSIListVolume,
 							NamespaceCapabilityCSIReadVolume,
@@ -78,6 +79,7 @@ func TestParse(t *testing.T) {
 						Policy: PolicyRead,
 						Capabilities: []string{
 							NamespaceCapabilityListJobs,
+							NamespaceCapabilityParseJob,
 							NamespaceCapabilityReadJob,
 							NamespaceCapabilityCSIListVolume,
 							NamespaceCapabilityCSIReadVolume,
@@ -91,6 +93,7 @@ func TestParse(t *testing.T) {
 						Policy: PolicyWrite,
 						Capabilities: []string{
 							NamespaceCapabilityListJobs,
+							NamespaceCapabilityParseJob,
 							NamespaceCapabilityReadJob,
 							NamespaceCapabilityCSIListVolume,
 							NamespaceCapabilityCSIReadVolume,

client/allocdir/alloc_dir.go

@@ -6,17 +6,17 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"net/http"
 	"os"
 	"path/filepath"
+	"strings"
 	"sync"
 	"time"
 
-	"net/http"
-	"strings"
-
 	hclog "github.com/hashicorp/go-hclog"
 	multierror "github.com/hashicorp/go-multierror"
 	cstructs "github.com/hashicorp/nomad/client/structs"
+	"github.com/hashicorp/nomad/helper/escapingfs"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hpcloud/tail/watch"
 	tomb "gopkg.in/tomb.v1"
@@ -366,7 +366,7 @@ func (d *AllocDir) Build() error {
 
 // List returns the list of files at a path relative to the alloc dir
 func (d *AllocDir) List(path string) ([]*cstructs.AllocFileInfo, error) {
-	if escapes, err := structs.PathEscapesAllocDir("", path); err != nil {
+	if escapes, err := escapingfs.PathEscapesAllocDir(d.AllocDir, "", path); err != nil {
 		return nil, fmt.Errorf("Failed to check if path escapes alloc directory: %v", err)
 	} else if escapes {
 		return nil, fmt.Errorf("Path escapes the alloc directory")
@@ -392,7 +392,7 @@ func (d *AllocDir) List(path string) ([]*cstructs.AllocFileInfo, error) {
 
 // Stat returns information about the file at a path relative to the alloc dir
 func (d *AllocDir) Stat(path string) (*cstructs.AllocFileInfo, error) {
-	if escapes, err := structs.PathEscapesAllocDir("", path); err != nil {
+	if escapes, err := escapingfs.PathEscapesAllocDir(d.AllocDir, "", path); err != nil {
 		return nil, fmt.Errorf("Failed to check if path escapes alloc directory: %v", err)
 	} else if escapes {
 		return nil, fmt.Errorf("Path escapes the alloc directory")
@@ -442,7 +442,7 @@ func detectContentType(fileInfo os.FileInfo, path string) string {
 
 // ReadAt returns a reader for a file at the path relative to the alloc dir
 func (d *AllocDir) ReadAt(path string, offset int64) (io.ReadCloser, error) {
-	if escapes, err := structs.PathEscapesAllocDir("", path); err != nil {
+	if escapes, err := escapingfs.PathEscapesAllocDir(d.AllocDir, "", path); err != nil {
 		return nil, fmt.Errorf("Failed to check if path escapes alloc directory: %v", err)
 	} else if escapes {
 		return nil, fmt.Errorf("Path escapes the alloc directory")
@@ -473,7 +473,7 @@ func (d *AllocDir) ReadAt(path string, offset int64) (io.ReadCloser, error) {
 // BlockUntilExists blocks until the passed file relative the allocation
 // directory exists. The block can be cancelled with the passed context.
 func (d *AllocDir) BlockUntilExists(ctx context.Context, path string) (chan error, error) {
-	if escapes, err := structs.PathEscapesAllocDir("", path); err != nil {
+	if escapes, err := escapingfs.PathEscapesAllocDir(d.AllocDir, "", path); err != nil {
 		return nil, fmt.Errorf("Failed to check if path escapes alloc directory: %v", err)
 	} else if escapes {
 		return nil, fmt.Errorf("Path escapes the alloc directory")
@@ -499,7 +499,7 @@ func (d *AllocDir) BlockUntilExists(ctx context.Context, path string) (chan erro
 // allocation directory. The offset should be the last read offset. The context is
 // used to clean up the watch.
 func (d *AllocDir) ChangeEvents(ctx context.Context, path string, curOffset int64) (*watch.FileChanges, error) {
-	if escapes, err := structs.PathEscapesAllocDir("", path); err != nil {
+	if escapes, err := escapingfs.PathEscapesAllocDir(d.AllocDir, "", path); err != nil {
 		return nil, fmt.Errorf("Failed to check if path escapes alloc directory: %v", err)
 	} else if escapes {
 		return nil, fmt.Errorf("Path escapes the alloc directory")

client/allocdir/alloc_dir_test.go

@@ -331,28 +331,30 @@ func TestAllocDir_EscapeChecking(t *testing.T) {
 
 // Test that `nomad fs` can't read secrets
 func TestAllocDir_ReadAt_SecretDir(t *testing.T) {
-	tmp, err := ioutil.TempDir("", "AllocDir")
-	if err != nil {
-		t.Fatalf("Couldn't create temp dir: %v", err)
-	}
-	defer os.RemoveAll(tmp)
+	tmp := t.TempDir()
 
 	d := NewAllocDir(testlog.HCLogger(t), tmp)
-	if err := d.Build(); err != nil {
-		t.Fatalf("Build() failed: %v", err)
-	}
-	defer d.Destroy()
+	err := d.Build()
+	require.NoError(t, err)
+	defer func() {
+		_ = d.Destroy()
+	}()
 
 	td := d.NewTaskDir(t1.Name)
-	if err := td.Build(false, nil); err != nil {
-		t.Fatalf("TaskDir.Build() failed: %v", err)
-	}
+	err = td.Build(false, nil)
+	require.NoError(t, err)
 
-	// ReadAt of secret dir should fail
-	secret := filepath.Join(t1.Name, TaskSecrets, "test_file")
-	if _, err := d.ReadAt(secret, 0); err == nil || !strings.Contains(err.Error(), "secret file prohibited") {
-		t.Fatalf("ReadAt of secret file didn't error: %v", err)
-	}
+	// something to write and test reading
+	target := filepath.Join(t1.Name, TaskSecrets, "test_file")
+
+	// create target file in the task secrets dir
+	full := filepath.Join(d.AllocDir, target)
+	err = ioutil.WriteFile(full, []byte("hi"), 0600)
+	require.NoError(t, err)
+
+	// ReadAt of a file in the task secrets dir should fail
+	_, err = d.ReadAt(target, 0)
+	require.EqualError(t, err, "Reading secret file prohibited: web/secrets/test_file")
 }
 
 func TestAllocDir_SplitPath(t *testing.T) {

client/allocrunner/taskrunner/getter/getter.go

@@ -6,22 +6,20 @@ import (
 	"net/http"
 	"net/url"
 	"strings"
-	"sync"
 
+	"github.com/hashicorp/go-cleanhttp"
 	gg "github.com/hashicorp/go-getter"
 
 	"github.com/hashicorp/nomad/nomad/structs"
 )
 
-var (
-	// getters is the map of getters suitable for Nomad. It is initialized once
-	// and the lock is used to guard access to it.
-	getters map[string]gg.Getter
-	lock    sync.Mutex
-
-	// supported is the set of download schemes supported by Nomad
-	supported = []string{"http", "https", "s3", "hg", "git", "gcs"}
-)
+// httpClient is a shared HTTP client for use across all http/https Getter
+// instantiations. The HTTP client is designed to be thread-safe, and using a pooled
+// transport will help reduce excessive connections when clients are downloading lots
+// of artifacts.
+var httpClient = &http.Client{
+	Transport: cleanhttp.DefaultPooledTransport(),
+}
 
 const (
 	// gitSSHPrefix is the prefix for downloading via git using ssh
@@ -35,53 +33,36 @@ type EnvReplacer interface {
 	ClientPath(string, bool) (string, bool)
 }
 
-func makeGetters(headers http.Header) map[string]gg.Getter {
-	getters := make(map[string]gg.Getter, len(supported))
-	for _, getter := range supported {
-		switch {
-		case getter == "http" && len(headers) > 0:
-			fallthrough
-		case getter == "https" && len(headers) > 0:
-			getters[getter] = &gg.HttpGetter{
-				Netrc:  true,
-				Header: headers,
-			}
-		default:
-			if defaultGetter, ok := gg.Getters[getter]; ok {
-				getters[getter] = defaultGetter
-			}
-		}
-	}
-	return getters
-}
-
 // getClient returns a client that is suitable for Nomad downloading artifacts.
 func getClient(src string, headers http.Header, mode gg.ClientMode, dst string) *gg.Client {
-	client := &gg.Client{
-		Src:   src,
-		Dst:   dst,
-		Mode:  mode,
-		Umask: 060000000,
+	return &gg.Client{
+		Src:     src,
+		Dst:     dst,
+		Mode:    mode,
+		Umask:   060000000,
+		Getters: createGetters(headers),
 	}
+}
 
-	switch len(headers) {
-	case 0:
-		// When no headers are present use the memoized getters, creating them
-		// on demand if they do not exist yet.
-		lock.Lock()
-		if getters == nil {
-			getters = makeGetters(nil)
-		}
-		lock.Unlock()
-		client.Getters = getters
-	default:
-		// When there are headers present, we must create fresh gg.HttpGetter
-		// objects, because that is where gg stores the headers to use in its
-		// artifact HTTP GET requests.
-		client.Getters = makeGetters(headers)
+func createGetters(header http.Header) map[string]gg.Getter {
+	httpGetter := &gg.HttpGetter{
+		Netrc:  true,
+		Client: httpClient,
+		Header: header,
+	}
+	// Explicitly create fresh set of supported Getter for each Client, because
+	// go-getter is not thread-safe. Use a shared HTTP client for http/https Getter,
+	// with pooled transport which is thread-safe.
+	//
+	// If a getter type is not listed here, it is not supported (e.g. file).
+	return map[string]gg.Getter{
+		"git":   new(gg.GitGetter),
+		"gcs":   new(gg.GCSGetter),
+		"hg":    new(gg.HgGetter),
+		"s3":    new(gg.S3Getter),
+		"http":  httpGetter,
+		"https": httpGetter,
 	}
-
-	return client
 }
 
 // getGetterUrl returns the go-getter URL to download the artifact.

command/agent/agent_endpoint.go

@@ -15,7 +15,6 @@ import (
 	"github.com/docker/docker/pkg/ioutils"
 	log "github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/go-msgpack/codec"
-	"github.com/hashicorp/nomad/acl"
 	cstructs "github.com/hashicorp/nomad/client/structs"
 	"github.com/hashicorp/nomad/command/agent/host"
 	"github.com/hashicorp/nomad/command/agent/pprof"
@@ -59,24 +58,7 @@ func (s *HTTPServer) AgentSelfRequest(resp http.ResponseWriter, req *http.Reques
 		return nil, CodedError(405, ErrInvalidMethod)
 	}
 
-	var secret string
-	s.parseToken(req, &secret)
-
-	var aclObj *acl.ACL
-	var err error
-
-	// Get the member as a server
-	var member serf.Member
-	if srv := s.agent.Server(); srv != nil {
-		member = srv.LocalMember()
-		aclObj, err = srv.ResolveToken(secret)
-	} else {
-		// Not a Server, so use the Client for token resolution. Note
-		// this gets forwarded to a server with AllowStale = true if
-		// the local ACL cache TTL has expired (30s by default)
-		aclObj, err = s.agent.Client().ResolveToken(secret)
-	}
-
+	aclObj, err := s.ResolveToken(req)
 	if err != nil {
 		return nil, err
 	}
@@ -86,6 +68,12 @@ func (s *HTTPServer) AgentSelfRequest(resp http.ResponseWriter, req *http.Reques
 		return nil, structs.ErrPermissionDenied
 	}
 
+	// Get the member as a server
+	var member serf.Member
+	if srv := s.agent.Server(); srv != nil {
+		member = srv.LocalMember()
+	}
+
 	self := agentSelf{
 		Member: nomadMember(member),
 		Stats:  s.agent.Stats(),
@@ -668,26 +656,18 @@ func (s *HTTPServer) AgentHostRequest(resp http.ResponseWriter, req *http.Reques
 		return nil, CodedError(405, ErrInvalidMethod)
 	}
 
-	var secret string
-	s.parseToken(req, &secret)
+	aclObj, err := s.ResolveToken(req)
+	if err != nil {
+		return nil, err
+	}
 
 	// Check agent read permissions
-	var aclObj *acl.ACL
 	var enableDebug bool
-	var err error
 	if srv := s.agent.Server(); srv != nil {
-		aclObj, err = srv.ResolveToken(secret)
 		enableDebug = srv.GetConfig().EnableDebug
 	} else {
-		// Not a Server, so use the Client for token resolution. Note
-		// this gets forwarded to a server with AllowStale = true if
-		// the local ACL cache TTL has expired (30s by default)
-		aclObj, err = s.agent.Client().ResolveToken(secret)
 		enableDebug = s.agent.Client().GetConfig().EnableDebug
 	}
-	if err != nil {
-		return nil, err
-	}
 
 	if (aclObj != nil && !aclObj.AllowAgentRead()) ||
 		(aclObj == nil && !enableDebug) {