Merge pull request #177 from metal-stack/fix-initial-dns

Fix DNS based CWNPs for network-isolated Clusters
metal-stack · Apr 22, 2024 · a9deaca · a9deaca
2 parents c166803 + 0243368
commit a9deaca
Show file tree

Hide file tree

Showing 13 changed files with 101 additions and 29 deletions.
diff --git a/controllers/clusterwidenetworkpolicy_controller.go b/controllers/clusterwidenetworkpolicy_controller.go
@@ -142,7 +142,7 @@ func (r *ClusterwideNetworkPolicyReconciler) manageDNSProxy(
 
 	if enableDNS && r.DnsProxy == nil {
 		r.Log.Info("DNS Proxy is initialized")
-		if r.DnsProxy, err = dns.NewDNSProxy(f.Spec.DNSPort, ctrl.Log.WithName("DNS proxy")); err != nil {
+		if r.DnsProxy, err = dns.NewDNSProxy(f.Spec.DNSServerAddress, f.Spec.DNSPort, ctrl.Log.WithName("DNS proxy")); err != nil {
 			return fmt.Errorf("failed to init DNS proxy: %w", err)
 		}
 		go r.DnsProxy.Run(ctx)
@@ -154,7 +154,12 @@ func (r *ClusterwideNetworkPolicyReconciler) manageDNSProxy(
 
 	// If proxy is ON, update DNS address(if it's set in spec)
 	if r.DnsProxy != nil && f.Spec.DNSServerAddress != "" {
-		if err = r.DnsProxy.UpdateDNSServerAddr(f.Spec.DNSServerAddress); err != nil {
+		port := 53
+		if f.Spec.DNSPort != nil {
+			port = int(*f.Spec.DNSPort)
+		}
+		addr := fmt.Sprintf("%s:%d", f.Spec.DNSServerAddress, port)
+		if err = r.DnsProxy.UpdateDNSServerAddr(addr); err != nil {
 			return fmt.Errorf("failed to update DNS server address: %w", err)
 		}
 	}
@@ -237,11 +242,6 @@ func (r *ClusterwideNetworkPolicyReconciler) allowedCWNPs(ctx context.Context, c
 }
 
 func (r *ClusterwideNetworkPolicyReconciler) updateCWNPState(ctx context.Context, cwnp firewallv1.ClusterwideNetworkPolicy, state firewallv1.PolicyDeploymentState, msg string) error {
-	// do nothing if message and state already have the desired values
-	if cwnp.Status.Message == msg && cwnp.Status.State == state {
-		return nil
-	}
-
 	cwnp.Status.Message = msg
 	cwnp.Status.State = state
 

diff --git a/go.mod b/go.mod
@@ -13,7 +13,7 @@ require (
 	github.com/metal-stack/firewall-controller-manager v0.3.2
 	github.com/metal-stack/metal-go v0.28.1
 	github.com/metal-stack/metal-lib v0.15.1
-	github.com/metal-stack/metal-networker v0.42.0
+	github.com/metal-stack/metal-networker v0.43.0
 	github.com/metal-stack/v v1.0.3
 	github.com/miekg/dns v1.1.58
 	github.com/txn2/txeh v1.5.5

diff --git a/go.sum b/go.sum
@@ -166,8 +166,8 @@ github.com/metal-stack/metal-hammer v0.12.3 h1:XY6PwTnOqBlhL9z/sk13/rdy8XRYdBAdf
 github.com/metal-stack/metal-hammer v0.12.3/go.mod h1:2igSC1ZnqxZcARkkUW9qA8PV04VdN9qmUIfUAZ1lGhs=
 github.com/metal-stack/metal-lib v0.15.1 h1:QCmtZ6ci6pHsf3RQnSDbbvYshpyRaxCSeXghVvbDFuA=
 github.com/metal-stack/metal-lib v0.15.1/go.mod h1:x1nyPRi+b/WeK7N41cm4R8w4pScnhOYv8hos2UM4lXY=
-github.com/metal-stack/metal-networker v0.42.0 h1:tVuYw/3GN8lGhkJ91vTD+ax8fAAouFzBRysKxqQlUXo=
-github.com/metal-stack/metal-networker v0.42.0/go.mod h1:exBcBdyDzngo2s3848tASbndizrPlS6a2/Eg90xLTwc=
+github.com/metal-stack/metal-networker v0.43.0 h1:MbanA43IINJyoHnMTsUS3o93T3bncTtVy11BexlSMy8=
+github.com/metal-stack/metal-networker v0.43.0/go.mod h1:exBcBdyDzngo2s3848tASbndizrPlS6a2/Eg90xLTwc=
 github.com/metal-stack/v v1.0.3 h1:Sh2oBlnxrCUD+mVpzfC8HiqL045YWkxs0gpTvkjppqs=
 github.com/metal-stack/v v1.0.3/go.mod h1:YTahEu7/ishwpYKnp/VaW/7nf8+PInogkfGwLcGPdXg=
 github.com/miekg/dns v1.1.58 h1:ca2Hdkz+cDg/7eNF6V56jjzuZ4aCAE+DbVkILdQWG/4=

diff --git a/pkg/dns/dns_proxy_handler.go b/pkg/dns/dns_proxy_handler.go
@@ -38,7 +38,7 @@ func NewDNSProxyHandler(log logr.Logger, cache *DNSCache) *DNSProxyHandler {
 		log:           log.WithName("DNS handler"),
 		udpClient:     udpClient,
 		tcpClient:     tcpClient,
-		dnsServerAddr: defaultDNSServerAddr,
+		dnsServerAddr: cache.dnsServerAddr,
 		updateCache:   getUpdateCacheFunc(log, cache),
 	}
 }

diff --git a/pkg/dns/dnscache.go b/pkg/dns/dnscache.go
@@ -115,12 +115,12 @@ type DNSCache struct {
 	ipv6Enabled   bool
 }
 
-func newDNSCache(ipv4Enabled, ipv6Enabled bool, log logr.Logger) *DNSCache {
+func newDNSCache(dns string, ipv4Enabled, ipv6Enabled bool, log logr.Logger) *DNSCache {
 	return &DNSCache{
 		log:           log,
 		fqdnToEntry:   map[string]cacheEntry{},
 		setNames:      map[string]struct{}{},
-		dnsServerAddr: defaultDNSServerAddr,
+		dnsServerAddr: dns,
 		ipv4Enabled:   ipv4Enabled,
 		ipv6Enabled:   ipv6Enabled,
 	}

diff --git a/pkg/dns/dnsproxy.go b/pkg/dns/dnsproxy.go
@@ -36,8 +36,11 @@ type DNSProxy struct {
 	handler DNSHandler
 }
 
-func NewDNSProxy(port *uint, log logr.Logger) (*DNSProxy, error) {
-	cache := newDNSCache(true, false, log.WithName("DNS cache"))
+func NewDNSProxy(dns string, port *uint, log logr.Logger) (*DNSProxy, error) {
+	if dns == "" {
+		dns = defaultDNSServerAddr
+	}
+	cache := newDNSCache(dns, true, false, log.WithName("DNS cache"))
 	handler := NewDNSProxyHandler(log, cache)
 
 	host, err := getHost()
@@ -121,6 +124,10 @@ func (p *DNSProxy) IsInitialized() bool {
 	return p != nil
 }
 
+func (p *DNSProxy) CacheAddr() (string, error) {
+	return getHost()
+}
+
 func getHost() (string, error) {
 	c, err := netconf.New(network.GetLogger(), network.MetalNetworkerConfig)
 	if err != nil || c == nil {

diff --git a/pkg/nftables/firewall.go b/pkg/nftables/firewall.go
@@ -40,6 +40,7 @@ type FQDNCache interface {
 	GetSetsForRendering(fqdns []firewallv1.FQDNSelector) (result []dns.RenderIPSet)
 	GetSetsForFQDN(fqdn firewallv1.FQDNSelector, fqdnSets []firewallv1.IPSet) (result []firewallv1.IPSet)
 	IsInitialized() bool
+	CacheAddr() (string, error)
 }
 
 // Firewall assembles nftable rules based on k8s entities

diff --git a/pkg/nftables/mocks/mock_fqdncache.go b/pkg/nftables/mocks/mock_fqdncache.go
diff --git a/pkg/nftables/networkpolicy.go b/pkg/nftables/networkpolicy.go
@@ -60,6 +60,19 @@ func clusterwideNetworkPolicyIngressRules(np firewallv1.ClusterwideNetworkPolicy
 	return uniqueSorted(rules)
 }
 
+func clusterwideNetworkPolicyEgressDNSCacheRules(cache FQDNCache, logAcceptedConnections bool) (nftablesRules, error) {
+	addr, err := cache.CacheAddr()
+	if err != nil {
+		return nil, err
+	}
+	base := []string{"ip saddr == @cluster_prefixes", fmt.Sprintf("ip daddr { %s }", addr)}
+	comment := fmt.Sprintf("accept intercepted traffic for dns cache")
+	return nftablesRules{
+		assembleDestinationPortRule(base, "tcp", []string{"53"}, logAcceptedConnections, comment+" tcp"),
+		assembleDestinationPortRule(base, "udp", []string{"53"}, logAcceptedConnections, comment+" udp"),
+	}, nil
+}
+
 func clusterwideNetworkPolicyEgressRules(
 	cache FQDNCache,
 	np firewallv1.ClusterwideNetworkPolicy,

diff --git a/pkg/nftables/nftables.tpl b/pkg/nftables/nftables.tpl
@@ -82,3 +82,15 @@ table inet firewall {
 	}
 {{- end }}
 }
+{{- if .AdditionalDNSAddrs }}
+
+# Add additional DNS addresses for dnat redirection for the dns proxy
+table inet nat {
+	set proxy_dns_servers {
+		type ipv4_addr
+		flags interval
+		auto-merge
+		elements = { {{ StringsJoin .AdditionalDNSAddrs ", " }} }
+	}
+}
+{{- end }}
diff --git a/pkg/nftables/rendering.go b/pkg/nftables/rendering.go
@@ -15,12 +15,13 @@ import (
 
 // firewallRenderingData holds the data available in the nftables template
 type firewallRenderingData struct {
-	ForwardingRules  forwardingRules
-	RateLimitRules   nftablesRules
-	SnatRules        nftablesRules
-	Sets             []dns.RenderIPSet
-	InternalPrefixes string
-	PrivateVrfID     uint
+	ForwardingRules    forwardingRules
+	RateLimitRules     nftablesRules
+	SnatRules          nftablesRules
+	Sets               []dns.RenderIPSet
+	InternalPrefixes   string
+	PrivateVrfID       uint
+	AdditionalDNSAddrs []string
 }
 
 func newFirewallRenderingData(f *Firewall) (*firewallRenderingData, error) {
@@ -56,13 +57,25 @@ func newFirewallRenderingData(f *Firewall) (*firewallRenderingData, error) {
 		return &firewallRenderingData{}, err
 	}
 
-	var sets []dns.RenderIPSet
+	var (
+		sets     []dns.RenderIPSet
+		dnsAddrs = []string{}
+	)
 	if f.cache.IsInitialized() {
 		sets = f.cache.GetSetsForRendering(f.clusterwideNetworkPolicies.GetFQDNs())
+		rules, err := clusterwideNetworkPolicyEgressDNSCacheRules(f.cache, f.logAcceptedConnections)
+		if err != nil {
+			return &firewallRenderingData{}, err
+		}
+		if f.firewall.Spec.DNSServerAddress != "" {
+			dnsAddrs = append(dnsAddrs, f.firewall.Spec.DNSServerAddress)
+		}
+		egress = append(egress, rules...)
 	}
 	return &firewallRenderingData{
-		PrivateVrfID:     uint(*f.primaryPrivateNet.Vrf),
-		InternalPrefixes: strings.Join(f.firewall.Spec.InternalPrefixes, ", "),
+		AdditionalDNSAddrs: dnsAddrs,
+		PrivateVrfID:       uint(*f.primaryPrivateNet.Vrf),
+		InternalPrefixes:   strings.Join(f.firewall.Spec.InternalPrefixes, ", "),
 		ForwardingRules: forwardingRules{
 			Ingress: ingress,
 			Egress:  egress,

diff --git a/pkg/nftables/rendering_test.go b/pkg/nftables/rendering_test.go
@@ -37,10 +37,11 @@ func TestFirewallRenderingData_renderString(t *testing.T) {
 					Egress:  []string{"egress rule 1", "egress rule 2"},
 					Ingress: []string{"ingress rule 1", "ingress rule 2"},
 				},
-				InternalPrefixes: "1.2.3.0/24, 2.3.4.0/8",
-				RateLimitRules:   []string{"meta iifname \"eth0\" limit rate over 10 mbytes/second counter name drop_ratelimit drop"},
-				SnatRules:        []string{"ip saddr { 10.0.0.0/8 } oifname \"vlan104009\" counter snat 185.1.2.3 comment \"snat internet\""},
-				PrivateVrfID:     uint(42),
+				InternalPrefixes:   "1.2.3.0/24, 2.3.4.0/8",
+				RateLimitRules:     []string{"meta iifname \"eth0\" limit rate over 10 mbytes/second counter name drop_ratelimit drop"},
+				SnatRules:          []string{"ip saddr { 10.0.0.0/8 } oifname \"vlan104009\" counter snat 185.1.2.3 comment \"snat internet\""},
+				PrivateVrfID:       uint(42),
+				AdditionalDNSAddrs: []string{"8.9.10.11", "4.5.6.7"},
 			},
 			wantErr: false,
 		},
@@ -98,7 +99,7 @@ func TestFirewallRenderingData_renderString(t *testing.T) {
 			rendered, _ := os.ReadFile(path.Join("test_data", tt.name+".nftable.v4"))
 			want := string(rendered)
 			if got != want {
-				t.Errorf("Firewall.renderString() diff: %v", cmp.Diff(got, want))
+				t.Errorf("Firewall.renderString() diff: %v", cmp.Diff(want, got))
 			}
 		})
 	}

diff --git a/pkg/nftables/test_data/more-rules.nftable.v4 b/pkg/nftables/test_data/more-rules.nftable.v4
@@ -65,3 +65,13 @@ table inet firewall {
 		ip saddr { 10.0.0.0/8 } oifname "vlan104009" counter snat 185.1.2.3 comment "snat internet"
 	}
 }
+
+# Add additional DNS addresses for dnat redirection for the dns proxy
+table inet nat {
+	set proxy_dns_servers {
+		type ipv4_addr
+		flags interval
+		auto-merge
+		elements = { 8.9.10.11, 4.5.6.7 }
+	}
+}