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

Added file.drive_letter #620

Merged
merged 10 commits into from
Dec 11, 2019
Merged

Added file.drive_letter #620

merged 10 commits into from
Dec 11, 2019

Conversation

webmat
Copy link
Contributor

@webmat webmat commented Nov 20, 2019

No description provided.

@webmat webmat self-assigned this Nov 20, 2019
code/go/ecs/file.go Outdated Show resolved Hide resolved
code/go/ecs/file.go Outdated Show resolved Hide resolved
- New field now called `file.drive_letter`
- Dir & path should include the drive letter
@webmat webmat changed the title Added file.drive Added file.drive_letter Nov 25, 2019
@dcode
Copy link
Contributor

dcode commented Dec 3, 2019

My only hesitation on this is that drive_letter is very OS specific. Something like filesystem_root might be more agnostic, but you can't tell that directly from a path. From a POSIX stat call on a given file, you would know what device that file existed on (and its inode). You can essentially get almost this same information with a Windows stat call (see _stat structure), but by convention this isn't as common, I think.

For a given Linux file, for counter example, it might be nice to know that a binary existed in a filesystem mounted at /home, which existed on /dev/sda5. You can't tell that from directly looking at the path, but if I'm populating data from an agent on a system, that information would be available via the mount table and/or stat call.

At the end of the day, we're trying to facilitate analysis of the data though, not to be pedantically correct. So, I think this is a reasonable approach to facilitate analysis, but I wanted to share food for thought to see if others had a better abstraction.

@NeilADesai
Copy link

There are also times where someone on a Windows host might access the drives without using the drive letter: https://support.microsoft.com/en-us/help/100027/info-direct-drive-access-under-win32. I have seen this used in forensics and by malicious users/software.

@webmat
Copy link
Contributor Author

webmat commented Dec 3, 2019

@NeilADesai Actually we already have file.device, whose definition right now is pretty Linux specific. Do you think it would make sense to expand its definition for capturing drives such as \\.\PhysicalDrive1 in there? Is it the canonical ID of the drive? The drive numbering sounds ephemeral to me (haven't read the whole thing).

@rw-access rw-access self-requested a review December 6, 2019 19:02
@webmat
Copy link
Contributor Author

webmat commented Dec 9, 2019

I'm satisfied with this one. I'm ready to merge.

Just waiting for official reviews :-)

schemas/file.yml Outdated
ignore_above: 1
short: Drive letter where the file is located.
description: >
Drive letter where the file is located.
Copy link
Member

Choose a reason for hiding this comment

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

I think the docs should mention this is a Windows concept to alleviate any confusion on the part of implementers working on non-Windows file info.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andrewkroh Done.

Do you see any issue with stripping off the :?

Copy link
Member

Choose a reason for hiding this comment

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

No issue. It doesn't provide any additional information.

@webmat webmat merged commit 838f50c into elastic:master Dec 11, 2019
dcode pushed a commit to dcode/ecs that referenced this pull request Apr 15, 2020
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.

None yet

5 participants