Project

General

Profile

Actions

Bug #5866

closed

detect: multi-tenancy crash

Added by Hongliang Liu almost 2 years ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Target version:
Affected Versions:
Effort:
Difficulty:
Label:

Description

The config file we use:

%YAML 1.1
---
af-packet:
  - interface: antrea-l7-tap0
    threads: auto
    cluster-id: 80
    cluster-type: cluster_flow
    defrag: no
    use-mmap: yes
    tpacket-v2: yes
    checksum-checks: no
    copy-mode: ips
    copy-iface: antrea-l7-tap1
  - interface:  antrea-l7-tap1
    threads: auto
    cluster-id: 81
    cluster-type: cluster_flow
    defrag: no
    use-mmap: yes
    tpacket-v2: yes
    checksum-checks: no
    copy-mode: ips
    copy-iface: antrea-l7-tap0
multi-detect:
  enabled: yes
  selector: vlan

The extra config above which is included in /etc/suricata/suricata.yaml, and Suricata is started with command:

suricata -c /etc/suricata/suricata.yaml --af-packet

How to reproduce the issue:

  1. There is a client (assuming its IP is 10.10.0.1) and server (assuming its IP is 10.10.0.2), and the connections between the client the server are enforced to pass Suricata. Note that, the client and the server are in VLAN 1.
  2. Open a terminal the on client, run the command as following. The connections are expected to be passed after Suricata rules are added in subsequent steps.
    for ((i=0;i<1000000;i++)) do curl http://10.10.0.2/api/v2/x;  done
    
  3. Open another terminal the on client, run the command as following. The connections are expected to be rejected after Suricata rules are added in subsequent steps.
    for ((i=0;i<1000000;i++)) do curl http://10.10.0.2/api/v1/x;  done
    
  4. Add a tenant. Note that, DO NOT stop the command in Step 2 and 3.
    1. Add a config file /etc/suricata/antrea-tenant-1.yaml for the tenant as following:
      %YAML 1.1
      
      ---
      default-rule-path: /etc/suricata/rules
      rule-files:
        - /etc/suricata/rules/antrea-l7-networkpolicy-1.rules
      
    2. Add a rule file /etc/suricata/rules/antrea-l7-networkpolicy-1.rules for the tenant as following:
      reject ip any any -> any any (msg: "Reject by AntreaClusterNetworkPolicy:ingress-allow-http-request-to-api-v2"; flow: to_server, established; sid: 1;)
      pass http any any -> any any (msg: "Allow http by AntreaClusterNetworkPolicy:ingress-allow-http-request-to-api-v2"; http.uri; content:"/api/v2/"; startswith; http.method; content:"GET"; sid: 2;)
      
    3. Register the tenant with the command as following:
      suricatasc -c "register-tenant 1 /etc/suricata/antrea-tenant-1.yaml" 
      
    4. Register the tenant handler with the command as following:
      suricatasc -c "register-tenant-handler 1 vlan 1" 
      
  5. After a few seconds, delete the tenant. Note that, DO NOT stop the command in Step 2 and 3.
    1. Unregister the tenant handler with the command as following:
      suricatasc -c "unregister-tenant-handler 1 vlan 1" 
      
    2. Unregister the tenant with the command as following:
      suricatasc -c "register-tenant 1" 
      
    3. Delete file /etc/suricata/antrea-tenant-1.yaml.
    4. Delete file /etc/suricata/rules/antrea-l7-networkpolicy-1.rules.
  6. Repeat Step 4 and Step 5 several times, stop at Step 4 finally, which means that the tenant is still there and corresponding rules take effect. Generally, the Suricata process will get Segment fault(coredumped) during repeating Step 4 and Step 5, or after stoping repeating for a while.
  7. If the Suricata process is still in good shape, stop the command in Step 2 and run it again for a while, the Suricata process might get Segment fault(coredumped) too.

Coredumped files. I got two coredumped files and open it with gdb. We can see the the proccess is broken at this line: https://github.com/OISF/suricata/blob/49713ebaa0b8edb057d60f1cfe9126946645a848/src/detect.c#L362

The value of det_ctx->non_pf_store_cnt should be modified unexpectedlly.


Files


Subtasks 1 (0 open1 closed)

Bug #5951: detect: multi-tenancy crash (6.0.x backport)ClosedPhilippe AntoineActions
Actions #2

Updated by Hongliang Liu almost 2 years ago

  • File deleted (clipboard-202302160933-5zio1.png)
Actions #3

Updated by Hongliang Liu almost 2 years ago

  • Assignee changed from OISF Dev to Victor Julien
Actions #4

Updated by Victor Julien almost 2 years ago

  • Assignee changed from Victor Julien to Philippe Antoine
Actions #5

Updated by Philippe Antoine almost 2 years ago

  • Status changed from New to In Review
Actions #6

Updated by Hongliang Liu almost 2 years ago

Philippe Antoine wrote in #note-5:

@Hongliang Liu does https://github.com/OISF/suricata/pull/8611 solve the issue ?

Thanks for updating the issue, I'll make a quick test according to your patch. BTW, will you backport the patch to old releases like 6.0.x we are using? Thanks.

Actions #7

Updated by Philippe Antoine almost 2 years ago

  • Label Needs backport added

Indeed, it should be backported

Actions #8

Updated by Philippe Antoine almost 2 years ago

  • Target version changed from TBD to 7.0.0-rc2
Actions #9

Updated by Victor Julien almost 2 years ago

  • Subject changed from af-packet/ips: Suricata process exits with segment fault (coredumped) to detect: multi-tenancy crash
  • Label Needs backport to 6.0 added
  • Label deleted (Needs backport)
Actions #10

Updated by Hongliang Liu almost 2 years ago

  • Label Needs backport added
  • Label deleted (Needs backport to 6.0)

Thanks a lot, guys. It works perfectly with the patch! BTW, will you guys include this patch in the new 6.0.11 release? When will you release 6.0.11?

Actions #11

Updated by Philippe Antoine almost 2 years ago

  • Label Needs backport to 6.0 added
  • Label deleted (Needs backport)

First, we need to merge the fix in the master branch, then we will be able to backport it

Actions #12

Updated by OISF Ticketbot almost 2 years ago

  • Subtask #5951 added
Actions #13

Updated by OISF Ticketbot almost 2 years ago

  • Label deleted (Needs backport to 6.0)
Actions #14

Updated by Philippe Antoine almost 2 years ago

  • Status changed from In Review to Resolved
Actions #15

Updated by Philippe Antoine over 1 year ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF