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

Jump to bottom

Fix ACL ICMP rules #1471

Merged
merged 2 commits into from
Sep 18, 2019
Merged

Fix ACL ICMP rules #1471

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50c221d
Fix ACL ICMP rules
VladoLavor Sep 17, 2019
489a442
Added integration test case
VladoLavor Sep 17, 2019
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
2 changes: 1 addition & 1 deletion plugins/vpp/aclplugin/vppcalls/vpp1904/acl_vppcalls.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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