Skip to content

Conversation

@sgress454
Copy link
Contributor

For #7509
For fleetdm/fleet#28535

Details

This PR updates the logic for generating, storing and returning host identifiers when --host-identifier=uuid is used. Previously, once a UUID was determined (either by getting a valid hardware UUID from the device, or failing that, creating a random UUID) it was stored in the local db and returned henceforth on every call to getHostUUID(). With this update, the logic becomes:

  1. Always check for a hardware UUID the first time getHostUUID() is called.
  2. If one is found, store it in the db.
  3. Check the db for a value.
  4. If one is found, return it.
  5. Otherwise generate a random UUID, store it in the db and return it.

This handles the most pertinent cases of 1) moving an osquery db between two hosts with different hardware UUIDs and 2) the hardware UUID of a host changing (e.g. due to a motherboard being swapped out). It also maintains the behavior of randomly-generated UUIDs persisting across osquery restarts (provided that no hardware UUID is found) so that VMs using --host-identifier=uuid get a consistent UUID.

The behavior when moving from a system with a hardware UUID to a system without one (or moving between two VMs) may still be considered sub-optimal, but those use-cases don't seem as problematic and I'm not sure what a good solve would be.

Testing

I tested this manually with:

./osquery/osqueryi --verbose --database_path=./osquery.db --disable_database=false

and manipulating the db using ldb. I also simulated VM usage by temporarily adding my laptop's UUID to the "placeholders" list.

I'd love to add a unit test for the new logic but the only way I can think to do that would be to change getHostUUID to use dependency-injection so I could mock generateNewUUID(), getDatabaseValue() and setDatabaseValue(). I haven't seen that pattern elsewhere in osquery though.

@sgress454 sgress454 requested review from a team as code owners May 30, 2025 17:10
@sgress454 sgress454 force-pushed the sgress454/28535-fix-hardware-uuid-detection branch from 2df7187 to 77449ae Compare June 5, 2025 15:06
getvictor
getvictor previously approved these changes Jun 6, 2025
Copy link
Contributor

@getvictor getvictor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving, with comments.


if (!checked_hardware_uuid) {
// Attempt to get the hardware UUID from the system.
std::string hardware_uuid = osquery::generateHostUUID();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit. The function names make the code hard to read/maintain. The generateHostUUID function actually tries to get the UUID from hardware, and does not generate. While this function either gets from hardware, gets from DB, or generates. It would be nice to have better names.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok updating this, it's only called from one spot.

@sgress454 sgress454 force-pushed the sgress454/28535-fix-hardware-uuid-detection branch from 77449ae to 835552b Compare June 10, 2025 15:59
@sgress454 sgress454 force-pushed the sgress454/28535-fix-hardware-uuid-detection branch from 835552b to babae5c Compare June 10, 2025 16:13
Copy link
Contributor Author

@sgress454 sgress454 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@getvictor updated generateHostUUID to getHardwareUUID as suggested, and addressed other comment.

Copy link
Contributor

@getvictor getvictor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@getvictor getvictor merged commit 85c8389 into osquery:master Jun 10, 2025
34 of 42 checks passed
@Anthony-76 Anthony-76 mentioned this pull request Jan 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants