From edd163252d9fb49fea7da12dea4761999e78a975 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Wed, 17 Jul 2019 15:19:30 +0200 Subject: [PATCH 1/8] protodetect: be more tolerant Do not mask protocols on both directions with only first packet For instance : When the first packet is no valid DNS but on port 53 (a junk request) second packet (error response from server) does not get checked for DNS as first packet bit masked away DNS for both directions Ticket: #2757 --- src/app-layer-detect-proto.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/app-layer-detect-proto.c b/src/app-layer-detect-proto.c index b61d3c924d5b..3eb2752850e3 100644 --- a/src/app-layer-detect-proto.c +++ b/src/app-layer-detect-proto.c @@ -556,7 +556,11 @@ static AppProto AppLayerProtoDetectPPGetProto(Flow *f, const uint8_t *buf, uint3 } else { /* first try the destination port */ pp_port_dp = AppLayerProtoDetectGetProbingParsers(alpd_ctx.ctx_pp, ipproto, dp); - alproto_masks = &f->probing_parser_toclient_alproto_masks; + if (dir == idir) { + // do not update alproto_masks to let a chance to second packet + // for instance when sending a junk packet to a DNS server + alproto_masks = &f->probing_parser_toclient_alproto_masks; + } if (pp_port_dp != NULL) { SCLogDebug("toclient - Probing parser found for destination port %"PRIu16, dp); From 3051f7f23f245fc2029a7c61df89bb8d12f9a2ae Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Wed, 17 Jul 2019 15:21:13 +0200 Subject: [PATCH 2/8] protodetect: use both directions over UDP As is already done for TCP Ticket: #2757 --- src/app-layer.c | 77 +++++++++++++++++++++++++++++++++++++------------ 1 file changed, 59 insertions(+), 18 deletions(-) diff --git a/src/app-layer.c b/src/app-layer.c index eb4ce8aca40e..87a2d12d5e2f 100644 --- a/src/app-layer.c +++ b/src/app-layer.c @@ -685,8 +685,9 @@ int AppLayerHandleTCPData(ThreadVars *tv, TcpReassemblyThreadCtx *ra_ctx, /* if we don't know the proto yet and we have received a stream * initializer message, we run proto detection. - * We receive 2 stream init msgs (one for each direction) but we - * only run the proto detection once. */ + * We receive 2 stream init msgs (one for each direction), we + * only run the proto detection for both and emit an event + * in the case protocols mismatch. */ if (alproto == ALPROTO_UNKNOWN && (flags & STREAM_START)) { DEBUG_VALIDATE_BUG_ON(FlowChangeProto(f)); /* run protocol detection */ @@ -785,8 +786,10 @@ int AppLayerHandleTCPData(ThreadVars *tv, TcpReassemblyThreadCtx *ra_ctx, int AppLayerHandleUdp(ThreadVars *tv, AppLayerThreadCtx *tctx, Packet *p, Flow *f) { SCEnter(); + AppProto *alproto; + AppProto *alproto_otherdir; - if (f->alproto == ALPROTO_FAILED) { + if (f->alproto_ts == ALPROTO_FAILED && f->alproto_tc == ALPROTO_FAILED) { SCReturnInt(0); } @@ -794,33 +797,75 @@ int AppLayerHandleUdp(ThreadVars *tv, AppLayerThreadCtx *tctx, Packet *p, Flow * uint8_t flags = 0; if (p->flowflags & FLOW_PKT_TOSERVER) { flags |= STREAM_TOSERVER; + alproto = &f->alproto_ts; + alproto_otherdir = &f->alproto_tc; } else { flags |= STREAM_TOCLIENT; + alproto = &f->alproto_tc; + alproto_otherdir = &f->alproto_ts; } AppLayerProfilingReset(tctx); /* if the protocol is still unknown, run detection */ - if (f->alproto == ALPROTO_UNKNOWN) { + if (*alproto == ALPROTO_UNKNOWN) { SCLogDebug("Detecting AL proto on udp mesg (len %" PRIu32 ")", p->payload_len); bool reverse_flow = false; PACKET_PROFILING_APP_PD_START(tctx); - f->alproto = AppLayerProtoDetectGetProto(tctx->alpd_tctx, - f, p->payload, p->payload_len, - IPPROTO_UDP, flags, &reverse_flow); + *alproto = AppLayerProtoDetectGetProto( + tctx->alpd_tctx, f, p->payload, p->payload_len, IPPROTO_UDP, flags, &reverse_flow); PACKET_PROFILING_APP_PD_END(tctx); - if (f->alproto != ALPROTO_UNKNOWN) { - AppLayerIncFlowCounter(tv, f); - - if (p->flowflags & FLOW_PKT_TOSERVER) { - f->alproto_ts = f->alproto; - } else { - f->alproto_tc = f->alproto; + switch (*alproto) { + case ALPROTO_UNKNOWN: + if (*alproto_otherdir != ALPROTO_UNKNOWN) { + // Use recognized side + f->alproto = *alproto_otherdir; + // do not keep ALPROTO_UNKNOWN for this side so as not to loop + *alproto = *alproto_otherdir; + if (*alproto_otherdir == ALPROTO_FAILED) { + SCLogDebug("ALPROTO_UNKNOWN flow %p", f); + } + } else { + // First side of protocol is unknown + *alproto = ALPROTO_FAILED; + } + break; + case ALPROTO_FAILED: + if (*alproto_otherdir != ALPROTO_UNKNOWN) { + // Use recognized side + f->alproto = *alproto_otherdir; + if (*alproto_otherdir == ALPROTO_FAILED) { + SCLogDebug("ALPROTO_UNKNOWN flow %p", f); + } + } + // else wait for second side of protocol + break; + default: + if (*alproto_otherdir != ALPROTO_UNKNOWN && *alproto_otherdir != ALPROTO_FAILED) { + if (*alproto_otherdir != *alproto) { + AppLayerDecoderEventsSetEventRaw( + &p->app_layer_events, APPLAYER_MISMATCH_PROTOCOL_BOTH_DIRECTIONS); + // data already sent to parser, we cannot change the protocol to use the one + // of the server + } + } else { + f->alproto = *alproto; + } + } + if (*alproto_otherdir == ALPROTO_UNKNOWN) { + if (f->alproto == ALPROTO_UNKNOWN) { + // so as to increase stat about .app_layer.flow.failed_udp + f->alproto = ALPROTO_FAILED; } + // If the other side is unknown, this is the first packet of the flow + AppLayerIncFlowCounter(tv, f); + } + // parse the data if we recognized one protocol + if (f->alproto != ALPROTO_UNKNOWN && f->alproto != ALPROTO_FAILED) { if (reverse_flow) { SCLogDebug("reversing flow after proto detect told us so"); PacketSwap(p); @@ -832,10 +877,6 @@ int AppLayerHandleUdp(ThreadVars *tv, AppLayerThreadCtx *tctx, Packet *p, Flow * r = AppLayerParserParse(tv, tctx->alp_tctx, f, f->alproto, flags, p->payload, p->payload_len); PACKET_PROFILING_APP_END(tctx, f->alproto); - } else { - f->alproto = ALPROTO_FAILED; - AppLayerIncFlowCounter(tv, f); - SCLogDebug("ALPROTO_UNKNOWN flow %p", f); } PACKET_PROFILING_APP_STORE(tctx, p); /* we do only inspection in one direction, so flag both From 90573dc9d4d51cc44bbd4ff7517f839dbe982cda Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 20 May 2022 19:32:03 +0000 Subject: [PATCH 3/8] github-actions: bump actions/upload-artifact from 3.0.0 to 3.1.0 Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 3.0.0 to 3.1.0. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](https://github.com/actions/upload-artifact/compare/6673cd052c4cd6fcf4b4e6e60ea986c889389535...3cea5372237819ed00197afe530f5a7ea3e805c8) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- .github/workflows/builds.yml | 6 +++--- .github/workflows/cifuzz.yml | 2 +- .github/workflows/commits.yml | 2 +- .github/workflows/scorecards-analysis.yml | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/builds.yml b/.github/workflows/builds.yml index f52bcd340b45..c78426df1476 100644 --- a/.github/workflows/builds.yml +++ b/.github/workflows/builds.yml @@ -113,7 +113,7 @@ jobs: - name: Cleaning up run: rm -rf libhtp suricata-update suricata-verify - name: Uploading prep archive - uses: actions/upload-artifact@6673cd052c4cd6fcf4b4e6e60ea986c889389535 + uses: actions/upload-artifact@3cea5372237819ed00197afe530f5a7ea3e805c8 with: name: prep path: . @@ -137,7 +137,7 @@ jobs: cargo install --target x86_64-unknown-linux-musl --debug cbindgen cp $HOME/.cargo/bin/cbindgen . - name: Uploading prep archive - uses: actions/upload-artifact@6673cd052c4cd6fcf4b4e6e60ea986c889389535 + uses: actions/upload-artifact@3cea5372237819ed00197afe530f5a7ea3e805c8 with: name: prep path: . @@ -249,7 +249,7 @@ jobs: run: | mkdir dist mv suricata-*.tar.gz dist - - uses: actions/upload-artifact@6673cd052c4cd6fcf4b4e6e60ea986c889389535 + - uses: actions/upload-artifact@3cea5372237819ed00197afe530f5a7ea3e805c8 name: Uploading distribution with: name: dist diff --git a/.github/workflows/cifuzz.yml b/.github/workflows/cifuzz.yml index b567d760125f..63db455cdce6 100644 --- a/.github/workflows/cifuzz.yml +++ b/.github/workflows/cifuzz.yml @@ -23,7 +23,7 @@ jobs: dry-run: false sanitizer: ${{ matrix.sanitizer }} - name: Upload Crash - uses: actions/upload-artifact@6673cd052c4cd6fcf4b4e6e60ea986c889389535 + uses: actions/upload-artifact@3cea5372237819ed00197afe530f5a7ea3e805c8 if: failure() with: name: ${{ matrix.sanitizer }}-artifacts diff --git a/.github/workflows/commits.yml b/.github/workflows/commits.yml index 478b2056388f..4d1a91f292d2 100644 --- a/.github/workflows/commits.yml +++ b/.github/workflows/commits.yml @@ -93,7 +93,7 @@ jobs: make -ik distclean > /dev/null done - run: sccache -s - - uses: actions/upload-artifact@6673cd052c4cd6fcf4b4e6e60ea986c889389535 + - uses: actions/upload-artifact@3cea5372237819ed00197afe530f5a7ea3e805c8 name: Uploading build log if: always() with: diff --git a/.github/workflows/scorecards-analysis.yml b/.github/workflows/scorecards-analysis.yml index fecdc748cfed..adf589540735 100644 --- a/.github/workflows/scorecards-analysis.yml +++ b/.github/workflows/scorecards-analysis.yml @@ -42,7 +42,7 @@ jobs: # Upload the results as artifacts (optional). - name: "Upload artifact" - uses: actions/upload-artifact@6673cd052c4cd6fcf4b4e6e60ea986c889389535 # v2.3.1 + uses: actions/upload-artifact@3cea5372237819ed00197afe530f5a7ea3e805c8 # v2.3.1 with: name: SARIF file path: results.sarif From 477a6f3dd2decb7c58820c8e534855227dbc26ed Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 17 May 2022 19:43:53 +0000 Subject: [PATCH 4/8] github-actions: bump github/codeql-action from 2.1.9 to 2.1.11 Bumps [github/codeql-action](https://github.com/github/codeql-action) from 2.1.9 to 2.1.11. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/github/codeql-action/compare/7502d6e991ca767d2db617bfd823a1ed925a0d59...a3a6c128d771b6b9bdebb1c9d0583ebd2728a108) --- updated-dependencies: - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- .github/workflows/scorecards-analysis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/scorecards-analysis.yml b/.github/workflows/scorecards-analysis.yml index adf589540735..fb7888ece313 100644 --- a/.github/workflows/scorecards-analysis.yml +++ b/.github/workflows/scorecards-analysis.yml @@ -50,6 +50,6 @@ jobs: # Upload the results to GitHub's code scanning dashboard. - name: "Upload to code-scanning" - uses: github/codeql-action/upload-sarif@7502d6e991ca767d2db617bfd823a1ed925a0d59 # v1.0.26 + uses: github/codeql-action/upload-sarif@a3a6c128d771b6b9bdebb1c9d0583ebd2728a108 # v1.0.26 with: sarif_file: results.sarif From d745d28d4a400d08609985f453c675f2a1a1bcc6 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Fri, 13 May 2022 09:43:11 +0200 Subject: [PATCH 5/8] dcerpc: use vecdeque tx iterator Ticket: #5321 --- rust/src/dcerpc/dcerpc.rs | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/rust/src/dcerpc/dcerpc.rs b/rust/src/dcerpc/dcerpc.rs index 7aae120ffc5b..a70bcb64f866 100644 --- a/rust/src/dcerpc/dcerpc.rs +++ b/rust/src/dcerpc/dcerpc.rs @@ -15,7 +15,7 @@ * 02110-1301, USA. */ -use crate::applayer::*; +use crate::applayer::{self, *}; use crate::core::{self, *}; use crate::dcerpc::parser; use nom7::error::{Error, ErrorKind}; @@ -186,6 +186,13 @@ pub struct DCERPCTransaction { pub tx_data: AppLayerTxData, } +impl Transaction for DCERPCTransaction { + fn id(&self) -> u64 { + // need +1 to match state.tx_id + self.id + 1 + } +} + impl DCERPCTransaction { pub fn new() -> Self { return Self { @@ -316,6 +323,16 @@ pub struct DCERPCState { pub flow: Option<*const core::Flow>, } +impl State for DCERPCState { + fn get_transaction_count(&self) -> usize { + self.transactions.len() + } + + fn get_transaction_by_index(&self, index: usize) -> Option<&DCERPCTransaction> { + self.transactions.get(index) + } +} + impl DCERPCState { pub fn new() -> Self { return Self { @@ -1349,7 +1366,7 @@ pub unsafe extern "C" fn rs_dcerpc_register_parser() { localstorage_new: None, localstorage_free: None, get_files: None, - get_tx_iterator: None, + get_tx_iterator: Some(applayer::state_get_tx_iterator::), get_tx_data: rs_dcerpc_get_tx_data, apply_tx_config: None, flags: APP_LAYER_PARSER_OPT_ACCEPT_GAPS, From 28ac75b50594f464949c036bbb34ceff759bdc9c Mon Sep 17 00:00:00 2001 From: Juliana Fajardini Date: Mon, 9 May 2022 11:24:18 -0300 Subject: [PATCH 6/8] detect/alert: directly increment alerts.discarded In the unlikely case of AlertQueueExpand failure, we were incrementing the discarded alerts stats in AlertQueueAppend via the Packet member in the DetectEngineThreadCtx, which may not be initialized yet. Bug #5353 --- src/detect-engine-alert.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/detect-engine-alert.c b/src/detect-engine-alert.c index 98fe24c2dcc5..359c224de9d4 100644 --- a/src/detect-engine-alert.c +++ b/src/detect-engine-alert.c @@ -268,7 +268,7 @@ void AlertQueueAppend(DetectEngineThreadCtx *det_ctx, const Signature *s, Packet /* we must grow the alert queue */ if (pos == AlertQueueExpand(det_ctx)) { /* this means we failed to expand the queue */ - det_ctx->p->alerts.discarded++; + p->alerts.discarded++; return; } } From 3ea6572e22d9ffcb26d9d408a91a3c0a5291c847 Mon Sep 17 00:00:00 2001 From: Jason Ish Date: Wed, 4 May 2022 13:18:09 -0600 Subject: [PATCH 7/8] rules: use primary default-rule-path if set on command line When reloading rules, respect `--set default-rule-path=...` from the command line if set. Previously the rule reload would always take the default-rule-path from the configuration file, even if overrided on the command line. Issue: #1911 --- src/detect-engine-loader.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/detect-engine-loader.c b/src/detect-engine-loader.c index 4c2f4e58d43f..f6c6b74739b5 100644 --- a/src/detect-engine-loader.c +++ b/src/detect-engine-loader.c @@ -68,16 +68,22 @@ char *DetectLoadCompleteSigPath(const DetectEngineCtx *de_ctx, const char *sig_f return NULL; } - if (strlen(de_ctx->config_prefix) > 0) { + /* If we have a configuration prefix, only use it if the primary configuration node + * is not marked as final, as that means it was provided on the command line with + * a --set. */ + ConfNode *default_rule_path = ConfGetNode("default-rule-path"); + if ((!default_rule_path || !default_rule_path->final) && strlen(de_ctx->config_prefix) > 0) { snprintf(varname, sizeof(varname), "%s.default-rule-path", de_ctx->config_prefix); - } else { - snprintf(varname, sizeof(varname), "default-rule-path"); + default_rule_path = ConfGetNode(varname); + } + if (default_rule_path) { + defaultpath = default_rule_path->val; } /* Path not specified */ if (PathIsRelative(sig_file)) { - if (ConfGet(varname, &defaultpath) == 1) { + if (defaultpath) { SCLogDebug("Default path: %s", defaultpath); size_t path_len = sizeof(char) * (strlen(defaultpath) + strlen(sig_file) + 2); @@ -93,7 +99,7 @@ char *DetectLoadCompleteSigPath(const DetectEngineCtx *de_ctx, const char *sig_f strlcat(path, "/", path_len); #endif strlcat(path, sig_file, path_len); - } else { + } else { path = SCStrdup(sig_file); if (unlikely(path == NULL)) return NULL; From 49647ad1206cf3620b22c8d0b8819393345cebc6 Mon Sep 17 00:00:00 2001 From: Jason Ish Date: Mon, 23 May 2022 09:26:44 -0600 Subject: [PATCH 8/8] github-ci: bump fedora versions 35 -> 36 34 -> 35 33 -> 34 --- .github/workflows/builds.yml | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/.github/workflows/builds.yml b/.github/workflows/builds.yml index c78426df1476..4813abee5855 100644 --- a/.github/workflows/builds.yml +++ b/.github/workflows/builds.yml @@ -319,10 +319,10 @@ jobs: - run: tar xf prep/suricata-verify.tar.gz - run: python3 ./suricata-verify/run.py -q - fedora-35: - name: Fedora 35 (debug, clang, asan, wshadow, rust-strict) + fedora-36: + name: Fedora 36 (debug, clang, asan, wshadow, rust-strict) runs-on: ubuntu-latest - container: fedora:35 + container: fedora:36 needs: [prepare-deps, prepare-cbindgen] steps: @@ -406,10 +406,10 @@ jobs: - run: test -e /usr/local/bin/libsuricata-config - run: test ! -e /usr/local/lib/libsuricata.so - fedora-34: - name: Fedora 34 (debug, clang, asan, wshadow, rust-strict) + fedora-35: + name: Fedora 35 (debug, clang, asan, wshadow, rust-strict) runs-on: ubuntu-latest - container: fedora:34 + container: fedora:35 needs: [prepare-deps, prepare-cbindgen] steps: @@ -493,10 +493,10 @@ jobs: - run: test -e /usr/local/bin/libsuricata-config - run: test ! -e /usr/local/lib/libsuricata.so - fedora-33: - name: Fedora 33 (debug, clang, asan, wshadow, rust-strict) + fedora-34: + name: Fedora 34 (debug, clang, asan, wshadow, rust-strict) runs-on: ubuntu-latest - container: fedora:33 + container: fedora:34 needs: [prepare-deps, prepare-cbindgen] steps: @@ -567,10 +567,10 @@ jobs: - name: Running suricata-verify run: python3 ./suricata-verify/run.py -q - fedora-34-no-jansson: - name: Fedora 34 (no jansson) + fedora-35-no-jansson: + name: Fedora 35 (no jansson) runs-on: ubuntu-latest - container: fedora:34 + container: fedora:35 needs: [prepare-deps, prepare-cbindgen] steps: