From e768924a62699ec0870211817628bc6866c64769 Mon Sep 17 00:00:00 2001 From: m1093782566 Date: Mon, 18 Dec 2017 15:01:32 +0800 Subject: [PATCH] validate entry in ipset --- pkg/proxy/ipvs/ipset.go | 4 + pkg/util/ipset/ipset.go | 156 ++++++++- pkg/util/ipset/ipset_test.go | 646 +++++++++++++++++++++++++++++++++++ pkg/util/ipset/types.go | 10 - 4 files changed, 792 insertions(+), 24 deletions(-) diff --git a/pkg/proxy/ipvs/ipset.go b/pkg/proxy/ipvs/ipset.go index 4cba7ff4f5..bc639b92f5 100644 --- a/pkg/proxy/ipvs/ipset.go +++ b/pkg/proxy/ipvs/ipset.go @@ -89,6 +89,10 @@ func NewIPSet(handle utilipset.Interface, name string, setType utilipset.Type, i return set } +func (set *IPSet) validateEntry(entry *utilipset.Entry) (bool, error) { + return entry.Validate(&set.IPSet) +} + func (set *IPSet) isEmpty() bool { return len(set.activeEntries.UnsortedList()) == 0 } diff --git a/pkg/util/ipset/ipset.go b/pkg/util/ipset/ipset.go index e2971e7d24..7442135c9e 100644 --- a/pkg/util/ipset/ipset.go +++ b/pkg/util/ipset/ipset.go @@ -19,6 +19,7 @@ package ipset import ( "bytes" "fmt" + "net" "regexp" "strconv" "strings" @@ -86,6 +87,7 @@ type IPSet struct { MaxElem int // PortRange specifies the port range of bitmap:port type ipset. PortRange string + // TODO: add comment message for ipset } // Validate checks if a given ipset is valid or not. @@ -133,10 +135,96 @@ type Entry struct { Net string // IP2 is the entry's second IP. IP2 may not be empty for `hash:ip,port,ip` type ip set. IP2 string - // SetType specifies the type of ip set where the entry exists. + // SetType is the type of ipset where the entry exists. SetType Type } +// Validate checks if a given ipset entry is valid or not. The set parameter is the ipset that entry belongs to. +func (e *Entry) Validate(set *IPSet) (bool, error) { + switch e.SetType { + case HashIPPort: + // set default protocol to tcp if empty + if len(e.Protocol) == 0 { + e.Protocol = ProtocolTCP + } + + if net.ParseIP(e.IP) == nil { + return false, fmt.Errorf("error parsing entry's ip address %v for %s type set", e.IP, HashIPPort) + } + + if valid := validateProtocol(e.Protocol); !valid { + return false, fmt.Errorf("invalid entry's protocol: %s, supported protocols are [%s, %s]", e.Protocol, ProtocolTCP, ProtocolUDP) + } + + if e.Port < 0 { + return false, fmt.Errorf("port number %d should be >=0 ", e.Port) + } + case HashIPPortIP: + // set default protocol to tcp if empty + if len(e.Protocol) == 0 { + e.Protocol = ProtocolTCP + } + + if net.ParseIP(e.IP) == nil { + return false, fmt.Errorf("error parsing entry's ip address %v for %s type set", e.IP, HashIPPortIP) + } + + if valid := validateProtocol(e.Protocol); !valid { + return false, fmt.Errorf("invalid entry's protocol, supported protocols are [%s, %s]", ProtocolTCP, ProtocolUDP) + } + + if e.Port < 0 { + return false, fmt.Errorf("port number %d should be >=0 ", e.Port) + } + + // IP2 can not be empty for `hash:ip,port,ip` type ip set + if net.ParseIP(e.IP2) == nil { + return false, fmt.Errorf("error parsing entry's second ip address %v for %s type set", e.IP2, HashIPPortIP) + } + case HashIPPortNet: + // set default protocol to tcp if empty + if len(e.Protocol) == 0 { + e.Protocol = ProtocolTCP + } + + if net.ParseIP(e.IP) == nil { + return false, fmt.Errorf("error parsing entry's ip address %v for %s type set", e.IP, HashIPPortIP) + } + + if valid := validateProtocol(e.Protocol); !valid { + return false, fmt.Errorf("invalid entry's protocol, supported protocols are [%s, %s]", ProtocolTCP, ProtocolUDP) + } + + if e.Port < 0 { + return false, fmt.Errorf("port number %d should be >=0 ", e.Port) + } + + // Net can not be empty for `hash:ip,port,net` type ip set + if _, ipNet, _ := net.ParseCIDR(e.Net); ipNet == nil { + return false, fmt.Errorf("error parsing entry's ip net %v for %s type set", e.Net, HashIPPortNet) + } + case BitmapPort: + if e.Port < 0 { + return false, fmt.Errorf("port number %d should be >=0 ", e.Port) + } + + // check if port number satisfies its ipset's requirement of port range + if set == nil { + return false, fmt.Errorf("can not reference ip set where the entry exists") + } + begin, end, err := parsePortRange(set.PortRange) + if err != nil { + return false, err + } + if e.Port < begin || e.Port > end { + return false, fmt.Errorf("entry's port number %d is not in the port range %s of its ipset", e.Port, set.PortRange) + } + } + + return true, nil +} + +// String returns the string format for ipset entry. func (e *Entry) String() string { switch e.SetType { case HashIPPort: @@ -172,28 +260,30 @@ func New(exec utilexec.Interface) Interface { // CreateSet creates a new set, it will ignore error when the set already exists if ignoreExistErr=true. func (runner *runner) CreateSet(set *IPSet, ignoreExistErr bool) error { - // Using default values. + // Setting default values if not present if set.HashSize == 0 { set.HashSize = 1024 } if set.MaxElem == 0 { set.MaxElem = 65536 } + // Default protocol is IPv4 if set.HashFamily == "" { set.HashFamily = ProtocolFamilyIPV4 } - if len(set.HashFamily) != 0 && set.HashFamily != ProtocolFamilyIPV4 && set.HashFamily != ProtocolFamilyIPV6 { - return fmt.Errorf("Currently supported protocol families are: %s and %s, %s is not supported", ProtocolFamilyIPV4, ProtocolFamilyIPV6, set.HashFamily) - } // Default ipset type is "hash:ip,port" if len(set.SetType) == 0 { set.SetType = HashIPPort } - // Check if setType is supported - if !IsValidIPSetType(set.SetType) { - return fmt.Errorf("Currently supported ipset types are: %v, %s is not supported", ValidIPSetTypes, set.SetType) + if len(set.PortRange) == 0 { + set.PortRange = DefaultPortRange } + // Validate ipset before creating + valid, err := set.Validate() + if err != nil || !valid { + return fmt.Errorf("ipset is invalid because of %v", err) + } return runner.createSet(set, ignoreExistErr) } @@ -209,12 +299,6 @@ func (runner *runner) createSet(set *IPSet, ignoreExistErr bool) error { ) } if set.SetType == BitmapPort { - if len(set.PortRange) == 0 { - set.PortRange = DefaultPortRange - } - if !validatePortRange(set.PortRange) { - return fmt.Errorf("invalid port range for %s type ip set: %s, expect: a-b", BitmapPort, set.PortRange) - } args = append(args, "range", set.PortRange) } if ignoreExistErr { @@ -396,4 +480,48 @@ func IsNotFoundError(err error) bool { return false } +// checks if given hash family is supported in ipset +func validateProtocol(protocol string) bool { + if protocol == ProtocolTCP || protocol == ProtocolUDP { + return true + } + return false +} + +// parsePortRange parse the begin and end port from a raw string(format: a-b). beginPort <= endPort +// in the return value. +func parsePortRange(portRange string) (beginPort int, endPort int, err error) { + if len(portRange) == 0 { + portRange = DefaultPortRange + } + + strs := strings.Split(portRange, "-") + if len(strs) != 2 { + // port number -1 indicates invalid + return -1, -1, fmt.Errorf("port range should be in the format of `a-b`") + } + for i := range strs { + num, err := strconv.Atoi(strs[i]) + if err != nil { + // port number -1 indicates invalid + return -1, -1, err + } + if num < 0 { + // port number -1 indicates invalid + return -1, -1, fmt.Errorf("port number %d should be >=0", num) + } + if i == 0 { + beginPort = num + continue + } + endPort = num + // switch when first port number > second port number + if beginPort > endPort { + endPort = beginPort + beginPort = num + } + } + return beginPort, endPort, nil +} + var _ = Interface(&runner{}) diff --git a/pkg/util/ipset/ipset_test.go b/pkg/util/ipset/ipset_test.go index 59a5712185..f556982d14 100644 --- a/pkg/util/ipset/ipset_test.go +++ b/pkg/util/ipset/ipset_test.go @@ -312,6 +312,7 @@ func TestDelEntry(t *testing.T) { }, } runner := New(&fexec) + err := runner.DelEntry(testEntry.String(), "FOOBAR") if err != nil { t.Errorf("expected success, got %v", err) @@ -654,3 +655,648 @@ func Test_validateFamily(t *testing.T) { } } } + +func Test_validateProtocol(t *testing.T) { + testCases := []struct { + protocol string + valid bool + desc string + }{ + { // case[0] + protocol: "foo", + valid: false, + }, + { // case[1] + protocol: ProtocolTCP, + valid: true, + }, + { // case[2] + protocol: ProtocolUDP, + valid: true, + }, + { // case[3] + protocol: "ipv4", + valid: false, + }, + { // case[4] + protocol: "ipv6", + valid: false, + }, + { // case[5] + protocol: "TCP", + valid: false, + desc: "should be low case", + }, + { // case[6] + protocol: "UDP", + valid: false, + desc: "should be low case", + }, + { // case[7] + protocol: "", + valid: false, + }, + } + for i := range testCases { + valid := validateProtocol(testCases[i].protocol) + if valid != testCases[i].valid { + t.Errorf("case [%d]: unexpected mismatch, expect valid[%v], got valid[%v], desc: %s", i, testCases[i].valid, valid, testCases[i].desc) + } + } +} + +func TestValidateIPSet(t *testing.T) { + testCases := []struct { + ipset *IPSet + valid bool + desc string + }{ + { // case[0] + ipset: &IPSet{ + Name: "test", + SetType: HashIPPort, + HashFamily: ProtocolFamilyIPV4, + HashSize: 1024, + MaxElem: 1024, + }, + valid: true, + }, + { // case[1] + ipset: &IPSet{ + Name: "SET", + SetType: BitmapPort, + HashFamily: ProtocolFamilyIPV6, + HashSize: 65535, + MaxElem: 2048, + PortRange: DefaultPortRange, + }, + valid: true, + }, + { // case[2] + ipset: &IPSet{ + Name: "foo", + SetType: BitmapPort, + HashFamily: ProtocolFamilyIPV6, + HashSize: 65535, + MaxElem: 2048, + }, + valid: false, + desc: "should specify right port range for bitmap type set", + }, + { // case[3] + ipset: &IPSet{ + Name: "bar", + SetType: BitmapPort, + HashFamily: ProtocolFamilyIPV6, + HashSize: 0, + MaxElem: 2048, + }, + valid: false, + desc: "wrong hash size number", + }, + { // case[4] + ipset: &IPSet{ + Name: "baz", + SetType: BitmapPort, + HashFamily: ProtocolFamilyIPV6, + HashSize: 1024, + MaxElem: -1, + }, + valid: false, + desc: "wrong hash max elem number", + }, + { // case[5] + ipset: &IPSet{ + Name: "baz", + SetType: HashIPPortNet, + HashFamily: "ip", + HashSize: 1024, + MaxElem: 1024, + }, + valid: false, + desc: "wrong protocol", + }, + { // case[6] + ipset: &IPSet{ + Name: "foo-bar", + SetType: "xxx", + HashFamily: ProtocolFamilyIPV4, + HashSize: 1024, + MaxElem: 1024, + }, + valid: false, + desc: "wrong set type", + }, + } + for i := range testCases { + valid, _ := testCases[i].ipset.Validate() + if valid != testCases[i].valid { + t.Errorf("case [%d]: unexpected mismatch, expect valid[%v], got valid[%v], desc: %s", i, testCases[i].valid, valid, testCases[i].desc) + } + } +} + +func Test_parsePortRange(t *testing.T) { + testCases := []struct { + portRange string + expectErr bool + beginPort int + endPort int + desc string + }{ + { // case[0] + portRange: "1-100", + expectErr: false, + beginPort: 1, + endPort: 100, + }, + { // case[1] + portRange: "0-0", + expectErr: false, + beginPort: 0, + endPort: 0, + }, + { // case[2] + portRange: "100-10", + expectErr: false, + beginPort: 10, + endPort: 100, + }, + { // case[3] + portRange: "1024", + expectErr: true, + desc: "single port number is not allowed", + }, + { // case[4] + portRange: DefaultPortRange, + expectErr: false, + beginPort: 0, + endPort: 65535, + }, + { // case[5] + portRange: "1-", + expectErr: true, + desc: "should specify end port", + }, + { // case[6] + portRange: "-100", + expectErr: true, + desc: "should specify begin port", + }, + { // case[7] + portRange: "1:100", + expectErr: true, + desc: "delimiter should be -", + }, + { // case[8] + portRange: "1~100", + expectErr: true, + desc: "delimiter should be -", + }, + { // case[9] + portRange: "1,100", + expectErr: true, + desc: "delimiter should be -", + }, + { // case[10] + portRange: "100-100", + expectErr: false, + desc: "begin port number can be equal to end port number", + beginPort: 100, + endPort: 100, + }, + { // case[11] + portRange: "", + expectErr: false, + desc: "empty string indicates default port range", + beginPort: 0, + endPort: 65535, + }, + { // case[12] + portRange: "-1-12", + expectErr: true, + desc: "port number can not be negative value", + }, + { // case[13] + portRange: "-1--8", + expectErr: true, + desc: "port number can not be negative value", + }, + } + for i := range testCases { + begin, end, err := parsePortRange(testCases[i].portRange) + if err != nil { + if !testCases[i].expectErr { + t.Errorf("case [%d]: unexpected err: %v, desc: %s", i, err, testCases[i].desc) + } + continue + } + if begin != testCases[i].beginPort || end != testCases[i].endPort { + t.Errorf("case [%d]: unexpected mismatch [beginPort, endPort] pair, expect [%d, %d], got [%d, %d], desc: %s", i, testCases[i].beginPort, testCases[i].endPort, begin, end, testCases[i].desc) + } + } +} + +// This is a coarse test, but it offers some modicum of confidence as the code is evolved. +func TestValidateEntry(t *testing.T) { + testCases := []struct { + entry *Entry + set *IPSet + valid bool + desc string + }{ + { // case[0] + entry: &Entry{ + SetType: BitmapPort, + }, + set: &IPSet{ + PortRange: DefaultPortRange, + }, + valid: true, + desc: "port number can be empty, default is 0. And port number is in the range of its ipset's port range", + }, + { // case[1] + entry: &Entry{ + SetType: BitmapPort, + Port: 0, + }, + set: &IPSet{ + PortRange: DefaultPortRange, + }, + valid: true, + desc: "port number can be 0. And port number is in the range of its ipset's port range", + }, + { // case[2] + entry: &Entry{ + SetType: BitmapPort, + Port: -1, + }, + valid: false, + desc: "port number can not be negative value", + }, + { // case[3] + entry: &Entry{ + SetType: BitmapPort, + Port: 1080, + }, + set: &IPSet{ + Name: "baz", + PortRange: DefaultPortRange, + }, + desc: "port number is in the range of its ipset's port range", + valid: true, + }, + { // case[4] + entry: &Entry{ + SetType: BitmapPort, + Port: 1080, + }, + set: &IPSet{ + Name: "foo", + PortRange: "0-1079", + }, + desc: "port number is NOT in the range of its ipset's port range", + valid: false, + }, + { // case[5] + entry: &Entry{ + SetType: HashIPPort, + IP: "1.2.3.4", + Protocol: ProtocolTCP, + Port: 8080, + }, + set: &IPSet{ + Name: "bar", + }, + valid: true, + }, + { // case[6] + entry: &Entry{ + SetType: HashIPPort, + IP: "1.2.3.4", + Protocol: ProtocolUDP, + Port: 0, + }, + set: &IPSet{ + Name: "bar", + }, + valid: true, + }, + { // case[7] + entry: &Entry{ + SetType: HashIPPort, + IP: "FE80:0000:0000:0000:0202:B3FF:FE1E:8329", + Protocol: ProtocolTCP, + Port: 1111, + }, + set: &IPSet{ + Name: "ipv6", + }, + valid: true, + }, + { // case[8] + entry: &Entry{ + SetType: HashIPPort, + IP: "", + Protocol: ProtocolTCP, + Port: 1234, + }, + set: &IPSet{ + Name: "empty-ip", + }, + valid: false, + }, + { // case[9] + entry: &Entry{ + SetType: HashIPPort, + IP: "1-2-3-4", + Protocol: ProtocolTCP, + Port: 8900, + }, + set: &IPSet{ + Name: "bad-ip", + }, + valid: false, + }, + { // case[10] + entry: &Entry{ + SetType: HashIPPort, + IP: "10.20.30.40", + Protocol: "", + Port: 8090, + }, + set: &IPSet{ + Name: "empty-protocol", + }, + valid: true, + }, + { // case[11] + entry: &Entry{ + SetType: HashIPPort, + IP: "10.20.30.40", + Protocol: "ICMP", + Port: 8090, + }, + set: &IPSet{ + Name: "unsupported-protocol", + }, + valid: false, + }, + { // case[11] + entry: &Entry{ + SetType: HashIPPort, + IP: "10.20.30.40", + Protocol: "ICMP", + Port: -1, + }, + set: &IPSet{ + // TODO: set name string with white space? + Name: "negative-port-number", + }, + valid: false, + }, + { // case[12] + entry: &Entry{ + SetType: HashIPPortIP, + IP: "10.20.30.40", + Protocol: ProtocolUDP, + Port: 53, + IP2: "10.20.30.40", + }, + set: &IPSet{ + Name: "LOOP-BACK", + }, + valid: true, + }, + { // case[13] + entry: &Entry{ + SetType: HashIPPortIP, + IP: "10.20.30.40", + Protocol: ProtocolUDP, + Port: 53, + IP2: "", + }, + set: &IPSet{ + Name: "empty IP2", + }, + valid: false, + }, + { // case[14] + entry: &Entry{ + SetType: HashIPPortIP, + IP: "10.20.30.40", + Protocol: ProtocolUDP, + Port: 53, + IP2: "foo", + }, + set: &IPSet{ + Name: "invalid IP2", + }, + valid: false, + }, + { // case[15] + entry: &Entry{ + SetType: HashIPPortIP, + IP: "10.20.30.40", + Protocol: ProtocolTCP, + Port: 0, + IP2: "1.2.3.4", + }, + set: &IPSet{ + Name: "zero port", + }, + valid: true, + }, + { // case[16] + entry: &Entry{ + SetType: HashIPPortIP, + IP: "10::40", + Protocol: ProtocolTCP, + Port: 10000, + IP2: "1::4", + }, + set: &IPSet{ + Name: "IPV6", + // TODO: check set's hash family + }, + valid: true, + }, + { // case[17] + entry: &Entry{ + SetType: HashIPPortIP, + IP: "", + Protocol: ProtocolTCP, + Port: 1234, + IP2: "1.2.3.4", + }, + set: &IPSet{ + Name: "empty-ip", + }, + valid: false, + }, + { // case[18] + entry: &Entry{ + SetType: HashIPPortIP, + IP: "1-2-3-4", + Protocol: ProtocolTCP, + Port: 8900, + IP2: "10.20.30.41", + }, + set: &IPSet{ + Name: "bad-ip", + }, + valid: false, + }, + { // case[19] + entry: &Entry{ + SetType: HashIPPortIP, + IP: "10.20.30.40", + Protocol: "SCTP ", + Port: 8090, + IP2: "10.20.30.41", + }, + set: &IPSet{ + Name: "unsupported-protocol", + }, + valid: false, + }, + { // case[20] + entry: &Entry{ + SetType: HashIPPortIP, + IP: "10.20.30.40", + Protocol: "ICMP", + Port: -1, + IP2: "100.200.30.41", + }, + set: &IPSet{ + Name: "negative-port-number", + }, + valid: false, + }, + { // case[21] + entry: &Entry{ + SetType: HashIPPortNet, + IP: "10.20.30.40", + Protocol: ProtocolTCP, + Port: 53, + // TODO: CIDR /32 may not be valid + Net: "10.20.30.0/24", + }, + set: &IPSet{ + Name: "abc", + }, + valid: true, + }, + { // case[22] + entry: &Entry{ + SetType: HashIPPortNet, + IP: "11.21.31.41", + Protocol: ProtocolUDP, + Port: 1122, + Net: "", + }, + set: &IPSet{ + Name: "empty Net", + }, + valid: false, + }, + { // case[23] + entry: &Entry{ + SetType: HashIPPortNet, + IP: "10.20.30.40", + Protocol: ProtocolUDP, + Port: 8080, + Net: "x-y-z-w", + }, + set: &IPSet{ + Name: "invalid Net", + }, + valid: false, + }, + { // case[24] + entry: &Entry{ + SetType: HashIPPortNet, + IP: "10.20.30.40", + Protocol: ProtocolTCP, + Port: 0, + Net: "10.1.0.0/16", + }, + set: &IPSet{ + Name: "zero port", + }, + valid: true, + }, + { // case[25] + entry: &Entry{ + SetType: HashIPPortNet, + IP: "10::40", + Protocol: ProtocolTCP, + Port: 80, + Net: "2001:db8::/32", + }, + set: &IPSet{ + Name: "IPV6", + // TODO: check set's hash family + }, + valid: true, + }, + { // case[26] + entry: &Entry{ + SetType: HashIPPortNet, + IP: "", + Protocol: ProtocolTCP, + Port: 1234, + Net: "1.2.3.4/22", + }, + set: &IPSet{ + Name: "empty-ip", + }, + valid: false, + }, + { // case[27] + entry: &Entry{ + SetType: HashIPPortNet, + IP: "1-2-3-4", + Protocol: ProtocolTCP, + Port: 8900, + Net: "10.20.30.41/31", + }, + set: &IPSet{ + Name: "bad-ip", + }, + valid: false, + }, + { // case[28] + entry: &Entry{ + SetType: HashIPPortIP, + IP: "10.20.30.40", + Protocol: "FOO", + Port: 8090, + IP2: "10.20.30.0/10", + }, + set: &IPSet{ + Name: "unsupported-protocol", + }, + valid: false, + }, + { // case[29] + entry: &Entry{ + SetType: HashIPPortIP, + IP: "10.20.30.40", + Protocol: ProtocolUDP, + Port: -1, + IP2: "100.200.30.0/12", + }, + set: &IPSet{ + Name: "negative-port-number", + }, + valid: false, + }, + } + for i := range testCases { + valid, _ := testCases[i].entry.Validate(testCases[i].set) + if valid != testCases[i].valid { + t.Errorf("case [%d]: unexpected mismatch, expect valid[%v], got valid[%v], desc: %s", i, testCases[i].valid, valid, testCases[i].entry) + } + } +} diff --git a/pkg/util/ipset/types.go b/pkg/util/ipset/types.go index d2406c0087..cacfd6f8d8 100644 --- a/pkg/util/ipset/types.go +++ b/pkg/util/ipset/types.go @@ -58,13 +58,3 @@ var ValidIPSetTypes = []Type{ BitmapPort, HashIPPortNet, } - -// IsValidIPSetType checks if the given ipset type is valid. -func IsValidIPSetType(set Type) bool { - for _, valid := range ValidIPSetTypes { - if set == valid { - return true - } - } - return false -}