Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
9d7a421
output/smtp: remove unused function parameters
catenacyber May 11, 2023
a00fc00
output/dns: remove unused function parameters
catenacyber May 11, 2023
09830fd
output/ftp: remove unused function parameters
catenacyber May 11, 2023
18bb970
output/alert: rewrite code for app-layer properties
catenacyber May 11, 2023
19688ba
output/ftp: have ftp properties in alerts
catenacyber May 11, 2023
30c8fb6
output/tftp: have tftp properties in alerts
catenacyber May 11, 2023
c0eeac3
output/mqtt: reuse standard code
catenacyber May 12, 2023
f01e8d6
output/rfb: reuse standard code
catenacyber May 12, 2023
f941be0
output/snmp: reuse standard code
catenacyber May 12, 2023
ac58f4d
output/krb5: have krb5 properties in alerts
catenacyber May 12, 2023
ee833ff
output/ftp-data: reuse standard code
catenacyber May 14, 2023
00d8267
output/tls: reuse standard code
catenacyber May 14, 2023
4248f4d
output: comments for non-generic app-layers
catenacyber May 14, 2023
3be26a4
output: generic tx json logger
catenacyber May 14, 2023
9c5f0f4
output/http2: generic tx json logger
catenacyber May 15, 2023
89483ce
output/rdp: generic tx json logger
catenacyber May 15, 2023
d203d3a
output: code reuse for generic tx json logger
catenacyber May 15, 2023
81f8576
output/rfb: generic tx json logger
catenacyber May 15, 2023
4430125
output/sip: generic tx json logger
catenacyber May 15, 2023
5c7b7c1
output/snmp: generic tx json logger
catenacyber May 15, 2023
51e94c4
output/quic: generic tx json logger
catenacyber May 15, 2023
1fc1088
output/krb5: generic tx json logger
catenacyber May 15, 2023
31608fc
output/tftp: generic tx json logger
catenacyber May 15, 2023
f8bb8ef
output/dnp3: restrict function scope to one file
catenacyber May 15, 2023
1ff1ad8
output/modbus: generic tx json logger
catenacyber May 15, 2023
d5a2864
output/ssh: generic tx json logger
catenacyber May 15, 2023
41b36b2
output/http2: reuse code for file events
catenacyber May 15, 2023
9ccda72
output/template: reuse code for file events
catenacyber May 15, 2023
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
2 changes: 0 additions & 2 deletions rust/src/modbus/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ pub extern "C" fn rs_modbus_to_json(tx: &mut ModbusTransaction, js: &mut JsonBui

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this commit does a lot more than the commit message indicates

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, so, any ideas about the commit segmentation ?
One big commit ? One commit per protocol ? (but if so, the first commit introducing the generic changes should also be applied on some protocol, otherwise, commit-check will fail because of unused functions)

Should I split this PR ?

/// populate a json object with transactional information, for logging
fn log(tx: &ModbusTransaction, js: &mut JsonBuilder) -> Result<(), JsonError> {
js.open_object("modbus")?;
js.set_uint("id", tx.id)?;

if let Some(req) = &tx.request {
Expand All @@ -42,7 +41,6 @@ fn log(tx: &ModbusTransaction, js: &mut JsonBuilder) -> Result<(), JsonError> {
js.close()?;
}

js.close()?;
Ok(())
}

Expand Down
2 changes: 0 additions & 2 deletions rust/src/quic/logger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ fn quic_tls_extension_name(e: u16) -> Option<String> {
}

fn log_template(tx: &QuicTransaction, js: &mut JsonBuilder) -> Result<(), JsonError> {
js.open_object("quic")?;
if tx.header.ty != QuicType::Short {
js.set_string("version", String::from(tx.header.version).as_str())?;

Expand Down Expand Up @@ -144,7 +143,6 @@ fn log_template(tx: &QuicTransaction, js: &mut JsonBuilder) -> Result<(), JsonEr
js.close()?;
}

js.close()?;
Ok(())
}

Expand Down
2 changes: 0 additions & 2 deletions rust/src/rdp/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ pub extern "C" fn rs_rdp_to_json(tx: &mut RdpTransaction, js: &mut JsonBuilder)

/// populate a json object with transactional information, for logging
fn log(tx: &RdpTransaction, js: &mut JsonBuilder) -> Result<(), JsonError> {
js.open_object("rdp")?;
js.set_uint("tx_id", tx.id)?;

match &tx.item {
Expand Down Expand Up @@ -58,7 +57,6 @@ fn log(tx: &RdpTransaction, js: &mut JsonBuilder) -> Result<(), JsonError> {
}
}

js.close()?;
Ok(())
}

Expand Down
6 changes: 1 addition & 5 deletions rust/src/sip/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ use crate::jsonbuilder::{JsonBuilder, JsonError};
use crate::sip::sip::SIPTransaction;

fn log(tx: &SIPTransaction, js: &mut JsonBuilder) -> Result<(), JsonError> {
js.open_object("sip")?;

if let Some(req) = &tx.request {
js.set_string("method", &req.method)?
.set_string("uri", &req.path)?
Expand All @@ -43,12 +41,10 @@ fn log(tx: &SIPTransaction, js: &mut JsonBuilder) -> Result<(), JsonError> {
js.set_string("response_line", resp_line)?;
}

js.close()?;

Ok(())
}

#[no_mangle]
pub extern "C" fn rs_sip_log_json(tx: &mut SIPTransaction, js: &mut JsonBuilder) -> bool {
log(tx, js).is_ok()
}
}
4 changes: 4 additions & 0 deletions scripts/setup-app-layer.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,10 @@ def logger_patch_output_c(proto):
output = io.StringIO()
inlines = open(filename).readlines()
for i, line in enumerate(inlines):
if line.find("ALPROTO_TEMPLATE") > -1:
new_line = line.replace("TEMPLATE", proto.upper()).replace(
"template", proto.lower())
output.write(new_line)
if line.find("output-json-template.h") > -1:
output.write(line.replace("template", proto.lower()))
if line.find("/* Template JSON logger.") > -1:
Expand Down
180 changes: 17 additions & 163 deletions src/output-json-alert.c
Original file line number Diff line number Diff line change
Expand Up @@ -150,101 +150,6 @@ static void AlertJsonTls(const Flow *f, JsonBuilder *js)
return;
}

static void AlertJsonSsh(const Flow *f, JsonBuilder *js)
{
void *ssh_state = FlowGetAppState(f);
if (ssh_state) {
JsonBuilderMark mark = { 0, 0, 0 };
void *tx_ptr = rs_ssh_state_get_tx(ssh_state, 0);
jb_get_mark(js, &mark);
jb_open_object(js, "ssh");
if (rs_ssh_log_json(tx_ptr, js)) {
jb_close(js);
} else {
jb_restore_mark(js, &mark);
}
}

return;
}

static void AlertJsonHttp2(const Flow *f, const uint64_t tx_id, JsonBuilder *js)
{
void *h2_state = FlowGetAppState(f);
if (h2_state) {
void *tx_ptr = rs_http2_state_get_tx(h2_state, tx_id);
if (tx_ptr) {
JsonBuilderMark mark = { 0, 0, 0 };
jb_get_mark(js, &mark);
jb_open_object(js, "http");
if (rs_http2_log_json(tx_ptr, js)) {
jb_close(js);
} else {
jb_restore_mark(js, &mark);
}
}
}

return;
}

static void AlertJsonDnp3(const Flow *f, const uint64_t tx_id, JsonBuilder *js)
{
DNP3State *dnp3_state = (DNP3State *)FlowGetAppState(f);
if (dnp3_state) {
DNP3Transaction *tx = AppLayerParserGetTx(IPPROTO_TCP, ALPROTO_DNP3,
dnp3_state, tx_id);
if (tx) {
JsonBuilderMark mark = { 0, 0, 0 };
jb_get_mark(js, &mark);
bool logged = false;
jb_open_object(js, "dnp3");
if (tx->is_request && tx->done) {
jb_open_object(js, "request");
JsonDNP3LogRequest(js, tx);
jb_close(js);
logged = true;
}
if (!tx->is_request && tx->done) {
jb_open_object(js, "response");
JsonDNP3LogResponse(js, tx);
jb_close(js);
logged = true;
}
if (logged) {
/* Close dnp3 object. */
jb_close(js);
} else {
jb_restore_mark(js, &mark);
}
}
}
}

static void AlertJsonDns(const Flow *f, const uint64_t tx_id, JsonBuilder *js)
{
void *dns_state = (void *)FlowGetAppState(f);
if (dns_state) {
void *txptr = AppLayerParserGetTx(f->proto, ALPROTO_DNS,
dns_state, tx_id);
if (txptr) {
jb_open_object(js, "dns");
JsonBuilder *qjs = JsonDNSLogQuery(txptr, tx_id);
if (qjs != NULL) {
jb_set_object(js, "query", qjs);
jb_free(qjs);
}
JsonBuilder *ajs = JsonDNSLogAnswer(txptr, tx_id);
if (ajs != NULL) {
jb_set_object(js, "answer", ajs);
jb_free(ajs);
}
jb_close(js);
}
}
return;
}

static void AlertJsonSNMP(const Flow *f, const uint64_t tx_id, JsonBuilder *js)
{
void *snmp_state = (void *)FlowGetAppState(f);
Expand All @@ -259,41 +164,6 @@ static void AlertJsonSNMP(const Flow *f, const uint64_t tx_id, JsonBuilder *js)
}
}

