Bypass v7 #2276

regit · 2016-09-24T12:53:14Z

This is an update of #2228 changing a lot of things.

Main change is that it introduces a bypassed state in flow which will allow suricata to get rid fast of bypassed flows instead of waiting for "established" timeout. This will lower the pressure on the flow table. Also as the bypassed information is added in the EVE entry of flow the user will know that bypass has been done on the flow.

Another change is that definition of bypass is not done anymore at registration but it is done inside the capture. For instance, in NFQ capture (which is the only one implemented in this PR), the bypass is activated if the bypass_mask is set in the configuration file.

Also remarks on PR #2228 have been addressed.

PR builds:

PR regit: https://buildbot.openinfosecfoundation.org/builders/regit/builds/193
PR regit-pcap: https://buildbot.openinfosecfoundation.org/builders/regit-pcap/builds/189

This patch adds a bypassed state to flow that will be set on flow when a bypass decision is taken. This will allow to remove faster the flow entry from the flow table instead of waiting for the "established" timeout.

This will allow us to remove the Flow from the table and treat it separately.

We possibly have packets for a flow in the queue and we don't need to bypass for each of them if the flow is already bypassed.

This permits to enable/disable in suricata.yaml and the bypass function will be called when stream.depth is reached.

If a flow is encrypted, suricata is stopping the inspection so we can just get it out via bypass. This patch also invert to test to check the option value. This should limit the number of tests in conditional. For a basic test of feature, use the following ruleset: ``` table ip filter { chain output { type filter hook output priority 0; meta mark set ct mark counter mark 0x00000001 counter accept oif lo tcp dport ssh counter queue num 0 oif lo tcp sport ssh counter queue num 0 } chain connmark_save { type filter hook output priority 1; mark 0x00000001 counter ct mark set mark counter } } ``` And use mark and mask of 1 in nfq configuration. Then you can test the system by scp big file to 127.0.0.1.

This adds a new keyword which permits to call the bypass callback when a sig is matched. The callback must be called when the match of the sig is complete.

If a packet triggers a rule which contains both bypass and filestore keywords, it won't be stored since it's not inspected. To avoid that, when a rule containing filestore keyword we make sure that also bypass keyword is present.

inliniac · 2016-09-24T18:03:36Z

src/decode.c

