diff --git a/rust/src/applayertemplate/template.rs b/rust/src/applayertemplate/template.rs index fdb9b3e774..636f759312 100644 --- a/rust/src/applayertemplate/template.rs +++ b/rust/src/applayertemplate/template.rs @@ -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)); } diff --git a/rust/src/dhcp/dhcp.rs b/rust/src/dhcp/dhcp.rs index 885f6b6216..15c06e29ac 100644 --- a/rust/src/dhcp/dhcp.rs +++ b/rust/src/dhcp/dhcp.rs @@ -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)); } diff --git a/rust/src/nfs/nfs.rs b/rust/src/nfs/nfs.rs index b2a66f4da0..f3eb59f513 100644 --- a/rust/src/nfs/nfs.rs +++ b/rust/src/nfs/nfs.rs @@ -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)); diff --git a/rust/src/smb/smb.rs b/rust/src/smb/smb.rs index 8c854dc04e..9f2f1d42e4 100644 --- a/rust/src/smb/smb.rs +++ b/rust/src/smb/smb.rs @@ -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)); diff --git a/src/app-layer-parser.c b/src/app-layer-parser.c index aab6372404..1b8fba9ab5 100644 --- a/src/app-layer-parser.c +++ b/src/app-layer-parser.c @@ -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. */ diff --git a/src/app-layer-smb-tcp-rust.c b/src/app-layer-smb-tcp-rust.c index a41658121c..3681c124be 100644 --- a/src/app-layer-smb-tcp-rust.c +++ b/src/app-layer-smb-tcp-rust.c @@ -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 */ diff --git a/src/output-tx.c b/src/output-tx.c index b061b89d03..33fb8786ac 100644 --- a/src/output-tx.c +++ b/src/output-tx.c @@ -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