Skip to content

Commit

Permalink
Fix ACL ICMP rules (#1471)
Browse files Browse the repository at this point in the history 
* Fix ACL ICMP rules

Signed-off-by: Vladimir Lavor <vlavor@cisco.com>

* Added integration test case

Signed-off-by: Vladimir Lavor <vlavor@cisco.com>
  • Loading branch information
@VladoLavor @ondrej-fabry
VladoLavor authored and ondrej-fabry committed Sep 18, 2019
1 parent 643255e commit 70a8145
Show file tree
Hide file tree
Showing 8 changed files with 92 additions and 38 deletions.
2 changes: 1 addition & 1 deletion plugins/vpp/aclplugin/vppcalls/vpp1904/acl_vppcalls.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ func icmpACL(icmpRule *acl.ACL_Rule_IpRule_Icmp, aclRule *aclapi.ACLRule) *aclap
aclRule.SrcportOrIcmptypeLast = uint16(icmpRule.IcmpTypeRange.Last)
// ICMPv6 code range
aclRule.DstportOrIcmpcodeFirst = uint16(icmpRule.IcmpCodeRange.First)
aclRule.DstportOrIcmpcodeLast = uint16(icmpRule.IcmpCodeRange.First)
aclRule.DstportOrIcmpcodeLast = uint16(icmpRule.IcmpCodeRange.Last)
} else {
aclRule.Proto = vppcalls.ICMPv4Proto // IANA ICMPv4
aclRule.IsIPv6 = 0
Expand Down
12 changes: 9 additions & 3 deletions plugins/vpp/aclplugin/vppcalls/vpp1904/dump_vppcalls.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -628,9 +628,15 @@ func (h *ACLVppHandler) getUDPMatchRule(r acl_api.ACLRule) *acl.ACL_Rule_IpRule_
// format into the ACL Plugin's NB format
func (h *ACLVppHandler) getIcmpMatchRule(r acl_api.ACLRule) *acl.ACL_Rule_IpRule_Icmp {
icmp := &acl.ACL_Rule_IpRule_Icmp{
Icmpv6: r.IsIPv6 > 0,
IcmpCodeRange: &acl.ACL_Rule_IpRule_Icmp_Range{},
IcmpTypeRange: &acl.ACL_Rule_IpRule_Icmp_Range{},
Icmpv6: r.IsIPv6 > 0,
IcmpCodeRange: &acl.ACL_Rule_IpRule_Icmp_Range{
First: uint32(r.DstportOrIcmpcodeFirst),
Last: uint32(r.DstportOrIcmpcodeLast),
},
IcmpTypeRange: &acl.ACL_Rule_IpRule_Icmp_Range{
First: uint32(r.SrcportOrIcmptypeFirst),
Last: uint32(r.SrcportOrIcmptypeLast),
},
}
return icmp
}
Expand Down
2 changes: 1 addition & 1 deletion plugins/vpp/aclplugin/vppcalls/vpp1908/acl_vppcalls.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ func icmpACL(icmpRule *acl.ACL_Rule_IpRule_Icmp, aclRule *aclapi.ACLRule) *aclap
aclRule.SrcportOrIcmptypeLast = uint16(icmpRule.IcmpTypeRange.Last)
// ICMPv6 code range
aclRule.DstportOrIcmpcodeFirst = uint16(icmpRule.IcmpCodeRange.First)
aclRule.DstportOrIcmpcodeLast = uint16(icmpRule.IcmpCodeRange.First)
aclRule.DstportOrIcmpcodeLast = uint16(icmpRule.IcmpCodeRange.Last)
} else {
aclRule.Proto = vppcalls.ICMPv4Proto // IANA ICMPv4
aclRule.IsIPv6 = 0
Expand Down
10 changes: 8 additions & 2 deletions plugins/vpp/aclplugin/vppcalls/vpp1908/dump_vppcalls.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -629,8 +629,14 @@ func (h *ACLVppHandler) getUDPMatchRule(r acl_api.ACLRule) *acl.ACL_Rule_IpRule_
func (h *ACLVppHandler) getIcmpMatchRule(r acl_api.ACLRule) *acl.ACL_Rule_IpRule_Icmp {
icmp := &acl.ACL_Rule_IpRule_Icmp{
Icmpv6: r.IsIPv6 > 0,
IcmpCodeRange: &acl.ACL_Rule_IpRule_Icmp_Range{},
IcmpTypeRange: &acl.ACL_Rule_IpRule_Icmp_Range{},
IcmpCodeRange: &acl.ACL_Rule_IpRule_Icmp_Range{
First: uint32(r.DstportOrIcmpcodeFirst),
Last: uint32(r.DstportOrIcmpcodeLast),
},
IcmpTypeRange: &acl.ACL_Rule_IpRule_Icmp_Range{
First: uint32(r.SrcportOrIcmptypeFirst),
Last: uint32(r.SrcportOrIcmptypeLast),
},
}
return icmp
}
Expand Down
2 changes: 1 addition & 1 deletion plugins/vpp/aclplugin/vppcalls/vpp2001/acl_vppcalls.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ func icmpACL(icmpRule *acl.ACL_Rule_IpRule_Icmp, aclRule *vpp_acl.ACLRule) *vpp_
aclRule.SrcportOrIcmptypeLast = uint16(icmpRule.IcmpTypeRange.Last)
// ICMPv6 code range
aclRule.DstportOrIcmpcodeFirst = uint16(icmpRule.IcmpCodeRange.First)
aclRule.DstportOrIcmpcodeLast = uint16(icmpRule.IcmpCodeRange.First)
aclRule.DstportOrIcmpcodeLast = uint16(icmpRule.IcmpCodeRange.Last)
} else {
aclRule.Proto = vppcalls.ICMPv4Proto // IANA ICMPv4
aclRule.IsIPv6 = 0
Expand Down
16 changes: 8 additions & 8 deletions plugins/vpp/aclplugin/vppcalls/vpp2001/acl_vppcalls_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,12 @@ var aclIPrules = []*acl.ACL_Rule{
Icmp: &acl.ACL_Rule_IpRule_Icmp{
Icmpv6: false,
IcmpCodeRange: &acl.ACL_Rule_IpRule_Icmp_Range{
First: 150,
Last: 250,
First: 1,
Last: 2,
},
IcmpTypeRange: &acl.ACL_Rule_IpRule_Icmp_Range{
First: 1150,
Last: 1250,
First: 3,
Last: 4,
},
},
},
Expand All @@ -156,12 +156,12 @@ var aclIPrules = []*acl.ACL_Rule{
Icmp: &acl.ACL_Rule_IpRule_Icmp{
Icmpv6: true,
IcmpCodeRange: &acl.ACL_Rule_IpRule_Icmp_Range{
First: 150,
Last: 250,
First: 10,
Last: 20,
},
IcmpTypeRange: &acl.ACL_Rule_IpRule_Icmp_Range{
First: 1150,
Last: 1250,
First: 30,
Last: 40,
},
},
},
Expand Down
10 changes: 8 additions & 2 deletions plugins/vpp/aclplugin/vppcalls/vpp2001/dump_vppcalls.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -629,8 +629,14 @@ func (h *ACLVppHandler) getUDPMatchRule(r vpp_acl.ACLRule) *acl.ACL_Rule_IpRule_
func (h *ACLVppHandler) getIcmpMatchRule(r vpp_acl.ACLRule) *acl.ACL_Rule_IpRule_Icmp {
icmp := &acl.ACL_Rule_IpRule_Icmp{
Icmpv6: r.IsIPv6 > 0,
IcmpCodeRange: &acl.ACL_Rule_IpRule_Icmp_Range{},
IcmpTypeRange: &acl.ACL_Rule_IpRule_Icmp_Range{},
IcmpCodeRange: &acl.ACL_Rule_IpRule_Icmp_Range{
First: uint32(r.DstportOrIcmpcodeFirst),
Last: uint32(r.DstportOrIcmpcodeLast),
},
IcmpTypeRange: &acl.ACL_Rule_IpRule_Icmp_Range{
First: uint32(r.SrcportOrIcmptypeFirst),
Last: uint32(r.SrcportOrIcmptypeLast),
},
}
return icmp
}
Expand Down
76 changes: 56 additions & 20 deletions tests/integration/vpp/040_acl_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,9 @@ func TestCRUDIPAcl(t *testing.T) {
//RuleName: "permitIPv6",
newACLIPRule(true, "dead::1/64", "dead::2/64"),
//RuleName: "denyICMP",
newACLIPRuleIcmp(false, false, 150, 250, 1150, 1250),
newACLIPRuleIcmp(false, false, 10, 20, 30, 40),
//RuleName: "denyICMPv6",
newACLIPRuleIcmp(false, true, 150, 250, 1150, 1250),
newACLIPRuleIcmp(false, true, 10, 20, 30, 40),
//RuleName: "permitTCP",
newACLIPRuleTCP(true, 10, 20, 150, 250, 1150, 1250),
//RuleName: "denyUDP",
Expand All @@ -189,21 +189,30 @@ func TestCRUDIPAcl(t *testing.T) {
t.Log("amount of acls dumped: 1")

var rules []*acl.ACL_Rule
var isPresent bool
var isForInterface bool
var isIPRulePresent, isICMPRulePresent, isICMP6RulePresent, isForInterface bool
for _, item := range acls {
rules = item.ACL.Rules
if (item.Meta.Index == aclIdx) && (aclname == item.Meta.Tag) {
t.Logf("found ACL \"%v\"", item.Meta.Tag)
for _, rule := range rules {
//t.Logf("%+v", rule)
t.Logf("%+v", rule)
//here maybe all rules should be checked
if (rule.IpRule.GetIp().SourceNetwork == "192.168.1.1/32") &&
(rule.IpRule.GetIp().DestinationNetwork == "10.20.0.0/24") {
isPresent = true
break
isIPRulePresent = true
}
if (rule.IpRule.GetIcmp().GetIcmpCodeRange().GetFirst() == 10) &&
(rule.IpRule.GetIcmp().GetIcmpCodeRange().GetLast() == 20) &&
(rule.IpRule.GetIcmp().GetIcmpTypeRange().GetFirst() == 30) &&
(rule.IpRule.GetIcmp().GetIcmpTypeRange().GetLast() == 40) {
if rule.IpRule.GetIcmp().GetIcmpv6() {
isICMP6RulePresent = true
} else {
isICMPRulePresent = true
}
}
}

// check assignation to interface
for _, intf := range item.ACL.Interfaces.Ingress {
if intf == ifName {
Expand All @@ -213,7 +222,9 @@ func TestCRUDIPAcl(t *testing.T) {
}
}
}
Expect(isPresent).To(BeTrue(), "Configured IP should be present")
Expect(isIPRulePresent).To(BeTrue(), "Configured IP rule should be present")
Expect(isICMPRulePresent).To(BeTrue(), "Configured ICMP rule should be present")
Expect(isICMP6RulePresent).To(BeTrue(), "Configured ICMPv6 rule should be present")
Expect(isForInterface).To(BeTrue(), "acl should be assigned to interface")

indexes := []uint32{ifIdx, ifIdx2}
Expand Down Expand Up @@ -258,9 +269,9 @@ func TestCRUDIPAcl(t *testing.T) {
//RuleName: "permitIPv6",
newACLIPRule(true, "dead::1/64", "dead::2/64"),
//RuleName: "denyICMP",
newACLIPRuleIcmp(false, false, 150, 250, 1150, 1250),
newACLIPRuleIcmp(false, false, 11, 21, 31, 41),
//RuleName: "denyICMPv6",
newACLIPRuleIcmp(false, true, 150, 250, 1150, 1250),
newACLIPRuleIcmp(false, true, 11, 21, 31, 41),
//RuleName: "permitTCP",
newACLIPRuleTCP(true, 10, 20, 150, 250, 1150, 1250),
//RuleName: "denyUDP",
Expand All @@ -280,7 +291,9 @@ func TestCRUDIPAcl(t *testing.T) {
Expect(acls).Should(HaveLen(2))
t.Log("amount of acls dumped: 2")

isPresent = false
isIPRulePresent = false
isICMPRulePresent = false
isICMP6RulePresent = false
isForInterface = false
for _, item := range acls {
rules = item.ACL.Rules
Expand All @@ -290,8 +303,17 @@ func TestCRUDIPAcl(t *testing.T) {
//t.Logf("%+v", rule)
if (rule.IpRule.GetIp().SourceNetwork == "192.168.1.1/32") &&
(rule.IpRule.GetIp().DestinationNetwork == "10.20.0.0/24") {
isPresent = true
break
isIPRulePresent = true
}
if (rule.IpRule.GetIcmp().GetIcmpCodeRange().GetFirst() == 11) &&
(rule.IpRule.GetIcmp().GetIcmpCodeRange().GetLast() == 21) &&
(rule.IpRule.GetIcmp().GetIcmpTypeRange().GetFirst() == 31) &&
(rule.IpRule.GetIcmp().GetIcmpTypeRange().GetLast() == 41) {
if rule.IpRule.GetIcmp().GetIcmpv6() {
isICMP6RulePresent = true
} else {
isICMPRulePresent = true
}
}
}
// check assignation to interface
Expand All @@ -303,7 +325,9 @@ func TestCRUDIPAcl(t *testing.T) {
}
}
}
Expect(isPresent).To(BeTrue(), "Configured IP should be present")
Expect(isIPRulePresent).To(BeTrue(), "Configured IP should be present")
Expect(isICMPRulePresent).To(BeTrue(), "Configured ICMP rule should be present")
Expect(isICMP6RulePresent).To(BeTrue(), "Configured ICMPv6 rule should be present")
Expect(isForInterface).To(BeTrue(), "acl should be assigned to interface")

//negative tests
Expand Down Expand Up @@ -347,6 +371,7 @@ func TestCRUDIPAcl(t *testing.T) {
rule2modify := []*acl.ACL_Rule{
newACLIPRule(true, "10.20.30.1/32", "10.20.0.0/24"),
newACLIPRule(true, "dead:dead::3/64", "dead:dead::4/64"),
newACLIPRuleIcmp(true, false, 15, 25, 35, 45),
}

const aclname4 = "test_modify0"
Expand All @@ -359,7 +384,8 @@ func TestCRUDIPAcl(t *testing.T) {
Expect(acls).Should(HaveLen(1))
t.Log("amount of acls dumped: 1")

isPresent = false
isIPRulePresent = false
isICMPRulePresent = false
isForInterface = false
var modifiedacl aclplugin_vppcalls.ACLDetails
for _, item := range acls {
Expand All @@ -376,8 +402,17 @@ func TestCRUDIPAcl(t *testing.T) {
//here maybe should be checked all rules
if (rule.IpRule.GetIp().SourceNetwork == "10.20.30.1/32") &&
(rule.IpRule.GetIp().DestinationNetwork == "10.20.0.0/24") {
isPresent = true
break
isIPRulePresent = true
}
if (rule.IpRule.GetIcmp().GetIcmpCodeRange().GetFirst() == 15) &&
(rule.IpRule.GetIcmp().GetIcmpCodeRange().GetLast() == 25) &&
(rule.IpRule.GetIcmp().GetIcmpTypeRange().GetFirst() == 35) &&
(rule.IpRule.GetIcmp().GetIcmpTypeRange().GetLast() == 45) {
if rule.IpRule.GetIcmp().GetIcmpv6() {
isICMP6RulePresent = true
} else {
isICMPRulePresent = true
}
}
}
// check assignation to interface
Expand All @@ -389,7 +424,8 @@ func TestCRUDIPAcl(t *testing.T) {
}
}
}
Expect(isPresent).To(BeTrue(), "Configured IP should be present")
Expect(isIPRulePresent).To(BeTrue(), "Configured IP should be present")
Expect(isICMPRulePresent).To(BeTrue(), "Configured ICMP rule should be present")
Expect(isForInterface).To(BeTrue(), "acl should be assigned to interface")

// negative test
Expand All @@ -407,7 +443,7 @@ func TestCRUDIPAcl(t *testing.T) {
Expect(acls).Should(HaveLen(1))
t.Log("amount of acls dumped: 1")

isPresent = false
isIPRulePresent = false
isForInterface = false
for _, item := range acls {
if item.Meta.Index == aclIdx && (aclname4 == item.Meta.Tag) {
Expand All @@ -428,7 +464,7 @@ func TestCRUDIPAcl(t *testing.T) {
Expect(acls).Should(HaveLen(1))
t.Log("amount of acls dumped: 1")

isPresent = false
isIPRulePresent = false
isForInterface = false
for _, item := range acls {
if item.Meta.Index == aclIdx { //&& (aclname2 == item.Meta.Tag) {
Expand Down

0 comments on commit 70a8145

Please sign in to comment.