auth: add client-only ACL

.semgrep/rpc_endpoint.yml

@@ -40,17 +40,38 @@ rules:
           }
           ...
 
-      # Pattern used by endpoints that are used by both ACLs and Clients.
-      # These endpoints will always have a ctx passed to Authenticate
+
+      # Pattern used by endpoints that are used only for client-to-server.
+      # Authorization can be done after forwarding, but must check the
+      # AllowClientOp policy
       - pattern-not-inside: |
-          authErr := $A.$B.Authenticate($A.ctx, args)
+          aclObj, err := $A.$B.AuthenticateClientOnly($A.ctx, args)
+          ...
+          if done, err := $A.$B.forward($METHOD, ...); done {
+            return err
+          }
+          ...
+          if !aclObj.AllowClientOp() {
+            return structs.ErrPermissionDenied
+          }
+          ...
+
+
+      # Pattern used by endpoints that are used only for client-to-server.
+      # Authorization can be done after forwarding, but must check the
+      # AllowClientOp policy. This should not be added to any new endpoints.
+      - pattern-not-inside: |
+          aclObj, err := $A.$B.AuthenticateClientOnlyLegacy($A.ctx, args)
           ...
           if done, err := $A.$B.forward($METHOD, ...); done {
             return err
           }
           ...
-          ... := $A.$B.ResolveClientOrACL(...)
+          if !aclObj.AllowClientOp() {
+            return structs.ErrPermissionDenied
+          }
           ...
+
       # Pattern used by ACL endpoints that need to interact with the token directly
       - pattern-not-inside: |
           authErr := $A.$B.Authenticate($A.ctx, args)

acl/acl.go

@@ -78,6 +78,7 @@ type ACL struct {
 
 	// The attributes below detail a virtual policy that we never expose
 	// directly to the end user.
+	client   string
 	server   string
 	isLeader bool
 }
