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

auditd-based file integrity monitoring #3492

Merged
merged 70 commits into from Aug 24, 2017

Conversation

Projects
None yet
5 participants
@alessandrogario
Contributor

alessandrogario commented Jul 24, 2017

I have implemented file integrity monitoring using the auditd service. This can come in handy as inotify can only subscribe for a limited amount of entities, and can’t be easily changed at runtime by osquery without risking invalidation of the existing watchers.

Using auditd for file integrity monitoring offers the following advantages:

  • Correlates process id and executable path atomically using the audit event records. This means that you don’t risk resolving the process id to the wrong executable path.
  • Supports anything that the open() syscall can receive (special directories, NFS/SMB/FUSE mount points, etc.)
  • Logs the actual syscall parameters for later inspection.

In general, our implementation can be easily expanded in the future to support more event publishers, since the audit netlink can now be shared. This should also empower subscribers, allowing them to use a more lightweight filtering logic suitable for the task they needs to perform. In order to make auditd suitable for file integrity monitoring, we overcame several performance issues when receiving and processing events from the audit netlink. Additionally, subscribers are no longer blocking each other and/or the netlink reader when processing the event records.

How auditd-based file integrity monitoring works

The new implementation works by tracking process/handle creation and destruction by monitoring the execve/exit/exit_group and open/close/dup syscalls.

The file descriptors are saved inside a handle table (one for each process) in the auditd_file_events subscriber. The tables are then used to solve the file descriptors when the following system calls are executed: read/write/mmap.

Rows are only inserted inside the table when a file changes state. Valid state changes are: Open -> Read -> Write -> Close.

Configuration is not yet supported but I plan on adding it, in a way similar to the existing file_events table.

Issues found and fixed in the original code

While I was separating the netlink reader and the AuditAssembler logic, I encountered and fixed the following issues:

  • [d075b79] Audit event IDs were not correctly handled; according to the documentation (see the commit message for more information) the primary key is made by both the event time and the event id. This caused the class to receive events that perceived as duplicates, tossing them away using an internal history of the last received events.

  • [ec57b07] Fixed syscall filtering. This caused subscribers to receive events for syscalls they did not subscribe for.

Modified or added components:

AuditNetlink (new)
This class is responsible for reading from the audit netlink, and has been implemented to make it possible to have more than one consumer.

This is needed because the existing AuditAssembler component is not capable of sharing the audit handle and it doesn’t support complex filtering logic (which makes it unsuitable for certain use-cases). Another issue is that it does not allow the consumers to subscribe for syscalls that receive different types (and amount) of records (i.e.: open() and mmap()).

Two threads are used, one for receiving the records, and another one for parsing them. Subscribers are given a handle to request new updates.

Configuration options have been all moved here, as having all settings here is better than implementing conflict-resolution logic to merge all the (possibly) different rules requested by each subscriber.

AuditFim (new)
This new publisher receives events from the AuditNetlink class, and uses record terminators (AUDIT_EOE) to isolate and dispatch file system changes to the auditd_file_events subscriber. This should have less overhead than attempting to complete the events manually (as the AuditAssembler class does), and was initially implemented because some of the syscalls I listen for (i.e.: read and write) are called very often.

AuditEventPublisher (changed)
This class has been updated to use the new AuditNetlink class. This component was not suitable for my use case (see the previous two points), so I decoupled it from the audit netlink reader.

auditd_file_events (new)
This table receives the events from the AuditFim publisher, building a file descriptor table for each running process. This is also where filtering is implemented, in order to only show file state changes (vs showing all syscalls).

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Jul 24, 2017

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

facebook-github-bot commented Jul 24, 2017

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot commented Jul 25, 2017

@alessandrogario updated the pull request - view changes

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot commented Jul 25, 2017

@alessandrogario updated the pull request - view changes

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Jul 25, 2017

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

facebook-github-bot commented Jul 25, 2017

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot commented Jul 29, 2017

@alessandrogario updated the pull request - view changes

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot commented Jul 30, 2017

@alessandrogario updated the pull request - view changes

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot commented Jul 30, 2017

@alessandrogario updated the pull request - view changes

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot commented Jul 30, 2017

@alessandrogario updated the pull request - view changes

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot commented Jul 31, 2017

@alessandrogario updated the pull request - view changes

