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.

diff --git a/src/app-layer-detect-proto.h b/src/app-layer-detect-proto.h
index 18b441f9503ffa55c69c907e817c73ee208abca4..23332d9dc995f3693c93f0e5821014dd314b62bf 100644 (file)
--- 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 93adcef1875f12973b8d418b8f498f62a08118ed..5fc2a4d62ca661d1079fbdfcda9aa22a30aa73bb 100644 (file)
--- 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 175953c64637d3abd3dd5c9f918b41438cb5d2c2..194c03520fd1b8df905f4d1ffdfb8ce0bb205e07 100644 (file)
--- 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 63da76b564f3d32fc5e7909f588800dba394cd83..79ed5577fb3921d6823fcac805b591ef4db0ea1a 100644 (file)
--- 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 3ea9e91de726ab4ed195df6244c3e1bc2f33cf50..1d453dd96dea91f17fd85eba251a1749c0fd4869 100644 (file)
--- 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 &