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.

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

drop both IPv4 and IPv6 traffic in networkpolicy drop acl #3940

Merged
merged 2 commits into from
Apr 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
64 changes: 32 additions & 32 deletions mocks/pkg/ovs/interface.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 16 additions & 17 deletions pkg/controller/network_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,17 +298,16 @@ func (c *Controller) handleUpdateNp(key string) error {

ingressACLOps = append(ingressACLOps, ops...)
}

if err = c.OVNNbClient.Transact("add-ingress-acls", ingressACLOps); err != nil {
return fmt.Errorf("add ingress acls to %s: %v", pgName, err)
}

if err = c.OVNNbClient.SetACLLog(pgName, protocol, logEnable, true); err != nil {
// just log and do not return err here
klog.Errorf("failed to set ingress acl log for np %s, %v", key, err)
}
}
}
if err := c.OVNNbClient.Transact("add-ingress-acls", ingressACLOps); err != nil {
return fmt.Errorf("add ingress acls to %s: %v", pgName, err)
}

if err := c.OVNNbClient.SetACLLog(pgName, logEnable, true); err != nil {
// just log and do not return err here
klog.Errorf("failed to set ingress acl log for np %s, %v", key, err)
}

ass, err := c.OVNNbClient.ListAddressSets(map[string]string{
networkPolicyKey: fmt.Sprintf("%s/%s/%s", np.Namespace, npName, "ingress"),
Expand Down Expand Up @@ -427,16 +426,16 @@ func (c *Controller) handleUpdateNp(key string) error {
egressACLOps = append(egressACLOps, ops...)
}

if err = c.OVNNbClient.Transact("add-egress-acls", egressACLOps); err != nil {
return fmt.Errorf("add egress acls to %s: %v", pgName, err)
}

if err = c.OVNNbClient.SetACLLog(pgName, protocol, logEnable, false); err != nil {
// just log and do not return err here
klog.Errorf("failed to set egress acl log for np %s, %v", key, err)
}
}
}
if err := c.OVNNbClient.Transact("add-egress-acls", egressACLOps); err != nil {
return fmt.Errorf("add egress acls to %s: %v", pgName, err)
}

if err := c.OVNNbClient.SetACLLog(pgName, logEnable, false); err != nil {
// just log and do not return err here
klog.Errorf("failed to set egress acl log for np %s, %v", key, err)
}

ass, err := c.OVNNbClient.ListAddressSets(map[string]string{
networkPolicyKey: fmt.Sprintf("%s/%s/%s", np.Namespace, npName, "egress"),
Expand Down
2 changes: 1 addition & 1 deletion pkg/ovs/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ type ACL interface {
CreateSgBaseACL(sgName, direction string) error
UpdateSgACL(sg *kubeovnv1.SecurityGroup, direction string) error
UpdateLogicalSwitchACL(lsName, cidrBlock string, subnetAcls []kubeovnv1.ACL, allowEWTraffic bool) error
SetACLLog(pgName, protocol string, logEnable, isIngress bool) error
SetACLLog(pgName string, logEnable, isIngress bool) error
SetLogicalSwitchPrivate(lsName, cidrBlock, nodeSwitchCIDR string, allowSubnets []string) error
SGLostACL(sg *kubeovnv1.SecurityGroup) (bool, error)
DeleteAcls(parentName, parentType, direction string, externalIDs map[string]string) error
Expand Down
30 changes: 9 additions & 21 deletions pkg/ovs/ovn-nb-acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,10 @@ func (c *OVNNbClient) UpdateIngressACLOps(pgName, asIngressName, asExceptName, p

if strings.HasSuffix(asIngressName, ".0") || strings.HasSuffix(asIngressName, ".all") {
// create the default drop rule for only once
ipSuffix := "ip4"
if protocol == kubeovnv1.ProtocolIPv6 {
ipSuffix = "ip6"
}

/* default drop acl */
// both IPv4 and IPv6 traffic should be forbade in dual-stack situation
allIPMatch := NewAndACLMatch(
NewACLMatch("outport", "==", "@"+pgName, ""),
NewACLMatch(ipSuffix, "", "", ""),
NewACLMatch("ip", "", "", ""),
)
options := func(acl *ovnnb.ACL) {
if logEnable {
Expand Down Expand Up @@ -75,15 +70,10 @@ func (c *OVNNbClient) UpdateEgressACLOps(pgName, asEgressName, asExceptName, pro

if strings.HasSuffix(asEgressName, ".0") || strings.HasSuffix(asEgressName, ".all") {
// create the default drop rule for only once
ipSuffix := "ip4"
if protocol == kubeovnv1.ProtocolIPv6 {
ipSuffix = "ip6"
}

/* default drop acl */
// both IPv4 and IPv6 traffic should be forbade in dual-stack situation
allIPMatch := NewAndACLMatch(
NewACLMatch("inport", "==", "@"+pgName, ""),
NewACLMatch(ipSuffix, "", "", ""),
NewACLMatch("ip", "", "", ""),
)
options := func(acl *ovnnb.ACL) {
if logEnable {
Expand Down Expand Up @@ -621,23 +611,18 @@ func (c *OVNNbClient) SetLogicalSwitchPrivate(lsName, cidrBlock, nodeSwitchCIDR
return nil
}

func (c *OVNNbClient) SetACLLog(pgName, protocol string, logEnable, isIngress bool) error {
func (c *OVNNbClient) SetACLLog(pgName string, logEnable, isIngress bool) error {
direction := ovnnb.ACLDirectionToLport
portDirection := "outport"
if !isIngress {
direction = ovnnb.ACLDirectionFromLport
portDirection = "inport"
}

ipSuffix := "ip4"
if protocol == kubeovnv1.ProtocolIPv6 {
ipSuffix = "ip6"
}

// match all traffic to or from pgName
allIPMatch := NewAndACLMatch(
NewACLMatch(portDirection, "==", "@"+pgName, ""),
NewACLMatch(ipSuffix, "", "", ""),
NewACLMatch("ip", "", "", ""),
)

acl, err := c.GetACL(pgName, direction, util.IngressDefaultDrop, allIPMatch.String(), true)
Expand All @@ -650,6 +635,9 @@ func (c *OVNNbClient) SetACLLog(pgName, protocol string, logEnable, isIngress bo
return nil // skip if acl not found
}

if acl.Log == logEnable {
return nil
}
acl.Log = logEnable

err = c.UpdateACL(acl, &acl.Log)
Expand Down
16 changes: 8 additions & 8 deletions pkg/ovs/ovn-nb-acl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func (suite *OvnClientTestSuite) testUpdateIngressACLOps() {
require.NoError(t, err)
require.Len(t, ops, 4)

expect(ops[0].Row, "drop", ovnnb.ACLDirectionToLport, fmt.Sprintf("outport == @%s && ip4", pgName), util.IngressDefaultDrop)
expect(ops[0].Row, "drop", ovnnb.ACLDirectionToLport, fmt.Sprintf("outport == @%s && ip", pgName), util.IngressDefaultDrop)

matches := newNetworkPolicyACLMatch(pgName, asIngressName, asExceptName, protocol, ovnnb.ACLDirectionToLport, npp, nil)
i := 1
Expand All @@ -120,7 +120,7 @@ func (suite *OvnClientTestSuite) testUpdateIngressACLOps() {
require.NoError(t, err)
require.Len(t, ops, 3)

expect(ops[0].Row, "drop", ovnnb.ACLDirectionToLport, fmt.Sprintf("outport == @%s && ip6", pgName), util.IngressDefaultDrop)
expect(ops[0].Row, "drop", ovnnb.ACLDirectionToLport, fmt.Sprintf("outport == @%s && ip", pgName), util.IngressDefaultDrop)

matches := newNetworkPolicyACLMatch(pgName, asIngressName, asExceptName, protocol, ovnnb.ACLDirectionToLport, nil, nil)
i := 1
Expand Down Expand Up @@ -164,7 +164,7 @@ func (suite *OvnClientTestSuite) testUpdateEgressACLOps() {
require.NoError(t, err)
require.Len(t, ops, 4)

expect(ops[0].Row, "drop", ovnnb.ACLDirectionFromLport, fmt.Sprintf("inport == @%s && ip4", pgName), util.EgressDefaultDrop)
expect(ops[0].Row, "drop", ovnnb.ACLDirectionFromLport, fmt.Sprintf("inport == @%s && ip", pgName), util.EgressDefaultDrop)

matches := newNetworkPolicyACLMatch(pgName, asEgressName, asExceptName, protocol, ovnnb.ACLDirectionFromLport, npp, nil)
i := 1
Expand All @@ -190,7 +190,7 @@ func (suite *OvnClientTestSuite) testUpdateEgressACLOps() {
require.NoError(t, err)
require.Len(t, ops, 3)

expect(ops[0].Row, "drop", ovnnb.ACLDirectionFromLport, fmt.Sprintf("inport == @%s && ip6", pgName), util.EgressDefaultDrop)
expect(ops[0].Row, "drop", ovnnb.ACLDirectionFromLport, fmt.Sprintf("inport == @%s && ip", pgName), util.EgressDefaultDrop)

matches := newNetworkPolicyACLMatch(pgName, asEgressName, asExceptName, protocol, ovnnb.ACLDirectionFromLport, nil, nil)
i := 1
Expand Down Expand Up @@ -719,7 +719,7 @@ func (suite *OvnClientTestSuite) testSetACLLog() {
require.NoError(t, err)

t.Run("set ingress acl log to false", func(t *testing.T) {
match := fmt.Sprintf("outport == @%s && ip4", pgName)
match := fmt.Sprintf("outport == @%s && ip", pgName)
acl := newACL(pgName, ovnnb.ACLDirectionToLport, util.IngressDefaultDrop, match, ovnnb.ACLActionDrop, func(acl *ovnnb.ACL) {
acl.Name = &pgName
acl.Log = true
Expand All @@ -729,7 +729,7 @@ func (suite *OvnClientTestSuite) testSetACLLog() {
err = ovnClient.CreateAcls(pgName, portGroupKey, acl)
require.NoError(t, err)

err = ovnClient.SetACLLog(pgName, kubeovnv1.ProtocolIPv4, false, true)
err = ovnClient.SetACLLog(pgName, false, true)
require.NoError(t, err)

acl, err = ovnClient.GetACL(pgName, ovnnb.ACLDirectionToLport, util.IngressDefaultDrop, match, false)
Expand All @@ -738,7 +738,7 @@ func (suite *OvnClientTestSuite) testSetACLLog() {
})

t.Run("set egress acl log to false", func(t *testing.T) {
match := fmt.Sprintf("inport == @%s && ip4", pgName)
match := fmt.Sprintf("inport == @%s && ip", pgName)
acl := newACL(pgName, ovnnb.ACLDirectionFromLport, util.IngressDefaultDrop, match, ovnnb.ACLActionDrop, func(acl *ovnnb.ACL) {
acl.Name = &pgName
acl.Log = false
Expand All @@ -748,7 +748,7 @@ func (suite *OvnClientTestSuite) testSetACLLog() {
err = ovnClient.CreateAcls(pgName, portGroupKey, acl)
require.NoError(t, err)

err = ovnClient.SetACLLog(pgName, kubeovnv1.ProtocolIPv4, true, false)
err = ovnClient.SetACLLog(pgName, true, false)
require.NoError(t, err)

acl, err = ovnClient.GetACL(pgName, ovnnb.ACLDirectionFromLport, util.IngressDefaultDrop, match, false)
Expand Down
Loading