+
+    if (p->BypassPacketsFlow) {
+        /* only set bypassed state if succesful */
+        SC_ATOMIC_SET(p->flow->flow_state, FLOW_STATE_BYPASSED);


Use FlowUpdateState

inliniac · 2016-09-24T18:04:23Z

src/decode.h

@@ -448,6 +448,8 @@ typedef struct Packet_
    /** The release function for packet structure and data */
    void (*ReleasePacket)(struct Packet_ *);

+    void (*BypassPacketsFlow)(struct Packet_ *);


missing comment explaining what this is for

inliniac · 2016-09-24T18:05:23Z

src/detect-bypass.c

+
+    SCMutexLock(&f.m);
+    int r = AppLayerParserParse(alp_tctx, &f, ALPROTO_HTTP, STREAM_TOSERVER, http_buf1, http_len1);
+    if (r != 0) {


use FAIL_IF

inliniac · 2016-09-24T18:05:32Z

src/detect-bypass.c

+
+    SCMutexLock(&f.m);
+    r = AppLayerParserParse(alp_tctx, &f, ALPROTO_HTTP, STREAM_TOCLIENT, http_buf2, http_len2);
+    if (r != 0) {


inliniac · 2016-09-24T18:06:06Z

src/detect-bypass.c

+
+    FAIL_IF(PacketAlertCheck(p1, 1));
+
+    SCMutexLock(&f.m);


wrong lock func, use FLOWLOCK_WRLOCK

(I have a patch that mass changes this for the entire tree)

inliniac · 2016-09-24T18:06:42Z

src/detect-filestore.c

+    StreamTcpInitConfig(TRUE);
+
+    de_ctx = DetectEngineCtxInit();
+    if (de_ctx == NULL)


FAIL_IF

Please update the rest of the unittests as well

inliniac · 2016-09-24T18:09:15Z

src/source-nfq.c

@@ -507,6 +524,10 @@ static int NFQCallBack(struct nfq_q_handle *qh, struct nfgenmsg *nfmsg,
    PKT_SET_SRC(p, PKT_SRC_WIRE);

    p->nfq_v.nfq_index = ntv->nfq_index;
+    /* if bypass mask is set then we may want to bypass so set pointer */
+    if (nfq_config.bypass_mask) {


so this mask is used as a global switch to see if bypass is enabled at all for nfq?

yes, it may be a bit implicit but I was seeing a new variable has bit too much.

inliniac · 2016-09-24T18:10:47Z

src/detect-filestore.c

+    SigGroupBuild(de_ctx);
+    DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx);
+
+    SCMutexLock(&f.m);


FLOWLOCK_WRLOCK

inliniac · 2016-09-24T18:12:13Z

src/source-nfq.c

@@ -492,6 +501,14 @@ static void NFQReleasePacket(Packet *p)
    PacketFreeOrRelease(p);
 }

+static void NFQBypassCallback(Packet *p)


this will need to act on the root packet in case of tunnel packets

regit · 2016-09-24T19:18:04Z

ok, updating the branch.

An empty value for coredump.max-dump in the config-file leads to a segfault because of a NULL-pointer dereference in CoredumpLoadConfig(). Here is a configuration example: coredump.max-dump: [] This lets suricata crash with a segfault: ASAN-output: ==9412==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f22e851aa28 bp 0x7ffd90006fc0 sp 0x7ffd90006740 T0) 0 0x7f22e851aa27 in strcasecmp (/usr/lib/x86_64-linux-gnu/libasan.so.3+0x51a27) 1 0x5608a7ec0108 in CoredumpLoadConfig /root/suricata-1/src/util-coredump-config.c:52 2 0x5608a7e8bb22 in PostConfLoadedSetup /root/suricata-1/src/suricata.c:2752 3 0x5608a7e8c577 in main /root/suricata-1/src/suricata.c:2892 4 0x7f22e4c622b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0) 5 0x5608a7a30c59 in _start (/usr/local/bin/suricata+0xc4c59) Bug OISF#2276

Giuseppe Longo and others added 12 commits September 23, 2016 13:42

packet: add API for bypass

5a9afec

flow: add bypassed state

463fa65

This patch adds a bypassed state to flow that will be set on flow when a bypass decision is taken. This will allow to remove faster the flow entry from the flow table instead of waiting for the "established" timeout.

flow-bypass: display info about bypass in log

d7156c6

decode: tag Flow as bypassed when packet is

fea55b1

This will allow us to remove the Flow from the table and treat it separately.

decode: don't bypass if flow already bypassed

b9b9ae9

We possibly have packets for a flow in the queue and we don't need to bypass for each of them if the flow is already bypassed.

decode: don't try multiple bypass if failed once

f219c69

nfq: introduce bypass function

a7f02cf

stream-tcp: enable bypass setting

dd08bb8

This permits to enable/disable in suricata.yaml and the bypass function will be called when stream.depth is reached.

bypass: add bypass keyword

c76d545

This adds a new keyword which permits to call the bypass callback when a sig is matched. The callback must be called when the match of the sig is complete.

filestore: avoid conflict with bypass keyword

5b72953

If a packet triggers a rule which contains both bypass and filestore keywords, it won't be stored since it's not inspected. To avoid that, when a rule containing filestore keyword we make sure that also bypass keyword is present.

flow: add pruned bypassed flow counter

fb5bcb6

inliniac reviewed Sep 24, 2016

View reviewed changes

regit closed this Sep 24, 2016

regit mentioned this pull request Sep 24, 2016

Bypass v8 #2278

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bypass v7 #2276

Bypass v7 #2276

regit commented Sep 24, 2016

inliniac Sep 24, 2016

inliniac Sep 24, 2016

inliniac Sep 24, 2016

inliniac Sep 24, 2016

inliniac Sep 24, 2016

inliniac Sep 24, 2016

inliniac Sep 24, 2016

regit Sep 24, 2016

inliniac Sep 24, 2016

inliniac Sep 24, 2016

regit commented Sep 24, 2016

Bypass v7 #2276

Bypass v7 #2276

Conversation

regit commented Sep 24, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

regit commented Sep 24, 2016