@alessandrogario alessandrogario changed the title from [WIP] auditd-based file integrity monitoring to auditd-based file integrity monitoring Jul 31, 2017

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot commented Aug 2, 2017

@alessandrogario updated the pull request - view changes

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot commented Aug 2, 2017

@alessandrogario updated the pull request - view changes

@alessandrogario

This comment has been minimized.

Show comment
Hide comment
@alessandrogario

alessandrogario Aug 2, 2017

Contributor

I forgot to introduce some low-level syscalls (create/mknod/mknodat). They are now supported (the changes are smalls, as they are all handled the same way).

Contributor

alessandrogario commented Aug 2, 2017

I forgot to introduce some low-level syscalls (create/mknod/mknodat). They are now supported (the changes are smalls, as they are all handled the same way).

@theopolis

This comment has been minimized.

Show comment
Hide comment
@theopolis

theopolis Aug 3, 2017

Contributor

ok to test

Contributor

theopolis commented Aug 3, 2017

ok to test

@osqueryer

This comment has been minimized.

Show comment
Hide comment
@osqueryer

osqueryer Aug 3, 2017

Collaborator

👎 The commit 77c997c (Job results: 5408) failed one or more tests (Linux).

Collaborator

osqueryer commented Aug 3, 2017

👎 The commit 77c997c (Job results: 5408) failed one or more tests (Linux).

@osqueryer

This comment has been minimized.

Show comment
Hide comment
@osqueryer

osqueryer Aug 3, 2017

Collaborator

👎 The commit 77c997c (Job results: 5409) failed one or more tests (Linux).

Collaborator

osqueryer commented Aug 3, 2017

👎 The commit 77c997c (Job results: 5409) failed one or more tests (Linux).

@theopolis

This comment has been minimized.

Show comment
Hide comment
@theopolis

theopolis Aug 3, 2017

Contributor

The way I'm going to test this will be a little disorganized. Trying to move fast and include nitpick/style/approach/direction questions together, just want to apologize before hand. :P

I've created a squashed version of all the commits and will be using that to track changes locally as I learn the new implementation and tweak/performance-monitor on my test hosts.

Contributor

theopolis commented Aug 3, 2017

The way I'm going to test this will be a little disorganized. Trying to move fast and include nitpick/style/approach/direction questions together, just want to apologize before hand. :P

I've created a squashed version of all the commits and will be using that to track changes locally as I learn the new implementation and tweak/performance-monitor on my test hosts.

@theopolis

This comment has been minimized.

Show comment
Hide comment
@theopolis

theopolis Aug 3, 2017

Contributor

How does the publisher shutdown?

sudo ./build/linux/osquery/osqueryi --nodisable_events --nodisable_audit --audit_allow_config --audit_persist --audit_allow_sockets --verbose

osquery> ^D

I0802 19:52:07.823665  1052 auditdnetlink.cpp:798] Uninstalling the audit rules we have installed
I0802 19:52:07.823678  1052 auditdnetlink.cpp:814] Restoring the default configuration for the audit service
    

^C^C^C^C^C

Seems like I need to kill -9 the shell.

Contributor

theopolis commented Aug 3, 2017

How does the publisher shutdown?

sudo ./build/linux/osquery/osqueryi --nodisable_events --nodisable_audit --audit_allow_config --audit_persist --audit_allow_sockets --verbose

osquery> ^D

I0802 19:52:07.823665  1052 auditdnetlink.cpp:798] Uninstalling the audit rules we have installed
I0802 19:52:07.823678  1052 auditdnetlink.cpp:814] Restoring the default configuration for the audit service
    

^C^C^C^C^C

Seems like I need to kill -9 the shell.

@theopolis

This comment has been minimized.

Show comment
Hide comment
@theopolis

theopolis Aug 3, 2017

Contributor

I can't get the process_events table to work.

I used to be able to do:

sudo ./build/linux/osquery/osqueryi \
  --nodisable_events --nodisable_audit --audit_allow_config --audit_persist

From reading the code I'm guessing you now need:

 --audit_allow_process_events

But that's not working either?

Contributor

theopolis commented Aug 3, 2017

I can't get the process_events table to work.

I used to be able to do:

sudo ./build/linux/osquery/osqueryi \
  --nodisable_events --nodisable_audit --audit_allow_config --audit_persist

From reading the code I'm guessing you now need:

 --audit_allow_process_events

But that's not working either?

@theopolis

This comment has been minimized.

Show comment
Hide comment
@theopolis

theopolis Aug 3, 2017

Contributor

So if I try to initialize:

I0802 19:55:03.866938  1273 init.cpp:392] osquery initialized [version=2.6.0-21-g5692f91]
I0802 19:55:03.866977  1273 extensions.cpp:286] Could not autoload extensions: Failed reading: /etc/osquery/extensions.load
I0802 19:55:03.867067  1273 init.cpp:651] Error reading config: config file does not exist: /etc/osquery/osquery.conf
I0802 19:55:03.867076  1273 events.cpp:824] Event publisher failed setup: auditfim: Publisher disabled via configuration
I0802 19:55:03.867087  1275 interface.cpp:326] Extension manager service starting: /home/teddy/.osquery/shell.em
I0802 19:55:03.867091  1273 events.cpp:824] Event publisher failed setup: kernel: Cannot access /dev/osquery
I0802 19:55:03.867100  1273 events.cpp:824] Event publisher failed setup: syslog: Publisher disabled via configuration
I0802 19:55:03.867132  1273 events.cpp:1077] Error registering subscriber: auditd_fim_events: Subscriber disabled via configuration
I0802 19:55:03.870287  1279 events.cpp:745] Starting event publisher run loop: audit
I0802 19:55:03.870312  1281 events.cpp:745] Starting event publisher run loop: udev
I0802 19:55:03.870288  1280 events.cpp:745] Starting event publisher run loop: inotify

Are there two audit-related publishers? Ideally there should be just one publisher that publishes to multiple subscribers.

Contributor

theopolis commented Aug 3, 2017

So if I try to initialize:

I0802 19:55:03.866938  1273 init.cpp:392] osquery initialized [version=2.6.0-21-g5692f91]
I0802 19:55:03.866977  1273 extensions.cpp:286] Could not autoload extensions: Failed reading: /etc/osquery/extensions.load
I0802 19:55:03.867067  1273 init.cpp:651] Error reading config: config file does not exist: /etc/osquery/osquery.conf
I0802 19:55:03.867076  1273 events.cpp:824] Event publisher failed setup: auditfim: Publisher disabled via configuration
I0802 19:55:03.867087  1275 interface.cpp:326] Extension manager service starting: /home/teddy/.osquery/shell.em
I0802 19:55:03.867091  1273 events.cpp:824] Event publisher failed setup: kernel: Cannot access /dev/osquery
I0802 19:55:03.867100  1273 events.cpp:824] Event publisher failed setup: syslog: Publisher disabled via configuration
I0802 19:55:03.867132  1273 events.cpp:1077] Error registering subscriber: auditd_fim_events: Subscriber disabled via configuration
I0802 19:55:03.870287  1279 events.cpp:745] Starting event publisher run loop: audit
I0802 19:55:03.870312  1281 events.cpp:745] Starting event publisher run loop: udev
I0802 19:55:03.870288  1280 events.cpp:745] Starting event publisher run loop: inotify

Are there two audit-related publishers? Ideally there should be just one publisher that publishes to multiple subscribers.

@theopolis

First past comments, nothing crazy. Some of my previous thread comments are more high-pri.

Show outdated Hide outdated osquery/events/linux/audit.h
Show outdated Hide outdated osquery/events/linux/audit.h
Show outdated Hide outdated osquery/config/parsers/auditd_fim.cpp
Show outdated Hide outdated osquery/events/linux/auditdfim.h
Show outdated Hide outdated osquery/events/linux/auditdfim.h
Show outdated Hide outdated osquery/tables/events/linux/auditd_fim_events.cpp
Show outdated Hide outdated osquery/tables/events/linux/auditd_fim_events.cpp
Show outdated Hide outdated osquery/tables/events/linux/auditd_fim_events.cpp
Show outdated Hide outdated osquery/tables/events/linux/auditd_fim_events.cpp
Show outdated Hide outdated osquery/tables/events/linux/auditd_fim_events.cpp
@alessandrogario

This comment has been minimized.

Show comment
Hide comment
@alessandrogario

alessandrogario Aug 3, 2017

Contributor

The process_events table is working fine for me using the following flags:

osqueryd --audit_persist --disable_audit=false --audit_allow_config=true
    --disable_events=false --audit_allow_process_events=true

The reason that it may not work for you is that I have added a new flag to control whether it is enabled or not the same way socket_events works. The default setting is disabled.

EDIT:

I have also successfully tried with

osqueryi --nodisable_events --disable_audit=false --audit_allow_config
    --audit_persist --audit_allow_process_events --verbose
Contributor

alessandrogario commented Aug 3, 2017

The process_events table is working fine for me using the following flags:

osqueryd --audit_persist --disable_audit=false --audit_allow_config=true
    --disable_events=false --audit_allow_process_events=true

The reason that it may not work for you is that I have added a new flag to control whether it is enabled or not the same way socket_events works. The default setting is disabled.

EDIT:

I have also successfully tried with

osqueryi --nodisable_events --disable_audit=false --audit_allow_config
    --audit_persist --audit_allow_process_events --verbose
@alessandrogario

This comment has been minimized.

Show comment
Hide comment
@alessandrogario

alessandrogario Aug 3, 2017

Contributor

There is one AuditNetlink class fetching the events from the audit netlink and two publishers. The AuditNetlink class is generic and supports more than one consumer.

The first publisher (audit.cpp, used by socket_events and process_events) was not suitable for my use case for the following reasons:

  • It is not possible to listen for two or more syscalls accepting different (both in type and in quantity) arguments.
  • Each subscriber is required to state what kind (and how many) records it needs.
  • It does not make use of the AUDIT_EOE record terminator to understand if an event is terminated or not (this should be slower).
  • The algorithm that understands if an event is complete or not does not correctly support audit events that issue the same record type more than once.
  • It used to discard valid events, due to how audit event IDs were handled (fixed, see d075b79)
  • It used to leak unrequested syscalls to the subscribers in case audit_allow_config was not enabled (fixed, see ec57b07).
  • The filtering rules mix event types with event components (i.e.: AUDIT_SYSCALL is not only the first record in a multi-record sequence, but also the audit event type. The subsequent records, up until the next AUDIT_EOE, are just child records)

In order to avoid refactoring all the existing tables, a second publisher has been implemented.

The AuditdFim component is a lot simpler, as it doesn't need to specifically keep track of which records the subscriber expects to receive (the syscall number is enough) because it makes use of the AUDIT_EOE terminator. This should also be faster than completing them manually.

Contributor

alessandrogario commented Aug 3, 2017

There is one AuditNetlink class fetching the events from the audit netlink and two publishers. The AuditNetlink class is generic and supports more than one consumer.

The first publisher (audit.cpp, used by socket_events and process_events) was not suitable for my use case for the following reasons:

  • It is not possible to listen for two or more syscalls accepting different (both in type and in quantity) arguments.
  • Each subscriber is required to state what kind (and how many) records it needs.
  • It does not make use of the AUDIT_EOE record terminator to understand if an event is terminated or not (this should be slower).
  • The algorithm that understands if an event is complete or not does not correctly support audit events that issue the same record type more than once.
  • It used to discard valid events, due to how audit event IDs were handled (fixed, see d075b79)
  • It used to leak unrequested syscalls to the subscribers in case audit_allow_config was not enabled (fixed, see ec57b07).
  • The filtering rules mix event types with event components (i.e.: AUDIT_SYSCALL is not only the first record in a multi-record sequence, but also the audit event type. The subsequent records, up until the next AUDIT_EOE, are just child records)

In order to avoid refactoring all the existing tables, a second publisher has been implemented.

The AuditdFim component is a lot simpler, as it doesn't need to specifically keep track of which records the subscriber expects to receive (the syscall number is enough) because it makes use of the AUDIT_EOE terminator. This should also be faster than completing them manually.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot commented Aug 3, 2017

@alessandrogario updated the pull request - view changes

@osqueryer

This comment has been minimized.

Show comment
Hide comment
@osqueryer

osqueryer Aug 3, 2017

Collaborator

👎 The commit 5819a4b (Job results: 5432) failed one or more tests (Linux).

Collaborator

osqueryer commented Aug 3, 2017

👎 The commit 5819a4b (Job results: 5432) failed one or more tests (Linux).

@osqueryer

This comment has been minimized.

Show comment
Hide comment
@osqueryer

osqueryer Aug 3, 2017

Collaborator

👎 The commit 5819a4b (Job results: 5433) failed one or more tests (Linux).

Collaborator

osqueryer commented Aug 3, 2017

👎 The commit 5819a4b (Job results: 5433) failed one or more tests (Linux).

@alessandrogario alessandrogario changed the base branch from master to audit-redesign Aug 20, 2017

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot commented Aug 20, 2017

@alessandrogario updated the pull request - view changes

@osqueryer

This comment has been minimized.

Show comment
Hide comment
@osqueryer

osqueryer Aug 20, 2017

Collaborator

👎 The commit fc03850 (Job results: 2454) failed one or more tests (Windows).

Collaborator

osqueryer commented Aug 20, 2017

👎 The commit fc03850 (Job results: 2454) failed one or more tests (Windows).

@osqueryer

This comment has been minimized.

Show comment
Hide comment
@osqueryer

osqueryer Aug 20, 2017

Collaborator

👎 The commit fc03850 (Job results: 2455) failed one or more tests (Windows).

Collaborator

osqueryer commented Aug 20, 2017

👎 The commit fc03850 (Job results: 2455) failed one or more tests (Windows).

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot commented Aug 20, 2017

@alessandrogario updated the pull request - view changes

@osqueryer

This comment has been minimized.

Show comment
Hide comment
@osqueryer

osqueryer Aug 20, 2017

Collaborator

👎 The commit d19eecf (Job results: 2456) failed one or more tests (Windows).

Collaborator

osqueryer commented Aug 20, 2017

👎 The commit d19eecf (Job results: 2456) failed one or more tests (Windows).

@osqueryer

This comment has been minimized.

Show comment
Hide comment
@osqueryer

osqueryer Aug 20, 2017

Collaborator

👎 The commit d19eecf (Job results: 2457) failed one or more tests (Windows).

Collaborator

osqueryer commented Aug 20, 2017

👎 The commit d19eecf (Job results: 2457) failed one or more tests (Windows).

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot commented Aug 20, 2017

@alessandrogario updated the pull request - view changes

@osqueryer

This comment has been minimized.

Show comment
Hide comment
@osqueryer

osqueryer Aug 20, 2017

Collaborator

👎 The commit bb426e7 (Job results: 2458) failed one or more tests (Windows).

Collaborator

osqueryer commented Aug 20, 2017

👎 The commit bb426e7 (Job results: 2458) failed one or more tests (Windows).

@osqueryer

This comment has been minimized.

Show comment
Hide comment
@osqueryer

osqueryer Aug 20, 2017

Collaborator

👎 The commit bb426e7 (Job results: 2459) failed one or more tests (Windows).

Collaborator

osqueryer commented Aug 20, 2017

👎 The commit bb426e7 (Job results: 2459) failed one or more tests (Windows).

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot commented Aug 21, 2017

@alessandrogario updated the pull request - view changes

@theopolis theopolis merged commit 8685fc3 into facebook:audit-redesign Aug 24, 2017

5 checks passed

Code Audit Build finished.
Details
FreeBSD Build finished.
Details
Linux Build finished.
Details
Windows Build finished.
Details
macOS/OS X Build finished.
Details

alessandrogario added a commit to trailofbits/osquery-pr that referenced this pull request Nov 18, 2017

alessandrogario added a commit to trailofbits/osquery-pr that referenced this pull request Dec 11, 2017

alessandrogario added a commit to trailofbits/osquery-pr that referenced this pull request Dec 21, 2017

alessandrogario added a commit to trailofbits/osquery-pr that referenced this pull request Dec 21, 2017

alessandrogario added a commit to trailofbits/osquery-pr that referenced this pull request Dec 21, 2017

alessandrogario added a commit to trailofbits/osquery-pr that referenced this pull request Dec 21, 2017

theopolis added a commit to theopolis/osquery that referenced this pull request Jan 14, 2018

theopolis added a commit to theopolis/osquery that referenced this pull request Jan 14, 2018

theopolis added a commit to theopolis/osquery that referenced this pull request Jan 15, 2018

theopolis added a commit to theopolis/osquery that referenced this pull request Jan 16, 2018

@alessandrogario alessandrogario deleted the trailofbits:alessandro/feature/auditd_file_events branch Mar 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment