Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows version conditions do not apply for Windows 10 #274

Closed
DeKe42 opened this issue Jul 16, 2018 · 9 comments
Closed

Windows version conditions do not apply for Windows 10 #274

DeKe42 opened this issue Jul 16, 2018 · 9 comments
Assignees

Comments

@DeKe42
Copy link
Contributor

DeKe42 commented Jul 16, 2018

Some artifacts have conditions in the form of os_major_version >= X AND os_minor_version >= Y.
This fails starting with Windows 10, which can have version 10.0. A condition like os_major_version >= 6 AND os_minor_version >= 1 will fail here although the artifact applies.

One example is

conditions: [os_major_version >= 6 AND os_minor_version >= 1]

I can think of multiple solutions to this:

  • More complicated conditions like (os_major_version >= 6 AND os_minor_version >= 1) OR os_major version >= 7
  • A semantic version compare, something like os_version >= "6.1"
@joachimmetz joachimmetz self-assigned this Jul 16, 2018
@joachimmetz
Copy link
Member

@DeKe42 thanks for the report. I'll consult with @grrrrrrrrr how GRR implements this. Moving to versions like "6.1" looks like a better approach. Alternative is to drop the version all together.

@joachimmetz
Copy link
Member

joachimmetz commented Jul 18, 2018

Affected artifact definitions:

windows.yaml
WindowsActiveDesktop, WindowsAMCacheHveFile, WinAppXRT, WindowsEnvironmentVariableAppxProcess, WindowsEventLogApplication, WindowsEventLogSecurity, WindowsEventLogSystem, WindowsXMLEventLogApplication, WindowsXMLEventLogSecurity, WindowsXMLEventLogSystem, WindowsXMLEventLogTerminalServices, WindowsOpenSaveMRU, WindowsOpenSavePidlMRU, WindowsRecentFileCacheBCF, WindowsRunGrpConv, WindowsSetupApiLogs,

wmi.yaml
WMIDrivers, WMIHotFixes, WMIInstalledSoftware, WMILoginUsers, WMIPhysicalMemory, WMIProcessList,

@joachimmetz
Copy link
Member

@joachimmetz
Copy link
Member

joachimmetz commented Jul 18, 2018

GRR knowledge base gets filled with a separate value for os_major_version and os_minor_version. For Linux via provides but not for Windows?

Looks like SetCoreGRRKnowledgeBaseValues is used to set os_major_version:
https://github.com/google/grr/blob/68a45fd446704285af25403d8dde4d3630524165/grr/server/grr_response_server/artifact.py#L80

Values in client_obj are set by FromCurrentSystem:
https://github.com/google/grr/blob/68a45fd446704285af25403d8dde4d3630524165/grr/core/grr_response_core/lib/rdfvalues/client.py#L991

@coperni
Copy link

coperni commented Jul 2, 2022

I'm keen on taking this issue on. Going for the latter solution with semantic versioning seems the cleanest approach. Assuming my changes get approved I will make a follow-up PR here to adjust some of these definitions.

@joachimmetz any concerns with the semantic versioning approach? I know this has been sitting in the back burner and your thoughts may have changed.

coperni added a commit to coperni/grr that referenced this issue Jul 2, 2022
A semantic version should be used for checking the condition of
the client's operating system rather than evaluating the OS major
AND minor version independently.

See: ForensicArtifacts/artifacts/issues/274
@joachimmetz
Copy link
Member

what do you specifically mean with the "semantic versioning approach" in this context. as in treating the version as string? A semantic version compare, something like os_version >= "6.1"

I'm wondering what value the version information adds. Maybe we can it all together.

@coperni
Copy link

coperni commented Jul 3, 2022

I am referring to the second solution proposed by @DeKe42, the "semantic version compare" e.g. os_version >= "X.T". As it stands today artifact collection in grr is still broken (on Windows 10+) for these artifacts.

I don't believe adding os_version will add any additional value to the ForesenicArtifact repository as a whole if we leave os_major_version and os_minor_version in tact. This is more of a remedy patch for the grr codebases CheckCondition logic.

What are you specifically referring to when you say can it all together? The issue #274? The proposed solution of adding os_version as a condition item? Or the affected condition statements for the aforementioned artifacts?

@joachimmetz
Copy link
Member

What are you specifically referring to when you say can it all together?

I mean removing it. What is the value it adds? Let me check with the GRR folks but this might be a left over from early days.

@joachimmetz
Copy link
Member

joachimmetz commented Jul 10, 2022

Removed conditions #515

Rationale: os_major_version and os_minor_version use the kernel version. Artifact definitions are typically product level specific not kernel level.

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

No branches or pull requests

3 participants