detect: set detect table for non-firewall mode as well

This also exposed a difference between the handling of TD alerts in
firewall vs non-firewall mode. In firewall mode the table/hook is also
part of the alert ordering to make sure actions from packet:td are
applied before app:td. Handle that explicitly for now.
pull/13410/head
Victor Julien 6 months ago committed by Victor Julien
parent 7896148798
commit f1fdc1801e

@ -192,12 +192,10 @@ static void PacketApplySignatureActions(Packet *p, const Signature *s, const Pac
/* REJECT also sets ACTION_DROP, just make it more visible with this check */ /* REJECT also sets ACTION_DROP, just make it more visible with this check */
if (pa->action & ACTION_DROP_REJECT) { if (pa->action & ACTION_DROP_REJECT) {
uint8_t drop_reason = PKT_DROP_REASON_RULES; uint8_t drop_reason = PKT_DROP_REASON_RULES;
if (s->flags & SIG_FLAG_FIREWALL) { if (s->detect_table == DETECT_TABLE_PACKET_PRE_STREAM) {
if (s->firewall_table == FIREWALL_TABLE_PACKET_PRE_STREAM) { drop_reason = PKT_DROP_REASON_STREAM_PRE_HOOK;
drop_reason = PKT_DROP_REASON_STREAM_PRE_HOOK; } else if (s->detect_table == DETECT_TABLE_PACKET_PRE_FLOW) {
} else if (s->firewall_table == FIREWALL_TABLE_PACKET_PRE_FLOW) { drop_reason = PKT_DROP_REASON_FLOW_PRE_HOOK;
drop_reason = PKT_DROP_REASON_FLOW_PRE_HOOK;
}
} }
/* PacketDrop will update the packet action, too */ /* PacketDrop will update the packet action, too */
@ -336,11 +334,11 @@ void AlertQueueAppend(DetectEngineThreadCtx *det_ctx, const Signature *s, Packet
* The Signature::num field is set based on internal priority. Higher priority * The Signature::num field is set based on internal priority. Higher priority
* rules have lower nums. * rules have lower nums.
*/ */
static int AlertQueueSortHelper(const void *a, const void *b) static int AlertQueueSortHelperFirewall(const void *a, const void *b)
{ {
const PacketAlert *pa0 = a; const PacketAlert *pa0 = a;
const PacketAlert *pa1 = b; const PacketAlert *pa1 = b;
if (pa0->s->firewall_table == pa1->s->firewall_table) { if (pa0->s->detect_table == pa1->s->detect_table) {
if (pa1->iid == pa0->iid) { if (pa1->iid == pa0->iid) {
if (pa1->tx_id == PACKET_ALERT_NOTX) { if (pa1->tx_id == PACKET_ALERT_NOTX) {
return -1; return -1;
@ -352,7 +350,23 @@ static int AlertQueueSortHelper(const void *a, const void *b)
return pa0->iid < pa1->iid ? -1 : 1; return pa0->iid < pa1->iid ? -1 : 1;
} }
} }
return pa0->s->firewall_table < pa1->s->firewall_table ? -1 : 1; return pa0->s->detect_table < pa1->s->detect_table ? -1 : 1;
}
static int AlertQueueSortHelper(const void *a, const void *b)
{
const PacketAlert *pa0 = a;
const PacketAlert *pa1 = b;
if (pa1->iid == pa0->iid) {
if (pa1->tx_id == PACKET_ALERT_NOTX) {
return -1;
} else if (pa0->tx_id == PACKET_ALERT_NOTX) {
return 1;
}
return pa0->tx_id < pa1->tx_id ? 1 : -1;
} else {
return pa0->iid < pa1->iid ? -1 : 1;
}
} }
/** \internal /** \internal
@ -400,7 +414,8 @@ static inline void PacketAlertFinalizeProcessQueue(
if (det_ctx->alert_queue_size > 1) { if (det_ctx->alert_queue_size > 1) {
/* sort the alert queue before thresholding and appending to Packet */ /* sort the alert queue before thresholding and appending to Packet */
qsort(det_ctx->alert_queue, det_ctx->alert_queue_size, sizeof(PacketAlert), qsort(det_ctx->alert_queue, det_ctx->alert_queue_size, sizeof(PacketAlert),
AlertQueueSortHelper); (de_ctx->flags & DE_HAS_FIREWALL) ? AlertQueueSortHelperFirewall
: AlertQueueSortHelper);
} }
bool dropped = false; bool dropped = false;

@ -126,22 +126,22 @@ const struct SignatureProperties signature_properties[SIG_TYPE_MAX] = {
// rule types documentation tag end: SignatureProperties // rule types documentation tag end: SignatureProperties
// clang-format on // clang-format on
const char *DetectTableToString(enum FirewallTable table) const char *DetectTableToString(enum DetectTable table)
{ {
switch (table) { switch (table) {
case FIREWALL_TABLE_NOT_SET: case DETECT_TABLE_NOT_SET:
return "not_set"; return "not_set";
case FIREWALL_TABLE_PACKET_PRE_FLOW: case DETECT_TABLE_PACKET_PRE_FLOW:
return "pre_flow"; return "pre_flow";
case FIREWALL_TABLE_PACKET_PRE_STREAM: case DETECT_TABLE_PACKET_PRE_STREAM:
return "pre_stream"; return "pre_stream";
case FIREWALL_TABLE_PACKET_FILTER: case DETECT_TABLE_PACKET_FILTER:
return "packet_filter"; return "packet_filter";
case FIREWALL_TABLE_PACKET_TD: case DETECT_TABLE_PACKET_TD:
return "packet_td"; return "packet_td";
case FIREWALL_TABLE_APP_FILTER: case DETECT_TABLE_APP_FILTER:
return "app_filter"; return "app_filter";
case FIREWALL_TABLE_APP_TD: case DETECT_TABLE_APP_TD:
return "app_td"; return "app_td";
default: default:
return "unknown"; return "unknown";

@ -27,7 +27,7 @@
#include "detect.h" #include "detect.h"
#include "suricata.h" #include "suricata.h"
const char *DetectTableToString(enum FirewallTable table); const char *DetectTableToString(enum DetectTable table);
/* start up registry funcs */ /* start up registry funcs */

@ -2460,16 +2460,16 @@ static void SigSetupPrefilter(DetectEngineCtx *de_ctx, Signature *s)
*/ */
static bool DetectRuleValidateTable(const Signature *s) static bool DetectRuleValidateTable(const Signature *s)
{ {
if (s->firewall_table == 0) if (s->detect_table == 0)
return true; return true;
const uint8_t table_as_flag = BIT_U8(s->firewall_table); const uint8_t table_as_flag = BIT_U8(s->detect_table);
for (SigMatch *sm = s->init_data->smlists[DETECT_SM_LIST_MATCH]; sm != NULL; sm = sm->next) { for (SigMatch *sm = s->init_data->smlists[DETECT_SM_LIST_MATCH]; sm != NULL; sm = sm->next) {
const uint8_t kw_tables_supported = sigmatch_table[sm->type].tables; const uint8_t kw_tables_supported = sigmatch_table[sm->type].tables;
if (kw_tables_supported != 0 && (kw_tables_supported & table_as_flag) == 0) { if (kw_tables_supported != 0 && (kw_tables_supported & table_as_flag) == 0) {
SCLogError("rule %u uses hook \"%s\", but keyword \"%s\" doesn't support this hook", SCLogError("rule %u uses hook \"%s\", but keyword \"%s\" doesn't support this hook",
s->id, DetectTableToString(s->firewall_table), sigmatch_table[sm->type].name); s->id, DetectTableToString(s->detect_table), sigmatch_table[sm->type].name);
return false; return false;
} }
} }
@ -2487,33 +2487,34 @@ static bool DetectFirewallRuleValidate(const DetectEngineCtx *de_ctx, const Sign
return true; return true;
} }
static void DetectFirewallRuleSetTable(Signature *s) static void DetectRuleSetTable(Signature *s)
{ {
enum FirewallTable table; enum DetectTable table;
if (s->flags & SIG_FLAG_FIREWALL) { if (s->flags & SIG_FLAG_FIREWALL) {
if (s->type == SIG_TYPE_PKT) { if (s->type == SIG_TYPE_PKT) {
if (s->init_data->hook.type == SIGNATURE_HOOK_TYPE_PKT && if (s->init_data->hook.type == SIGNATURE_HOOK_TYPE_PKT &&
s->init_data->hook.t.pkt.ph == SIGNATURE_HOOK_PKT_PRE_STREAM) s->init_data->hook.t.pkt.ph == SIGNATURE_HOOK_PKT_PRE_STREAM)
table = FIREWALL_TABLE_PACKET_PRE_STREAM; table = DETECT_TABLE_PACKET_PRE_STREAM;
else if (s->init_data->hook.type == SIGNATURE_HOOK_TYPE_PKT && else if (s->init_data->hook.type == SIGNATURE_HOOK_TYPE_PKT &&
s->init_data->hook.t.pkt.ph == SIGNATURE_HOOK_PKT_PRE_FLOW) s->init_data->hook.t.pkt.ph == SIGNATURE_HOOK_PKT_PRE_FLOW)
table = FIREWALL_TABLE_PACKET_PRE_FLOW; table = DETECT_TABLE_PACKET_PRE_FLOW;
else else
table = FIREWALL_TABLE_PACKET_FILTER; table = DETECT_TABLE_PACKET_FILTER;
} else if (s->type == SIG_TYPE_APP_TX) { } else if (s->type == SIG_TYPE_APP_TX) {
table = FIREWALL_TABLE_APP_FILTER; table = DETECT_TABLE_APP_FILTER;
} else { } else {
BUG_ON(1); BUG_ON(1);
} }
} else { } else {
// TODO pre_flow/pre_stream
if (s->type != SIG_TYPE_APP_TX) { if (s->type != SIG_TYPE_APP_TX) {
table = FIREWALL_TABLE_PACKET_TD; table = DETECT_TABLE_PACKET_TD;
} else { } else {
table = FIREWALL_TABLE_APP_TD; table = DETECT_TABLE_APP_TD;
} }
} }
s->firewall_table = (uint8_t)table; s->detect_table = (uint8_t)table;
} }
static int SigValidateFirewall(const DetectEngineCtx *de_ctx, const Signature *s) static int SigValidateFirewall(const DetectEngineCtx *de_ctx, const Signature *s)
@ -2829,9 +2830,7 @@ static int SigValidateConsolidate(
SigConsolidateTcpBuffer(s); SigConsolidateTcpBuffer(s);
SignatureSetType(de_ctx, s); SignatureSetType(de_ctx, s);
if (de_ctx->flags & DE_HAS_FIREWALL) { DetectRuleSetTable(s);
DetectFirewallRuleSetTable(s);
}
int r = SigValidateFileHandling(s); int r = SigValidateFileHandling(s);
if (r == 0) { if (r == 0) {

@ -550,22 +550,21 @@ enum SignatureHookType {
SIGNATURE_HOOK_TYPE_APP, SIGNATURE_HOOK_TYPE_APP,
}; };
// TODO should probably be renamed to DetectTable, similar for values enum DetectTable {
enum FirewallTable { DETECT_TABLE_NOT_SET = 0,
FIREWALL_TABLE_NOT_SET = 0, DETECT_TABLE_PACKET_PRE_FLOW,
FIREWALL_TABLE_PACKET_PRE_FLOW, DETECT_TABLE_PACKET_PRE_STREAM,
FIREWALL_TABLE_PACKET_PRE_STREAM, DETECT_TABLE_PACKET_FILTER,
FIREWALL_TABLE_PACKET_FILTER, DETECT_TABLE_PACKET_TD,
FIREWALL_TABLE_PACKET_TD, DETECT_TABLE_APP_FILTER,
FIREWALL_TABLE_APP_FILTER, DETECT_TABLE_APP_TD,
FIREWALL_TABLE_APP_TD,
#define DETECT_TABLE_PACKET_PRE_FLOW_FLAG BIT_U8(DETECT_TABLE_PACKET_PRE_FLOW)
#define DETECT_TABLE_PACKET_PRE_FLOW_FLAG BIT_U8(FIREWALL_TABLE_PACKET_PRE_FLOW) #define DETECT_TABLE_PACKET_PRE_STREAM_FLAG BIT_U8(DETECT_TABLE_PACKET_PRE_STREAM)
#define DETECT_TABLE_PACKET_PRE_STREAM_FLAG BIT_U8(FIREWALL_TABLE_PACKET_PRE_STREAM) #define DETECT_TABLE_PACKET_FILTER_FLAG BIT_U8(DETECT_TABLE_PACKET_FILTER)
#define DETECT_TABLE_PACKET_FILTER_FLAG BIT_U8(FIREWALL_TABLE_PACKET_FILTER) #define DETECT_TABLE_PACKET_TD_FLAG BIT_U8(DETECT_TABLE_PACKET_TD)
#define DETECT_TABLE_PACKET_TD_FLAG BIT_U8(FIREWALL_TABLE_PACKET_TD) #define DETECT_TABLE_APP_FILTER_FLAG BIT_U8(DETECT_TABLE_APP_FILTER)
#define DETECT_TABLE_APP_FILTER_FLAG BIT_U8(FIREWALL_TABLE_APP_FILTER) #define DETECT_TABLE_APP_TD_FLAG BIT_U8(DETECT_TABLE_APP_TD)
#define DETECT_TABLE_APP_TD_FLAG BIT_U8(FIREWALL_TABLE_APP_TD)
}; };
// dns:request_complete should add DetectBufferTypeGetByName("dns:request_complete"); // dns:request_complete should add DetectBufferTypeGetByName("dns:request_complete");
@ -700,8 +699,8 @@ typedef struct Signature_ {
/** classification id **/ /** classification id **/
uint16_t class_id; uint16_t class_id;
/** firewall: pseudo table this rule is part of (enum FirewallTable) */ /** detect: pseudo table this rule is part of (enum DetectTable) */
uint8_t firewall_table; uint8_t detect_table;
/** firewall: progress value for this signature */ /** firewall: progress value for this signature */
uint8_t app_progress_hook; uint8_t app_progress_hook;

Loading…
Cancel
Save