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

Fix DNS based CWNPs for network-isolated Clusters #177

Merged
merged 10 commits into from
Apr 22, 2024
Merged
14 changes: 7 additions & 7 deletions controllers/clusterwidenetworkpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
}
Expand Down Expand Up @@ -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
}
Comment on lines -240 to -243
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will try to investigate the impact on update traffic this will produce as this essentially updates all CWNPs of a cluster very regularly. Maybe it's worth to implement an update that only occurs if necessary (by comparing contents).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a significant difference between requests of this firewall-controller and 2.3.0 for CWNPs in API server dashboards, so I assume we can start doing this.


cwnp.Status.Message = msg
cwnp.Status.State = state

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
2 changes: 1 addition & 1 deletion pkg/dns/dns_proxy_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/dns/dnscache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
11 changes: 9 additions & 2 deletions pkg/dns/dnsproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions pkg/nftables/firewall.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions pkg/nftables/mocks/mock_fqdncache.go

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

13 changes: 13 additions & 0 deletions pkg/nftables/networkpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 12 additions & 0 deletions pkg/nftables/nftables.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
31 changes: 22 additions & 9 deletions pkg/nftables/rendering.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 6 additions & 5 deletions pkg/nftables/rendering_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down Expand Up @@ -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))
}
})
}
Expand Down
10 changes: 10 additions & 0 deletions pkg/nftables/test_data/more-rules.nftable.v4
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
}
Loading