@@ -301,6 +302,7 @@ func NewACL(management bool, policies []*Policy) (*ACL, error) {
 	acl.variables = svTxn.Commit()
 	acl.wildcardVariables = wsvTxn.Commit()
 
+	acl.client = PolicyDeny
 	acl.server = PolicyDeny
 	acl.isLeader = false
 
@@ -332,6 +334,11 @@ func (a *ACL) AllowNamespaceOperation(ns string, op string) bool {
 		return true
 	}
 
+	// Clients need to be able to read their namespaced objects
+	if a.client != PolicyDeny {
+		return true
+	}
+
 	// If using the all namespaces wildcard, allow if any namespace allows the
 	// operation.
 	if ns == AllNamespacesSentinel && a.anyNamespaceAllowsOp(op) {
@@ -731,6 +738,9 @@ func (a *ACL) AllowNodeRead() bool {
 		return true
 	case a.node == PolicyRead:
 		return true
+	case a.client == PolicyRead,
+		a.client == PolicyWrite:
+		return true
 	case a.server == PolicyRead,
 		a.server == PolicyWrite:
 		return true
@@ -813,6 +823,9 @@ func (a *ACL) AllowPluginRead() bool {
 		return true
 	case a.management:
 		return true
+	case a.client == PolicyRead,
+		a.client == PolicyWrite:
+		return true
 	case a.plugin == PolicyRead:
 		return true
 	default:
@@ -828,6 +841,9 @@ func (a *ACL) AllowPluginList() bool {
 		return true
 	case a.management:
 		return true
+	case a.client == PolicyRead,
+		a.client == PolicyWrite:
+		return true
 	case a.plugin == PolicyList:
 		return true
 	case a.plugin == PolicyRead:
@@ -837,6 +853,15 @@ func (a *ACL) AllowPluginList() bool {
 	}
 }
 
+func (a *ACL) AllowServiceRegistrationReadList(ns string, isWorkload bool) bool {
+	if a == nil {
+		// ACL is nil only if ACLs are disabled
+		// TODO(tgross): return false when there are no nil ACLs
+		return true
+	}
+	return isWorkload || a.AllowNsOp(ns, NamespaceCapabilityReadJob)
+}
+
 // AllowServerOp checks if server-only operations are allowed
 func (a *ACL) AllowServerOp() bool {
 	if a == nil {
@@ -847,6 +872,15 @@ func (a *ACL) AllowServerOp() bool {
 	return a.server != PolicyDeny || a.isLeader
 }
 
+func (a *ACL) AllowClientOp() bool {
+	if a == nil {
+		// ACL is nil only if ACLs are disabled
+		// TODO(tgross): return false when there are no nil ACLs
+		return true
+	}
+	return a.client != PolicyDeny
+}
+
 // IsManagement checks if this represents a management token
 func (a *ACL) IsManagement() bool {
 	return a.management
@@ -856,14 +890,18 @@ func (a *ACL) IsManagement() bool {
 // a list of operations. Returns true (allowed) if acls are disabled or if
 // *any* capabilities match.
 func NamespaceValidator(ops ...string) func(*ACL, string) bool {
-	return func(acl *ACL, ns string) bool {
+	return func(a *ACL, ns string) bool {
 		// Always allow if ACLs are disabled.
-		if acl == nil {
+		if a == nil {
+			return true
+		}
+		// Clients need to be able to read namespaced objects
+		if a.client != PolicyDeny {
 			return true
 		}
 
 		for _, op := range ops {
-			if acl.AllowNamespaceOperation(ns, op) {
+			if a.AllowNamespaceOperation(ns, op) {
 				// An operation is allowed, return true
 				return true
 			}

acl/acl_test.go

@@ -100,6 +100,7 @@ func TestACLManagement(t *testing.T) {
 	must.True(t, acl.AllowQuotaRead())
 	must.True(t, acl.AllowQuotaWrite())
 	must.True(t, acl.AllowServerOp())
+	must.True(t, acl.AllowClientOp())
 }
 
 func TestACLMerge(t *testing.T) {
@@ -143,6 +144,7 @@ func TestACLMerge(t *testing.T) {
 	must.True(t, acl.AllowQuotaRead())
 	must.True(t, acl.AllowQuotaWrite())
 	must.False(t, acl.AllowServerOp())
+	must.False(t, acl.AllowClientOp())
 
 	// Merge read + blank
 	p3, err := Parse("")
@@ -178,6 +180,7 @@ func TestACLMerge(t *testing.T) {
 	must.True(t, acl.AllowQuotaRead())
 	must.False(t, acl.AllowQuotaWrite())
 	must.False(t, acl.AllowServerOp())
+	must.False(t, acl.AllowClientOp())
 
 	// Merge read + deny
 	p4, err := Parse(denyAll)

acl/virtual.go

@@ -3,8 +3,20 @@
 
 package acl
 
+var ClientACL = initClientACL()
 var ServerACL = initServerACL()
 
+func initClientACL() *ACL {
+	aclObj, err := NewACL(false, []*Policy{})
+	if err != nil {
+		panic(err)
+	}
+	aclObj.client = PolicyWrite
+	aclObj.agent = PolicyRead
+	aclObj.server = PolicyRead
+	return aclObj
+}
+
 func initServerACL() *ACL {
 	aclObj, err := NewACL(false, []*Policy{})
 	if err != nil {

client/widmgr/widmgr_int_test.go

@@ -30,7 +30,7 @@ func TestWIDMgr(t *testing.T) {
 	t.Cleanup(ta.Shutdown)
 
 	mgr := widmgr.NewSigner(widmgr.SignerConfig{
-		NodeSecret: uuid.Generate(), // not checked when ACLs disabled
+		NodeSecret: ta.Client().Node().SecretID,
 		Region:     "global",
 		RPC:        ta,
 	})

command/agent/event_endpoint_test.go

@@ -212,7 +212,7 @@ func TestHTTP_Alloc_Port_Response(t *testing.T) {
 	ci.Parallel(t)
 
 	httpTest(t, nil, func(srv *TestAgent) {
-		client := srv.Client()
+		client := srv.APIClient()
 		defer srv.Shutdown()
 		defer client.Close()
 

command/agent/plugins_test.go

@@ -31,6 +31,6 @@ func testServer(t *testing.T, runClient bool, cb func(*Config)) (*TestAgent, *ap
 	})
 	t.Cleanup(a.Shutdown)
 
-	c := a.Client()
+	c := a.APIClient()
 	return a, c, a.HTTPAddr()
 }

command/agent/testagent.go

@@ -309,7 +309,7 @@ func (a *TestAgent) HTTPAddr() string {
 	return proto + a.Server.Addr
 }
 
-func (a *TestAgent) Client() *api.Client {
+func (a *TestAgent) APIClient() *api.Client {
 	conf := api.DefaultConfig()
 	conf.Address = a.HTTPAddr()
 	c, err := api.NewClient(conf)

command/operator_debug_test.go

@@ -898,7 +898,7 @@ func testServerWithoutLeader(t *testing.T, runClient bool, cb func(*agent.Config
 	})
 	t.Cleanup(func() { a.Shutdown() })
 
-	c := a.Client()
+	c := a.APIClient()
 	return a, c, a.HTTPAddr()
 }
 

command/operator_scheduler_set_config_test.go

@@ -22,7 +22,7 @@ func TestOperatorSchedulerSetConfig_Run(t *testing.T) {
 	ui := cli.NewMockUi()
 	c := &OperatorSchedulerSetConfig{Meta: Meta{Ui: ui}}
 
-	bootstrappedConfig, _, err := srv.Client().Operator().SchedulerGetConfiguration(nil)
+	bootstrappedConfig, _, err := srv.APIClient().Operator().SchedulerGetConfiguration(nil)
 	require.NoError(t, err)
 	require.NotEmpty(t, bootstrappedConfig.SchedulerConfig)
 
@@ -34,7 +34,7 @@ func TestOperatorSchedulerSetConfig_Run(t *testing.T) {
 
 	// Read the configuration again and test that nothing has changed which
 	// ensures our empty flags are working correctly.
-	nonModifiedConfig, _, err := srv.Client().Operator().SchedulerGetConfiguration(nil)
+	nonModifiedConfig, _, err := srv.APIClient().Operator().SchedulerGetConfiguration(nil)
 	require.NoError(t, err)
 	schedulerConfigEquals(t, bootstrappedConfig.SchedulerConfig, nonModifiedConfig.SchedulerConfig)
 
@@ -56,7 +56,7 @@ func TestOperatorSchedulerSetConfig_Run(t *testing.T) {
 	s := ui.OutputWriter.String()
 	require.Contains(t, s, "Scheduler configuration updated!")
 
-	modifiedConfig, _, err := srv.Client().Operator().SchedulerGetConfiguration(nil)
+	modifiedConfig, _, err := srv.APIClient().Operator().SchedulerGetConfiguration(nil)
 	require.NoError(t, err)
 	schedulerConfigEquals(t, &api.SchedulerConfiguration{
 		SchedulerAlgorithm: "spread",

command/testing_test.go

@@ -31,7 +31,7 @@ func testServer(t *testing.T, runClient bool, cb func(*agent.Config)) (*agent.Te
 	})
 	t.Cleanup(a.Shutdown)
 
-	c := a.Client()
+	c := a.APIClient()
 	return a, c, a.HTTPAddr()
 }
 
@@ -46,7 +46,7 @@ func testClient(t *testing.T, name string, cb func(*agent.Config)) (*agent.TestA
 	})
 	t.Cleanup(a.Shutdown)
 
-	c := a.Client()
+	c := a.APIClient()
 	t.Logf("Waiting for client %s to join server(s) %s", name, a.GetConfig().Client.Servers)
 	testutil.WaitForClient(t, a.Agent.RPC, a.Agent.Client().NodeID(), a.Agent.Client().Region())
 

command/var_lock_test.go

@@ -101,7 +101,7 @@ func TestVarLockCommand_Good(t *testing.T) {
 	code := cmd.Run([]string{"-address=" + url, "test/var/shell", "touch ", filePath})
 	require.Equal(t, 0, code, "expected exit 0, got: %d; %v", code, ui.ErrorWriter.String())
 
-	sv, _, err := srv.Client().Variables().Peek("test/var/shell", nil)
+	sv, _, err := srv.APIClient().Variables().Peek("test/var/shell", nil)
 	must.NoError(t, err)
 
 	must.NotNil(t, sv)
@@ -135,7 +135,7 @@ func TestVarLockCommand_Good_NoShell(t *testing.T) {
 	code := cmd.Run([]string{"-address=" + url, "-shell=false", "test/var/noShell", "touch", filePath})
 	require.Zero(t, 0, code)
 
-	sv, _, err := srv.Client().Variables().Peek("test/var/noShell", nil)
+	sv, _, err := srv.APIClient().Variables().Peek("test/var/noShell", nil)
 	must.NoError(t, err)
 
 	must.NotNil(t, sv)

nomad/acl.go

@@ -16,6 +16,14 @@ func (srv *Server) AuthenticateServerOnly(ctx *RPCContext, args structs.RequestW
 	return srv.auth.AuthenticateServerOnly(ctx, args)
 }
 
+func (srv *Server) AuthenticateClientOnly(ctx *RPCContext, args structs.RequestWithIdentity) (*acl.ACL, error) {
+	return srv.auth.AuthenticateClientOnly(ctx, args)
+}
+
+func (srv *Server) AuthenticateClientOnlyLegacy(ctx *RPCContext, args structs.RequestWithIdentity) (*acl.ACL, error) {
+	return srv.auth.AuthenticateClientOnlyLegacy(ctx, args)
+}
+
 func (srv *Server) ResolveACL(args structs.RequestWithIdentity) (*acl.ACL, error) {
 	return srv.auth.ResolveACL(args)
 }
@@ -28,10 +36,6 @@ func (srv *Server) ResolveToken(secretID string) (*acl.ACL, error) {
 	return srv.auth.ResolveToken(secretID)
 }
 
-func (srv *Server) ResolveClientOrACL(args structs.RequestWithIdentity) (*acl.ACL, error) {
-	return srv.auth.ResolveClientOrACL(args)
-}
-
 func (srv *Server) ResolvePoliciesForClaims(claims *structs.IdentityClaims) ([]*structs.ACLPolicy, error) {
 	return srv.auth.ResolvePoliciesForClaims(claims)
 }