Skip to content

Commit

Permalink
Properly mark DHCP & DNS traffic entering switch NI from outside
Browse files Browse the repository at this point in the history
After a recent change that prevents non-app traffic from being
forwarded between ports based on the assigned mark, it is
important that even implicitly allowed DHCP & DNS traffic
entering switch NI from outside is properly marked.

However, in our implementation we skipped DHCP & DNS marking rules
for ingress direction, assuming that replies would be matched with
their requests in the conntrack table and mark would get propagated.
With such assumption, we only added DHCP & DNS marking rules for
the egress direction (app requests).
But apparently, request-reply conntrack matching does not work for
DHCP, so let's avoid this assumption altogether and install marking
rules for DHCP & DNS for both directions (for switch NI).

Signed-off-by: Milan Lenco <milan@zededa.com>
(cherry picked from commit f7e3dbf)
  • Loading branch information
milan-zededa committed Apr 26, 2024
1 parent 6bc3385 commit 39b4844
Showing 1 changed file with 11 additions and 15 deletions.
26 changes: 11 additions & 15 deletions pkg/pillar/nireconciler/linux_acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,11 @@ const (
// Describes protocol that is allowed implicitly because it provides some essential
// function for applications.
type essentialProto struct {
label string
ingressMatch []string
egressMatch []string
mark uint32
markChainName string
canOrigOutside bool // true if communication can be initiated from outside the edge node
label string
ingressMatch []string
egressMatch []string
mark uint32
markChainName string
}

// User-configured ACL rule.
Expand Down Expand Up @@ -163,21 +162,18 @@ func getEssentialIPv6Protos(niType types.NetworkInstanceType,
switch niType {
case types.NetworkInstanceTypeSwitch:
protos = append(protos, essentialProto{
label: "ICMPv6",
egressMatch: []string{"-p", "ipv6-icmp"},
ingressMatch: []string{"-p", "ipv6-icmp"},
mark: iptables.ControlProtocolMarkingIDMap["app_icmpv6"],
markChainName: "icmpv6",
canOrigOutside: true,
label: "ICMPv6",
egressMatch: []string{"-p", "ipv6-icmp"},
ingressMatch: []string{"-p", "ipv6-icmp"},
mark: iptables.ControlProtocolMarkingIDMap["app_icmpv6"],
markChainName: "icmpv6",
})
protos = append(protos, essentialProto{
label: "DHCPv6",
egressMatch: []string{"-p", "udp", "--dport", "dhcpv6-server"},
ingressMatch: []string{"-p", "udp", "--sport", "dhcpv6-server"},
mark: iptables.ControlProtocolMarkingIDMap["app_dhcp"],
markChainName: "dhcpv6",
// RFC 6644 (DHCPv6 Reconfigure) allows the server to initiate communication.
canOrigOutside: true,
})
protos = append(protos, essentialProto{
label: "DNS over UDP",
Expand Down Expand Up @@ -960,7 +956,7 @@ func (r *LinuxNIReconciler) getIntendedAppConnMangleIptables(vif vifInfo,
}
// 1.1. Mark essential protocols allowed implicitly.
for _, proto := range essentialProtos {
if proto.ingressMatch == nil || !proto.canOrigOutside {
if proto.ingressMatch == nil {
continue
}
markChain := markChainPrefix + proto.markChainName
Expand Down

0 comments on commit 39b4844

Please sign in to comment.