diff --git a/lib/backend/model/rule.go b/lib/backend/model/rule.go index cf5a1dcd8..b7e73d896 100644 --- a/lib/backend/model/rule.go +++ b/lib/backend/model/rule.go @@ -31,10 +31,13 @@ type Rule struct { Protocol *numorstring.Protocol `json:"protocol,omitempty" validate:"omitempty"` NotProtocol *numorstring.Protocol `json:"!protocol,omitempty" validate:"omitempty"` - ICMPType *int `json:"icmp_type,omitempty" validate:"omitempty,gte=1,lte=255"` - ICMPCode *int `json:"icmp_code,omitempty" validate:"omitempty,gte=1,lte=255"` - NotICMPType *int `json:"!icmp_type,omitempty" validate:"omitempty,gte=1,lte=255"` - NotICMPCode *int `json:"!icmp_code,omitempty" validate:"omitempty,gte=1,lte=255"` + // ICMP validation notes: 0 is a valid (common) ICMP type and code. Type = 255 is not assigned + // to any protocol and the Linux kernel doesn't support matching on it so we validate against + // it. + ICMPType *int `json:"icmp_type,omitempty" validate:"omitempty,gte=0,lt=255"` + ICMPCode *int `json:"icmp_code,omitempty" validate:"omitempty,gte=0,lte=255"` + NotICMPType *int `json:"!icmp_type,omitempty" validate:"omitempty,gte=0,lt=255"` + NotICMPCode *int `json:"!icmp_code,omitempty" validate:"omitempty,gte=0,lte=255"` SrcTag string `json:"src_tag,omitempty" validate:"omitempty,tag"` SrcNet *net.IPNet `json:"src_net,omitempty" validate:"omitempty"` diff --git a/lib/backend/model/rule_test.go b/lib/backend/model/rule_test.go index a9fd96759..81145b2d4 100644 --- a/lib/backend/model/rule_test.go +++ b/lib/backend/model/rule_test.go @@ -1,4 +1,4 @@ -// Copyright (c) 2016 Tigera, Inc. All rights reserved. +// Copyright (c) 2016-2017 Tigera, Inc. All rights reserved. // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -35,6 +35,8 @@ var icmpProto = numorstring.ProtocolFromString("icmp") var intProto = numorstring.ProtocolFromInt(123) var icmpType = 10 var icmpCode = 6 +var icmpTypeZero = 0 +var icmpCodeZero = 0 var portRange, _ = numorstring.PortFromRange(10, 20) var ports = []numorstring.Port{ numorstring.SinglePort(1234), @@ -59,6 +61,10 @@ var ruleStringTests = []ruleTest{ "deny icmp type 10"}, {model.Rule{Protocol: &icmpProto, ICMPType: &icmpType, ICMPCode: &icmpCode}, "allow icmp type 10 code 6"}, + {model.Rule{Action: "deny", Protocol: &icmpProto, ICMPType: &icmpTypeZero}, + "deny icmp type 0"}, + {model.Rule{Protocol: &icmpProto, ICMPType: &icmpTypeZero, ICMPCode: &icmpCodeZero}, + "allow icmp type 0 code 0"}, // And negations of packet-wide matches. {model.Rule{Action: "allow", NotProtocol: &tcpProto}, "allow !tcp"}, {model.Rule{Action: "deny", Protocol: &icmpProto, NotICMPType: &icmpType}, diff --git a/lib/validator/validator_test.go b/lib/validator/validator_test.go index ba905c005..fd3f7d2f2 100644 --- a/lib/validator/validator_test.go +++ b/lib/validator/validator_test.go @@ -79,6 +79,20 @@ func init() { // Empty rule is valid, it means "allow all". Entry("empty rule (m)", model.Rule{}, true), + // (Backend model) ICMP. + Entry("should accept ICMP 0/any (m)", model.Rule{ICMPType: &V0}, true), + Entry("should accept ICMP 0/0 (m)", model.Rule{ICMPType: &V0, ICMPCode: &V0}, true), + Entry("should accept ICMP 128/0 (m)", model.Rule{ICMPType: &V128, ICMPCode: &V0}, true), + Entry("should accept ICMP 254/255 (m)", model.Rule{ICMPType: &V254, ICMPCode: &V255}, true), + Entry("should reject ICMP 255/255 (m)", model.Rule{ICMPType: &V255, ICMPCode: &V255}, false), + Entry("should reject ICMP 128/256 (m)", model.Rule{ICMPType: &V128, ICMPCode: &V256}, false), + Entry("should accept not ICMP 0/any (m)", model.Rule{NotICMPType: &V0}, true), + Entry("should accept not ICMP 0/0 (m)", model.Rule{NotICMPType: &V0, NotICMPCode: &V0}, true), + Entry("should accept not ICMP 128/0 (m)", model.Rule{NotICMPType: &V128, NotICMPCode: &V0}, true), + Entry("should accept not ICMP 254/255 (m)", model.Rule{NotICMPType: &V254, NotICMPCode: &V255}, true), + Entry("should reject not ICMP 255/255 (m)", model.Rule{NotICMPType: &V255, NotICMPCode: &V255}, false), + Entry("should reject not ICMP 128/256 (m)", model.Rule{NotICMPType: &V128, NotICMPCode: &V256}, false), + // (Backend model) Actions. Entry("should accept allow action (m)", model.Rule{Action: "allow"}, true), Entry("should accept deny action (m)", model.Rule{Action: "deny"}, true),