[PATCH v2] qga: add security info to guest-get-osinfo

Elizabeth Ashurov posted 1 patch 2 days, 10 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260330151941.2207789-1-eashurov@redhat.com
Maintainers: Kostiantyn Kostiuk <kkostiuk@redhat.com>, Michael Roth <michael.roth@amd.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
qga/commands-win32.c | 421 +++++++++++++++++++++++++++++++++++++++++++
qga/qapi-schema.json |  91 +++++++++-
2 files changed, 511 insertions(+), 1 deletion(-)
[PATCH v2] qga: add security info to guest-get-osinfo
Posted by Elizabeth Ashurov 2 days, 10 hours ago
Extend guest-get-osinfo to include security features status
(VBS, Secure Boot, TPM) in a nested 'security' field.
OS-specific data (e.g. Windows DeviceGuard) is separated
using a union to allow future per-OS extensions.

The implementation queries Win32_DeviceGuard and Win32_Tpm via
WMI, and reads the SecureBoot UEFI variable through
GetFirmwareEnvironmentVariable().

Signed-off-by: Elizabeth Ashurov <eashurov@redhat.com>
---
 qga/commands-win32.c | 421 +++++++++++++++++++++++++++++++++++++++++++
 qga/qapi-schema.json |  91 +++++++++-
 2 files changed, 511 insertions(+), 1 deletion(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index c0bf3467bd..39ebdcf2cd 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -28,6 +28,7 @@
 #include <wtsapi32.h>
 #include <wininet.h>
 #include <pdh.h>
+#include <wbemidl.h>
 
 #include "guest-agent-core.h"
 #include "vss-win32.h"
@@ -2252,6 +2253,8 @@ static char *ga_get_current_arch(void)
     return result;
 }
 
+static void populate_security_info(GuestOSInfo *osinfo);
+
 GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
 {
     Error *local_err = NULL;
@@ -2289,6 +2292,8 @@ GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
     info->variant = g_strdup(server ? "server" : "client");
     info->variant_id = g_strdup(server ? "server" : "client");
 
+    populate_security_info(info);
+
     return info;
 }
 
@@ -2764,3 +2769,419 @@ GuestNetworkRouteList *qmp_guest_network_get_route(Error **errp)
     g_hash_table_destroy(interface_metric_cache);
     return head;
 }
+
+/*
+ * WMI GUIDs
+ */
+static const GUID qga_CLSID_WbemLocator = {
+    0x4590f811, 0x1d3a, 0x11d0,
+    {0x89, 0x1f, 0x00, 0xaa, 0x00, 0x4b, 0x2e, 0x24}
+};
+static const GUID qga_IID_IWbemLocator = {
+    0xdc12a687, 0x737f, 0x11cf,
+    {0x88, 0x4d, 0x00, 0xaa, 0x00, 0x4b, 0x2e, 0x24}
+};
+
+static IWbemServices *wmi_connect_to_namespace(const wchar_t *namespace_path,
+                                               Error **errp)
+{
+    HRESULT hr;
+    IWbemLocator *locator = NULL;
+    IWbemServices *services = NULL;
+    BSTR bstr_ns = SysAllocString(namespace_path);
+
+    if (!bstr_ns) {
+        error_setg(errp, "failed to allocate WMI namespace string");
+        return NULL;
+    }
+
+    hr = CoCreateInstance(&qga_CLSID_WbemLocator, NULL, CLSCTX_INPROC_SERVER,
+                          &qga_IID_IWbemLocator, (LPVOID *)&locator);
+    if (FAILED(hr)) {
+        error_setg_win32(errp, hr, "failed to create IWbemLocator");
+        goto out;
+    }
+
+    hr = locator->lpVtbl->ConnectServer(locator, bstr_ns, NULL, NULL, NULL,
+                                        0, NULL, NULL, &services);
+    if (FAILED(hr)) {
+        error_setg_win32(errp, hr, "failed to connect to WMI namespace");
+        goto out;
+    }
+
+    hr = CoSetProxyBlanket((IUnknown *)services,
+                           RPC_C_AUTHN_WINNT, RPC_C_AUTHZ_NONE, NULL,
+                           RPC_C_AUTHN_LEVEL_CALL,
+                           RPC_C_IMP_LEVEL_IMPERSONATE,
+                           NULL, EOAC_NONE);
+    if (FAILED(hr)) {
+        error_setg_win32(errp, hr, "failed to set WMI proxy blanket");
+        services->lpVtbl->Release(services);
+        services = NULL;
+    }
+
+out:
+    SysFreeString(bstr_ns);
+    if (locator) {
+        locator->lpVtbl->Release(locator);
+    }
+    return services;
+}
+
+static IEnumWbemClassObject *wmi_exec_query(IWbemServices *services,
+                                            const wchar_t *query,
+                                            Error **errp)
+{
+    HRESULT hr;
+    IEnumWbemClassObject *enumerator = NULL;
+    BSTR bstr_wql = SysAllocString(L"WQL");
+    BSTR bstr_query = SysAllocString(query);
+
+    if (!bstr_wql || !bstr_query) {
+        error_setg(errp, "failed to allocate WMI query strings");
+        goto out;
+    }
+
+    hr = services->lpVtbl->ExecQuery(services, bstr_wql, bstr_query,
+                                     WBEM_FLAG_RETURN_IMMEDIATELY |
+                                     WBEM_FLAG_FORWARD_ONLY,
+                                     NULL, &enumerator);
+    if (FAILED(hr)) {
+        error_setg_win32(errp, hr, "WMI query failed");
+    }
+
+out:
+    SysFreeString(bstr_wql);
+    SysFreeString(bstr_query);
+    return enumerator;
+}
+
+static HRESULT wmi_get_property(IWbemClassObject *obj, const wchar_t *name,
+                                VARIANT *var)
+{
+    return obj->lpVtbl->Get(obj, name, 0, var, NULL, NULL);
+}
+
+/* Read a WMI integer property (VT_I4 or VT_UI4). */
+static bool wmi_get_int_property(IWbemClassObject *obj,
+                                 const wchar_t *name,
+                                 int64_t *out)
+{
+    VARIANT var;
+    bool ret = false;
+
+    VariantInit(&var);
+    if (SUCCEEDED(wmi_get_property(obj, name, &var))) {
+        if (V_VT(&var) == VT_I4) {
+            *out = V_I4(&var);
+            ret = true;
+        } else if (V_VT(&var) == VT_UI4) {
+            *out = V_UI4(&var);
+            ret = true;
+        }
+    }
+    VariantClear(&var);
+    return ret;
+}
+
+/* Read an integer SAFEARRAY WMI property into a QAPI intList. */
+static bool wmi_safearray_to_int_list(IWbemClassObject *obj,
+                                      const wchar_t *prop_name,
+                                      intList **list)
+{
+    VARIANT var;
+    HRESULT hr;
+    LONG lb, ub, i;
+    uint32_t *data = NULL;
+
+    VariantInit(&var);
+    hr = wmi_get_property(obj, prop_name, &var);
+    if (FAILED(hr) || V_VT(&var) == VT_NULL) {
+        VariantClear(&var);
+        return false;
+    }
+
+    if (!(V_VT(&var) & VT_ARRAY)) {
+        VariantClear(&var);
+        return false;
+    }
+
+    SAFEARRAY *sa = V_ARRAY(&var);
+    if (FAILED(SafeArrayGetLBound(sa, 1, &lb)) ||
+        FAILED(SafeArrayGetUBound(sa, 1, &ub))) {
+        VariantClear(&var);
+        return false;
+    }
+
+    if (FAILED(SafeArrayAccessData(sa, (void **)&data))) {
+        VariantClear(&var);
+        return false;
+    }
+
+    intList **tail = list;
+    for (i = 0; i <= ub - lb; i++) {
+        QAPI_LIST_APPEND(tail, (int64_t)data[i]);
+    }
+
+    SafeArrayUnaccessData(sa);
+    VariantClear(&var);
+    return true;
+}
+
+/*
+ * Query Win32_DeviceGuard WMI class for VBS and related properties.
+ */
+static void get_device_guard_info(GuestSecurityInfoWindows *info,
+                                  Error **errp)
+{
+    Error *local_err = NULL;
+    IWbemServices *services = NULL;
+    IEnumWbemClassObject *enumerator = NULL;
+    IWbemClassObject *obj = NULL;
+    ULONG count = 0;
+    HRESULT hr;
+    int64_t val;
+
+    services = wmi_connect_to_namespace(
+        L"ROOT\\Microsoft\\Windows\\DeviceGuard", &local_err);
+    if (!services) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    enumerator = wmi_exec_query(services,
+        L"SELECT * FROM Win32_DeviceGuard", &local_err);
+    if (!enumerator) {
+        error_propagate(errp, local_err);
+        goto out;
+    }
+
+    hr = enumerator->lpVtbl->Next(enumerator, WBEM_INFINITE, 1,
+                                   &obj, &count);
+    if (FAILED(hr)) {
+        error_setg_win32(errp, hr, "failed to enumerate Win32_DeviceGuard");
+        goto out;
+    }
+    if (count == 0) {
+        error_setg(errp, "no Win32_DeviceGuard instance found");
+        goto out;
+    }
+
+    if (wmi_get_int_property(obj, L"VirtualizationBasedSecurityStatus",
+                             &val)) {
+        info->has_vbs_status = true;
+        info->vbs_status = val;
+    }
+
+    if (wmi_get_int_property(obj, L"CodeIntegrityPolicyEnforcementStatus",
+                             &val)) {
+        info->has_code_integrity_policy_enforcement_status = true;
+        info->code_integrity_policy_enforcement_status = val;
+    }
+
+    if (wmi_get_int_property(obj,
+                             L"UsermodeCodeIntegrityPolicyEnforcementStatus",
+                             &val)) {
+        info->has_usr_cfg_code_integrity_policy_enforcement_status = true;
+        info->usr_cfg_code_integrity_policy_enforcement_status = val;
+    }
+
+    if (wmi_safearray_to_int_list(obj, L"AvailableSecurityProperties",
+                                  &info->available_security_properties)) {
+        info->has_available_security_properties = true;
+    }
+
+    if (wmi_safearray_to_int_list(obj, L"RequiredSecurityProperties",
+                                  &info->required_security_properties)) {
+        info->has_required_security_properties = true;
+    }
+
+    if (wmi_safearray_to_int_list(obj, L"SecurityServicesConfigured",
+                                  &info->security_services_configured)) {
+        info->has_security_services_configured = true;
+    }
+
+    if (wmi_safearray_to_int_list(obj, L"SecurityServicesRunning",
+                                  &info->security_services_running)) {
+        info->has_security_services_running = true;
+    }
+
+    obj->lpVtbl->Release(obj);
+    obj = NULL;
+
+    /* Drain remaining results */
+    while (true) {
+        hr = enumerator->lpVtbl->Next(enumerator, WBEM_INFINITE, 1,
+                                      &obj, &count);
+        if (FAILED(hr) || count == 0) {
+            break;
+        }
+        obj->lpVtbl->Release(obj);
+        obj = NULL;
+    }
+
+out:
+    if (obj) {
+        obj->lpVtbl->Release(obj);
+    }
+    if (enumerator) {
+        enumerator->lpVtbl->Release(enumerator);
+    }
+    if (services) {
+        services->lpVtbl->Release(services);
+    }
+}
+
+/*
+ * Read the SecureBoot UEFI variable.  On legacy BIOS systems the field
+ * is omitted (not applicable).
+ */
+static void get_secure_boot_status(GuestSecurityInfo *info,
+                                   Error **errp)
+{
+    Error *local_err = NULL;
+    BYTE value = 0;
+    DWORD ret;
+
+    acquire_privilege(SE_SYSTEM_ENVIRONMENT_NAME, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    ret = GetFirmwareEnvironmentVariableA("SecureBoot",
+        "{8be4df61-93ca-11d2-aa0d-00e098032b8c}", &value, sizeof(value));
+
+    if (ret == 0) {
+        DWORD err = GetLastError();
+        if (err == ERROR_INVALID_FUNCTION) {
+            return;
+        }
+        if (err == ERROR_ENVVAR_NOT_FOUND) {
+            info->has_secure_boot = true;
+            info->secure_boot = false;
+            return;
+        }
+        error_setg_win32(errp, err,
+                         "failed to read SecureBoot UEFI variable");
+        return;
+    }
+
+    info->has_secure_boot = true;
+    info->secure_boot = (value == 1);
+}
+
+/*
+ * Query Win32_Tpm WMI class for TPM presence and version.
+ */
+static void get_tpm_info(GuestSecurityInfo *info, Error **errp)
+{
+    Error *local_err = NULL;
+    IWbemServices *services = NULL;
+    IEnumWbemClassObject *enumerator = NULL;
+    IWbemClassObject *obj = NULL;
+    ULONG count = 0;
+    HRESULT hr;
+    VARIANT var;
+
+    services = wmi_connect_to_namespace(
+        L"ROOT\\CIMV2\\Security\\MicrosoftTpm", &local_err);
+    if (!services) {
+        /* TPM namespace may not exist -- field omitted (unknown) */
+        error_free(local_err);
+        return;
+    }
+
+    enumerator = wmi_exec_query(services,
+        L"SELECT * FROM Win32_Tpm", &local_err);
+    if (!enumerator) {
+        error_free(local_err);
+        goto out;
+    }
+
+    hr = enumerator->lpVtbl->Next(enumerator, WBEM_INFINITE, 1,
+                                   &obj, &count);
+    if (FAILED(hr) || count == 0) {
+        info->has_tpm_present = true;
+        info->tpm_present = false;
+        goto out;
+    }
+
+    info->has_tpm_present = true;
+    info->tpm_present = true;
+
+    VariantInit(&var);
+    if (SUCCEEDED(wmi_get_property(obj, L"SpecVersion", &var)) &&
+        V_VT(&var) == VT_BSTR && V_BSTR(&var)) {
+        info->tpm_version = g_utf16_to_utf8(
+            (const gunichar2 *)V_BSTR(&var), -1, NULL, NULL, NULL);
+        if (info->tpm_version) {
+            /* keep only the part before the first comma */
+            char *comma = strchr(info->tpm_version, ',');
+            if (comma) {
+                *comma = '\0';
+            }
+        }
+    }
+    VariantClear(&var);
+
+    obj->lpVtbl->Release(obj);
+    obj = NULL;
+
+    /* Drain remaining results */
+    while (true) {
+        hr = enumerator->lpVtbl->Next(enumerator, WBEM_INFINITE, 1,
+                                      &obj, &count);
+        if (FAILED(hr) || count == 0) {
+            break;
+        }
+        obj->lpVtbl->Release(obj);
+        obj = NULL;
+    }
+
+out:
+    if (obj) {
+        obj->lpVtbl->Release(obj);
+    }
+    if (enumerator) {
+        enumerator->lpVtbl->Release(enumerator);
+    }
+    if (services) {
+        services->lpVtbl->Release(services);
+    }
+}
+
+static void populate_security_info(GuestOSInfo *osinfo)
+{
+    Error *local_err = NULL;
+    GuestSecurityInfo *info = g_new0(GuestSecurityInfo, 1);
+
+    info->os = g_new0(GuestSecurityInfoOs, 1);
+    info->os->type = GUEST_SECURITY_INFO_TYPE_WINDOWS;
+
+    get_device_guard_info(&info->os->u.windows, &local_err);
+    if (local_err) {
+        g_warning("DeviceGuard query failed: %s",
+                  error_get_pretty(local_err));
+        error_free(local_err);
+        local_err = NULL;
+    }
+
+    get_secure_boot_status(info, &local_err);
+    if (local_err) {
+        g_warning("SecureBoot query failed: %s",
+                  error_get_pretty(local_err));
+        error_free(local_err);
+        local_err = NULL;
+    }
+
+    get_tpm_info(info, &local_err);
+    if (local_err) {
+        g_warning("TPM query failed: %s",
+                  error_get_pretty(local_err));
+        error_free(local_err);
+        local_err = NULL;
+    }
+
+    osinfo->security = info;
+}
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index c57bc9a02f..2247f77cff 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1490,6 +1490,8 @@
 #     * POSIX: as defined by os-release(5)
 #     * Windows: contains string "server" or "client"
 #
+# @security: Security features status (since 10.3)
+#
 # .. note:: On POSIX systems the fields @id, @name, @pretty-name,
 #    @version, @version-id, @variant and @variant-id follow the
 #    definition specified in os-release(5).  Refer to the manual page
@@ -1508,7 +1510,8 @@
       '*kernel-release': 'str', '*kernel-version': 'str',
       '*machine': 'str', '*id': 'str', '*name': 'str',
       '*pretty-name': 'str', '*version': 'str', '*version-id': 'str',
-      '*variant': 'str', '*variant-id': 'str' } }
+      '*variant': 'str', '*variant-id': 'str',
+      '*security': 'GuestSecurityInfo' } }
 
 ##
 # @guest-get-osinfo:
@@ -1952,3 +1955,89 @@
   'returns': ['GuestNetworkRoute'],
   'if': { 'any': ['CONFIG_LINUX', 'CONFIG_WIN32'] }
 }
+
+##
+# @GuestSecurityInfoWindows:
+#
+# Windows-specific security features from Win32_DeviceGuard.
+#
+# @vbs-status: VirtualizationBasedSecurityStatus
+#
+# @available-security-properties:
+#     AvailableSecurityProperties
+#
+# @code-integrity-policy-enforcement-status:
+#     CodeIntegrityPolicyEnforcementStatus
+#
+# @required-security-properties: RequiredSecurityProperties
+#
+# @security-services-configured:
+#     SecurityServicesConfigured
+#
+# @security-services-running: SecurityServicesRunning
+#
+# @usr-cfg-code-integrity-policy-enforcement-status:
+#     UsermodeCodeIntegrityPolicyEnforcementStatus
+#
+# Since: 10.3
+##
+{ 'struct': 'GuestSecurityInfoWindows',
+  'data': {
+      '*vbs-status': 'int',
+      '*available-security-properties': ['int'],
+      '*code-integrity-policy-enforcement-status': 'int',
+      '*required-security-properties': ['int'],
+      '*security-services-configured': ['int'],
+      '*security-services-running': ['int'],
+      '*usr-cfg-code-integrity-policy-enforcement-status':
+          'int' } }
+
+##
+# @GuestSecurityInfoType:
+#
+# Guest operating system type for security info.
+#
+# @windows: Microsoft Windows
+#
+# Since: 10.3
+##
+{ 'enum': 'GuestSecurityInfoType',
+  'data': ['windows'] }
+
+##
+# @GuestSecurityInfoOs:
+#
+# OS-specific security information.
+#
+# @type: guest operating system type
+#
+# Since: 10.3
+##
+{ 'union': 'GuestSecurityInfoOs',
+  'base': { 'type': 'GuestSecurityInfoType' },
+  'discriminator': 'type',
+  'data': {
+      'windows': 'GuestSecurityInfoWindows' } }
+
+##
+# @GuestSecurityInfo:
+#
+# Guest security features status.  Fields are optional; a missing
+# field means the information is not available on this platform.
+#
+# @tpm-present: Whether a TPM device is present
+#
+# @tpm-version: TPM specification version (e.g. "2.0")
+#
+# @secure-boot: Whether UEFI Secure Boot is enabled
+#
+# @os: OS-specific security information
+#
+# Since: 10.3
+##
+{ 'struct': 'GuestSecurityInfo',
+  'data': {
+      '*tpm-present': 'bool',
+      '*tpm-version': 'str',
+      '*secure-boot': 'bool',
+      '*os': 'GuestSecurityInfoOs' } }
-- 
2.51.0
Re: [PATCH v2] qga: add security info to guest-get-osinfo
Posted by Daniel P. Berrangé 1 day, 13 hours ago
On Mon, Mar 30, 2026 at 06:19:41PM +0300, Elizabeth Ashurov wrote:
> Extend guest-get-osinfo to include security features status
> (VBS, Secure Boot, TPM) in a nested 'security' field.
> OS-specific data (e.g. Windows DeviceGuard) is separated
> using a union to allow future per-OS extensions.
> 
> The implementation queries Win32_DeviceGuard and Win32_Tpm via
> WMI, and reads the SecureBoot UEFI variable through
> GetFirmwareEnvironmentVariable().
> 
> Signed-off-by: Elizabeth Ashurov <eashurov@redhat.com>
> ---
>  qga/commands-win32.c | 421 +++++++++++++++++++++++++++++++++++++++++++

This should also provide a Linux impl for the TPM and secureboot info as
that is platform portable and easily available.

>  qga/qapi-schema.json |  91 +++++++++-
>  2 files changed, 511 insertions(+), 1 deletion(-)

