suricata: T751: Initial support for suricata #3399
Conversation
sever-sever
requested review from
a team,
dmbaturin,
sarthurdev,
zdc,
jestabro,
sever-sever,
c-po and
nicolas-fort
and removed request for
a team
|
I haven't created the documentation but plan on doing so once the PR is reviewed as approved. |
|
Maybe I don't understand the function of this component. Why not use snort to do IDS/IPS? |
I'm more familiar with Suricata and it offers the functionalities I need (a.k.a. had on OPNsense) such as protocol detection and file extraction; I don't know if Snort has that capability. Snort and Suricata support similarly-ish rules and are hence both commonly supported by providers (e.g., ET PRO). As an example, one can log all HTTP(S) traffic by using: Running another cURL command such as {"timestamp":"2024-05-02T12:46:23.433575+0000","flow_id":1239289377543499,"in_iface":"eth1","event_type":"http","src_ip":"192.168.2.101","src_port":57872,"dest_ip":"104.18.2.133","dest_port":80,"proto":"TCP","tx_id":0,"http":{"hostname":"vyos.dev","url":"/","http_user_agent":"curl/8.5.0","http_content_type":"text/html","http_method":"GET","protocol":"HTTP/1.1","status":308,"redirect":"https://vyos.dev","length":173}}
{"timestamp":"2024-05-02T12:46:23.433594+0000","flow_id":986788250211682,"in_iface":"eth0","event_type":"http","src_ip":"192.168.1.141","src_port":57872,"dest_ip":"104.18.2.133","dest_port":80,"proto":"TCP","tx_id":0,"http":{"hostname":"vyos.dev","url":"/","http_user_agent":"curl/8.5.0","http_content_type":"text/html","http_method":"GET","protocol":"HTTP/1.1","status":308,"redirect":"https://vyos.dev","length":173}}
{"timestamp":"2024-05-02T12:46:23.553402+0000","flow_id":567290204495683,"in_iface":"eth1","event_type":"tls","src_ip":"192.168.2.101","src_port":38794,"dest_ip":"104.18.3.133","dest_port":443,"proto":"TCP","tls":{"sni":"vyos.dev","version":"TLS 1.3","ja3":{"hash":"0149f47eabf9a20d0893e2a44e5a6323","string":"771,4866-4867-4865-49196-49200-159-52393-52392-52394-49195-49199-158-49188-49192-107-49187-49191-103-49162-49172-57-49161-49171-51-157-156-61-60-53-47-255,0-11-10-16-22-23-49-13-43-45-51-21,29-23-30-25-24-256-257-258-259-260,0-1-2"},"ja3s":{"hash":"907bf3ecef1c987c889946b737b43de8","string":"771,4866,51-43"}}}
{"timestamp":"2024-05-02T12:46:23.553410+0000","flow_id":1159626324156255,"in_iface":"eth0","event_type":"tls","src_ip":"192.168.1.141","src_port":38794,"dest_ip":"104.18.3.133","dest_port":443,"proto":"TCP","tls":{"sni":"vyos.dev","version":"TLS 1.3","ja3":{"hash":"0149f47eabf9a20d0893e2a44e5a6323","string":"771,4866-4867-4865-49196-49200-159-52393-52392-52394-49195-49199-158-49188-49192-107-49187-49191-103-49162-49172-57-49161-49171-51-157-156-61-60-53-47-255,0-11-10-16-22-23-49-13-43-45-51-21,29-23-30-25-24-256-257-258-259-260,0-1-2"},"ja3s":{"hash":"907bf3ecef1c987c889946b737b43de8","string":"771,4866,51-43"}}}PS: Just pushed a quick fix for the configuration generation; Realized I hadn't completed support for other protocols. |
There was a problem hiding this comment.
It looks good to me, especially as the first implementation/integration.
Some nuances could be fixed later.
@0xThiebaut thanks!
| <list>home-net external-net http-servers smtp-servers sql-servers dns-servers telnet-servers aim-servers dc-servers dnp3-server dnp3-client modbus-client modbus-server enip-client enip-server</list> | ||
| </completionHelp> | ||
| <constraint> | ||
| <regex>[a-z0-9-]+</regex> |
There was a problem hiding this comment.
The address-group node lists available completion helpers, but the regex allows much more.
Is it really the case that it's not limited to the ones listed under completionHelp?
Please also add a valueHelp XML definition for the ones listed by the completion helper.
There was a problem hiding this comment.
Is it really the case that it's not limited to the ones listed under completionHelp?
Yes, the "Rule-vars" allow for additional variables to be leveraged in rules; Those for which defaults are defined in this PR are those provided by default for Suricata but users can define additional variables to both customize existing groups (e.g., HOME_NET: "[$LAN_NET,$DMZ_NET]") or to support additional custom rules.
The reason I don't want to restrict the address-group and port-group nodes to those defined by default is that it opens the avenue to support additional rules through Suricata-Update. While the current PR only leverages Suricata-Update to fetch the default ET Open rules, future improvements can introduce commands to CRUD other repositories (including private ones) which could require additional variables.
There was a problem hiding this comment.
Okay, I understand what you're trying to accomplish here, having predefined "well known" and custom groups. Why not reflect this in the CLI as is? Something like:
<node name="address-group">
<children>
<leafNode name="predefined">
<properties>
<completionHelp>
<list>home-net external-net http-servers smtp-servers sql-servers dns-servers telnet-servers aim-servers dc-servers dnp3-server dnp3-client modbus-client modbus-server enip-client enip-server</list>
</completionHelp>
<constraint>
<regex>(home-net|external-net|http-servers|smtp-servers|sql-servers|dns-servers|telnet-servers|aim-servers|dc-servers|dnp3-server|dnp3-client|modbus-client|modbus-server|enip-client|enip-server</list>
</constraint>
<multi/>
</properties>
</leafNode>
<tagNode name="custom">
<properties>
<help>Address group name</help>
<valueHelp>
<format>txt</format>
<description>Wild Foo Bar one Can do</description>
<valueHelp>
<constraint>
#include <include/constraint/alpha-numeric-hyphen-underscore.xml.i>
</constraint>
</properties>
</tagNode>
</children>
</node>My main concern here is that the predefined groupd e.g. HOME_NET: "[$LAN_NET,$DMZ_NET]"
are hard to understand from a completion helper standpoint, where is LAN_NET and DMZ_NET defined?
Who defines the addresses for listed in external-net or telnet-servers ?
There was a problem hiding this comment.
@c-po predefined is not leafNode
Something like this:
set service ids suricata address-group <predefined> home-net address 100.64.0.0/24
set service ids suricata address-group <predefined> home-net address 100.64.1.0/28
...
set service ids suricata address-group <custom> mygroup address 192.0.2.0/24
There was a problem hiding this comment.
First of all, as maintainers, I'm more than happy to accommodate whichever approach you prefer. I don't however believe it would be beneficial to split the values over custom/predefined nodes.
My main concern here is that the predefined groupd e.g. HOME_NET: "[$LAN_NET,$DMZ_NET]"
are hard to understand from a completion helper standpoint, where is LAN_NET and DMZ_NET defined?
The nodes LAN_NET and DMZ_NET are just examples which the user can choose to define as follow:
set service ids suricata address-group lan-net address 192.168.1.0/24
set service ids suricata address-group dmz-net address 192.168.2.0/24
set service ids suricata address-group whatever-the-user-wants address 192.168.3.0/24
When a user defines a group (address or port), the group can be used both in other groups (e.g., HOME_NET: "[$LAN_NET,$DMZ_NET,$WHATEVER_THE_USER_WANTS]") as well as, most importantly, in Suricata rules (e.g., alert http $DMZ_NET any -> $LAN_NET any (msg:"HTTP request from DMZ to internal LAN" ....).
While the later will be for future improvements (additional conf commands for Suricata update), the former allows advanced users to ease their group management. The above groups can for example generate HOME_NET: "[$LAN_NET,$DMZ_NET,$WHATEVER_THE_USER_WANTS]" as follow:
set service ids suricata address-group home-net group lan-net
set service ids suricata address-group home-net group dmz-net
set service ids suricata address-group home-net group whatever-the-user-wants
It however doesn't mean the user has to define groups, the following example is equivalent for home-net definitions.
set service ids suricata address-group home-net address 192.168.1.0/24
set service ids suricata address-group home-net address 192.168.2.0/24
set service ids suricata address-group home-net address 192.168.3.0/24
Who defines the addresses for listed in external-net or telnet-servers ?
Suricata provides sane defaults (vars and values) to ensure rules work out of the box. These are the defaults I provided in the definitions (names) and configuration script (values). While users are encouraged to override these with their network's values, the defaults provides already a solid functional base. For example, telnet-servers defaults to $HOME_NET but using any of the below commands would override the default.
set service ids suricata address-group telnet-servers address 192.168.4.0/24
set service ids suricata address-group telnet-servers group some-other-defined-group
I'm not really a fan of splitting a similar functionality over predefined/custom nodes while, in essence, these are similar. For example, the following two entries would either collide or we would need to add additional restrictions without these having an actual practical need.
set service ids suricata address-group predefined home-net address 192.168.1.0/24
set service ids suricata address-group custom home-net address 192.168.2.0/24
| <help>Address group</help> | ||
| <completionHelp> | ||
| <path>service ids suricata address-group</path> | ||
| <list>home-net external-net http-servers smtp-servers sql-servers dns-servers telnet-servers aim-servers dc-servers dnp3-server dnp3-client modbus-client modbus-server enip-client enip-server</list> |
There was a problem hiding this comment.
See comment above for address-group
| <list>http-ports shellcode-ports oracle-ports ssh-ports dnp3-ports modbus-ports file-data-ports ftp-ports geneve-ports vxlan-ports teredo-ports</list> | ||
| </completionHelp> | ||
| <constraint> | ||
| <regex>[a-z0-9-]+</regex> |
There was a problem hiding this comment.
missing valueHelp and a too relaxed regex. Should be:
<regex>(http-ports|shellcode-ports|oracle-ports|ssh-ports|dnp3-ports|modbus-ports|file-data-ports|ftp-ports|geneve-ports|vxlan-ports|teredo-ports)</regex>
There was a problem hiding this comment.
The relaxed port-group values is due to the same reasoning as the above address-group values.
| <multi/> | ||
| </properties> | ||
| </leafNode> | ||
| <leafNode name="group"> |
There was a problem hiding this comment.
Same XML node "group" as above, please use a .xml.i representation so you can include the same building block for group multiple times to avoid code-duplication.
| <properties> | ||
| <help>Log file name in default Suricata log directory</help> | ||
| </properties> | ||
| <defaultValue>eve.json</defaultValue> |
There was a problem hiding this comment.
We probably should add <valueHelp> and <constraint> nodes here.
|
In general I like the idea and support this PR. Let's weed out some more minor things. Do you plan an op-mode command to make the log file "human" readable? |
Currently not no. The many different types of events (alerts, anomalies, http, dns, ...) would require significant effort to render in a more-than-json "human" format while, for regular operations, such logs are typically forwarded to a SIEM/collector which provides the "human" format and subsequent actions (e.g., notifications) upon alert generation. While I am sure many would enjoy the availability of Suricata in VyOS (continuous identification of network incidents), I don't believe these users do plan on, nor expect, reading "human" logs from op-mode (occasional manual action). |
|
Next steps for this PR will be discussed during next maintainer meeting this week. Please be patient. |
|
@0xThiebaut thanks for being patient. In todays maintainer meeting we agreed on two more changes that we would like to get before we can finally merge this in.
We try to have no "default" values inside Python helper scripts to make automatic use of the defaults during CLI and Help generation. This approach has a nice idea in the background but it possibly makes false assumptions or assumptions that will be outdated in the future. If a user want's to define an address-group named set service ids suricata address-group home-net address 10.0.0.0/8
set service ids suricata address-group home-net address 172.16.0.0/12
set service ids suricata address-group home-net address 192.168.0.0/16Thanks for the final changes before the merge! |
Change Summary
Added initial support for Suricata (IDS):
Types of changes
Related Task(s)
Related PR(s)
Component(s) name
Proposed changes
Add initial support for Suricata (IDS):
The commands have been added under
service ids suricata.How to test
On VyOS, configure the Suricata IDS:
Suricata should be running, as can be verified with
systemctl status suricata.On a host whose default gateway is VyOS, test the IDS using the following cURL command:
On VyOS, the alert should have been logged by Suricata as seen through
cat /var/log/suricata/eve.json:{"timestamp":"2024-05-02T15:15:37.859804+0000","flow_id":1042851045125391,"in_iface":"eth0","event_type":"alert","src_ip":"18.239.208.47","src_port":80,"dest_ip":"192.168.1.141","dest_port":53920,"proto":"TCP","alert":{"action":"allowed","gid":1,"signature_id":2100498,"rev":7,"signature":"GPL ATTACK_RESPONSE id check returned root","category":"Potentially Bad Traffic","severity":2,"metadata":{"created_at":["2010_09_23"],"updated_at":["2019_07_26"]}},"http":{"hostname":"testmynids.org","url":"/uid/index.html","http_user_agent":"curl/8.5.0","http_content_type":"text/html","http_method":"GET","protocol":"HTTP/1.1","status":200,"length":39},"files":[{"filename":"/uid/index.html","sid":[],"gaps":false,"state":"CLOSED","stored":false,"size":39,"tx_id":0}],"app_proto":"http","flow":{"pkts_toserver":5,"pkts_toclient":4,"bytes_toserver":429,"bytes_toclient":810,"start":"2024-05-02T15:15:37.797967+0000"}}Smoketest result
N/A, testing the proper functioning of the IDS requires generating alert-able traffic.
Checklist: