From: Ming Lu <ming.lu@citrix.com>
Subject: CA-430073: VM.create bypasses RBAC on HA restart priority and
 appliance

VM.create only needs the VM admin role, but it takes a whole VM record,
including fields whose setters need a higher role. Two of them are
checked in this PR:

  - ha_restart_priority: a VM with "restart" is protected by HA and takes
    resources from the pool's point of view.
  - appliance: is managed from pool's point of view as well.

There are other VM fields which currently require higher role. But one
of them, ha_always_run, is deprecated. Others seem not necessary to be
managed by higher role, or from the pool's point of view.

This is CVE-2026-102706, part of XSA-530.

Signed-off-by: Ming Lu <ming.lu@citrix.com>
Reviewed-by: Andrii Sultanov <andriy.sultanov@vates.tech>

diff --git a/ocaml/xapi/helpers.ml b/ocaml/xapi/helpers.ml
index ef46341cd143..c4cd232b1256 100644
--- a/ocaml/xapi/helpers.ml
+++ b/ocaml/xapi/helpers.ml
@@ -2735,3 +2735,28 @@ let assert_map_with_rbac ~__context ~match_protected ~object_name ~field_name
             , [action; "cannot determine the originating session"]
             )
         )
+
+(* With similar purpose of assert_map_with_rbac, check if setting a non-default
+   value on a field of an object is allowed. *)
+let assert_field_value_with_rbac ~__context ~object_name ~field_name ~is_default
+    =
+  if not is_default then
+    let action = Printf.sprintf "%s.set_%s" object_name field_name in
+    let permission = String.lowercase_ascii action in
+    match original_session ~__context with
+    | Ok None ->
+        ()
+    | Ok (Some session_id) ->
+        if not (Rbac.is_access_allowed ~__context ~session_id ~permission) then
+          raise
+            (Rbac.disallowed_permission_exn ~extra_dmsg:"" ~extra_msg:""
+               ~__context ~permission ~action
+            )
+    | Error () ->
+        raise
+          Api_errors.(
+            Server_error
+              ( rbac_permission_denied
+              , [action; "cannot determine the originating session"]
+              )
+          )
diff --git a/ocaml/xapi/import.ml b/ocaml/xapi/import.ml
index b71fb7c2dbd0..16ebb4c9264c 100644
--- a/ocaml/xapi/import.ml
+++ b/ocaml/xapi/import.ml
@@ -521,6 +521,10 @@ module VM : HandlerTools = struct
               ~field_name:"other_config" ~value:vm_record.API.vM_other_config ;
             Xapi_vm_helpers.assert_protected_map_keys_allowed ~__context
               ~field_name:"platform" ~value:vm_record.API.vM_platform ;
+            (* The appliance is cleared on import, so it isn't checked. *)
+            Xapi_vm_helpers.assert_protected_field_allowed ~__context
+              ~field_name:"ha_restart_priority"
+              ~is_default:(vm_record.API.vM_ha_restart_priority = "") ;
             if is_live config then assert_can_live_import __context vm_record ;
             ( if needs_cpu_check config vm_record then
                 let vmm_record =
diff --git a/ocaml/xapi/xapi_vm.ml b/ocaml/xapi/xapi_vm.ml
index 2573cb8af024..da87a18752c4 100644
--- a/ocaml/xapi/xapi_vm.ml
+++ b/ocaml/xapi/xapi_vm.ml
@@ -595,6 +595,10 @@ let create ~__context ~name_label ~name_description ~power_state ~user_version
     ~field_name:"other_config" ~value:other_config ;
   Xapi_vm_helpers.assert_protected_map_keys_allowed ~__context
     ~field_name:"platform" ~value:platform ;
+  Xapi_vm_helpers.assert_protected_field_allowed ~__context
+    ~field_name:"ha_restart_priority" ~is_default:(ha_restart_priority = "") ;
+  Xapi_vm_helpers.assert_protected_field_allowed ~__context
+    ~field_name:"appliance" ~is_default:(appliance = Ref.null) ;
   (* NB apart from the above, parameter validation is delayed until VM.start *)
   let uuid = Uuidx.make () in
   let vm_ref = Ref.make () in
diff --git a/ocaml/xapi/xapi_vm_clone.ml b/ocaml/xapi/xapi_vm_clone.ml
index ea76589db52b..3d62851e5209 100644
--- a/ocaml/xapi/xapi_vm_clone.ml
+++ b/ocaml/xapi/xapi_vm_clone.ml
@@ -431,11 +431,22 @@ let assert_protected_map_keys_allowed ~__context ~vm disk_op =
   | Disk_op_snapshot | Disk_op_checkpoint ->
       ()
 
+let assert_protected_fields_allowed ~__context ~vm disk_op =
+  match disk_op with
+  | Disk_op_clone | Disk_op_copy _ ->
+      (* The appliance of the new VM is reset, so it isn't checked. *)
+      Xapi_vm_helpers.assert_protected_field_allowed ~__context
+        ~field_name:"ha_restart_priority"
+        ~is_default:(Db.VM.get_ha_restart_priority ~__context ~self:vm = "")
+  | Disk_op_snapshot | Disk_op_checkpoint ->
+      ()
+
 (* NB this function may be called when the VM is suspended for copy/clone
    operations. Snapshot can be done in live. *)
 let clone ?snapshot_info_record ?(ignore_vdis = []) disk_op ~__context ~vm
     ~new_name =
   assert_protected_map_keys_allowed ~__context ~vm disk_op ;
+  assert_protected_fields_allowed ~__context ~vm disk_op ;
   Helpers.call_api_functions ~__context (fun rpc session_id ->
       let task_id = Ref.string_of (Context.get_task_id __context) in
       let vbds = Db.VM.get_VBDs ~__context ~self:vm in
diff --git a/ocaml/xapi/xapi_vm_helpers.ml b/ocaml/xapi/xapi_vm_helpers.ml
index bcb5e8875cb7..c6270a6a8245 100644
--- a/ocaml/xapi/xapi_vm_helpers.ml
+++ b/ocaml/xapi/xapi_vm_helpers.ml
@@ -1743,3 +1743,7 @@ let assert_protected_map_keys_allowed ~__context ~field_name ~value =
   let match_protected = Helpers.match_protected_key "VM" in
   Helpers.assert_map_with_rbac ~__context ~match_protected ~object_name:"vm"
     ~field_name ~value
+
+let assert_protected_field_allowed ~__context ~field_name ~is_default =
+  Helpers.assert_field_value_with_rbac ~__context ~object_name:"vm" ~field_name
+    ~is_default