From 136dad83f844694145bbd5f2e506147348878a94 Mon Sep 17 00:00:00 2001 From: Shivani Bhardwaj Date: Mon, 12 Jan 2026 13:56:54 +0530 Subject: [PATCH] detect/proto: check ipproto enabled setting first If the key `app-layer.protocols.PROTO.enabled` is present, the protocol is enabled for all carrier protocols. This is not ideal. Only if the key `app-layer.protocols.PROTO.enabled` is missing, an attempt is made to look for a setting specific to the ipproto passed at the time of registration e.g. `app-layer.protocols.PROTO.udp.enabled`. By default, check for carrier proto specific setting. If it is not found, then fall back to the generic setting. Issue a warning in case an inconsistent combination of global and ipproto specific setting is found. Bug 8205 --- doc/userguide/configuration/suricata-yaml.rst | 8 ++ doc/userguide/upgrade.rst | 4 + src/app-layer-detect-proto.c | 87 +++++++++++-------- src/app-layer-parser.c | 82 ++++++++++------- suricata.yaml.in | 2 + 5 files changed, 114 insertions(+), 69 deletions(-) diff --git a/doc/userguide/configuration/suricata-yaml.rst b/doc/userguide/configuration/suricata-yaml.rst index 0047d2a24ba2..e2ed82e0a6ee 100644 --- a/doc/userguide/configuration/suricata-yaml.rst +++ b/doc/userguide/configuration/suricata-yaml.rst @@ -1535,6 +1535,14 @@ the default behavior). Each supported protocol has a dedicated subsection under ``protocols``. +.. note:: All applayer parsers can be enabled or disabled for specific carrier + protocols. Suricata first looks for carrier protocol specific setting and + if not found, falls back to the common enabled setting. e.g. if ``sip`` is + being registered, Suricata will first look if ``app-layer.protocols.sip.tcp.enabled`` + and ``app-layer.protocols.sip.udp.enabled`` are set. If not, then a search would be + made for ``app-layer.protocols.sip.enabled`` and that setting would apply to both + SIP/TCP as well as SIP/UDP. + Asn1_max_frames ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/doc/userguide/upgrade.rst b/doc/userguide/upgrade.rst index 436151fe6aa4..1d99928edd2d 100644 --- a/doc/userguide/upgrade.rst +++ b/doc/userguide/upgrade.rst @@ -77,6 +77,10 @@ Other Changes really enforced and there will be no hassh computation even if rules try to use it. +- Any inconsistent protocol enable/disable settings will issue a warning and would + favor ipproto specific settings over protocol's global enable/disable settings. e.g. + ``app-layer.protocols.sip.tcp.enabled`` would be read and preferred over + ``app-layer.protocols.sip.enabled``. Upgrading to 8.0.1 ------------------ diff --git a/src/app-layer-detect-proto.c b/src/app-layer-detect-proto.c index 50365899991d..f2307fbe82b0 100644 --- a/src/app-layer-detect-proto.c +++ b/src/app-layer-detect-proto.c @@ -1916,67 +1916,80 @@ int SCAppLayerProtoDetectConfProtoDetectionEnabledDefault( BUG_ON(ipproto == NULL || alproto == NULL); - int enabled = 1; char param[100]; - SCConfNode *node; + SCConfNode *g_proto, *i_proto; int r; + bool g_enabled = false; + bool i_enabled = false; if (RunmodeIsUnittests()) - goto enabled; + SCReturnInt(1); #ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION // so that fuzzig takes place for DNP3 and such default_enabled = true; #endif - r = snprintf(param, sizeof(param), "%s%s%s", "app-layer.protocols.", - alproto, ".enabled"); + r = snprintf(param, sizeof(param), "%s%s%s%s%s", "app-layer.protocols.", alproto, ".", ipproto, + ".enabled"); if (r < 0) { FatalError("snprintf failure."); } else if (r > (int)sizeof(param)) { FatalError("buffer not big enough to write param."); } + SCLogDebug("Looking for %s", param); - node = SCConfGetNode(param); - if (node == NULL) { - SCLogDebug("Entry for %s not found.", param); - r = snprintf(param, sizeof(param), "%s%s%s%s%s", "app-layer.protocols.", - alproto, ".", ipproto, ".enabled"); - if (r < 0) { - FatalError("snprintf failure."); - } else if (r > (int)sizeof(param)) { - FatalError("buffer not big enough to write param."); + i_proto = SCConfGetNode(param); + if (i_proto && i_proto->val) { + if (SCConfValIsTrue(i_proto->val)) { + i_enabled = true; + } else if (SCConfValIsFalse(i_proto->val)) { + i_enabled = false; + } else if (strcasecmp(i_proto->val, "detection-only") == 0) { + i_enabled = true; + } else { + FatalError("Invalid value found for %s.", param); } + } - node = SCConfGetNode(param); - if (node == NULL) { - SCLogDebug("Entry for %s not found.", param); - if (default_enabled) { - goto enabled; - } else { - goto disabled; - } - } + r = snprintf(param, sizeof(param), "%s%s%s", "app-layer.protocols.", alproto, ".enabled"); + if (r < 0) { + FatalError("snprintf failure."); + } else if (r > (int)sizeof(param)) { + FatalError("buffer not big enough to write param."); } - if (node->val) { - if (SCConfValIsTrue(node->val)) { - goto enabled; - } else if (SCConfValIsFalse(node->val)) { - goto disabled; - } else if (strcasecmp(node->val, "detection-only") == 0) { - goto enabled; + SCLogDebug("Looking for %s", param); + g_proto = SCConfGetNode(param); + if (g_proto && g_proto->val) { + if (SCConfValIsTrue(g_proto->val)) { + g_enabled = true; + } else if (SCConfValIsFalse(g_proto->val)) { + g_enabled = false; + } else if (strcasecmp(g_proto->val, "detection-only") == 0) { + g_enabled = true; + } else { + FatalError("Invalid value found for %s", param); } } - /* Invalid or null value. */ - SCLogError("Invalid value found for %s.", param); - exit(EXIT_FAILURE); + if ((i_proto && g_proto) && (i_enabled ^ g_enabled)) { + SCLogWarning("Inconsistent global (%s) and respective ipproto (%s) settings found for " + "alproto %s and ipproto %s", + g_enabled ? "TRUE" : "FALSE", i_enabled ? "TRUE" : "FALSE", alproto, ipproto); + } + + if (i_proto) { + SCReturnInt(i_enabled); + } + if (g_proto) { + SCReturnInt(g_enabled); + } + if (!default_enabled) { + SCReturnInt(0); + } - disabled: - enabled = 0; - enabled: - SCReturnInt(enabled); + SCReturnInt(1); } int SCAppLayerProtoDetectConfProtoDetectionEnabled(const char *ipproto, const char *alproto) diff --git a/src/app-layer-parser.c b/src/app-layer-parser.c index 0ab84a8269de..4a47acacff85 100644 --- a/src/app-layer-parser.c +++ b/src/app-layer-parser.c @@ -346,55 +346,73 @@ int SCAppLayerParserConfParserEnabled(const char *ipproto, const char *alproto_n { SCEnter(); - int enabled = 1; char param[100]; - SCConfNode *node; + SCConfNode *g_proto, *i_proto; + bool g_enabled = false; + bool i_enabled = false; int r; if (RunmodeIsUnittests()) - goto enabled; + SCReturnInt(1); - r = snprintf(param, sizeof(param), "%s%s%s", "app-layer.protocols.", - alproto_name, ".enabled"); + r = snprintf(param, sizeof(param), "%s%s%s%s%s", "app-layer.protocols.", alproto_name, ".", + ipproto, ".enabled"); if (r < 0) { FatalError("snprintf failure."); } else if (r > (int)sizeof(param)) { FatalError("buffer not big enough to write param."); } - - node = SCConfGetNode(param); - if (node == NULL) { - SCLogDebug("Entry for %s not found.", param); - r = snprintf(param, sizeof(param), "%s%s%s%s%s", "app-layer.protocols.", - alproto_name, ".", ipproto, ".enabled"); - if (r < 0) { - FatalError("snprintf failure."); - } else if (r > (int)sizeof(param)) { - FatalError("buffer not big enough to write param."); + SCLogDebug("Looking for %s", param); + + i_proto = SCConfGetNode(param); + if (i_proto && i_proto->val) { + if (SCConfValIsTrue(i_proto->val)) { + i_enabled = true; + } else if (SCConfValIsFalse(i_proto->val)) { + i_enabled = false; + } else if (strcasecmp(i_proto->val, "detection-only") == 0) { + i_enabled = false; + } else { + FatalError("Invalid value found for %s.", param); } + } - node = SCConfGetNode(param); - if (node == NULL) { - SCLogDebug("Entry for %s not found.", param); - goto enabled; + r = snprintf(param, sizeof(param), "%s%s%s", "app-layer.protocols.", alproto_name, ".enabled"); + if (r < 0) { + FatalError("snprintf failure."); + } else if (r > (int)sizeof(param)) { + FatalError("buffer not big enough to write param."); + } + + SCLogDebug("Looking for %s", param); + g_proto = SCConfGetNode(param); + if (g_proto && g_proto->val) { + if (SCConfValIsTrue(g_proto->val)) { + g_enabled = true; + } else if (SCConfValIsFalse(g_proto->val)) { + g_enabled = false; + } else if (strcasecmp(g_proto->val, "detection-only") == 0) { + g_enabled = false; + } else { + FatalError("Invalid value found for %s", param); } } - if (SCConfValIsTrue(node->val)) { - goto enabled; - } else if (SCConfValIsFalse(node->val)) { - goto disabled; - } else if (strcasecmp(node->val, "detection-only") == 0) { - goto disabled; - } else { - SCLogError("Invalid value found for %s.", param); - exit(EXIT_FAILURE); + if ((i_proto && g_proto) && (i_enabled ^ g_enabled)) { + /* these checks are also performed for detection-only, no need to issue double warning */ + SCLogDebug("Inconsistent global (%s) and respective ipproto (%s) settings found for " + "alproto %s and ipproto %s", + g_enabled ? "TRUE" : "FALSE", i_enabled ? "TRUE" : "FALSE", alproto_name, ipproto); + } + + if (i_proto) { + SCReturnInt(i_enabled); + } + if (g_proto) { + SCReturnInt(g_enabled); } - disabled: - enabled = 0; - enabled: - SCReturnInt(enabled); + SCReturnInt(1); } /***** Parser related registration *****/ diff --git a/suricata.yaml.in b/suricata.yaml.in index 3b29513f9dfb..2df0499fae76 100644 --- a/suricata.yaml.in +++ b/suricata.yaml.in @@ -923,6 +923,8 @@ pcap-file: # The option "enabled" takes 3 values - "yes", "no", "detection-only". # "yes" enables both detection and the parser, "no" disables both, and # "detection-only" enables protocol detection only (parser disabled). +# This option is first evaluated per carrier protocol e.g. sip.tcp.enabled will be +# evaluated first and if it is not found then, sip.enabled will be checked. app-layer: # error-policy: ignore protocols: