[PATCH 1/3] suricata: Update config file.

[PATCH 1/3] suricata: Update config file.

[Stefan Schantl]

[-- Attachment #1: Type: text/plain, Size: 4350 bytes --]

* This will enable swf decompression.
* Enable modbus parser.
* Enable dnp3 parser.
* Enable enip parser.

Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
---
 config/suricata/suricata.yaml | 84 +++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/config/suricata/suricata.yaml b/config/suricata/suricata.yaml
index 0ad36e705..49921db86 100644
--- a/config/suricata/suricata.yaml
+++ b/config/suricata/suricata.yaml
@@ -525,6 +525,20 @@ app-layer:
            # auto will use http-body-inline mode in IPS mode, yes or no set it statically
            http-body-inline: auto
 
+           # Decompress SWF files.
+           # 2 types: 'deflate', 'lzma', 'both' will decompress deflate and lzma
+           # compress-depth:
+           # Specifies the maximum amount of data to decompress,
+           # set 0 for unlimited.
+           # decompress-depth:
+           # Specifies the maximum amount of decompressed data to obtain,
+           # set 0 for unlimited.
+           swf-decompression:
+             enabled: yes
+             type: both
+             compress-depth: 0
+             decompress-depth: 0
+
            # Take a random value for inspection sizes around the specified value.
            # This lower the risk of some evasion technics but could lead
            # detection change between runs. It is set to 'yes' by default.
@@ -539,6 +553,76 @@ app-layer:
            double-decode-path: no
            double-decode-query: no
 
+           # Can disable LZMA decompression
+           #lzma-enabled: yes
+           # Memory limit usage for LZMA decompression dictionary
+           # Data is decompressed until dictionary reaches this size
+           #lzma-memlimit: 1mb
+           # Maximum decompressed size with a compression ratio
+           # above 2048 (only LZMA can reach this ratio, deflate cannot)
+           #compression-bomb-limit: 1mb
+           # Maximum time spent decompressing a single transaction in usec
+           #decompression-time-limit: 100000
+
+         server-config:
+
+           #- apache:
+           #    address: [192.168.1.0/24, 127.0.0.0/8, "::1"]
+           #    personality: Apache_2
+           #    # Can be specified in kb, mb, gb.  Just a number indicates
+           #    # it's in bytes.
+           #    request-body-limit: 4096
+           #    response-body-limit: 4096
+           #    double-decode-path: no
+           #    double-decode-query: no
+
+           #- iis7:
+           #    address:
+           #      - 192.168.0.0/24
+           #      - 192.168.10.0/24
+           #    personality: IIS_7_0
+           #    # Can be specified in kb, mb, gb.  Just a number indicates
+           #    # it's in bytes.
+           #    request-body-limit: 4096
+           #    response-body-limit: 4096
+           #    double-decode-path: no
+           #    double-decode-query: no
+
+    # Note: Modbus probe parser is minimalist due to the poor significant field
+    # Only Modbus message length (greater than Modbus header length)
+    # And Protocol ID (equal to 0) are checked in probing parser
+    # It is important to enable detection port and define Modbus port
+    # to avoid false positive
+    modbus:
+      # How many unreplied Modbus requests are considered a flood.
+      # If the limit is reached, app-layer-event:modbus.flooded; will match.
+      #request-flood: 500
+
+      enabled: yes
+      detection-ports:
+        dp: 502
+      # According to MODBUS Messaging on TCP/IP Implementation Guide V1.0b, it
+      # is recommended to keep the TCP connection opened with a remote device
+      # and not to open and close it for each MODBUS/TCP transaction. In that
+      # case, it is important to set the depth of the stream reassembling as
+      # unlimited (stream.reassembly.depth: 0)
+
+      # Stream reassembly size for modbus. By default track it completely.
+      stream-depth: 0
+
+    # DNP3
+    dnp3:
+      enabled: yes
+      detection-ports:
+        dp: 20000
+
+    # SCADA EtherNet/IP and CIP protocol support
+    enip:
+      enabled: yes
+      detection-ports:
+        dp: 44818
+        sp: 44818
+
     ntp:
       enabled: yes
     dhcp:
-- 
2.30.2

[PATCH 2/3] suricata: Move default loaded rulefiles to own included file.

[Stefan Schantl]

[-- Attachment #1: Type: text/plain, Size: 4322 bytes --]

Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
---
 config/rootfiles/common/suricata            |  1 +
 config/suricata/suricata-default-rules.yaml | 22 ++++++++++++++++++
 config/suricata/suricata.yaml               | 25 ++++-----------------
 lfs/suricata                                |  3 +++
 4 files changed, 30 insertions(+), 21 deletions(-)
 create mode 100644 config/suricata/suricata-default-rules.yaml

diff --git a/config/rootfiles/common/suricata b/config/rootfiles/common/suricata
index ff31ec7d2..41193f4ea 100644
--- a/config/rootfiles/common/suricata
+++ b/config/rootfiles/common/suricata
@@ -37,6 +37,7 @@ usr/share/suricata
 #usr/share/suricata/rules/smtp-events.rules
 #usr/share/suricata/rules/stream-events.rules
 #usr/share/suricata/rules/tls-events.rules
+var/ipfire/suricata/suricata-default-rules.yaml
 var/lib/suricata
 var/lib/suricata/classification.config
 var/lib/suricata/reference.config
diff --git a/config/suricata/suricata-default-rules.yaml b/config/suricata/suricata-default-rules.yaml
new file mode 100644
index 000000000..d13aa622a
--- /dev/null
+++ b/config/suricata/suricata-default-rules.yaml
@@ -0,0 +1,22 @@
+%YAML 1.1
+---
+
+# Default rules which helps
+ - /usr/share/suricata/rules/app-layer-events.rules
+ - /usr/share/suricata/rules/decoder-events.rules
+ - /usr/share/suricata/rules/dhcp-events.rules
+ - /usr/share/suricata/rules/dnp3-events.rules
+ - /usr/share/suricata/rules/dns-events.rules
+ - /usr/share/suricata/rules/files.rules
+ - /usr/share/suricata/rules/http2-events.rules
+ - /usr/share/suricata/rules/http-events.rules
+ - /usr/share/suricata/rules/ipsec-events.rules
+ - /usr/share/suricata/rules/kerberos-events.rules
+ - /usr/share/suricata/rules/modbus-events.rules
+ - /usr/share/suricata/rules/mqtt-events.rules
+ - /usr/share/suricata/rules/nfs-events.rules
+ - /usr/share/suricata/rules/ntp-events.rules
+ - /usr/share/suricata/rules/smb-events.rules
+ - /usr/share/suricata/rules/smtp-events.rules
+ - /usr/share/suricata/rules/stream-events.rules
+ - /usr/share/suricata/rules/tls-events.rules
diff --git a/config/suricata/suricata.yaml b/config/suricata/suricata.yaml
index 49921db86..7b2557fce 100644
--- a/config/suricata/suricata.yaml
+++ b/config/suricata/suricata.yaml
@@ -46,28 +46,11 @@ vars:
 ##
 default-rule-path: /var/lib/suricata
 rule-files:
-    # Default rules
-    - /usr/share/suricata/rules/app-layer-events.rules
-    - /usr/share/suricata/rules/decoder-events.rules
-    - /usr/share/suricata/rules/dhcp-events.rules
-    - /usr/share/suricata/rules/dnp3-events.rules
-    - /usr/share/suricata/rules/dns-events.rules
-    - /usr/share/suricata/rules/files.rules
-    - /usr/share/suricata/rules/http2-events.rules
-    - /usr/share/suricata/rules/http-events.rules
-    - /usr/share/suricata/rules/ipsec-events.rules
-    - /usr/share/suricata/rules/kerberos-events.rules
-    - /usr/share/suricata/rules/modbus-events.rules
-    - /usr/share/suricata/rules/mqtt-events.rules
-    - /usr/share/suricata/rules/nfs-events.rules
-    - /usr/share/suricata/rules/ntp-events.rules
-    - /usr/share/suricata/rules/smb-events.rules
-    - /usr/share/suricata/rules/smtp-events.rules
-    - /usr/share/suricata/rules/stream-events.rules
-    - /usr/share/suricata/rules/tls-events.rules
-
     # Include enabled ruleset files from external file
-    - !include: /var/ipfire/suricata/suricata-used-rulefiles.yaml
+    include: /var/ipfire/suricata/suricata-used-rulefiles.yaml
+
+    # Include default rules.
+    include: /var/ipfire/suricata/suricata-default-rules.yaml
 
 classification-file: /var/lib/suricata/classification.config
 reference-config-file: /var/lib/suricata/reference.config
diff --git a/lfs/suricata b/lfs/suricata
index f5b68da8f..96c2b33fe 100644
--- a/lfs/suricata
+++ b/lfs/suricata
@@ -96,6 +96,9 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
 	# Install IPFire related config file.
 	install -m 0644 $(DIR_SRC)/config/suricata/suricata.yaml /etc/suricata
 
+	# Install yaml file for loading default rules.
+	install -m 0664 $(DIR_SRC)/config/suricata/suricata-default-rules.yaml /var/ipfire/suricata
+
 	# Create emtpy rules directory.
 	-mkdir -p /var/lib/suricata
 
-- 
2.30.2

[PATCH 3/3] suricata: Cleanup default loaded rules file.

[Stefan Schantl]

[-- Attachment #1: Type: text/plain, Size: 1089 bytes --]

There are no such rules file available and therefore cannot be loaded.

Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
---
 config/suricata/suricata-default-rules.yaml | 2 --
 1 file changed, 2 deletions(-)

diff --git a/config/suricata/suricata-default-rules.yaml b/config/suricata/suricata-default-rules.yaml
index d13aa622a..64493e462 100644
--- a/config/suricata/suricata-default-rules.yaml
+++ b/config/suricata/suricata-default-rules.yaml
@@ -8,12 +8,10 @@
  - /usr/share/suricata/rules/dnp3-events.rules
  - /usr/share/suricata/rules/dns-events.rules
  - /usr/share/suricata/rules/files.rules
- - /usr/share/suricata/rules/http2-events.rules
  - /usr/share/suricata/rules/http-events.rules
  - /usr/share/suricata/rules/ipsec-events.rules
  - /usr/share/suricata/rules/kerberos-events.rules
  - /usr/share/suricata/rules/modbus-events.rules
- - /usr/share/suricata/rules/mqtt-events.rules
  - /usr/share/suricata/rules/nfs-events.rules
  - /usr/share/suricata/rules/ntp-events.rules
  - /usr/share/suricata/rules/smb-events.rules
-- 
2.30.2

Re: [PATCH 1/3] suricata: Update config file.

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 4934 bytes --]

Hello,

I would like to NACK this patch.

Do we need these parsers? I have no idea if we have any users for those. And if that is the case, I would prefer to keep them off to reduce the attack surface of the IPS.

Is there any strong reason that I have missed?

-Michael

> On 8 Dec 2021, at 17:10, Stefan Schantl <stefan.schantl(a)ipfire.org> wrote:
> 
> * This will enable swf decompression.
> * Enable modbus parser.
> * Enable dnp3 parser.
> * Enable enip parser.
> 
> Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
> ---
> config/suricata/suricata.yaml | 84 +++++++++++++++++++++++++++++++++++
> 1 file changed, 84 insertions(+)
> 
> diff --git a/config/suricata/suricata.yaml b/config/suricata/suricata.yaml
> index 0ad36e705..49921db86 100644
> --- a/config/suricata/suricata.yaml
> +++ b/config/suricata/suricata.yaml
> @@ -525,6 +525,20 @@ app-layer:
>            # auto will use http-body-inline mode in IPS mode, yes or no set it statically
>            http-body-inline: auto
> 
> +           # Decompress SWF files.
> +           # 2 types: 'deflate', 'lzma', 'both' will decompress deflate and lzma
> +           # compress-depth:
> +           # Specifies the maximum amount of data to decompress,
> +           # set 0 for unlimited.
> +           # decompress-depth:
> +           # Specifies the maximum amount of decompressed data to obtain,
> +           # set 0 for unlimited.
> +           swf-decompression:
> +             enabled: yes
> +             type: both
> +             compress-depth: 0
> +             decompress-depth: 0
> +
>            # Take a random value for inspection sizes around the specified value.
>            # This lower the risk of some evasion technics but could lead
>            # detection change between runs. It is set to 'yes' by default.
> @@ -539,6 +553,76 @@ app-layer:
>            double-decode-path: no
>            double-decode-query: no
> 
> +           # Can disable LZMA decompression
> +           #lzma-enabled: yes
> +           # Memory limit usage for LZMA decompression dictionary
> +           # Data is decompressed until dictionary reaches this size
> +           #lzma-memlimit: 1mb
> +           # Maximum decompressed size with a compression ratio
> +           # above 2048 (only LZMA can reach this ratio, deflate cannot)
> +           #compression-bomb-limit: 1mb
> +           # Maximum time spent decompressing a single transaction in usec
> +           #decompression-time-limit: 100000
> +
> +         server-config:
> +
> +           #- apache:
> +           #    address: [192.168.1.0/24, 127.0.0.0/8, "::1"]
> +           #    personality: Apache_2
> +           #    # Can be specified in kb, mb, gb.  Just a number indicates
> +           #    # it's in bytes.
> +           #    request-body-limit: 4096
> +           #    response-body-limit: 4096
> +           #    double-decode-path: no
> +           #    double-decode-query: no
> +
> +           #- iis7:
> +           #    address:
> +           #      - 192.168.0.0/24
> +           #      - 192.168.10.0/24
> +           #    personality: IIS_7_0
> +           #    # Can be specified in kb, mb, gb.  Just a number indicates
> +           #    # it's in bytes.
> +           #    request-body-limit: 4096
> +           #    response-body-limit: 4096
> +           #    double-decode-path: no
> +           #    double-decode-query: no
> +
> +    # Note: Modbus probe parser is minimalist due to the poor significant field
> +    # Only Modbus message length (greater than Modbus header length)
> +    # And Protocol ID (equal to 0) are checked in probing parser
> +    # It is important to enable detection port and define Modbus port
> +    # to avoid false positive
> +    modbus:
> +      # How many unreplied Modbus requests are considered a flood.
> +      # If the limit is reached, app-layer-event:modbus.flooded; will match.
> +      #request-flood: 500
> +
> +      enabled: yes
> +      detection-ports:
> +        dp: 502
> +      # According to MODBUS Messaging on TCP/IP Implementation Guide V1.0b, it
> +      # is recommended to keep the TCP connection opened with a remote device
> +      # and not to open and close it for each MODBUS/TCP transaction. In that
> +      # case, it is important to set the depth of the stream reassembling as
> +      # unlimited (stream.reassembly.depth: 0)
> +
> +      # Stream reassembly size for modbus. By default track it completely.
> +      stream-depth: 0
> +
> +    # DNP3
> +    dnp3:
> +      enabled: yes
> +      detection-ports:
> +        dp: 20000
> +
> +    # SCADA EtherNet/IP and CIP protocol support
> +    enip:
> +      enabled: yes
> +      detection-ports:
> +        dp: 44818
> +        sp: 44818
> +
>     ntp:
>       enabled: yes
>     dhcp:
> -- 
> 2.30.2
> 

Re: [PATCH 2/3] suricata: Move default loaded rulefiles to own included file.

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 4647 bytes --]

Acked-by: Michael Tremer <michael.tremer(a)ipfire.org>

> On 8 Dec 2021, at 17:10, Stefan Schantl <stefan.schantl(a)ipfire.org> wrote:
> 
> Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
> ---
> config/rootfiles/common/suricata            |  1 +
> config/suricata/suricata-default-rules.yaml | 22 ++++++++++++++++++
> config/suricata/suricata.yaml               | 25 ++++-----------------
> lfs/suricata                                |  3 +++
> 4 files changed, 30 insertions(+), 21 deletions(-)
> create mode 100644 config/suricata/suricata-default-rules.yaml
> 
> diff --git a/config/rootfiles/common/suricata b/config/rootfiles/common/suricata
> index ff31ec7d2..41193f4ea 100644
> --- a/config/rootfiles/common/suricata
> +++ b/config/rootfiles/common/suricata
> @@ -37,6 +37,7 @@ usr/share/suricata
> #usr/share/suricata/rules/smtp-events.rules
> #usr/share/suricata/rules/stream-events.rules
> #usr/share/suricata/rules/tls-events.rules
> +var/ipfire/suricata/suricata-default-rules.yaml
> var/lib/suricata
> var/lib/suricata/classification.config
> var/lib/suricata/reference.config
> diff --git a/config/suricata/suricata-default-rules.yaml b/config/suricata/suricata-default-rules.yaml
> new file mode 100644
> index 000000000..d13aa622a
> --- /dev/null
> +++ b/config/suricata/suricata-default-rules.yaml
> @@ -0,0 +1,22 @@
> +%YAML 1.1
> +---
> +
> +# Default rules which helps
> + - /usr/share/suricata/rules/app-layer-events.rules
> + - /usr/share/suricata/rules/decoder-events.rules
> + - /usr/share/suricata/rules/dhcp-events.rules
> + - /usr/share/suricata/rules/dnp3-events.rules
> + - /usr/share/suricata/rules/dns-events.rules
> + - /usr/share/suricata/rules/files.rules
> + - /usr/share/suricata/rules/http2-events.rules
> + - /usr/share/suricata/rules/http-events.rules
> + - /usr/share/suricata/rules/ipsec-events.rules
> + - /usr/share/suricata/rules/kerberos-events.rules
> + - /usr/share/suricata/rules/modbus-events.rules
> + - /usr/share/suricata/rules/mqtt-events.rules
> + - /usr/share/suricata/rules/nfs-events.rules
> + - /usr/share/suricata/rules/ntp-events.rules
> + - /usr/share/suricata/rules/smb-events.rules
> + - /usr/share/suricata/rules/smtp-events.rules
> + - /usr/share/suricata/rules/stream-events.rules
> + - /usr/share/suricata/rules/tls-events.rules
> diff --git a/config/suricata/suricata.yaml b/config/suricata/suricata.yaml
> index 49921db86..7b2557fce 100644
> --- a/config/suricata/suricata.yaml
> +++ b/config/suricata/suricata.yaml
> @@ -46,28 +46,11 @@ vars:
> ##
> default-rule-path: /var/lib/suricata
> rule-files:
> -    # Default rules
> -    - /usr/share/suricata/rules/app-layer-events.rules
> -    - /usr/share/suricata/rules/decoder-events.rules
> -    - /usr/share/suricata/rules/dhcp-events.rules
> -    - /usr/share/suricata/rules/dnp3-events.rules
> -    - /usr/share/suricata/rules/dns-events.rules
> -    - /usr/share/suricata/rules/files.rules
> -    - /usr/share/suricata/rules/http2-events.rules
> -    - /usr/share/suricata/rules/http-events.rules
> -    - /usr/share/suricata/rules/ipsec-events.rules
> -    - /usr/share/suricata/rules/kerberos-events.rules
> -    - /usr/share/suricata/rules/modbus-events.rules
> -    - /usr/share/suricata/rules/mqtt-events.rules
> -    - /usr/share/suricata/rules/nfs-events.rules
> -    - /usr/share/suricata/rules/ntp-events.rules
> -    - /usr/share/suricata/rules/smb-events.rules
> -    - /usr/share/suricata/rules/smtp-events.rules
> -    - /usr/share/suricata/rules/stream-events.rules
> -    - /usr/share/suricata/rules/tls-events.rules
> -
>     # Include enabled ruleset files from external file
> -    - !include: /var/ipfire/suricata/suricata-used-rulefiles.yaml
> +    include: /var/ipfire/suricata/suricata-used-rulefiles.yaml
> +
> +    # Include default rules.
> +    include: /var/ipfire/suricata/suricata-default-rules.yaml
> 
> classification-file: /var/lib/suricata/classification.config
> reference-config-file: /var/lib/suricata/reference.config
> diff --git a/lfs/suricata b/lfs/suricata
> index f5b68da8f..96c2b33fe 100644
> --- a/lfs/suricata
> +++ b/lfs/suricata
> @@ -96,6 +96,9 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
> 	# Install IPFire related config file.
> 	install -m 0644 $(DIR_SRC)/config/suricata/suricata.yaml /etc/suricata
> 
> +	# Install yaml file for loading default rules.
> +	install -m 0664 $(DIR_SRC)/config/suricata/suricata-default-rules.yaml /var/ipfire/suricata
> +
> 	# Create emtpy rules directory.
> 	-mkdir -p /var/lib/suricata
> 
> -- 
> 2.30.2
> 

Re: [PATCH 3/3] suricata: Cleanup default loaded rules file.

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 1278 bytes --]

Reviewed-by: Michael Tremer <michael.tremer(a)ipfire.org>

> On 8 Dec 2021, at 17:10, Stefan Schantl <stefan.schantl(a)ipfire.org> wrote:
> 
> There are no such rules file available and therefore cannot be loaded.
> 
> Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
> ---
> config/suricata/suricata-default-rules.yaml | 2 --
> 1 file changed, 2 deletions(-)
> 
> diff --git a/config/suricata/suricata-default-rules.yaml b/config/suricata/suricata-default-rules.yaml
> index d13aa622a..64493e462 100644
> --- a/config/suricata/suricata-default-rules.yaml
> +++ b/config/suricata/suricata-default-rules.yaml
> @@ -8,12 +8,10 @@
>  - /usr/share/suricata/rules/dnp3-events.rules
>  - /usr/share/suricata/rules/dns-events.rules
>  - /usr/share/suricata/rules/files.rules
> - - /usr/share/suricata/rules/http2-events.rules
>  - /usr/share/suricata/rules/http-events.rules
>  - /usr/share/suricata/rules/ipsec-events.rules
>  - /usr/share/suricata/rules/kerberos-events.rules
>  - /usr/share/suricata/rules/modbus-events.rules
> - - /usr/share/suricata/rules/mqtt-events.rules
>  - /usr/share/suricata/rules/nfs-events.rules
>  - /usr/share/suricata/rules/ntp-events.rules
>  - /usr/share/suricata/rules/smb-events.rules
> -- 
> 2.30.2
> 

Re: [PATCH 3/3] suricata: Cleanup default loaded rules file.

[Peter Müller]

[-- Attachment #1: Type: text/plain, Size: 1185 bytes --]

Reviewed-by: Peter Müller <peter.mueller(a)ipfire.org>

> There are no such rules file available and therefore cannot be loaded.
> 
> Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
> ---
>  config/suricata/suricata-default-rules.yaml | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/config/suricata/suricata-default-rules.yaml b/config/suricata/suricata-default-rules.yaml
> index d13aa622a..64493e462 100644
> --- a/config/suricata/suricata-default-rules.yaml
> +++ b/config/suricata/suricata-default-rules.yaml
> @@ -8,12 +8,10 @@
>   - /usr/share/suricata/rules/dnp3-events.rules
>   - /usr/share/suricata/rules/dns-events.rules
>   - /usr/share/suricata/rules/files.rules
> - - /usr/share/suricata/rules/http2-events.rules
>   - /usr/share/suricata/rules/http-events.rules
>   - /usr/share/suricata/rules/ipsec-events.rules
>   - /usr/share/suricata/rules/kerberos-events.rules
>   - /usr/share/suricata/rules/modbus-events.rules
> - - /usr/share/suricata/rules/mqtt-events.rules
>   - /usr/share/suricata/rules/nfs-events.rules
>   - /usr/share/suricata/rules/ntp-events.rules
>   - /usr/share/suricata/rules/smb-events.rules
> 

Re: [PATCH 2/3] suricata: Move default loaded rulefiles to own included file.

[Peter Müller]

[-- Attachment #1: Type: text/plain, Size: 4862 bytes --]

Hello Stefan,

thank you for submitting this.

There is one very minor comment, please see below. Apart from it, this patch looks good to me.

Reviewed-by: Peter Müller <peter.mueller(a)ipfire.org>

Thanks, and best regards,
Peter Müller


> Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
> ---
>  config/rootfiles/common/suricata            |  1 +
>  config/suricata/suricata-default-rules.yaml | 22 ++++++++++++++++++
>  config/suricata/suricata.yaml               | 25 ++++-----------------
>  lfs/suricata                                |  3 +++
>  4 files changed, 30 insertions(+), 21 deletions(-)
>  create mode 100644 config/suricata/suricata-default-rules.yaml
> 
> diff --git a/config/rootfiles/common/suricata b/config/rootfiles/common/suricata
> index ff31ec7d2..41193f4ea 100644
> --- a/config/rootfiles/common/suricata
> +++ b/config/rootfiles/common/suricata
> @@ -37,6 +37,7 @@ usr/share/suricata
>  #usr/share/suricata/rules/smtp-events.rules
>  #usr/share/suricata/rules/stream-events.rules
>  #usr/share/suricata/rules/tls-events.rules
> +var/ipfire/suricata/suricata-default-rules.yaml
>  var/lib/suricata
>  var/lib/suricata/classification.config
>  var/lib/suricata/reference.config
> diff --git a/config/suricata/suricata-default-rules.yaml b/config/suricata/suricata-default-rules.yaml
> new file mode 100644
> index 000000000..d13aa622a
> --- /dev/null
> +++ b/config/suricata/suricata-default-rules.yaml
> @@ -0,0 +1,22 @@
> +%YAML 1.1
> +---
> +
> +# Default rules which helps

... to keep things nice and tidy? Looks like the second half of the comment is missing. :-)

> + - /usr/share/suricata/rules/app-layer-events.rules
> + - /usr/share/suricata/rules/decoder-events.rules
> + - /usr/share/suricata/rules/dhcp-events.rules
> + - /usr/share/suricata/rules/dnp3-events.rules
> + - /usr/share/suricata/rules/dns-events.rules
> + - /usr/share/suricata/rules/files.rules
> + - /usr/share/suricata/rules/http2-events.rules
> + - /usr/share/suricata/rules/http-events.rules
> + - /usr/share/suricata/rules/ipsec-events.rules
> + - /usr/share/suricata/rules/kerberos-events.rules
> + - /usr/share/suricata/rules/modbus-events.rules
> + - /usr/share/suricata/rules/mqtt-events.rules
> + - /usr/share/suricata/rules/nfs-events.rules
> + - /usr/share/suricata/rules/ntp-events.rules
> + - /usr/share/suricata/rules/smb-events.rules
> + - /usr/share/suricata/rules/smtp-events.rules
> + - /usr/share/suricata/rules/stream-events.rules
> + - /usr/share/suricata/rules/tls-events.rules
> diff --git a/config/suricata/suricata.yaml b/config/suricata/suricata.yaml
> index 49921db86..7b2557fce 100644
> --- a/config/suricata/suricata.yaml
> +++ b/config/suricata/suricata.yaml
> @@ -46,28 +46,11 @@ vars:
>  ##
>  default-rule-path: /var/lib/suricata
>  rule-files:
> -    # Default rules
> -    - /usr/share/suricata/rules/app-layer-events.rules
> -    - /usr/share/suricata/rules/decoder-events.rules
> -    - /usr/share/suricata/rules/dhcp-events.rules
> -    - /usr/share/suricata/rules/dnp3-events.rules
> -    - /usr/share/suricata/rules/dns-events.rules
> -    - /usr/share/suricata/rules/files.rules
> -    - /usr/share/suricata/rules/http2-events.rules
> -    - /usr/share/suricata/rules/http-events.rules
> -    - /usr/share/suricata/rules/ipsec-events.rules
> -    - /usr/share/suricata/rules/kerberos-events.rules
> -    - /usr/share/suricata/rules/modbus-events.rules
> -    - /usr/share/suricata/rules/mqtt-events.rules
> -    - /usr/share/suricata/rules/nfs-events.rules
> -    - /usr/share/suricata/rules/ntp-events.rules
> -    - /usr/share/suricata/rules/smb-events.rules
> -    - /usr/share/suricata/rules/smtp-events.rules
> -    - /usr/share/suricata/rules/stream-events.rules
> -    - /usr/share/suricata/rules/tls-events.rules
> -
>      # Include enabled ruleset files from external file
> -    - !include: /var/ipfire/suricata/suricata-used-rulefiles.yaml
> +    include: /var/ipfire/suricata/suricata-used-rulefiles.yaml
> +
> +    # Include default rules.
> +    include: /var/ipfire/suricata/suricata-default-rules.yaml
>  
>  classification-file: /var/lib/suricata/classification.config
>  reference-config-file: /var/lib/suricata/reference.config
> diff --git a/lfs/suricata b/lfs/suricata
> index f5b68da8f..96c2b33fe 100644
> --- a/lfs/suricata
> +++ b/lfs/suricata
> @@ -96,6 +96,9 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
>  	# Install IPFire related config file.
>  	install -m 0644 $(DIR_SRC)/config/suricata/suricata.yaml /etc/suricata
>  
> +	# Install yaml file for loading default rules.
> +	install -m 0664 $(DIR_SRC)/config/suricata/suricata-default-rules.yaml /var/ipfire/suricata
> +
>  	# Create emtpy rules directory.
>  	-mkdir -p /var/lib/suricata
>  
> 

Re: [PATCH 2/3] suricata: Move default loaded rulefiles to own included file.

[Peter Müller]

[-- Attachment #1: Type: text/plain, Size: 5097 bytes --]

... and, for the records, it fixes bug #12739, which is currently show-stopping Core Update 162. :-)


> Hello Stefan,
> 
> thank you for submitting this.
> 
> There is one very minor comment, please see below. Apart from it, this patch looks good to me.
> 
> Reviewed-by: Peter Müller <peter.mueller(a)ipfire.org>
> 
> Thanks, and best regards,
> Peter Müller
> 
> 
>> Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
>> ---
>>  config/rootfiles/common/suricata            |  1 +
>>  config/suricata/suricata-default-rules.yaml | 22 ++++++++++++++++++
>>  config/suricata/suricata.yaml               | 25 ++++-----------------
>>  lfs/suricata                                |  3 +++
>>  4 files changed, 30 insertions(+), 21 deletions(-)
>>  create mode 100644 config/suricata/suricata-default-rules.yaml
>>
>> diff --git a/config/rootfiles/common/suricata b/config/rootfiles/common/suricata
>> index ff31ec7d2..41193f4ea 100644
>> --- a/config/rootfiles/common/suricata
>> +++ b/config/rootfiles/common/suricata
>> @@ -37,6 +37,7 @@ usr/share/suricata
>>  #usr/share/suricata/rules/smtp-events.rules
>>  #usr/share/suricata/rules/stream-events.rules
>>  #usr/share/suricata/rules/tls-events.rules
>> +var/ipfire/suricata/suricata-default-rules.yaml
>>  var/lib/suricata
>>  var/lib/suricata/classification.config
>>  var/lib/suricata/reference.config
>> diff --git a/config/suricata/suricata-default-rules.yaml b/config/suricata/suricata-default-rules.yaml
>> new file mode 100644
>> index 000000000..d13aa622a
>> --- /dev/null
>> +++ b/config/suricata/suricata-default-rules.yaml
>> @@ -0,0 +1,22 @@
>> +%YAML 1.1
>> +---
>> +
>> +# Default rules which helps
> 
> ... to keep things nice and tidy? Looks like the second half of the comment is missing. :-)
> 
>> + - /usr/share/suricata/rules/app-layer-events.rules
>> + - /usr/share/suricata/rules/decoder-events.rules
>> + - /usr/share/suricata/rules/dhcp-events.rules
>> + - /usr/share/suricata/rules/dnp3-events.rules
>> + - /usr/share/suricata/rules/dns-events.rules
>> + - /usr/share/suricata/rules/files.rules
>> + - /usr/share/suricata/rules/http2-events.rules
>> + - /usr/share/suricata/rules/http-events.rules
>> + - /usr/share/suricata/rules/ipsec-events.rules
>> + - /usr/share/suricata/rules/kerberos-events.rules
>> + - /usr/share/suricata/rules/modbus-events.rules
>> + - /usr/share/suricata/rules/mqtt-events.rules
>> + - /usr/share/suricata/rules/nfs-events.rules
>> + - /usr/share/suricata/rules/ntp-events.rules
>> + - /usr/share/suricata/rules/smb-events.rules
>> + - /usr/share/suricata/rules/smtp-events.rules
>> + - /usr/share/suricata/rules/stream-events.rules
>> + - /usr/share/suricata/rules/tls-events.rules
>> diff --git a/config/suricata/suricata.yaml b/config/suricata/suricata.yaml
>> index 49921db86..7b2557fce 100644
>> --- a/config/suricata/suricata.yaml
>> +++ b/config/suricata/suricata.yaml
>> @@ -46,28 +46,11 @@ vars:
>>  ##
>>  default-rule-path: /var/lib/suricata
>>  rule-files:
>> -    # Default rules
>> -    - /usr/share/suricata/rules/app-layer-events.rules
>> -    - /usr/share/suricata/rules/decoder-events.rules
>> -    - /usr/share/suricata/rules/dhcp-events.rules
>> -    - /usr/share/suricata/rules/dnp3-events.rules
>> -    - /usr/share/suricata/rules/dns-events.rules
>> -    - /usr/share/suricata/rules/files.rules
>> -    - /usr/share/suricata/rules/http2-events.rules
>> -    - /usr/share/suricata/rules/http-events.rules
>> -    - /usr/share/suricata/rules/ipsec-events.rules
>> -    - /usr/share/suricata/rules/kerberos-events.rules
>> -    - /usr/share/suricata/rules/modbus-events.rules
>> -    - /usr/share/suricata/rules/mqtt-events.rules
>> -    - /usr/share/suricata/rules/nfs-events.rules
>> -    - /usr/share/suricata/rules/ntp-events.rules
>> -    - /usr/share/suricata/rules/smb-events.rules
>> -    - /usr/share/suricata/rules/smtp-events.rules
>> -    - /usr/share/suricata/rules/stream-events.rules
>> -    - /usr/share/suricata/rules/tls-events.rules
>> -
>>      # Include enabled ruleset files from external file
>> -    - !include: /var/ipfire/suricata/suricata-used-rulefiles.yaml
>> +    include: /var/ipfire/suricata/suricata-used-rulefiles.yaml
>> +
>> +    # Include default rules.
>> +    include: /var/ipfire/suricata/suricata-default-rules.yaml
>>  
>>  classification-file: /var/lib/suricata/classification.config
>>  reference-config-file: /var/lib/suricata/reference.config
>> diff --git a/lfs/suricata b/lfs/suricata
>> index f5b68da8f..96c2b33fe 100644
>> --- a/lfs/suricata
>> +++ b/lfs/suricata
>> @@ -96,6 +96,9 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
>>  	# Install IPFire related config file.
>>  	install -m 0644 $(DIR_SRC)/config/suricata/suricata.yaml /etc/suricata
>>  
>> +	# Install yaml file for loading default rules.
>> +	install -m 0664 $(DIR_SRC)/config/suricata/suricata-default-rules.yaml /var/ipfire/suricata
>> +
>>  	# Create emtpy rules directory.
>>  	-mkdir -p /var/lib/suricata
>>  
>>

Re: [PATCH 1/3] suricata: Update config file.

[Peter Müller]

[-- Attachment #1: Type: text/plain, Size: 5830 bytes --]

Hello Michael, hello Stefan,

first, thanks for working on this.

While I have no strong opinion on SWF and DNP3 - I have not seen both in production for
a long time, but there might be legacy/special setups out there which needs them -, SCADA-
related protocol parsers won't probably help the majority of our users, but are very helpful
in networks where SCADA is used.

To me, coming to a decision is tricky: I would oppose against making this configurable,
since most users won't understand what they are configuring. Truth to be told, we have very
little insights into use-cases for IPFire apart from common network setups, so at least I
am a bit lost when it comes to set a default for our users.

Thanks, and best regards,
Peter Müller


> Hello,
> 
> I would like to NACK this patch.
> 
> Do we need these parsers? I have no idea if we have any users for those. And if that is the case, I would prefer to keep them off to reduce the attack surface of the IPS.
> 
> Is there any strong reason that I have missed?
> 
> -Michael
> 
>> On 8 Dec 2021, at 17:10, Stefan Schantl <stefan.schantl(a)ipfire.org> wrote:
>>
>> * This will enable swf decompression.
>> * Enable modbus parser.
>> * Enable dnp3 parser.
>> * Enable enip parser.
>>
>> Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
>> ---
>> config/suricata/suricata.yaml | 84 +++++++++++++++++++++++++++++++++++
>> 1 file changed, 84 insertions(+)
>>
>> diff --git a/config/suricata/suricata.yaml b/config/suricata/suricata.yaml
>> index 0ad36e705..49921db86 100644
>> --- a/config/suricata/suricata.yaml
>> +++ b/config/suricata/suricata.yaml
>> @@ -525,6 +525,20 @@ app-layer:
>>            # auto will use http-body-inline mode in IPS mode, yes or no set it statically
>>            http-body-inline: auto
>>
>> +           # Decompress SWF files.
>> +           # 2 types: 'deflate', 'lzma', 'both' will decompress deflate and lzma
>> +           # compress-depth:
>> +           # Specifies the maximum amount of data to decompress,
>> +           # set 0 for unlimited.
>> +           # decompress-depth:
>> +           # Specifies the maximum amount of decompressed data to obtain,
>> +           # set 0 for unlimited.
>> +           swf-decompression:
>> +             enabled: yes
>> +             type: both
>> +             compress-depth: 0
>> +             decompress-depth: 0
>> +
>>            # Take a random value for inspection sizes around the specified value.
>>            # This lower the risk of some evasion technics but could lead
>>            # detection change between runs. It is set to 'yes' by default.
>> @@ -539,6 +553,76 @@ app-layer:
>>            double-decode-path: no
>>            double-decode-query: no
>>
>> +           # Can disable LZMA decompression
>> +           #lzma-enabled: yes
>> +           # Memory limit usage for LZMA decompression dictionary
>> +           # Data is decompressed until dictionary reaches this size
>> +           #lzma-memlimit: 1mb
>> +           # Maximum decompressed size with a compression ratio
>> +           # above 2048 (only LZMA can reach this ratio, deflate cannot)
>> +           #compression-bomb-limit: 1mb
>> +           # Maximum time spent decompressing a single transaction in usec
>> +           #decompression-time-limit: 100000
>> +
>> +         server-config:
>> +
>> +           #- apache:
>> +           #    address: [192.168.1.0/24, 127.0.0.0/8, "::1"]
>> +           #    personality: Apache_2
>> +           #    # Can be specified in kb, mb, gb.  Just a number indicates
>> +           #    # it's in bytes.
>> +           #    request-body-limit: 4096
>> +           #    response-body-limit: 4096
>> +           #    double-decode-path: no
>> +           #    double-decode-query: no
>> +
>> +           #- iis7:
>> +           #    address:
>> +           #      - 192.168.0.0/24
>> +           #      - 192.168.10.0/24
>> +           #    personality: IIS_7_0
>> +           #    # Can be specified in kb, mb, gb.  Just a number indicates
>> +           #    # it's in bytes.
>> +           #    request-body-limit: 4096
>> +           #    response-body-limit: 4096
>> +           #    double-decode-path: no
>> +           #    double-decode-query: no
>> +
>> +    # Note: Modbus probe parser is minimalist due to the poor significant field
>> +    # Only Modbus message length (greater than Modbus header length)
>> +    # And Protocol ID (equal to 0) are checked in probing parser
>> +    # It is important to enable detection port and define Modbus port
>> +    # to avoid false positive
>> +    modbus:
>> +      # How many unreplied Modbus requests are considered a flood.
>> +      # If the limit is reached, app-layer-event:modbus.flooded; will match.
>> +      #request-flood: 500
>> +
>> +      enabled: yes
>> +      detection-ports:
>> +        dp: 502
>> +      # According to MODBUS Messaging on TCP/IP Implementation Guide V1.0b, it
>> +      # is recommended to keep the TCP connection opened with a remote device
>> +      # and not to open and close it for each MODBUS/TCP transaction. In that
>> +      # case, it is important to set the depth of the stream reassembling as
>> +      # unlimited (stream.reassembly.depth: 0)
>> +
>> +      # Stream reassembly size for modbus. By default track it completely.
>> +      stream-depth: 0
>> +
>> +    # DNP3
>> +    dnp3:
>> +      enabled: yes
>> +      detection-ports:
>> +        dp: 20000
>> +
>> +    # SCADA EtherNet/IP and CIP protocol support
>> +    enip:
>> +      enabled: yes
>> +      detection-ports:
>> +        dp: 44818
>> +        sp: 44818
>> +
>>     ntp:
>>       enabled: yes
>>     dhcp:
>> -- 
>> 2.30.2
>>
>