static void AlertJsonRDP(const Flow *f, const uint64_t tx_id, JsonBuilder *js)
{
void *rdp_state = (void *)FlowGetAppState(f);
if (rdp_state != NULL) {
void *tx = AppLayerParserGetTx(f->proto, ALPROTO_RDP, rdp_state,
tx_id);
if (tx != NULL) {
JsonBuilderMark mark = { 0, 0, 0 };
jb_get_mark(js, &mark);
if (!rs_rdp_to_json(tx, js)) {
jb_restore_mark(js, &mark);
}
}
}
}

static void AlertJsonBitTorrentDHT(const Flow *f, const uint64_t tx_id, JsonBuilder *js)
{
void *bittorrent_dht_state = (void *)FlowGetAppState(f);
if (bittorrent_dht_state != NULL) {
void *tx =
AppLayerParserGetTx(f->proto, ALPROTO_BITTORRENT_DHT, bittorrent_dht_state, tx_id);
if (tx != NULL) {
JsonBuilderMark mark = { 0, 0, 0 };
jb_get_mark(js, &mark);
jb_open_object(js, "bittorrent_dht");
if (rs_bittorrent_dht_logger_log(tx, js)) {
jb_close(js);
} else {
jb_restore_mark(js, &mark);
}
}
}
}

