aiden
/
suricata
mirror of https://github.com/OISF/suricata
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

detect: unify alert handling; fix bugs

Browse Source 
Unify handling of signature matches between various rule types and
between noalert and regular rules.

"noalert" sigs are added to the alert queue initially, but removed
from it after handling their actions. This way all actions are applied
from a single place.

Make sure flow drop and pass are mutually exclusive.

The above addresses issue with pass and drops not getting applied
correctly in various cases.

Bug: #4663
Bug: #4670
pull/6383/head
Victor Julien 4 years ago
parent ae89874b06
commit aa93984b7e
7 changed files with 70 additions and 94 deletions
Show Stats Download Patch File Download Diff File

6
src/decode.h
Unescape Escape View File

@ -282,10 +282,8 @@ typedef struct PacketAlert_ {
uint64_t tx_id;
} PacketAlert;
/** After processing an alert by the thresholding module, if at
* last it gets triggered, we might want to stick the drop action to
* the flow on IPS mode */
#define PACKET_ALERT_FLAG_DROP_FLOW 0x01
/* flag to indicate the rule action (drop/pass) needs to be applied to the flow */
#define PACKET_ALERT_FLAG_APPLY_ACTION_TO_FLOW 0x1
/** alert was generated based on state */
#define PACKET_ALERT_FLAG_STATE_MATCH 0x02
/** alert was generated based on stream */

78
src/detect-engine-alert.c
Unescape Escape View File

@ -228,14 +228,47 @@ int PacketAlertAppend(DetectEngineThreadCtx *det_ctx, const Signature *s,
static inline void RuleActionToFlow(const uint8_t action, Flow *f)
{
if (action & ACTION_DROP)
f->flags |= FLOW_ACTION_DROP;
if (action & ACTION_REJECT_ANY)
f->flags |= FLOW_ACTION_DROP;
if (action & (ACTION_DROP | ACTION_REJECT_ANY | ACTION_PASS)) {
if (f->flags & (FLOW_ACTION_DROP | FLOW_ACTION_PASS)) {
/* drop or pass already set. First to set wins. */
SCLogDebug("not setting %s flow already set to %s",
(action & ACTION_PASS) ? "pass" : "drop",
(f->flags & FLOW_ACTION_DROP) ? "drop" : "pass");
} else {
if (action & (ACTION_DROP | ACTION_REJECT_ANY)) {
f->flags |= FLOW_ACTION_DROP;
SCLogDebug("setting flow action drop");
}
if (action & ACTION_PASS) {
f->flags |= FLOW_ACTION_PASS;
SCLogDebug("setting flow action pass");
FlowSetNoPacketInspectionFlag(f);
}
}
}
}
if (action & ACTION_PASS) {
FlowSetNoPacketInspectionFlag(f);
/** \brief Apply action(s) and Set 'drop' sig info,
* if applicable */
static void PacketApplySignatureActions(Packet *p, const Signature *s, const uint8_t alert_flags)
{
SCLogDebug("packet %" PRIu64 " sid %u action %02x alert_flags %02x", p->pcap_cnt, s->id,
s->action, alert_flags);
PacketUpdateAction(p, s->action);
if (s->action & ACTION_DROP) {
if (p->alerts.drop.action == 0) {
p->alerts.drop.num = s->num;
p->alerts.drop.action = s->action;
p->alerts.drop.s = (Signature *)s;
}
if ((p->flow != NULL) && (alert_flags & PACKET_ALERT_FLAG_APPLY_ACTION_TO_FLOW)) {
RuleActionToFlow(s->action, p->flow);
}
} else if (s->action & ACTION_PASS) {
if ((p->flow != NULL) && (alert_flags & PACKET_ALERT_FLAG_APPLY_ACTION_TO_FLOW)) {
RuleActionToFlow(s->action, p->flow);
}
}
}
@ -254,8 +287,8 @@ void PacketAlertFinalize(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx
int i = 0;
while (i < p->alerts.cnt) {
SCLogDebug("Sig->num: %"PRIu32, p->alerts.alerts[i].num);
const Signature *s = de_ctx->sig_array[p->alerts.alerts[i].num];
SCLogDebug("Sig->num: %" PRIu32 " SID %u", p->alerts.alerts[i].num, s->id);
int res = PacketAlertHandle(de_ctx, det_ctx, s, p, &p->alerts.alerts[i]);
if (res > 0) {
@ -275,35 +308,34 @@ void PacketAlertFinalize(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx
}
}
/* IP-only and PD-only matches should apply to the flow */
if (s->flags & (SIG_FLAG_IPONLY | SIG_FLAG_PDONLY)) {
if (p->flow != NULL) {
RuleActionToFlow(s->action, p->flow);
/* For DROP and PASS sigs we need to apply the action to the flow if
* - sig is IP or PD only
* - match is in applayer
* - match is in stream */
if (s->action & (ACTION_DROP | ACTION_PASS)) {
if ((p->alerts.alerts[i].flags &
(PACKET_ALERT_FLAG_STATE_MATCH | PACKET_ALERT_FLAG_STREAM_MATCH)) ||
(s->flags & (SIG_FLAG_IPONLY | SIG_FLAG_PDONLY | SIG_FLAG_APPLAYER))) {
p->alerts.alerts[i].flags |= PACKET_ALERT_FLAG_APPLY_ACTION_TO_FLOW;
SCLogDebug("packet %" PRIu64 " sid %u action %02x alert_flags %02x (set "
"PACKET_ALERT_FLAG_APPLY_ACTION_TO_FLOW)",
p->pcap_cnt, s->id, s->action, p->alerts.alerts[i].flags);
}
}
/* set actions on packet */
DetectSignatureApplyActions(p, p->alerts.alerts[i].s, p->alerts.alerts[i].flags);
PacketApplySignatureActions(p, p->alerts.alerts[i].s, p->alerts.alerts[i].flags);
if (PacketTestAction(p, ACTION_PASS)) {
/* Ok, reset the alert cnt to end in the previous of pass
* so we ignore the rest with less prio */
p->alerts.cnt = i;
break;
/* if the signature wants to drop, check if the
* PACKET_ALERT_FLAG_DROP_FLOW flag is set. */
} else if ((PacketTestAction(p, ACTION_DROP)) &&
((p->alerts.alerts[i].flags & PACKET_ALERT_FLAG_DROP_FLOW) ||
(s->flags & SIG_FLAG_APPLAYER)) &&
p->flow != NULL) {
/* This will apply only on IPS mode (check StreamTcpPacket) */
p->flow->flags |= FLOW_ACTION_DROP; // XXX API?
}
}
/* Thresholding removes this alert */
if (res == 0 || res == 2) {
if (res == 0 || res == 2 || (s->flags & SIG_FLAG_NOALERT)) {
PacketAlertRemove(p, i);
if (p->alerts.cnt == 0)

10
src/detect-engine-iponly.c
Unescape Escape View File

@ -1115,15 +1115,7 @@ void IPOnlyMatchPacket(ThreadVars *tv,
}
}
}
if (!(s->flags & SIG_FLAG_NOALERT)) {
if (s->action & ACTION_DROP)
PacketAlertAppend(det_ctx, s, p, 0, PACKET_ALERT_FLAG_DROP_FLOW);
else
PacketAlertAppend(det_ctx, s, p, 0, 0);
} else {
/* apply actions for noalert/rule suppressed as well */
DetectSignatureApplyActions(p, s, 0);
}
PacketAlertAppend(det_ctx, s, p, 0, 0);
}
}
}

5
src/detect-engine.c
Unescape Escape View File

@ -1403,11 +1403,6 @@ static int DetectEngineInspectRulePayloadMatches(
pmatch = DetectEngineInspectStreamPayload(de_ctx, det_ctx, s, p->flow, p);
if (pmatch) {
det_ctx->flags |= DETECT_ENGINE_THREAD_CTX_STREAM_CONTENT_MATCH;
/* Tell the engine that this reassembled stream can drop the
* rest of the pkts with no further inspection */
if (s->action & ACTION_DROP)
*alert_flags |= PACKET_ALERT_FLAG_DROP_FLOW;
*alert_flags |= PACKET_ALERT_FLAG_STREAM_MATCH;
}
}

60
src/detect.c
Unescape Escape View File

@ -548,18 +548,12 @@ static void DetectRunInspectIPOnly(ThreadVars *tv, const DetectEngineCtx *de_ctx
/* save in the flow that we scanned this direction... */
FlowSetIPOnlyFlag(pflow, p->flowflags & FLOW_PKT_TOSERVER ? 1 : 0);
} else if (((p->flowflags & FLOW_PKT_TOSERVER) &&
(pflow->flags & FLOW_TOSERVER_IPONLY_SET)) ||
((p->flowflags & FLOW_PKT_TOCLIENT) &&
(pflow->flags & FLOW_TOCLIENT_IPONLY_SET)))
{
/* If we have a drop from IP only module,
* we will drop the rest of the flow packets
* This will apply only to inline/IPS */
if (pflow->flags & FLOW_ACTION_DROP) {
PACKET_DROP(p);
}
}
/* If we have a drop from IP only module,
* we will drop the rest of the flow packets
* This will apply only to inline/IPS */
if (pflow->flags & FLOW_ACTION_DROP) {
PACKET_DROP(p);
}
} else { /* p->flags & PKT_HAS_FLOW */
/* no flow */
@ -800,12 +794,7 @@ static inline void DetectRulePacketRules(
#endif
DetectRunPostMatch(tv, det_ctx, p, s);
if (!(sflags & SIG_FLAG_NOALERT)) {
PacketAlertAppend(det_ctx, s, p, 0, alert_flags);
} else {
/* apply actions even if not alerting */
DetectSignatureApplyActions(p, s, alert_flags);
}
PacketAlertAppend(det_ctx, s, p, 0, alert_flags);
next:
DetectVarProcessList(det_ctx, pflow, p);
DetectReplaceFree(det_ctx);
@ -1475,16 +1464,9 @@ static void DetectRunTx(ThreadVars *tv,
/* match */
DetectRunPostMatch(tv, det_ctx, p, s);
uint8_t alert_flags = (PACKET_ALERT_FLAG_STATE_MATCH|PACKET_ALERT_FLAG_TX);
if (s->action & ACTION_DROP)
alert_flags |= PACKET_ALERT_FLAG_DROP_FLOW;
const uint8_t alert_flags = (PACKET_ALERT_FLAG_STATE_MATCH | PACKET_ALERT_FLAG_TX);
SCLogDebug("%p/%"PRIu64" sig %u (%u) matched", tx.tx_ptr, tx.tx_id, s->id, s->num);
if (!(s->flags & SIG_FLAG_NOALERT)) {
PacketAlertAppend(det_ctx, s, p, tx.tx_id, alert_flags);
} else {
DetectSignatureApplyActions(p, s, alert_flags);
}
PacketAlertAppend(det_ctx, s, p, tx.tx_id, alert_flags);
}
DetectVarProcessList(det_ctx, p->flow, p);
RULE_PROFILING_END(det_ctx, s, r, p);
@ -1530,30 +1512,6 @@ next:
}
}
/** \brief Apply action(s) and Set 'drop' sig info,
* if applicable */
void DetectSignatureApplyActions(Packet *p,
const Signature *s, const uint8_t alert_flags)
{
PacketUpdateAction(p, s->action);
if (s->action & ACTION_DROP) {
if (p->alerts.drop.action == 0) {
p->alerts.drop.num = s->num;
p->alerts.drop.action = s->action;
p->alerts.drop.s = (Signature *)s;
}
} else if (s->action & ACTION_PASS) {
/* if an stream/app-layer match we enforce the pass for the flow */
if ((p->flow != NULL) &&
(alert_flags & (PACKET_ALERT_FLAG_STATE_MATCH|PACKET_ALERT_FLAG_STREAM_MATCH)))
{
FlowSetNoPacketInspectionFlag(p->flow);
}
}
}
static DetectEngineThreadCtx *GetTenantById(HashTable *h, uint32_t id)
{
/* technically we need to pass a DetectEngineThreadCtx struct with the

2
src/detect.h
Unescape Escape View File

@ -1483,8 +1483,6 @@ int DetectUnregisterThreadCtxFuncs(DetectEngineCtx *, DetectEngineThreadCtx *,vo
int DetectRegisterThreadCtxFuncs(DetectEngineCtx *, const char *name, void *(*InitFunc)(void *), void *data, void (*FreeFunc)(void *), int);
void *DetectThreadCtxGetKeywordThreadCtx(DetectEngineThreadCtx *, int);
void DetectSignatureApplyActions(Packet *p, const Signature *s, const uint8_t);
void RuleMatchCandidateTxArrayInit(DetectEngineThreadCtx *det_ctx, uint32_t size);
void RuleMatchCandidateTxArrayFree(DetectEngineThreadCtx *det_ctx);

3
src/flow.h
Unescape Escape View File

@ -111,6 +111,9 @@ typedef struct AppLayerParserState_ AppLayerParserState;
/** Indicate that the flow did trigger an expectation creation */
#define FLOW_HAS_EXPECTATION BIT_U32(27)
/** All packets in this flow should be passed */
#define FLOW_ACTION_PASS BIT_U32(28)
/* File flags */
#define FLOWFILE_INIT 0

Write Preview
Loading…
Cancel
Save