luxolus
/
open-nomad
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

api: prevent excessice CPU load on job parse 

Add new namespace ACL requirement for the /v1/jobs/parse endpoint and
return early if HCLv2 parsing fails.

The endpoint now requires the new `parse-job` ACL capability or
`submit-job`.
This commit is contained in:
Luiz Aoqui 2022-02-01 18:54:53 -05:00
parent 437bb4b86d
commit 15f9d54dea
No known key found for this signature in database
GPG Key ID: 29F459C0D9CBB573
10 changed files with 301 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.changelog/12038.txt Normal file
View File

@ -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)
```

4
acl/policy.go
View File

@ -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,

3
acl/policy_test.go
View File

@ -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,

52
command/agent/agent_endpoint.go
View File

@ -16,7 +16,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"
"github.com/hashicorp/nomad/api"
cstructs "github.com/hashicorp/nomad/client/structs"
"github.com/hashicorp/nomad/command/agent/host"
@ -62,24 +61,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
}
@ -89,6 +71,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(),
@ -671,27 +659,19 @@ func (s *HTTPServer) AgentHostRequest(resp http.ResponseWriter, req *http.Reques
return nil, CodedError(405, ErrInvalidMethod)
}
var secret string
s.parseToken(req, &secret)
// 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
}
aclObj, err := s.ResolveToken(req)
if err != nil {
return nil, err
}
// Check agent read permissions
var enableDebug bool
if srv := s.agent.Server(); srv != nil {
enableDebug = srv.GetConfig().EnableDebug
} else {
enableDebug = s.agent.Client().GetConfig().EnableDebug
}
if (aclObj != nil && !aclObj.AllowAgentRead()) ||
(aclObj == nil && !enableDebug) {
return nil, structs.ErrPermissionDenied

26
command/agent/http.go
View File

@ -23,6 +23,7 @@ import (
multierror "github.com/hashicorp/go-multierror"
"github.com/rs/cors"
"github.com/hashicorp/nomad/acl"
"github.com/hashicorp/nomad/helper/noxssrw"
"github.com/hashicorp/nomad/helper/tlsutil"
"github.com/hashicorp/nomad/nomad/structs"
@ -270,6 +271,31 @@ func (s *HTTPServer) Shutdown() {
}
}
// ResolveToken extracts the ACL token secret ID from the request and
// translates it into an ACL object. Returns nil if ACLs are disabled.
func (s *HTTPServer) ResolveToken(req *http.Request) (*acl.ACL, error) {
var secret string
s.parseToken(req, &secret)
var aclObj *acl.ACL
var err error
if srv := s.agent.Server(); srv != nil {
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)
}
if err != nil {
return nil, fmt.Errorf("failed to resolve ACL token: %v", err)
}
return aclObj, nil
}
// registerHandlers is used to attach our handlers to the mux
func (s HTTPServer) registerHandlers(enableDebug bool) {
s.mux.HandleFunc("/v1/jobs", s.wrap(s.JobsRequest))

58
command/agent/http_test.go
View File

@ -23,6 +23,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/hashicorp/nomad/acl"
"github.com/hashicorp/nomad/api"
"github.com/hashicorp/nomad/helper"
"github.com/hashicorp/nomad/helper/testlog"
@ -1315,6 +1316,57 @@ func TestHTTPServer_Limits_OK(t *testing.T) {
}
}
func TestHTTPServer_ResolveToken(t *testing.T) {
t.Parallel()
// Setup two servers, one with ACL enabled and another with ACL disabled.
noACLServer := makeHTTPServer(t, func(c *Config) {
c.ACL = &ACLConfig{Enabled: false}
})
defer noACLServer.Shutdown()
ACLServer := makeHTTPServer(t, func(c *Config) {
c.ACL = &ACLConfig{Enabled: true}
})
defer ACLServer.Shutdown()
// Register sample token.
state := ACLServer.Agent.server.State()
token := mock.CreatePolicyAndToken(t, state, 1000, "node", mock.NodePolicy(acl.PolicyWrite))
// Tests cases.
t.Run("acl disabled", func(t *testing.T) {
req := &http.Request{Body: http.NoBody}
got, err := noACLServer.Server.ResolveToken(req)
require.NoError(t, err)
require.Nil(t, got)
})
t.Run("token not found", func(t *testing.T) {
req := &http.Request{
Body: http.NoBody,
Header: make(map[string][]string),
}
setToken(req, mock.ACLToken())
got, err := ACLServer.Server.ResolveToken(req)
require.Nil(t, got)
require.Error(t, err)
require.Contains(t, err.Error(), "ACL token not found")
})
t.Run("set token", func(t *testing.T) {
req := &http.Request{
Body: http.NoBody,
Header: make(map[string][]string),
}
setToken(req, token)
got, err := ACLServer.Server.ResolveToken(req)
require.NoError(t, err)
require.NotNil(t, got)
require.True(t, got.AllowNodeWrite())
})
}
func Test_IsAPIClientError(t *testing.T) {
trueCases := []int{400, 403, 404, 499}
for _, c := range trueCases {
@ -1410,6 +1462,12 @@ func setToken(req *http.Request, token *structs.ACLToken) {
req.Header.Set("X-Nomad-Token", token.SecretID)
}
func setNamespace(req *http.Request, ns string) {
q := req.URL.Query()
q.Add("namespace", ns)
req.URL.RawQuery = q.Encode()
}
func encodeReq(obj interface{}) io.ReadCloser {
buf := bytes.NewBuffer(nil)
enc := json.NewEncoder(buf)

21
command/agent/job_endpoint.go
View File

@ -7,6 +7,7 @@ import (
"strings"
"github.com/golang/snappy"
"github.com/hashicorp/nomad/acl"
api "github.com/hashicorp/nomad/api"
"github.com/hashicorp/nomad/helper"
"github.com/hashicorp/nomad/jobspec"
@ -703,6 +704,25 @@ func (s *HTTPServer) JobsParseRequest(resp http.ResponseWriter, req *http.Reques
return nil, CodedError(405, ErrInvalidMethod)
}
var namespace string
parseNamespace(req, &namespace)
aclObj, err := s.ResolveToken(req)
if err != nil {
return nil, err
}
// Check job parse permissions
if aclObj != nil {
hasParseJob := aclObj.AllowNsOp(namespace, acl.NamespaceCapabilityParseJob)
hasSubmitJob := aclObj.AllowNsOp(namespace, acl.NamespaceCapabilitySubmitJob)
allowed := hasParseJob || hasSubmitJob
if !allowed {
return nil, structs.ErrPermissionDenied
}
}
args := &api.JobsParseRequest{}
if err := decodeBody(req, &args); err != nil {
return nil, CodedError(400, err.Error())
@ -712,7 +732,6 @@ func (s *HTTPServer) JobsParseRequest(resp http.ResponseWriter, req *http.Reques
}
var jobStruct *api.Job
var err error
if args.HCLv1 {
jobStruct, err = jobspec.Parse(strings.NewReader(args.JobHCL))
} else {

123
command/agent/job_endpoint_test.go
View File

@ -12,6 +12,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/hashicorp/nomad/acl"
api "github.com/hashicorp/nomad/api"
"github.com/hashicorp/nomad/helper"
"github.com/hashicorp/nomad/nomad/mock"
@ -407,6 +408,128 @@ func TestHTTP_JobsParse(t *testing.T) {
}
})
}
func TestHTTP_JobsParse_ACL(t *testing.T) {
t.Parallel()
httpACLTest(t, nil, func(s *TestAgent) {
state := s.Agent.server.State()
// ACL tokens used in tests.
nodeToken := mock.CreatePolicyAndToken(
t, state, 1000, "node",
mock.NodePolicy(acl.PolicyWrite),
)
parseJobDevToken := mock.CreatePolicyAndToken(
t, state, 1002, "parse-job-dev",
mock.NamespacePolicy("dev", "", []string{"parse-job"}),
)
readNsDevToken := mock.CreatePolicyAndToken(
t, state, 1004, "read-dev",
mock.NamespacePolicy("dev", "read", nil),
)
parseJobDefaultToken := mock.CreatePolicyAndToken(
t, state, 1006, "parse-job-default",
mock.NamespacePolicy("default", "", []string{"parse-job"}),
)
submitJobDefaultToken := mock.CreatePolicyAndToken(
t, state, 1008, "submit-job-default",
mock.NamespacePolicy("default", "", []string{"submit-job"}),
)
readNsDefaultToken := mock.CreatePolicyAndToken(
t, state, 1010, "read-default",
mock.NamespacePolicy("default", "read", nil),
)
testCases := []struct {
name string
token *structs.ACLToken
namespace string
expectError bool
}{
{
name: "missing ACL token",
token: nil,
expectError: true,
},
{
name: "wrong permissions",
token: nodeToken,
expectError: true,
},
{
name: "wrong namespace",
token: readNsDevToken,
expectError: true,
},
{
name: "wrong namespace capability",
token: parseJobDevToken,
expectError: true,
},
{
name: "default namespace read",
token: readNsDefaultToken,
expectError: false,
},
{
name: "non-default namespace read",
token: readNsDevToken,
namespace: "dev",
expectError: false,
},
{
name: "default namespace parse-job capability",
token: parseJobDefaultToken,
expectError: false,
},
{
name: "default namespace submit-job capability",
token: submitJobDefaultToken,
expectError: false,
},
{
name: "non-default namespace capability",
token: parseJobDevToken,
namespace: "dev",
expectError: false,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
buf := encodeReq(api.JobsParseRequest{JobHCL: mock.HCL()})
req, err := http.NewRequest("POST", "/v1/jobs/parse", buf)
require.NoError(t, err)
if tc.namespace != "" {
setNamespace(req, tc.namespace)
}
if tc.token != nil {
setToken(req, tc.token)
}
respW := httptest.NewRecorder()
obj, err := s.Server.JobsParseRequest(respW, req)
if tc.expectError {
require.Error(t, err)
require.Equal(t, structs.ErrPermissionDenied.Error(), err.Error())
} else {
require.NoError(t, err)
require.NotNil(t, obj)
job := obj.(*api.Job)
expected := mock.Job()
require.Equal(t, expected.Name, *job.Name)
require.ElementsMatch(t, expected.Datacenters, job.Datacenters)
}
})
}
})
}
func TestHTTP_JobQuery(t *testing.T) {
t.Parallel()
httpTest(t, nil, func(s *TestAgent) {

6
jobspec2/parse.go
View File

@ -96,6 +96,12 @@ func decode(c *jobConfig) error {
diags = append(diags, ds...)
}
// Return early if the input job or variable files are not valid.
// Decoding and evaluating invalid files may result in unexpected results.
if diags.HasErrors() {
return diags
}
diags = append(diags, c.decodeBody(file.Body)...)
if diags.HasErrors() {

43
jobspec2/parse_test.go
View File

@ -374,6 +374,49 @@ job "example" {
require.Equal(t, "3", out.TaskGroups[2].Tasks[0].Meta["VERSION"])
}
func TestParse_InvalidHCL(t *testing.T) {
t.Run("invalid body", func(t *testing.T) {
hcl := `invalid{hcl`
_, err := ParseWithConfig(&ParseConfig{
Path: "input.hcl",
Body: []byte(hcl),
ArgVars: []string{},
AllowFS: true,
})
require.Error(t, err)
})
t.Run("invalid vars file", func(t *testing.T) {
tmp, err := ioutil.TempFile("", "nomad-jobspec2-")
require.NoError(t, err)
defer os.Remove(tmp.Name())
vars := `invalid{hcl`
_, err = tmp.Write([]byte(vars))
require.NoError(t, err)
hcl := `
variables {
region_var = "default"
}
job "example" {
datacenters = [for s in ["dc1", "dc2"] : upper(s)]
region = var.region_var
}
`
_, err = ParseWithConfig(&ParseConfig{
Path: "input.hcl",
Body: []byte(hcl),
VarFiles: []string{tmp.Name()},
ArgVars: []string{},
AllowFS: true,
})
require.Error(t, err)
})
}
func TestParse_InvalidScalingSyntax(t *testing.T) {
cases := []struct {
name string