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.

Sign up for GitHub

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

Jump to bottom

Suricata improvements #330

Closed
wants to merge 5 commits into from
Closed

Suricata improvements #330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6111c3c
Fixed wildcard SID modification
securitym0nkey Mar 11, 2017
485d3d8
Fixed modified signatures missing the modify icon at the webgui
securitym0nkey Mar 11, 2017
8f50b71
Fixed rule modification for multiple comma separated SIDs
securitym0nkey Mar 11, 2017
9b0d73d
New feature for the SID Mgmt: rule content modification by category
securitym0nkey Mar 11, 2017
bf9b6f0
Improved regex for SID content mgmt
securitym0nkey Mar 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 64 additions & 34 deletions security/pfSense-pkg-suricata/files/usr/local/pkg/suricata/suricata.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1896,7 +1896,7 @@ function suricata_load_vrt_policy($policy, $mode='alert', $all_rules=null) {
return $vrt_policy_rules;
}

function suricata_parse_sidconf_file($sidconf_file) {
function suricata_parse_sidconf_file($sidconf_file,$split_lines=TRUE) {

/**********************************************/
/* This function loads and processes the file */
Expand All @@ -1908,6 +1908,11 @@ function suricata_parse_sidconf_file($sidconf_file) {
/* $sidconf_file ==> full path and name of */
/* file to process */
/* */
/* $split_lines ==> determines whether lines */
/* should be split at */
/* commas into multiple */
/* sid modification */
/* */
/* Returns ==> an array containing */
/* SID modifier tokens */
/**********************************************/
Expand Down Expand Up @@ -1941,11 +1946,17 @@ function suricata_parse_sidconf_file($sidconf_file) {
// Trim leading and trailing spaces plus newline and any carriage returns
$buf = trim($line[0], ' \r\n');

// Now split the SID mod arguments at the commas, if more than one
// per line, and add to our $sid_mods array.
$line = explode(",", $buf);
foreach ($line as $ent) {
$sid_mods[] = trim($ent);
if ($split_lines) {
// If split mode split the SID mod arguments at the commas, if more than one
// per line, and add to our $sid_mods array.
$line = explode(",", $buf);
foreach ($line as $ent) {
$sid_mods[] = trim($ent);
}
}
else{
//Otherwise add 1 line as 1 modification to the $sid_mods array
$sid_mods[] = $buf;
}
}

Expand Down Expand Up @@ -2500,38 +2511,57 @@ function suricata_modify_sid_content(&$rule_map, $sid_mods, $log_results = FALSE
// Walk the SID mod tokens and decode each one
foreach ($sid_mods as $tok) {
$matches = array();
if (preg_match('/([\d+|,|\*]*)\s+"(.+)"\s+"(.*)"/', $tok, $matches)) {
if (preg_match('/((?<sid_category_list> (?<sid_category_plus_comma>[\da-z-_*]+,)* (?<last_sid_cageory>[\da-z-_*]+)) \s+)? "(?<from>.*)" \s+ "(?<to>.*)"/x', $tok, $matches)) {
// If a valid config token is found. A valid token is:
// An optional group of a comma separated list(sid_category_list) of SIDs(numbers) or categories(lowercase chars,numbers,- or _) plus whitespaces followed by
// a search term(from) framed by double quotes followed
// whitespaces followed by
// a replacement(to) framed by double quotes

$tokencounter++;
$sidlist = explode(",", $matches[1]);
$from = '/' . preg_quote($matches[2], '/') . '/';
$to = $matches[3];
$from = '/' . preg_quote($matches['from'], '/') . '/';
$to = $matches['to'];
$count = 0;

// Now walk the provided rule map and make the modifications
if ($matches[1] == "*") {
// If wildcard '*' provided for SID, then check them all
foreach ($rule_map[1] as $rulem) {
foreach ($rulem as $k2 => $v) {
$modcount++;
$rule_map[1][$k2]['rule'] = preg_replace($from, $to, $v['rule'], -1, $count);
if ($count > 0) {
$rule_map[1][$k2]['managed'] = 1;
$sids[1][$k2] = 'modify';
$modifiedcount++;
}
}
}
if ($matches['sid_category_list'] == "") {
// If the sidlist is empty it's the same as a wildcard, therefore create a list with just one wildcard item
$sidlist = array("*");
}
else {
// Otherwise just check the provided SIDs
foreach ($sidlist as $sid) {
if (isset($rule_map[1][$sid])) {
$modcount++;
$rule_map[1][$sid]['rule'] = preg_replace($from, $to, $rule_map[1][$sid]['rule'], -1, $count);
if ($count > 0) {
$rule_map[1][$sid]['managed'] = 1;
$sids[1][$sid] = 'modify';
$modifiedcount++;
// Otherwise split the list
$sidlist = explode(",", $matches['sid_category_list']);
}


// Walk over all sids
foreach ($sidlist as $sid) {
if (isset($rule_map[1][$sid])) {
// If sid is present change modify it
$modcount++;
$rule_map[1][$sid]['rule'] = preg_replace($from, $to, $rule_map[1][$sid]['rule'], -1, $count);
if ($count > 0) {
$rule_map[1][$sid]['managed'] = 1;
$rule_map[1][$sid]['modified'] = 1;
$sids[1][$sid] = 'modify';
$modifiedcount++;
}
}
else {
// Otherwise it's a wildcard or category modification
if (is_array($rule_map)) {
// If rule_map is an array walk over it to find all rules which need be be modified
foreach ($rule_map[1] as $k2 => $v) {
if ($sid == "*" || $v['category'] == $sid) {
// If a wildcard changed is handled or the category is matching the rule needs to get modified
$modcount++;
$rule_map[1][$k2]['rule'] = preg_replace($from, $to, $v['rule'], -1, $count);
if ($count > 0) {
$rule_map[1][$k2]['managed'] = 1;
$rule_map[1][$k2]['modified'] = 1;
$sids[1][$k2] = 'modify';
$modifiedcount++;
}
}
}
}
}
Expand Down Expand Up @@ -2685,7 +2715,7 @@ function suricata_process_modifysid(&$rule_map, $suricatacfg, $log_results = FAL
log_error(gettext("[Suricata] Error - unable to open 'modify_sid_file' \"{$suricatacfg['modify_sid_file']}\" specified for " . convert_friendly_interface_to_friendly_descr($suricatacfg['interface'])));
return;
} else {
$sid_mods = suricata_parse_sidconf_file("{$suricata_sidmods_dir}{$suricatacfg['modify_sid_file']}");
$sid_mods = suricata_parse_sidconf_file("{$suricata_sidmods_dir}{$suricatacfg['modify_sid_file']}",FALSE);
}

if (!empty($sid_mods)) {
Expand Down
12 changes: 10 additions & 2 deletions security/pfSense-pkg-suricata/files/var/db/suricata/sidmods/modifysid-sample.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# example modifysid.conf
#
# formatting is simple
# <sid or sid list> "what I'm replacing" "what I'm replacing it with"
# <sid, category, list of sids&categories> "what I'm replacing" "what I'm replacing it with"
#
# Note that this will only work with GID:1 rules, simply because modifying
# GID:3 SO stub rules would not actually affect the rule.
Expand All @@ -19,5 +19,13 @@
# "HTTP_PORTS" "HTTPS_PORTS"

# multiple sids can be specified as noted below:
# 302,429,1821 "\$EXTERNAL_NET" "\$HOME_NET"
# 302,429,1821 "$EXTERNAL_NET" "$HOME_NET"
Copy link
Contributor

@doktornotor doktornotor Mar 12, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Pretty sure these should remain escaped as it was before, and any added example should have those escaped, otherwise it's never going to match things in the rulesets. IOW, this it to replace the string $EXTERNAL_NET with a string $HOME_NET for the matching SIDs, not to expand those things to whatever is defined there and stick that in the rules.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code is escaping it itself. See line 2506. Actually that way you can't use regex for replacement. pulledpork supports replacement by regex in such case you need to escape it. I thought of removing the preg_quote so that the current example would be correct. But dropped that idea to no break the config for people who have noticed that escaping is not needed as displayed in the example.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, currently the code performs internal escaping of the regex.


# modify all signatures in a category. Example: replace "$EXTERNAL_NETS" with "any" to be alerts on insider threats as well
# emerging-scan "$EXTERNAL_NET" "any"

# modify all signatures in multiple categories
# emerging-scan,emerging-sql "$EXTERNAL_NET" "any"

# modify all signatures for a category and specific SIDs from other categories
# emerging-sql,2100691,2009817 "$EXTERNAL_NET" "any"
Copy link
Contributor

Choose a reason for hiding this comment

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

See previous comment.