From 3fdf52239d611c0d436934e3adf99544ba686419 Mon Sep 17 00:00:00 2001 From: Eric Leblond Date: Thu, 28 Nov 2013 17:36:03 +0100 Subject: [PATCH] defrag: don't modify packet if defrag fails If defrag fails dur to an invalid decoding, we are not modifying the origin packet anymore. --- src/decode-ipv4.c | 14 ++------ src/decode-ipv6.c | 15 ++------- src/decode.c | 10 +++--- src/decode.h | 1 + src/defrag.c | 85 ++++++++++++++++++++++++++++------------------- src/defrag.h | 2 +- 6 files changed, 63 insertions(+), 64 deletions(-) diff --git a/src/decode-ipv4.c b/src/decode-ipv4.c index 268286472a..e1442fa9d7 100644 --- a/src/decode-ipv4.c +++ b/src/decode-ipv4.c @@ -46,8 +46,6 @@ #include "util-print.h" #include "util-profiling.h" -#include "tmqh-packetpool.h" - /* Generic validation * * [--type--][--len---] @@ -517,8 +515,6 @@ static int DecodeIPV4Packet(Packet *p, uint8_t *pkt, uint16_t len) int DecodeIPV4(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt, uint16_t len, PacketQueue *pq) { - int ret; - SCPerfCounterIncr(dtv->counter_ipv4, tv->sc_perf_pca); SCLogDebug("pkt %p len %"PRIu16"", pkt, len); @@ -533,15 +529,9 @@ int DecodeIPV4(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt, u /* If a fragment, pass off for re-assembly. */ if (unlikely(IPV4_GET_IPOFFSET(p) > 0 || IPV4_GET_MF(p) == 1)) { - Packet *rp = Defrag(tv, dtv, p); + Packet *rp = Defrag(tv, dtv, p, pq); if (rp != NULL) { - /* Got re-assembled packet, re-run through decoder. */ - ret = DecodeIPV4(tv, dtv, rp, (void *)rp->ip4h, IPV4_GET_IPLEN(rp), pq); - if (unlikely(ret != TM_ECODE_OK)) { - TmqhOutputPacketpool(tv, rp); - } else { - PacketEnqueue(pq, rp); - } + PacketEnqueue(pq, rp); } p->flags |= PKT_IS_FRAGMENT; return TM_ECODE_OK; diff --git a/src/decode-ipv6.c b/src/decode-ipv6.c index 013926b6aa..95fe2cd781 100644 --- a/src/decode-ipv6.c +++ b/src/decode-ipv6.c @@ -44,8 +44,6 @@ #include "util-profiling.h" #include "host.h" -#include "tmqh-packetpool.h" - #define IPV6_EXTHDRS ip6eh.ip6_exthdrs #define IPV6_EH_CNT ip6eh.ip6_exthdrs_cnt @@ -596,18 +594,9 @@ int DecodeIPV6(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt, u /* Pass to defragger if a fragment. */ if (IPV6_EXTHDR_ISSET_FH(p)) { - Packet *rp = Defrag(tv, dtv, p); + Packet *rp = Defrag(tv, dtv, p, pq); if (rp != NULL) { - ret = DecodeIPV6(tv, dtv, rp, (uint8_t *)rp->ip6h, IPV6_GET_PLEN(rp) + IPV6_HEADER_LEN, pq); - if (unlikely(ret != TM_ECODE_OK)) { - TmqhOutputPacketpool(tv, rp); - } else { - /* add the tp to the packet queue. */ - PacketEnqueue(pq,rp); - /* Not really a tunnel packet, but we're piggybacking that - * functionality for now. */ - SET_TUNNEL_PKT(p); - } + PacketEnqueue(pq,rp); } } diff --git a/src/decode.c b/src/decode.c index 3928ed944e..8989f4f917 100644 --- a/src/decode.c +++ b/src/decode.c @@ -302,11 +302,14 @@ Packet *PacketDefragPktSetup(Packet *parent, uint8_t *pkt, uint16_t len, uint8_t p->ts.tv_sec = parent->ts.tv_sec; p->ts.tv_usec = parent->ts.tv_usec; p->datalink = DLT_RAW; - - /* set tunnel flags */ - /* tell new packet it's part of a tunnel */ SET_TUNNEL_PKT(p); + + SCReturnPtr(p, "Packet"); +} + +void PacketDefragPktFinishSetup(Packet *p, Packet *parent) +{ /* tell parent packet it's part of a tunnel */ SET_TUNNEL_PKT(parent); @@ -317,7 +320,6 @@ Packet *PacketDefragPktSetup(Packet *parent, uint8_t *pkt, uint16_t len, uint8_t * is the packet we will now run through the system separately. We do * check it against the ip/port/other header checks though */ DecodeSetNoPayloadInspectionFlag(parent); - SCReturnPtr(p, "Packet"); } void DecodeRegisterPerfCounters(DecodeThreadVars *dtv, ThreadVars *tv) diff --git a/src/decode.h b/src/decode.h index 444b579bc8..fe98764f4e 100644 --- a/src/decode.h +++ b/src/decode.h @@ -815,6 +815,7 @@ void DecodeRegisterPerfCounters(DecodeThreadVars *, ThreadVars *); Packet *PacketTunnelPktSetup(ThreadVars *tv, DecodeThreadVars *dtv, Packet *parent, uint8_t *pkt, uint16_t len, uint8_t proto, PacketQueue *pq); Packet *PacketDefragPktSetup(Packet *parent, uint8_t *pkt, uint16_t len, uint8_t proto); +void PacketDefragPktFinishSetup(Packet *p, Packet *parent); Packet *PacketGetFromQueueOrAlloc(void); Packet *PacketGetFromAlloc(void); void PacketFree(Packet *p); diff --git a/src/defrag.c b/src/defrag.c index 9a9603d144..f0dccc5fa7 100644 --- a/src/defrag.c +++ b/src/defrag.c @@ -55,6 +55,9 @@ #include "defrag-queue.h" #include "defrag-config.h" +#include "tmqh-packetpool.h" +#include "decode.h" + #ifdef UNITTESTS #include "util-unittest.h" #endif @@ -466,7 +469,7 @@ done: * \todo Allocate packet buffers from a pool. */ static Packet * -DefragInsertFrag(ThreadVars *tv, DecodeThreadVars *dtv, DefragTracker *tracker, Packet *p) +DefragInsertFrag(ThreadVars *tv, DecodeThreadVars *dtv, DefragTracker *tracker, Packet *p, PacketQueue *pq) { Packet *r = NULL; int ltrim = 0; @@ -714,17 +717,31 @@ insert: if (r != NULL && tv != NULL && dtv != NULL) { SCPerfCounterIncr(dtv->counter_defrag_ipv4_reassembled, tv->sc_perf_pca); + if (pq && DecodeIPV4(tv, dtv, r, (void *)r->ip4h, + IPV4_GET_IPLEN(r), pq) != TM_ECODE_OK) { + TmqhOutputPacketpool(tv, r); + } else { + PacketDefragPktFinishSetup(r, p); + } } } else if (tracker->af == AF_INET6) { r = Defrag6Reassemble(tv, tracker, p); if (r != NULL && tv != NULL && dtv != NULL) { SCPerfCounterIncr(dtv->counter_defrag_ipv6_reassembled, - tv->sc_perf_pca); + tv->sc_perf_pca); + if (pq && DecodeIPV6(tv, dtv, r, (uint8_t *)r->ip6h, + IPV6_GET_PLEN(r) + IPV6_HEADER_LEN, + pq) != TM_ECODE_OK) { + TmqhOutputPacketpool(tv, r); + } else { + PacketDefragPktFinishSetup(r, p); + } } } } + done: if (overlap) { if (af == AF_INET) { @@ -825,7 +842,7 @@ DefragGetTracker(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p) * NULL is returned. */ Packet * -Defrag(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p) +Defrag(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, PacketQueue *pq) { uint16_t frag_offset; uint8_t more_frags; @@ -866,7 +883,7 @@ Defrag(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p) if (tracker == NULL) return NULL; - Packet *rp = DefragInsertFrag(tv, dtv, tracker, p); + Packet *rp = DefragInsertFrag(tv, dtv, tracker, p, pq); DefragTrackerRelease(tracker); return rp; @@ -1076,12 +1093,12 @@ DefragInOrderSimpleTest(void) if (p3 == NULL) goto end; - if (Defrag(NULL, NULL, p1) != NULL) + if (Defrag(NULL, NULL, p1, NULL) != NULL) goto end; - if (Defrag(NULL, NULL, p2) != NULL) + if (Defrag(NULL, NULL, p2, NULL) != NULL) goto end; - reassembled = Defrag(NULL, NULL, p3); + reassembled = Defrag(NULL, NULL, p3, NULL); if (reassembled == NULL) { goto end; } @@ -1153,12 +1170,12 @@ DefragReverseSimpleTest(void) if (p3 == NULL) goto end; - if (Defrag(NULL, NULL, p3) != NULL) + if (Defrag(NULL, NULL, p3, NULL) != NULL) goto end; - if (Defrag(NULL, NULL, p2) != NULL) + if (Defrag(NULL, NULL, p2, NULL) != NULL) goto end; - reassembled = Defrag(NULL, NULL, p1); + reassembled = Defrag(NULL, NULL, p1, NULL); if (reassembled == NULL) goto end; @@ -1225,11 +1242,11 @@ IPV6DefragInOrderSimpleTest(void) if (p3 == NULL) goto end; - if (Defrag(NULL, NULL, p1) != NULL) + if (Defrag(NULL, NULL, p1, NULL) != NULL) goto end; - if (Defrag(NULL, NULL, p2) != NULL) + if (Defrag(NULL, NULL, p2, NULL) != NULL) goto end; - reassembled = Defrag(NULL, NULL, p3); + reassembled = Defrag(NULL, NULL, p3, NULL); if (reassembled == NULL) goto end; @@ -1295,11 +1312,11 @@ IPV6DefragReverseSimpleTest(void) if (p3 == NULL) goto end; - if (Defrag(NULL, NULL, p3) != NULL) + if (Defrag(NULL, NULL, p3, NULL) != NULL) goto end; - if (Defrag(NULL, NULL, p2) != NULL) + if (Defrag(NULL, NULL, p2, NULL) != NULL) goto end; - reassembled = Defrag(NULL, NULL, p1); + reassembled = Defrag(NULL, NULL, p1, NULL); if (reassembled == NULL) goto end; @@ -1417,7 +1434,7 @@ DefragDoSturgesNovakTest(int policy, u_char *expected, size_t expected_len) /* Send all but the last. */ for (i = 0; i < 9; i++) { - Packet *tp = Defrag(NULL, NULL, packets[i]); + Packet *tp = Defrag(NULL, NULL, packets[i], NULL); if (tp != NULL) { SCFree(tp); goto end; @@ -1428,7 +1445,7 @@ DefragDoSturgesNovakTest(int policy, u_char *expected, size_t expected_len) } int overlap = 0; for (; i < 16; i++) { - Packet *tp = Defrag(NULL, NULL, packets[i]); + Packet *tp = Defrag(NULL, NULL, packets[i], NULL); if (tp != NULL) { SCFree(tp); goto end; @@ -1442,7 +1459,7 @@ DefragDoSturgesNovakTest(int policy, u_char *expected, size_t expected_len) } /* And now the last one. */ - Packet *reassembled = Defrag(NULL, NULL, packets[16]); + Packet *reassembled = Defrag(NULL, NULL, packets[16], NULL); if (reassembled == NULL) { goto end; } @@ -1552,7 +1569,7 @@ IPV6DefragDoSturgesNovakTest(int policy, u_char *expected, size_t expected_len) /* Send all but the last. */ for (i = 0; i < 9; i++) { - Packet *tp = Defrag(NULL, NULL, packets[i]); + Packet *tp = Defrag(NULL, NULL, packets[i], NULL); if (tp != NULL) { SCFree(tp); goto end; @@ -1563,7 +1580,7 @@ IPV6DefragDoSturgesNovakTest(int policy, u_char *expected, size_t expected_len) } int overlap = 0; for (; i < 16; i++) { - Packet *tp = Defrag(NULL, NULL, packets[i]); + Packet *tp = Defrag(NULL, NULL, packets[i], NULL); if (tp != NULL) { SCFree(tp); goto end; @@ -1576,7 +1593,7 @@ IPV6DefragDoSturgesNovakTest(int policy, u_char *expected, size_t expected_len) goto end; /* And now the last one. */ - Packet *reassembled = Defrag(NULL, NULL, packets[16]); + Packet *reassembled = Defrag(NULL, NULL, packets[16], NULL); if (reassembled == NULL) goto end; if (memcmp(GET_PKT_DATA(reassembled) + 40, expected, expected_len) != 0) @@ -2036,7 +2053,7 @@ DefragTimeoutTest(void) if (p == NULL) goto end; - Packet *tp = Defrag(NULL, NULL, p); + Packet *tp = Defrag(NULL, NULL, p, NULL); SCFree(p); @@ -2053,7 +2070,7 @@ DefragTimeoutTest(void) goto end; p->ts.tv_sec += (defrag_context->timeout + 1); - Packet *tp = Defrag(NULL, NULL, p); + Packet *tp = Defrag(NULL, NULL, p, NULL); if (tp != NULL) { SCFree(tp); @@ -2101,7 +2118,7 @@ DefragIPv4NoDataTest(void) goto end; /* We do not expect a packet returned. */ - if (Defrag(NULL, NULL, p) != NULL) + if (Defrag(NULL, NULL, p, NULL) != NULL) goto end; /* The fragment should have been ignored so no fragments should @@ -2140,7 +2157,7 @@ DefragIPv4TooLargeTest(void) goto end; /* We do not expect a packet returned. */ - if (Defrag(NULL, NULL, p) != NULL) + if (Defrag(NULL, NULL, p, NULL) != NULL) goto end; if (!ENGINE_ISSET_EVENT(p, IPV4_FRAG_PKT_TOO_LARGE)) goto end; @@ -2181,18 +2198,18 @@ DefragVlanTest(void) { goto end; /* With no VLAN IDs set, packets should re-assemble. */ - if ((r = Defrag(NULL, NULL, p1)) != NULL) + if ((r = Defrag(NULL, NULL, p1, NULL)) != NULL) goto end; - if ((r = Defrag(NULL, NULL, p2)) == NULL) + if ((r = Defrag(NULL, NULL, p2, NULL)) == NULL) goto end; SCFree(r); /* With mismatched VLANs, packets should not re-assemble. */ p1->vlan_id[0] = 1; p2->vlan_id[0] = 2; - if ((r = Defrag(NULL, NULL, p1)) != NULL) + if ((r = Defrag(NULL, NULL, p1, NULL)) != NULL) goto end; - if ((r = Defrag(NULL, NULL, p2)) != NULL) + if ((r = Defrag(NULL, NULL, p2, NULL)) != NULL) goto end; /* Pass. */ @@ -2226,9 +2243,9 @@ DefragVlanQinQTest(void) { goto end; /* With no VLAN IDs set, packets should re-assemble. */ - if ((r = Defrag(NULL, NULL, p1)) != NULL) + if ((r = Defrag(NULL, NULL, p1, NULL)) != NULL) goto end; - if ((r = Defrag(NULL, NULL, p2)) == NULL) + if ((r = Defrag(NULL, NULL, p2, NULL)) == NULL) goto end; SCFree(r); @@ -2237,9 +2254,9 @@ DefragVlanQinQTest(void) { p2->vlan_id[0] = 1; p1->vlan_id[1] = 1; p2->vlan_id[1] = 2; - if ((r = Defrag(NULL, NULL, p1)) != NULL) + if ((r = Defrag(NULL, NULL, p1, NULL)) != NULL) goto end; - if ((r = Defrag(NULL, NULL, p2)) != NULL) + if ((r = Defrag(NULL, NULL, p2, NULL)) != NULL) goto end; /* Pass. */ diff --git a/src/defrag.h b/src/defrag.h index cf5abe8724..dd4aac09bd 100644 --- a/src/defrag.h +++ b/src/defrag.h @@ -133,7 +133,7 @@ void DefragReload(void); /**< use only in unittests */ uint8_t DefragGetOsPolicy(Packet *); void DefragTrackerFreeFrags(DefragTracker *); -Packet *Defrag(ThreadVars *, DecodeThreadVars *, Packet *); +Packet *Defrag(ThreadVars *, DecodeThreadVars *, Packet *, PacketQueue *); void DefragRegisterTests(void); #endif /* __DEFRAG_H__ */ -- 2.47.2