> +
> +/*
> + * Read the SecureBoot UEFI variable.  On legacy BIOS systems the field
> + * is omitted (not applicable).
> + */
> +static void get_secure_boot_status(GuestSecurityInfo *info,
> +                                   Error **errp)
> +{
> +    Error *local_err = NULL;
> +    BYTE value = 0;
> +    DWORD ret;
> +
> +    acquire_privilege(SE_SYSTEM_ENVIRONMENT_NAME, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +
> +    ret = GetFirmwareEnvironmentVariableA("SecureBoot",
> +        "{8be4df61-93ca-11d2-aa0d-00e098032b8c}", &value, sizeof(value));
> +
> +    if (ret == 0) {
> +        DWORD err = GetLastError();
> +        if (err == ERROR_INVALID_FUNCTION) {
> +            return;
> +        }
> +        if (err == ERROR_ENVVAR_NOT_FOUND) {
> +            info->has_secure_boot = true;
> +            info->secure_boot = false;
> +            return;
> +        }
> +        error_setg_win32(errp, err,
> +                         "failed to read SecureBoot UEFI variable");
> +        return;
> +    }
> +
> +    info->has_secure_boot = true;
> +    info->secure_boot = (value == 1);

Is that comparison of 'value == 1' definitely correct ?  My
understanding was that secure boot state is indicated by the 5th byte
in that EFI variable.



> +}
> +
> +/*
> + * Query Win32_Tpm WMI class for TPM presence and version.
> + */
> +static void get_tpm_info(GuestSecurityInfo *info, Error **errp)
> +{
> +    Error *local_err = NULL;
> +    IWbemServices *services = NULL;
> +    IEnumWbemClassObject *enumerator = NULL;
> +    IWbemClassObject *obj = NULL;
> +    ULONG count = 0;
> +    HRESULT hr;
> +    VARIANT var;
> +
> +    services = wmi_connect_to_namespace(
> +        L"ROOT\\CIMV2\\Security\\MicrosoftTpm", &local_err);
> +    if (!services) {
> +        /* TPM namespace may not exist -- field omitted (unknown) */
> +        error_free(local_err);
> +        return;
> +    }
> +
> +    enumerator = wmi_exec_query(services,
> +        L"SELECT * FROM Win32_Tpm", &local_err);
> +    if (!enumerator) {
> +        error_free(local_err);
> +        goto out;
> +    }
> +
> +    hr = enumerator->lpVtbl->Next(enumerator, WBEM_INFINITE, 1,
> +                                   &obj, &count);
> +    if (FAILED(hr) || count == 0) {
> +        info->has_tpm_present = true;
> +        info->tpm_present = false;
> +        goto out;
> +    }
> +
> +    info->has_tpm_present = true;
> +    info->tpm_present = true;
> +
> +    VariantInit(&var);
> +    if (SUCCEEDED(wmi_get_property(obj, L"SpecVersion", &var)) &&
> +        V_VT(&var) == VT_BSTR && V_BSTR(&var)) {
> +        info->tpm_version = g_utf16_to_utf8(
> +            (const gunichar2 *)V_BSTR(&var), -1, NULL, NULL, NULL);
> +        if (info->tpm_version) {
> +            /* keep only the part before the first comma */
> +            char *comma = strchr(info->tpm_version, ',');

It would be helpful if the comment included a full example string
from "SpecVersion", so we can see what we're parsing here.

> +            if (comma) {
> +                *comma = '\0';
> +            }
> +        }
> +    }
> +    VariantClear(&var);


> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index c57bc9a02f..2247f77cff 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -1490,6 +1490,8 @@
>  #     * POSIX: as defined by os-release(5)
>  #     * Windows: contains string "server" or "client"
>  #
> +# @security: Security features status (since 10.3)
> +#
>  # .. note:: On POSIX systems the fields @id, @name, @pretty-name,
>  #    @version, @version-id, @variant and @variant-id follow the
>  #    definition specified in os-release(5).  Refer to the manual page
> @@ -1508,7 +1510,8 @@
>        '*kernel-release': 'str', '*kernel-version': 'str',
>        '*machine': 'str', '*id': 'str', '*name': 'str',
>        '*pretty-name': 'str', '*version': 'str', '*version-id': 'str',
> -      '*variant': 'str', '*variant-id': 'str' } }
> +      '*variant': 'str', '*variant-id': 'str',
> +      '*security': 'GuestSecurityInfo' } }
>  
>  ##
>  # @guest-get-osinfo:
> @@ -1952,3 +1955,89 @@
>    'returns': ['GuestNetworkRoute'],
>    'if': { 'any': ['CONFIG_LINUX', 'CONFIG_WIN32'] }
>  }
> +
> +##
> +# @GuestSecurityInfoWindows:
> +#
> +# Windows-specific security features from Win32_DeviceGuard.
> +#
> +# @vbs-status: VirtualizationBasedSecurityStatus
> +#
> +# @available-security-properties:
> +#     AvailableSecurityProperties
> +#
> +# @code-integrity-policy-enforcement-status:
> +#     CodeIntegrityPolicyEnforcementStatus
> +#
> +# @required-security-properties: RequiredSecurityProperties
> +#
> +# @security-services-configured:
> +#     SecurityServicesConfigured
> +#
> +# @security-services-running: SecurityServicesRunning
> +#
> +# @usr-cfg-code-integrity-policy-enforcement-status:
> +#     UsermodeCodeIntegrityPolicyEnforcementStatus

None of these docs are actually useful. Can you explain in plain
language what they each mean.

> +#
> +# Since: 10.3
> +##
> +{ 'struct': 'GuestSecurityInfoWindows',
> +  'data': {
> +      '*vbs-status': 'int',
> +      '*available-security-properties': ['int'],
> +      '*code-integrity-policy-enforcement-status': 'int',
> +      '*required-security-properties': ['int'],
> +      '*security-services-configured': ['int'],
> +      '*security-services-running': ['int'],
> +      '*usr-cfg-code-integrity-policy-enforcement-status':
> +          'int' } }

I'm guessing there that the are all largely reflecting enumerated
values from the Windows API. I'm wondering how they will be
consumed by whatever calls this command. Would they be better off
as enums, instead of plain ints, or arrays of ints ?

> +
> +##
> +# @GuestSecurityInfoType:
> +#
> +# Guest operating system type for security info.
> +#
> +# @windows: Microsoft Windows
> +#
> +# Since: 10.3
> +##
> +{ 'enum': 'GuestSecurityInfoType',
> +  'data': ['windows'] }
> +
> +##
> +# @GuestSecurityInfoOs:
> +#
> +# OS-specific security information.
> +#
> +# @type: guest operating system type
> +#
> +# Since: 10.3
> +##
> +{ 'union': 'GuestSecurityInfoOs',
> +  'base': { 'type': 'GuestSecurityInfoType' },
> +  'discriminator': 'type',
> +  'data': {
> +      'windows': 'GuestSecurityInfoWindows' } }
> +
> +##
> +# @GuestSecurityInfo:
> +#
> +# Guest security features status.  Fields are optional; a missing
> +# field means the information is not available on this platform.
> +#
> +# @tpm-present: Whether a TPM device is present
> +#
> +# @tpm-version: TPM specification version (e.g. "2.0")
> +#
> +# @secure-boot: Whether UEFI Secure Boot is enabled
> +#
> +# @os: OS-specific security information
> +#
> +# Since: 10.3
> +##
> +{ 'struct': 'GuestSecurityInfo',
> +  'data': {
> +      '*tpm-present': 'bool',
> +      '*tpm-version': 'str',

Should we have a GuestSecurityTPMInfo struct ?

    '*tpm': 'GuestSecurityTPMInfo'

We would not need a "present" value, as the mere existence of the struct
data would indicate presence. Meanwhile this struct would contain a
mandatory version field:

   { 'struct': 'GuestSecurityTPMInfo',
     'data': {
         'major-version': 'int'
     }
   }

I suggest only reporting major version, as an int, not an arbitrary
string as that's ambiguous IMHO.  We could later add other TPM info
to this struct if needed.

For Linux you can provide this TPM info by checking whether
/sys/class/tpm/tpm0 exists, and reading /sys/class/tpm/tpm0/tpm_version_major

> +      '*secure-boot': 'bool',

For Linux you can provide this secureboot info by reading
/sys/firmware/efi/efivars/SecureBoot-8be4df61-93ca-11d2-aa0d-00e098032b8c

This bool only tells us whether secureboot is enabled or not. I wonder
if we should allow for other info here too. Systemd's bootctl reads
various other secureboot fields "AuditMode", "DeployedMode", 'SetupMode'
and 'MokSBStateRT'.

https://github.com/systemd/systemd/blob/main/src/basic/efivars.c#L381

We could represent each as bools within a struct, eg

  '*secure-boot': 'GuestSecuritySecureBootInfo'

which would have several mandatory fields

   { 'struct': 'GuestSecuritySecureBootInfo',
     'data': {
         'enabled': 'bool',
	 'audit-mode': 'bool',
	 'deployed-mode': 'bool',
	 'setup-mode': 'bool',
	 'mok-sb-state': 'bool',
     }
   }

These are again all available from EFI variables in sysfs, or the
equivalent API you have already used on Windows.

Any missing variable can be assumed "false", or we would make all
the variables except 'enabled' be optional.

> +      '*os': 'GuestSecurityInfoOs' } }

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|
Re: [PATCH v2] qga: add security info to guest-get-osinfo
Posted by Kostiantyn Kostiuk 1 day, 12 hours ago
Best Regards,
Kostiantyn Kostiuk.


On Tue, Mar 31, 2026 at 3:08 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Mon, Mar 30, 2026 at 06:19:41PM +0300, Elizabeth Ashurov wrote:
> > Extend guest-get-osinfo to include security features status
> > (VBS, Secure Boot, TPM) in a nested 'security' field.
> > OS-specific data (e.g. Windows DeviceGuard) is separated
> > using a union to allow future per-OS extensions.
> >
> > The implementation queries Win32_DeviceGuard and Win32_Tpm via
> > WMI, and reads the SecureBoot UEFI variable through
> > GetFirmwareEnvironmentVariable().
> >
> > Signed-off-by: Elizabeth Ashurov <eashurov@redhat.com>
> > ---
> >  qga/commands-win32.c | 421 +++++++++++++++++++++++++++++++++++++++++++
>
> This should also provide a Linux impl for the TPM and secureboot info as
> that is platform portable and easily available.
>
> >  qga/qapi-schema.json |  91 +++++++++-
> >  2 files changed, 511 insertions(+), 1 deletion(-)
>
> > +
> > +/*
> > + * Read the SecureBoot UEFI variable.  On legacy BIOS systems the field
> > + * is omitted (not applicable).
> > + */
> > +static void get_secure_boot_status(GuestSecurityInfo *info,
> > +                                   Error **errp)
> > +{
> > +    Error *local_err = NULL;
> > +    BYTE value = 0;
> > +    DWORD ret;
> > +
> > +    acquire_privilege(SE_SYSTEM_ENVIRONMENT_NAME, &local_err);
> > +    if (local_err) {
> > +        error_propagate(errp, local_err);
> > +        return;
> > +    }
> > +
> > +    ret = GetFirmwareEnvironmentVariableA("SecureBoot",
> > +        "{8be4df61-93ca-11d2-aa0d-00e098032b8c}", &value,
> sizeof(value));
> > +
> > +    if (ret == 0) {
> > +        DWORD err = GetLastError();
> > +        if (err == ERROR_INVALID_FUNCTION) {
> > +            return;
> > +        }
> > +        if (err == ERROR_ENVVAR_NOT_FOUND) {
> > +            info->has_secure_boot = true;
> > +            info->secure_boot = false;
> > +            return;
> > +        }
> > +        error_setg_win32(errp, err,
> > +                         "failed to read SecureBoot UEFI variable");
> > +        return;
> > +    }
> > +
> > +    info->has_secure_boot = true;
> > +    info->secure_boot = (value == 1);
>
> Is that comparison of 'value == 1' definitely correct ?  My
> understanding was that secure boot state is indicated by the 5th byte
> in that EFI variable.
>

In MSFT sample
https://github.com/microsoft/Windows-universal-samples/blob/main/Samples/CustomCapability/Service/Client/uefi.cpp
the reverse logic is used.
But from our testing on Windows, this comparison is correct.


>
>
>
> > +}
> > +
> > +/*
> > + * Query Win32_Tpm WMI class for TPM presence and version.
> > + */
> > +static void get_tpm_info(GuestSecurityInfo *info, Error **errp)
> > +{
> > +    Error *local_err = NULL;
> > +    IWbemServices *services = NULL;
> > +    IEnumWbemClassObject *enumerator = NULL;
> > +    IWbemClassObject *obj = NULL;
> > +    ULONG count = 0;
> > +    HRESULT hr;
> > +    VARIANT var;
> > +
> > +    services = wmi_connect_to_namespace(
> > +        L"ROOT\\CIMV2\\Security\\MicrosoftTpm", &local_err);
> > +    if (!services) {
> > +        /* TPM namespace may not exist -- field omitted (unknown) */
> > +        error_free(local_err);
> > +        return;
> > +    }
> > +
> > +    enumerator = wmi_exec_query(services,
> > +        L"SELECT * FROM Win32_Tpm", &local_err);
> > +    if (!enumerator) {
> > +        error_free(local_err);
> > +        goto out;
> > +    }
> > +
> > +    hr = enumerator->lpVtbl->Next(enumerator, WBEM_INFINITE, 1,
> > +                                   &obj, &count);
> > +    if (FAILED(hr) || count == 0) {
> > +        info->has_tpm_present = true;
> > +        info->tpm_present = false;
> > +        goto out;
> > +    }
> > +
> > +    info->has_tpm_present = true;
> > +    info->tpm_present = true;
> > +
> > +    VariantInit(&var);
> > +    if (SUCCEEDED(wmi_get_property(obj, L"SpecVersion", &var)) &&
> > +        V_VT(&var) == VT_BSTR && V_BSTR(&var)) {
> > +        info->tpm_version = g_utf16_to_utf8(
> > +            (const gunichar2 *)V_BSTR(&var), -1, NULL, NULL, NULL);
> > +        if (info->tpm_version) {
> > +            /* keep only the part before the first comma */
> > +            char *comma = strchr(info->tpm_version, ',');
>
> It would be helpful if the comment included a full example string
> from "SpecVersion", so we can see what we're parsing here.
>
> > +            if (comma) {
> > +                *comma = '\0';
> > +            }
> > +        }
> > +    }
> > +    VariantClear(&var);
>
>
> > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> > index c57bc9a02f..2247f77cff 100644
> > --- a/qga/qapi-schema.json
> > +++ b/qga/qapi-schema.json
> > @@ -1490,6 +1490,8 @@
> >  #     * POSIX: as defined by os-release(5)
> >  #     * Windows: contains string "server" or "client"
> >  #
> > +# @security: Security features status (since 10.3)
> > +#
> >  # .. note:: On POSIX systems the fields @id, @name, @pretty-name,
> >  #    @version, @version-id, @variant and @variant-id follow the
> >  #    definition specified in os-release(5).  Refer to the manual page
> > @@ -1508,7 +1510,8 @@
> >        '*kernel-release': 'str', '*kernel-version': 'str',
> >        '*machine': 'str', '*id': 'str', '*name': 'str',
> >        '*pretty-name': 'str', '*version': 'str', '*version-id': 'str',
> > -      '*variant': 'str', '*variant-id': 'str' } }
> > +      '*variant': 'str', '*variant-id': 'str',
> > +      '*security': 'GuestSecurityInfo' } }
> >
> >  ##
> >  # @guest-get-osinfo:
> > @@ -1952,3 +1955,89 @@
> >    'returns': ['GuestNetworkRoute'],
> >    'if': { 'any': ['CONFIG_LINUX', 'CONFIG_WIN32'] }
> >  }
> > +
> > +##
> > +# @GuestSecurityInfoWindows:
> > +#
> > +# Windows-specific security features from Win32_DeviceGuard.
> > +#
> > +# @vbs-status: VirtualizationBasedSecurityStatus
> > +#
> > +# @available-security-properties:
> > +#     AvailableSecurityProperties
> > +#
> > +# @code-integrity-policy-enforcement-status:
> > +#     CodeIntegrityPolicyEnforcementStatus
> > +#
> > +# @required-security-properties: RequiredSecurityProperties
> > +#
> > +# @security-services-configured:
> > +#     SecurityServicesConfigured
> > +#
> > +# @security-services-running: SecurityServicesRunning
> > +#
> > +# @usr-cfg-code-integrity-policy-enforcement-status:
> > +#     UsermodeCodeIntegrityPolicyEnforcementStatus
>
> None of these docs are actually useful. Can you explain in plain
> language what they each mean.
>
> > +#
> > +# Since: 10.3
> > +##
> > +{ 'struct': 'GuestSecurityInfoWindows',
> > +  'data': {
> > +      '*vbs-status': 'int',
> > +      '*available-security-properties': ['int'],
> > +      '*code-integrity-policy-enforcement-status': 'int',
> > +      '*required-security-properties': ['int'],
> > +      '*security-services-configured': ['int'],
> > +      '*security-services-running': ['int'],
> > +      '*usr-cfg-code-integrity-policy-enforcement-status':
> > +          'int' } }
>
> I'm guessing there that the are all largely reflecting enumerated
> values from the Windows API. I'm wondering how they will be
> consumed by whatever calls this command. Would they be better off
> as enums, instead of plain ints, or arrays of ints ?
>

There is no any existing enum in the Win32 API, as I know.
If we introduce it, we should care about any updates from the MSFT side.
Explanation of these numbers
https://learn.microsoft.com/en-us/windows/security/hardware-security/enable-virtualization-based-protection-of-code-integrity?tabs=security

looks complicated, and I am not sure whether it is better to return
"HardwareEnforcedStackProtectionInAuditMode" is better than 6.

Currently, QGA returns data in the same format as MSFT provides it.
I think the management system should make a decision based on this.


>
> > +
> > +##
> > +# @GuestSecurityInfoType:
> > +#
> > +# Guest operating system type for security info.
> > +#
> > +# @windows: Microsoft Windows
> > +#
> > +# Since: 10.3
> > +##
> > +{ 'enum': 'GuestSecurityInfoType',
> > +  'data': ['windows'] }
> > +
> > +##
> > +# @GuestSecurityInfoOs:
> > +#
> > +# OS-specific security information.
> > +#
> > +# @type: guest operating system type
> > +#
> > +# Since: 10.3
> > +##
> > +{ 'union': 'GuestSecurityInfoOs',
> > +  'base': { 'type': 'GuestSecurityInfoType' },
> > +  'discriminator': 'type',
> > +  'data': {
> > +      'windows': 'GuestSecurityInfoWindows' } }
> > +
> > +##
> > +# @GuestSecurityInfo:
> > +#
> > +# Guest security features status.  Fields are optional; a missing
> > +# field means the information is not available on this platform.
> > +#
> > +# @tpm-present: Whether a TPM device is present
> > +#
> > +# @tpm-version: TPM specification version (e.g. "2.0")
> > +#
> > +# @secure-boot: Whether UEFI Secure Boot is enabled
> > +#
> > +# @os: OS-specific security information
> > +#
> > +# Since: 10.3
> > +##
> > +{ 'struct': 'GuestSecurityInfo',
> > +  'data': {
> > +      '*tpm-present': 'bool',
> > +      '*tpm-version': 'str',
>
> Should we have a GuestSecurityTPMInfo struct ?
>
>     '*tpm': 'GuestSecurityTPMInfo'
>
> We would not need a "present" value, as the mere existence of the struct
> data would indicate presence. Meanwhile this struct would contain a
> mandatory version field:
>
>    { 'struct': 'GuestSecurityTPMInfo',
>      'data': {
>          'major-version': 'int'
>      }
>    }
>
> I suggest only reporting major version, as an int, not an arbitrary
> string as that's ambiguous IMHO.  We could later add other TPM info
> to this struct if needed.
>
> For Linux you can provide this TPM info by checking whether
> /sys/class/tpm/tpm0 exists, and reading
> /sys/class/tpm/tpm0/tpm_version_major
>
> > +      '*secure-boot': 'bool',
>
> For Linux you can provide this secureboot info by reading
> /sys/firmware/efi/efivars/SecureBoot-8be4df61-93ca-11d2-aa0d-00e098032b8c
>
> This bool only tells us whether secureboot is enabled or not. I wonder
> if we should allow for other info here too. Systemd's bootctl reads
> various other secureboot fields "AuditMode", "DeployedMode", 'SetupMode'
> and 'MokSBStateRT'.
>
> https://github.com/systemd/systemd/blob/main/src/basic/efivars.c#L381
>
> We could represent each as bools within a struct, eg
>
>   '*secure-boot': 'GuestSecuritySecureBootInfo'
>
> which would have several mandatory fields
>
>    { 'struct': 'GuestSecuritySecureBootInfo',
>      'data': {
>          'enabled': 'bool',
>          'audit-mode': 'bool',
>          'deployed-mode': 'bool',
>          'setup-mode': 'bool',
>          'mok-sb-state': 'bool',
>

Any real use cases for all other fields?
Or just get it because all of them are related to SB?


>      }
>    }
>
> These are again all available from EFI variables in sysfs, or the
> equivalent API you have already used on Windows.
>
> Any missing variable can be assumed "false", or we would make all
> the variables except 'enabled' be optional.
>
> > +      '*os': 'GuestSecurityInfoOs' } }
>
> With regards,
> Daniel
> --
> |: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
> |: https://libvirt.org          ~~          https://entangle-photo.org :|
> |: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|
>
>
Re: [PATCH v2] qga: add security info to guest-get-osinfo
Posted by Daniel P. Berrangé 1 day, 12 hours ago
On Tue, Mar 31, 2026 at 03:55:01PM +0300, Kostiantyn Kostiuk wrote:
> Best Regards,
> Kostiantyn Kostiuk.
> 
> 
> On Tue, Mar 31, 2026 at 3:08 PM Daniel P. Berrangé <berrange@redhat.com>
> wrote:
> 
> > On Mon, Mar 30, 2026 at 06:19:41PM +0300, Elizabeth Ashurov wrote:
> > > Extend guest-get-osinfo to include security features status
> > > (VBS, Secure Boot, TPM) in a nested 'security' field.
> > > OS-specific data (e.g. Windows DeviceGuard) is separated
> > > using a union to allow future per-OS extensions.
> > >
> > > The implementation queries Win32_DeviceGuard and Win32_Tpm via
> > > WMI, and reads the SecureBoot UEFI variable through
> > > GetFirmwareEnvironmentVariable().
> > >
> > > Signed-off-by: Elizabeth Ashurov <eashurov@redhat.com>
> > > ---
> > >  qga/commands-win32.c | 421 +++++++++++++++++++++++++++++++++++++++++++
> >
> > This should also provide a Linux impl for the TPM and secureboot info as
> > that is platform portable and easily available.
> >
> > >  qga/qapi-schema.json |  91 +++++++++-
> > >  2 files changed, 511 insertions(+), 1 deletion(-)
> >
> > > +
> > > +/*
> > > + * Read the SecureBoot UEFI variable.  On legacy BIOS systems the field
> > > + * is omitted (not applicable).
> > > + */
> > > +static void get_secure_boot_status(GuestSecurityInfo *info,
> > > +                                   Error **errp)
> > > +{
> > > +    Error *local_err = NULL;
> > > +    BYTE value = 0;
> > > +    DWORD ret;
> > > +
> > > +    acquire_privilege(SE_SYSTEM_ENVIRONMENT_NAME, &local_err);
> > > +    if (local_err) {
> > > +        error_propagate(errp, local_err);
> > > +        return;
> > > +    }
> > > +
> > > +    ret = GetFirmwareEnvironmentVariableA("SecureBoot",
> > > +        "{8be4df61-93ca-11d2-aa0d-00e098032b8c}", &value,
> > sizeof(value));
> > > +
> > > +    if (ret == 0) {
> > > +        DWORD err = GetLastError();
> > > +        if (err == ERROR_INVALID_FUNCTION) {
> > > +            return;
> > > +        }
> > > +        if (err == ERROR_ENVVAR_NOT_FOUND) {
> > > +            info->has_secure_boot = true;
> > > +            info->secure_boot = false;
> > > +            return;
> > > +        }
> > > +        error_setg_win32(errp, err,
> > > +                         "failed to read SecureBoot UEFI variable");
> > > +        return;
> > > +    }
> > > +
> > > +    info->has_secure_boot = true;
> > > +    info->secure_boot = (value == 1);
> >
> > Is that comparison of 'value == 1' definitely correct ?  My
> > understanding was that secure boot state is indicated by the 5th byte
> > in that EFI variable.
> >
> 
> In MSFT sample
> https://github.com/microsoft/Windows-universal-samples/blob/main/Samples/CustomCapability/Service/Client/uefi.cpp
> the reverse logic is used.
> But from our testing on Windows, this comparison is correct.
> 
> 
> >
> >
> >
> > > +}
> > > +
> > > +/*
> > > + * Query Win32_Tpm WMI class for TPM presence and version.
> > > + */
> > > +static void get_tpm_info(GuestSecurityInfo *info, Error **errp)
> > > +{
> > > +    Error *local_err = NULL;
> > > +    IWbemServices *services = NULL;
> > > +    IEnumWbemClassObject *enumerator = NULL;
> > > +    IWbemClassObject *obj = NULL;
> > > +    ULONG count = 0;
> > > +    HRESULT hr;
> > > +    VARIANT var;
> > > +
> > > +    services = wmi_connect_to_namespace(
> > > +        L"ROOT\\CIMV2\\Security\\MicrosoftTpm", &local_err);
> > > +    if (!services) {
> > > +        /* TPM namespace may not exist -- field omitted (unknown) */
> > > +        error_free(local_err);
> > > +        return;
> > > +    }
> > > +
> > > +    enumerator = wmi_exec_query(services,
> > > +        L"SELECT * FROM Win32_Tpm", &local_err);
> > > +    if (!enumerator) {
> > > +        error_free(local_err);
> > > +        goto out;
> > > +    }
> > > +
> > > +    hr = enumerator->lpVtbl->Next(enumerator, WBEM_INFINITE, 1,
> > > +                                   &obj, &count);
> > > +    if (FAILED(hr) || count == 0) {
> > > +        info->has_tpm_present = true;
> > > +        info->tpm_present = false;
> > > +        goto out;
> > > +    }
> > > +
> > > +    info->has_tpm_present = true;
> > > +    info->tpm_present = true;
> > > +
> > > +    VariantInit(&var);
> > > +    if (SUCCEEDED(wmi_get_property(obj, L"SpecVersion", &var)) &&
> > > +        V_VT(&var) == VT_BSTR && V_BSTR(&var)) {
> > > +        info->tpm_version = g_utf16_to_utf8(
> > > +            (const gunichar2 *)V_BSTR(&var), -1, NULL, NULL, NULL);
> > > +        if (info->tpm_version) {
> > > +            /* keep only the part before the first comma */
> > > +            char *comma = strchr(info->tpm_version, ',');
> >
> > It would be helpful if the comment included a full example string
> > from "SpecVersion", so we can see what we're parsing here.
> >
> > > +            if (comma) {
> > > +                *comma = '\0';
> > > +            }
> > > +        }
> > > +    }
> > > +    VariantClear(&var);
> >
> >
> > > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> > > index c57bc9a02f..2247f77cff 100644
> > > --- a/qga/qapi-schema.json
> > > +++ b/qga/qapi-schema.json
> > > @@ -1490,6 +1490,8 @@
> > >  #     * POSIX: as defined by os-release(5)
> > >  #     * Windows: contains string "server" or "client"
> > >  #
> > > +# @security: Security features status (since 10.3)
> > > +#
> > >  # .. note:: On POSIX systems the fields @id, @name, @pretty-name,
> > >  #    @version, @version-id, @variant and @variant-id follow the
> > >  #    definition specified in os-release(5).  Refer to the manual page
> > > @@ -1508,7 +1510,8 @@
> > >        '*kernel-release': 'str', '*kernel-version': 'str',
> > >        '*machine': 'str', '*id': 'str', '*name': 'str',
> > >        '*pretty-name': 'str', '*version': 'str', '*version-id': 'str',
> > > -      '*variant': 'str', '*variant-id': 'str' } }
> > > +      '*variant': 'str', '*variant-id': 'str',
> > > +      '*security': 'GuestSecurityInfo' } }
> > >
> > >  ##
> > >  # @guest-get-osinfo:
> > > @@ -1952,3 +1955,89 @@
> > >    'returns': ['GuestNetworkRoute'],
> > >    'if': { 'any': ['CONFIG_LINUX', 'CONFIG_WIN32'] }
> > >  }
> > > +
> > > +##
> > > +# @GuestSecurityInfoWindows:
> > > +#
> > > +# Windows-specific security features from Win32_DeviceGuard.
> > > +#
> > > +# @vbs-status: VirtualizationBasedSecurityStatus
> > > +#
> > > +# @available-security-properties:
> > > +#     AvailableSecurityProperties
> > > +#
> > > +# @code-integrity-policy-enforcement-status:
> > > +#     CodeIntegrityPolicyEnforcementStatus
> > > +#
> > > +# @required-security-properties: RequiredSecurityProperties
> > > +#
> > > +# @security-services-configured:
> > > +#     SecurityServicesConfigured
> > > +#
> > > +# @security-services-running: SecurityServicesRunning
> > > +#
> > > +# @usr-cfg-code-integrity-policy-enforcement-status:
> > > +#     UsermodeCodeIntegrityPolicyEnforcementStatus
> >
> > None of these docs are actually useful. Can you explain in plain
> > language what they each mean.
> >
> > > +#
> > > +# Since: 10.3
> > > +##
> > > +{ 'struct': 'GuestSecurityInfoWindows',
> > > +  'data': {
> > > +      '*vbs-status': 'int',
> > > +      '*available-security-properties': ['int'],
> > > +      '*code-integrity-policy-enforcement-status': 'int',
> > > +      '*required-security-properties': ['int'],
> > > +      '*security-services-configured': ['int'],
> > > +      '*security-services-running': ['int'],
> > > +      '*usr-cfg-code-integrity-policy-enforcement-status':
> > > +          'int' } }
> >
> > I'm guessing there that the are all largely reflecting enumerated
> > values from the Windows API. I'm wondering how they will be
> > consumed by whatever calls this command. Would they be better off
> > as enums, instead of plain ints, or arrays of ints ?
> >
> 
> There is no any existing enum in the Win32 API, as I know.
> If we introduce it, we should care about any updates from the MSFT side.
> Explanation of these numbers
> https://learn.microsoft.com/en-us/windows/security/hardware-security/enable-virtualization-based-protection-of-code-integrity?tabs=security
> 
> looks complicated, and I am not sure whether it is better to return
> "HardwareEnforcedStackProtectionInAuditMode" is better than 6.
> 
> Currently, QGA returns data in the same format as MSFT provides it.
> I think the management system should make a decision based on this.
> 
> 
> >
> > > +
> > > +##
> > > +# @GuestSecurityInfoType:
> > > +#
> > > +# Guest operating system type for security info.
> > > +#
> > > +# @windows: Microsoft Windows
> > > +#
> > > +# Since: 10.3
> > > +##
> > > +{ 'enum': 'GuestSecurityInfoType',
> > > +  'data': ['windows'] }
> > > +
> > > +##
> > > +# @GuestSecurityInfoOs:
> > > +#
> > > +# OS-specific security information.
> > > +#
> > > +# @type: guest operating system type
> > > +#
> > > +# Since: 10.3
> > > +##
> > > +{ 'union': 'GuestSecurityInfoOs',
> > > +  'base': { 'type': 'GuestSecurityInfoType' },
> > > +  'discriminator': 'type',
> > > +  'data': {
> > > +      'windows': 'GuestSecurityInfoWindows' } }
> > > +
> > > +##
> > > +# @GuestSecurityInfo:
> > > +#
> > > +# Guest security features status.  Fields are optional; a missing
> > > +# field means the information is not available on this platform.
> > > +#
> > > +# @tpm-present: Whether a TPM device is present
> > > +#
> > > +# @tpm-version: TPM specification version (e.g. "2.0")
> > > +#
> > > +# @secure-boot: Whether UEFI Secure Boot is enabled
> > > +#
> > > +# @os: OS-specific security information
> > > +#
> > > +# Since: 10.3
> > > +##
> > > +{ 'struct': 'GuestSecurityInfo',
> > > +  'data': {
> > > +      '*tpm-present': 'bool',
> > > +      '*tpm-version': 'str',
> >
> > Should we have a GuestSecurityTPMInfo struct ?
> >
> >     '*tpm': 'GuestSecurityTPMInfo'
> >
> > We would not need a "present" value, as the mere existence of the struct
> > data would indicate presence. Meanwhile this struct would contain a
> > mandatory version field:
> >
> >    { 'struct': 'GuestSecurityTPMInfo',
> >      'data': {
> >          'major-version': 'int'
> >      }
> >    }
> >
> > I suggest only reporting major version, as an int, not an arbitrary
> > string as that's ambiguous IMHO.  We could later add other TPM info
> > to this struct if needed.
> >
> > For Linux you can provide this TPM info by checking whether
> > /sys/class/tpm/tpm0 exists, and reading
> > /sys/class/tpm/tpm0/tpm_version_major
> >
> > > +      '*secure-boot': 'bool',
> >
> > For Linux you can provide this secureboot info by reading
> > /sys/firmware/efi/efivars/SecureBoot-8be4df61-93ca-11d2-aa0d-00e098032b8c
> >
> > This bool only tells us whether secureboot is enabled or not. I wonder
> > if we should allow for other info here too. Systemd's bootctl reads
> > various other secureboot fields "AuditMode", "DeployedMode", 'SetupMode'
> > and 'MokSBStateRT'.
> >
> > https://github.com/systemd/systemd/blob/main/src/basic/efivars.c#L381
> >
> > We could represent each as bools within a struct, eg
> >
> >   '*secure-boot': 'GuestSecuritySecureBootInfo'
> >
> > which would have several mandatory fields
> >
> >    { 'struct': 'GuestSecuritySecureBootInfo',
> >      'data': {
> >          'enabled': 'bool',
> >          'audit-mode': 'bool',
> >          'deployed-mode': 'bool',
> >          'setup-mode': 'bool',
> >          'mok-sb-state': 'bool',
> >
> 
> Any real use cases for all other fields?
> Or just get it because all of them are related to SB?

The security characteristics of a system with secure boot are more
granular than just a boolean on/off. These other variables give you
the full insight into the security of the system.

Further details are in the spec

  http://www.uefi.org/sites/default/files/resources/UEFI%20Spec%202_7_A%20Sept%206.pdf

Specifically the section "31.3 Firmware/OS Key Exchange: creating
trust relationships" is a large diagram showing the different modes
that are expressed by these variables. The exception mok-sb-state
which is something exposed by Shim, not a core UEFI variable, but
still important to overall system security.

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|


Re: [PATCH v2] qga: add security info to guest-get-osinfo
Posted by Markus Armbruster via qemu development 1 day, 19 hours ago
Hi Elizabeth!  You neglected to cc: me.  Recommend to use
scripts/get_maintainer.pl to find all the possibly involved maintainers,
then use common sense to trim.

Elizabeth Ashurov <eashurov@redhat.com> writes:

> Extend guest-get-osinfo to include security features status
> (VBS, Secure Boot, TPM) in a nested 'security' field.
> OS-specific data (e.g. Windows DeviceGuard) is separated
> using a union to allow future per-OS extensions.
>
> The implementation queries Win32_DeviceGuard and Win32_Tpm via
> WMI, and reads the SecureBoot UEFI variable through
> GetFirmwareEnvironmentVariable().
>
> Signed-off-by: Elizabeth Ashurov <eashurov@redhat.com>

[...]

> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index c57bc9a02f..2247f77cff 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -1490,6 +1490,8 @@
>  #     * POSIX: as defined by os-release(5)
>  #     * Windows: contains string "server" or "client"
>  #
> +# @security: Security features status (since 10.3)
> +#

When is this member present?

Since 11.1.  More of the same below, not flagging it again.

>  # .. note:: On POSIX systems the fields @id, @name, @pretty-name,
>  #    @version, @version-id, @variant and @variant-id follow the
>  #    definition specified in os-release(5).  Refer to the manual page
> @@ -1508,7 +1510,8 @@
>        '*kernel-release': 'str', '*kernel-version': 'str',
>        '*machine': 'str', '*id': 'str', '*name': 'str',
>        '*pretty-name': 'str', '*version': 'str', '*version-id': 'str',
> -      '*variant': 'str', '*variant-id': 'str' } }
> +      '*variant': 'str', '*variant-id': 'str',
> +      '*security': 'GuestSecurityInfo' } }
>  
>  ##
>  # @guest-get-osinfo:
> @@ -1952,3 +1955,89 @@
>    'returns': ['GuestNetworkRoute'],
>    'if': { 'any': ['CONFIG_LINUX', 'CONFIG_WIN32'] }
>  }
> +
> +##
> +# @GuestSecurityInfoWindows:
> +#
> +# Windows-specific security features from Win32_DeviceGuard.
> +#
> +# @vbs-status: VirtualizationBasedSecurityStatus

PleaseWriteNemberDescriptionsInEnglish: words, spaces, punctuation, the
works.

> +#
> +# @available-security-properties:
> +#     AvailableSecurityProperties
> +#
> +# @code-integrity-policy-enforcement-status:
> +#     CodeIntegrityPolicyEnforcementStatus
> +#
> +# @required-security-properties: RequiredSecurityProperties
> +#
> +# @security-services-configured:
> +#     SecurityServicesConfigured
> +#
> +# @security-services-running: SecurityServicesRunning
> +#
> +# @usr-cfg-code-integrity-policy-enforcement-status:
> +#     UsermodeCodeIntegrityPolicyEnforcementStatus
> +#
> +# Since: 10.3
> +##
> +{ 'struct': 'GuestSecurityInfoWindows',
> +  'data': {
> +      '*vbs-status': 'int',
> +      '*available-security-properties': ['int'],
> +      '*code-integrity-policy-enforcement-status': 'int',
> +      '*required-security-properties': ['int'],
> +      '*security-services-configured': ['int'],
> +      '*security-services-running': ['int'],
> +      '*usr-cfg-code-integrity-policy-enforcement-status':
> +          'int' } }

Please don't break this line.

> +
> +##
> +# @GuestSecurityInfoType:
> +#
> +# Guest operating system type for security info.
> +#
> +# @windows: Microsoft Windows
> +#
> +# Since: 10.3
> +##
> +{ 'enum': 'GuestSecurityInfoType',
> +  'data': ['windows'] }
> +
> +##
> +# @GuestSecurityInfoOs:
> +#
> +# OS-specific security information.
> +#
> +# @type: guest operating system type
> +#
> +# Since: 10.3
> +##
> +{ 'union': 'GuestSecurityInfoOs',
> +  'base': { 'type': 'GuestSecurityInfoType' },
> +  'discriminator': 'type',
> +  'data': {
> +      'windows': 'GuestSecurityInfoWindows' } }
> +
> +##
> +# @GuestSecurityInfo:
> +#
> +# Guest security features status.  Fields are optional; a missing
> +# field means the information is not available on this platform.

What do you mean by "platform"?  Host, guest, both?

> +#
> +# @tpm-present: Whether a TPM device is present
> +#
> +# @tpm-version: TPM specification version (e.g. "2.0")
> +#
> +# @secure-boot: Whether UEFI Secure Boot is enabled
> +#
> +# @os: OS-specific security information
> +#
> +# Since: 10.3
> +##
> +{ 'struct': 'GuestSecurityInfo',
> +  'data': {
> +      '*tpm-present': 'bool',
> +      '*tpm-version': 'str',
> +      '*secure-boot': 'bool',
> +      '*os': 'GuestSecurityInfoOs' } }