New API methods for status about SecureBoot and UEFI certs #5566

benjamreis · 2024-04-17T11:19:36Z

Implement what's discussed here: #5548

New API calls:

VM.set_uefi_mode: calls varstore-sb-state to edit the uefi mode of a VM
Takes in input the uuid of a VM and a mode (setup or user)
Returns the output of the script calls
VM.get_secureboot_readiness API call

Returns the SecureBoot status of a VM:
- not_supported: VM's firmware is not UEFI
- disabled: Secureboot is disabled on this VM
- first_boot: Secured boot is enabled on this VM and its NVRAM.EFI-variables is empty
- ready: Secured boot is enabled on this VM and PK, KEK, db and dbx are defined in its EFI variables
- ready_no_dbx: Secured boot is enabled on this VM and PK, KEK, db but not dbx are defined in its EFI variables
- setup_mode: Secured boot is enabled on this VM and PK is not defined in its EFI variables
- certs_incomplete: Secured boot is enabled on this VM and the certificates defines in its EFI variables are incomplete
Pool.get_guest_secureboot_readiness API call

Returns a pool's state for guest SecureBoot:
- ready: the active pool UEFI certificates (custom ones first, default ones if no custom ones) contain PK, KEK, db and dbx
- ready_no_dbx: the active pool UEFI certificates contain PK, KEK and db but not dbx
- not_ready: otherwise

lindig

Looks ok otherwise.

ocaml/idl/datamodel_vm.ml

lindig · 2024-04-18T13:06:15Z

Can this be changed from the CLI? It looks like it can't but I think it should be observable and update-able from the CLI.

benjamreis · 2024-04-18T15:00:42Z

Can this be changed from the CLI? It looks like it can't but I think it should be observable and update-able from the CLI.

Working on it as we speak. We will need the CLI implem for our tests. :)

Edit: Done 👍

benjamreis · 2024-04-22T08:29:14Z

ocaml/xapi/xapi_pool.ml

+  let tmp_auth_file = Filename.concat tmp_dir filename in
+  Helpers.touch_file tmp_auth_file ; tmp_auth_file
+
+let get_guest_secureboot_readiness ~__context ~self =


This method is a bit complex as it needs to extract the tarball in a pool (custom)_uefi_certificates db field in a temp directory and then list them before removing the extracted files...

Another (simpler) solution would be to list auth files in varstore_dir as it's supposed to always be in sync with the XAPI DB field unless a user would have manually modified it which wouldn't be supported.
What do you guys think?

As long as its limitations are documented, I don't mind using the method that doesn't rely on creating files

benjamreis · 2024-05-07T08:54:20Z

Hi!

Any update on this? It's really important for us to have in our next XCP-ng release and we'd like to start implementing the tests and clients side once we're assured the API won't move too much.

Thx

lindig · 2024-05-07T09:05:51Z

ocaml/idl/datamodel_vm.ml

+           variables"
+        )
+      ; ( "certs_incomplete"
+        , "Secured boot is enabled on this VM and the certificates defines in \


Typo: defined.

lindig · 2024-05-07T09:06:18Z

ocaml/idl/datamodel_vm.ml

+        ("not_supported", "VM's firmware is not UEFI")
+      ; ("disabled", "Secureboot is disabled on this VM")
+      ; ( "first_boot"
+        , "Secured boot is enabled on this VM and its NVRAM.EFI-variables is \


lindig · 2024-05-07T09:07:49Z

ocaml/xapi-cli-server/cli_frontend.ml

+    , {
+        reqd= ["uuid"; "mode"]
+      ; optn= []
+      ; help= "Set a VM in UEFI setup or user mode."


Maybe indicate that either "setup" or "user" is expected by putting them in (single) quotes.

lindig · 2024-05-07T09:09:46Z

ocaml/xapi-cli-server/record_util.ml

+  | "user" ->
+      `user
+  | s ->
+      raise (Record_failure ("Expected 'user','setup', got " ^ s))


"Expected one of 'user', 'record' but got '%s'"

I'm replicating how other Record_failure are written, I think it's better to keep the message standarize.

lindig · 2024-05-07T09:12:02Z

ocaml/xapi/xapi_pool.ml

+
+let get_auth_filename tmp_dir tarpath =
+  let filename =
+    if String.contains tarpath '/' then


Filename.basename can be called unconditionally.

lindig · 2024-05-07T09:13:24Z

ocaml/xapi/xapi_pool.ml

+    else
+      tarpath
+  in
+  let tmp_auth_file = Filename.concat tmp_dir filename in


This is risky because there could be clashes over this path if tmp_dir is some public temp dir.

https://ocaml.org/manual/5.1/api/Filename.html#VALtemp_file should be used here

lindig · 2024-05-07T09:14:30Z

ocaml/xapi/xapi_pool.ml

+        (fun () -> Sys.remove filename) ;
+      let tmp_auth_files = Sys.readdir tmp_dir in
+      Array.iter (fun file -> debug "*** BRS: auth file %s" file) tmp_auth_files ;
+      let pk_present = Array.mem "PK.auth" tmp_auth_files in


what happens when Array.mem fails?

I don't see how it can fail in the doc.

psafont · 2024-05-08T14:49:40Z

ocaml/xapi/message_forwarding.ml

@@ -3037,6 +3037,12 @@ functor
      let restart_device_models ~__context ~self =
        info "VM.restart_device_models: self = '%s'" (vm_uuid ~__context self) ;
        Local.VM.restart_device_models ~__context ~self
+
+      let set_uefi_mode ~__context ~self ~mode =


The call is not being forwarded to the host that hosts the vm, Is the command meant to be run on the coordinator ever time?

This method eventually calls varstore-sb-state which can be called on any host of the pool as the call will be redirected where the VM is located. I did not want to add another redirection before that.

the call will be redirected where the VM is located

Which layer is doing this redirection? In xapi this is exactly the place that must do the redirection, if at all.

IIUC varstored-tools call XAPI from within the scripts when running to dispatch to the relevent.

psafont · 2024-05-08T15:11:50Z

ocaml/idl/datamodel_vm.ml

+           defined in its EFI variables"
+        )
+      ; ( "setup_mode"
+        , "Secured boot is enabled on this VM and PK is not defined in its EFI \


In which use cases does this state interact with set_uefi_mode call? And how? The latter doesn't affect the database value directly, while this is db-only

set_uefi_mode calls varstore-sb-state which will affect the VM's NVRAM's EFI-variables.
This method only deduct the state of the VM EFI variables.

psafont · 2024-05-08T15:18:57Z

ocaml/xapi/xapi_vm.ml

+            `first_boot (* TO BE VERIFIED *)
+        | Some _ -> (
+            let varstore_ls =
+              Helpers.call_script !Xapi_globs.varstore_ls


What host does this need to be run on? Always on the coordinator?

Same as above, varstore-ls will redirect the call when relevant.

psafont · 2024-05-08T15:23:54Z

ocaml/xapi/xapi_vm.ml

+          List.assoc_opt "EFI-variables" (Db.VM.get_NVRAM ~__context ~self)
+        with
+        | None ->
+            `first_boot (* TO BE VERIFIED *)


Is there a plan to verify this?

It's been tested and verified, I'll retest it after the latest changes and remove the comment after revalidation.

ocaml/xapi/xapi_vm.ml

psafont · 2024-05-08T15:34:57Z

ocaml/xapi/xapi_pool.ml

+    else
+      tarpath
+  in
+  let tmp_auth_file = Filename.concat tmp_dir filename in


https://ocaml.org/manual/5.1/api/Filename.html#VALtemp_file should be used here

psafont

Aside from the comments that need to be addressed, you should rebase the PR on top of the latest master, it will make the strange python errors go away

Calls `varstore-sb-state` to edit the uefi mode of a VM Takes in input the uuid of a VM and a mode (`setup` or `user`) Returns the output of the script calls Signed-off-by: Benjamin Reis <benjamin.reis@vates.tech>

Returns the SecureBoot status of a VM: - `not_supported`: VM's firmware is not UEFI - `disabled`: Secureboot is disabled on this VM - `first_boot`: Secured boot is enabled on this VM and its NVRAM.EFI-variables is empty - `ready`: Secured boot is enabled on this VM and PK, KEK, db and dbx are defined in its EFI variables - `ready_no_dbx`: Secured boot is enabled on this VM and PK, KEK, db but not dbx are defined in its EFI variables - `setup_mode`: Secured boot is enabled on this VM and PK is not defined in its EFI variables - `certs_incomplete`: Secured boot is enabled on this VM and the certificates defines in its EFI variables are incomplete Signed-off-by: Benjamin Reis <benjamin.reis@vates.tech>

Returns a pool's state for guest SecureBoot: - `ready`: the active pool UEFI certificates (custom ones first, default ones if no custom ones) contain PK, KEK, db and dbx - `ready_no_dbx`: the active pool UEFI certificates contain PK, KEK and db but not dbx - `not_ready`: otherwise Signed-off-by: Benjamin Reis <benjamin.reis@vates.tech>

Signed-off-by: Benjamin Reis <benjamin.reis@vates.tech>

See: xapi-project/xen-api#5566 Signed-off-by: Benjamin Reis <benjamin.reis@vates.tech>

psafont · 2024-05-16T14:19:00Z

ocaml/xapi/xapi_vm.ml

+                (fun elem ->
+                  (* Lines follow this pattern: <GUID> <KEY>*)
+                  let splitted = String.split_on_char ' ' elem in
+                  List.nth splitted 1


Nth can raise with a pretty nasty error that's difficult to find where it originated. Please use nth_opt and deal with the option by replacing List.map with List.filter_map

benjamreis force-pushed the sb-state-api branch 2 times, most recently from a0e8fa2 to 2efb70f Compare April 17, 2024 11:28

lindig reviewed Apr 18, 2024

View reviewed changes

ocaml/idl/datamodel_vm.ml Show resolved Hide resolved

benjamreis force-pushed the sb-state-api branch from 2efb70f to 62b8a61 Compare April 18, 2024 11:44

benjamreis force-pushed the sb-state-api branch 6 times, most recently from afc5343 to 41cd758 Compare April 19, 2024 09:11

benjamreis commented Apr 22, 2024

View reviewed changes

benjamreis force-pushed the sb-state-api branch 3 times, most recently from f9180a8 to 1f84bd0 Compare April 22, 2024 08:46

benjamreis marked this pull request as ready for review April 22, 2024 09:13

benjamreis requested a review from lindig April 22, 2024 13:24

lindig reviewed May 7, 2024

View reviewed changes

psafont reviewed May 8, 2024

View reviewed changes

psafont requested changes May 8, 2024

View reviewed changes

benjamreis added 3 commits May 13, 2024 09:25

Add VM.set_uefi_mode API call

65fad75

Calls `varstore-sb-state` to edit the uefi mode of a VM Takes in input the uuid of a VM and a mode (`setup` or `user`) Returns the output of the script calls Signed-off-by: Benjamin Reis <benjamin.reis@vates.tech>

benjamreis force-pushed the sb-state-api branch from 1f84bd0 to 483271f Compare May 13, 2024 07:25

benjamreis added 2 commits May 13, 2024 09:43

Rebase me: fix typo in vm.get_secureboot_readiness

edd738a

Signed-off-by: Benjamin Reis <benjamin.reis@vates.tech>

Rebase me: calrify vm-set-uefi-mode help

43b96a0

Signed-off-by: Benjamin Reis <benjamin.reis@vates.tech>

benjamreis force-pushed the sb-state-api branch from 8e1853d to 9751634 Compare May 14, 2024 06:36

Rebase me: simpler method for Pool.get_guest_secureboot_readiness

978b260

Signed-off-by: Benjamin Reis <benjamin.reis@vates.tech>

benjamreis force-pushed the sb-state-api branch 2 times, most recently from 5a0e802 to 9746d6d Compare May 14, 2024 07:11

lindig approved these changes May 14, 2024

View reviewed changes

benjamreis force-pushed the sb-state-api branch from 9746d6d to ccc3796 Compare May 14, 2024 11:56

Rebase me: use is_infix directly

1c111b0

Signed-off-by: Benjamin Reis <benjamin.reis@vates.tech>

benjamreis force-pushed the sb-state-api branch from c1fc346 to 1c111b0 Compare May 14, 2024 12:06

benjamreis added a commit to xcp-ng/xcp-ng-tests that referenced this pull request May 15, 2024

Test Secureboot state API

586b6b6

See: xapi-project/xen-api#5566 Signed-off-by: Benjamin Reis <benjamin.reis@vates.tech>

benjamreis mentioned this pull request May 15, 2024

Test Secureboot state API xcp-ng/xcp-ng-tests#227

Open

benjamreis added a commit to xcp-ng-rpms/xapi that referenced this pull request May 15, 2024

New API methods for status about SecureBoot and UEFI certs

c5720ef

See: xapi-project/xen-api#5566 Signed-off-by: Benjamin Reis <benjamin.reis@vates.tech>

benjamreis added a commit to xcp-ng-rpms/xapi that referenced this pull request May 15, 2024

New API methods for status about SecureBoot and UEFI certs

63e7151

See: xapi-project/xen-api#5566 Signed-off-by: Benjamin Reis <benjamin.reis@vates.tech>

benjamreis added a commit to xcp-ng-rpms/xapi that referenced this pull request May 15, 2024

New API methods for status about SecureBoot and UEFI certs

0efce96

See: xapi-project/xen-api#5566 Signed-off-by: Benjamin Reis <benjamin.reis@vates.tech>

benjamreis mentioned this pull request May 15, 2024

New API methods for status about SecureBoot and UEFI certs xcp-ng-rpms/xapi#64

Open

benjamreis added a commit to xcp-ng/xcp-ng-tests that referenced this pull request May 15, 2024

Test Secureboot state API

3d594e4

See: xapi-project/xen-api#5566 Signed-off-by: Benjamin Reis <benjamin.reis@vates.tech>

benjamreis requested a review from psafont May 16, 2024 07:02

psafont reviewed May 16, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New API methods for status about SecureBoot and UEFI certs #5566

New API methods for status about SecureBoot and UEFI certs #5566

benjamreis commented Apr 17, 2024 •

edited

lindig left a comment

lindig commented Apr 18, 2024

benjamreis commented Apr 18, 2024 •

edited

benjamreis Apr 22, 2024

psafont May 8, 2024

benjamreis commented May 7, 2024

lindig May 7, 2024

lindig May 7, 2024

lindig May 7, 2024

lindig May 7, 2024

benjamreis May 13, 2024

lindig May 7, 2024

lindig May 7, 2024

psafont May 8, 2024

lindig May 7, 2024

benjamreis May 13, 2024

psafont May 8, 2024

benjamreis May 13, 2024

psafont May 14, 2024

benjamreis May 15, 2024

psafont May 8, 2024

benjamreis May 13, 2024

psafont May 8, 2024

benjamreis May 13, 2024

psafont May 8, 2024

benjamreis May 13, 2024

psafont May 8, 2024

psafont left a comment

psafont May 16, 2024

New API methods for status about SecureBoot and UEFI certs #5566

Are you sure you want to change the base?

New API methods for status about SecureBoot and UEFI certs #5566

Conversation

benjamreis commented Apr 17, 2024 • edited

lindig left a comment

Choose a reason for hiding this comment

lindig commented Apr 18, 2024

benjamreis commented Apr 18, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benjamreis commented May 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

psafont left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benjamreis commented Apr 17, 2024 •

edited

benjamreis commented Apr 18, 2024 •

edited