From: Jason Ish Date: Thu, 13 Oct 2022 21:51:10 +0000 (-0600) Subject: dnp3: fixups to work with unified json tx logger X-Git-Tag: suricata-7.0.0-rc1~445 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=27672c950c3721527b4a48a1bf5eb116ea85df60;p=thirdparty%2Fsuricata.git dnp3: fixups to work with unified json tx logger Update DNP3 to work with a single TX logger, and just register one logger instead of 2. This primarily creates a TX per message instead of correlating replies to requests, which fits the DNP3 model better, but we didn't really have this concept nailed down when DNP3 was written. --- diff --git a/src/app-layer-dnp3.c b/src/app-layer-dnp3.c index b985e57616..06b4c64960 100644 --- a/src/app-layer-dnp3.c +++ b/src/app-layer-dnp3.c @@ -490,7 +490,7 @@ static void DNP3SetEventTx(DNP3Transaction *tx, uint8_t event) /** * \brief Allocation a DNP3 transaction. */ -static DNP3Transaction *DNP3TxAlloc(DNP3State *dnp3) +static DNP3Transaction *DNP3TxAlloc(DNP3State *dnp3, bool request) { DNP3Transaction *tx = SCCalloc(1, sizeof(DNP3Transaction)); if (unlikely(tx == NULL)) { @@ -501,8 +501,8 @@ static DNP3Transaction *DNP3TxAlloc(DNP3State *dnp3) dnp3->curr = tx; tx->dnp3 = dnp3; tx->tx_num = dnp3->transaction_max; - TAILQ_INIT(&tx->request_objects); - TAILQ_INIT(&tx->response_objects); + tx->is_request = request; + TAILQ_INIT(&tx->objects); TAILQ_INSERT_TAIL(&dnp3->tx_list, tx, next); /* Check for flood state. */ @@ -872,12 +872,8 @@ static void DNP3HandleUserDataRequest(DNP3State *dnp3, const uint8_t *input, if (!DNP3_TH_FIR(th)) { TAILQ_FOREACH(ttx, &dnp3->tx_list, next) { - if (ttx->request_lh.src == lh->src && - ttx->request_lh.dst == lh->dst && - ttx->has_request && - !ttx->request_done && - NEXT_TH_SEQNO(DNP3_TH_SEQ(ttx->request_th)) == DNP3_TH_SEQ(th)) - { + if (ttx->lh.src == lh->src && ttx->lh.dst == lh->dst && ttx->is_request && !ttx->done && + NEXT_TH_SEQNO(DNP3_TH_SEQ(ttx->th)) == DNP3_TH_SEQ(th)) { tx = ttx; break; } @@ -889,7 +885,7 @@ static void DNP3HandleUserDataRequest(DNP3State *dnp3, const uint8_t *input, /* Update the saved transport header so subsequent segments * will be matched to this sequence number. */ - tx->response_th = th; + tx->th = th; } else { ah = (DNP3ApplicationHeader *)(input + sizeof(DNP3LinkHeader) + @@ -901,24 +897,21 @@ static void DNP3HandleUserDataRequest(DNP3State *dnp3, const uint8_t *input, } /* Create a transaction. */ - tx = DNP3TxAlloc(dnp3); + tx = DNP3TxAlloc(dnp3, true); if (unlikely(tx == NULL)) { return; } - tx->request_lh = *lh; - tx->request_th = th; - tx->request_ah = *ah; - tx->has_request = 1; - + tx->lh = *lh; + tx->th = th; + tx->ah = *ah; } if (!DNP3ReassembleApplicationLayer(input + sizeof(DNP3LinkHeader), - input_len - sizeof(DNP3LinkHeader), - &tx->request_buffer, &tx->request_buffer_len)) { + input_len - sizeof(DNP3LinkHeader), &tx->buffer, &tx->buffer_len)) { /* Malformed, set event and mark as done. */ DNP3SetEvent(dnp3, DNP3_DECODER_EVENT_MALFORMED); - tx->request_done = 1; + tx->done = 1; return; } @@ -927,26 +920,11 @@ static void DNP3HandleUserDataRequest(DNP3State *dnp3, const uint8_t *input, return; } - tx->request_done = 1; - - /* Some function codes do not expect a reply. */ - switch (tx->request_ah.function_code) { - case DNP3_APP_FC_CONFIRM: - case DNP3_APP_FC_DIR_OPERATE_NR: - case DNP3_APP_FC_FREEZE_NR: - case DNP3_APP_FC_FREEZE_CLEAR_NR: - case DNP3_APP_FC_FREEZE_AT_TIME_NR: - case DNP3_APP_FC_AUTH_REQ_NR: - tx->response_done = 1; - default: - break; - } + tx->done = 1; - if (DNP3DecodeApplicationObjects( - tx, tx->request_buffer + sizeof(DNP3ApplicationHeader), - tx->request_buffer_len - sizeof(DNP3ApplicationHeader), - &tx->request_objects)) { - tx->request_complete = 1; + if (DNP3DecodeApplicationObjects(tx, tx->buffer + sizeof(DNP3ApplicationHeader), + tx->buffer_len - sizeof(DNP3ApplicationHeader), &tx->objects)) { + tx->complete = 1; } } @@ -971,11 +949,8 @@ static void DNP3HandleUserDataResponse(DNP3State *dnp3, const uint8_t *input, if (!DNP3_TH_FIR(th)) { TAILQ_FOREACH(ttx, &dnp3->tx_list, next) { - if (ttx->response_lh.src == lh->src && - ttx->response_lh.dst == lh->dst && - ttx->has_response && !ttx->response_done && - NEXT_TH_SEQNO(DNP3_TH_SEQ(ttx->response_th)) == DNP3_TH_SEQ(th)) - { + if (ttx->lh.src == lh->src && ttx->lh.dst == lh->dst && !ttx->is_request && + !ttx->done && NEXT_TH_SEQNO(DNP3_TH_SEQ(ttx->th)) == DNP3_TH_SEQ(th)) { tx = ttx; break; } @@ -987,53 +962,27 @@ static void DNP3HandleUserDataResponse(DNP3State *dnp3, const uint8_t *input, /* Replace the transport header in the transaction with this * one in case there are more frames. */ - tx->response_th = th; + tx->th = th; } else { ah = (DNP3ApplicationHeader *)(input + offset); offset += sizeof(DNP3ApplicationHeader); iin = (DNP3InternalInd *)(input + offset); - if (ah->function_code == DNP3_APP_FC_UNSOLICITED_RESP) { - tx = DNP3TxAlloc(dnp3); - if (unlikely(tx == NULL)) { - return; - } - - /* There is no request associated with an unsolicited - * response, so mark the request done as far as - * transaction state handling is concerned. */ - tx->request_done = 1; - } - else { - /* Find transaction. */ - TAILQ_FOREACH(ttx, &dnp3->tx_list, next) { - if (ttx->has_request && - ttx->request_done && - ttx->request_lh.src == lh->dst && - ttx->request_lh.dst == lh->src && - !ttx->has_response && - !ttx->response_done && - DNP3_APP_SEQ(ttx->request_ah.control) == DNP3_APP_SEQ(ah->control)) { - tx = ttx; - break; - } - } - if (tx == NULL) { - return; - } + tx = DNP3TxAlloc(dnp3, false); + if (unlikely(tx == NULL)) { + return; } - - tx->has_response = 1; - tx->response_lh = *lh; - tx->response_th = th; - tx->response_ah = *ah; - tx->response_iin = *iin; + tx->lh = *lh; + tx->th = th; + tx->ah = *ah; + tx->iin = *iin; } + BUG_ON(tx->is_request); + if (!DNP3ReassembleApplicationLayer(input + sizeof(DNP3LinkHeader), - input_len - sizeof(DNP3LinkHeader), - &tx->response_buffer, &tx->response_buffer_len)) { + input_len - sizeof(DNP3LinkHeader), &tx->buffer, &tx->buffer_len)) { DNP3SetEvent(dnp3, DNP3_DECODER_EVENT_MALFORMED); return; } @@ -1042,13 +991,12 @@ static void DNP3HandleUserDataResponse(DNP3State *dnp3, const uint8_t *input, return; } - tx->response_done = 1; + tx->done = 1; offset = sizeof(DNP3ApplicationHeader) + sizeof(DNP3InternalInd); - if (DNP3DecodeApplicationObjects(tx, tx->response_buffer + offset, - tx->response_buffer_len - offset, - &tx->response_objects)) { - tx->response_complete = 1; + if (DNP3DecodeApplicationObjects( + tx, tx->buffer + offset, tx->buffer_len - offset, &tx->objects)) { + tx->complete = 1; } } @@ -1385,12 +1333,8 @@ static void DNP3TxFree(DNP3Transaction *tx) { SCEnter(); - if (tx->request_buffer != NULL) { - SCFree(tx->request_buffer); - } - - if (tx->response_buffer != NULL) { - SCFree(tx->response_buffer); + if (tx->buffer != NULL) { + SCFree(tx->buffer); } AppLayerDecoderEventsFreeEvents(&tx->tx_data.events); @@ -1399,8 +1343,7 @@ static void DNP3TxFree(DNP3Transaction *tx) DetectEngineStateFree(tx->tx_data.de_state); } - DNP3TxFreeObjectList(&tx->request_objects); - DNP3TxFreeObjectList(&tx->response_objects); + DNP3TxFreeObjectList(&tx->objects); SCFree(tx); SCReturn; @@ -1491,11 +1434,30 @@ static int DNP3GetAlstateProgress(void *tx, uint8_t direction) SCReturnInt(1); } - if (direction & STREAM_TOCLIENT && dnp3tx->response_done) { - retval = 1; - } - else if (direction & STREAM_TOSERVER && dnp3tx->request_done) { - retval = 1; + /* This is a unidirectional protocol. + * + * If progress is being checked in the TOSERVER (request) + * direction, always return complete if the message is not a + * request, as there will never be replies on transactions created + * in the TOSERVER direction. + * + * Like wise, if progress is being checked in the TOCLIENT + * direction, requests will never be seen. So always return + * complete if the transaction is not a reply. + * + * Otherwise, if TOSERVER and transaction is a request, return + * complete if the transaction is complete. And if TOCLIENT and + * transaction is a response, return complete if the transaction + * is complete. + */ + if (direction & STREAM_TOSERVER) { + if (!dnp3tx->is_request || dnp3tx->complete) { + retval = 1; + } + } else if (direction & STREAM_TOCLIENT) { + if (dnp3tx->is_request || dnp3tx->complete) { + retval = 1; + } } SCReturnInt(retval); @@ -1626,6 +1588,12 @@ void RegisterDNP3Parsers(void) AppLayerParserRegisterTxDataFunc(IPPROTO_TCP, ALPROTO_DNP3, DNP3GetTxData); AppLayerParserRegisterStateDataFunc(IPPROTO_TCP, ALPROTO_DNP3, DNP3GetStateData); +#if 0 + /* While this parser is now fully unidirectional. setting this + * flag breaks detection at this time. */ + AppLayerParserRegisterOptionFlags( + IPPROTO_TCP, ALPROTO_DNP3, APP_LAYER_PARSER_OPT_UNIDIR_TXS); +#endif } else { SCLogConfig("Parser disabled for protocol %s. " @@ -2130,17 +2098,19 @@ static int DNP3ParserTestRequestResponse(void) FAIL_IF(tx == NULL); FAIL_IF(tx->tx_num != 1); FAIL_IF(tx != state->curr); - FAIL_IF(tx->request_buffer == NULL); - FAIL_IF(tx->request_buffer_len != 20); - FAIL_IF(tx->request_ah.function_code != DNP3_APP_FC_DIR_OPERATE); + FAIL_IF(tx->buffer == NULL); + FAIL_IF(tx->buffer_len != 20); + FAIL_IF(tx->ah.function_code != DNP3_APP_FC_DIR_OPERATE); SCMutexLock(&flow.m); FAIL_IF(AppLayerParserParse(NULL, alp_tctx, &flow, ALPROTO_DNP3, STREAM_TOCLIENT, response, sizeof(response))); SCMutexUnlock(&flow.m); - FAIL_IF(DNP3GetTx(state, 0) != tx); - FAIL_IF(!tx->response_done); - FAIL_IF(tx->response_buffer == NULL); + DNP3Transaction *tx0 = DNP3GetTx(state, 1); + FAIL_IF(tx0 == NULL); + FAIL_IF(tx0 == tx); + FAIL_IF(!tx0->done); + FAIL_IF(tx0->buffer == NULL); AppLayerParserThreadCtxFree(alp_tctx); StreamTcpFreeConfig(true); @@ -2197,19 +2167,19 @@ static int DNP3ParserTestUnsolicitedResponseConfirm(void) FAIL_IF(tx == NULL); FAIL_IF(tx->tx_num != 1); FAIL_IF(tx != state->curr); - FAIL_IF(tx->request_buffer != NULL); - FAIL_IF(tx->response_buffer == NULL); - FAIL_IF(!tx->response_done); - FAIL_IF(tx->response_ah.function_code != DNP3_APP_FC_UNSOLICITED_RESP); + FAIL_IF(!tx->done); + FAIL_IF(tx->ah.function_code != DNP3_APP_FC_UNSOLICITED_RESP); SCMutexLock(&flow.m); FAIL_IF(AppLayerParserParse(NULL, alp_tctx, &flow, ALPROTO_DNP3, STREAM_TOSERVER, confirm, sizeof(confirm))); SCMutexUnlock(&flow.m); - FAIL_IF(DNP3GetTx(state, 0) != tx); - FAIL_IF(!tx->response_done); - FAIL_IF(tx->response_buffer == NULL); - /* FAIL_IF(tx->iin1 != 0 || tx->iin2 != 0); */ + + /* Confirms are ignored currently. With the move to + unidirectional transactions it might be easy to support these + now. */ + DNP3Transaction *resptx = DNP3GetTx(state, 1); + FAIL_IF(resptx); AppLayerParserThreadCtxFree(alp_tctx); StreamTcpFreeConfig(true); @@ -2219,6 +2189,9 @@ static int DNP3ParserTestUnsolicitedResponseConfirm(void) /** * \test Test flood state. + * + * Note that flood state needs to revisited with the modification to a + * unidirectional protocol. */ static int DNP3ParserTestFlooded(void) { @@ -2263,10 +2236,11 @@ static int DNP3ParserTestFlooded(void) FAIL_IF(tx == NULL); FAIL_IF(tx->tx_num != 1); FAIL_IF(tx != state->curr); - FAIL_IF(tx->request_buffer == NULL); - FAIL_IF(tx->request_buffer_len != 20); - /* FAIL_IF(tx->app_function_code != DNP3_APP_FC_DIR_OPERATE); */ - FAIL_IF(tx->response_done); + FAIL_IF(tx->buffer == NULL); + FAIL_IF(tx->buffer_len != 20); + FAIL_IF(tx->ah.function_code != DNP3_APP_FC_DIR_OPERATE); + FAIL_IF_NOT(tx->done); + FAIL_IF_NOT(DNP3GetAlstateProgress(tx, STREAM_TOSERVER)); for (int i = 0; i < DNP3_DEFAULT_REQ_FLOOD_COUNT - 1; i++) { SCMutexLock(&flow.m); @@ -2275,7 +2249,7 @@ static int DNP3ParserTestFlooded(void) SCMutexUnlock(&flow.m); } FAIL_IF(state->flooded); - FAIL_IF(DNP3GetAlstateProgress(tx, 0)); + FAIL_IF_NOT(DNP3GetAlstateProgress(tx, STREAM_TOSERVER)); /* One more request should trip us into flooded state. */ SCMutexLock(&flow.m); @@ -2287,9 +2261,6 @@ static int DNP3ParserTestFlooded(void) /* Progress for the oldest tx should return 1. */ FAIL_IF(!DNP3GetAlstateProgress(tx, 0)); - /* But progress for the current state should still return 0. */ - FAIL_IF(DNP3GetAlstateProgress(state->curr, 0)); - AppLayerParserThreadCtxFree(alp_tctx); StreamTcpFreeConfig(true); FLOW_DESTROY(&flow); @@ -2393,9 +2364,9 @@ static int DNP3ParserTestPartialFrame(void) FAIL_IF(tx == NULL); FAIL_IF(tx->tx_num != 1); FAIL_IF(tx != state->curr); - FAIL_IF(tx->request_buffer == NULL); - FAIL_IF(tx->request_buffer_len != 20); - FAIL_IF(tx->request_ah.function_code != DNP3_APP_FC_DIR_OPERATE); + FAIL_IF(tx->buffer == NULL); + FAIL_IF(tx->buffer_len != 20); + FAIL_IF(tx->ah.function_code != DNP3_APP_FC_DIR_OPERATE); /* Send partial response. */ SCMutexLock(&flow.m); @@ -2405,7 +2376,8 @@ static int DNP3ParserTestPartialFrame(void) FAIL_IF(r != 0); FAIL_IF(state->response_buffer.len != sizeof(response_partial1)); FAIL_IF(state->response_buffer.offset != 0); - FAIL_IF(tx->response_done); + tx = DNP3GetTx(state, 1); + FAIL_IF_NOT_NULL(tx); /* Send rest of response. */ SCMutexLock(&flow.m); @@ -2418,10 +2390,11 @@ static int DNP3ParserTestPartialFrame(void) FAIL_IF(state->response_buffer.len != 0); FAIL_IF(state->response_buffer.offset != 0); - /* Transaction should be replied to now. */ - FAIL_IF(!tx->response_done); - FAIL_IF(tx->response_buffer == NULL); - FAIL_IF(tx->response_buffer_len == 0); + /* There should now be a response transaction. */ + tx = DNP3GetTx(state, 1); + FAIL_IF_NULL(tx); + FAIL_IF(tx->buffer == NULL); + FAIL_IF(tx->buffer_len == 0); AppLayerParserThreadCtxFree(alp_tctx); StreamTcpFreeConfig(true); @@ -2508,9 +2481,9 @@ static int DNP3ParserTestParsePDU01(void) FAIL_IF(pdus < 1); DNP3Transaction *dnp3tx = DNP3GetTx(dnp3state, 0); FAIL_IF_NULL(dnp3tx); - FAIL_IF(!dnp3tx->has_request); - FAIL_IF(TAILQ_EMPTY(&dnp3tx->request_objects)); - DNP3Object *object = TAILQ_FIRST(&dnp3tx->request_objects); + FAIL_IF(!dnp3tx->is_request); + FAIL_IF(TAILQ_EMPTY(&dnp3tx->objects)); + DNP3Object *object = TAILQ_FIRST(&dnp3tx->objects); FAIL_IF(object->group != 1 || object->variation != 0); FAIL_IF(object->count != 0); @@ -2548,8 +2521,8 @@ static int DNP3ParserDecodeG70V3Test(void) FAIL_IF(bytes != sizeof(pkt)); DNP3Transaction *tx = DNP3GetTx(dnp3state, 0); FAIL_IF_NULL(tx); - FAIL_IF_NOT(tx->has_request); - DNP3Object *obj = TAILQ_FIRST(&tx->request_objects); + FAIL_IF_NOT(tx->is_request); + DNP3Object *obj = TAILQ_FIRST(&tx->objects); FAIL_IF_NULL(obj); FAIL_IF_NOT(obj->group == 70); FAIL_IF_NOT(obj->variation == 3); diff --git a/src/app-layer-dnp3.h b/src/app-layer-dnp3.h index 94220c91c3..5b395dc9bd 100644 --- a/src/app-layer-dnp3.h +++ b/src/app-layer-dnp3.h @@ -208,37 +208,20 @@ typedef struct DNP3Transaction_ { AppLayerTxData tx_data; uint64_t tx_num; /**< Internal transaction ID. */ + bool is_request; /**< Is this tx a request? */ struct DNP3State_ *dnp3; - uint8_t has_request; - uint8_t request_done; - DNP3LinkHeader request_lh; - DNP3TransportHeader request_th; - DNP3ApplicationHeader request_ah; - uint8_t *request_buffer; /**< Reassembled request - * buffer. */ - uint32_t request_buffer_len; - uint8_t request_complete; /**< Was the decode - * complete. It will not be - * complete if we hit objects - * we do not know. */ - DNP3ObjectList request_objects; - - uint8_t has_response; - uint8_t response_done; - DNP3LinkHeader response_lh; - DNP3TransportHeader response_th; - DNP3ApplicationHeader response_ah; - DNP3InternalInd response_iin; - uint8_t *response_buffer; /**< Reassembed response - * buffer. */ - uint32_t response_buffer_len; - uint8_t response_complete; /**< Was the decode - * complete. It will not be - * complete if we hit objects - * we do not know. */ - DNP3ObjectList response_objects; + uint8_t *buffer; /**< Reassembled request buffer. */ + uint32_t buffer_len; + DNP3ObjectList objects; + DNP3LinkHeader lh; + DNP3TransportHeader th; + DNP3ApplicationHeader ah; + DNP3InternalInd iin; + uint8_t done; + uint8_t complete; /**< Was the decode complete. It will not be + complete if we hit objects we do not know. */ TAILQ_ENTRY(DNP3Transaction_) next; } DNP3Transaction; diff --git a/src/detect-dnp3.c b/src/detect-dnp3.c index f3d64c3b32..8099fea4c4 100644 --- a/src/detect-dnp3.c +++ b/src/detect-dnp3.c @@ -156,21 +156,17 @@ static InspectionBuffer *GetDNP3Data(DetectEngineThreadCtx *det_ctx, DNP3Transaction *tx = (DNP3Transaction *)txv; SCLogDebug("tx %p", tx); - const uint8_t *data = NULL; - uint32_t data_len = 0; - - if (flow_flags & STREAM_TOSERVER) { - data = tx->request_buffer; - data_len = tx->request_buffer_len; - } else if (flow_flags & STREAM_TOCLIENT) { - data = tx->response_buffer; - data_len = tx->response_buffer_len; + if ((flow_flags & STREAM_TOSERVER && !tx->is_request) || + (flow_flags & STREAM_TOCLIENT && tx->is_request)) { + return NULL; } - if (data == NULL || data_len == 0) + + if (tx->buffer == NULL || tx->buffer_len == 0) { return NULL; + } - SCLogDebug("tx %p data %p data_len %u", tx, data, data_len); - InspectionBufferSetup(det_ctx, list_id, buffer, data, data_len); + SCLogDebug("tx %p data %p data_len %u", tx, tx->buffer, tx->buffer_len); + InspectionBufferSetup(det_ctx, list_id, buffer, tx->buffer, tx->buffer_len); InspectionBufferApplyTransforms(buffer, transforms); } return buffer; @@ -425,11 +421,10 @@ static int DetectDNP3FuncMatch(DetectEngineThreadCtx *det_ctx, DetectDNP3 *detect = (DetectDNP3 *)ctx; int match = 0; - if (flags & STREAM_TOSERVER) { - match = detect->function_code == tx->request_ah.function_code; - } - else if (flags & STREAM_TOCLIENT) { - match = detect->function_code == tx->response_ah.function_code; + if (flags & STREAM_TOSERVER && tx->is_request) { + match = detect->function_code == tx->ah.function_code; + } else if (flags & STREAM_TOCLIENT && !tx->is_request) { + match = detect->function_code == tx->ah.function_code; } return match; @@ -443,11 +438,10 @@ static int DetectDNP3ObjMatch(DetectEngineThreadCtx *det_ctx, DetectDNP3 *detect = (DetectDNP3 *)ctx; DNP3ObjectList *objects = NULL; - if (flags & STREAM_TOSERVER) { - objects = &tx->request_objects; - } - else if (flags & STREAM_TOCLIENT) { - objects = &tx->response_objects; + if (flags & STREAM_TOSERVER && tx->is_request) { + objects = &tx->objects; + } else if (flags & STREAM_TOCLIENT && !tx->is_request) { + objects = &tx->objects; } if (objects != NULL) { @@ -471,8 +465,8 @@ static int DetectDNP3IndMatch(DetectEngineThreadCtx *det_ctx, DetectDNP3 *detect = (DetectDNP3 *)ctx; if (flags & STREAM_TOCLIENT) { - if ((tx->response_iin.iin1 & (detect->ind_flags >> 8)) || - (tx->response_iin.iin2 & (detect->ind_flags & 0xf))) { + if ((tx->iin.iin1 & (detect->ind_flags >> 8)) || + (tx->iin.iin2 & (detect->ind_flags & 0xf))) { return 1; } } diff --git a/src/output-json-alert.c b/src/output-json-alert.c index 69ab4dfd31..3ec980b83c 100644 --- a/src/output-json-alert.c +++ b/src/output-json-alert.c @@ -198,13 +198,13 @@ static void AlertJsonDnp3(const Flow *f, const uint64_t tx_id, JsonBuilder *js) jb_get_mark(js, &mark); bool logged = false; jb_open_object(js, "dnp3"); - if (tx->has_request && tx->request_done) { + if (tx->is_request && tx->done) { jb_open_object(js, "request"); JsonDNP3LogRequest(js, tx); jb_close(js); logged = true; } - if (tx->has_response && tx->response_done) { + if (!tx->is_request && tx->done) { jb_open_object(js, "response"); JsonDNP3LogResponse(js, tx); jb_close(js); diff --git a/src/output-json-dnp3.c b/src/output-json-dnp3.c index 5bf1e1359e..05d6aab048 100644 --- a/src/output-json-dnp3.c +++ b/src/output-json-dnp3.c @@ -145,27 +145,27 @@ void JsonDNP3LogRequest(JsonBuilder *js, DNP3Transaction *dnp3tx) JB_SET_STRING(js, "type", "request"); jb_open_object(js, "control"); - JsonDNP3LogLinkControl(js, dnp3tx->request_lh.control); + JsonDNP3LogLinkControl(js, dnp3tx->lh.control); jb_close(js); - jb_set_uint(js, "src", DNP3_SWAP16(dnp3tx->request_lh.src)); - jb_set_uint(js, "dst", DNP3_SWAP16(dnp3tx->request_lh.dst)); + jb_set_uint(js, "src", DNP3_SWAP16(dnp3tx->lh.src)); + jb_set_uint(js, "dst", DNP3_SWAP16(dnp3tx->lh.dst)); jb_open_object(js, "application"); jb_open_object(js, "control"); - JsonDNP3LogApplicationControl(js, dnp3tx->request_ah.control); + JsonDNP3LogApplicationControl(js, dnp3tx->ah.control); jb_close(js); - jb_set_uint(js, "function_code", dnp3tx->request_ah.function_code); + jb_set_uint(js, "function_code", dnp3tx->ah.function_code); - if (!TAILQ_EMPTY(&dnp3tx->request_objects)) { + if (!TAILQ_EMPTY(&dnp3tx->objects)) { jb_open_array(js, "objects"); - JsonDNP3LogObjects(js, &dnp3tx->request_objects); + JsonDNP3LogObjects(js, &dnp3tx->objects); jb_close(js); } - jb_set_bool(js, "complete", dnp3tx->request_complete); + jb_set_bool(js, "complete", dnp3tx->complete); /* Close application. */ jb_close(js); @@ -173,41 +173,40 @@ void JsonDNP3LogRequest(JsonBuilder *js, DNP3Transaction *dnp3tx) void JsonDNP3LogResponse(JsonBuilder *js, DNP3Transaction *dnp3tx) { - if (dnp3tx->response_ah.function_code == DNP3_APP_FC_UNSOLICITED_RESP) { + if (dnp3tx->ah.function_code == DNP3_APP_FC_UNSOLICITED_RESP) { JB_SET_STRING(js, "type", "unsolicited_response"); - } - else { + } else { JB_SET_STRING(js, "type", "response"); } jb_open_object(js, "control"); - JsonDNP3LogLinkControl(js, dnp3tx->response_lh.control); + JsonDNP3LogLinkControl(js, dnp3tx->lh.control); jb_close(js); - jb_set_uint(js, "src", DNP3_SWAP16(dnp3tx->response_lh.src)); - jb_set_uint(js, "dst", DNP3_SWAP16(dnp3tx->response_lh.dst)); + jb_set_uint(js, "src", DNP3_SWAP16(dnp3tx->lh.src)); + jb_set_uint(js, "dst", DNP3_SWAP16(dnp3tx->lh.dst)); jb_open_object(js, "application"); jb_open_object(js, "control"); - JsonDNP3LogApplicationControl(js, dnp3tx->response_ah.control); + JsonDNP3LogApplicationControl(js, dnp3tx->ah.control); jb_close(js); - jb_set_uint(js, "function_code", dnp3tx->response_ah.function_code); + jb_set_uint(js, "function_code", dnp3tx->ah.function_code); - if (!TAILQ_EMPTY(&dnp3tx->response_objects)) { + if (!TAILQ_EMPTY(&dnp3tx->objects)) { jb_open_array(js, "objects"); - JsonDNP3LogObjects(js, &dnp3tx->response_objects); + JsonDNP3LogObjects(js, &dnp3tx->objects); jb_close(js); } - jb_set_bool(js, "complete", dnp3tx->response_complete); + jb_set_bool(js, "complete", dnp3tx->complete); /* Close application. */ jb_close(js); jb_open_object(js, "iin"); - JsonDNP3LogIin(js, (uint16_t)(dnp3tx->response_iin.iin1 << 8 | dnp3tx->response_iin.iin2)); + JsonDNP3LogIin(js, (uint16_t)(dnp3tx->iin.iin1 << 8 | dnp3tx->iin.iin2)); jb_close(js); } @@ -218,20 +217,17 @@ static int JsonDNP3LoggerToServer(ThreadVars *tv, void *thread_data, LogDNP3LogThread *thread = (LogDNP3LogThread *)thread_data; DNP3Transaction *tx = vtx; - if (tx->has_request && tx->request_done) { - JsonBuilder *js = - CreateEveHeader(p, LOG_DIR_FLOW, "dnp3", NULL, thread->dnp3log_ctx->eve_ctx); - if (unlikely(js == NULL)) { - return TM_ECODE_OK; - } - - jb_open_object(js, "dnp3"); - JsonDNP3LogRequest(js, tx); - jb_close(js); - OutputJsonBuilderBuffer(js, thread->ctx); - jb_free(js); + JsonBuilder *js = CreateEveHeader(p, LOG_DIR_FLOW, "dnp3", NULL, thread->dnp3log_ctx->eve_ctx); + if (unlikely(js == NULL)) { + return TM_ECODE_OK; } + jb_open_object(js, "dnp3"); + JsonDNP3LogRequest(js, tx); + jb_close(js); + OutputJsonBuilderBuffer(js, thread->ctx); + jb_free(js); + SCReturnInt(TM_ECODE_OK); } @@ -242,20 +238,32 @@ static int JsonDNP3LoggerToClient(ThreadVars *tv, void *thread_data, LogDNP3LogThread *thread = (LogDNP3LogThread *)thread_data; DNP3Transaction *tx = vtx; - if (tx->has_response && tx->response_done) { - JsonBuilder *js = - CreateEveHeader(p, LOG_DIR_FLOW, "dnp3", NULL, thread->dnp3log_ctx->eve_ctx); - if (unlikely(js == NULL)) { - return TM_ECODE_OK; - } - - jb_open_object(js, "dnp3"); - JsonDNP3LogResponse(js, tx); - jb_close(js); - OutputJsonBuilderBuffer(js, thread->ctx); - jb_free(js); + JsonBuilder *js = CreateEveHeader(p, LOG_DIR_FLOW, "dnp3", NULL, thread->dnp3log_ctx->eve_ctx); + if (unlikely(js == NULL)) { + return TM_ECODE_OK; } + jb_open_object(js, "dnp3"); + JsonDNP3LogResponse(js, tx); + jb_close(js); + OutputJsonBuilderBuffer(js, thread->ctx); + jb_free(js); + + SCReturnInt(TM_ECODE_OK); +} + +static int JsonDNP3Logger(ThreadVars *tv, void *thread_data, const Packet *p, Flow *f, void *state, + void *vtx, uint64_t tx_id) +{ + SCEnter(); + DNP3Transaction *tx = vtx; + static int count = 0; + if (tx->is_request && tx->done) { + JsonDNP3LoggerToServer(tv, thread_data, p, f, state, vtx, tx_id); + } else if (!tx->is_request && tx->done) { + JsonDNP3LoggerToClient(tv, thread_data, p, f, state, vtx, tx_id); + } + SCLogNotice("count = %d", ++count); SCReturnInt(TM_ECODE_OK); } @@ -338,13 +346,7 @@ static TmEcode JsonDNP3LogThreadDeinit(ThreadVars *t, void *data) void JsonDNP3LogRegister(void) { - /* Register direction aware eve sub-modules. */ - OutputRegisterTxSubModuleWithProgress(LOGGER_JSON_DNP3_TS, "eve-log", - "JsonDNP3Log", "eve-log.dnp3", OutputDNP3LogInitSub, ALPROTO_DNP3, - JsonDNP3LoggerToServer, 0, 1, JsonDNP3LogThreadInit, - JsonDNP3LogThreadDeinit, NULL); - OutputRegisterTxSubModuleWithProgress(LOGGER_JSON_DNP3_TC, "eve-log", - "JsonDNP3Log", "eve-log.dnp3", OutputDNP3LogInitSub, ALPROTO_DNP3, - JsonDNP3LoggerToClient, 1, 1, JsonDNP3LogThreadInit, - JsonDNP3LogThreadDeinit, NULL); + OutputRegisterTxSubModule(LOGGER_JSON_TX, "eve-log", "JsonDNP3Log", "eve-log.dnp3", + OutputDNP3LogInitSub, ALPROTO_DNP3, JsonDNP3Logger, JsonDNP3LogThreadInit, + JsonDNP3LogThreadDeinit, NULL); } diff --git a/src/suricata-common.h b/src/suricata-common.h index 0b79005b27..1e7e316af1 100644 --- a/src/suricata-common.h +++ b/src/suricata-common.h @@ -446,13 +446,6 @@ typedef enum { LOGGER_TLS_STORE, LOGGER_TLS, LOGGER_JSON_TX, - - /* DNP3 loggers. These ones don't fit the common model of a - transaction logger yet so are left with their own IDs for - now. */ - LOGGER_JSON_DNP3_TS, - LOGGER_JSON_DNP3_TC, - LOGGER_FILE, LOGGER_FILEDATA, diff --git a/src/util-lua-dnp3.c b/src/util-lua-dnp3.c index 7a791a0db0..09f8694d6c 100644 --- a/src/util-lua-dnp3.c +++ b/src/util-lua-dnp3.c @@ -106,21 +106,21 @@ static void DNP3PushRequest(lua_State *luastate, DNP3Transaction *tx) /* Link header. */ lua_pushliteral(luastate, "link_header"); lua_newtable(luastate); - DNP3PushLinkHeader(luastate, &tx->request_lh); + DNP3PushLinkHeader(luastate, &tx->lh); lua_settable(luastate, -3); /* Transport header. */ - LUA_PUSHT_INT(luastate, "transport_header", tx->request_th); + LUA_PUSHT_INT(luastate, "transport_header", tx->th); /* Application header. */ lua_pushliteral(luastate, "application_header"); lua_newtable(luastate); - DNP3PushApplicationHeader(luastate, &tx->request_ah); + DNP3PushApplicationHeader(luastate, &tx->ah); lua_settable(luastate, -3); lua_pushliteral(luastate, "objects"); lua_newtable(luastate); - DNP3PushObjects(luastate, &tx->request_objects); + DNP3PushObjects(luastate, &tx->objects); lua_settable(luastate, -3); } @@ -129,25 +129,24 @@ static void DNP3PushResponse(lua_State *luastate, DNP3Transaction *tx) /* Link header. */ lua_pushliteral(luastate, "link_header"); lua_newtable(luastate); - DNP3PushLinkHeader(luastate, &tx->response_lh); + DNP3PushLinkHeader(luastate, &tx->lh); lua_settable(luastate, -3); /* Transport header. */ - LUA_PUSHT_INT(luastate, "transport_header", tx->response_th); + LUA_PUSHT_INT(luastate, "transport_header", tx->th); /* Application header. */ lua_pushliteral(luastate, "application_header"); lua_newtable(luastate); - DNP3PushApplicationHeader(luastate, &tx->response_ah); + DNP3PushApplicationHeader(luastate, &tx->ah); lua_settable(luastate, -3); /* Internal indicators. */ - LUA_PUSHT_INT(luastate, "indicators", - tx->response_iin.iin1 << 8 | tx->response_iin.iin2); + LUA_PUSHT_INT(luastate, "indicators", tx->iin.iin1 << 8 | tx->iin.iin2); lua_pushliteral(luastate, "objects"); lua_newtable(luastate); - DNP3PushObjects(luastate, &tx->response_objects); + DNP3PushObjects(luastate, &tx->objects); lua_settable(luastate, -3); } @@ -168,22 +167,22 @@ static int DNP3GetTx(lua_State *luastate) lua_pushinteger(luastate, tx->tx_num); lua_settable(luastate, -3); - LUA_PUSHT_INT(luastate, "has_request", tx->has_request); - if (tx->has_request) { + LUA_PUSHT_INT(luastate, "has_request", tx->is_request ? 1 : 0); + if (tx->is_request) { lua_pushliteral(luastate, "request"); lua_newtable(luastate); - LUA_PUSHT_INT(luastate, "done", tx->request_done); - LUA_PUSHT_INT(luastate, "complete", tx->request_complete); + LUA_PUSHT_INT(luastate, "done", tx->done); + LUA_PUSHT_INT(luastate, "complete", tx->complete); DNP3PushRequest(luastate, tx); lua_settable(luastate, -3); } - LUA_PUSHT_INT(luastate, "has_response", tx->has_response); - if (tx->has_response) { + LUA_PUSHT_INT(luastate, "has_response", tx->is_request ? 0 : 1); + if (!tx->is_request) { lua_pushliteral(luastate, "response"); lua_newtable(luastate); - LUA_PUSHT_INT(luastate, "done", tx->response_done); - LUA_PUSHT_INT(luastate, "complete", tx->response_complete); + LUA_PUSHT_INT(luastate, "done", tx->done); + LUA_PUSHT_INT(luastate, "complete", tx->complete); DNP3PushResponse(luastate, tx); lua_settable(luastate, -3); } diff --git a/src/util-profiling.c b/src/util-profiling.c index 0805ce6f96..55b5029fd2 100644 --- a/src/util-profiling.c +++ b/src/util-profiling.c @@ -1262,8 +1262,6 @@ const char * PacketProfileLoggertIdToString(LoggerId id) CASE_CODE(LOGGER_TLS_STORE); CASE_CODE(LOGGER_TLS); CASE_CODE(LOGGER_JSON_TX); - CASE_CODE(LOGGER_JSON_DNP3_TS); - CASE_CODE(LOGGER_JSON_DNP3_TC); CASE_CODE(LOGGER_FILE); CASE_CODE(LOGGER_FILEDATA); CASE_CODE (LOGGER_ALERT_DEBUG);