Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions doc/userguide/devguide/extending/app-layer/transactions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,55 @@ And in Rust:
return 0;
}

Per-Transaction Byte Value Cache
================================

A rule can produce a value with ``byte_extract`` or ``byte_math`` in one
buffer and then use it in another — for example, pull a length out of
``http.request_header`` and feed it to a ``byte_test`` in ``file.data``.
For that to work, the value has to stick around long enough for the
consumer to see it, which isn't guaranteed: ``det_ctx->byte_values`` is
per-worker, shared across every transaction the worker touches, and it
gets clobbered easily.

Two common ways it gets clobbered:

- **Bidirectional (``=>``) rules with pipelined HTTP.** Suricata inspects
every transaction's toserver buffers before it touches any toclient
buffer, so a later TX's ``byte_extract`` happily overwrites an earlier
TX's value before the earlier TX's toclient consumer ever runs.
- **Unidirectional rules split across packets.** If the producer and
consumer sit at different progress levels, the consumer may not run
until a later packet. In between, another TX on the same worker can
land and stomp on ``det_ctx->byte_values``. The detect engine won't
re-run the producer's inspect engine either — once it's done, its ID
is recorded in the sig's ``inspect_flags`` and it's skipped on resume.

To keep values safe, the per-TX ``DetectEngineState`` carries its own
``byte_values`` array that mirrors ``det_ctx->byte_values``:

- **Save.** After ``byte_extract`` or ``byte_math`` runs, the value is
copied into the TX's cache.
- **Restore.** At the top of each content inspection pass
(``recursion.count == 0``), the cached values are copied into
``det_ctx->byte_values``. Each side keeps its own allocation so that
values remain stable across multiple packets.
- **Reset.** The cache is cleared on detect engine reload
(``ResetTxState``), since byte variable local IDs can shift with a new
ruleset. It's freed when the ``DetectEngineState`` is destroyed.

The cache is on for any rule that uses byte variables, unidirectional
included. Save only fires when an extraction actually runs, and restore
early-returns on an empty cache, so rules that never touch byte
variables don't pay a measurable cost.

Key functions:

- ``DetectEngineStateSaveByteValue()`` in ``detect-engine-state.c``
- ``DetectEngineStateRestoreByteValues()`` in ``detect-engine-state.c``
- ``DetectEngineContentInspectionRestoreByteValues()`` in ``detect-engine-content-inspection.c``
- ``DetectEngineStateSaveByteValueFromTx()`` in ``detect-engine-content-inspection.c``

Work In Progress changes
========================

Expand Down
4 changes: 0 additions & 4 deletions doc/userguide/rules/differences-from-snort.rst
Original file line number Diff line number Diff line change
Expand Up @@ -256,10 +256,6 @@ See :doc:`http-keywords` for all HTTP keywords.
``byte_extract`` from ``http_*`` buffers, including
``http_header`` which does not always work as expected in Snort.

- In Suricata, variables extracted using ``byte_extract`` must be used
in the same buffer, otherwise they will have the value "0" (zero). Snort
does allow cross-buffer byte extraction and usage.

- Be sure to always positively and negatively test Suricata rules that
use ``byte_extract`` and ``byte_test`` to verify that they
work as expected.
Expand Down
20 changes: 20 additions & 0 deletions doc/userguide/rules/payload-keywords.rst
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,12 @@ Format::
byte_test: 2, =, var, 13; \
msg:"Byte extract and byte math with byte test verification"; sid: 1;)

.. note::

Like ``byte_extract``, the result variable from ``byte_math`` can be
used across different buffers and direction boundaries. The computed
value is cached per transaction.


byte_jump
---------
Expand Down Expand Up @@ -778,6 +784,20 @@ Format::
flow:established,to_server; content:"\|00 FF\|"; \
byte_extract:2,0,cmp_ver,relative; content:"FooBar"; distance:0; byte_test:2,=,cmp_ver,0; sid:3;)

.. note::

Variables produced by ``byte_extract`` or ``byte_math`` can be used
across different buffers, including across direction boundaries in
bidirectional (``=>``) rules. Produced values are cached per transaction
so they persist when the engine switches between buffers or directions.
For example, a value extracted from ``http.uri`` (toserver) can be
consumed by ``byte_test`` in ``http.stat_code`` (toclient)::

alert http any any => any any \
(msg:"Cross-buffer byte_extract"; flow:established; \
http.uri; content:"/data/"; byte_extract:2,0,uri_val,relative,string,dec; \
http.stat_code; content:"2"; byte_test:1,>,uri_val,0,relative; sid:4;)

.. _keyword_entropy:

entropy
Expand Down
3 changes: 3 additions & 0 deletions rust/sys/src/sys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -953,6 +953,9 @@ pub type DetectEngineStateDirection = DetectEngineStateDirection_;
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub struct DetectEngineState_ {
pub dir_state: [DetectEngineStateDirection; 2usize],
#[doc = " Storage for byte_extract values across buffers (direction-independent)"]
pub byte_values: *mut u64,
pub byte_values_size: u32,
}
impl Default for DetectEngineState_ {
fn default() -> Self {
Expand Down
69 changes: 68 additions & 1 deletion src/detect-engine-content-inspection.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* Copyright (C) 2007-2023 Open Information Security Foundation
/* Copyright (C) 2007-2026 Open Information Security Foundation
*
* You can copy, redistribute or modify this Program under the terms of
* the GNU General Public License version 2 as published by the Free
Expand Down Expand Up @@ -30,9 +30,12 @@
#include "detect.h"
#include "detect-engine.h"
#include "detect-parse.h"
#include "detect-engine-state.h"

#include "rust.h"

#include "app-layer-parser.h"

#include "detect-asn1.h"
#include "detect-content.h"
#include "detect-pcre.h"
Expand Down Expand Up @@ -76,6 +79,48 @@ struct DetectEngineContentInspectionCtx {
} recursion;
};

static inline AppLayerTxData *DetectEngineContentInspectionRestoreByteValues(
DetectEngineThreadCtx *det_ctx, Flow *f)
{
if (!likely(det_ctx->tx_id_set && f != NULL && f->alstate != NULL))
return NULL;

void *tx = AppLayerParserGetTx(f->proto, f->alproto, f->alstate, det_ctx->tx_id);
if (!likely(tx != NULL))
return NULL;

AppLayerTxData *tx_data = AppLayerParserGetTxData(f->proto, f->alproto, tx);
if (tx_data != NULL && tx_data->de_state != NULL) {
DetectEngineStateRestoreByteValues(det_ctx, tx_data->de_state);
}

return tx_data;
}

static inline void DetectEngineStateSaveByteValueFromTx(DetectEngineThreadCtx *det_ctx, Flow *f,
AppLayerTxData **tx_data, uint32_t local_id, uint64_t value)
{
if (tx_data == NULL || det_ctx == NULL)
return;

if (*tx_data == NULL && likely(det_ctx->tx_id_set && f != NULL && f->alstate != NULL)) {
void *tx = AppLayerParserGetTx(f->proto, f->alproto, f->alstate, det_ctx->tx_id);
if (likely(tx != NULL)) {
*tx_data = AppLayerParserGetTxData(f->proto, f->alproto, tx);
}
}

if (likely(*tx_data != NULL)) {
if (unlikely((*tx_data)->de_state == NULL)) {
(*tx_data)->de_state = DetectEngineStateAlloc();
}
if (likely((*tx_data)->de_state != NULL)) {
DetectEngineStateSaveByteValue(
(*tx_data)->de_state, local_id, value, det_ctx->byte_values_len);
}
}
}

/**
* \brief Run the actual payload match functions
*
Expand Down Expand Up @@ -113,6 +158,14 @@ static int DetectEngineContentInspectionInternal(DetectEngineThreadCtx *det_ctx,
SCEnter();
KEYWORD_PROFILING_START;

/* Restore byte keyword values from detection state at the top level only.
* Only needed for bidirectional rules; unidirectional rules inspect all
* buffers together per-TX so det_ctx->byte_values is never clobbered. */
AppLayerTxData *tx_data = NULL;
if (ctx->recursion.count == 0 && (s->flags & SIG_FLAG_TXBOTHDIR)) {
tx_data = DetectEngineContentInspectionRestoreByteValues(det_ctx, f);
}

ctx->recursion.count++;
if (unlikely(ctx->recursion.count == ctx->recursion.limit)) {
KEYWORD_PROFILING_END(det_ctx, smd->type, 0);
Expand Down Expand Up @@ -594,6 +647,13 @@ static int DetectEngineContentInspectionInternal(DetectEngineThreadCtx *det_ctx,

SCLogDebug("[BE] Fetched value for index %d: %"PRIu64,
bed->local_id, det_ctx->byte_values[bed->local_id]);
/* Save byte keyword value to detection state for cross-buffer use.
* Only needed for bidirectional rules where toserver and toclient
* buffers are inspected separately across all TXs. */
if (s->flags & SIG_FLAG_TXBOTHDIR) {
DetectEngineStateSaveByteValueFromTx(
det_ctx, f, &tx_data, bed->local_id, det_ctx->byte_values[bed->local_id]);
}
goto match;

} else if (smd->type == DETECT_BYTEMATH) {
Expand Down Expand Up @@ -630,6 +690,13 @@ static int DetectEngineContentInspectionInternal(DetectEngineThreadCtx *det_ctx,

SCLogDebug("[BM] Fetched value for index %d: %"PRIu64,
bmd->local_id, det_ctx->byte_values[bmd->local_id]);
/* Save byte keyword value to detection state for cross-buffer use.
* Only needed for bidirectional rules where toserver and toclient
* buffers are inspected separately across all TXs. */
if (s->flags & SIG_FLAG_TXBOTHDIR) {
DetectEngineStateSaveByteValueFromTx(
det_ctx, f, &tx_data, bmd->local_id, det_ctx->byte_values[bmd->local_id]);
}
goto match;

} else if (smd->type == DETECT_BSIZE) {
Expand Down
70 changes: 69 additions & 1 deletion src/detect-engine-state.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* Copyright (C) 2007-2021 Open Information Security Foundation
/* Copyright (C) 2007-2026 Open Information Security Foundation
*
* You can copy, redistribute or modify this Program under the terms of
* the GNU General Public License version 2 as published by the Free
Expand Down Expand Up @@ -37,6 +37,7 @@
*/

#include "suricata-common.h"
#include "util-validate.h"

#include "decode.h"

Expand Down Expand Up @@ -171,6 +172,7 @@ void SCDetectEngineStateFree(DetectEngineState *state)
DeStateStore *store_next;
int i = 0;

SCFree(state->byte_values);
for (i = 0; i < 2; i++) {
store = state->dir_state[i].head;
while (store != NULL) {
Expand Down Expand Up @@ -235,6 +237,64 @@ void DetectRunStoreStateTx(
SCLogDebug("Stored for TX %"PRIu64, tx_id);
}

/**
* \brief Save a byte keyword value to detection state for cross-buffer use.
*
* \param state The detection engine state (must not be NULL)
* \param local_id The parse-time sequential index of the byte variable
* \param value The extracted value to save
*/
void DetectEngineStateSaveByteValue(
DetectEngineState *state, uint32_t local_id, uint64_t value, uint32_t byte_values_len)
{
DEBUG_VALIDATE_BUG_ON(state == NULL);
if (unlikely(state == NULL))
return;

DEBUG_VALIDATE_BUG_ON(local_id >= byte_values_len);
if (unlikely(local_id >= byte_values_len))
return;

/* Allocate the per-tx cache array on first use, matching det_ctx size */
if (unlikely(state->byte_values == NULL)) {
state->byte_values = SCCalloc(byte_values_len, sizeof(uint64_t));
if (unlikely(state->byte_values == NULL)) {
return;
}
state->byte_values_size = byte_values_len;
}

state->byte_values[local_id] = value;
SCLogDebug("Saved byte value: local_id=%u value=%" PRIu64, local_id, value);
}

/**
* \brief Retrieve byte keyword values from detection state
*
* \param det_ctx Thread detection context
* \param state The detection engine state
*/
void DetectEngineStateRestoreByteValues(DetectEngineThreadCtx *det_ctx, DetectEngineState *state)
{
DEBUG_VALIDATE_BUG_ON(state == NULL || det_ctx == NULL);
if (unlikely(state == NULL || det_ctx == NULL))
return;

if (state->byte_values == NULL || state->byte_values_size == 0)
return;

/* det_ctx->byte_values is preallocated at engine startup (detect-engine.c)
* and must never be NULL here. Both arrays are the same size. */
DEBUG_VALIDATE_BUG_ON(det_ctx->byte_values == NULL);
DEBUG_VALIDATE_BUG_ON(state->byte_values_size != det_ctx->byte_values_len);

/* Copy the per-tx cache into the worker scratch buffer. Each side keeps
* its own allocation so that values remain correct across multiple
* packets: a pointer swap would rotate ownership on every packet and
* leave each TX reading another TX's values on the second packet. */
memcpy(det_ctx->byte_values, state->byte_values, state->byte_values_size * sizeof(uint64_t));
}

static inline void ResetTxState(DetectEngineState *s)
{
if (s) {
Expand All @@ -249,6 +309,14 @@ static inline void ResetTxState(DetectEngineState *s)
s->dir_state[1].flags = 0;
/* reset 'cur' back to the list head */
s->dir_state[1].cur = s->dir_state[1].head;
/* Free byte_values on reload: local_ids and byte_values_len may
* change with the new ruleset, so the next Save reallocates at
* the current det_ctx->byte_values_len. */
if (s->byte_values != NULL) {
SCFree(s->byte_values);
s->byte_values = NULL;
s->byte_values_size = 0;
}
}
}

Expand Down
14 changes: 13 additions & 1 deletion src/detect-engine-state.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* Copyright (C) 2007-2013 Open Information Security Foundation
/* Copyright (C) 2007-2026 Open Information Security Foundation
*
* You can copy, redistribute or modify this Program under the terms of
* the GNU General Public License version 2 as published by the Free
Expand Down Expand Up @@ -34,6 +34,9 @@
#ifndef SURICATA_DETECT_ENGINE_STATE_H
#define SURICATA_DETECT_ENGINE_STATE_H

// forward declarations
typedef struct DetectEngineThreadCtx_ DetectEngineThreadCtx;

// forward declaration for bindgen
#define SigIntId uint32_t

Expand Down Expand Up @@ -94,6 +97,10 @@ typedef struct DetectEngineStateDirection_ {

typedef struct DetectEngineState_ {
DetectEngineStateDirection dir_state[2];

/** Storage for byte_extract values across buffers (direction-independent) */
uint64_t *byte_values;
uint32_t byte_values_size;
} DetectEngineState;

/**
Expand All @@ -110,6 +117,11 @@ DetectEngineState *DetectEngineStateAlloc(void);
*/
void SCDetectEngineStateFree(DetectEngineState *state);

void DetectEngineStateSaveByteValue(
DetectEngineState *state, uint32_t local_id, uint64_t value, uint32_t byte_values_len);

void DetectEngineStateRestoreByteValues(DetectEngineThreadCtx *det_ctx, DetectEngineState *state);

#endif /* SURICATA_DETECT_ENGINE_STATE_H */

/**
Expand Down
6 changes: 3 additions & 3 deletions src/detect-engine.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* Copyright (C) 2007-2022 Open Information Security Foundation
/* Copyright (C) 2007-2026 Open Information Security Foundation
*
* You can copy, redistribute or modify this Program under the terms of
* the GNU General Public License version 2 as published by the Free
Expand Down Expand Up @@ -3313,8 +3313,8 @@ static TmEcode ThreadCtxDoInit (DetectEngineCtx *de_ctx, DetectEngineThreadCtx *
AlertQueueInit(det_ctx);

/* byte_extract storage */
det_ctx->byte_values = SCMalloc(sizeof(*det_ctx->byte_values) *
(de_ctx->byte_extract_max_local_id + 1));
det_ctx->byte_values_len = de_ctx->byte_extract_max_local_id + 1;
det_ctx->byte_values = SCMalloc(sizeof(*det_ctx->byte_values) * det_ctx->byte_values_len);
if (det_ctx->byte_values == NULL) {
return TM_ECODE_FAILED;
}
Expand Down
Loading
Loading