Skip to content

Commit 035bfdb

Browse files
authored
Use CIDRs to decide allowed IPs for testing (#826)
This fixes validation of IPv6 addresses, their string representation may vary depending on leading zeroes or abbreviated addresses. It also extends the number of IP addresses that can be used for testing, specially IPv6 addresses.
1 parent dcfabeb commit 035bfdb

File tree

3 files changed

+69
-35
lines changed

3 files changed

+69
-35
lines changed
Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,9 @@
1-
1.128.3.4
2-
175.16.199.1
3-
216.160.83.57
4-
216.160.83.61
5-
81.2.69.143
6-
81.2.69.144
7-
81.2.69.145
8-
81.2.69.193
9-
89.160.20.112
10-
89.160.20.156
11-
67.43.156.12
12-
67.43.156.13
13-
67.43.156.14
14-
67.43.156.15
15-
2a02:cf40:add:4002:91f2:a9b2:e09a:6fc6
1+
1.128.0.0/11
2+
175.16.199.0/24
3+
216.160.83.56/29
4+
81.2.69.142/31
5+
81.2.69.192/28
6+
89.160.20.112/28
7+
89.160.20.128/25
8+
67.43.156.0/24
9+
2a02:cf40::/29

internal/fields/validate.go

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package fields
66

77
import (
8+
"bufio"
89
_ "embed"
910
"encoding/json"
1011
"fmt"
@@ -35,7 +36,7 @@ type Validator struct {
3536
disabledDependencyManagement bool
3637

3738
enabledAllowedIPCheck bool
38-
allowedIPs map[string]struct{}
39+
allowedCIDRs []*net.IPNet
3940
}
4041

4142
// ValidatorOption represents an optional flag that can be passed to CreateValidatorForDataStream.
@@ -86,7 +87,7 @@ func CreateValidatorForDataStream(dataStreamRootPath string, opts ...ValidatorOp
8687
}
8788
}
8889

89-
v.allowedIPs = initializeAllowedIPsList()
90+
v.allowedCIDRs = initializeAllowedCIDRsList()
9091

9192
v.Schema, err = loadFieldsForDataStream(dataStreamRootPath)
9293
if err != nil {
@@ -118,20 +119,17 @@ func CreateValidatorForDataStream(dataStreamRootPath string, opts ...ValidatorOp
118119
//go:embed _static/allowed_geo_ips.txt
119120
var allowedGeoIPs string
120121

121-
func initializeAllowedIPsList() map[string]struct{} {
122-
m := map[string]struct{}{
123-
"0.0.0.0": {}, "255.255.255.255": {},
124-
"0:0:0:0:0:0:0:0": {}, "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff": {}, "::": {},
125-
}
126-
for _, ip := range strings.Split(allowedGeoIPs, "\n") {
127-
ip = strings.Trim(ip, " \n\t")
128-
if ip == "" {
129-
continue
122+
func initializeAllowedCIDRsList() (cidrs []*net.IPNet) {
123+
s := bufio.NewScanner(strings.NewReader(allowedGeoIPs))
124+
for s.Scan() {
125+
_, cidr, err := net.ParseCIDR(s.Text())
126+
if err != nil {
127+
panic("invalid ip in _static/allowed_geo_ips.txt: " + s.Text())
130128
}
131-
m[ip] = struct{}{}
129+
cidrs = append(cidrs, cidr)
132130
}
133131

134-
return m
132+
return cidrs
135133
}
136134

137135
func loadFieldsForDataStream(dataStreamRootPath string) ([]FieldDefinition, error) {
@@ -442,17 +440,18 @@ func (v *Validator) parseElementValue(key string, definition FieldDefinition, va
442440
// - 0.0.0.0 and 255.255.255.255 for IPv4
443441
// - 0:0:0:0:0:0:0:0 and ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff for IPv6
444442
func (v *Validator) isAllowedIPValue(s string) bool {
445-
if _, found := v.allowedIPs[s]; found {
446-
return true
447-
}
448-
449443
ip := net.ParseIP(s)
450444
if ip == nil {
451445
return false
452446
}
453447

454-
if ip.IsPrivate() || ip.IsLoopback() ||
455-
ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() {
448+
for _, allowedCIDR := range v.allowedCIDRs {
449+
if allowedCIDR.Contains(ip) {
450+
return true
451+
}
452+
}
453+
454+
if ip.IsUnspecified() || ip.IsPrivate() || ip.IsLoopback() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() {
456455
return true
457456
}
458457

internal/fields/validate_test.go

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,42 @@ func Test_parseElementValue(t *testing.T) {
194194
},
195195
fail: true,
196196
},
197+
{
198+
key: "ip in allowed list",
199+
value: "1.128.3.4",
200+
definition: FieldDefinition{
201+
Type: "ip",
202+
},
203+
},
204+
{
205+
key: "ipv6 in allowed list",
206+
value: "2a02:cf40:add:4002:91f2:a9b2:e09a:6fc6",
207+
definition: FieldDefinition{
208+
Type: "ip",
209+
},
210+
},
211+
{
212+
key: "unspecified ipv6",
213+
value: "::",
214+
definition: FieldDefinition{
215+
Type: "ip",
216+
},
217+
},
218+
{
219+
key: "abbreviated ipv6 in allowed list with leading 0",
220+
value: "2a02:cf40:0add:0::1",
221+
definition: FieldDefinition{
222+
Type: "ip",
223+
},
224+
},
225+
{
226+
key: "ip not in geoip database",
227+
value: "8.8.8.8",
228+
definition: FieldDefinition{
229+
Type: "ip",
230+
},
231+
fail: true,
232+
},
197233
// text
198234
{
199235
key: "text",
@@ -266,7 +302,12 @@ func Test_parseElementValue(t *testing.T) {
266302
},
267303
},
268304
} {
269-
v := Validator{disabledDependencyManagement: true}
305+
v := Validator{
306+
disabledDependencyManagement: true,
307+
enabledAllowedIPCheck: true,
308+
allowedCIDRs: initializeAllowedCIDRsList(),
309+
}
310+
270311
t.Run(test.key, func(t *testing.T) {
271312
err := v.parseElementValue(test.key, test.definition, test.value)
272313
if test.fail {

0 commit comments

Comments
 (0)