feat(windows): obtain machine-guid [NR-480703] #1809
Conversation
sigilioso
force-pushed
the
feat/windows-machine-guid
branch
3 times, most recently
from
0caa201 to
07e9736
Compare
sigilioso
force-pushed
the
feat/windows-machine-guid
branch
from
07e9736 to
1bb265c
Compare
sigilioso
changed the title
danielorihuela
left a comment
danielorihuela
left a comment
There was a problem hiding this comment.
I don't see any issue. But I'm curious, the machine id is unique for each machine, right? Have you tried out if the same result is returned? Maybe it's not because of using a VM.
Apparently, the |
|
We'll wait for #1752 to merge this. |
Okey. I thought this was the MAC and that it should be the same in unix, windows, etc. I see now that is a unique UUID that can be changed. Thanks!!! |
sigilioso
requested review from
a team,
DavSanchez,
danielorihuela and
paologallinaharbur
DavSanchez
left a comment
DavSanchez
left a comment
There was a problem hiding this comment.
I only have some nits due to not knowing much about what's happening in the unsafe blocks, the syscalls etc. If I get some new insights when I advance in the Windows process management I'll open PRs with proposals. Thanks! 👌
| } | ||
|
|
||
| // Allocate buffer and read the actual value | ||
| let mut buffer: Vec<u16> = vec![0; (data_size / 2) as usize]; |
There was a problem hiding this comment.
Ah didn't know the macro allowed you to set the capacity 👌
| #[derive(Default)] | ||
| pub struct MachineIdentityProvider {} |
There was a problem hiding this comment.
If we don't need Default due to constraints or something I'd leave it. We can create a value of this (zero-sized) type just with let x = MachineIdentityProvider;.
| #[derive(Default)] | |
| pub struct MachineIdentityProvider {} | |
| pub struct MachineIdentityProvider; |
There was a problem hiding this comment.
That's true, however, the interface that both unix and windows implementations satisfy include the default.
We could add some abstractions to overcome it, but I'm not sure if it is worth it.
| unsafe { | ||
| // Open the registry key | ||
| let mut registry_key: *mut std::ffi::c_void = std::ptr::null_mut(); |
| unsafe fn close_registry_key(hkey: *mut std::ffi::c_void) -> Result<(), SystemDetectorError> { | ||
| unsafe { | ||
| let result = RegCloseKey(hkey); | ||
| if result != ERROR_SUCCESS { | ||
| return Err(SystemDetectorError::MachineIDError(format!( | ||
| "failed to close the registry key: error code {result}" | ||
| ))); | ||
| } | ||
| Ok(()) | ||
| } | ||
| } |
There was a problem hiding this comment.
Is this a thing that can cause weird behavior if we forget to perform it? If so, we should probably tie it to a value lifecycle to ensure it gets called always (like impl Drop, though that would be at the expense of error handling) or something, particularly if we are going to reuse this code.
Just out of curiosity.
There was a problem hiding this comment.
I'm not completely sure of the consequences of not-closing. If we finally refactor the code (note that everything is private as of now) to make it reusable let's take it into account. For now the helper is merely mapping the error in case of failure.
This PR adds support for obtaining the machine identifier on Windows systems by reading the
MachineGuidfrom the Windows Registry (HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Cryptography). Previously, the machine ID detection only worked on Unix-based systems (Linux/macOS) by reading from/etc/machine-idor/var/lib/dbus/machine-id.Checklist
Summary of changes
resource-detection/src/system/machine_identifier/windows.rsthat reads theMachineGuidfrom the Windows Registry path.machine_identifier/unix.rs+machine_identifier/windows.rs)IdentifierProviderMachineId→MachineIdentityProvider.Dependencies
winreg: Windows-specific dependency for registry accessrstestto workspace dependencies.THIRD_PARTY_NOTICES.mdManual testing
MachineGuidwas read from the registry as expected).