Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v1.x: Fix validation for ICMP type and code in backend rules. #654

Merged
merged 1 commit into from
Nov 7, 2017
Merged

v1.x: Fix validation for ICMP type and code in backend rules. #654

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions lib/backend/model/rule.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
8 changes: 7 additions & 1 deletion lib/backend/model/rule_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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),
Expand All @@ -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},
Expand Down
14 changes: 14 additions & 0 deletions lib/validator/validator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down