Skip to content

Commit

Permalink
detect/iponly: use flow first flags
Browse files Browse the repository at this point in the history
Instead of ip-only specific flags, reuse the FLOW_PKT_TOSERVER_FIRST and
FLOW_PKT_TOCLIENT_FIRST flags.

Fixes false positives on one sided streams that trigger a opposing flow
timeout packet at the flow's end. That pseudo packet would trigger a
match even though it shouldn't.

Ticket: #7521.
  • Loading branch information
victorjulien committed Jan 23, 2025
1 parent 95e8427 commit 897d1f3
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 45 deletions.
19 changes: 8 additions & 11 deletions src/alert-debuglog.c
Original file line number Diff line number Diff line change
Expand Up @@ -216,17 +216,14 @@ static TmEcode AlertDebugLogger(ThreadVars *tv, const Packet *p, void *thread_da
p->flow->todstpktcnt, p->flow->tosrcpktcnt,
p->flow->todstbytecnt + p->flow->tosrcbytecnt);
MemBufferWriteString(aft->buffer,
"FLOW IPONLY SET: TOSERVER: %s, TOCLIENT: %s\n"
"FLOW ACTION: DROP: %s\n"
"FLOW NOINSPECTION: PACKET: %s, PAYLOAD: %s, APP_LAYER: %s\n"
"FLOW APP_LAYER: DETECTED: %s, PROTO %"PRIu16"\n",
p->flow->flags & FLOW_TOSERVER_IPONLY_SET ? "TRUE" : "FALSE",
p->flow->flags & FLOW_TOCLIENT_IPONLY_SET ? "TRUE" : "FALSE",
p->flow->flags & FLOW_ACTION_DROP ? "TRUE" : "FALSE",
p->flow->flags & FLOW_NOPACKET_INSPECTION ? "TRUE" : "FALSE",
p->flow->flags & FLOW_NOPAYLOAD_INSPECTION ? "TRUE" : "FALSE",
applayer ? "TRUE" : "FALSE",
(p->flow->alproto != ALPROTO_UNKNOWN) ? "TRUE" : "FALSE", p->flow->alproto);
"FLOW ACTION: DROP: %s\n"
"FLOW NOINSPECTION: PACKET: %s, PAYLOAD: %s, APP_LAYER: %s\n"
"FLOW APP_LAYER: DETECTED: %s, PROTO %" PRIu16 "\n",
p->flow->flags & FLOW_ACTION_DROP ? "TRUE" : "FALSE",
p->flow->flags & FLOW_NOPACKET_INSPECTION ? "TRUE" : "FALSE",
p->flow->flags & FLOW_NOPAYLOAD_INSPECTION ? "TRUE" : "FALSE",
applayer ? "TRUE" : "FALSE",
(p->flow->alproto != ALPROTO_UNKNOWN) ? "TRUE" : "FALSE", p->flow->alproto);
AlertDebugLogFlowVars(aft, p);
}

Expand Down
2 changes: 1 addition & 1 deletion src/detect-engine-iponly.c
Original file line number Diff line number Diff line change
Expand Up @@ -2097,7 +2097,7 @@ static int IPOnlyTestSig15(void)
p[0]->flow = &f;
p[0]->flow->flowvar = &flowvar;
p[0]->flags |= PKT_HAS_FLOW;
p[0]->flowflags |= FLOW_PKT_TOSERVER;
p[0]->flowflags |= (FLOW_PKT_TOSERVER | FLOW_PKT_TOSERVER_FIRST);

const char *sigs[numsigs];
sigs[0]= "alert tcp 192.168.1.5 any -> any any (msg:\"Testing src ip (sid 1)\"; "
Expand Down
2 changes: 1 addition & 1 deletion src/detect-flowbits.c
Original file line number Diff line number Diff line change
Expand Up @@ -974,7 +974,7 @@ static int FlowBitsTestSig06(void)
p->payload_len = buflen;
p->proto = IPPROTO_TCP;
p->flags |= PKT_HAS_FLOW;
p->flowflags |= FLOW_PKT_TOSERVER;
p->flowflags |= (FLOW_PKT_TOSERVER | FLOW_PKT_TOSERVER_FIRST);

de_ctx = DetectEngineCtxInit();
FAIL_IF_NULL(de_ctx);
Expand Down
13 changes: 1 addition & 12 deletions src/detect.c
Original file line number Diff line number Diff line change
Expand Up @@ -564,23 +564,12 @@ static void DetectRunInspectIPOnly(ThreadVars *tv, const DetectEngineCtx *de_ctx
Flow * const pflow, Packet * const p)
{
if (pflow) {
/* set the iponly stuff */
if (pflow->flags & FLOW_TOCLIENT_IPONLY_SET)
p->flowflags |= FLOW_PKT_TOCLIENT_IPONLY_SET;
if (pflow->flags & FLOW_TOSERVER_IPONLY_SET)
p->flowflags |= FLOW_PKT_TOSERVER_IPONLY_SET;

if (((p->flowflags & FLOW_PKT_TOSERVER) && !(p->flowflags & FLOW_PKT_TOSERVER_IPONLY_SET)) ||
((p->flowflags & FLOW_PKT_TOCLIENT) && !(p->flowflags & FLOW_PKT_TOCLIENT_IPONLY_SET)))
{
if (p->flowflags & (FLOW_PKT_TOSERVER_FIRST | FLOW_PKT_TOCLIENT_FIRST)) {
SCLogDebug("testing against \"ip-only\" signatures");

PACKET_PROFILING_DETECT_START(p, PROF_DETECT_IPONLY);
IPOnlyMatchPacket(tv, de_ctx, det_ctx, &de_ctx->io_ctx, p);
PACKET_PROFILING_DETECT_END(p, PROF_DETECT_IPONLY);

/* save in the flow that we scanned this direction... */
FlowSetIPOnlyFlag(pflow, p->flowflags & FLOW_PKT_TOSERVER ? 1 : 0);
}
} else { /* p->flags & PKT_HAS_FLOW */
/* no flow */
Expand Down
11 changes: 0 additions & 11 deletions src/flow.c
Original file line number Diff line number Diff line change
Expand Up @@ -144,16 +144,6 @@ void FlowCleanupAppLayer(Flow *f)
f->alparser = NULL;
}

/** \brief Set the IPOnly scanned flag for 'direction'.
*
* \param f Flow to set the flag in
* \param direction direction to set the flag in
*/
void FlowSetIPOnlyFlag(Flow *f, int direction)
{
direction ? (f->flags |= FLOW_TOSERVER_IPONLY_SET) : (f->flags |= FLOW_TOCLIENT_IPONLY_SET);
}

/** \brief Set flag to indicate that flow has alerts
*
* \param f flow
Expand Down Expand Up @@ -213,7 +203,6 @@ int FlowChangeProto(Flow *f)
static inline void FlowSwapFlags(Flow *f)
{
SWAP_FLAGS(f->flags, FLOW_TO_SRC_SEEN, FLOW_TO_DST_SEEN);
SWAP_FLAGS(f->flags, FLOW_TOSERVER_IPONLY_SET, FLOW_TOCLIENT_IPONLY_SET);
SWAP_FLAGS(f->flags, FLOW_SGH_TOSERVER, FLOW_SGH_TOCLIENT);

SWAP_FLAGS(f->flags, FLOW_TOSERVER_DROP_LOGGED, FLOW_TOCLIENT_DROP_LOGGED);
Expand Down
15 changes: 6 additions & 9 deletions src/flow.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,9 @@ typedef struct AppLayerParserState_ AppLayerParserState;
/** next packet in toclient direction will act on updated app-layer state */
#define FLOW_TC_APP_UPDATE_NEXT BIT_U32(2)

/** Flow was inspected against IP-Only sigs in the toserver direction */
#define FLOW_TOSERVER_IPONLY_SET BIT_U32(3)
/** Flow was inspected against IP-Only sigs in the toclient direction */
#define FLOW_TOCLIENT_IPONLY_SET BIT_U32(4)
// vacancy bit 3

// vacancy bit 4

/** Packet belonging to this flow should not be inspected at all */
#define FLOW_NOPACKET_INSPECTION BIT_U32(5)
Expand Down Expand Up @@ -232,13 +231,11 @@ typedef struct AppLayerParserState_ AppLayerParserState;
#define FLOW_PKT_TOSERVER 0x01
#define FLOW_PKT_TOCLIENT 0x02
#define FLOW_PKT_ESTABLISHED 0x04
#define FLOW_PKT_TOSERVER_IPONLY_SET 0x08
#define FLOW_PKT_TOCLIENT_IPONLY_SET 0x10
#define FLOW_PKT_TOSERVER_FIRST 0x20
#define FLOW_PKT_TOCLIENT_FIRST 0x40
#define FLOW_PKT_TOSERVER_FIRST 0x08
#define FLOW_PKT_TOCLIENT_FIRST 0x10
/** last pseudo packet in the flow. Can be used to trigger final clean,
* logging, etc. */
#define FLOW_PKT_LAST_PSEUDO 0x80
#define FLOW_PKT_LAST_PSEUDO 0x20

#define FLOW_END_FLAG_EMERGENCY 0x01
#define FLOW_END_FLAG_TIMEOUT 0x02
Expand Down

0 comments on commit 897d1f3

Please sign in to comment.