SV: fixes for namespace handling (#13705)

* ACL check namespace value in SecureVariable
* Error on wildcard namespace

nomad/secure_variables_endpoint.go

@@ -28,7 +28,11 @@ type SecureVariables struct {
 	encrypter *Encrypter
 }
 
-// Upsert creates or updates secure variables held within Nomad.
+// Upsert creates or updates secure variables held within Nomad. Due to ACL
+// checking, every element in Data will be checked for namespace and targeted
+// to the namespace in the SecureVariable. Therefore, the caller must ensure
+// that the provided struct's Namespace is the desired destination. Unset
+// Namespace values will default to `args.RequestNamespace`
 func (sv *SecureVariables) Upsert(
 	args *structs.SecureVariablesUpsertRequest,
 	reply *structs.SecureVariablesUpsertResponse) error {
@@ -38,18 +42,6 @@ func (sv *SecureVariables) Upsert(
 	}
 	defer metrics.MeasureSince([]string{"nomad", "secure_variables", "upsert"}, time.Now())
 
-	// Perform the ACL token resolution.
-	if aclObj, err := sv.srv.ResolveToken(args.AuthToken); err != nil {
-		return err
-	} else if aclObj != nil {
-		for _, variable := range args.Data {
-			if !aclObj.AllowSecureVariableOperation(args.RequestNamespace(),
-				variable.Path, acl.PolicyWrite) {
-				return structs.ErrPermissionDenied
-			}
-		}
-	}
-
 	// Use a multierror, so we can capture all validation errors and pass this
 	// back so they can be addressed by the caller in a single pass.
 	var mErr multierror.Error
@@ -61,6 +53,27 @@ func (sv *SecureVariables) Upsert(
 	// Iterate the secure variables and validate them. Any error results in the
 	// call failing.
 	for i, v := range args.Data {
+
+		// Check if the Namespace is explicitly set on the secure variable. If
+		// not, use the RequestNamespace
+		targetNS := v.Namespace
+		if targetNS == "" {
+			targetNS = args.RequestNamespace()
+			v.Namespace = targetNS
+		}
+
+		// Perform the ACL token resolution.
+		if aclObj, err := sv.srv.ResolveToken(args.AuthToken); err != nil {
+			return err
+		} else if aclObj != nil {
+			for _, variable := range args.Data {
+				if !aclObj.AllowSecureVariableOperation(targetNS,
+					variable.Path, acl.PolicyWrite) {
+					return structs.ErrPermissionDenied
+				}
+			}
+		}
+
 		v.Canonicalize()
 		if err := v.Validate(); err != nil {
 			mErr.Errors = append(mErr.Errors, err)
@@ -93,6 +106,7 @@ 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
 	}

nomad/structs/secure_variables.go

@@ -151,6 +151,9 @@ func (sv SecureVariableDecrypted) Validate() error {
 	if len(sv.Items) == 0 {
 		return errors.New("empty variables are invalid")
 	}
+	if sv.Namespace == AllNamespacesSentinel {
+		return errors.New("can not target wildcard (\"*\")namespace")
+	}
 	return nil
 }