Closed GrigoriyMikhalkin closed 1 year ago
I'd propose to start with support for plain / unencrypted DNS traffic over UDP and TCP. The encrypted variants can only be addressed by MITM which is not that easy.
If the refactoring is done and there are tests for the current feature set, it's sufficient :-)
I also vote to leave DNS over HTTPS out of the picture for now.
@mwindower @majst01 I finished with functionality, tests and CI. Please review the code.
let's add a comment for the fqdn for the respective nft rule
@majst01 PR is ready for review.
Error during dns lookup:
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: 2021-07-29T14:22:55.891+0200 ERROR DNS cache failed to load data for FQDN {"error": "failed to get DNS data about fqdn www.spiegel.de.: dns: failed to unpack truncated message"}
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: github.com/metal-stack/firewall-controller/pkg/dns.(*DNSCache).getSetNameForFQDN
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: github.com/metal-stack/firewall-controller/pkg/dns/dnscache.go:218
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: github.com/metal-stack/firewall-controller/pkg/dns.(*DNSCache).getSetNameForFQDN
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: github.com/metal-stack/firewall-controller/pkg/dns/dnscache.go:218
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: github.com/metal-stack/firewall-controller/pkg/dns.(*DNSCache).GetSetsForFQDN
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: github.com/metal-stack/firewall-controller/pkg/dns/dnscache.go:125
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: github.com/metal-stack/firewall-controller/pkg/nftables.clusterwideNetworkPolicyEgressToFQDNRules
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: github.com/metal-stack/firewall-controller/pkg/nftables/networkpolicy.go:133
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: github.com/metal-stack/firewall-controller/pkg/nftables.clusterwideNetworkPolicyEgressRules
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: github.com/metal-stack/firewall-controller/pkg/nftables/networkpolicy.go:100
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: github.com/metal-stack/firewall-controller/pkg/nftables.clusterwideNetworkPolicyRules
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: github.com/metal-stack/firewall-controller/pkg/nftables/networkpolicy.go:23
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: github.com/metal-stack/firewall-controller/pkg/nftables.newFirewallRenderingData
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: github.com/metal-stack/firewall-controller/pkg/nftables/rendering.go:32
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: github.com/metal-stack/firewall-controller/pkg/nftables.(*Firewall).renderFile
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: github.com/metal-stack/firewall-controller/pkg/nftables/firewall.go:176
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: github.com/metal-stack/firewall-controller/pkg/nftables.(*Firewall).Reconcile
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: github.com/metal-stack/firewall-controller/pkg/nftables/firewall.go:137
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: github.com/metal-stack/firewall-controller/controllers.(*ClusterwideNetworkPolicyReconciler).reconcileRules
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: github.com/metal-stack/firewall-controller/controllers/clusterwidenetworkpolicy_controller.go:88
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: github.com/metal-stack/firewall-controller/controllers.(*ClusterwideNetworkPolicyReconciler).Reconcile
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: github.com/metal-stack/firewall-controller/controllers/clusterwidenetworkpolicy_controller.go:66
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: sigs.k8s.io/controller-runtime@v0.6.3/pkg/internal/controller/controller.go:244
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: sigs.k8s.io/controller-runtime@v0.6.3/pkg/internal/controller/controller.go:218
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).worker
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: sigs.k8s.io/controller-runtime@v0.6.3/pkg/internal/controller/controller.go:197
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: k8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: k8s.io/apimachinery@v0.20.1/pkg/util/wait/wait.go:155
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: k8s.io/apimachinery/pkg/util/wait.BackoffUntil
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: k8s.io/apimachinery@v0.20.1/pkg/util/wait/wait.go:156
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: k8s.io/apimachinery/pkg/util/wait.JitterUntil
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: k8s.io/apimachinery@v0.20.1/pkg/util/wait/wait.go:133
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: k8s.io/apimachinery/pkg/util/wait.Until
Jul 29 14:22:55 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[423680]: k8s.io/apimachinery@v0.20.1/pkg/util/wait/wait.go:90
Improvement:
Sometimes recursion occurs:
Jul 29 14:44:18 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[424151]: github.com/metal-stack/firewall-controller/pkg/dns/dnscache.go:218
Jul 29 14:44:18 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[424151]: github.com/metal-stack/firewall-controller/pkg/dns.(*DNSCache).getSetNameForFQDN
Jul 29 14:44:18 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[424151]: github.com/metal-stack/firewall-controller/pkg/dns/dnscache.go:218
Jul 29 14:44:18 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[424151]: github.com/metal-stack/firewall-controller/pkg/dns.(*DNSCache).getSetNameForFQDN
Jul 29 14:44:18 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[424151]: github.com/metal-stack/firewall-controller/pkg/dns/dnscache.go:218
Jul 29 14:44:18 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[424151]: github.com/metal-stack/firewall-controller/pkg/dns.(*DNSCache).getSetNameForFQDN
Jul 29 14:44:18 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[424151]: github.com/metal-stack/firewall-controller/pkg/dns/dnscache.go:218
Jul 29 14:44:18 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[424151]: github.com/metal-stack/firewall-controller/pkg/dns.(*DNSCache).getSetNameForFQDN
Jul 29 14:44:18 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[424151]: github.com/metal-stack/firewall-controller/pkg/dns/dnscache.go:218
Jul 29 14:44:18 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[424151]: github.com/metal-stack/firewall-controller/pkg/dns.(*DNSCache).getSetNameForFQDN
Jul 29 14:44:18 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[424151]: github.com/metal-stack/firewall-controller/pkg/dns/dnscache.go:218
Jul 29 14:44:18 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[424151]: github.com/metal-stack/firewall-controller/pkg/dns.(*DNSCache).getSetNameForFQDN
Jul 29 14:44:18 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[424151]: github.com/metal-stack/firewall-controller/pkg/dns/dnscache.go:218
Jul 29 14:44:18 shoot--p9sk4j--inttest0-firewall-5b3b7 ip[424151]: github.com/metal-stack/firewall-controller/pkg/dns.(*DNSCache).getSetNameForFQDN
cc @majst01 @Gerrit91
Updated. One possible improvement: i have to use additional logger for metal-networker
, as it expects SugaredLogger
. Possibly we could change to structured logging instead. Not sure if it would be convenient to use in metal-images
.
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: 1.669723262049428e+09 INFO Observed a panic in reconciler: runtime error: invalid memory address or nil pointer dereference {"controller": "clusterwidenetworkpolicy", "controllerGroup":
"metal-stack.io", "controllerKind": "ClusterwideNetworkPolicy", "ClusterwideNetworkPolicy": {"name":"allow-to-vpn","namespace":"firewall"}, "namespace": "firewall", "name": "allow-to-vpn", "reconcileID": "1bc60181-eaf4-4d35-9407-d5067ade56d3"}
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: panic: runtime error: invalid memory address or nil pointer dereference [recovered]
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: panic: runtime error: invalid memory address or nil pointer dereference
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: [signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x1590999]
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: goroutine 439 [running]:
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile.func1()
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: sigs.k8s.io/controller-runtime@v0.13.1/pkg/internal/controller/controller.go:118 +0x1f4
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: panic({0x172c7e0, 0x27a8610})
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: runtime/panic.go:838 +0x207
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: github.com/metal-stack/firewall-controller/pkg/dns.(*DNSProxy).GetSetsForRendering(0x15d2ebf?, {0x2804498?, 0x19338c0?, 0x2?})
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: github.com/metal-stack/firewall-controller/pkg/dns/dnsproxy.go:112 +0x19
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: github.com/metal-stack/firewall-controller/pkg/nftables.newFirewallRenderingData(0xc0007ab180)
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: github.com/metal-stack/firewall-controller/pkg/nftables/rendering.go:49 +0x3ff
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: github.com/metal-stack/firewall-controller/pkg/nftables.(*Firewall).renderFile(0xc0007ab180, {0xc000138540, 0x2f})
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: github.com/metal-stack/firewall-controller/pkg/nftables/firewall.go:186 +0x2c
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: github.com/metal-stack/firewall-controller/pkg/nftables.(*Firewall).Reconcile(0xc0007ab180)
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: github.com/metal-stack/firewall-controller/pkg/nftables/firewall.go:142 +0x1be
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: github.com/metal-stack/firewall-controller/controllers.(*ClusterwideNetworkPolicyReconciler).reconcileRules(0xc00045abe0, {0x1bd3478, 0xc000a235c0}, {{0x1bd53c8?, 0xc000a235f0?}, 0x7f769112dec8?}, {{{0x0, 0x
0}, {0x0, 0x0}}, ...})
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: github.com/metal-stack/firewall-controller/controllers/clusterwidenetworkpolicy_controller.go:98 +0x37b
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: github.com/metal-stack/firewall-controller/controllers.(*ClusterwideNetworkPolicyReconciler).Reconcile(0xc00045abe0, {0x1bd3478, 0xc000a235c0}, {{{0xc000a63278?, 0x10?}, {0xc000a63280?, 0x40d787?}}})
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: github.com/metal-stack/firewall-controller/controllers/clusterwidenetworkpolicy_controller.go:70 +0x232
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile(0x1bd33d0?, {0x1bd3478?, 0xc000a235c0?}, {{{0xc000a63278?, 0x1867e20?}, {0xc000a63280?, 0x4041f4?}}})
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: sigs.k8s.io/controller-runtime@v0.13.1/pkg/internal/controller/controller.go:121 +0xc8
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler(0xc00036b7c0, {0x1bd33d0, 0xc0000bc9c0}, {0x178a680?, 0xc000410000?})
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: sigs.k8s.io/controller-runtime@v0.13.1/pkg/internal/controller/controller.go:320 +0x33c
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem(0xc00036b7c0, {0x1bd33d0, 0xc0000bc9c0})
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: sigs.k8s.io/controller-runtime@v0.13.1/pkg/internal/controller/controller.go:273 +0x1d9
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2()
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: sigs.k8s.io/controller-runtime@v0.13.1/pkg/internal/controller/controller.go:234 +0x85
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: created by sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 ip[1494589]: sigs.k8s.io/controller-runtime@v0.13.1/pkg/internal/controller/controller.go:230 +0x325
Nov 29 13:01:02 shoot--pcfgbt--cilium-firewall-8ac65 systemd[1]: firewall-controller.service: Main process exited, code=exited, status=2/INVALIDARGUMENT
apiVersion: metal-stack.io/v1
kind: ClusterwideNetworkPolicy
metadata:
annotations:
resources.gardener.cloud/description: |-
DO NOT EDIT - This resource is managed by gardener-resource-manager.
Any modifications are discarded and the resource is returned to the original state.
resources.gardener.cloud/origin: shoot--test--fraequ01b-7fdfdbfb-7566-4a2e-8b3f-eb09e1a025a7-gardener-soil-test:shoot--pcfgbt--cilium/extension-controlplane-shoot
creationTimestamp: "2022-09-21T07:06:35Z"
generation: 1
labels:
shoot.gardener.cloud/no-cleanup: "true"
name: allow-to-vpn
namespace: firewall
resourceVersion: "878"
uid: 5a966720-59fe-4b1c-aafb-019d7930626b
spec:
egress:
- ports:
- port: 4314
protocol: UDP
- port: 4314
protocol: TCP
to:
- cidr: 0.0.0.0/0
Merged? Almost can't believe it! Thanks for your patience and your willingness to spend so many hours on this, @GrigoriyMikhalkin. ♥
@mwindower, @majst01 almost finished. There's few small things left(refactoring, tests). And one thing to discuss -- DNS over TLS/HTTTPS. At the moment proxy only works with unencrypted udp/tls connections. For encrypted connection, should we add support for TLS or HTTPS or both? If we add this support, how it should work? Unencrypted request comes to proxy and then proxy encrypts it and sends to DNS. Or initial request will be encrypted? In that case we will need to decrypt it and encrypt again to send to DNS.
Also, there question, how TLS configuration should be set by user. Passed via flags to executable or via CRD(with secrets?) in K8s.