Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing protocols and transformations #156

Open
duanehoward opened this issue Jun 6, 2021 · 10 comments
Open

Missing protocols and transformations #156

duanehoward opened this issue Jun 6, 2021 · 10 comments
Assignees

Comments

@duanehoward
Copy link
Collaborator

From issue #154 @satta notes:

There are other issues with parsing ETPRO at the moment, such as missing support for noalert without values in some places, transformations (such as dotprefix) and missing protocols (icmpv6, ftp-data, etc.) and more. These seem to be smaller in nature and probably easy to fix.

@duanehoward
Copy link
Collaborator Author

@satta
I'm failing to find good references in the current Suricata documentation around support for keywords icmpv6, ftp-data etc for protocols.

Can you provide a quick pointer in the source to where these other protocols are defined (probably an issue should be opened with OISF to update the docs to make them complete as well).

Additionally can you give me an example of where noalert isn't working as expected? Should be simple enough to add a test and iterate on that.

@satta
Copy link

satta commented Jun 7, 2021

I don't think they are listed in the documentation itself. The last time I needed this information, I just looked them up in the source code.

App-layer protocols:

$ grep 'proto_name = ' src/app-layer-protos.c | cut -f2 -d= | egrep -v 'NULL|template|failed' 
 "http";
 "ftp";
 "smtp";
 "tls";
 "ssh";
 "imap";
 "jabber";
 "smb";
 "dcerpc";
 "irc";
 "dns";
 "modbus";
 "enip";
 "dnp3";
 "nfs";
 "ntp";
 "ftp-data";
 "tftp";
 "ikev2";
 "krb5";
 "dhcp";
 "snmp";
 "sip";
 "rfb";
 "mqtt";
 "rdp";

Lower layer protocols are mentioned in src/detect-engine-proto.c:

$ grep strcasecmp src/detect-engine-proto.c | cut -f2 -d\"
tcp
tcp-pkt
tcp-stream
udp
icmpv4
icmpv6
icmp
sctp
ipv4
ip4
ipv6
ip6
ip
pkthdr

@satta
Copy link

satta commented Jun 7, 2021

Here's an example for the noalert issue (ET OPEN):

alert http any any -> $HOME_NET any (msg:"ET EXPLOIT IBM Data Risk Manager Authentication Bypass - Session ID Assignment (set)"; flow:established,to_server; xbits:set,ET.IBMDRM1,track ip_dst, expire 10; noalert; http.method; content:"GET"; http.uri; content:"/albatross/saml/idpSelection"; startswith; fast_pattern; content:"id="; content:"userName="; reference:url,github.com/pedrib/PoC/blob/master/advisories/IBM/ibm_drm/ibm_drm_rce.md; classtype:attempted-admin; sid:2029986; rev:2; metadata:attack_target Server, created_at 2020_04_21, deployment Perimeter, deployment Datacenter, deployment SSLDecrypt, former_category EXPLOIT, performance_impact Low, signature_severity Major, updated_at 2020_04_21;) 

which does not parse giving no valid value for noalert tag as the error.

@satta
Copy link

satta commented Jun 7, 2021

You can test the parser with https://gist.github.com/satta/a19397fd91ce182df7570efbd2a14b92 and the ET OPEN ruleset (https://rules.emergingthreats.net/open/suricata-5.0/emerging-all.rules.tar.gz). For example, here are the issues still seen with the current master:

$ egrep -v '^#' emerging-all.rules | ./test-gonids | egrep -o ':    .*' | sort | uniq -c
      1 :    could not parse base64Decode: offset must be positive, non-zero values only
      1 :    no valid value for app-layer-protocol tag
      1 :    no valid value for noalert tag
     38 :    rule contains unsupported option(s): dotprefix
      1 :    some or all destination ips are invalid: [!134.170.0.0/16,$EXTERNAL_NET]
      1 :    some or all destination ips are invalid: [!208.87.232.0/21,!216.115.208.0/20,!216.219.112.0/20,!66.151.158.0/24,!66.151.150.160/27,!66.151.115.128/26,!64.74.80.0/24,!202.173.24.0/21,!67.217.64.0/19,!78.108.112.0/20,!68.64.0.0/19,!206.183.100.0/22,!173.199.0.0/18,!103.15.16.0/22,!180.153.30.0/23,!140.207.108.0/23,!23.239.224.0/19,!185.36.20.0/22,!8.28.150.0/24,!54.208.0.0/15,!54.248.0.0/15,!70.42.29.0/27,!72.5.190.0/24,!104.129.194.0/24,!104.129.200.0/24,!199.168.148.0/24,!199.168.151.0/24,!216.52.207.64/26,$EXTERNAL_NET]
      1 :    some or all destination ips are invalid: [!224.0.0.1,$EXTERNAL_NET]
      1 :    some or all destination ips are invalid: [82.163.143.135,82.163.142.137]
      1 :    some or all destination ips are invalid: [92.63.0.0/16,91.218.114.0/24,149.56.245.196]
      1 :    some or all destination ports are invalid: [!$HTTP_PORTS,1024:]
      8 :    some or all destination ports are invalid: [0:1023]
      1 :    some or all destination ports are invalid: [:1024]
     54 :    some or all destination ports are invalid: [1024:]
      1 :    some or all destination ports are invalid: [104,2104,22104]
      1 :    some or all destination ports are invalid: [!11000,!11001,!12000]
      1 :    some or all destination ports are invalid: [135,139,445,1024:2048]
      2 :    some or all destination ports are invalid: [137,138,139,445]
     42 :    some or all destination ports are invalid: [139,445]
      1 :    some or all destination ports are invalid: [16992,16993,623,664]
      1 :    some or all destination ports are invalid: [20000:]
      1 :    some or all destination ports are invalid: [21,25,110,143,443,465,587,636,989:995,5061,5222]
     26 :    some or all destination ports are invalid: [23,2323]
      4 :    some or all destination ports are invalid: [23,2323,3323,4323]
      3 :    some or all destination ports are invalid: [2375,2376]
      1 :    some or all destination ports are invalid: [25,2525,587]
      1 :    some or all destination ports are invalid: [25,26,587,2525]
      1 :    some or all destination ports are invalid: [!25,!445,!1500]
      3 :    some or all destination ports are invalid: [25,465,587]
     26 :    some or all destination ports are invalid: [25,587]
      1 :    some or all destination ports are invalid: [25,587,2525]
      1 :    some or all destination ports are invalid: [!3478,1023:]
      1 :    some or all destination ports are invalid: [442,443,446,447,8001]
      2 :    some or all destination ports are invalid: [443,636,989,990,992,993,994,995,5061,25]
      1 :    some or all destination ports are invalid: [443,7080,8080]
      9 :    some or all destination ports are invalid: [445,139]
      2 :    some or all destination ports are invalid: [5060,5061]
      1 :    some or all destination ports are invalid: [554,9527]
      2 :    some or all destination ports are invalid: [5555,7547]
      1 :    some or all destination ports are invalid: [!5721,!5938]
      1 :    some or all destination ports are invalid: [!5800,!445]
      1 :    some or all destination ports are invalid: [!5938,!1433]
      1 :    some or all destination ports are invalid: [!5938,!1935,!3265,!2394]
      6 :    some or all destination ports are invalid: [60000:]
      1 :    some or all destination ports are invalid: [623,664]
     16 :    some or all destination ports are invalid: [6892,6893]
      1 :    some or all destination ports are invalid: [7080,8080,443]
      5 :    some or all destination ports are invalid: [7080,8080,443,80,4143,995,21,50000,20,8090,8443,990,22]
      1 :    some or all destination ports are invalid: [80,443]
      1 :    some or all destination ports are invalid: [8080:]
      1 :    some or all destination ports are invalid: [81:]
      1 :    some or all destination ports are invalid: [8443,9090]
      2 :    some or all destination ports are invalid: [9200,9292]
      2 :    some or all destination ports are invalid: [9530,9527,23]
      1 :    some or all source ips are invalid: [108.160.162.0/20,162.125.0.0/16,192.189.200.0/23,199.47.216.0/22,205.189.0.0/24,209.99.70.0/24,45.58.64.0/20]
      1 :    some or all source ips are invalid: [195.22.26.192/26,195.22.28.192/27,195.38.137.100,195.22.4.21,195.157.15.100,212.61.180.100]
      1 :    some or all source ports are invalid: [0:20,22:24,26:118,120:138,140:444,446:464,466:586,588:901,903:1432,1434:65535]
      1 :    some or all source ports are invalid: [10000:]
      1 :    some or all source ports are invalid: [1023:]
     36 :    some or all source ports are invalid: [1024:]
      1 :    some or all source ports are invalid: [104,2104,22104]
      1 :    some or all source ports are invalid: [20000:]
      2 :    some or all source ports are invalid: [21,1024:]
      2 :    some or all source ports are invalid: [21,22,23,25,53,80,443,8080]
      2 :    some or all source ports are invalid: [21,25,110,143,443,465,587,636,989:995,5061,5222]
      1 :    some or all source ports are invalid: [!3389,1024:65535]
      1 :    some or all source ports are invalid: [443,$HTTP_PORTS]
      5 :    some or all source ports are invalid: [443,4443]
      3 :    some or all source ports are invalid: [443,465,993,995,25]
      2 :    some or all source ports are invalid: [445,139]
      1 :    some or all source ports are invalid: [80,443]
      1 :    some or all source ports are invalid: [81:]

