[PATCH libguestfs 0/2] lib: Return correct osinfo field for Windows 11
https://bugzilla.redhat.com/show_bug.cgi?id=2012658 We don't currently detect the correct version for Windows 11 (client, server is OK). After some discussion it turns out the accepted way to do this is to look at the Windows build ID. So add some code in the daemon to grab the build ID from the Windows registry and expose it through a new API ("guestfs_inspect_get_build_id") and adjust the osinfo code on the library side accordingly. Because, at least in theory, Linux distros could have build IDs, I didn't call the API "guestfs_inspect_windows_...". There is a possible build ID field in /etc/os-release, although it seems to be unloved. This fixes the <osinfo> field. I intentionally did not adjust either the Windows version (still 10.0) nor the product name (which is completely bogus, but apparently how Microsoft intended it to be). Rich.
Add an API to return the build ID of the guest. This to allow a future change to be able to distinguish between Windows 10 and Windows 11 which can only be done using the build ID. For Windows we can read the CurrentBuildNumber key from the registry. For Linux there happens to be a BUILD_ID field in /etc/os-release. I've never seen a Linux distro that actually uses this. --- daemon/inspect.ml | 6 ++++++ daemon/inspect_fs_unix.ml | 2 ++ daemon/inspect_fs_windows.ml | 14 ++++++++++++++ daemon/inspect_types.ml | 5 +++++ daemon/inspect_types.mli | 1 + generator/actions_inspection.ml | 19 +++++++++++++++++++ generator/proc_nr.ml | 3 ++- lib/MAX_PROC_NR | 2 +- 8 files changed, 50 insertions(+), 2 deletions(-) diff --git a/daemon/inspect.ml b/daemon/inspect.ml index fb75b4a6cb..20217c0257 100644 --- a/daemon/inspect.ml +++ b/daemon/inspect.ml @@ -335,6 +335,12 @@ and inspect_get_hostname root = | Some v -> v | None -> "unknown" +and inspect_get_build_id root = + let root = search_for_root root in + match root.inspection_data.build_id with + | Some v -> v + | None -> "unknown" + and inspect_get_windows_systemroot root = let root = search_for_root root in match root.inspection_data.windows_systemroot with diff --git a/daemon/inspect_fs_unix.ml b/daemon/inspect_fs_unix.ml index 8045ef0d43..811e23b18c 100644 --- a/daemon/inspect_fs_unix.ml +++ b/daemon/inspect_fs_unix.ml @@ -96,6 +96,8 @@ let rec parse_os_release release_file data = data.product_name <- Some value else if key = "VERSION_ID" then parse_os_release_version_id value data + else if key = "BUILD_ID" then + data.build_id <- Some value ) values; (* If we haven't got all the fields, exit right away. *) diff --git a/daemon/inspect_fs_windows.ml b/daemon/inspect_fs_windows.ml index c4a05bc383..7bc5de7f77 100644 --- a/daemon/inspect_fs_windows.ml +++ b/daemon/inspect_fs_windows.ml @@ -263,6 +263,20 @@ and check_windows_software_registry software_hive data = with Not_found -> () ); + + (* CurrentBuildNumber (build_id). + * + * In modern Windows, the "CurrentBuild" and "CurrentBuildNumber" + * keys are the same. But in Windows XP, "CurrentBuild" + * contained something quite different. So always use + * "CurrentBuildNumber". + *) + (try + let v = List.assoc "CurrentBuildNumber" values in + data.build_id <- Some (Hivex.value_string h v) + with + Not_found -> () + ); with | Not_found -> if verbose () then diff --git a/daemon/inspect_types.ml b/daemon/inspect_types.ml index 9395c51f95..328a2146be 100644 --- a/daemon/inspect_types.ml +++ b/daemon/inspect_types.ml @@ -48,6 +48,7 @@ and inspection_data = { mutable version : version option; mutable arch : string option; mutable hostname : string option; + mutable build_id : string option; mutable fstab : fstab_entry list; mutable windows_systemroot : string option; mutable windows_software_hive : string option; @@ -167,6 +168,8 @@ and string_of_inspection_data data = data.arch; Option.may (fun v -> bpf " hostname: %s\n" v) data.hostname; + Option.may (fun v -> bpf " build ID: %s\n" v) + data.build_id; if data.fstab <> [] then ( let v = List.map ( fun (a, b) -> sprintf "(%s, %s)" (Mountable.to_string a) b @@ -272,6 +275,7 @@ let null_inspection_data = { version = None; arch = None; hostname = None; + build_id = None; fstab = []; windows_systemroot = None; windows_software_hive = None; @@ -294,6 +298,7 @@ let merge_inspection_data child parent = parent.version <- merge child.version parent.version; parent.arch <- merge child.arch parent.arch; parent.hostname <- merge child.hostname parent.hostname; + parent.build_id <- merge child.build_id parent.build_id; parent.fstab <- child.fstab @ parent.fstab; parent.windows_systemroot <- merge child.windows_systemroot parent.windows_systemroot; diff --git a/daemon/inspect_types.mli b/daemon/inspect_types.mli index 29c76e8abb..05a3ffd4e4 100644 --- a/daemon/inspect_types.mli +++ b/daemon/inspect_types.mli @@ -51,6 +51,7 @@ and inspection_data = { mutable version : version option; mutable arch : string option; mutable hostname : string option; + mutable build_id : string option; mutable fstab : fstab_entry list; mutable windows_systemroot : string option; mutable windows_software_hive : string option; diff --git a/generator/actions_inspection.ml b/generator/actions_inspection.ml index f8b7449938..70de22ec0a 100644 --- a/generator/actions_inspection.ml +++ b/generator/actions_inspection.ml @@ -529,6 +529,25 @@ hive is a valid Windows Registry hive. You can use C<guestfs_hivex_open> to read or write to the hive. +Please read L<guestfs(3)/INSPECTION> for more details." }; + + { defaults with + name = "inspect_get_build_id"; added = (1, 49, 8); + style = RString (RPlainString, "buildid"), [String (Mountable, "root")], []; + impl = OCaml "Inspect.inspect_get_build_id"; + shortdesc = "get the system build ID"; + longdesc = "\ +This returns the build ID of the system, or the string +C<\"unknown\"> if the system does not have a build ID. + +For Windows, this gets the build number. Although it is +returned as a string, it is (so far) always a number. See +L<https://en.wikipedia.org/wiki/List_of_Microsoft_Windows_versions> +for some possible values. + +For Linux, this returns the C<BUILD_ID> string from +F</etc/os-release>, although this is not often used. + Please read L<guestfs(3)/INSPECTION> for more details." }; { defaults with diff --git a/generator/proc_nr.ml b/generator/proc_nr.ml index edd9bd99d2..0f17b1c06f 100644 --- a/generator/proc_nr.ml +++ b/generator/proc_nr.ml @@ -514,7 +514,8 @@ let proc_nr = [ 509, "cryptsetup_close"; 510, "internal_list_rpm_applications"; 511, "internal_readdir"; -512, "clevis_luks_unlock" +512, "clevis_luks_unlock"; +513, "inspect_get_build_id"; ] (* End of list. If adding a new entry, add it at the end of the list diff --git a/lib/MAX_PROC_NR b/lib/MAX_PROC_NR index 4d0e90cbcb..31cf34b8d0 100644 --- a/lib/MAX_PROC_NR +++ b/lib/MAX_PROC_NR @@ -1 +1 @@ -512 +513 -- 2.37.3
For Windows Client, we can only distinguish between Windows 10 and Windows 11 using the build ID. The product name in both cases is "Windows 10 <something>", apparently intentionally. References: https://learn.microsoft.com/en-us/answers/questions/586619/windows-11-build-... https://github.com/cygwin/cygwin/blob/a263fe0b268580273c1adc4b1bad2561479902... https://en.wikipedia.org/wiki/List_of_Microsoft_Windows_versions After this fix, the output of virt-inspector changes to this, which is a bit odd, but correct: <name>windows</name> <arch>x86_64</arch> <distro>windows</distro> <product_name>Windows 10 Pro</product_name> <product_variant>Client</product_variant> <major_version>10</major_version> <minor_version>0</minor_version> <windows_systemroot>/Windows</windows_systemroot> <windows_current_control_set>ControlSet001</windows_current_control_set> <osinfo>win11</osinfo> Thanks: Yaakov Selkowitz Reported-by: Yongkui Guo Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2012658 --- lib/inspect-osinfo.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/lib/inspect-osinfo.c b/lib/inspect-osinfo.c index 90e57e6dfa..9a22a9d037 100644 --- a/lib/inspect-osinfo.c +++ b/lib/inspect-osinfo.c @@ -86,6 +86,8 @@ guestfs_impl_inspect_get_osinfo (guestfs_h *g, const char *root) else if (STREQ (type, "windows")) { CLEANUP_FREE char *product_name = NULL; CLEANUP_FREE char *product_variant = NULL; + CLEANUP_FREE char *build_id_str = NULL; + int build_id; product_name = guestfs_inspect_get_product_name (g, root); if (!product_name) @@ -142,8 +144,27 @@ guestfs_impl_inspect_get_osinfo (guestfs_h *g, const char *root) return safe_strdup (g, "win2k19"); else return safe_strdup (g, "win2k16"); - } else - return safe_strdup (g, "win10"); + } + else { + /* For Windows >= 10 Client we can only distinguish between + * versions by looking at the build ID. See: + * https://learn.microsoft.com/en-us/answers/questions/586619/windows-11-build-... + * https://github.com/cygwin/cygwin/blob/a263fe0b268580273c1adc4b1bad2561479902... + */ + build_id_str = guestfs_inspect_get_build_id (g, root); + if (!build_id_str) + return NULL; + + if (sscanf (build_id_str, "%d", &build_id) != 1) { + error (g, "cannot parse Windows build ID from this guest"); + return NULL; + } + + if (build_id >= 2200) + return safe_strdup (g, "win11"); + else + return safe_strdup (g, "win10"); + } } break; } -- 2.37.3
On Thu, Dec 01, 2022 at 10:34:09AM +0000, Richard W.M. Jones wrote:
For Windows Client, we can only distinguish between Windows 10 and Windows 11 using the build ID. The product name in both cases is "Windows 10 <something>", apparently intentionally.
References: https://learn.microsoft.com/en-us/answers/questions/586619/windows-11-build-... https://github.com/cygwin/cygwin/blob/a263fe0b268580273c1adc4b1bad2561479902... https://en.wikipedia.org/wiki/List_of_Microsoft_Windows_versions
After this fix, the output of virt-inspector changes to this, which is a bit odd, but correct:
<name>windows</name> <arch>x86_64</arch> <distro>windows</distro> <product_name>Windows 10 Pro</product_name> <product_variant>Client</product_variant> <major_version>10</major_version> <minor_version>0</minor_version> <windows_systemroot>/Windows</windows_systemroot> <windows_current_control_set>ControlSet001</windows_current_control_set> <osinfo>win11</osinfo>
Thanks: Yaakov Selkowitz Reported-by: Yongkui Guo Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2012658 --- lib/inspect-osinfo.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/lib/inspect-osinfo.c b/lib/inspect-osinfo.c index 90e57e6dfa..9a22a9d037 100644 --- a/lib/inspect-osinfo.c +++ b/lib/inspect-osinfo.c @@ -86,6 +86,8 @@ guestfs_impl_inspect_get_osinfo (guestfs_h *g, const char *root) else if (STREQ (type, "windows")) { CLEANUP_FREE char *product_name = NULL; CLEANUP_FREE char *product_variant = NULL; + CLEANUP_FREE char *build_id_str = NULL; + int build_id;
product_name = guestfs_inspect_get_product_name (g, root); if (!product_name) @@ -142,8 +144,27 @@ guestfs_impl_inspect_get_osinfo (guestfs_h *g, const char *root) return safe_strdup (g, "win2k19"); else return safe_strdup (g, "win2k16"); - } else - return safe_strdup (g, "win10"); + } + else { + /* For Windows >= 10 Client we can only distinguish between + * versions by looking at the build ID. See: + * https://learn.microsoft.com/en-us/answers/questions/586619/windows-11-build-... + * https://github.com/cygwin/cygwin/blob/a263fe0b268580273c1adc4b1bad2561479902... + */ + build_id_str = guestfs_inspect_get_build_id (g, root); + if (!build_id_str) + return NULL; + + if (sscanf (build_id_str, "%d", &build_id) != 1) { + error (g, "cannot parse Windows build ID from this guest"); + return NULL; + } + + if (build_id >= 2200)
This should of course be 22000! Updated my copy. Rich.
+ return safe_strdup (g, "win11"); + else + return safe_strdup (g, "win10"); + } } break; } -- 2.37.3
_______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
On 12/01/22 12:20, Richard W.M. Jones wrote:
On Thu, Dec 01, 2022 at 10:34:09AM +0000, Richard W.M. Jones wrote:
For Windows Client, we can only distinguish between Windows 10 and Windows 11 using the build ID. The product name in both cases is "Windows 10 <something>", apparently intentionally.
References: https://learn.microsoft.com/en-us/answers/questions/586619/windows-11-build-... https://github.com/cygwin/cygwin/blob/a263fe0b268580273c1adc4b1bad2561479902... https://en.wikipedia.org/wiki/List_of_Microsoft_Windows_versions
After this fix, the output of virt-inspector changes to this, which is a bit odd, but correct:
<name>windows</name> <arch>x86_64</arch> <distro>windows</distro> <product_name>Windows 10 Pro</product_name> <product_variant>Client</product_variant> <major_version>10</major_version> <minor_version>0</minor_version> <windows_systemroot>/Windows</windows_systemroot> <windows_current_control_set>ControlSet001</windows_current_control_set> <osinfo>win11</osinfo>
Thanks: Yaakov Selkowitz Reported-by: Yongkui Guo Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2012658 --- lib/inspect-osinfo.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/lib/inspect-osinfo.c b/lib/inspect-osinfo.c index 90e57e6dfa..9a22a9d037 100644 --- a/lib/inspect-osinfo.c +++ b/lib/inspect-osinfo.c @@ -86,6 +86,8 @@ guestfs_impl_inspect_get_osinfo (guestfs_h *g, const char *root) else if (STREQ (type, "windows")) { CLEANUP_FREE char *product_name = NULL; CLEANUP_FREE char *product_variant = NULL; + CLEANUP_FREE char *build_id_str = NULL; + int build_id;
product_name = guestfs_inspect_get_product_name (g, root); if (!product_name) @@ -142,8 +144,27 @@ guestfs_impl_inspect_get_osinfo (guestfs_h *g, const char *root) return safe_strdup (g, "win2k19"); else return safe_strdup (g, "win2k16"); - } else - return safe_strdup (g, "win10"); + } + else { + /* For Windows >= 10 Client we can only distinguish between + * versions by looking at the build ID. See: + * https://learn.microsoft.com/en-us/answers/questions/586619/windows-11-build-... + * https://github.com/cygwin/cygwin/blob/a263fe0b268580273c1adc4b1bad2561479902... + */ + build_id_str = guestfs_inspect_get_build_id (g, root); + if (!build_id_str) + return NULL;
Does this deserve an error() call like the one below?
+ + if (sscanf (build_id_str, "%d", &build_id) != 1) { + error (g, "cannot parse Windows build ID from this guest"); + return NULL; + } + + if (build_id >= 2200)
This should of course be 22000! Updated my copy.
In addition, please replace sscanf() with strtol(). The behavior of the former is undefined when the subject sequence forms a valid decimal string, but the numeric value does not fit into an "int". And, this is untrusted data. strtol() handles this securely (although strtol() is not trivial to use). I seem to remember gnulib offers xstrol() -- indeed, our trimmed, embedded copy of gnulib offers xstrtol(), from commit 0f54df53d26e ("build: Remove gnulib.", 2021-04-08); so that's the function to use here, IMO. series Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Rich.
+ return safe_strdup (g, "win11"); + else + return safe_strdup (g, "win10"); + } } break; } -- 2.37.3
_______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
On Fri, Dec 02, 2022 at 10:11:32AM +0100, Laszlo Ersek wrote:
On 12/01/22 12:20, Richard W.M. Jones wrote:
On Thu, Dec 01, 2022 at 10:34:09AM +0000, Richard W.M. Jones wrote:
For Windows Client, we can only distinguish between Windows 10 and Windows 11 using the build ID. The product name in both cases is "Windows 10 <something>", apparently intentionally.
References: https://learn.microsoft.com/en-us/answers/questions/586619/windows-11-build-... https://github.com/cygwin/cygwin/blob/a263fe0b268580273c1adc4b1bad2561479902... https://en.wikipedia.org/wiki/List_of_Microsoft_Windows_versions
After this fix, the output of virt-inspector changes to this, which is a bit odd, but correct:
<name>windows</name> <arch>x86_64</arch> <distro>windows</distro> <product_name>Windows 10 Pro</product_name> <product_variant>Client</product_variant> <major_version>10</major_version> <minor_version>0</minor_version> <windows_systemroot>/Windows</windows_systemroot> <windows_current_control_set>ControlSet001</windows_current_control_set> <osinfo>win11</osinfo>
Thanks: Yaakov Selkowitz Reported-by: Yongkui Guo Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2012658 --- lib/inspect-osinfo.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/lib/inspect-osinfo.c b/lib/inspect-osinfo.c index 90e57e6dfa..9a22a9d037 100644 --- a/lib/inspect-osinfo.c +++ b/lib/inspect-osinfo.c @@ -86,6 +86,8 @@ guestfs_impl_inspect_get_osinfo (guestfs_h *g, const char *root) else if (STREQ (type, "windows")) { CLEANUP_FREE char *product_name = NULL; CLEANUP_FREE char *product_variant = NULL; + CLEANUP_FREE char *build_id_str = NULL; + int build_id;
product_name = guestfs_inspect_get_product_name (g, root); if (!product_name) @@ -142,8 +144,27 @@ guestfs_impl_inspect_get_osinfo (guestfs_h *g, const char *root) return safe_strdup (g, "win2k19"); else return safe_strdup (g, "win2k16"); - } else - return safe_strdup (g, "win10"); + } + else { + /* For Windows >= 10 Client we can only distinguish between + * versions by looking at the build ID. See: + * https://learn.microsoft.com/en-us/answers/questions/586619/windows-11-build-... + * https://github.com/cygwin/cygwin/blob/a263fe0b268580273c1adc4b1bad2561479902... + */ + build_id_str = guestfs_inspect_get_build_id (g, root); + if (!build_id_str) + return NULL;
Does this deserve an error() call like the one below?
+ + if (sscanf (build_id_str, "%d", &build_id) != 1) { + error (g, "cannot parse Windows build ID from this guest"); + return NULL; + } + + if (build_id >= 2200)
This should of course be 22000! Updated my copy.
In addition, please replace sscanf() with strtol(). The behavior of the former is undefined when the subject sequence forms a valid decimal string, but the numeric value does not fit into an "int". And, this is untrusted data. strtol() handles this securely (although strtol() is not trivial to use).
I really think we (meaning Eric :-) should get scanf fixed, but yes I'll replace this with xstrol since we have it around.
I seem to remember gnulib offers xstrol() -- indeed, our trimmed, embedded copy of gnulib offers xstrtol(), from commit 0f54df53d26e ("build: Remove gnulib.", 2021-04-08); so that's the function to use here, IMO.
series Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks, Rich.
Rich.
+ return safe_strdup (g, "win11"); + else + return safe_strdup (g, "win10"); + } } break; } -- 2.37.3
_______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
On Fri, Dec 02, 2022 at 09:59:57AM +0000, Richard W.M. Jones wrote:
In addition, please replace sscanf() with strtol(). The behavior of the former is undefined when the subject sequence forms a valid decimal string, but the numeric value does not fit into an "int". And, this is untrusted data. strtol() handles this securely (although strtol() is not trivial to use).
I really think we (meaning Eric :-) should get scanf fixed, but yes I'll replace this with xstrol since we have it around.
POSIX is reluctant to add a sane errno setting for scanf failures on integer overflow without existing practice, and we've probably missed any window of raising it as a complaint to the C23 folks. It's been known-broken for 30+ years, and no one has had any bright ideas how to fix it so that code that cares can check for overflow without too much boilerplate and existing code that doesn't expect errors doesn't break. The strtol* family is the only portably safe way to parse untrusted input as integers. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
participants (3)
-
Eric Blake -
Laszlo Ersek -
Richard W.M. Jones