Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix default allow mgmt ACL using conntrack 
By applying an "allow-related" ACL to every logical switch for mgmt
traffic it causes all traffic between pods to go through conntrack. This
is undesirable, especially when no network policy is being used.

Signed-off-by: Tim Rozet <trozet@redhat.com>
  • Loading branch information
@trozet
trozet committed Aug 12, 2020
1 parent 427e311 commit fd47587
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 7 deletions.
20 changes: 16 additions & 4 deletions go-controller/pkg/ovn/master_test.go
View file
Expand Up @@ -189,7 +189,10 @@ func defaultFakeExec(nodeSubnet, nodeName string, sctpSupport bool) (*ovntest.Fa
})
}
fexec.AddFakeCmdsNoOutputNoError([]string{
"ovn-nbctl --timeout=15 --may-exist acl-add " + nodeName + " to-lport 1001 ip4.src==" + nodeMgmtPortIP.String() + " allow-related",
"ovn-nbctl --timeout=15 --may-exist acl-add " + nodeName + " to-lport 1001 ip4.src==" + nodeMgmtPortIP.String() + " allow " +
"-- --may-exist acl-add " + nodeName + " to-lport 1001 ip4.dst==" + nodeMgmtPortIP.String() + " allow " +
"-- --may-exist acl-add " + nodeName + " from-lport 1001 ip4.src==" + nodeMgmtPortIP.String() + " allow " +
"-- --may-exist acl-add " + nodeName + " from-lport 1001 ip4.dst==" + nodeMgmtPortIP.String() + " allow",
"ovn-nbctl --timeout=15 -- --may-exist lsp-add " + nodeName + " " + util.K8sPrefix + nodeName + " -- lsp-set-addresses " + util.K8sPrefix + nodeName + " " + mgmtMAC + " " + nodeMgmtPortIP.String(),
})
fexec.AddFakeCmd(&ovntest.ExpectedCmd{
Expand Down Expand Up @@ -607,7 +610,10 @@ subnet=%s
"ovn-nbctl --timeout=15 set logical_switch " + masterName + " load_balancer=" + tcpLBUUID,
"ovn-nbctl --timeout=15 add logical_switch " + masterName + " load_balancer " + udpLBUUID,
"ovn-nbctl --timeout=15 add logical_switch " + masterName + " load_balancer " + sctpLBUUID,
"ovn-nbctl --timeout=15 --may-exist acl-add " + masterName + " to-lport 1001 ip4.src==" + masterMgmtPortIP + " allow-related",
"ovn-nbctl --timeout=15 --may-exist acl-add " + masterName + " to-lport 1001 ip4.src==" + masterMgmtPortIP + " allow " +
"-- --may-exist acl-add " + masterName + " to-lport 1001 ip4.dst==" + masterMgmtPortIP + " allow " +
"-- --may-exist acl-add " + masterName + " from-lport 1001 ip4.src==" + masterMgmtPortIP + " allow " +
"-- --may-exist acl-add " + masterName + " from-lport 1001 ip4.dst==" + masterMgmtPortIP + " allow",
"ovn-nbctl --timeout=15 -- --may-exist lsp-add " + masterName + " " + util.K8sPrefix + masterName + " -- lsp-set-addresses " + util.K8sPrefix + masterName + " " + masterMgmtPortMAC + " " + masterMgmtPortIP,
})
fexec.AddFakeCmd(&ovntest.ExpectedCmd{
Expand Down Expand Up @@ -820,7 +826,10 @@ var _ = Describe("Gateway Init Operations", func() {
"ovn-nbctl --timeout=15 set logical_switch " + nodeName + " load_balancer=" + tcpLBUUID,
"ovn-nbctl --timeout=15 add logical_switch " + nodeName + " load_balancer " + udpLBUUID,
"ovn-nbctl --timeout=15 add logical_switch " + nodeName + " load_balancer " + sctpLBUUID,
"ovn-nbctl --timeout=15 --may-exist acl-add " + nodeName + " to-lport 1001 ip4.src==" + masterMgmtPortIP + " allow-related",
"ovn-nbctl --timeout=15 --may-exist acl-add " + nodeName + " to-lport 1001 ip4.src==" + masterMgmtPortIP + " allow " +
"-- --may-exist acl-add " + nodeName + " to-lport 1001 ip4.dst==" + masterMgmtPortIP + " allow " +
"-- --may-exist acl-add " + nodeName + " from-lport 1001 ip4.src==" + masterMgmtPortIP + " allow " +
"-- --may-exist acl-add " + nodeName + " from-lport 1001 ip4.dst==" + masterMgmtPortIP + " allow",
"ovn-nbctl --timeout=15 -- --may-exist lsp-add " + nodeName + " " + util.K8sPrefix + nodeName + " -- lsp-set-addresses " + util.K8sPrefix + nodeName + " " + brLocalnetMAC + " " + masterMgmtPortIP,
})
fexec.AddFakeCmd(&ovntest.ExpectedCmd{
Expand Down Expand Up @@ -1009,7 +1018,10 @@ var _ = Describe("Gateway Init Operations", func() {
"ovn-nbctl --timeout=15 set logical_switch " + nodeName + " load_balancer=" + tcpLBUUID,
"ovn-nbctl --timeout=15 add logical_switch " + nodeName + " load_balancer " + udpLBUUID,
"ovn-nbctl --timeout=15 add logical_switch " + nodeName + " load_balancer " + sctpLBUUID,
"ovn-nbctl --timeout=15 --may-exist acl-add " + nodeName + " to-lport 1001 ip4.src==" + nodeMgmtPortIP + " allow-related",
"ovn-nbctl --timeout=15 --may-exist acl-add " + nodeName + " to-lport 1001 ip4.src==" + nodeMgmtPortIP + " allow " +
"-- --may-exist acl-add " + nodeName + " to-lport 1001 ip4.dst==" + nodeMgmtPortIP + " allow " +
"-- --may-exist acl-add " + nodeName + " from-lport 1001 ip4.src==" + nodeMgmtPortIP + " allow " +
"-- --may-exist acl-add " + nodeName + " from-lport 1001 ip4.dst==" + nodeMgmtPortIP + " allow",
"ovn-nbctl --timeout=15 -- --may-exist lsp-add " + nodeName + " " + util.K8sPrefix + nodeName + " -- lsp-set-addresses " + util.K8sPrefix + nodeName + " " + nodeMgmtPortMAC + " " + nodeMgmtPortIP,
})
fexec.AddFakeCmd(&ovntest.ExpectedCmd{
Expand Down
11 changes: 8 additions & 3 deletions go-controller/pkg/ovn/policy.go
View file
Expand Up @@ -100,9 +100,14 @@ func addAllowACLFromNode(logicalSwitch string, mgmtPortIP net.IP) error {
if utilnet.IsIPv6(mgmtPortIP) {
ipFamily = "ip6"
}
match := fmt.Sprintf("%s.src==%s", ipFamily, mgmtPortIP.String())
_, stderr, err := util.RunOVNNbctl("--may-exist", "acl-add", logicalSwitch,
"to-lport", defaultAllowPriority, match, "allow-related")

matchSrc := fmt.Sprintf("%s.src==%s", ipFamily, mgmtPortIP.String())
matchDst := fmt.Sprintf("%s.dst==%s", ipFamily, mgmtPortIP.String())
_, stderr, err := util.RunOVNNbctl(
"--may-exist", "acl-add", logicalSwitch, "to-lport", defaultAllowPriority, matchSrc, "allow",
"--", "--may-exist", "acl-add", logicalSwitch, "to-lport", defaultAllowPriority, matchDst, "allow",
"--", "--may-exist", "acl-add", logicalSwitch, "from-lport", defaultAllowPriority, matchSrc, "allow",
"--", "--may-exist", "acl-add", logicalSwitch, "from-lport", defaultAllowPriority, matchDst, "allow")
if err != nil {
return fmt.Errorf("failed to create the node acl for "+
"logical_switch=%s, stderr: %q (%v)", logicalSwitch, stderr, err)
Expand Down

0 comments on commit fd47587

Please sign in to comment.