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

app-layer: improve transaction cleanup handling

Browse Source 
The app layers with a custom iterator would skip a tx if during
the ..Cleanup() pass a transaction was removed.

Address this by storing the current index instead of the next
index. Also pass in the next "min_tx_id" to be incremented from
the last TX. Update loops to do this increment.

Also make sure that the min_id is properly updated if the last
TX is removed when out of order.

Finally add a SMB unittest to test this.

Reported by: Ilya Bakhtin
pull/3576/head
Victor Julien 7 years ago
parent d34e41068f
commit 0e40231189
7 changed files with 202 additions and 7 deletions
Show Stats Download Patch File Download Diff File

2
rust/src/applayertemplate/template.rs
Unescape Escape View File

@ -221,7 +221,7 @@ impl TemplateState {
index += 1;
continue;
}
*state = index as u64 + 1;
*state = index as u64;
return Some((tx, tx.tx_id - 1, (len - index) > 1));
}

2
rust/src/dhcp/dhcp.rs
Unescape Escape View File

@ -210,7 +210,7 @@ impl DHCPState {
index += 1;
continue;
}
*state = index as u64 + 1;
*state = index as u64;
return Some((tx, tx.tx_id - 1, (len - index) > 1));
}

5
rust/src/nfs/nfs.rs
Unescape Escape View File

@ -428,7 +428,10 @@ impl NFSState {
index += 1;
continue;
}
*state = index as u64 + 1;
// store current index in the state and not the next
// as transactions might be freed between now and the
// next time we are called.
*state = index as u64;
SCLogDebug!("returning tx_id {} has_next? {} (len {} index {}), tx {:?}",
tx.id - 1, (len - index) > 1, len, index, tx);
return Some((tx, tx.id - 1, (len - index) > 1));

5
rust/src/smb/smb.rs
Unescape Escape View File

@ -874,7 +874,10 @@ impl SMBState {
index += 1;
continue;
}
*state = index as u64 + 1;
// store current index in the state and not the next
// as transactions might be freed between now and the
// next time we are called.
*state = index as u64;
//SCLogDebug!("returning tx_id {} has_next? {} (len {} index {}), tx {:?}",
// tx.id - 1, (len - index) > 1, len, index, tx);
return Some((tx, tx.id - 1, (len - index) > 1));

20
src/app-layer-parser.c
Unescape Escape View File

@ -768,6 +768,7 @@ void AppLayerParserSetTransactionInspectId(const Flow *f, AppLayerParserState *p
}
if (!ires.has_next)
break;
idx++;
}
pstate->inspect_id[direction] = idx;
SCLogDebug("inspect_id now %"PRIu64, pstate->inspect_id[direction]);
@ -808,6 +809,7 @@ void AppLayerParserSetTransactionInspectId(const Flow *f, AppLayerParserState *p
}
if (!ires.has_next)
break;
idx++;
}
}
@ -901,7 +903,7 @@ void AppLayerParserTransactionsCleanup(Flow *f)
break;
void *tx = ires.tx_ptr;
i = ires.tx_id;
i = ires.tx_id; // actual tx id for the tx the IterFunc returned
SCLogDebug("%p/%"PRIu64" checking", tx, i);
@ -950,13 +952,25 @@ void AppLayerParserTransactionsCleanup(Flow *f)
SCLogDebug("%p/%"PRIu64" freed", tx, i);
/* if we didn't skip any tx so far, up the minimum */
SCLogDebug("i %"PRIu64", new_min %"PRIu64, i, new_min);
SCLogDebug("skipped? %s i %"PRIu64", new_min %"PRIu64, skipped ? "true" : "false", i, new_min);
if (!skipped)
new_min = i + 1;
SCLogDebug("final i %"PRIu64", new_min %"PRIu64, i, new_min);
next:
if (!ires.has_next)
if (!ires.has_next) {
/* this was the last tx. See if we skipped any. If not
* we removed all and can update the minimum to the max
* id. */
SCLogDebug("no next: cur tx i %"PRIu64", total %"PRIu64, i, total_txs);
if (!skipped) {
new_min = total_txs;
SCLogDebug("no next: cur tx i %"PRIu64", total %"PRIu64": "
"new_min updated to %"PRIu64, i, total_txs, new_min);
}
break;
}
i++;
}
/* see if we need to bring all trackers up to date. */

174
src/app-layer-smb-tcp-rust.c
Unescape Escape View File

@ -208,6 +208,10 @@ static SuricataFileContext sfc = { &sbcfg };
#define SMB_CONFIG_DEFAULT_STREAM_DEPTH 0
#ifdef UNITTESTS
static void SMBParserRegisterTests(void);
#endif
static uint32_t stream_depth = SMB_CONFIG_DEFAULT_STREAM_DEPTH;
void RegisterRustSMBTCPParsers(void)
@ -299,7 +303,177 @@ void RegisterRustSMBTCPParsers(void)
SCLogInfo("Parsed disabled for %s protocol. Protocol detection"
"still on.", proto_name);
}
#ifdef UNITTESTS
AppLayerParserRegisterProtocolUnittests(IPPROTO_TCP, ALPROTO_SMB, SMBParserRegisterTests);
#endif
return;
}
#ifdef UNITTESTS
#include "stream-tcp.h"
#include "util-unittest-helper.h"
/** \test multi transactions and cleanup */
static int SMBParserTxCleanupTest(void)
{
uint64_t ret[4];
AppLayerParserThreadCtx *alp_tctx = AppLayerParserThreadCtxAlloc();
FAIL_IF_NULL(alp_tctx);
StreamTcpInitConfig(TRUE);
TcpSession ssn;
memset(&ssn, 0, sizeof(ssn));
Flow *f = UTHBuildFlow(AF_INET, "1.2.3.4", "1.2.3.5", 1024, 445);
FAIL_IF_NULL(f);
f->protoctx = &ssn;
f->proto = IPPROTO_TCP;
f->alproto = ALPROTO_SMB;
char req_str[] ="\x00\x00\x00\x79\xfe\x53\x4d\x42\x40\x00\x01\x00\x00\x00\x00\x00" \
"\x05\x00\xe0\x1e\x10\x00\x00\x00\x00\x00\x00\x00\x0b\x00\x00\x00" \
"\x00\x00\x00\x00\x00\x00\x00\x00\x10\x72\xd2\x9f\x36\xc2\x08\x14" \
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
"\x00\x00\x00\x00\x39\x00\x00\x00\x02\x00\x00\x00\x00\x00\x00\x00" \
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x80\x00\x00\x00" \
"\x00\x00\x00\x00\x07\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00" \
"\x78\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00";
req_str[28] = 0x01;
int r = AppLayerParserParse(NULL, alp_tctx, f, ALPROTO_SMB,
STREAM_TOSERVER | STREAM_START, (uint8_t *)req_str, sizeof(req_str));
FAIL_IF_NOT(r == 0);
req_str[28]++;
r = AppLayerParserParse(NULL, alp_tctx, f, ALPROTO_SMB,
STREAM_TOSERVER, (uint8_t *)req_str, sizeof(req_str));
FAIL_IF_NOT(r == 0);
req_str[28]++;
r = AppLayerParserParse(NULL, alp_tctx, f, ALPROTO_SMB,
STREAM_TOSERVER, (uint8_t *)req_str, sizeof(req_str));
FAIL_IF_NOT(r == 0);
req_str[28]++;
r = AppLayerParserParse(NULL, alp_tctx, f, ALPROTO_SMB,
STREAM_TOSERVER, (uint8_t *)req_str, sizeof(req_str));
FAIL_IF_NOT(r == 0);
req_str[28]++;
r = AppLayerParserParse(NULL, alp_tctx, f, ALPROTO_SMB,
STREAM_TOSERVER, (uint8_t *)req_str, sizeof(req_str));
FAIL_IF_NOT(r == 0);
req_str[28]++;
r = AppLayerParserParse(NULL, alp_tctx, f, ALPROTO_SMB,
STREAM_TOSERVER, (uint8_t *)req_str, sizeof(req_str));
FAIL_IF_NOT(r == 0);
req_str[28]++;
r = AppLayerParserParse(NULL, alp_tctx, f, ALPROTO_SMB,
STREAM_TOSERVER, (uint8_t *)req_str, sizeof(req_str));
FAIL_IF_NOT(r == 0);
req_str[28]++;
r = AppLayerParserParse(NULL, alp_tctx, f, ALPROTO_SMB,
STREAM_TOSERVER, (uint8_t *)req_str, sizeof(req_str));
FAIL_IF_NOT(r == 0);
req_str[28]++;
AppLayerParserTransactionsCleanup(f);
UTHAppLayerParserStateGetIds(f->alparser, &ret[0], &ret[1], &ret[2], &ret[3]);
FAIL_IF_NOT(ret[0] == 0); // inspect_id[0]
FAIL_IF_NOT(ret[1] == 0); // inspect_id[1]
FAIL_IF_NOT(ret[2] == 0); // log_id
FAIL_IF_NOT(ret[3] == 0); // min_id
char resp_str[] = "\x00\x00\x00\x98\xfe\x53\x4d\x42\x40\x00\x01\x00\x00\x00\x00\x00" \
"\x05\x00\x21\x00\x11\x00\x00\x00\x00\x00\x00\x00\x0b\x00\x00\x00" \
"\x00\x00\x00\x00\x00\x00\x00\x00\x10\x72\xd2\x9f\x36\xc2\x08\x14" \
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" \
"\x00\x00\x00\x00\x59\x00\x00\x00\x01\x00\x00\x00\x48\x38\x40\xb3" \
"\x0f\xa8\xd3\x01\x84\x9a\x2b\x46\xf7\xa8\xd3\x01\x48\x38\x40\xb3" \
"\x0f\xa8\xd3\x01\x48\x38\x40\xb3\x0f\xa8\xd3\x01\x00\x00\x00\x00" \
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x10\x00\x00\x00" \
"\x00\x00\x00\x00\x9e\x8f\xb8\x91\x00\x00\x00\x00\x01\x5b\x11\xbb" \
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00";
resp_str[28] = 0x01;
r = AppLayerParserParse(NULL, alp_tctx, f, ALPROTO_SMB,
STREAM_TOCLIENT | STREAM_START, (uint8_t *)resp_str, sizeof(resp_str));
FAIL_IF_NOT(r == 0);
resp_str[28] = 0x04;
r = AppLayerParserParse(NULL, alp_tctx, f, ALPROTO_SMB,
STREAM_TOCLIENT, (uint8_t *)resp_str, sizeof(resp_str));
FAIL_IF_NOT(r == 0);
resp_str[28] = 0x05;
r = AppLayerParserParse(NULL, alp_tctx, f, ALPROTO_SMB,
STREAM_TOCLIENT, (uint8_t *)resp_str, sizeof(resp_str));
FAIL_IF_NOT(r == 0);
resp_str[28] = 0x06;
r = AppLayerParserParse(NULL, alp_tctx, f, ALPROTO_SMB,
STREAM_TOCLIENT, (uint8_t *)resp_str, sizeof(resp_str));
FAIL_IF_NOT(r == 0);
resp_str[28] = 0x08;
r = AppLayerParserParse(NULL, alp_tctx, f, ALPROTO_SMB,
STREAM_TOCLIENT, (uint8_t *)resp_str, sizeof(resp_str));
FAIL_IF_NOT(r == 0);
resp_str[28] = 0x02;
r = AppLayerParserParse(NULL, alp_tctx, f, ALPROTO_SMB,
STREAM_TOCLIENT, (uint8_t *)resp_str, sizeof(resp_str));
FAIL_IF_NOT(r == 0);
resp_str[28] = 0x07;
r = AppLayerParserParse(NULL, alp_tctx, f, ALPROTO_SMB,
STREAM_TOCLIENT, (uint8_t *)resp_str, sizeof(resp_str));
FAIL_IF_NOT(r == 0);
AppLayerParserTransactionsCleanup(f);
UTHAppLayerParserStateGetIds(f->alparser, &ret[0], &ret[1], &ret[2], &ret[3]);
FAIL_IF_NOT(ret[0] == 2); // inspect_id[0]
FAIL_IF_NOT(ret[1] == 2); // inspect_id[1]
FAIL_IF_NOT(ret[2] == 2); // log_id
FAIL_IF_NOT(ret[3] == 2); // min_id
resp_str[28] = 0x03;
r = AppLayerParserParse(NULL, alp_tctx, f, ALPROTO_SMB,
STREAM_TOCLIENT, (uint8_t *)resp_str, sizeof(resp_str));
FAIL_IF_NOT(r == 0);
AppLayerParserTransactionsCleanup(f);
UTHAppLayerParserStateGetIds(f->alparser, &ret[0], &ret[1], &ret[2], &ret[3]);
FAIL_IF_NOT(ret[0] == 8); // inspect_id[0]
FAIL_IF_NOT(ret[1] == 8); // inspect_id[1]
FAIL_IF_NOT(ret[2] == 8); // log_id
FAIL_IF_NOT(ret[3] == 8); // min_id
req_str[28] = 0x09;
r = AppLayerParserParse(NULL, alp_tctx, f, ALPROTO_SMB,
STREAM_TOSERVER | STREAM_EOF, (uint8_t *)req_str, sizeof(req_str));
FAIL_IF_NOT(r == 0);
AppLayerParserTransactionsCleanup(f);
UTHAppLayerParserStateGetIds(f->alparser, &ret[0], &ret[1], &ret[2], &ret[3]);
FAIL_IF_NOT(ret[0] == 8); // inspect_id[0] not updated by ..Cleanup() until full tx is done
FAIL_IF_NOT(ret[1] == 8); // inspect_id[1]
FAIL_IF_NOT(ret[2] == 8); // log_id
FAIL_IF_NOT(ret[3] == 8); // min_id
resp_str[28] = 0x09;
r = AppLayerParserParse(NULL, alp_tctx, f, ALPROTO_SMB,
STREAM_TOCLIENT | STREAM_EOF, (uint8_t *)resp_str, sizeof(resp_str));
FAIL_IF_NOT(r == 0);
AppLayerParserTransactionsCleanup(f);
UTHAppLayerParserStateGetIds(f->alparser, &ret[0], &ret[1], &ret[2], &ret[3]);
FAIL_IF_NOT(ret[0] == 9); // inspect_id[0]
FAIL_IF_NOT(ret[1] == 9); // inspect_id[1]
FAIL_IF_NOT(ret[2] == 9); // log_id
FAIL_IF_NOT(ret[3] == 9); // min_id
AppLayerParserThreadCtxFree(alp_tctx);
StreamTcpFreeConfig(TRUE);
UTHFreeFlow(f);
PASS;
}
static void SMBParserRegisterTests(void)
{
UtRegisterTest("SMBParserTxCleanupTest", SMBParserTxCleanupTest);
}
#endif /* UNITTESTS */
#endif /* HAVE_RUST */

1
src/output-tx.c
Unescape Escape View File

@ -270,6 +270,7 @@ next_logger:
next_tx:
if (!ires.has_next)
break;
tx_id++;
}
/* Update the the last ID that has been logged with all

Write Preview
Loading…
Cancel
Save