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

tunnel: refactor tunnel verdict handling

Browse Source 
Observed:

STARTTLS creates 2 pseudo packets which are tied to a real packet.
TPR (tunnel packet ref) counter increased to 2.

Pseudo 1: goes through 'verdict', increments 'ready to verdict' to 1.
Packet pool return code frees this packet and decrements TPR in root
to 1. RTV counter not changed. So both are now 1.

Pseudo 2: verdict code sees RTV == TPR, so verdict is set based on
pseudo packet. This is too soon. Packet pool return code frees this
packet and decrements TPR in root to 0.

Real packet: TRP is 0 so set verdict on this packet. As verdict was
already set, NFQ reports an issue.

The decrementing of TPR doesn't seem to make sense as RTV is not
updated.

Solution:

This patch refactors the ref count and verdict count logic. The beef
is now handled in the generic function TmqhOutputPacketpool(). NFQ
and IPFW call a utility function VerdictTunnelPacket to see if they
need to verdict a packet.

Remove some unused macro's for managing these counters.
pull/2804/head
Victor Julien 8 years ago
parent 7c119cc595
commit d61fa0c43c
5 changed files with 76 additions and 86 deletions
Show Stats Download Patch File Download Diff File

46
src/decode.h
Unescape Escape View File

@ -870,10 +870,8 @@ void CaptureStatsSetup(ThreadVars *tv, CaptureStats *s);
((p)->action |= a)); \ ((p)->action |= a)); \
} while (0) } while (0)
#define TUNNEL_INCR_PKT_RTV(p) do { \ #define TUNNEL_INCR_PKT_RTV_NOLOCK(p) do { \
SCMutexLock((p)->root ? &(p)->root->tunnel_mutex : &(p)->tunnel_mutex); \
((p)->root ? (p)->root->tunnel_rtv_cnt++ : (p)->tunnel_rtv_cnt++); \ ((p)->root ? (p)->root->tunnel_rtv_cnt++ : (p)->tunnel_rtv_cnt++); \
SCMutexUnlock((p)->root ? &(p)->root->tunnel_mutex : &(p)->tunnel_mutex); \
} while (0) } while (0)
#define TUNNEL_INCR_PKT_TPR(p) do { \ #define TUNNEL_INCR_PKT_TPR(p) do { \
@ -882,16 +880,6 @@ void CaptureStatsSetup(ThreadVars *tv, CaptureStats *s);
SCMutexUnlock((p)->root ? &(p)->root->tunnel_mutex : &(p)->tunnel_mutex); \ SCMutexUnlock((p)->root ? &(p)->root->tunnel_mutex : &(p)->tunnel_mutex); \
} while (0) } while (0)
#define TUNNEL_DECR_PKT_TPR(p) do { \
SCMutexLock((p)->root ? &(p)->root->tunnel_mutex : &(p)->tunnel_mutex); \
((p)->root ? (p)->root->tunnel_tpr_cnt-- : (p)->tunnel_tpr_cnt--); \
SCMutexUnlock((p)->root ? &(p)->root->tunnel_mutex : &(p)->tunnel_mutex); \
} while (0)
#define TUNNEL_DECR_PKT_TPR_NOLOCK(p) do { \
((p)->root ? (p)->root->tunnel_tpr_cnt-- : (p)->tunnel_tpr_cnt--); \
} while (0)
#define TUNNEL_PKT_RTV(p) ((p)->root ? (p)->root->tunnel_rtv_cnt : (p)->tunnel_rtv_cnt) #define TUNNEL_PKT_RTV(p) ((p)->root ? (p)->root->tunnel_rtv_cnt : (p)->tunnel_rtv_cnt)
#define TUNNEL_PKT_TPR(p) ((p)->root ? (p)->root->tunnel_tpr_cnt : (p)->tunnel_tpr_cnt) #define TUNNEL_PKT_TPR(p) ((p)->root ? (p)->root->tunnel_tpr_cnt : (p)->tunnel_tpr_cnt)
@ -1132,5 +1120,37 @@ int DecoderParseDataFromFileSerie(char *fileprefix, DecoderFunc Decoder);
#define PKT_SET_SRC(p, src_val) ((p)->pkt_src = src_val) #define PKT_SET_SRC(p, src_val) ((p)->pkt_src = src_val)
/** \brief return true if *this* packet needs to trigger a verdict.
*
* If we have the root packet, and we have none outstanding,
* we can verdict now.
*
* If we have a upper layer packet, it's the only one and root
* is already processed, we can verdict now.
*
* Otherwise, a future packet will issue the verdict.
*/
static inline bool VerdictTunnelPacket(Packet *p)
{
bool verdict = true;
SCMutex *m = p->root ? &p->root->tunnel_mutex : &p->tunnel_mutex;
SCMutexLock(m);
const uint16_t outstanding = TUNNEL_PKT_TPR(p) - TUNNEL_PKT_RTV(p);
SCLogDebug("tunnel: outstanding %u", outstanding);
/* if there are packets outstanding, we won't verdict this one */
if (IS_TUNNEL_ROOT_PKT(p) && !IS_TUNNEL_PKT_VERDICTED(p) && !outstanding) {
// verdict
SCLogDebug("root %p: verdict", p);
} else if (!IS_TUNNEL_ROOT_PKT(p) && outstanding == 1 && p->root && IS_TUNNEL_PKT_VERDICTED(p->root)) {
// verdict
SCLogDebug("tunnel %p: verdict", p);
} else {
verdict = false;
}
SCMutexUnlock(m);
return verdict;
}
#endif /* __DECODE_H__ */ #endif /* __DECODE_H__ */

22
src/source-ipfw.c
Unescape Escape View File

@ -623,30 +623,12 @@ TmEcode VerdictIPFW(ThreadVars *tv, Packet *p, void *data, PacketQueue *pq, Pack
* if this is a tunnel packet we check if we are ready to verdict * if this is a tunnel packet we check if we are ready to verdict
* already. */ * already. */
if (IS_TUNNEL_PKT(p)) { if (IS_TUNNEL_PKT(p)) {
char verdict = 1; bool verdict = VerdictTunnelPacket(p);
SCMutex *m = p->root ? &p->root->tunnel_mutex : &p->tunnel_mutex;
SCMutexLock(m);
/* if there are more tunnel packets than ready to verdict packets,
* we won't verdict this one
*/
if (TUNNEL_PKT_TPR(p) > TUNNEL_PKT_RTV(p)) {
SCLogDebug("VerdictIPFW: not ready to verdict yet: "
"TUNNEL_PKT_TPR(p) > TUNNEL_PKT_RTV(p) = %" PRId32
" > %" PRId32 "", TUNNEL_PKT_TPR(p), TUNNEL_PKT_RTV(p));
verdict = 0;
}
SCMutexUnlock(m);
/* don't verdict if we are not ready */ /* don't verdict if we are not ready */
if (verdict == 1) { if (verdict == true) {
SCLogDebug("Setting verdict on tunnel"); SCLogDebug("Setting verdict on tunnel");
retval = IPFWSetVerdict(tv, ptv, p->root ? p->root : p); retval = IPFWSetVerdict(tv, ptv, p->root ? p->root : p);
} else {
TUNNEL_INCR_PKT_RTV(p);
} }
} else { } else {
/* no tunnel, verdict normally */ /* no tunnel, verdict normally */

33
src/source-nfq.c
Unescape Escape View File

@ -943,7 +943,8 @@ static void NFQRecvPkt(NFQQueueVars *t, NFQThreadVars *tv)
NFQMutexUnlock(t); NFQMutexUnlock(t);
if (ret != 0) { if (ret != 0) {
SCLogWarning(SC_ERR_NFQ_HANDLE_PKT, "nfq_handle_packet error %" PRId32 "", ret); SCLogWarning(SC_ERR_NFQ_HANDLE_PKT, "nfq_handle_packet error %"PRId32" %s",
ret, strerror(errno));
} }
} }
} }
@ -1145,37 +1146,21 @@ TmEcode VerdictNFQ(ThreadVars *tv, Packet *p, void *data, PacketQueue *pq, Packe
/* if this is a tunnel packet we check if we are ready to verdict /* if this is a tunnel packet we check if we are ready to verdict
* already. */ * already. */
if (IS_TUNNEL_PKT(p)) { if (IS_TUNNEL_PKT(p)) {
char verdict = 1; SCLogDebug("tunnel pkt: %p/%p %s", p, p->root, p->root ? "upper layer" : "root");
//printf("VerdictNFQ: tunnel pkt: %p %s\n", p, p->root ? "upper layer" : "root"); bool verdict = VerdictTunnelPacket(p);
SCMutex *m = p->root ? &p->root->tunnel_mutex : &p->tunnel_mutex;
SCMutexLock(m);
/* if there are more tunnel packets than ready to verdict packets,
* we won't verdict this one */
if (TUNNEL_PKT_TPR(p) > TUNNEL_PKT_RTV(p)) {
SCLogDebug("not ready to verdict yet: TUNNEL_PKT_TPR(p) > "
"TUNNEL_PKT_RTV(p) = %" PRId32 " > %" PRId32,
TUNNEL_PKT_TPR(p), TUNNEL_PKT_RTV(p));
verdict = 0;
}
SCMutexUnlock(m);
/* don't verdict if we are not ready */ /* don't verdict if we are not ready */
if (verdict == 1) { if (verdict == true) {
//printf("VerdictNFQ: setting verdict\n");
ret = NFQSetVerdict(p->root ? p->root : p); ret = NFQSetVerdict(p->root ? p->root : p);
if (ret != TM_ECODE_OK) if (ret != TM_ECODE_OK) {
return ret; return ret;
} else { }
TUNNEL_INCR_PKT_RTV(p);
} }
} else { } else {
/* no tunnel, verdict normally */ /* no tunnel, verdict normally */
ret = NFQSetVerdict(p); ret = NFQSetVerdict(p);
if (ret != TM_ECODE_OK) if (ret != TM_ECODE_OK) {
return ret; return ret;
}
} }
return TM_ECODE_OK; return TM_ECODE_OK;
} }

1
src/stream-tcp.c
Unescape Escape View File

@ -5844,6 +5844,7 @@ static void StreamTcpPseudoPacketCreateDetectLogFlush(ThreadVars *tv,
#endif #endif
} }
SCLogDebug("np %p", np);
PacketEnqueue(pq, np); PacketEnqueue(pq, np);
StatsIncr(tv, stt->counter_tcp_pseudo); StatsIncr(tv, stt->counter_tcp_pseudo);

60
src/tmqh-packetpool.c
Unescape Escape View File

@ -443,7 +443,7 @@ Packet *TmqhInputPacketpool(ThreadVars *tv)
void TmqhOutputPacketpool(ThreadVars *t, Packet *p) void TmqhOutputPacketpool(ThreadVars *t, Packet *p)
{ {
int proot = 0; bool proot = false;
SCEnter(); SCEnter();
SCLogDebug("Packet %p, p->root %p, alloced %s", p, p->root, p->flags & PKT_ALLOC ? "true" : "false"); SCLogDebug("Packet %p, p->root %p, alloced %s", p, p->root, p->flags & PKT_ALLOC ? "true" : "false");
@ -458,17 +458,19 @@ void TmqhOutputPacketpool(ThreadVars *t, Packet *p)
if (IS_TUNNEL_ROOT_PKT(p)) { if (IS_TUNNEL_ROOT_PKT(p)) {
SCLogDebug("IS_TUNNEL_ROOT_PKT == TRUE"); SCLogDebug("IS_TUNNEL_ROOT_PKT == TRUE");
if (TUNNEL_PKT_TPR(p) == 0) { const uint16_t outstanding = TUNNEL_PKT_TPR(p) - TUNNEL_PKT_RTV(p);
SCLogDebug("TUNNEL_PKT_TPR(p) == 0, no more tunnel packet " SCLogDebug("root pkt: outstanding %u", outstanding);
"depending on this root"); if (outstanding == 0) {
SCLogDebug("no tunnel packets outstanding, no more tunnel "
"packet(s) depending on this root");
/* if this packet is the root and there are no /* if this packet is the root and there are no
* more tunnel packets, return it to the pool */ * more tunnel packets to consider
*
/* fall through */ * return it to the pool */
} else { } else {
SCLogDebug("tunnel root Packet %p: TUNNEL_PKT_TPR(p) > 0, so " SCLogDebug("tunnel root Packet %p: outstanding > 0, so "
"packets are still depending on this root, setting " "packets are still depending on this root, setting "
"p->tunnel_verdicted == 1", p); "SET_TUNNEL_PKT_VERDICTED", p);
/* if this is the root and there are more tunnel /* if this is the root and there are more tunnel
* packets, return this to the pool. It's still referenced * packets, return this to the pool. It's still referenced
* by the tunnel packets, and we will return it * by the tunnel packets, and we will return it
@ -482,34 +484,34 @@ void TmqhOutputPacketpool(ThreadVars *t, Packet *p)
} else { } else {
SCLogDebug("NOT IS_TUNNEL_ROOT_PKT, so tunnel pkt"); SCLogDebug("NOT IS_TUNNEL_ROOT_PKT, so tunnel pkt");
/* the p->root != NULL here seems unnecessary: IS_TUNNEL_PKT checks TUNNEL_INCR_PKT_RTV_NOLOCK(p);
* that p->tunnel_pkt == 1, IS_TUNNEL_ROOT_PKT checks that + const uint16_t outstanding = TUNNEL_PKT_TPR(p) - TUNNEL_PKT_RTV(p);
* p->root == NULL. So when we are here p->root can only be SCLogDebug("tunnel pkt: outstanding %u", outstanding);
* non-NULL, right? CLANG thinks differently. May be a FP, but /* all tunnel packets are processed except us. Root already
* better safe than sorry. VJ */ * processed. So return tunnel pkt and root packet to the
if (p->root != NULL && IS_TUNNEL_PKT_VERDICTED(p->root) && * pool. */
TUNNEL_PKT_TPR(p) == 1) if (outstanding == 0 &&
p->root && IS_TUNNEL_PKT_VERDICTED(p->root))
{ {
SCLogDebug("p->root->tunnel_verdicted == 1 && TUNNEL_PKT_TPR(p) == 1"); SCLogDebug("root verdicted == true && no outstanding");
/* the root is ready and we are the last tunnel packet,
* lets enqueue them both. */
TUNNEL_DECR_PKT_TPR_NOLOCK(p);
/* handle the root */ /* handle freeing the root as well*/
SCLogDebug("setting proot = 1 for root pkt, p->root %p " SCLogDebug("setting proot = 1 for root pkt, p->root %p "
"(tunnel packet %p)", p->root, p); "(tunnel packet %p)", p->root, p);
proot = 1; proot = true;
/* fall through */ /* fall through */
} else {
/* root not ready yet, so get rid of the tunnel pkt only */
SCLogDebug("NOT p->root->tunnel_verdicted == 1 && " } else {
"TUNNEL_PKT_TPR(p) == 1 (%" PRIu32 ")", TUNNEL_PKT_TPR(p)); /* root not ready yet, or not the last tunnel packet,
* so get rid of the tunnel pkt only */
TUNNEL_DECR_PKT_TPR_NOLOCK(p); SCLogDebug("NOT IS_TUNNEL_PKT_VERDICTED (%s) || "
"outstanding > 0 (%u)",
(p->root && IS_TUNNEL_PKT_VERDICTED(p->root)) ? "true" : "false",
outstanding);
/* fall through */ /* fall through */
} }
} }
SCMutexUnlock(m); SCMutexUnlock(m);
@ -520,7 +522,7 @@ void TmqhOutputPacketpool(ThreadVars *t, Packet *p)
FlowDeReference(&p->flow); FlowDeReference(&p->flow);
/* we're done with the tunnel root now as well */ /* we're done with the tunnel root now as well */
if (proot == 1) { if (proot == true) {
SCLogDebug("getting rid of root pkt... alloc'd %s", p->root->flags & PKT_ALLOC ? "true" : "false"); SCLogDebug("getting rid of root pkt... alloc'd %s", p->root->flags & PKT_ALLOC ? "true" : "false");
FlowDeReference(&p->root->flow); FlowDeReference(&p->root->flow);

Write Preview
Loading…
Cancel
Save