From ad02040860cf5d973460ef7e76d74531529995f8 Mon Sep 17 00:00:00 2001 From: Sascha Steinbiss Date: Sun, 12 May 2024 01:44:07 +0200 Subject: [PATCH] mqtt: enable limiting of logged message length Ticket: #6984 --- rust/src/mqtt/logger.rs | 64 ++++++++++++++++++++++------------ rust/src/mqtt/mqtt_property.rs | 22 ++++++------ src/output-json-mqtt.c | 19 +++++++--- suricata.yaml.in | 7 ++++ 4 files changed, 74 insertions(+), 38 deletions(-) diff --git a/rust/src/mqtt/logger.rs b/rust/src/mqtt/logger.rs index af0db17bc2..4ebdd1541c 100644 --- a/rust/src/mqtt/logger.rs +++ b/rust/src/mqtt/logger.rs @@ -26,9 +26,9 @@ use std; pub const MQTT_LOG_PASSWORDS: u32 = BIT_U32!(0); #[inline] -fn log_mqtt_topic(js: &mut JsonBuilder, t: &MQTTSubscribeTopicData) -> Result<(), JsonError> { +fn log_mqtt_topic(js: &mut JsonBuilder, t: &MQTTSubscribeTopicData, max_log_len: usize ) -> Result<(), JsonError> { js.start_object()?; - js.set_string("topic", &t.topic_name)?; + js.set_string_limited("topic", &t.topic_name, max_log_len)?; js.set_uint("qos", t.qos as u64)?; js.close()?; return Ok(()); @@ -42,7 +42,9 @@ fn log_mqtt_header(js: &mut JsonBuilder, hdr: &FixedHeader) -> Result<(), JsonEr return Ok(()); } -fn log_mqtt(tx: &MQTTTransaction, flags: u32, js: &mut JsonBuilder) -> Result<(), JsonError> { +fn log_mqtt( + tx: &MQTTTransaction, flags: u32, max_log_len: usize, js: &mut JsonBuilder, +) -> Result<(), JsonError> { js.open_object("mqtt")?; for msg in tx.msg.iter() { match msg.op { @@ -51,7 +53,7 @@ fn log_mqtt(tx: &MQTTTransaction, flags: u32, js: &mut JsonBuilder) -> Result<() log_mqtt_header(js, &msg.header)?; js.set_string("protocol_string", &conn.protocol_string)?; js.set_uint("protocol_version", conn.protocol_version as u64)?; - js.set_string("client_id", &conn.client_id)?; + js.set_string_limited("client_id", &conn.client_id, max_log_len)?; js.open_object("flags")?; js.set_bool("username", conn.username_flag)?; js.set_bool("password", conn.password_flag)?; @@ -60,7 +62,7 @@ fn log_mqtt(tx: &MQTTTransaction, flags: u32, js: &mut JsonBuilder) -> Result<() js.set_bool("clean_session", conn.clean_session)?; js.close()?; // flags if let Some(user) = &conn.username { - js.set_string("username", user)?; + js.set_string_limited("username", user, max_log_len)?; } if flags & MQTT_LOG_PASSWORDS != 0 { if let Some(pass) = &conn.password { @@ -70,15 +72,19 @@ fn log_mqtt(tx: &MQTTTransaction, flags: u32, js: &mut JsonBuilder) -> Result<() if conn.will_flag { js.open_object("will")?; if let Some(will_topic) = &conn.will_topic { - js.set_string("topic", will_topic)?; + js.set_string_limited("topic", will_topic, max_log_len)?; } if let Some(will_message) = &conn.will_message { - js.set_string_from_bytes("message", will_message)?; + js.set_string_from_bytes_limited( + "message", + will_message, + max_log_len, + )?; } if let Some(will_properties) = &conn.will_properties { js.open_object("properties")?; for prop in will_properties { - prop.set_json(js)?; + prop.set_json(js, max_log_len)?; } js.close()?; // properties } @@ -87,7 +93,7 @@ fn log_mqtt(tx: &MQTTTransaction, flags: u32, js: &mut JsonBuilder) -> Result<() if let Some(properties) = &conn.properties { js.open_object("properties")?; for prop in properties { - prop.set_json(js)?; + prop.set_json(js, max_log_len)?; } js.close()?; // properties } @@ -101,7 +107,7 @@ fn log_mqtt(tx: &MQTTTransaction, flags: u32, js: &mut JsonBuilder) -> Result<() if let Some(properties) = &connack.properties { js.open_object("properties")?; for prop in properties { - prop.set_json(js)?; + prop.set_json(js, max_log_len)?; } js.close()?; // properties } @@ -110,15 +116,19 @@ fn log_mqtt(tx: &MQTTTransaction, flags: u32, js: &mut JsonBuilder) -> Result<() MQTTOperation::PUBLISH(ref publish) => { js.open_object("publish")?; log_mqtt_header(js, &msg.header)?; - js.set_string("topic", &publish.topic)?; + js.set_string_limited("topic", &publish.topic, max_log_len)?; if let Some(message_id) = publish.message_id { js.set_uint("message_id", message_id as u64)?; } - js.set_string_from_bytes("message", &publish.message)?; + js.set_string_from_bytes_limited( + "message", + &publish.message, + max_log_len, + )?; if let Some(properties) = &publish.properties { js.open_object("properties")?; for prop in properties { - prop.set_json(js)?; + prop.set_json(js, max_log_len)?; } js.close()?; // properties } @@ -134,7 +144,7 @@ fn log_mqtt(tx: &MQTTTransaction, flags: u32, js: &mut JsonBuilder) -> Result<() if let Some(properties) = &msgidonly.properties { js.open_object("properties")?; for prop in properties { - prop.set_json(js)?; + prop.set_json(js, max_log_len)?; } js.close()?; // properties } @@ -150,7 +160,7 @@ fn log_mqtt(tx: &MQTTTransaction, flags: u32, js: &mut JsonBuilder) -> Result<() if let Some(properties) = &msgidonly.properties { js.open_object("properties")?; for prop in properties { - prop.set_json(js)?; + prop.set_json(js, max_log_len)?; } js.close()?; // properties } @@ -166,7 +176,7 @@ fn log_mqtt(tx: &MQTTTransaction, flags: u32, js: &mut JsonBuilder) -> Result<() if let Some(properties) = &msgidonly.properties { js.open_object("properties")?; for prop in properties { - prop.set_json(js)?; + prop.set_json(js, max_log_len)?; } js.close()?; // properties } @@ -182,7 +192,7 @@ fn log_mqtt(tx: &MQTTTransaction, flags: u32, js: &mut JsonBuilder) -> Result<() if let Some(properties) = &msgidonly.properties { js.open_object("properties")?; for prop in properties { - prop.set_json(js)?; + prop.set_json(js, max_log_len)?; } js.close()?; // properties } @@ -193,14 +203,22 @@ fn log_mqtt(tx: &MQTTTransaction, flags: u32, js: &mut JsonBuilder) -> Result<() log_mqtt_header(js, &msg.header)?; js.set_uint("message_id", subs.message_id as u64)?; js.open_array("topics")?; + let mut topic_chars : usize = 0; + // We only log topics until the length of the concatenation of + // the topic names exceed the log limit set. for t in &subs.topics { - log_mqtt_topic(js, t)?; + if topic_chars + std::cmp::min(t.topic_name.len(), max_log_len) > max_log_len { + log_mqtt_topic(js, t, max_log_len - topic_chars)?; + break; + } + log_mqtt_topic(js, t, max_log_len)?; + topic_chars += std::cmp::min(t.topic_name.len(), max_log_len); } js.close()?; //topics if let Some(properties) = &subs.properties { js.open_object("properties")?; for prop in properties { - prop.set_json(js)?; + prop.set_json(js, max_log_len)?; } js.close()?; // properties } @@ -260,7 +278,7 @@ fn log_mqtt(tx: &MQTTTransaction, flags: u32, js: &mut JsonBuilder) -> Result<() if let Some(properties) = &auth.properties { js.open_object("properties")?; for prop in properties { - prop.set_json(js)?; + prop.set_json(js, max_log_len)?; } js.close()?; // properties } @@ -275,7 +293,7 @@ fn log_mqtt(tx: &MQTTTransaction, flags: u32, js: &mut JsonBuilder) -> Result<() if let Some(properties) = &disco.properties { js.open_object("properties")?; for prop in properties { - prop.set_json(js)?; + prop.set_json(js, max_log_len)?; } js.close()?; // properties } @@ -298,8 +316,8 @@ fn log_mqtt(tx: &MQTTTransaction, flags: u32, js: &mut JsonBuilder) -> Result<() #[no_mangle] pub unsafe extern "C" fn rs_mqtt_logger_log( - tx: *mut std::os::raw::c_void, flags: u32, js: &mut JsonBuilder, + tx: *mut std::os::raw::c_void, flags: u32, max_log_len: u32, js: &mut JsonBuilder, ) -> bool { let tx = cast_pointer!(tx, MQTTTransaction); - log_mqtt(tx, flags, js).is_ok() + log_mqtt(tx, flags, max_log_len as usize, js).is_ok() } diff --git a/rust/src/mqtt/mqtt_property.rs b/rust/src/mqtt/mqtt_property.rs index a716c4b652..432db5aafc 100644 --- a/rust/src/mqtt/mqtt_property.rs +++ b/rust/src/mqtt/mqtt_property.rs @@ -58,7 +58,7 @@ pub enum MQTTProperty { } impl crate::mqtt::mqtt_property::MQTTProperty { - pub fn set_json(&self, js: &mut JsonBuilder) -> Result<(), JsonError> { + pub fn set_json(&self, js: &mut JsonBuilder, limit: usize) -> Result<(), JsonError> { match self { crate::mqtt::mqtt_property::MQTTProperty::PAYLOAD_FORMAT_INDICATOR(v) => { js.set_uint("payload_format_indicator", *v as u64)?; @@ -67,13 +67,13 @@ impl crate::mqtt::mqtt_property::MQTTProperty { js.set_uint("message_expiry_interval", *v as u64)?; } crate::mqtt::mqtt_property::MQTTProperty::CONTENT_TYPE(v) => { - js.set_string("content_type", v)?; + js.set_string_limited("content_type", v, limit)?; } crate::mqtt::mqtt_property::MQTTProperty::RESPONSE_TOPIC(v) => { - js.set_string("response_topic", v)?; + js.set_string_limited("response_topic", v, limit)?; } crate::mqtt::mqtt_property::MQTTProperty::CORRELATION_DATA(v) => { - js.set_string_from_bytes("correlation_data", v)?; + js.set_string_from_bytes_limited("correlation_data", v, limit)?; } crate::mqtt::mqtt_property::MQTTProperty::SUBSCRIPTION_IDENTIFIER(v) => { js.set_uint("subscription_identifier", *v as u64)?; @@ -82,16 +82,16 @@ impl crate::mqtt::mqtt_property::MQTTProperty { js.set_uint("session_expiry_interval", *v as u64)?; } crate::mqtt::mqtt_property::MQTTProperty::ASSIGNED_CLIENT_IDENTIFIER(v) => { - js.set_string("assigned_client_identifier", v)?; + js.set_string_limited("assigned_client_identifier", v, limit)?; } crate::mqtt::mqtt_property::MQTTProperty::SERVER_KEEP_ALIVE(v) => { js.set_uint("server_keep_alive", *v as u64)?; } crate::mqtt::mqtt_property::MQTTProperty::AUTHENTICATION_METHOD(v) => { - js.set_string("authentication_method", v)?; + js.set_string_limited("authentication_method", v, limit)?; } crate::mqtt::mqtt_property::MQTTProperty::AUTHENTICATION_DATA(v) => { - js.set_string_from_bytes("authentication_data", v)?; + js.set_string_from_bytes_limited("authentication_data", v, limit)?; } crate::mqtt::mqtt_property::MQTTProperty::REQUEST_PROBLEM_INFORMATION(v) => { js.set_uint("request_problem_information", *v as u64)?; @@ -103,13 +103,13 @@ impl crate::mqtt::mqtt_property::MQTTProperty { js.set_uint("request_response_information", *v as u64)?; } crate::mqtt::mqtt_property::MQTTProperty::RESPONSE_INFORMATION(v) => { - js.set_string("response_information", v)?; + js.set_string_limited("response_information", v, limit)?; } crate::mqtt::mqtt_property::MQTTProperty::SERVER_REFERENCE(v) => { - js.set_string("server_reference", v)?; + js.set_string_limited("server_reference", v, limit)?; } crate::mqtt::mqtt_property::MQTTProperty::REASON_STRING(v) => { - js.set_string("reason_string", v)?; + js.set_string_limited("reason_string", v, limit)?; } crate::mqtt::mqtt_property::MQTTProperty::RECEIVE_MAXIMUM(v) => { js.set_uint("receive_maximum", *v as u64)?; @@ -127,7 +127,7 @@ impl crate::mqtt::mqtt_property::MQTTProperty { js.set_uint("retain_available", *v as u64)?; } crate::mqtt::mqtt_property::MQTTProperty::USER_PROPERTY((k, v)) => { - js.set_string(k, v)?; + js.set_string_limited(k, v, limit)?; } crate::mqtt::mqtt_property::MQTTProperty::MAXIMUM_PACKET_SIZE(v) => { js.set_uint("maximum_packet_size", *v as u64)?; diff --git a/src/output-json-mqtt.c b/src/output-json-mqtt.c index b743229f36..777107f3e9 100644 --- a/src/output-json-mqtt.c +++ b/src/output-json-mqtt.c @@ -34,6 +34,7 @@ #include "util-buffer.h" #include "util-debug.h" #include "util-byte.h" +#include "util-misc.h" #include "output.h" #include "output-json.h" @@ -45,10 +46,11 @@ #include "rust.h" #define MQTT_LOG_PASSWORDS BIT_U32(0) -#define MQTT_DEFAULTS (MQTT_LOG_PASSWORDS) +#define MQTT_DEFAULT_FLAGS (MQTT_LOG_PASSWORDS) +#define MQTT_DEFAULT_MAXLOGLEN 1024 typedef struct LogMQTTFileCtx_ { - uint32_t flags; + uint32_t flags, max_log_len; OutputJsonCtx *eve_ctx; } LogMQTTFileCtx; @@ -60,7 +62,7 @@ typedef struct LogMQTTLogThread_ { bool JsonMQTTAddMetadata(void *vtx, JsonBuilder *js) { - return rs_mqtt_logger_log(vtx, MQTT_DEFAULTS, js); + return rs_mqtt_logger_log(vtx, MQTT_DEFAULT_FLAGS, MQTT_DEFAULT_MAXLOGLEN, js); } static int JsonMQTTLogger(ThreadVars *tv, void *thread_data, @@ -80,7 +82,7 @@ static int JsonMQTTLogger(ThreadVars *tv, void *thread_data, return TM_ECODE_FAILED; } - if (!rs_mqtt_logger_log(tx, thread->mqttlog_ctx->flags, js)) + if (!rs_mqtt_logger_log(tx, thread->mqttlog_ctx->flags, thread->mqttlog_ctx->max_log_len, js)) goto error; OutputJsonBuilderBuffer(js, thread->ctx); @@ -112,6 +114,15 @@ static void JsonMQTTLogParseConfig(ConfNode *conf, LogMQTTFileCtx *mqttlog_ctx) } else { mqttlog_ctx->flags |= MQTT_LOG_PASSWORDS; } + uint32_t max_log_len = MQTT_DEFAULT_MAXLOGLEN; + query = ConfNodeLookupChildValue(conf, "string-log-limit"); + if (query != NULL) { + if (ParseSizeStringU32(query, &max_log_len) < 0) { + SCLogError("Error parsing string-log-limit from config - %s, ", query); + exit(EXIT_FAILURE); + } + } + mqttlog_ctx->max_log_len = max_log_len; } static OutputInitResult OutputMQTTLogInitSub(ConfNode *conf, diff --git a/suricata.yaml.in b/suricata.yaml.in index 5e4d039e9e..002f0a5044 100644 --- a/suricata.yaml.in +++ b/suricata.yaml.in @@ -314,6 +314,13 @@ outputs: - ssh - mqtt: # passwords: yes # enable output of passwords + # string-log-limit: 1kb # limit size of logged strings in bytes. + # Can be specified in kb, mb, gb. Just a number + # is parsed as bytes. Default is 1KB. + # Use a value of 0 to disable limiting. + # Note that the size is also bounded by + # the maximum parsed message size (see + # app-layer configuration) - http2 - pgsql: enabled: no -- 2.47.2