Migration from shelling out to osquery to using Thrift #34
Conversation
Create one instance of osqueryi child process while app is running, run queries over thrift
…ell to true (debug)
…wning. Reduced timeout on making thrift connection
sources/osquery_thrift.js
Outdated
| @@ -9,6 +12,11 @@ const osqueryPlatforms = { | |||
| linux: '../bin/osqueryi_linux' | |||
| } | |||
|
|
|||
| const socketPath = { | |||
| darwin: `/Users/${process.env.USER}/.osquery`, | |||
There was a problem hiding this comment.
Should/could this be in appData instead?
There was a problem hiding this comment.
It could and probably should, I went with using the default socket locations that osquery picks. I think I just need to run some tests to ensure that the provided socket path is reliably used.
sources/osquery_thrift.js
Outdated
| }) | ||
|
|
||
| setTimeout(() => { | ||
| this.connection = ThriftClient.getInstance({ path: socket }) |
There was a problem hiding this comment.
Can we run some kind of super fast "heartbeat" query at this stage, and retry on failure? Then we're at least polling to see if it's up instead of trying to guess a magic number that will work.
I suppose normal plugins don't have this problem because the load order is reversed: osquery launches them.
There was a problem hiding this comment.
Definitely, that would be a much more ideal setup. I tried something like that at first, but I haven't found any real documentation on the NodeJS API produced by thrift. This page exists but all of the links are broken 👎 . I ran this command to generate the thrift lib and have figured out some of the API by reviewing the source:
thrift -r --gen js:node /path/to/osquery/osquery.thriftI'll dive deeper into the thrift libraries and see what I can do in terms of polling and catching errors.
sources/osquery_thrift.js
Outdated
| this.connection.end() | ||
| } | ||
| log.error(`osquery execution error: ${err}`) | ||
| reject(new Error(`Unable to spawn osquery: ${err}`)) |
There was a problem hiding this comment.
Is this the only error case? Is it possible to crash osqueryi with a bad query, in which case we should try to bring it back up?
There was a problem hiding this comment.
From my tests so far, this is the only error case. The bad queries I've tested catch and report syntax errors and don't kill the osqueryi process.
osquery> select $%^&*( from bobby_tables;
Error: unrecognized token: "$"
osquery> i can haz segfault?;
Error: near "i": syntax error
osquery>I think it does make sense to cover ourselves should osquery crash for an unexpected reason
src/lib/ThriftClient.js
Outdated
| } | ||
|
|
||
| constructor(opts = { path: '/Users/' + process.env.USER + '/.osquery/shell.em'}) { | ||
| const path = opts.path || '/Users/' + process.env.USER + '/.osquery/shell.em' |
There was a problem hiding this comment.
Shouldn't have to do this since you've got the default in the arguments.
Also, same comment as above: should this be in appData?
There was a problem hiding this comment.
Good catch, this is a remnant from an earlier setup where I wasn't manually setting the socket path. Will remove this
|
Created #38 to track this issue |
…to ensure osquery connectivity is present, fixed issue with error handling, ensure env vars are set for all platforms, added powershell cache
This represents a very large change in the way the application works and I have some concerns about rolling it out.
TLDR; version of the changes:
OLD make shell calls to
osqueryibased on what data was requested in the API (e.g.runs:
NEW start one child process when the app launches:
# nodisable_extensions flag opens a thrift socket osqueryi --nodisable_extensionsUse
thriftprotocol to send all queries.Problems
setTimeout:vomitinmouth: :biohazard_sign: to make the thrift socket connection. I'm occasionally seeing this error:OSQUERY ERROR Error: Thrift: Fri Jun 8 12:13:27 2018 TSocket::open() connect() <Host: Port: 0>Connection refused, it doesn't seem to break anything on Mac but I have seen issues on Windows.Actions
There are some serious performance issues with
master(especially on Windows) and this branch fixes those issues. I would really appreciate any and all feedback on app performance/code quality/approach/etc.