Add `sigrules` to pass YARA signature with query #6568

kumarak · 2020-07-27T16:01:10Z

The PR adds a new column sigrules that can be used to pass a small set of YARA signature with the query.

osquery> select * from yara where path = '/etc/passwd' and sigrules = 'rule always_true { condition: true }';
+-------------+-------------+-------+-----------+---------+--------------------------------------+---------+------+
| path        | matches     | count | sig_group | sigfile | sigrules                             | strings | tags |
+-------------+-------------+-------+-----------+---------+--------------------------------------+---------+------+
| /etc/passwd | always_true | 1     |           |         | rule always_true { condition: true } |         |      |
+-------------+-------------+-------+-----------+---------+--------------------------------------+---------+------+

directionless

(nits)

tests/integration/tables/yara.cpp

specs/yara/yara.table

tests/integration/tables/yara.cpp

osquery/tables/yara/yara.cpp

osquery/tables/yara/yara_utils.cpp

tests/integration/tables/yara.cpp

theopolis · 2020-07-29T02:10:47Z

osquery/tables/yara/yara.cpp

+        continue;
+      }
+
+      rules[rule_string] = tmp_rules;


I feel like this caching could be dangerous, strings can be extremely large and people might run arbitrary queries via the distributed APIs. I suggest that we hash the rule string before adding it to the map here, add a known prefix, and that we clear these and free the memory after the scanning occurs.

Thanks, @theopolis for the review. I updated it to hash the rule string before adding to the cache and remove it after the scan to reduce/avoid the possibility of collision in case of multiple queries raised by distributed APIs. This is similar to not adding the sigrule string to the cache. Please let me know your thoughts.

theopolis · 2020-07-31T02:56:36Z

I thought more about this and I'm worried the feature could be abused to read arbitrary content from files. The yara table can do this now but it requires someone to write the signature file to disk first. My recommendation to accommodate for this we should default-disable this feature with a configuration option.

Edit: Ignore the next section, it seems too clunky

Additionally, I am curious if the rule compilation and usage can be accomplished with a function instead of a column. For example could one write a query like:

select * from yara where path = apply_signature('/etc/passwd', 'rule always_true { condition: true }');

Then it's easier to disable/enable the apply_signature function via a flag.

directionless · 2020-08-01T12:14:55Z

I thought more about this and I'm worried the feature could be abused to read arbitrary content from files.

Hrm. I was under the impression yara rules returned a boolean? Is there a way we can constrain the results to minimize the data leakage?

theopolis · 2020-08-02T15:44:27Z

From my understanding the strings column will include matched strings.

kumarak · 2020-08-11T17:44:32Z

@theopolis, I added an osquery flag enable_yara_table_extension to enable passing the signature rules with the query. Could you let me know your feedback? Also, if you think of other approaches to enable/disable the feature (maybe through configuration file) please let me know.

I have not explored the possibility of using the function to enable this feature yet. I will check how to use functions with the query and if it will be a convenient approach.

directionless · 2020-08-19T01:28:37Z

We talked a bit about this in office hours. A suggestion I made, which submitted for consideration and seemed to be supported, is to create 2 flag. One to enable the this feature. And another to include whether or not to populate the strings column at all.

I think there's clear value in this, and in #6558, but I also think the data exfiltration is real. So thinking about how to allow the functionality, while not allowing easy extraction.

directionless · 2020-08-19T02:26:54Z

osquery/tables/yara/yara.cpp

 FLAG(uint32,
     yara_delay,
     50,
     "Time in ms to sleep after scan of each file (default 50) to reduce "
     "memory spikes");

+FLAG(bool,
+     enable_yara_table_extension,


In osquery parlance, this is not an extension. Maybe enable_yara_sigrule?

Though probably should also gain enable_yara_string_return

I updated the configuration flag to enable_yara_sigrule and the rule strings are set to private by default. The private strings does not show-up in the yara table. disable_yara_string_private flag can be used to disable making strings to private.

osquery/tables/yara/yara.cpp

directionless · 2020-08-19T02:37:55Z

osquery/tables/yara/yara.cpp

+    // Check if this "ad-hoc" signature file has not been used/compiled.
+    if (rules.count(hashStr(file, YC_FILE)) == 0) {


This is cached based on filename, right? Short of restarting osquery, is there a way to clear that cache? I don't think there needs to be, but the limitation might need documenting.

The rule cache based on filename does not get cleanup. It avoids recompilation of file if they are already compiled.

directionless · 2020-08-19T02:41:20Z

osquery/tables/yara/yara.cpp

+  auto groups = context.constraints["sig_group"].getAll(EQUALS);
+  auto sigfiles = context.constraints["sigfile"].getAll(EQUALS);
+  auto sigrules = context.constraints["sigrule"].getAll(EQUALS);
+  if (groups.empty() && sigfiles.empty() && sigrules.empty()) {


Feels odd. These are checked here, but also then checked in the genYaraRuleFrom... routines. Do you think it's cleaner to call the genYaraRuleFrom... routines, and then check if scanContext is gt 0?

(This is not a leading question, I don't know what feels cleaner)

I updated the checks and it will get the rules compiled and add them to the scanContext. It checks if the scanContext is empty and return empty result.

kumarak · 2020-08-21T08:49:48Z

Thanks, @directionless for comments. I added two flags: enable_yara_sigrule and disable_yara_string_private.

If a query is generated with sigrule, the rule strings are set as private by default. The private strings does not show-up in the yara table. To disable the feature and show the string column disable_yara_string_private flags can be used.

osquery/tables/yara/yara_utils.cpp

theopolis · 2020-08-25T03:12:46Z

osquery/tables/yara/yara.cpp

+static std::shared_ptr<YARAConfigParserPlugin> getYaraParser(void) {
+  auto parser = Config::getParser("yara");
+  if (isNull(parser)) {
+    LOG(ERROR) << "YARA config parser plugin has no pointer";


"YARA config parser plugin not found"

osquery/tables/yara/yara.cpp

theopolis · 2020-08-25T03:17:07Z

osquery/tables/yara/yara.cpp

+
+Status genYaraRuleFromFile(QueryContext& queryContext,
+                           YaraScanContext& scanContext) {
+  auto yaraParser = getYaraParser();


The method getYaraParser may print an error then this function will "bubble up" a similar error but with a loss of precision. We should check that we do not unintentionally emit the same (or similar) error twice. For example, this status is passed up to genYara, do we end up printing a status message there too?

I suggest we avoid writing status messages in getYaraParser and allow the top-most calling method to choose to print.

I removed the status log printing from the top-most calling function and let getYaraParser print the error logs. These logs are more useful to debug the specific issues. The top-most function returns if the parser is null.

osquery/tables/yara/yara_utils.cpp

osquery/tables/yara/yara.cpp

theopolis · 2020-08-26T02:27:32Z

Thanks for the update! I'll take a final look at this tomorrow :)

osquery/tables/yara/yara_utils.cpp

kumarak · 2020-08-26T05:04:31Z

osquery/tables/yara/yara_utils.cpp

+/**
+ * Compile yara rules from string and load it into rule pointer.
+ */
+Status compileFromString(const std::string& rule_defs, YR_RULES** rules) {


The function compileFromString is similar to compileSingleFile and there are common code creating the yara compiler and error handling. I kept it as separate function to have different path for compiling the signature files and signature strings. Please let me know your thoughts.

directionless reviewed Jul 28, 2020

View reviewed changes

tests/integration/tables/yara.cpp Outdated Show resolved Hide resolved

specs/yara/yara.table Outdated Show resolved Hide resolved

tests/integration/tables/yara.cpp Outdated Show resolved Hide resolved

osquery/tables/yara/yara.cpp Outdated Show resolved Hide resolved

theopolis requested changes Jul 29, 2020

View reviewed changes

theopolis added the feature label Jul 29, 2020

kumarak force-pushed the kumarak/yara_table_extension branch from f4a96c3 to 8dbcba4 Compare July 30, 2020 23:56

kumarak force-pushed the kumarak/yara_table_extension branch from 8dbcba4 to 27ced24 Compare August 10, 2020 19:28

kumarak requested a review from theopolis August 11, 2020 17:44

kumarak force-pushed the kumarak/yara_table_extension branch from f2670c4 to e5463b2 Compare August 18, 2020 14:42

directionless mentioned this pull request Aug 19, 2020

Flexibility for yara table configuration and/or extensibility for other security scanners #6558

Closed

directionless reviewed Aug 19, 2020

View reviewed changes

kumarak added 7 commits August 21, 2020 04:01

Add sigrules column to pass yara rules with queries

371245d

update integration test

a67d2b8

hash rule string before adding to cache

40a9aa3

add config to enable the yara table extension

fe721b5

update yara extension config

4005eed

Update sigrule string hashing and function name

20c1c6b

Update flags to enable sigrule and make the string private by default

3c2ed27

kumarak force-pushed the kumarak/yara_table_extension branch from 46da858 to 3c2ed27 Compare August 21, 2020 08:02

theopolis requested changes Aug 25, 2020

View reviewed changes

Update config flag, refactoring and address review comments

01d677a

kumarak commented Aug 26, 2020

View reviewed changes

osquery/tables/yara/yara_utils.cpp Show resolved Hide resolved

remove enable_yara_sigrule flag

8c30bf2

kumarak commented Aug 26, 2020

View reviewed changes

Updated comments

9a645dc

kumarak requested a review from theopolis August 26, 2020 19:23

theopolis approved these changes Aug 29, 2020

View reviewed changes

theopolis merged commit d86e90c into osquery:master Aug 29, 2020

nabilschear pushed a commit to nabilschear/osquery that referenced this pull request Aug 31, 2020

Add sigrules to pass YARA signature with query (osquery#6568)

60c7064

kumarak deleted the kumarak/yara_table_extension branch September 30, 2020 01:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `sigrules` to pass YARA signature with query #6568

Add `sigrules` to pass YARA signature with query #6568

kumarak commented Jul 27, 2020

directionless left a comment

theopolis Jul 29, 2020

kumarak Jul 29, 2020

theopolis commented Jul 31, 2020 •

edited

directionless commented Aug 1, 2020

theopolis commented Aug 2, 2020

kumarak commented Aug 11, 2020

directionless commented Aug 19, 2020

directionless Aug 19, 2020

kumarak Aug 21, 2020

directionless Aug 19, 2020

kumarak Aug 21, 2020

directionless Aug 19, 2020

kumarak Aug 21, 2020

kumarak commented Aug 21, 2020

theopolis Aug 25, 2020

theopolis Aug 25, 2020

kumarak Aug 26, 2020

theopolis commented Aug 26, 2020

kumarak Aug 26, 2020

		// Check if this "ad-hoc" signature file has not been used/compiled.
		if (rules.count(hashStr(file, YC_FILE)) == 0) {

Add sigrules to pass YARA signature with query #6568

Add sigrules to pass YARA signature with query #6568

Conversation

kumarak commented Jul 27, 2020

directionless left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

theopolis commented Jul 31, 2020 • edited

directionless commented Aug 1, 2020

theopolis commented Aug 2, 2020

kumarak commented Aug 11, 2020

directionless commented Aug 19, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kumarak commented Aug 21, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

theopolis commented Aug 26, 2020

Choose a reason for hiding this comment

Add `sigrules` to pass YARA signature with query #6568

Add `sigrules` to pass YARA signature with query #6568

theopolis commented Jul 31, 2020 •

edited