From acc6d7b86925a8d90b9089cf12b501d9deed7efd Mon Sep 17 00:00:00 2001 From: unknown Date: Sun, 13 Jul 2025 18:50:23 -0700 Subject: [PATCH 1/3] Address first part of issue #151. Updates the Nets struct to include boolean values for negation. --- parser.go | 16 ++++++++++++++-- parser_test.go | 25 +++++++++++++++++++++++++ rule.go | 21 +++++++++++++++++---- rule_test.go | 19 ++++++++++++++----- 4 files changed, 70 insertions(+), 11 deletions(-) diff --git a/parser.go b/parser.go index e189a6b..7b6fbb1 100644 --- a/parser.go +++ b/parser.go @@ -473,8 +473,16 @@ func (r *Rule) network(key item, l *lexer) error { // This is a hack. We use a regexp to replace the outer `,` with `___` // to give us a discrete string to split on, avoiding the inner `,`. - // Specify TrimSuffix and TrimPrefix to ensure only one instance of `[` and `]` are trimmed. - tmp := strings.TrimSuffix(strings.TrimPrefix(key.value, "["), "]") + // Specify TrimSuffix and TrimPrefix to ensure only one instance of `!`, `[` and `]` are trimmed. + + // Notes to self, this fixes the "!" parsing issue, but drops the negation... maybe the negation needs to be set before calling this? + // Note: Network struct is slices of strings, negation is not yet a bool, so this might be fine for now? + // Note: Nope! Because we pop these off for validation we lose them entirely. So we end up with positive ports instead of negated ones. + // I also don't see tests covering negated networks like !$HOME_NET which this likely breaks. we'll need better handling of negation + // of the whole set, probably by adding a bool for each value. The *correct* way to do this is to finally move away from []string to a better structure... + tmp := strings.TrimPrefix(key.value, "!") + negated := len(tmp) < len(key.value) + tmp = strings.TrimSuffix(strings.TrimPrefix(tmp, "["), "]") items := strings.Split(nestedNetRE.ReplaceAllString(tmp, "___${1}"), "___") // Validate that no items contain spaces. @@ -485,24 +493,28 @@ func (r *Rule) network(key item, l *lexer) error { } switch key.typ { case itemSourceAddress: + r.Source.NegateNets = negated if validNetworks(items) { r.Source.Nets = append(r.Source.Nets, items...) } else { return fmt.Errorf("some or all source ips are invalid: %v", items) } case itemSourcePort: + r.Source.NegatePorts = negated if portsValid(items) { r.Source.Ports = append(r.Source.Ports, items...) } else { return fmt.Errorf("some or all source ports are invalid: %v", items) } case itemDestinationAddress: + r.Destination.NegateNets = negated if validNetworks(items) { r.Destination.Nets = append(r.Destination.Nets, items...) } else { return fmt.Errorf("some or all destination ips are invalid: %v", items) } case itemDestinationPort: + r.Destination.NegatePorts = negated if portsValid(items) { r.Destination.Ports = append(r.Destination.Ports, items...) } else { diff --git a/parser_test.go b/parser_test.go index 4187783..bf9ceb5 100644 --- a/parser_test.go +++ b/parser_test.go @@ -1980,6 +1980,31 @@ func TestParseRule(t *testing.T) { }, }, }, + { + name: "negated port list", + rule: `alert tcp any any -> any ![80,443,9000] (msg:"negated port list"; content:"123"; sid:1; rev:1;)`, + want: &Rule{ + Action: "alert", + Protocol: "tcp", + Source: Network{ + Nets: []string{"any"}, + Ports: []string{"any"}, + }, + Destination: Network{ + Nets: []string{"any"}, + NegatePorts: true, + Ports: []string{"80,443,9000"}, + }, + Description: "negated port list", + Matchers: []orderedMatcher{ + &Content{ + Pattern: []byte("123"), + }, + }, + SID: 1, + Revision: 1, + }, + }, { name: "raw network list", rule: `alert tcp [174.129.0.0/16,67.202.0.0/18] any -> $HOME_NET any (msg:"raw network list"; content:"hi"; sid:12345; rev:1;)`, diff --git a/rule.go b/rule.go index 086f11c..c4c5ca3 100644 --- a/rule.go +++ b/rule.go @@ -105,9 +105,12 @@ type Metadatas []*Metadata // Network describes the IP addresses and port numbers used in a rule. // TODO: Ensure all values either begin with $ (variable) or they are valid IPNet/int. +// TODO: Refactor Nets and Ports into structs, each with their own bool for negation. type Network struct { - Nets []string // Currently just []string because these can be variables $HOME_NET, not a valid IPNet. - Ports []string // Currently just []string because these can be variables $HTTP_PORTS, not just ints. + NegateNets bool // Negate the full set of Networks. + Nets []string // Currently just []string because these can be variables $HOME_NET, not a valid IPNet. + NegatePorts bool // Negate the full set of ports. + Ports []string // Currently just []string because these can be variables $HTTP_PORTS, not just ints. } // DataPos indicates the data position for content matches. These should be referenced for creation @@ -664,9 +667,19 @@ func netString(netPart []string) string { return s.String() } -// String retunrs a string for a Network. +// String returns a string for a Network. func (n Network) String() string { - return fmt.Sprintf("%s %s", netString(n.Nets), netString(n.Ports)) + var s strings.Builder + if n.NegateNets { + s.WriteString("!") + } + s.WriteString(netString(n.Nets)) + s.WriteString(" ") + if n.NegatePorts { + s.WriteString("!") + } + s.WriteString(netString(n.Ports)) + return s.String() } // String returns a string for a FastPattern. diff --git a/rule_test.go b/rule_test.go index 79a1955..af25206 100644 --- a/rule_test.go +++ b/rule_test.go @@ -343,9 +343,9 @@ func TestNetworkString(t *testing.T) { name: "complex net", input: Network{ Nets: []string{"$HOME_NET", "!$FOO_NET", "192.168.0.0/16"}, - Ports: []string{"$HTTP_PORTS", "!53", "$BAR_NET"}, + Ports: []string{"$HTTP_PORTS", "!53", "$BAR_PORTS"}, }, - want: "[$HOME_NET,!$FOO_NET,192.168.0.0/16] [$HTTP_PORTS,!53,$BAR_NET]", + want: "[$HOME_NET,!$FOO_NET,192.168.0.0/16] [$HTTP_PORTS,!53,$BAR_PORTS]", }, { name: "grouped ports", @@ -358,10 +358,19 @@ func TestNetworkString(t *testing.T) { { name: "grouped networks", input: Network{ - Nets: []string{"192.168.0.0/16", "![192.168.86.0/24,192.168.87.0/24]"}, - Ports: []string{"$HTTP_PORTS", "!53", "$BAR_NET"}, + Nets: []string{"192.168.0.0/16", "[192.168.86.0/24,192.168.87.0/24]"}, + Ports: []string{"$HTTP_PORTS", "!53", "$BAR_PORTS"}, }, - want: "[192.168.0.0/16,![192.168.86.0/24,192.168.87.0/24]] [$HTTP_PORTS,!53,$BAR_NET]", + want: "[192.168.0.0/16,[192.168.86.0/24,192.168.87.0/24]] [$HTTP_PORTS,!53,$BAR_PORTS]", + }, + { + name: "negated networks", + input: Network{ + NegateNets: true, + Nets: []string{"192.168.0.0/16", "[192.168.86.0/24,192.168.87.0/24]"}, + Ports: []string{"$HTTP_PORTS", "!53", "$BAR_PORTS"}, + }, + want: "![192.168.0.0/16,[192.168.86.0/24,192.168.87.0/24]] [$HTTP_PORTS,!53,$BAR_PORTS]", }, } { got := tt.input.String() From 3c61baf6a3170717beea4a3c8dfdd2f13948f8ca Mon Sep 17 00:00:00 2001 From: Duane Howard Date: Sun, 13 Jul 2025 18:55:56 -0700 Subject: [PATCH 2/3] Remove unnecessary comments, move some comments around. --- parser.go | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/parser.go b/parser.go index 7b6fbb1..3b4a9a3 100644 --- a/parser.go +++ b/parser.go @@ -470,18 +470,13 @@ func (r *Rule) protocol(key item, l *lexer) error { // network decodes an IDS rule network (networks and ports) based on its key. func (r *Rule) network(key item, l *lexer) error { - // This is a hack. We use a regexp to replace the outer `,` with `___` - // to give us a discrete string to split on, avoiding the inner `,`. - - // Specify TrimSuffix and TrimPrefix to ensure only one instance of `!`, `[` and `]` are trimmed. - - // Notes to self, this fixes the "!" parsing issue, but drops the negation... maybe the negation needs to be set before calling this? - // Note: Network struct is slices of strings, negation is not yet a bool, so this might be fine for now? - // Note: Nope! Because we pop these off for validation we lose them entirely. So we end up with positive ports instead of negated ones. - // I also don't see tests covering negated networks like !$HOME_NET which this likely breaks. we'll need better handling of negation - // of the whole set, probably by adding a bool for each value. The *correct* way to do this is to finally move away from []string to a better structure... + // Identify if the whole network component is negated. tmp := strings.TrimPrefix(key.value, "!") negated := len(tmp) < len(key.value) + + // This is a hack. We use a regexp to replace the outer `,` with `___` + // to give us a discrete string to split on, avoiding the inner `,`. + // Specify TrimSuffix and TrimPrefix to ensure only one instance of `[` and `]` are trimmed. tmp = strings.TrimSuffix(strings.TrimPrefix(tmp, "["), "]") items := strings.Split(nestedNetRE.ReplaceAllString(tmp, "___${1}"), "___") From 1214ed13460fa44bd08f5f8173fb85da8d508e89 Mon Sep 17 00:00:00 2001 From: Duane Howard Date: Sun, 13 Jul 2025 19:02:17 -0700 Subject: [PATCH 3/3] Add another test for netgated network blocks. --- parser_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/parser_test.go b/parser_test.go index bf9ceb5..94d6f55 100644 --- a/parser_test.go +++ b/parser_test.go @@ -2005,6 +2005,31 @@ func TestParseRule(t *testing.T) { Revision: 1, }, }, + { + name: "negated network list", + rule: `alert tcp ![$HOME_NET,192.168.1.1] any -> any $HTTP_PORTS (msg:"negated port list"; content:"123"; sid:1; rev:1;)`, + want: &Rule{ + Action: "alert", + Protocol: "tcp", + Source: Network{ + NegateNets: true, + Nets: []string{"$HOME_NET,192.168.1.1"}, + Ports: []string{"any"}, + }, + Destination: Network{ + Nets: []string{"any"}, + Ports: []string{"$HTTP_PORTS"}, + }, + Description: "negated port list", + Matchers: []orderedMatcher{ + &Content{ + Pattern: []byte("123"), + }, + }, + SID: 1, + Revision: 1, + }, + }, { name: "raw network list", rule: `alert tcp [174.129.0.0/16,67.202.0.0/18] any -> $HOME_NET any (msg:"raw network list"; content:"hi"; sid:12345; rev:1;)`,