Description
I have identified a bug in the EOF (End of Flow) handling logic in src/output-tx.c that prevents TLS-based sub-protocols using different logger registration types from working together properly.
Problem Details
When implementing TLS-based protocols with different logger registration approaches:
-
Condition-based loggers: Using [OutputRegisterTxSubModuleWithCondition](javascript:void(0)) (e.g., HTTPS/SMTPS/POP3S)
-
Progress-based loggers: Using [OutputRegisterTxSubModuleWithProgress](javascript:void(0)) (e.g., AnyDesk)
The current EOF handling logic in src/output-tx.c (lines ~290-320) has an issue:
c
if (eof) {
SCLogDebug("EOF, so log now");
} else {
// Only perform condition or progress checks when NOT at EOF
if (logger->LogCondition) {
// Condition check
} else {
// Progress check
}
}
This means that at EOF, ALL loggers bypass their condition or progress checks and output logs regardless of whether their specific criteria have been met. This prevents the intended coexistence of condition-based and progress-based loggers.
Expected Behavior
-
Condition-based loggers should still evaluate their conditions even at EOF
-
Progress-based loggers should still check their progress thresholds even at EOF
-
Only file-related loggers (LOGGER_FILE/LOGGER_FILEDATA) should bypass checks at EOF
Impact
This affects TLS-based protocol detection where:
-
Some protocols (like HTTPS/SMTPS/POP3S) use condition-based logging
-
Others (like AnyDesk) use progress-based logging for more accurate detection
-
At flow end, all protocols may inappropriately trigger, causing false positives
Proposed Solution
Modify the EOF handling logic to be more precise:
c
if (eof && (logger->logger_id == LOGGER_FILE || logger->logger_id == LOGGER_FILEDATA)) {
// Only file loggers should bypass checks at EOF
SCLogDebug("EOF: file-related logger, logging without checks");
} else if (logger->LogCondition) {
// Apply condition check regardless of EOF state
if (!logger->LogCondition(tv, p, alstate, tx, tx_id)) {
SCLogDebug("conditions not met, not logging");
goto next_logger;
}
} else {
// Apply progress check regardless of EOF state
if (tx_progress_tc < logger->tc_log_progress) {
SCLogDebug("progress not far enough, not logging");
goto next_logger;
}
if (tx_progress_ts < logger->ts_log_progress) {
SCLogDebug("progress not far enough, not logging");
goto next_logger;
}
}
This would ensure that condition-based and progress-based TLS loggers can coexist properly, with each respecting their own evaluation criteria even at EOF.
Environment
-
Suricata version: All versions including latest
-
Issue location:
src/output-tx.c, around lines 290-320 -
Affected functionality: TLS-based protocol detection with mixed logger types
Additional Information
This issue prevents the proper coexistence of different TLS-based protocol detection mechanisms. The current “log everything at EOF” approach is too broad and should be restricted to file logging only, allowing protocol-specific loggers to maintain their intended behavior.
/d/Work/source/suricata/suricata.git (main)$ git log -p -L 298,300:src/output-tx.ccommit 79499e4769799e6b4426cace062471a807d64d49Author: Victor Julien vjulien@oisf.netDate: Sat Feb 5 09:20:07 2022 +0100
app-layer: move files into transactions
Update APIs to store files in transactions instead of the per flow state.
Goal is to avoid the overhead of matching up files and transactions in
cases where there are many of both.
Update all protocol implementations to support this.
Update file logging logic to account for having files in transactions. Instead
of it acting separately on file containers, it is now tied into the
transaction logging.
Update the filestore keyword to consider a match if filestore output not
enabled.
diff --git a/src/output-tx.c b/src/output-tx.c— a/src/output-tx.c+++ b/src/output-tx.c@@ -157,0 +294,3 @@
+ if (eof) {
+ SCLogDebug("EOF, so log now");
+ } else {