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

Port the extended_attributes table to Linux, add support for Linux capabilities #6195

Conversation

alessandrogario
Copy link
Member

@alessandrogario alessandrogario commented Jan 22, 2020

This is a port of the following PR from 3.x to 4.x: #4505

osquery> SELECT * FROM extended_attributes WHERE path = "/home/alessandro/test_binary";
+------------------------------+------------------+-------------------------+--------------------------------------------------------------+--------+
| path                         | directory        | key                     | value                                                        | base64 |
+------------------------------+------------------+-------------------------+--------------------------------------------------------------+--------+
| /home/alessandro/test_binary | /home/alessandro | security.capability     | = cap_dac_override,cap_net_admin,cap_net_raw+eip             | 0      |
| /home/alessandro/test_binary | /home/alessandro | user.test_attribute2    | Hello from Trail of Bits!                                    | 0      |
| /home/alessandro/test_binary | /home/alessandro | user.test_attribute     | aGVsbG8sIG9zcXVlcnkh                                         | 1      |
+------------------------------+------------------+-------------------------+--------------------------------------------------------------+--------+

@alessandrogario alessandrogario force-pushed the alessandro/feature/linux-extended-attributes branch 19 times, most recently from ff8e8d3 to 3a68b0e Compare January 25, 2020 00:35
@alessandrogario alessandrogario marked this pull request as ready for review January 25, 2020 11:43
@alessandrogario alessandrogario force-pushed the alessandro/feature/linux-extended-attributes branch 3 times, most recently from 4cb2859 to 6d4c33a Compare January 29, 2020 23:39
@theopolis
Copy link
Member

I have a suggestion for an alternate approach to managing source-based libraries in Buck. What do you think of changing the documentation such that Buck open source users first run CMake to setup the source checkouts and patching, then we update the expected paths in Buck to look for a ./build folder or ./libraries/cmake/source. This avoids duplicating generated code and avoids duplicating submodule configurations.

@mike-myers-tob
Copy link
Member

I have a suggestion for an alternate approach to managing source-based libraries in Buck. What do you think of changing the documentation such that Buck open source users first run CMake to setup the source checkouts and patching, then we update the expected paths in Buck to look for a ./build folder or ./libraries/cmake/source. This avoids duplicating generated code and avoids duplicating submodule configurations.

Buck has since been removed from this version of osquery. How can we take a next step on this PR?

@theopolis
Copy link
Member

The build becomes much easier since we are nearly 100% using the source layer in CMake.

@theopolis
Copy link
Member

Ping @alessandrogario, are we still interested in merging these features? Is there a strong desire for this data?

@mike-myers-tob
Copy link
Member

Ping @alessandrogario, are we still interested in merging these features? Is there a strong desire for this data?

We did write this for one of our clients and would still like to see it merged. I was asking earlier about the next step. It looks like it hasn't been reviewed, but there was a Buck-related suggestion that is I assume no longer applicable. If we rebased this PR and removed the Buck files, would it be eligible for a PR review at that point?

@directionless
Copy link
Member

If we rebased this PR and removed the Buck files, would it be eligible for a PR review at that point?

I am broadly interested in this work. The PR does need a refresh though. (sorry)

@alessandrogario alessandrogario force-pushed the alessandro/feature/linux-extended-attributes branch from 6d4c33a to 881901c Compare July 28, 2020 20:03
@mike-myers-tob
Copy link
Member

If we rebased this PR and removed the Buck files, would it be eligible for a PR review at that point?

I am broadly interested in this work. The PR does need a refresh though. (sorry)

I believe the PR is refreshed and it was just not communicated well. If someone can review/approve, I think this PR is ready.

@alessandrogario alessandrogario force-pushed the alessandro/feature/linux-extended-attributes branch 3 times, most recently from 9835aec to e136773 Compare August 18, 2020 16:57
@amotmot
Copy link

amotmot commented Sep 26, 2020

+1 on this feature. Of note, file capabilities and process capabilities can be different as a set of capabilities can be preserved across an execve. For this reason, I analyze process capabilities so that inheritable capabilities (CapInh) are collected by osquery.
For example ...

cat /proc/$pid/status
[...]
CapInh:	0000000000000000
CapPrm:	0000000000000000
CapEff:	0000000000000000
CapBnd:	0000003ff7fdffff
CapAmb:	0000000000000000
NoNewPrivs:	1
Seccomp:	2

An approach the parses out /proc table will also allow include NoNewPrivs as well as Seccomp process attributes. Perhaps, we can extend https://osquery.io/schema/4.5.0/#processes table to include additional attributes? Could someone point me to the https://osquery.io/schema/4.5.0/#processes Linux C/C++ code?

Here's an example of a osquery-go-sdk extension ...

osquery> SELECT name, cmdline, seccomp, nonewprivs, capeff, capamb FROM process_security;
+-----------------+----------------------------------------------+---------+------------+------------------+------------------+
| name            | cmdline                                      | seccomp | nonewprivs | capeff           | capamb           |
+-----------------+----------------------------------------------+---------+------------+------------------+------------------+
| dns | dns             | 2       | 1          | 0000000000000000 | 0000000000000000 |
| edgeworker | /usr/local/bin/worker-debug              | 2       | 1          | 0000000000000000 | 0000000000000000 |

The PR LGTM aside from renaming extended_attributes to file_capabilties.

@theopolis
Copy link
Member

I like the idea of another processes_ table for Linux security attributes vs further extending the processes table, but that is not a strong opinion.

One user experience I like is having processes be as cross platform as possible, then each OS’s specific processes-related data shows up in nuanced and specific additional tables. In this scenario the overhead of joining those tables together should be minimal.

@directionless
Copy link
Member

A downside to using two joined tables, is you can get inconsistencies for rapidly changing data. (This is potentially an issue with processes, we've had customers bump into problems with other vendor solutions joining processes to open sockets)

@alessandrogario alessandrogario force-pushed the alessandro/feature/linux-extended-attributes branch from e136773 to 1459e63 Compare October 9, 2020 12:23
@alessandrogario alessandrogario added the ready for review Pull requests that are ready to be reviewed by a maintainer label Oct 9, 2020
@alessandrogario alessandrogario force-pushed the alessandro/feature/linux-extended-attributes branch from 1459e63 to 32430fe Compare October 28, 2020 14:12
@alessandrogario alessandrogario force-pushed the alessandro/feature/linux-extended-attributes branch from 32430fe to 2207e72 Compare November 19, 2020 18:37
@Smjert Smjert closed this Nov 22, 2020
@Smjert Smjert reopened this Nov 22, 2020
theopolis
theopolis previously approved these changes Nov 23, 2020
libraries/cmake/source/libcap/CMakeLists.txt Outdated Show resolved Hide resolved
libraries/cmake/source/modules/Findlibcap.cmake Outdated Show resolved Hide resolved
osquery/filesystem/posix/xattrs.cpp Outdated Show resolved Hide resolved
osquery/filesystem/tests/posix/xattrs.cpp Outdated Show resolved Hide resolved
osquery/tables/system/linux/extended_attributes.cpp Outdated Show resolved Hide resolved
@mike-myers-tob mike-myers-tob dismissed directionless’s stale review November 24, 2020 20:05

Those changes are implemented now

@mike-myers-tob mike-myers-tob merged commit 76c7733 into osquery:master Nov 24, 2020
@mike-myers-tob mike-myers-tob deleted the alessandro/feature/linux-extended-attributes branch November 24, 2020 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Linux ready for review Pull requests that are ready to be reviewed by a maintainer virtual tables
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants