secure vars: enforce ENT quotas (OSS work) (#13951)

Move the secure variables quota enforcement calls into the state store to ensure
quota checks are atomic with quota updates (in the same transaction).

Switch to a machine-size int instead of a uint64 for quota tracking. The
ENT-side quota spec is described as int, and negative values have a meaning as
"not permitted at all". Using the same type for tracking will make it easier to
the math around checks, and uint64 is infeasibly large anyways.

Add secure vars to quota HTTP API and CLI outputs and API docs.

api/quota.go

@@ -126,6 +126,12 @@ type QuotaLimit struct {
 	// useful for once we support GPUs
 	RegionLimit *Resources
 
+	// SecureVariablesLimit is the maximum total size of all secure
+	// variables SecureVariable.EncryptedData. A value of zero is
+	// treated as unlimited and a negative value is treated as fully
+	// disallowed.
+	SecureVariablesLimit *int `mapstructure:"secure_variables_limit" hcl:"secure_variables_limit,optional"`
+
 	// Hash is the hash of the object and is used to make replication efficient.
 	Hash []byte
 }

command/agent/search_endpoint_test.go

@@ -701,9 +701,9 @@ func TestHTTP_PrefixSearch_SecureVariables_ACL(t *testing.T) {
 		sv2 := sv1.Copy()
 		sv2.Namespace = ns.Name
 
-		_ = state.UpsertNamespaces(7000, []*structs.Namespace{ns})
-		_ = state.UpsertSecureVariables(structs.MsgTypeTestSetup, 8000, []*structs.SecureVariableEncrypted{sv1})
-		_ = state.UpsertSecureVariables(structs.MsgTypeTestSetup, 8001, []*structs.SecureVariableEncrypted{&sv2})
+		require.NoError(t, state.UpsertNamespaces(7000, []*structs.Namespace{ns}))
+		require.NoError(t, state.UpsertSecureVariables(structs.MsgTypeTestSetup, 8000, []*structs.SecureVariableEncrypted{sv1}))
+		require.NoError(t, state.UpsertSecureVariables(structs.MsgTypeTestSetup, 8001, []*structs.SecureVariableEncrypted{&sv2}))
 
 		rootToken := s.RootToken
 
@@ -807,9 +807,9 @@ func TestHTTP_FuzzySearch_SecureVariables_ACL(t *testing.T) {
 		sv2 := sv1.Copy()
 		sv2.Namespace = ns.Name
 
-		_ = state.UpsertNamespaces(7000, []*structs.Namespace{mock.Namespace()})
-		_ = state.UpsertSecureVariables(structs.MsgTypeTestSetup, 8000, []*structs.SecureVariableEncrypted{sv1})
-		_ = state.UpsertSecureVariables(structs.MsgTypeTestSetup, 8001, []*structs.SecureVariableEncrypted{&sv2})
+		require.NoError(t, state.UpsertNamespaces(7000, []*structs.Namespace{ns}))
+		require.NoError(t, state.UpsertSecureVariables(structs.MsgTypeTestSetup, 8000, []*structs.SecureVariableEncrypted{sv1}))
+		require.NoError(t, state.UpsertSecureVariables(structs.MsgTypeTestSetup, 8001, []*structs.SecureVariableEncrypted{&sv2}))
 
 		rootToken := s.RootToken
 		defNSToken := mock.CreatePolicyAndToken(t, state, 8002, "default", mock.NamespacePolicy("default", "read", nil))

command/quota_apply.go

@@ -198,6 +198,7 @@ func parseQuotaLimits(result *[]*api.QuotaLimit, list *ast.ObjectList) error {
 		valid := []string{
 			"region",
 			"region_limit",
+			"secure_variables_limit",
 		}
 		if err := helper.CheckHCLKeys(o.Val, valid); err != nil {
 			return err

command/quota_init.go

@@ -121,6 +121,7 @@ limit {
     memory     = 1000
     memory_max = 1000
   }
+  secure_variables_limit = 1000
 }
 `)
 
@@ -135,7 +136,8 @@ var defaultJsonQuotaSpec = strings.TrimSpace(`
 				"CPU": 2500,
 				"MemoryMB": 1000,
 				"MemoryMaxMB": 1000
-			}
+			},
+			"SecureVariablesLimit": 1000
 		}
 	]
 }

command/quota_status.go

@@ -158,7 +158,7 @@ func formatQuotaLimits(spec *api.QuotaSpec, usages map[string]*api.QuotaUsage) s
 	sort.Sort(api.QuotaLimitSort(spec.Limits))
 
 	limits := make([]string, len(spec.Limits)+1)
-	limits[0] = "Region|CPU Usage|Memory Usage|Memory Max Usage|Network Usage"
+	limits[0] = "Region|CPU Usage|Memory Usage|Memory Max Usage|Network Usage|Secure Variables Usage"
 	i := 0
 	for _, specLimit := range spec.Limits {
 		i++
@@ -185,7 +185,9 @@ func formatQuotaLimits(spec *api.QuotaSpec, usages map[string]*api.QuotaUsage) s
 			memory := fmt.Sprintf("- / %s", formatQuotaLimitInt(specLimit.RegionLimit.MemoryMB))
 			memoryMax := fmt.Sprintf("- / %s", formatQuotaLimitInt(specLimit.RegionLimit.MemoryMaxMB))
 			net := fmt.Sprintf("- / %s", formatQuotaLimitInt(&specBits))
-			limits[i] = fmt.Sprintf("%s|%s|%s|%s|%s", specLimit.Region, cpu, memory, memoryMax, net)
+
+			vars := fmt.Sprintf("- / %s", formatQuotaLimitInt(specLimit.SecureVariablesLimit))
+			limits[i] = fmt.Sprintf("%s|%s|%s|%s|%s|%s", specLimit.Region, cpu, memory, memoryMax, net, vars)
 			continue
 		}
 
@@ -204,7 +206,9 @@ func formatQuotaLimits(spec *api.QuotaSpec, usages map[string]*api.QuotaUsage) s
 		if len(used.RegionLimit.Networks) == 1 {
 			net = fmt.Sprintf("%d / %s", *used.RegionLimit.Networks[0].MBits, formatQuotaLimitInt(&specBits))
 		}
-		limits[i] = fmt.Sprintf("%s|%s|%s|%s|%s", specLimit.Region, cpu, memory, memoryMax, net)
+
+		vars := fmt.Sprintf("%d / %s", orZero(used.SecureVariablesLimit), formatQuotaLimitInt(specLimit.SecureVariablesLimit))
+		limits[i] = fmt.Sprintf("%s|%s|%s|%s|%s|%s", specLimit.Region, cpu, memory, memoryMax, net, vars)
 	}
 
 	return formatList(limits)

nomad/secure_variables_endpoint.go

@@ -79,6 +79,16 @@ func (sv *SecureVariables) Upsert(
 			mErr.Errors = append(mErr.Errors, err)
 			continue
 		}
+
+		ns, err := sv.srv.State().NamespaceByName(nil, v.Namespace)
+		if err != nil {
+			return err
+		}
+		if ns == nil {
+			return fmt.Errorf("secure variable %q is in nonexistent namespace %q",
+				v.Path, v.Namespace)
+		}
+
 		if args.CheckIndex != nil {
 			var conflict *structs.SecureVariableDecrypted
 			if err := sv.validateCASUpdate(*args.CheckIndex, v, &conflict); err != nil {
@@ -106,11 +116,6 @@ func (sv *SecureVariables) Upsert(
 		return &mErr
 	}
 
-	// TODO: This should be done on each Data in uArgs.
-	if err := sv.enforceQuota(uArgs); err != nil {
-		return err
-	}
-
 	// Update via Raft.
 	out, index, err := sv.srv.raftApply(structs.SecureVariableUpsertRequestType, uArgs)
 	if err != nil {

nomad/state/state_store_secure_variables.go

@@ -2,6 +2,7 @@ package state
 
 import (
 	"fmt"
+	"math"
 	"time"
 
 	"github.com/hashicorp/go-memdb"
@@ -146,7 +147,7 @@ func (s *StateStore) upsertSecureVariableImpl(index uint64, txn *txn, sv *struct
 		return fmt.Errorf("secure variable quota lookup failed: %v", err)
 	}
 
-	var quotaChange int
+	var quotaChange int64
 
 	// Setup the indexes correctly
 	nowNano := time.Now().UnixNano()
@@ -160,38 +161,55 @@ func (s *StateStore) upsertSecureVariableImpl(index uint64, txn *txn, sv *struct
 		sv.CreateTime = exist.CreateTime
 		sv.ModifyIndex = index
 		sv.ModifyTime = nowNano
-		quotaChange = len(sv.Data) - len(exist.Data)
+		quotaChange = int64(len(sv.Data) - len(exist.Data))
 	} else {
 		sv.CreateIndex = index
 		sv.CreateTime = nowNano
 		sv.ModifyIndex = index
 		sv.ModifyTime = nowNano
-		quotaChange = len(sv.Data)
+		quotaChange = int64(len(sv.Data))
 	}
 
 	// Insert the secure variable
 	if err := txn.Insert(TableSecureVariables, sv); err != nil {
 		return fmt.Errorf("secure variable insert failed: %v", err)
 	}
 
-	if quotaChange != 0 {
-		// Track quota usage
-		var quotaUsed *structs.SecureVariablesQuota
-		if existingQuota != nil {
-			quotaUsed = existingQuota.(*structs.SecureVariablesQuota)
-			quotaUsed = quotaUsed.Copy()
-		} else {
-			quotaUsed = &structs.SecureVariablesQuota{
-				Namespace:   sv.Namespace,
-				CreateIndex: index,
-			}
+	// Track quota usage
+	var quotaUsed *structs.SecureVariablesQuota
+	if existingQuota != nil {
+		quotaUsed = existingQuota.(*structs.SecureVariablesQuota)
+		quotaUsed = quotaUsed.Copy()
+	} else {
+		quotaUsed = &structs.SecureVariablesQuota{
+			Namespace:   sv.Namespace,
+			CreateIndex: index,
 		}
+	}
+
+	if quotaChange > math.MaxInt64-quotaUsed.Size {
+		// this limit is actually shared across all namespaces in the region's
+		// quota (if there is one), but we need this check here to prevent
+		// overflow as well
+		return fmt.Errorf("secure variables can store a maximum of %d bytes of encrypted data per namespace", math.MaxInt)
+	}
+
+	if quotaChange > 0 {
+		quotaUsed.Size += quotaChange
+	} else if quotaChange < 0 {
+		quotaUsed.Size -= helper.Min(quotaUsed.Size, -quotaChange)
+	}
+
+	err = s.enforceSecureVariablesQuota(index, txn, sv.Namespace, quotaChange)
+	if err != nil {
+		return err
+	}
+
+	// we check enforcement above even if there's no change because another
+	// namespace may have used up quota to make this no longer valid, but we
+	// only update the table if this namespace has changed
+	if quotaChange != 0 {
 		quotaUsed.ModifyIndex = index
-		if quotaChange > 0 {
-			quotaUsed.Size += uint64(quotaChange)
-		} else {
-			quotaUsed.Size -= uint64(helper.MinInt(int(quotaUsed.Size), -quotaChange))
-		}
 		if err := txn.Insert(TableSecureVariablesQuotas, quotaUsed); err != nil {
 			return fmt.Errorf("secure variable quota insert failed: %v", err)
 		}
@@ -275,7 +293,7 @@ func (s *StateStore) DeleteSecureVariableTxn(index uint64, namespace, path strin
 		quotaUsed := existingQuota.(*structs.SecureVariablesQuota)
 		quotaUsed = quotaUsed.Copy()
 		sv := existing.(*structs.SecureVariableEncrypted)
-		quotaUsed.Size -= uint64(len(sv.Data))
+		quotaUsed.Size -= helper.Min(quotaUsed.Size, int64(len(sv.Data)))
 		quotaUsed.ModifyIndex = index
 		if err := txn.Insert(TableSecureVariablesQuotas, quotaUsed); err != nil {
 			return fmt.Errorf("secure variable quota insert failed: %v", err)

nomad/state/state_store_secure_variables_oss.go

@@ -0,0 +1,8 @@
+//go:build !ent
+// +build !ent
+
+package state
+
+func (s *StateStore) enforceSecureVariablesQuota(_ uint64, _ *txn, _ string, _ int64) error {
+	return nil
+}

nomad/state/state_store_secure_variables_test.go

@@ -34,9 +34,9 @@ func TestStateStore_UpsertSecureVariables(t *testing.T) {
 	t.Log(printSecureVariables(svs))
 	insertIndex := uint64(20)
 
-	var expectedQuotaSize uint64
+	var expectedQuotaSize int
 	for _, v := range svs {
-		expectedQuotaSize += uint64(len(v.Data))
+		expectedQuotaSize += len(v.Data)
 	}
 
 	// Ensure new secure variables are inserted as expected with their
@@ -78,7 +78,7 @@ func TestStateStore_UpsertSecureVariables(t *testing.T) {
 
 		quotaUsed, err := testState.SecureVariablesQuotaByNamespace(ws, structs.DefaultNamespace)
 		require.NoError(t, err)
-		require.Equal(t, expectedQuotaSize, quotaUsed.Size)
+		require.Equal(t, int64(expectedQuotaSize), quotaUsed.Size)
 	})
 
 	svs = svm.List()
@@ -102,7 +102,7 @@ func TestStateStore_UpsertSecureVariables(t *testing.T) {
 
 		quotaUsed, err := testState.SecureVariablesQuotaByNamespace(ws, structs.DefaultNamespace)
 		require.NoError(t, err)
-		require.Equal(t, expectedQuotaSize, quotaUsed.Size)
+		require.Equal(t, int64(expectedQuotaSize), quotaUsed.Size)
 	})
 
 	// Modify a single one of the previously inserted secure variables
@@ -158,7 +158,7 @@ func TestStateStore_UpsertSecureVariables(t *testing.T) {
 
 		quotaUsed, err := testState.SecureVariablesQuotaByNamespace(ws, structs.DefaultNamespace)
 		require.NoError(t, err)
-		require.Equal(t, expectedQuotaSize+1, quotaUsed.Size)
+		require.Equal(t, int64(expectedQuotaSize+1), quotaUsed.Size)
 	})
 
 	svs = svm.List()
@@ -219,7 +219,7 @@ func TestStateStore_UpsertSecureVariables(t *testing.T) {
 
 		quotaUsed, err := testState.SecureVariablesQuotaByNamespace(ws, structs.DefaultNamespace)
 		require.NoError(t, err)
-		require.Equal(t, expectedQuotaSize+1, quotaUsed.Size)
+		require.Equal(t, int64(expectedQuotaSize+1), quotaUsed.Size)
 
 	})
 }
@@ -250,6 +250,11 @@ func TestStateStore_DeleteSecureVariable(t *testing.T) {
 	// remaining is left as expected.
 	t.Run("2 upsert variable and delete", func(t *testing.T) {
 
+		ns := mock.Namespace()
+		ns.Name = svs[0].Namespace
+		require.NoError(t, testState.UpsertNamespaces(initialIndex, []*structs.Namespace{ns}))
+
+		initialIndex++
 		require.NoError(t, testState.UpsertSecureVariables(
 			structs.MsgTypeTestSetup, initialIndex, svs))
 
@@ -270,19 +275,19 @@ func TestStateStore_DeleteSecureVariable(t *testing.T) {
 		require.NoError(t, err)
 
 		var delete1Count int
-		var expectedQuotaSize uint64
+		var expectedQuotaSize int
 
 		// Iterate all the stored variables and assert we have the expected
 		// number.
 		for raw := iter.Next(); raw != nil; raw = iter.Next() {
 			delete1Count++
 			v := raw.(*structs.SecureVariableEncrypted)
-			expectedQuotaSize += uint64(len(v.Data))
+			expectedQuotaSize += len(v.Data)
 		}
 		require.Equal(t, 1, delete1Count, "unexpected number of variables in table")
 		quotaUsed, err := testState.SecureVariablesQuotaByNamespace(ws, structs.DefaultNamespace)
 		require.NoError(t, err)
-		require.Equal(t, expectedQuotaSize, quotaUsed.Size)
+		require.Equal(t, int64(expectedQuotaSize), quotaUsed.Size)
 	})
 
 	t.Run("3 delete remaining variable", func(t *testing.T) {
@@ -310,18 +315,23 @@ func TestStateStore_DeleteSecureVariable(t *testing.T) {
 
 		quotaUsed, err := testState.SecureVariablesQuotaByNamespace(ws, structs.DefaultNamespace)
 		require.NoError(t, err)
-		require.Equal(t, uint64(0), quotaUsed.Size)
+		require.Equal(t, int64(0), quotaUsed.Size)
 	})
 }
 
 func TestStateStore_GetSecureVariables(t *testing.T) {
 	ci.Parallel(t)
 	testState := testStateStore(t)
 
+	ns := mock.Namespace()
+	ns.Name = "~*magical*~"
+	initialIndex := uint64(10)
+	require.NoError(t, testState.UpsertNamespaces(initialIndex, []*structs.Namespace{ns}))
+
 	// Generate some test secure variables and upsert them.
 	svs, _ := mockSecureVariables(2)
 	svs[0].Namespace = "~*magical*~"
-	initialIndex := uint64(10)
+	initialIndex++
 	require.NoError(t, testState.UpsertSecureVariables(structs.MsgTypeTestSetup, initialIndex, svs))
 
 	// Look up secure variables using the namespace of the first mock variable.
@@ -386,7 +396,12 @@ func TestStateStore_ListSecureVariablesByNamespaceAndPrefix(t *testing.T) {
 	svs[5].Namespace = "other"
 	svs[5].Path = "a/z/z"
 
+	ns := mock.Namespace()
+	ns.Name = "other"
 	initialIndex := uint64(10)
+	require.NoError(t, testState.UpsertNamespaces(initialIndex, []*structs.Namespace{ns}))
+
+	initialIndex++
 	require.NoError(t, testState.UpsertSecureVariables(structs.MsgTypeTestSetup, initialIndex, svs))
 
 	t.Run("ByNamespace", func(t *testing.T) {

nomad/structs/secure_variables.go

@@ -214,13 +214,14 @@ func (sv SecureVariableMetadata) GetCreateIndex() uint64 {
 	return sv.CreateIndex
 }
 
-// SecureVariablesQuota is used to track the total size of secure
-// variables entries per namespace. The total length of
-// SecureVariable.EncryptedData will be added to the SecureVariablesQuota
-// table in the same transaction as a write, update, or delete.
+// SecureVariablesQuota is used to track the total size of secure variables
+// entries per namespace. The total length of SecureVariable.EncryptedData in
+// bytes will be added to the SecureVariablesQuota table in the same transaction
+// as a write, update, or delete. This tracking effectively caps the maximum
+// size of secure variables in a given namespace to MaxInt64 bytes.
 type SecureVariablesQuota struct {
 	Namespace   string
-	Size        uint64
+	Size        int64
 	CreateIndex uint64
 	ModifyIndex uint64
 }