static void AlertJsonSourceTarget(const Packet *p, const PacketAlert *pa,
JsonBuilder *js, JsonAddrInfo *addr)
{
Expand Down Expand Up @@ -470,7 +340,24 @@ static void AlertAddAppLayer(const Packet *p, JsonBuilder *jb,
const uint64_t tx_id, const uint16_t option_flags)
{
const AppProto proto = FlowGetAppProtocol(p->flow);
AppLayerLogger *al = GetAppProtoLogger(proto);
JsonBuilderMark mark = { 0, 0, 0 };
if (al && al->name) {
void *state = FlowGetAppState(p->flow);
if (state) {
void *tx = AppLayerParserGetTx(p->flow->proto, proto, state, tx_id);
if (tx) {
jb_get_mark(jb, &mark);
jb_open_object(jb, al->name);
if (al->log(tx, jb)) {
jb_close(jb);
} else {
jb_restore_mark(jb, &mark);
}
}
}
return;
}
switch (proto) {
case ALPROTO_HTTP1:
// TODO: Could result in an empty http object being logged.
Expand All @@ -488,9 +375,6 @@ static void AlertAddAppLayer(const Packet *p, JsonBuilder *jb,
case ALPROTO_TLS:
AlertJsonTls(p->flow, jb);
break;
case ALPROTO_SSH:
AlertJsonSsh(p->flow, jb);
break;
case ALPROTO_SMTP:
jb_get_mark(jb, &mark);
jb_open_object(jb, "smtp");
Expand Down Expand Up @@ -534,9 +418,6 @@ static void AlertAddAppLayer(const Packet *p, JsonBuilder *jb,
jb_restore_mark(jb, &mark);
}
break;
case ALPROTO_SIP:
JsonSIPAddMetadata(jb, p->flow, tx_id);
break;
case ALPROTO_RFB:
jb_get_mark(jb, &mark);
if (!JsonRFBAddMetadata(p->flow, tx_id, jb)) {
Expand All @@ -549,15 +430,6 @@ static void AlertAddAppLayer(const Packet *p, JsonBuilder *jb,
EveFTPDataAddMetadata(p->flow, jb);
jb_close(jb);
break;
case ALPROTO_DNP3:
AlertJsonDnp3(p->flow, tx_id, jb);
break;
case ALPROTO_HTTP2:
AlertJsonHttp2(p->flow, tx_id, jb);
break;
case ALPROTO_DNS:
AlertJsonDns(p->flow, tx_id, jb);
break;
case ALPROTO_IKE:
jb_get_mark(jb, &mark);
if (!EveIKEAddMetadata(p->flow, tx_id, jb)) {
Expand All @@ -570,27 +442,9 @@ static void AlertAddAppLayer(const Packet *p, JsonBuilder *jb,
jb_restore_mark(jb, &mark);
}
break;
case ALPROTO_QUIC:
jb_get_mark(jb, &mark);
if (!JsonQuicAddMetadata(p->flow, tx_id, jb)) {
jb_restore_mark(jb, &mark);
}
break;
case ALPROTO_SNMP:
AlertJsonSNMP(p->flow, tx_id, jb);
break;
case ALPROTO_RDP:
AlertJsonRDP(p->flow, tx_id, jb);
break;
case ALPROTO_MODBUS:
jb_get_mark(jb, &mark);
if (!JsonModbusAddMetadata(p->flow, tx_id, jb)) {
jb_restore_mark(jb, &mark);
}
break;
case ALPROTO_BITTORRENT_DHT:
AlertJsonBitTorrentDHT(p->flow, tx_id, jb);
break;
default:
break;
}
Expand Down
19 changes: 19 additions & 0 deletions src/output-json-dnp3.c
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,25 @@ void JsonDNP3LogResponse(JsonBuilder *js, DNP3Transaction *dnp3tx)
jb_close(js);
}

bool AlertJsonDnp3(void *vtx, JsonBuilder *js)
{
DNP3Transaction *tx = (DNP3Transaction *)vtx;
bool logged = false;
if (tx->is_request && tx->done) {
jb_open_object(js, "request");
JsonDNP3LogRequest(js, tx);
jb_close(js);
logged = true;
}
if (!tx->is_request && tx->done) {
jb_open_object(js, "response");
JsonDNP3LogResponse(js, tx);
jb_close(js);
logged = true;
}
return logged;
}

static int JsonDNP3LoggerToServer(ThreadVars *tv, void *thread_data,
const Packet *p, Flow *f, void *state, void *vtx, uint64_t tx_id)
{
Expand Down
1 change: 1 addition & 0 deletions src/output-json-dnp3.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,6 @@ void JsonDNP3LogRequest(JsonBuilder *js, DNP3Transaction *);
void JsonDNP3LogResponse(JsonBuilder *js, DNP3Transaction *);

void JsonDNP3LogRegister(void);
bool AlertJsonDnp3(void *vtx, JsonBuilder *js);

#endif /* __OUTPUT_JSON_DNP3_H__ */
19 changes: 17 additions & 2 deletions src/output-json-dns.c
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ typedef struct LogDnsLogThread_ {
OutputJsonThreadCtx *ctx;
} LogDnsLogThread;

JsonBuilder *JsonDNSLogQuery(void *txptr, uint64_t tx_id)
static JsonBuilder *JsonDNSLogQuery(void *txptr)
{
JsonBuilder *queryjb = jb_new_array();
if (queryjb == NULL) {
Expand Down Expand Up @@ -292,7 +292,7 @@ JsonBuilder *JsonDNSLogQuery(void *txptr, uint64_t tx_id)
return queryjb;
}

JsonBuilder *JsonDNSLogAnswer(void *txptr, uint64_t tx_id)
static JsonBuilder *JsonDNSLogAnswer(void *txptr)
{
if (!rs_dns_do_log_answer(txptr, LOG_ALL_RRTYPES)) {
return NULL;
Expand All @@ -304,6 +304,21 @@ JsonBuilder *JsonDNSLogAnswer(void *txptr, uint64_t tx_id)
}
}

bool AlertJsonDns(void *txptr, JsonBuilder *js)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this return false if both calls return NULL?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likely so, this is a behavioral change

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likely so, this is a behavioral change

Not a big one right? It would allow the caller to reset to mark to prevent an empty dns object from being logger in alert.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does not look a big one

#9005 is the part without behavior changes

Then I intend to do the behavioral changes about alerts : addition of protocols missing alert metadata (like krb5) + behavioral change for dns alert metadata

{
JsonBuilder *qjs = JsonDNSLogQuery(txptr);
if (qjs != NULL) {
jb_set_object(js, "query", qjs);
jb_free(qjs);
}
JsonBuilder *ajs = JsonDNSLogAnswer(txptr);
if (ajs != NULL) {
jb_set_object(js, "answer", ajs);
jb_free(ajs);
}
return true;
}

static int JsonDnsLoggerToServer(ThreadVars *tv, void *thread_data,
const Packet *p, Flow *f, void *alstate, void *txptr, uint64_t tx_id)
{
Expand Down
3 changes: 1 addition & 2 deletions src/output-json-dns.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@

void JsonDnsLogRegister(void);

JsonBuilder *JsonDNSLogQuery(void *txptr, uint64_t tx_id) __attribute__((nonnull));
JsonBuilder *JsonDNSLogAnswer(void *txptr, uint64_t tx_id) __attribute__((nonnull));
bool AlertJsonDns(void *vtx, JsonBuilder *js);

#endif /* __OUTPUT_JSON_DNS_H__ */
Loading