You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by ro...@apache.org on 2019/03/22 15:18:21 UTC

[trafficcontrol] branch master updated: Added ip_allow config parser and test (#3370)

This is an automated email from the ASF dual-hosted git repository.

rob pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git


The following commit(s) were added to refs/heads/master by this push:
     new 2cfa7ad  Added ip_allow config parser and test (#3370)
2cfa7ad is described below

commit 2cfa7ad9ff853b654dfd8e168de7eed92237f0e2
Author: Matthew Allen Moltzau <Ma...@comcast.com>
AuthorDate: Fri Mar 22 09:18:15 2019 -0600

    Added ip_allow config parser and test (#3370)
    
    * Added functions to validate an IP range. No tests exist for it yet.
    
    * Initial ip_allow.config parser. Contains basic tests. Really rough.
    
    * Added tests for ip_allow.go and ValidateIPRange
    
    Small note:
      - I currently don't know if an empty ip_allow config should be allowed
      - I do not check for method redundancy (ie. method=GET|GET)
      - I do not check for duplicate lines or rules that contradict each other
    
    * Removed dot imports
    
    * Misc changes
    
    * Made GoDoc comment
    * Added ^ and $ tokens to the regex
---
 traffic_ops/testing/api/v14/config/common.go       |  98 +++++++++++
 traffic_ops/testing/api/v14/config/common_test.go  |  48 +++++
 traffic_ops/testing/api/v14/config/error.go        |   2 +
 .../testing/api/v14/config/ip_allow/ip_allow.go    | 161 +++++++++++++++++
 .../api/v14/config/ip_allow/ip_allow_test.go       | 196 +++++++++++++++++++++
 5 files changed, 505 insertions(+)

diff --git a/traffic_ops/testing/api/v14/config/common.go b/traffic_ops/testing/api/v14/config/common.go
index b21931e..68464e3 100644
--- a/traffic_ops/testing/api/v14/config/common.go
+++ b/traffic_ops/testing/api/v14/config/common.go
@@ -15,9 +15,12 @@
 package config
 
 import (
+	"bytes"
 	"fmt"
+	"net"
 	"regexp"
 	"strconv"
+	"strings"
 	"time"
 )
 
@@ -85,3 +88,98 @@ func ValidateDHMSTimeFormat(time string) error {
 
 	return nil
 }
+
+// expandIP matches a general IPv4 pattern and expands the captured octet
+// depending on the number of matches. This function does not validate
+// that the ip is correct or even that it matches the general pattern, but
+// it instead puts the input into a form that go's net package can validate.
+func expandIP(ip string) string {
+
+	// d.d.d.d with optional groups of (.d)
+	ipRegex := regexp.MustCompile(`^(\d+)(?:\.(\d+))?(?:\.(\d+))?(?:\.(\d+))?$`)
+	match := ipRegex.FindStringSubmatch(ip)
+	if match == nil {
+		return ""
+	}
+
+	// ping supports expanding IPv4 addresses
+	// PING 1     (0.0.0.1)
+	// PING 1.2   (1.0.0.2)
+	// PING 1.2.3 (1.2.0.3)
+	if match[2] == "" {
+		return fmt.Sprintf("0.0.0.%v", match[1])
+	}
+	if match[3] == "" {
+		return fmt.Sprintf("%v.0.0.%v", match[1], match[2])
+	}
+	if match[4] == "" {
+		return fmt.Sprintf("%v.%v.0.%v", match[1], match[2], match[3])
+	}
+	return ip
+}
+
+// parseIP first uses go's net package to test for the common case of a standard
+// ipv4 or ipv6 address. If that doesn't pass, it is possible that the address is
+// ipv4 written in shorthand notation. The ip is expanded, then we try again.
+func parseIP(ip string) net.IP {
+	if goip := net.ParseIP(ip); goip != nil {
+		return goip
+	}
+	if goip := net.ParseIP(expandIP(ip)); goip != nil {
+		return goip
+	}
+	return nil
+}
+
+// ValidateIPRange validates one of the following forms:
+// 1) IP (supports shorthand of IPv4 and IPv6 addresses)
+// 2) IP/n (CIDR)
+// 3) IP1-IP2 where IP1 < IP2 and type(IP1) == type(IP2)
+func ValidateIPRange(ip string) error {
+
+	var err error
+	var splt []string
+
+	splt = strings.Split(ip, "-")
+	if len(splt) == 2 {
+		ip1 := parseIP(splt[0])
+		ip2 := parseIP(splt[1])
+
+		// both must be valid
+		if ip1 == nil || ip2 == nil {
+			return fmt.Errorf("invalid IP range: %v \n", ip)
+		}
+
+		// must be of the same type
+		if (ip1.To4() == nil) != (ip2.To4() == nil) {
+			return fmt.Errorf("invalid IP range: %v \n", ip)
+		}
+
+		// ip2 must be less than ip1
+		if bytes.Compare(ip2, ip1) < 0 {
+			return fmt.Errorf("invalid IP range: %v \n", ip)
+		}
+		return nil
+	}
+
+	if goip := parseIP(ip); goip != nil {
+		return nil
+	}
+
+	// first try for CIDR
+	if _, _, err = net.ParseCIDR(ip); err == nil {
+		return nil
+	}
+
+	// if it looks like we have a CIDR pattern we try to expand the ip and try again
+	splt = strings.Split(ip, "/")
+	if len(splt) == 2 {
+		ip = fmt.Sprintf("%v/%v", expandIP(splt[0]), splt[1])
+		if _, _, err = net.ParseCIDR(ip); err == nil {
+			return nil
+		}
+		return fmt.Errorf("invalid CIDR address: %v \n", ip)
+	}
+
+	return fmt.Errorf("invalid IP range: %v", ip)
+}
diff --git a/traffic_ops/testing/api/v14/config/common_test.go b/traffic_ops/testing/api/v14/config/common_test.go
index c03947d..0a0bd76 100644
--- a/traffic_ops/testing/api/v14/config/common_test.go
+++ b/traffic_ops/testing/api/v14/config/common_test.go
@@ -108,3 +108,51 @@ func TestDHMSTimeFormat(t *testing.T) {
 	}
 
 }
+
+// TestIPRange
+// \__> parseIpP (only two simple branches)
+// \__> expandIP (simple enough to test all 5 cases)
+func TestIPRange(t *testing.T) {
+
+	var tests = []struct {
+		ip string
+		ok bool
+	}{
+		{"", false},
+		{"x", false},
+		{"0", true},
+		{"0.0", true},
+		{"0.0.0", true},
+		{"0.0.0.0", true},
+		{"256.0.0.0", false},
+		{"0.0.0.0/0", true},
+		{"0.0.0.0/x", false},
+		{"0.0.0/0", true},
+		{"0.0.0.0-255.255.255.255", true},
+		{"255.255.255.255-0.0.0.0", false},
+		{"0.0.0-255.255.255.255", true},
+		{"0.0.0.0-255.255.255", true},
+		{"0.0.0.x-255.255.255.255", false},
+		{"::/0", true},
+		{"::-ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", true},
+		{"ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff-::", false},
+		{"::-0", false},
+		{"0.0.0.0-ffff::ffff", false},
+		{"::-ffff::ffff", true},
+	}
+
+	for _, test := range tests {
+		if err := ValidateIPRange(test.ip); (err == nil) != test.ok {
+			if test.ok {
+				t.Errorf(`
+  test should have passed
+  ip: %v
+  error: %v`, test.ip, err)
+			} else {
+				t.Errorf(`
+  test should not have passed
+  ip: %v`, test.ip)
+			}
+		}
+	}
+}
diff --git a/traffic_ops/testing/api/v14/config/error.go b/traffic_ops/testing/api/v14/config/error.go
index 708c821..c5cfdcd 100644
--- a/traffic_ops/testing/api/v14/config/error.go
+++ b/traffic_ops/testing/api/v14/config/error.go
@@ -30,6 +30,7 @@ const (
 	InvalidHost
 	InvalidIP
 	UnknownMethod
+	InvalidIPRange
 	InvalidPort
 	InvalidRegex
 	InvalidTimeFormatDHMS
@@ -54,6 +55,7 @@ func init() {
 		InvalidHost,
 		InvalidIP,
 		UnknownMethod,
+		InvalidIPRange,
 		InvalidPort,
 		InvalidRegex,
 		InvalidTimeFormatDHMS,
diff --git a/traffic_ops/testing/api/v14/config/ip_allow/ip_allow.go b/traffic_ops/testing/api/v14/config/ip_allow/ip_allow.go
new file mode 100644
index 0000000..2441448
--- /dev/null
+++ b/traffic_ops/testing/api/v14/config/ip_allow/ip_allow.go
@@ -0,0 +1,161 @@
+/*
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package ip_allow
+
+import (
+	"regexp"
+	"strings"
+
+	"github.com/apache/trafficcontrol/traffic_ops/testing/api/v14/config"
+	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/test"
+)
+
+// Parse takes a string presumed to be an ATS ip_allow.config and validates that it is
+// syntatically correct.
+//
+// The general format of an ip_allow config is the following:
+//
+//  src_ip=<range of IP addresses> action=<action> [method=<list of methods separated by '|'>]
+//  dest_ip=<range of IP addresses> action=<action> [method=<list of methods separated by '|'>]
+//
+// For a full description of the syntax, refer to the ATS documentation for the ip_allow config:
+// https://docs.trafficserver.apache.org/en/latest/admin-guide/files/cache.config.en.html
+//
+func Parse(config string) test.Error {
+	lines := strings.Split(config, "\n")
+
+	if len(lines) == 1 {
+		return parseConfigRule(lines[0])
+	}
+
+	for i, ln := range lines {
+		err := parseConfigRule(ln)
+		if err != nil {
+			return err.Prepend("error on line %d: ", i+1)
+		}
+	}
+
+	return nil
+}
+
+func parseLabelValue(lhs string, rhs string) test.Error {
+
+	switch lhs {
+	case "src_ip":
+		fallthrough
+	case "dest_ip":
+		if err := config.ValidateIPRange(rhs); err != nil {
+			return config.ErrorContext.AddErrorCode(config.InvalidIPRange, err)
+		}
+	case "action":
+		switch rhs {
+		case "ip_allow":
+		case "ip_deny":
+		default:
+			return config.ErrorContext.NewError(config.InvalidAction)
+		}
+	case "method":
+		methods := strings.Split(rhs, "|")
+		for _, method := range methods {
+			switch method {
+			// assuming all methods are valid
+			// see RFC 2616-9 for list of all methods
+			// PUSH and PURGE are specific to ATS
+			case "all":
+			case "get":
+			case "put":
+			case "post":
+			case "delete":
+			case "trace":
+			case "options":
+			case "head":
+			case "connect":
+			case "patch":
+			case "purge":
+			case "push":
+			default:
+				return config.ErrorContext.NewError(config.UnknownMethod, `unknown method "%v"`, method)
+			}
+		}
+
+	default:
+		return config.ErrorContext.NewError(config.InvalidLabel)
+	}
+
+	return nil
+}
+
+func parseConfigRule(rule string) test.Error {
+
+	var ip bool
+	var action bool
+
+	var match []string
+	var err test.Error
+
+	rule = strings.Trim(rule, "\t ")
+	if rule == "" || strings.HasPrefix(rule, "#") {
+		return nil
+	}
+
+	assignments := strings.Fields(rule)
+	last := len(assignments) - 1
+	if last < 1 {
+		return config.ErrorContext.NewError(config.NotEnoughAssignments)
+	}
+
+	// neither the rhs or lhs can contain any whitespace
+	assignment := regexp.MustCompile(`^([a-z_\-\d]+)=(\S+)$`)
+	for _, elem := range assignments {
+		match = assignment.FindStringSubmatch(strings.ToLower(elem))
+		if match == nil {
+			return config.ErrorContext.NewError(config.BadAssignmentMatch, `could not match assignment: "%v"`, elem)
+		}
+
+		err = parseLabelValue(match[1], match[2])
+		if err == nil {
+			switch match[1] {
+			case "action":
+				if action {
+					return config.ErrorContext.NewError(config.ExcessLabel, "only one action is allowed per rule")
+				}
+				action = true
+			case "src_ip":
+				fallthrough
+			case "dest_ip":
+				if ip {
+					return config.ErrorContext.NewError(config.ExcessLabel, "only one of src_ip or dest_ip is allowed per rule")
+				}
+				ip = true
+			}
+			continue
+		}
+
+		if err.Code() == config.InvalidLabel {
+			return err
+		} else {
+			return err.Prepend(`could not parse %s:`, match[1])
+		}
+	}
+
+	if !ip {
+		return config.ErrorContext.NewError(config.MissingLabel, "missing either src_ip or dest_ip label")
+	}
+	if !action {
+		return config.ErrorContext.NewError(config.MissingLabel, "missing action label")
+	}
+
+	return nil
+}
diff --git a/traffic_ops/testing/api/v14/config/ip_allow/ip_allow_test.go b/traffic_ops/testing/api/v14/config/ip_allow/ip_allow_test.go
new file mode 100644
index 0000000..140df20
--- /dev/null
+++ b/traffic_ops/testing/api/v14/config/ip_allow/ip_allow_test.go
@@ -0,0 +1,196 @@
+/*
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package ip_allow_test
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/apache/trafficcontrol/traffic_ops/testing/api/v14/config"
+	"github.com/apache/trafficcontrol/traffic_ops/testing/api/v14/config/ip_allow"
+)
+
+var negativeTests = []config.NegativeTest{
+	{
+		"too few assignments",
+		"foo1=foo",
+		config.NotEnoughAssignments,
+	},
+	{
+		"empty assignment",
+		"src_ip= foo1=foo",
+		config.BadAssignmentMatch,
+	},
+	{
+		"missing equals in assignment",
+		"src_ip foo1=foo",
+		config.BadAssignmentMatch,
+	},
+	{
+		"missing src_ip label",
+		"action=ip_allow method=ALL",
+		config.MissingLabel,
+	},
+	{
+		"missing action label",
+		"src_ip=0/0 method=ALL",
+		config.MissingLabel,
+	},
+	{
+		"unknown field",
+		"foo1=foo foo2=foo foo3=foo",
+		config.InvalidLabel,
+	},
+	{
+		"extra action",
+		"src_ip=0/0 action=ip_allow action=ip_deny",
+		config.ExcessLabel,
+	},
+	{
+		"extra ip",
+		"src_ip=0/0 dest_ip=0.0.0.0 action=ip_allow",
+		config.ExcessLabel,
+	},
+	{
+		"invalid label on second line",
+		fmt.Sprintf("%s\n\n%s",
+			"src_ip=0/0 action=ip_allow",
+			"foo1=foo foo2=foo foo3=foo"),
+		config.InvalidLabel,
+	},
+	{
+		"bad src_ip",
+		"src_ip=x action=ip_allow",
+		config.InvalidIPRange,
+	},
+	{
+		"bad action",
+		"src_ip=0/0 action=ip_allowed",
+		config.InvalidAction,
+	},
+	{
+		"unknown method",
+		"src_ip=0/0 action=ip_allow method=x",
+		config.UnknownMethod,
+	},
+	{
+		"bad method delimiter",
+		"src_ip=0/0 action=ip_allow method=GET|",
+		config.UnknownMethod,
+	},
+	{
+		"bad method delimiter",
+		"src_ip=0/0 action=ip_allow method=GET,",
+		config.UnknownMethod,
+	},
+	{
+		"bad ip",
+		"dest_ip=bad_ip foo1=foo",
+		config.InvalidIPRange,
+	},
+}
+
+var positiveTests = []config.PositiveTest{
+	{
+		"IPv4",
+		"src_ip=127.0.0.1 action=ip_allow method=ALL",
+	},
+	{
+		"IPv6 shorthand",
+		"src_ip=::1 action=ip_allow method=ALL",
+	},
+	{
+		"IPv4 dash range",
+		"src_ip=0.0.0.0-255.255.255.255 action=ip_allow",
+	},
+	{
+		"Multiple methods",
+		"src_ip=0.0.0.0-255.255.255.255 action=ip_deny method=PUSH|PURGE|DELETE",
+	},
+	{
+		"Multiple methods",
+		"src_ip=0.0.0.0-255.255.255.255 action=ip_deny method=PUSH method=PURGE method=DELETE",
+	},
+	{
+		"IPv6 dash range",
+		"src_ip=::-ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff action=ip_deny method=PUSH|PURGE|DELETE",
+	},
+	{
+		"IPv4 CIDR",
+		"src_ip=123.45.6.0/24 action=ip_deny",
+	},
+	{
+		"IPv4 CIDR (shorthand)",
+		"src_ip=0/0 action=ip_deny method=PUSH|PURGE|DELETE",
+	},
+	{
+		"IPv6 CIDR",
+		"src_ip=::/0 action=ip_deny method=PUSH|PURGE|DELETE",
+	},
+	{
+		"normal config returned from traffic ops",
+		fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n",
+			"# DO NOT EDIT - Generated by Traffic Ops on Fri Mar 1 18:30:48 UTC 2019",
+			"src_ip=127.0.0.1 action=ip_allow method=ALL",
+			"src_ip=::1 action=ip_allow method=ALL",
+			"src_ip=0.0.0.0-255.255.255.255 action=ip_deny method=PUSH|PURGE|DELETE",
+			"src_ip=::-ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff action=ip_deny method=PUSH|PURGE|DELETE"),
+	},
+	{
+		"odd whitespace",
+		fmt.Sprintf("%s\n%s\n%s\n",
+			"src_ip=127.0.0.1\taction=ip_allow\tmethod=ALL",
+			"src_ip=::1  action=ip_allow  method=ALL",
+			"src_ip=0.0.0.0-255.255.255.255 \taction=ip_deny  \t method=PUSH|PURGE|DELETE"),
+	},
+	{
+		"multiline config with empty line",
+		fmt.Sprintf("%s\n\n%s",
+			"src_ip=0/0 action=ip_allow",
+			"src_ip=0/0 action=ip_deny"),
+	},
+	{
+		"many empty lines in config",
+		fmt.Sprintf("\n%s\n\n%s",
+			"src_ip=0/0 action=ip_deny\n",
+			"src_ip=0/0 action=ip_allow method=GET\n"),
+	},
+}
+
+func TestIPAllowConfig(t *testing.T) {
+
+	// Negative Tests
+	for _, test := range negativeTests {
+		actual := ip_allow.Parse(test.Config)
+		if actual == nil || actual.Code() != test.Expected {
+			t.Errorf(`
+  config: "%v"
+  returned error: "%v"
+  error should be related to: %v`, test.Config, actual, test.Description)
+		}
+	}
+
+	// Positive Tests
+	for _, test := range positiveTests {
+		actual := ip_allow.Parse(test.Config)
+		if actual != nil {
+			t.Errorf(`
+  config: "%v"
+  returned error: "%v"
+  error should be nil
+  description: %v`, test.Config, actual, test.Description)
+		}
+	}
+}