Rename hvci_status to deviceguard_status to better reflect the data collected.#8390
Rename hvci_status to deviceguard_status to better reflect the data collected.#8390directionless merged 3 commits intoosquery:masterfrom
hvci_status to deviceguard_status to better reflect the data collected.#8390Conversation
…ollected. Also fix incorrect comparison causing status data to always return UNKNOWN.
| data.GetLong("VirtualizationBasedSecurityStatus", vbs_status); | ||
| r["vbs_status"] = | ||
| vbs_methods.size() < vbs_status ? vbs_methods[vbs_status] : "UNKNOWN"; | ||
| vbs_methods.size() > vbs_status ? vbs_methods[vbs_status] : "UNKNOWN"; |
There was a problem hiding this comment.
Is this logic change intentional?
There was a problem hiding this comment.
Yes it is. Unfortunately the logic error appears to have been present since the initial commit of this module. The size of the vector should always be greater than the status value when the value is known. I have verified that the current release has broken behavior on these statuses as well.
Co-authored-by: Stefano Bonicatti <smjert@gmail.com>
michael-myers
changed the title
hvci_status to deviceguard_status to better reflect the data collected.
|
Any further concerns with this PR? |
Since this also changes the name of the table, I'll ask @directionless for thoughts. You can see an example here: https://github.com/osquery/osquery/blob/master/specs/etc_hosts.table#L1 But if we want to do a hard cut I'm fine either way. |
|
Thanks for the tip; I've added the old name in as an alias for good measure. |
|
Any update/concerns here? I was hoping to get this merged a while back after the alias was added. |
I don't have opinions here -- If the old table had a logic bug, but I think no one using it would have succeeded, and I don't feel like a hard cut is terrible. I'm not really up to date on this part of windows. So I'm deferring toi @Smjert here |
|
I've made the change to include the alias for good measure, but to be clear as of right now this module is broken without the logic fix in this PR. The sooner we can get this merged the better. We originally added this table, so we'd likely be the primary user anyway, at least initially. |
|
@Smjert let me know if you need anything else from me here. |
3 participants
Also fix incorrect comparison causing status data to always return
UNKNOWN.