From: Ming Lu Subject: CA-430073: VM.create bypasses per-key RBAC on other_config and platform protected keys This defeats the _R_POOL_ADMIN per-key gates that XSA-489 added for VM.other_config:pci and VM.platform:hvm_serial. Additionally, there are other paths that may bypass the gate: VM clone and import. Note that the API calls are forwarded to supporter hosts with session created by pool secret. It needs to be recovered to the original session for RBAC check. This is CVE-2026-102706, part of XSA-530. Signed-off-by: Ming Lu Reviewed-by: Andrii Sultanov diff --git a/ocaml/xapi/helpers.ml b/ocaml/xapi/helpers.ml index 5f1bdae2fd79..ef46341cd143 100644 --- a/ocaml/xapi/helpers.ml +++ b/ocaml/xapi/helpers.ml @@ -2579,8 +2579,8 @@ let match_protected_key objname fieldname = lower-privileged sessions must be able to manipulate a subset of entries (those not protected by a more privileged role). *) -let set_map_with_rbac ~__context ~self ~value ~get_fn ~set_fn ~match_protected - ~object_name ~field_name = +let set_map_with_rbac' ~session_id ~__context ~self ~value ~get_fn ~set_fn + ~match_protected ~object_name ~field_name = let match_protected = match_protected field_name in let module S = Set.Make (String) in let create_lookup kvs = @@ -2647,7 +2647,6 @@ let set_map_with_rbac ~__context ~self ~value ~get_fn ~set_fn ~match_protected in (* Find the first disallowed permission, indicating that we cannot perform the action. *) - let session_id = Context.get_session_id __context in match Rbac.find_first_disallowed_permission ~__context ~session_id ~permissions with @@ -2663,3 +2662,76 @@ let set_map_with_rbac ~__context ~self ~value ~get_fn ~set_fn ~match_protected (Rbac.disallowed_permission_exn ~extra_dmsg ~extra_msg ~__context ~permission:disallowed ~action ) + +let set_map_with_rbac ~__context ~self ~value ~get_fn ~set_fn ~match_protected + ~object_name ~field_name = + set_map_with_rbac' + ~session_id:(Context.get_session_id __context) + ~__context ~self ~value ~get_fn ~set_fn ~match_protected ~object_name + ~field_name + +(* Try to recover the original session from the task as it is still useful even + after passing the RBAC check in API message entry. For example, RBAC check + against some keys is only possible when the VM's metadata is received from an + input stream while the call has passed RBAC check in API message entry on + coordinator and has been forwarded to a supporter host. *) +let original_session ~__context = + match Context.get_session_id __context with + | exception Failure _ -> + (* No session, check is not needed. *) + Ok None + | session_id -> + let is_forwarding_session = + (* Quick check on local session. *) + (not (Session_check.is_local_session __context session_id)) + (* Quick check on forwarded_task as well. *) + && Context.forwarded_task __context + && + let session = Db.Session.get_record ~__context ~self:session_id in + session.API.session_pool && session.API.session_is_local_superuser + in + if is_forwarding_session then ( + let ( let* ) = Result.bind in + let* task_id = + if Context.task_in_database __context then + Ok (Context.get_task_id __context) + else ( + warn "%s: no task" __FUNCTION__ ; + Error () + ) + in + match Db.Task.get_session ~__context ~self:task_id with + | session_id when session_id <> Ref.null -> + Ok (Some session_id) + | _ -> + warn "%s: no session" __FUNCTION__ ; + Error () + | exception e -> + warn "%s: failed to get originating session: %s" __FUNCTION__ + (Printexc.to_string e) ; + Error () + ) else + Ok (Some session_id) + +(* With similar purpose of set_map_with_rbac, this is a pure checking function + to ensure the session is privileged enough to set values for the protected + keys if they present in the map [value]. *) +let assert_map_with_rbac ~__context ~match_protected ~object_name ~field_name + ~value = + match original_session ~__context with + | Ok None -> + () + | Ok (Some session_id) -> + set_map_with_rbac' ~session_id ~__context ~self:Ref.null ~value + ~get_fn:(fun ~__context:_ ~self:_ -> []) + ~set_fn:(fun ~__context:_ ~self:_ ~value:_ -> ()) + ~match_protected ~object_name ~field_name + | Error () -> + let action = Printf.sprintf "%s.set_%s" object_name field_name in + 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 067108cd8f81..b71fb7c2dbd0 100644 --- a/ocaml/xapi/import.ml +++ b/ocaml/xapi/import.ml @@ -517,6 +517,10 @@ module VM : HandlerTools = struct in match import_action with | Replace (_, vm_record) | Clean_import vm_record -> + Xapi_vm_helpers.assert_protected_map_keys_allowed ~__context + ~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 ; 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 f474ca306cb6..2573cb8af024 100644 --- a/ocaml/xapi/xapi_vm.ml +++ b/ocaml/xapi/xapi_vm.ml @@ -591,6 +591,10 @@ let create ~__context ~name_label ~name_description ~power_state ~user_version else other_config in + Xapi_vm_helpers.assert_protected_map_keys_allowed ~__context + ~field_name:"other_config" ~value:other_config ; + Xapi_vm_helpers.assert_protected_map_keys_allowed ~__context + ~field_name:"platform" ~value:platform ; (* 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 25a30eac2785..ea76589db52b 100644 --- a/ocaml/xapi/xapi_vm_clone.ml +++ b/ocaml/xapi/xapi_vm_clone.ml @@ -419,10 +419,23 @@ let copy_vm_record ?snapshot_info_record ~__context ~vm ~disk_op ~new_name let make_driver_params () = [(Constants._sm_epoch_hint, Uuidx.to_string (Uuidx.make ()))] +let assert_protected_map_keys_allowed ~__context ~vm disk_op = + match disk_op with + | Disk_op_clone | Disk_op_copy _ -> + Xapi_vm_helpers.assert_protected_map_keys_allowed ~__context + ~field_name:"other_config" + ~value:(Db.VM.get_other_config ~__context ~self:vm) ; + Xapi_vm_helpers.assert_protected_map_keys_allowed ~__context + ~field_name:"platform" + ~value:(Db.VM.get_platform ~__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 ; 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 f71ab3fd3b36..bcb5e8875cb7 100644 --- a/ocaml/xapi/xapi_vm_helpers.ml +++ b/ocaml/xapi/xapi_vm_helpers.ml @@ -1738,3 +1738,8 @@ let check_secureboot_certificates_state ~__context ~self : vm_uuid (Printexc.to_string e) ; `ok ) + +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