From 1ea5d27508f09fd202838041556d5efe59e43ac9 Mon Sep 17 00:00:00 2001 From: Anoop Saldanha Date: Thu, 3 Oct 2013 10:12:54 +0530 Subject: [PATCH] Fix for bug #989. In case of recursive call to protocol detection from within protocol detection, and the recursively invoked stream still hasn't been ack'ed yet, protocol detection doesn't take place. In such cases we will end up still calling the app layer with the wrong direction data. Introduce a check to not call app layer with wrong direction data. When sockets are re-used reset all relevant vars correctly. This commit fixes a bug where we were not reseting app proto detection vars. While fixing #989, we discovered some other bugs which have also been fixed, or rather some features which are now updated. One of the feature update being if we recieve wrong direction data first, we don't reset the protocol values for the flow. We let the flow retain the detected values. Unittests have been modified to accomodate the above change. --- src/app-layer-detect-proto.h | 3 ++ src/app-layer-htp.c | 5 ++- src/app-layer.c | 77 +++++++++++++++++++++++++++++------- src/stream-tcp-private.h | 2 + src/stream-tcp.c | 2 + 5 files changed, 74 insertions(+), 15 deletions(-) diff --git a/src/app-layer-detect-proto.h b/src/app-layer-detect-proto.h index 18b441f950..23332d9dc9 100644 --- a/src/app-layer-detect-proto.h +++ b/src/app-layer-detect-proto.h @@ -78,6 +78,9 @@ extern AlpProtoDetectCtx alp_proto_ctx; #define FLOW_SET_PM_DONE(f, dir) (((dir) & STREAM_TOSERVER) ? ((f)->flags |= FLOW_TS_PM_ALPROTO_DETECT_DONE) : ((f)->flags |= FLOW_TC_PM_ALPROTO_DETECT_DONE)) #define FLOW_SET_PP_DONE(f, dir) (((dir) & STREAM_TOSERVER) ? ((f)->flags |= FLOW_TS_PP_ALPROTO_DETECT_DONE) : ((f)->flags |= FLOW_TC_PP_ALPROTO_DETECT_DONE)) +#define FLOW_RESET_PM_DONE(f, dir) (((dir) & STREAM_TOSERVER) ? ((f)->flags &= ~FLOW_TS_PM_ALPROTO_DETECT_DONE) : ((f)->flags &= ~FLOW_TC_PM_ALPROTO_DETECT_DONE)) +#define FLOW_RESET_PP_DONE(f, dir) (((dir) & STREAM_TOSERVER) ? ((f)->flags &= ~FLOW_TS_PP_ALPROTO_DETECT_DONE) : ((f)->flags &= ~FLOW_TC_PP_ALPROTO_DETECT_DONE)) + void AlpProtoInit(AlpProtoDetectCtx *); void *AppLayerDetectProtoThread(void *td); diff --git a/src/app-layer-htp.c b/src/app-layer-htp.c index 93adcef187..5fc2a4d62c 100644 --- a/src/app-layer-htp.c +++ b/src/app-layer-htp.c @@ -761,7 +761,10 @@ static int HTPHandleResponseData(Flow *f, void *htp_state, hstate->f = f; if (hstate->connp == NULL) { SCLogError(SC_ERR_ALPARSER, "HTP state has no connp"); - SCReturnInt(-1); + /* till we have the new libhtp changes that allow response first, + * let's take response in first. */ + BUG_ON(1); + //SCReturnInt(-1); } /* Unset the body inspection (the callback should diff --git a/src/app-layer.c b/src/app-layer.c index 175953c646..194c03520f 100644 --- a/src/app-layer.c +++ b/src/app-layer.c @@ -192,9 +192,6 @@ int AppLayerHandleTCPData(ThreadVars *tv, TcpReassemblyThreadCtx *ra_ctx, if (*alproto_otherdir != ALPROTO_UNKNOWN && *alproto_otherdir != *alproto) { AppLayerDecoderEventsSetEventRaw(p->app_layer_events, APPLAYER_MISMATCH_PROTOCOL_BOTH_DIRECTIONS); - FlowSetSessionNoApplayerInspectionFlag(f); - StreamTcpSetStreamFlagAppProtoDetectionCompleted(&ssn->client); - StreamTcpSetStreamFlagAppProtoDetectionCompleted(&ssn->server); /* it indicates some data has already been sent to the parser */ if (ssn->data_first_seen_dir == APP_LAYER_DATA_ALREADY_SENT_TO_APP_LAYER) { f->alproto = *alproto = *alproto_otherdir; @@ -238,12 +235,14 @@ int AppLayerHandleTCPData(ThreadVars *tv, TcpReassemblyThreadCtx *ra_ctx, p->flowflags |= FLOW_PKT_TOCLIENT; } } - int ret; - if (StreamTcpInlineMode()) - ret = StreamTcpReassembleInlineAppLayer(tv, ra_ctx, ssn, opposing_stream, p); - else - ret = StreamTcpReassembleAppLayer(tv, ra_ctx, ssn, opposing_stream, p); + if (StreamTcpInlineMode()) { + ret = StreamTcpReassembleInlineAppLayer(tv, ra_ctx, ssn, + opposing_stream, p); + } else { + ret = StreamTcpReassembleAppLayer(tv, ra_ctx, ssn, + opposing_stream, p); + } if (stream == &ssn->client) { if (StreamTcpInlineMode()) { p->flowflags &= ~FLOW_PKT_TOCLIENT; @@ -262,6 +261,9 @@ int AppLayerHandleTCPData(ThreadVars *tv, TcpReassemblyThreadCtx *ra_ctx, } } if (ret < 0) { + FlowSetSessionNoApplayerInspectionFlag(f); + StreamTcpSetStreamFlagAppProtoDetectionCompleted(&ssn->client); + StreamTcpSetStreamFlagAppProtoDetectionCompleted(&ssn->server); r = -1; goto end; } @@ -289,13 +291,31 @@ int AppLayerHandleTCPData(ThreadVars *tv, TcpReassemblyThreadCtx *ra_ctx, { AppLayerDecoderEventsSetEventRaw(p->app_layer_events, APPLAYER_WRONG_DIRECTION_FIRST_DATA); - r = -1; - f->alproto = f->alproto_ts = f->alproto_tc = ALPROTO_UNKNOWN; FlowSetSessionNoApplayerInspectionFlag(f); StreamTcpSetStreamFlagAppProtoDetectionCompleted(&ssn->server); StreamTcpSetStreamFlagAppProtoDetectionCompleted(&ssn->client); /* Set a value that is neither STREAM_TOSERVER, nor STREAM_TOCLIENT */ ssn->data_first_seen_dir = APP_LAYER_DATA_ALREADY_SENT_TO_APP_LAYER; + r = -1; + goto end; + } + /* This can happen if the current direction is not the + * right direction, and the data from the other(also + * the right direction) direction is available to be sent + * to the app layer, but it is not ack'ed yet and hence + * the forced call to STreamTcpAppLayerReassemble still + * hasn't managed to send data from the other direction + * to the app layer. */ + if (al_proto_table[*alproto].first_data_dir && + !(al_proto_table[*alproto].first_data_dir & flags)) + { + BUG_ON(*alproto_otherdir != ALPROTO_UNKNOWN); + AppLayerParserCleanupState(f); + f->alproto = *alproto = ALPROTO_UNKNOWN; + StreamTcpResetStreamFlagAppProtoDetectionCompleted(stream); + FLOW_RESET_PM_DONE(f, flags); + FLOW_RESET_PP_DONE(f, flags); + r = 0; goto end; } } @@ -309,6 +329,35 @@ int AppLayerHandleTCPData(ThreadVars *tv, TcpReassemblyThreadCtx *ra_ctx, f->data_al_so_far[dir] = 0; } else { if (*alproto_otherdir != ALPROTO_UNKNOWN) { + /* this would handle this test case - + * http parser which says it wants to see toserver data first only. + * tcp handshake + * toclient data first received. - RUBBISH DATA which + * we don't detect as http + * toserver data next sent - we detect this as http. + * at this stage we see that toclient is the first data seen + * for this session and we try and redetect the app protocol, + * but we are unable to detect the app protocol like before. + * But since we have managed to detect the protocol for the + * other direction as http, we try to use that. At this + * stage we check if the direction of this stream matches + * to that acceptable by the app parser. If it is not the + * acceptable direction we error out. + */ + if ((ssn->data_first_seen_dir != APP_LAYER_DATA_ALREADY_SENT_TO_APP_LAYER) && + (al_proto_table[*alproto_otherdir].first_data_dir) && + !(al_proto_table[*alproto_otherdir].first_data_dir & flags)) + { + r = -1; + FlowSetSessionNoApplayerInspectionFlag(f); + StreamTcpSetStreamFlagAppProtoDetectionCompleted(&ssn->server); + StreamTcpSetStreamFlagAppProtoDetectionCompleted(&ssn->client); + goto end; + } + + if (data_len > 0) + ssn->data_first_seen_dir = APP_LAYER_DATA_ALREADY_SENT_TO_APP_LAYER; + PACKET_PROFILING_APP_START(dp_ctx, *alproto_otherdir); r = AppLayerParse(dp_ctx->alproto_local_storage[*alproto_otherdir], f, *alproto_otherdir, flags, data + data_al_so_far, data_len - data_al_so_far); @@ -324,10 +373,10 @@ int AppLayerHandleTCPData(ThreadVars *tv, TcpReassemblyThreadCtx *ra_ctx, } else { if (FLOW_IS_PM_DONE(f, STREAM_TOSERVER) && FLOW_IS_PP_DONE(f, STREAM_TOSERVER) && FLOW_IS_PM_DONE(f, STREAM_TOCLIENT) && FLOW_IS_PP_DONE(f, STREAM_TOCLIENT)) { + FlowSetSessionNoApplayerInspectionFlag(f); StreamTcpSetStreamFlagAppProtoDetectionCompleted(&ssn->server); StreamTcpSetStreamFlagAppProtoDetectionCompleted(&ssn->client); ssn->data_first_seen_dir = APP_LAYER_DATA_ALREADY_SENT_TO_APP_LAYER; - FlowSetSessionNoApplayerInspectionFlag(f); } } } @@ -2003,9 +2052,9 @@ static int AppLayerTest06(void) goto end; if (!StreamTcpIsSetStreamFlagAppProtoDetectionCompleted(&ssn->server) || !StreamTcpIsSetStreamFlagAppProtoDetectionCompleted(&ssn->client) || - f.alproto != ALPROTO_UNKNOWN || + f.alproto != ALPROTO_HTTP || f.alproto_ts != ALPROTO_UNKNOWN || - f.alproto_tc != ALPROTO_UNKNOWN || + f.alproto_tc != ALPROTO_HTTP || f.data_al_so_far[0] != 0 || f.data_al_so_far[1] != 0 || !(f.flags & FLOW_NO_APPLAYER_INSPECTION) || @@ -2246,7 +2295,7 @@ static int AppLayerTest07(void) f.alproto_tc != ALPROTO_HTTP || f.data_al_so_far[0] != 0 || f.data_al_so_far[1] != 0 || - !(f.flags & FLOW_NO_APPLAYER_INSPECTION) || + (f.flags & FLOW_NO_APPLAYER_INSPECTION) || !FLOW_IS_PM_DONE(&f, STREAM_TOSERVER) || FLOW_IS_PP_DONE(&f, STREAM_TOSERVER) || !FLOW_IS_PM_DONE(&f, STREAM_TOCLIENT) || FLOW_IS_PP_DONE(&f, STREAM_TOCLIENT) || ssn->data_first_seen_dir != APP_LAYER_DATA_ALREADY_SENT_TO_APP_LAYER) { diff --git a/src/stream-tcp-private.h b/src/stream-tcp-private.h index 63da76b564..79ed5577fb 100644 --- a/src/stream-tcp-private.h +++ b/src/stream-tcp-private.h @@ -224,5 +224,7 @@ typedef struct TcpSession_ { ((stream)->flags |= STREAMTCP_STREAM_FLAG_APPPROTO_DETECTION_COMPLETED) #define StreamTcpIsSetStreamFlagAppProtoDetectionCompleted(stream) \ ((stream)->flags & STREAMTCP_STREAM_FLAG_APPPROTO_DETECTION_COMPLETED) +#define StreamTcpResetStreamFlagAppProtoDetectionCompleted(stream) \ + ((stream)->flags &= ~STREAMTCP_STREAM_FLAG_APPPROTO_DETECTION_COMPLETED); #endif /* __STREAM_TCP_PRIVATE_H__ */ diff --git a/src/stream-tcp.c b/src/stream-tcp.c index 3ea9e91de7..1d453dd96d 100644 --- a/src/stream-tcp.c +++ b/src/stream-tcp.c @@ -4283,6 +4283,8 @@ int StreamTcpPacket (ThreadVars *tv, Packet *p, StreamTcpThread *stt, StreamTcpPacketSetState(p, ssn, TCP_NONE); p->flow->alproto_ts = p->flow->alproto_tc = p->flow->alproto = ALPROTO_UNKNOWN; + p->flow->data_al_so_far[0] = p->flow->data_al_so_far[1] = 0; + ssn->data_first_seen_dir = 0; p->flow->flags &= (~FLOW_TS_PM_ALPROTO_DETECT_DONE & ~FLOW_TS_PP_ALPROTO_DETECT_DONE & ~FLOW_TC_PM_ALPROTO_DETECT_DONE &