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

Add a generic hostsfile artifact #2930

Merged
merged 7 commits into from
Sep 9, 2023

Conversation

misje
Copy link
Contributor

@misje misje commented Sep 6, 2023

I wrote an artifact for parsing hosts files on Linux/macOS. Afterwards, I realised there already exists one for Windows. The file format is the same on all three supported OSes, so I don't see a reason for not having a generic artifact that runs on all of them.

The differences now are

  • I match the array of hostnames against the regex, allowing one to use anchors like "^"/"$" in the regex (instead of matching the string of hostname/aliases)
  • I provide flattened query as well, with one hostname per row
  • The parsing function is exported
  • The hosts filename parameter is now a glob

Let me know what you want to do with this. Include it and leave the existing Windows.System.HostsFile? Replace the existing? Suggest this to belong in the exchange instead? Something else?

I'll add the test when I know how to proceed.

@scudette
Copy link
Contributor

scudette commented Sep 6, 2023

This makes sense to expand this to all supported OSs. I like the multiple globs in a table which will cater to all OSs and the export function.

There is already a test here https://github.com/Velocidex/velociraptor/blob/master/artifacts/testdata/server/testcases/hostsfile.in.yaml

Do you think it has enough coverage?

We can add this generic one as an alias (or the windows one be the alias for this one) and remove the precondition?

@misje
Copy link
Contributor Author

misje commented Sep 9, 2023

I've just added a few full-line comments to the test file to improve the regex testing. Otherwise I think it's fine. I've renamed the .in file to run the new artifact name, but I haven't been able to produce new "golden" .out files. Exactly how do I go about doing that to override the previous .out file? Simply removing the file and running make golden fails because "files did not match". Also, what filter should I use to avoid running all tests?

How does artifact aliasing work?

@scudette
Copy link
Contributor

scudette commented Sep 9, 2023

Not sure we can use aliasing here because the parameters of the old artifact are not the same as this one.

I added a test and fixed a small bug in the filtering. I also realized there is not much documentation about running the golden tests so I added some.

@scudette scudette marked this pull request as ready for review September 9, 2023 13:21
@scudette scudette force-pushed the add-generic-hostsfile-artifact branch from 4e5d973 to a8cee65 Compare September 9, 2023 14:02
@scudette scudette merged commit 8928682 into Velocidex:master Sep 9, 2023
1 check passed
@misje
Copy link
Contributor Author

misje commented Sep 10, 2023

I notice that the Hosts source now returns an array in Hostname instead of a single space-separated string. I just wanted to verify that this is intended. The thought behind the combined string was readability, since there is already a flattened source which I would assume would be better for filtering/parsing. – Which may be an incorrect assumption, in which case I will avoid this in the future as well.

In the original version I also manually specified the columns in the final query to get a more natural column order: OSPath, Address, Hostname, Comment. This is now Hostname, Comment, Address, OSPath for the source HostsFlattened. Could we keep the original behaviour, perhaps?

@scudette
Copy link
Contributor

It's always better to have structured data rather than space separated data so it's preferable to have the hostnames in an array. I did notice that the output was slightly different from the windows hostname artifact so tried to get it as close to that as possible.

It's true the that flattened version is better for filtering and maybe we want to just have that and drop the other one?

It looks like the artifact is different enough from the windows version so it's not easy to change over. Maybe we just have two artifacts and deprecate the windows one?

We can change the order of the columns for sure.

@mgreen27
Copy link
Collaborator

I think I originally added the array view to show all items for an entry when running workflow in the gui, it made more sense to show all associated items instead of what hits on your filter.

I never really used a flattened view, but the additional scope works or another option is a notebook suggestion.

@scudette
Copy link
Contributor

Interesting :-) I specifically filtered the associated entries to only those that matched.

The only issue is that the view only actually shows aliases so if there is another line with the same IP address we dont show that as an associated hit.

scudette added a commit that referenced this pull request Sep 10, 2023
I wrote an artifact for parsing hosts files on Linux/macOS. Afterwards,
I realised there already [exists one for
Windows](https://github.com/Velocidex/velociraptor/blob/master/artifacts/definitions/Windows/System/HostsFile.yaml).
The file format is the same on all three supported OSes, so I don't see
a reason for not having a generic artifact that runs on all of them.

The differences now are

- I match the array of hostnames against the regex, allowing one to use
anchors like "^"/"$" in the regex (instead of matching the string of
hostname/aliases)
- I provide flattened query as well, with one hostname per row
- The parsing function is exported
- The hosts filename parameter is now a glob

Let me know what you want to do with this. Include it and leave the
existing Windows.System.HostsFile? Replace the existing? Suggest this to
belong in the exchange instead? Something else?

I'll add the test when I know how to proceed.

---------

Co-authored-by: Mike Cohen <[email protected]>
scudette added a commit that referenced this pull request Sep 10, 2023
I wrote an artifact for parsing hosts files on Linux/macOS. Afterwards,
I realised there already [exists one for
Windows](https://github.com/Velocidex/velociraptor/blob/master/artifacts/definitions/Windows/System/HostsFile.yaml).
The file format is the same on all three supported OSes, so I don't see
a reason for not having a generic artifact that runs on all of them.

The differences now are

- I match the array of hostnames against the regex, allowing one to use
anchors like "^"/"$" in the regex (instead of matching the string of
hostname/aliases)
- I provide flattened query as well, with one hostname per row
- The parsing function is exported
- The hosts filename parameter is now a glob

Let me know what you want to do with this. Include it and leave the
existing Windows.System.HostsFile? Replace the existing? Suggest this to
belong in the exchange instead? Something else?

I'll add the test when I know how to proceed.

---------

Co-authored-by: Mike Cohen <[email protected]>
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