Skip to content

Commit

Permalink
detect: rename port whitelisting to priority
Browse files Browse the repository at this point in the history 
This was done following the fact that this setting was historically
named incorrectly. The purpose of the setting was always to define the
ports that will be prioritized and have rule groups associated w them on
priority. Rename all occurences of this to correctly reflect the purpose
of the setting.
  • Loading branch information
@inashivb @victorjulien
inashivb authored and victorjulien committed Oct 15, 2024
1 parent abbdeed commit 37fa2a6
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 49 deletions.
36 changes: 17 additions & 19 deletions src/detect-engine-build.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
#include "util-conf.h"

/* Magic numbers to make the rules of a certain order fall in the same group */
#define DETECT_PGSCORE_RULE_PORT_WHITELISTED 111 /* Rule port group contains a whitelisted port */
#define DETECT_PGSCORE_RULE_PORT_PRIORITIZED 111 /* Rule port group contains a priority port */
#define DETECT_PGSCORE_RULE_MPM_FAST_PATTERN 99 /* Rule contains an MPM fast pattern */
#define DETECT_PGSCORE_RULE_MPM_NEGATED 77 /* Rule contains a negated MPM */
#define DETECT_PGSCORE_RULE_NO_MPM 55 /* Rule does not contain MPM */
Expand Down Expand Up @@ -1064,15 +1064,14 @@ static int RulesGroupByIPProto(DetectEngineCtx *de_ctx)
return 0;
}

static int PortIsWhitelisted(const DetectEngineCtx *de_ctx,
const DetectPort *a, int ipproto)
static int PortIsPriority(const DetectEngineCtx *de_ctx, const DetectPort *a, int ipproto)
{
DetectPort *w = de_ctx->tcp_whitelist;
DetectPort *w = de_ctx->tcp_priorityports;
if (ipproto == IPPROTO_UDP)
w = de_ctx->udp_whitelist;
w = de_ctx->udp_priorityports;

while (w) {
/* Make sure the whitelist port falls in the port range of a */
/* Make sure the priority port falls in the port range of a */
DEBUG_VALIDATE_BUG_ON(a->port > a->port2);
if (a->port == w->port && w->port2 == a->port2) {
return 1;
Expand All @@ -1083,7 +1082,7 @@ static int PortIsWhitelisted(const DetectEngineCtx *de_ctx,
return 0;
}

static int RuleSetWhitelist(Signature *s)
static int RuleSetScore(Signature *s)
{
DetectPort *p = NULL;
if (s->flags & SIG_FLAG_TOSERVER)
Expand All @@ -1094,27 +1093,27 @@ static int RuleSetWhitelist(Signature *s)
return 0;

/* for sigs that don't use 'any' as port, see if we want to
* whitelist poor sigs */
* prioritize poor sigs */
int wl = 0;
if (!(p->port == 0 && p->port2 == 65535)) {
/* pure pcre, bytetest, etc rules */
if (RuleInspectsPayloadHasNoMpm(s)) {
SCLogDebug("Rule %u MPM has 1 byte fast_pattern. Whitelisting SGH's.", s->id);
SCLogDebug("Rule %u MPM has 1 byte fast_pattern. Prioritizing SGH's.", s->id);
wl = DETECT_PGSCORE_RULE_MPM_FAST_PATTERN;

} else if (RuleMpmIsNegated(s)) {
SCLogDebug("Rule %u MPM is negated. Whitelisting SGH's.", s->id);
SCLogDebug("Rule %u MPM is negated. Prioritizing SGH's.", s->id);
wl = DETECT_PGSCORE_RULE_MPM_NEGATED;

/* one byte pattern in packet/stream payloads */
} else if (s->init_data->mpm_sm != NULL &&
s->init_data->mpm_sm_list == DETECT_SM_LIST_PMATCH &&
RuleGetMpmPatternSize(s) == 1) {
SCLogDebug("Rule %u No MPM. Payload inspecting. Whitelisting SGH's.", s->id);
SCLogDebug("Rule %u No MPM. Payload inspecting. Prioritizing SGH's.", s->id);
wl = DETECT_PGSCORE_RULE_NO_MPM;

} else if (DetectFlagsSignatureNeedsSynOnlyPackets(s)) {
SCLogDebug("Rule %u Needs SYN, so inspected often. Whitelisting SGH's.", s->id);
SCLogDebug("Rule %u Needs SYN, so inspected often. Prioritizing SGH's.", s->id);
wl = DETECT_PGSCORE_RULE_SYN_ONLY;
}
}
Expand Down Expand Up @@ -1232,7 +1231,7 @@ static int CreateGroupedPortList(DetectEngineCtx *de_ctx, DetectPort *port_list,

/* insert the ports into the tmplist, where it will
* be sorted descending on 'cnt' and on whether a group
* is whitelisted. */
* is prioritized. */
tmplist = port_list;
SortGroupList(&groups, &tmplist, SortCompare);
uint32_t left = unique_groups;
Expand Down Expand Up @@ -1520,8 +1519,7 @@ static DetectPort *RulesGroupByPorts(DetectEngineCtx *de_ctx, uint8_t ipproto, u

int wl = s->init_data->score;
while (p) {
int pwl = PortIsWhitelisted(de_ctx, p, ipproto) ? DETECT_PGSCORE_RULE_PORT_WHITELISTED
: 0;
int pwl = PortIsPriority(de_ctx, p, ipproto) ? DETECT_PGSCORE_RULE_PORT_PRIORITIZED : 0;
pwl = MAX(wl,pwl);

DetectPort *lookup = DetectPortHashLookup(de_ctx, p);
Expand Down Expand Up @@ -1618,11 +1616,11 @@ static DetectPort *RulesGroupByPorts(DetectEngineCtx *de_ctx, uint8_t ipproto, u
}
#if 0
for (iter = list ; iter != NULL; iter = iter->next) {
SCLogInfo("PORT %u-%u %p (sgh=%s, whitelisted=%s/%d)",
SCLogInfo("PORT %u-%u %p (sgh=%s, prioritized=%s/%d)",
iter->port, iter->port2, iter->sh,
iter->flags & PORT_SIGGROUPHEAD_COPY ? "ref" : "own",
iter->sh->init->whitelist ? "true" : "false",
iter->sh->init->whitelist);
iter->sh->init->score ? "true" : "false",
iter->sh->init->score);
}
#endif
SCLogPerf("%s %s: %u port groups, %u unique SGH's, %u copies",
Expand Down Expand Up @@ -1787,7 +1785,7 @@ int SigPrepareStage1(DetectEngineCtx *de_ctx)
DetectContentPropagateLimits(s);
SigParseApplyDsizeToContent(s);

RuleSetWhitelist(s);
RuleSetScore(s);

/* if keyword engines are enabled in the config, handle them here */
if (!g_skip_prefilter && de_ctx->prefilter_setting == DETECT_PREFILTER_AUTO &&
Expand Down
60 changes: 35 additions & 25 deletions src/detect-engine.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2662,8 +2662,8 @@ void DetectEngineCtxFree(DetectEngineCtx *de_ctx)
#endif
}

DetectPortCleanupList(de_ctx, de_ctx->tcp_whitelist);
DetectPortCleanupList(de_ctx, de_ctx->udp_whitelist);
DetectPortCleanupList(de_ctx, de_ctx->tcp_priorityports);
DetectPortCleanupList(de_ctx, de_ctx->udp_priorityports);

DetectBufferTypeFreeDetectEngine(de_ctx);
SCClassConfDeinit(de_ctx);
Expand Down Expand Up @@ -2915,55 +2915,65 @@ static int DetectEngineCtxLoadConf(DetectEngineCtx *de_ctx)
}
}

/* parse port grouping whitelisting settings */
/* parse port grouping priority settings */

const char *ports = NULL;
(void)ConfGet("detect.grouping.tcp-whitelist", &ports);
(void)ConfGet("detect.grouping.tcp-priority-ports", &ports);
if (ports) {
SCLogConfig("grouping: tcp-whitelist %s", ports);
SCLogConfig("grouping: tcp-priority-ports %s", ports);
} else {
ports = "53, 80, 139, 443, 445, 1433, 3306, 3389, 6666, 6667, 8080";
SCLogConfig("grouping: tcp-whitelist (default) %s", ports);

(void)ConfGet("detect.grouping.tcp-whitelist", &ports);
if (ports) {
SCLogConfig(
"grouping: tcp-priority-ports from legacy 'tcp-whitelist' setting: %s", ports);
} else {
ports = "53, 80, 139, 443, 445, 1433, 3306, 3389, 6666, 6667, 8080";
SCLogConfig("grouping: tcp-priority-ports (default) %s", ports);
}
}
if (DetectPortParse(de_ctx, &de_ctx->tcp_whitelist, ports) != 0) {
if (DetectPortParse(de_ctx, &de_ctx->tcp_priorityports, ports) != 0) {
SCLogWarning("'%s' is not a valid value "
"for detect.grouping.tcp-whitelist",
"for detect.grouping.tcp-priority-ports",
ports);
}
DetectPort *x = de_ctx->tcp_whitelist;
DetectPort *x = de_ctx->tcp_priorityports;
for ( ; x != NULL; x = x->next) {
if (x->port != x->port2) {
SCLogWarning("'%s' is not a valid value "
"for detect.grouping.tcp-whitelist: only single ports allowed",
"for detect.grouping.tcp-priority-ports: only single ports allowed",
ports);
DetectPortCleanupList(de_ctx, de_ctx->tcp_whitelist);
de_ctx->tcp_whitelist = NULL;
DetectPortCleanupList(de_ctx, de_ctx->tcp_priorityports);
de_ctx->tcp_priorityports = NULL;
break;
}
}

ports = NULL;
(void)ConfGet("detect.grouping.udp-whitelist", &ports);
(void)ConfGet("detect.grouping.udp-priority-ports", &ports);
if (ports) {
SCLogConfig("grouping: udp-whitelist %s", ports);
SCLogConfig("grouping: udp-priority-ports %s", ports);
} else {
ports = "53, 135, 5060";
SCLogConfig("grouping: udp-whitelist (default) %s", ports);

(void)ConfGet("detect.grouping.udp-whitelist", &ports);
if (ports) {
SCLogConfig(
"grouping: udp-priority-ports from legacy 'udp-whitelist' setting: %s", ports);
} else {
ports = "53, 135, 5060";
SCLogConfig("grouping: udp-priority-ports (default) %s", ports);
}
}
if (DetectPortParse(de_ctx, &de_ctx->udp_whitelist, ports) != 0) {
if (DetectPortParse(de_ctx, &de_ctx->udp_priorityports, ports) != 0) {
SCLogWarning("'%s' is not a valid value "
"for detect.grouping.udp-whitelist",
"for detect.grouping.udp-priority-ports",
ports);
}
for (x = de_ctx->udp_whitelist; x != NULL; x = x->next) {
for (x = de_ctx->udp_priorityports; x != NULL; x = x->next) {
if (x->port != x->port2) {
SCLogWarning("'%s' is not a valid value "
"for detect.grouping.udp-whitelist: only single ports allowed",
"for detect.grouping.udp-priority-ports: only single ports allowed",
ports);
DetectPortCleanupList(de_ctx, de_ctx->udp_whitelist);
de_ctx->udp_whitelist = NULL;
DetectPortCleanupList(de_ctx, de_ctx->udp_priorityports);
de_ctx->udp_priorityports = NULL;
break;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/detect.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -974,8 +974,8 @@ typedef struct DetectEngineCtx_ {

HashListTable *dport_hash_table;

DetectPort *tcp_whitelist;
DetectPort *udp_whitelist;
DetectPort *tcp_priorityports;
DetectPort *udp_priorityports;

/** table for storing the string representation with the parsers result */
HashListTable *address_table;
Expand Down
6 changes: 3 additions & 3 deletions suricata.yaml.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1702,12 +1702,12 @@ detect:
default: mpm

# the grouping values above control how many groups are created per
# direction. Port whitelisting forces that port to get its own group.
# direction. Port priority setting forces that port to get its own group.
# Very common ports will benefit, as well as ports with many expensive
# rules.
grouping:
#tcp-whitelist: 53, 80, 139, 443, 445, 1433, 3306, 3389, 6666, 6667, 8080
#udp-whitelist: 53, 135, 5060
#tcp-priority-ports: 53, 80, 139, 443, 445, 1433, 3306, 3389, 6666, 6667, 8080
#udp-priority-ports: 53, 135, 5060

# Thresholding hash table settings.
thresholds:
Expand Down

0 comments on commit 37fa2a6

Please sign in to comment.