@satta
Copy link

satta commented Jun 7, 2021

Can you provide a quick pointer in the source to where these other protocols are defined (probably an issue should be opened with OISF to update the docs to make them complete as well).

I can do that and open an issue in their Redmine later.

@duanehoward
Copy link
Collaborator Author

I think with the current revisions all of the protocol parsing issues should be resolved.
network parsing issues should be resolved as well, with one caveat: We currently define 0 as an invalid port in our validation, This appears to be used in a few places by ET OPEN this requires a bit of consideration as to whether we should allow this value or not, it probably doesn't hurt to do so unless there is a special meaning of 0 in Suricata.

Feel free to pull the newest revision and let me know. I still haven't addressed the noalert issue yet.

@duanehoward
Copy link
Collaborator Author

note: network parsing issues were addressed in #165

@duanehoward duanehoward self-assigned this Jun 9, 2021
@satta
Copy link

satta commented Jun 9, 2021

I think with the current revisions all of the protocol parsing issues should be resolved.

Thanks! Looks good.

network parsing issues should be resolved as well

Nice, I tried something complicated like

alert tcp [10.0.0.0/24,!10.0.0.5,![10.0.0.10,10.0.0.11]] any <> any [1:1000,!99,![80:90]] (msg: "foo"; sid:42; rev:1;)

and it looks like it was parsed correctly now. However, the parser still seems to disallow spaces between the brackets, which according to the Suricata documentation is OK to do.

I also noticed that apparently invalid source/dest patterns like

[10.0.0.0/24,![10.0.0.10,10.0.0.11] any

(note the disbalanced brackets) is currently successfully parsed to

10.0.0.0/24,![10.0.0.10,10.0.0.11 any

I think this should be an error.

with one caveat: We currently define 0 as an invalid port in our validation, This appears to be used in a few places by ET OPEN this requires a bit of consideration as to whether we should allow this value or not, it probably doesn't hurt to do so unless there is a special meaning of 0 in Suricata.

I think since this is not a syntax issue I would be inclined to allow the value 0, tool.

Feel free to pull the newest revision and let me know. I still haven't addressed the noalert issue yet.

I see. The dotprefix is also not in there yet. But getting there! Many thanks for looking into this! 👍🏻

@duanehoward
Copy link
Collaborator Author

IIRC we removed support for spaces in the past, possibly because it was introducing some odd complexity in the lexer, and we noted that basically all rulesets and examples don't use spaces. (the lexer can't look for a ; when finding the network tokens or similar and keying on [ and ] was problematic as well. I don't believe support for spaces is something we can quickly fix. PR's are welcome for that.

#167 should resolve the unbalanced [ and ] issue and adds support for ports to include 0.

The handling of nested groups appears to have introduced another bug in handling negated network groups where the negation is the first part of the pattern something like:
![$DNS_SERVERS,$SMTP_SERVERS]

This issue is strangely complex to find a solution for, and frankly the more I look at this, I wonder if it would be simpler to move the network definitions into better defined structs and lex them differently. That, however is not something I can handle in the short term, so I'll track that as a distinct issue.

TODO(duane): dotprefix and bare noalert

@satta
Copy link

satta commented Jun 10, 2021

IIRC we removed support for spaces in the past, possibly because it was introducing some odd complexity in the lexer, and we noted that basically all rulesets and examples don't use spaces. (the lexer can't look for a ; when finding the network tokens or similar and keying on [ and ] was problematic as well. I don't believe support for spaces is something we can quickly fix.

Yes, I suspected that. But I think that's more of a minor issue since we do get an error message if spaces are encountered instead of silently parsing the range incorrectly.

#167 should resolve the unbalanced [ and ] issue and adds support for ports to include 0.

Great, thanks!

The handling of nested groups appears to have introduced another bug in handling negated network groups where the negation is the first part of the pattern something like:
![$DNS_SERVERS,$SMTP_SERVERS]

This issue is strangely complex to find a solution for, and frankly the more I look at this, I wonder if it would be simpler to move the network definitions into better defined structs and lex them differently. That, however is not something I can handle in the short term, so I'll track that as a distinct issue.

Yes, it is more complex than one might think at first glance. When I wrote a rule parser in Python the last time, I used https://github.com/lark-parser/lark to work out a comprehensive grammar, which I found easier to tweak and optimize than having to work with actual parsing code. I didn't research if there is something similar for Go though. Maybe something like https://github.com/alecthomas/participle 🤔

Anyway, thanks so far for all the work you put in, it has really improved things!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants