From 118b30ed6adce7ac8a40ead0260996228f142b97 Mon Sep 17 00:00:00 2001 From: Stefan Majer Date: Tue, 30 Jan 2024 11:44:55 +0100 Subject: [PATCH] First draft of forbidden mode for isolated clusters (#172) Co-authored-by: Valentin Knabel Co-authored-by: Ulrich Schreiner Co-authored-by: Markus Wennrich Co-authored-by: Gerrit91 --- .github/workflows/build.yaml | 14 +- api/v1/clusterwidenetworkpolicy_types.go | 16 ++ ...l-stack.io_clusterwidenetworkpolicies.yaml | 15 +- .../clusterwidenetworkpolicy_controller.go | 99 +++++++- controllers/firewall_controller.go | 3 +- go.mod | 9 +- go.sum | 18 +- main.go | 1 + pkg/helper/helper.go | 70 ++++++ pkg/helper/helper_test.go | 232 ++++++++++++++++++ pkg/nftables/firewall.go | 10 +- pkg/nftables/ratelimit_test.go | 2 +- pkg/nftables/rendering.go | 14 +- pkg/nftables/service.go | 49 ++-- pkg/nftables/service_test.go | 135 +++++++++- pkg/nftables/snat_test.go | 2 +- 16 files changed, 626 insertions(+), 63 deletions(-) create mode 100644 pkg/helper/helper.go create mode 100644 pkg/helper/helper_test.go diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 48908609..9859f470 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -23,14 +23,14 @@ jobs: steps: - name: Log in to the container registry - uses: docker/login-action@v2 + uses: docker/login-action@v3 with: registry: ${{ env.REGISTRY }} username: ${{ secrets.DOCKER_REGISTRY_USER }} password: ${{ secrets.DOCKER_REGISTRY_TOKEN }} - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - uses: google-github-actions/auth@v1 with: @@ -40,9 +40,10 @@ jobs: uses: google-github-actions/setup-gcloud@v0 - name: Set up Go 1.21 - uses: actions/setup-go@v4 + uses: actions/setup-go@v5 with: go-version: '1.21' + cache: false - name: Lint uses: golangci/golangci-lint-action@v3 @@ -64,7 +65,7 @@ jobs: make - name: Push Docker image - uses: docker/build-push-action@v3 + uses: docker/build-push-action@v5 with: context: . push: true @@ -89,12 +90,13 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Set up Go 1.21 - uses: actions/setup-go@v4 + uses: actions/setup-go@v5 with: go-version: '1.21' + cache: false - name: Run tests run: | diff --git a/api/v1/clusterwidenetworkpolicy_types.go b/api/v1/clusterwidenetworkpolicy_types.go index 421855da..8864db65 100644 --- a/api/v1/clusterwidenetworkpolicy_types.go +++ b/api/v1/clusterwidenetworkpolicy_types.go @@ -27,6 +27,8 @@ const ( // +kubebuilder:object:root=true // +kubebuilder:resource:shortName=cwnp // +kubebuilder:subresource:status +// +kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.state" +// +kubebuilder:printcolumn:name="Message",type="string",JSONPath=".status.message" type ClusterwideNetworkPolicy struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` @@ -67,12 +69,26 @@ type PolicySpec struct { type FQDNState map[string][]IPSet +// PolicyDeploymentState describes the state of a CWNP deployment +type PolicyDeploymentState string + +const ( + // PolicyDeploymentStateDeployed the CWNP was deployed to a native nftable rule + PolicyDeploymentStateDeployed = PolicyDeploymentState("deployed") + // PolicyDeploymentStateIgnored the CWNP was not deployed to a native nftable rule because it is outside of allowed networks + PolicyDeploymentStateIgnored = PolicyDeploymentState("ignored") +) + // PolicyStatus defines the observed state for CWNP resource type PolicyStatus struct { // FQDNState stores mapping from FQDN rules to nftables sets used for a firewall rule. // Key is either MatchName or MatchPattern // +optional FQDNState FQDNState `json:"fqdn_state,omitempty"` + // State of the CWNP, can be either deployed or ignored + State PolicyDeploymentState `json:"state,omitempty"` + // Message describes why the state changed + Message string `json:"message,omitempty"` } // IngressRule describes a particular set of traffic that is allowed to the cluster. diff --git a/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml b/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml index 09a53e39..6ce83c93 100644 --- a/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml +++ b/config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml @@ -16,7 +16,14 @@ spec: singular: clusterwidenetworkpolicy scope: Namespaced versions: - - name: v1 + - additionalPrinterColumns: + - jsonPath: .status.state + name: Status + type: string + - jsonPath: .status.message + name: Message + type: string + name: v1 schema: openAPIV3Schema: description: ClusterwideNetworkPolicy contains the desired state for a cluster @@ -251,6 +258,12 @@ spec: description: FQDNState stores mapping from FQDN rules to nftables sets used for a firewall rule. Key is either MatchName or MatchPattern type: object + message: + description: Message describes why the state changed + type: string + state: + description: State of the CWNP, can be either deployed or ignored + type: string type: object type: object served: true diff --git a/controllers/clusterwidenetworkpolicy_controller.go b/controllers/clusterwidenetworkpolicy_controller.go index 897af31a..b26bc738 100644 --- a/controllers/clusterwidenetworkpolicy_controller.go +++ b/controllers/clusterwidenetworkpolicy_controller.go @@ -5,13 +5,17 @@ import ( "fmt" "time" + "go4.org/netipx" + "github.com/metal-stack/firewall-controller/v2/pkg/dns" + "github.com/metal-stack/firewall-controller/v2/pkg/helper" "github.com/metal-stack/firewall-controller/v2/pkg/nftables" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -33,7 +37,8 @@ type ClusterwideNetworkPolicyReconciler struct { FirewallName string SeedNamespace string - Log logr.Logger + Log logr.Logger + Recorder record.EventRecorder Interval time.Duration DnsProxy *dns.DNSProxy @@ -91,7 +96,14 @@ func (r *ClusterwideNetworkPolicyReconciler) Reconcile(ctx context.Context, _ ct if err := r.ShootClient.List(ctx, &services); err != nil { return ctrl.Result{}, err } - nftablesFirewall := nftables.NewFirewall(f, &cwnps, &services, r.DnsProxy, r.Log) + + validCwnps, err := r.allowedCWNPs(ctx, cwnps.Items, f.Spec.AllowedNetworks) + if err != nil { + return ctrl.Result{}, err + } + cwnps.Items = validCwnps + + nftablesFirewall := nftables.NewFirewall(f, &cwnps, &services, r.DnsProxy, r.Log, r.Recorder) if err := r.manageDNSProxy(ctx, f, cwnps, nftablesFirewall); err != nil { return ctrl.Result{}, err } @@ -103,8 +115,8 @@ func (r *ClusterwideNetworkPolicyReconciler) Reconcile(ctx context.Context, _ ct if updated { for _, i := range cwnps.Items { o := i - if err := r.ShootClient.Status().Update(ctx, &o); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to updated CWNP status: %w", err) + if err := r.updateCWNPState(ctx, o, firewallv1.PolicyDeploymentStateDeployed, ""); err != nil { + return ctrl.Result{}, err } } } @@ -181,3 +193,82 @@ func (r *ClusterwideNetworkPolicyReconciler) getReconciliationTicker(scheduleCha } } } + +func (r *ClusterwideNetworkPolicyReconciler) allowedCWNPs(ctx context.Context, cwnps []firewallv1.ClusterwideNetworkPolicy, allowedNetworks firewallv2.AllowedNetworks) ([]firewallv1.ClusterwideNetworkPolicy, error) { + if len(allowedNetworks.Egress) == 0 && len(allowedNetworks.Ingress) == 0 { + return cwnps, nil + } + + validCWNPs := make([]firewallv1.ClusterwideNetworkPolicy, 0, len(cwnps)) + + egressSet, err := helper.BuildNetworksIPSet(allowedNetworks.Egress) + if err != nil { + return nil, err + } + + ingressSet, err := helper.BuildNetworksIPSet(allowedNetworks.Ingress) + if err != nil { + return nil, err + } + + for _, cwnp := range cwnps { + cwnp := cwnp + oke, err := r.validateCWNPEgressTargetPrefix(cwnp, egressSet) + if err != nil { + return nil, err + } + oki, err := r.validateCWNPIngressTargetPrefix(cwnp, ingressSet) + if err != nil { + return nil, err + } + + if !oki || !oke { + // at least one of ingress and/or egress is not in the allowed network set + if err := r.updateCWNPState(ctx, cwnp, firewallv1.PolicyDeploymentStateIgnored, "ingress/egress does not match allowed networks"); err != nil { + return nil, err + } + continue + } + + validCWNPs = append(validCWNPs, cwnp) + } + + return validCWNPs, nil +} + +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 + + if err := r.ShootClient.Status().Update(ctx, &cwnp); err != nil { + return fmt.Errorf("failed to update status of CWNP %q to %q: %w", cwnp.Name, state, err) + } + return nil +} + +func (r *ClusterwideNetworkPolicyReconciler) validateCWNPEgressTargetPrefix(cwnp firewallv1.ClusterwideNetworkPolicy, ipSet *netipx.IPSet) (bool, error) { + for _, egress := range cwnp.Spec.Egress { + for _, to := range egress.To { + if ok, err := helper.ValidateCIDR(&cwnp, to.CIDR, ipSet, r.Recorder); !ok { + return false, err + } + } + } + return true, nil +} + +func (r *ClusterwideNetworkPolicyReconciler) validateCWNPIngressTargetPrefix(cwnp firewallv1.ClusterwideNetworkPolicy, ipSet *netipx.IPSet) (bool, error) { + for _, ingress := range cwnp.Spec.Ingress { + for _, from := range ingress.From { + if ok, err := helper.ValidateCIDR(&cwnp, from.CIDR, ipSet, r.Recorder); !ok { + return false, err + } + } + } + return true, nil +} diff --git a/controllers/firewall_controller.go b/controllers/firewall_controller.go index b9acad66..deaf365d 100644 --- a/controllers/firewall_controller.go +++ b/controllers/firewall_controller.go @@ -86,7 +86,8 @@ func (r *FirewallReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c if apierrors.IsNotFound(err) { r.Log.Info("flushing k8s firewall rules") - defaultFw := nftables.NewDefaultFirewall() + defaultFw := nftables.NewFirewall(&firewallv2.Firewall{}, &firewallv1.ClusterwideNetworkPolicyList{}, &corev1.ServiceList{}, nil, logr.Discard(), r.Recorder) + flushErr := defaultFw.Flush() if flushErr != nil { r.Log.Error(flushErr, "error flushing k8s firewall rules") diff --git a/go.mod b/go.mod index b054b1ba..c9ba840f 100644 --- a/go.mod +++ b/go.mod @@ -10,14 +10,15 @@ require ( github.com/google/go-cmp v0.6.0 github.com/google/nftables v0.1.1-0.20230115205135-9aa6fdf5a28c github.com/ks2211/go-suricata v0.0.0-20200823200910-986ce1470707 - github.com/metal-stack/firewall-controller-manager v0.3.0 + github.com/metal-stack/firewall-controller-manager v0.3.2 github.com/metal-stack/metal-go v0.26.2 github.com/metal-stack/metal-lib v0.14.3 github.com/metal-stack/metal-networker v0.41.0 github.com/metal-stack/v v1.0.3 - github.com/miekg/dns v1.1.56 - github.com/txn2/txeh v1.5.3 + github.com/miekg/dns v1.1.58 + github.com/txn2/txeh v1.5.5 github.com/vishvananda/netlink v1.2.1-beta.2 + go4.org/netipx v0.0.0-20231129151722-fdeea329fbba k8s.io/api v0.26.3 k8s.io/apiextensions-apiserver v0.26.3 k8s.io/apimachinery v0.28.2 @@ -83,7 +84,7 @@ require ( golang.org/x/term v0.16.0 // indirect golang.org/x/text v0.14.0 // indirect golang.org/x/time v0.5.0 // indirect - golang.org/x/tools v0.16.0 // indirect + golang.org/x/tools v0.17.0 // indirect gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect google.golang.org/appengine v1.6.8 // indirect google.golang.org/protobuf v1.32.0 // indirect diff --git a/go.sum b/go.sum index 575c5632..69b28cd3 100644 --- a/go.sum +++ b/go.sum @@ -172,8 +172,8 @@ github.com/mdlayher/netlink v1.7.2 h1:/UtM3ofJap7Vl4QWCPDGXY8d3GIY2UGSDbK+QWmY8/ github.com/mdlayher/netlink v1.7.2/go.mod h1:xraEF7uJbxLhc5fpHL4cPe221LI2bdttWlU+ZGLfQSw= github.com/mdlayher/socket v0.5.0 h1:ilICZmJcQz70vrWVes1MFera4jGiWNocSkykwwoy3XI= github.com/mdlayher/socket v0.5.0/go.mod h1:WkcBFfvyG8QENs5+hfQPl1X6Jpd2yeLIYgrGFmJiJxI= -github.com/metal-stack/firewall-controller-manager v0.3.0 h1:sBCL7iiG17ZO/1TREv2RYiMdX5VddSc92snR8OKcAF8= -github.com/metal-stack/firewall-controller-manager v0.3.0/go.mod h1:KjLZv/BatucZM9DQtBLN04wBGjvxEJRV1C+xDkCWwIE= +github.com/metal-stack/firewall-controller-manager v0.3.2 h1:SXamSfDzEZgXGLf4EAobVnBDtTq/QauXcggiifA5gHg= +github.com/metal-stack/firewall-controller-manager v0.3.2/go.mod h1:KjLZv/BatucZM9DQtBLN04wBGjvxEJRV1C+xDkCWwIE= github.com/metal-stack/metal-go v0.26.2 h1:KZRV1wtCsj0dMo4GpW2+XemmAkPZAYFjbGe7QhhcH1k= github.com/metal-stack/metal-go v0.26.2/go.mod h1:olJ3Az7RBh39Q5WFCJOQBd7cJi0xgGYwMTEIFvkDQQY= github.com/metal-stack/metal-hammer v0.12.0 h1:t6t73RGmDU1IFkHC7dJxu7xDIZZvwmqmu9/0xZVF/L0= @@ -184,8 +184,8 @@ github.com/metal-stack/metal-networker v0.41.0 h1:eefp8nzhF6eBDoGjFR8m+chkGUO5QF github.com/metal-stack/metal-networker v0.41.0/go.mod h1:jdHKFIbPBNHnvies0Tb8DlnPfbKV/HuikxPZH3kC6uA= 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.56 h1:5imZaSeoRNvpM9SzWNhEcP9QliKiz20/dA2QabIGVnE= -github.com/miekg/dns v1.1.56/go.mod h1:cRm6Oo2C8TY9ZS/TqsSrseAcncm74lfK5G+ikN2SWWY= +github.com/miekg/dns v1.1.58 h1:ca2Hdkz+cDg/7eNF6V56jjzuZ4aCAE+DbVkILdQWG/4= +github.com/miekg/dns v1.1.58/go.mod h1:Ypv+3b/KadlvW9vJfXOTf300O4UqaHFzFCuHz+rPkBY= github.com/mitchellh/copystructure v1.2.0 h1:vpKXTN4ewci03Vljg/q9QvCGUDttBOGBIa15WveJJGw= github.com/mitchellh/copystructure v1.2.0/go.mod h1:qLl+cE2AmVv+CoeAwDPye/v+N2HKCj9FbZEVFJRxO9s= github.com/mitchellh/mapstructure v1.5.0 h1:jeMsZIYE/09sWLaz43PL7Gy6RuMjD2eJVyuac5Z2hdY= @@ -259,8 +259,8 @@ github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcU github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8= github.com/subosito/gotenv v1.6.0/go.mod h1:Dk4QP5c2W3ibzajGcXpNraDfq2IrhjMIvMSWPKKo0FU= -github.com/txn2/txeh v1.5.3 h1:ZMgc3r+5/AFtE/ayCoICpvxj7xl/CYsZjnIGhozV/Kc= -github.com/txn2/txeh v1.5.3/go.mod h1:qYzGG9kCzeVEI12geK4IlanHWY8X4uy/I3NcW7mk8g4= +github.com/txn2/txeh v1.5.5 h1:UN4e/lCK5HGw/gGAi2GCVrNKg0GTCUWs7gs5riaZlz4= +github.com/txn2/txeh v1.5.5/go.mod h1:qYzGG9kCzeVEI12geK4IlanHWY8X4uy/I3NcW7mk8g4= github.com/vishvananda/netlink v1.2.1-beta.2 h1:Llsql0lnQEbHj0I1OuKyp8otXp0r3q0mPkuhwHfStVs= github.com/vishvananda/netlink v1.2.1-beta.2/go.mod h1:twkDnbuQxJYemMlGd4JFIcuhgX83tXhKS2B/PRMpOho= github.com/vishvananda/netns v0.0.0-20200728191858-db3c7e526aae/go.mod h1:DD4vA1DwXk04H54A1oHXtwZmA0grkVMdPxx/VGLCah0= @@ -286,6 +286,8 @@ go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= go.uber.org/zap v1.26.0 h1:sI7k6L95XOKS281NhVKOFCUNIvv9e0w4BF8N3u+tCRo= go.uber.org/zap v1.26.0/go.mod h1:dtElttAiwGvoJ/vj4IwHBS/gXsEu/pZ50mUIRWuG0so= +go4.org/netipx v0.0.0-20231129151722-fdeea329fbba h1:0b9z3AuHCjxk0x/opv64kcgZLBseWJUpBw5I82+2U4M= +go4.org/netipx v0.0.0-20231129151722-fdeea329fbba/go.mod h1:PLyyIXexvUFg3Owu6p/WfdlivPbZJsZdgWZlrGope/Y= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= @@ -382,8 +384,8 @@ golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roY golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= -golang.org/x/tools v0.16.0 h1:GO788SKMRunPIBCXiQyo2AaexLstOrVhuAL5YwsckQM= -golang.org/x/tools v0.16.0/go.mod h1:kYVVN6I1mBNoB1OX+noeBjbRk4IUEPa7JJ+TJMEooJ0= +golang.org/x/tools v0.17.0 h1:FvmRgNOcs3kOa+T20R1uhfP9F6HgG2mfxDv1vrx1Htc= +golang.org/x/tools v0.17.0/go.mod h1:xsh6VxdV005rRVaS6SSAf9oiAqljS7UZUacMZ8Bnsps= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/main.go b/main.go index a33c829c..33214ba7 100644 --- a/main.go +++ b/main.go @@ -241,6 +241,7 @@ func main() { SeedClient: seedMgr.GetClient(), ShootClient: shootMgr.GetClient(), Log: ctrl.Log.WithName("controllers").WithName("ClusterwideNetworkPolicy"), + Recorder: shootMgr.GetEventRecorderFor("FirewallController"), FirewallName: firewallName, SeedNamespace: seedNamespace, }).SetupWithManager(shootMgr); err != nil { diff --git a/pkg/helper/helper.go b/pkg/helper/helper.go new file mode 100644 index 00000000..90e774c6 --- /dev/null +++ b/pkg/helper/helper.go @@ -0,0 +1,70 @@ +package helper + +import ( + "fmt" + "net/netip" + + "go4.org/netipx" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/record" +) + +const ( + forbiddenCIDR = "ForbiddenCIDR" +) + +// Create an IPSet from a given list of strings describing networks. +func BuildNetworksIPSet(networks []string) (*netipx.IPSet, error) { + var externalBuilder netipx.IPSetBuilder + + for _, externalNetwork := range networks { + parsedExternal, err := netip.ParsePrefix(externalNetwork) + if err != nil { + return nil, fmt.Errorf("failed to parse prefix: %w", err) + } + externalBuilder.AddPrefix(parsedExternal) + } + externalSet, err := externalBuilder.IPSet() + if err != nil { + return nil, fmt.Errorf("failed to create ipset: %w", err) + } + return externalSet, nil +} + +func NetworkSetAsString(externalSet *netipx.IPSet) string { + var allowedNetworksStr string + if externalSet != nil { + for i, r := range externalSet.Ranges() { + if i > 0 { + allowedNetworksStr += "," + } + if p, ok := r.Prefix(); ok { + allowedNetworksStr += p.String() + } else { + allowedNetworksStr += r.String() + } + } + } + return allowedNetworksStr +} + +func ValidateCIDR(o runtime.Object, cidr string, ipset *netipx.IPSet, rec record.EventRecorder) (bool, error) { + parsedTo, err := netip.ParsePrefix(cidr) + if err != nil { + return false, fmt.Errorf("failed to parse to address: %w", err) + } + if !ipset.ContainsPrefix(parsedTo) { + allowedNetworksStr := NetworkSetAsString(ipset) + if rec != nil { + rec.Eventf( + o, + corev1.EventTypeWarning, + forbiddenCIDR, + "address:%q is outside of the allowed network range:%q, ignoring", + parsedTo.String(), allowedNetworksStr) + } + return false, nil + } + return true, nil +} diff --git a/pkg/helper/helper_test.go b/pkg/helper/helper_test.go new file mode 100644 index 00000000..f94befa4 --- /dev/null +++ b/pkg/helper/helper_test.go @@ -0,0 +1,232 @@ +package helper + +import ( + "net/netip" + "testing" + + "go4.org/netipx" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +func helpMustParseIPSet(ips []string) *netipx.IPSet { + res, _ := BuildNetworksIPSet(ips) + return res +} + +func TestBuildNetworksIPSet(t *testing.T) { + type args struct { + networks []string + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "legal values", + args: args{ + networks: []string{ + "192.168.0.0/16", + "1.2.3.4/24", + }, + }, + wantErr: false, + }, + { + name: "overlapping values", + args: args{ + networks: []string{ + "192.168.0.0/16", + "192.168.1.0/8", + }, + }, + wantErr: false, + }, + { + name: "illegal IP", + args: args{ + networks: []string{ + "292.168.0.0/16", + "192.168.1.0/8", + }, + }, + wantErr: true, + }, + { + name: "illegal mask", + args: args{ + networks: []string{ + "192.168.0.0/33", + "192.168.1.0/8", + }, + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := BuildNetworksIPSet(tt.args.networks) + if (err != nil) != tt.wantErr { + t.Errorf("BuildNetworksIPSet() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != nil { + for _, n := range tt.args.networks { + p, _ := netip.ParsePrefix(n) + if !got.ContainsPrefix(p) { + t.Errorf("BuildNetworksIPSet() = does not contain %v", p) + } + } + } + }) + } +} + +func TestNetworkSetAsString(t *testing.T) { + type args struct { + externalSet *netipx.IPSet + } + tests := []struct { + name string + args args + want string + }{ + { + name: "list of overlapping ranges", + args: args{ + externalSet: helpMustParseIPSet([]string{"192.168.0.1/8", "192.168.1.1/16"}), + }, + want: "192.0.0.0/8", + }, + { + name: "list of non overlapping ranges", + args: args{ + externalSet: helpMustParseIPSet([]string{"192.168.1.1/24", "192.168.2.1/24"}), + }, + want: "192.168.1.0-192.168.2.255", + }, + { + name: "list of ranges", + args: args{ + externalSet: helpMustParseIPSet([]string{"192.168.1.1/24", "193.168.2.1/24"}), + }, + want: "192.168.1.0/24,193.168.2.0/24", + }, + { + name: "list of ranges with single IP-cidr", + args: args{ + externalSet: helpMustParseIPSet([]string{"192.168.1.1/24", "193.168.2.1/24", "1.2.3.4/32"}), + }, + want: "1.2.3.4/32,192.168.1.0/24,193.168.2.0/24", + }, + { + name: "empty input", + args: args{ + externalSet: nil, + }, + want: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := NetworkSetAsString(tt.args.externalSet); got != tt.want { + t.Errorf("NetworkSetAsString() = %v, want %v", got, tt.want) + } + }) + } +} + +type mockRecorder struct { + evType string + evReason string + tpreason string + ok bool + invoke bool +} + +func (mr *mockRecorder) Event(object runtime.Object, eventtype, reason, message string) { + mr.invoke = true + mr.tpreason = eventtype + "," + reason + mr.ok = eventtype == mr.evType && reason == mr.evReason +} +func (mr *mockRecorder) Eventf(object runtime.Object, eventtype, reason, messageFmt string, args ...interface{}) { + mr.invoke = true + mr.tpreason = eventtype + "," + reason + mr.ok = eventtype == mr.evType && reason == mr.evReason +} +func (mr *mockRecorder) AnnotatedEventf(object runtime.Object, annotations map[string]string, eventtype, reason, messageFmt string, args ...interface{}) { + mr.invoke = true + mr.tpreason = eventtype + "," + reason + mr.ok = eventtype == mr.evType && reason == mr.evReason +} + +func TestValidateCIDR(t *testing.T) { + type args struct { + o runtime.Object + cidr string + ipset *netipx.IPSet + } + tests := []struct { + name string + args args + want bool + wantRecordEvent bool + wantErr bool + }{ + { + name: "cidr in ipset", + args: args{ + o: nil, + cidr: "192.168.0.6/24", + ipset: helpMustParseIPSet([]string{"192.168.0.0/16"}), + }, + want: true, + wantRecordEvent: false, + wantErr: false, + }, + { + name: "cidr not in ipset", + args: args{ + o: nil, + cidr: "193.168.0.6/24", + ipset: helpMustParseIPSet([]string{"192.168.0.0/16"}), + }, + want: false, + wantRecordEvent: true, + wantErr: false, + }, + { + name: "illegal cidr value", + args: args{ + o: nil, + cidr: "293.168.0.6/24", + ipset: helpMustParseIPSet([]string{"192.168.0.0/16"}), + }, + want: false, + wantRecordEvent: false, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rec := mockRecorder{evType: corev1.EventTypeWarning, evReason: forbiddenCIDR} + + got, err := ValidateCIDR(tt.args.o, tt.args.cidr, tt.args.ipset, &rec) + if (err != nil) != tt.wantErr { + t.Errorf("ValidateCIDR() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("ValidateCIDR() = %v, want %v", got, tt.want) + } + if tt.wantRecordEvent != rec.invoke { + t.Errorf("ValidateCIDR() log event = %v, wanted log event %v", rec.invoke, tt.wantRecordEvent) + } + if tt.wantRecordEvent && !rec.ok { + t.Errorf("ValidateCIDR() did log wrong type/reason %q", rec.tpreason) + } + + }) + } +} diff --git a/pkg/nftables/firewall.go b/pkg/nftables/firewall.go index 5470be84..19e4ac83 100644 --- a/pkg/nftables/firewall.go +++ b/pkg/nftables/firewall.go @@ -16,6 +16,7 @@ import ( "github.com/vishvananda/netlink" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/client-go/tools/record" mn "github.com/metal-stack/metal-lib/pkg/net" "github.com/metal-stack/metal-networker/pkg/netconf" @@ -45,6 +46,8 @@ type FQDNCache interface { type Firewall struct { log logr.Logger + recorder record.EventRecorder + firewall *firewallv2.Firewall clusterwideNetworkPolicies *firewallv1.ClusterwideNetworkPolicyList services *corev1.ServiceList @@ -67,11 +70,6 @@ type forwardingRules struct { Egress nftablesRules } -// NewDefaultFirewall creates a new default nftables firewall. -func NewDefaultFirewall() *Firewall { - return NewFirewall(&firewallv2.Firewall{}, &firewallv1.ClusterwideNetworkPolicyList{}, &corev1.ServiceList{}, nil, logr.Discard()) -} - // NewFirewall creates a new nftables firewall object based on k8s entities func NewFirewall( firewall *firewallv2.Firewall, @@ -79,6 +77,7 @@ func NewFirewall( svcs *corev1.ServiceList, cache FQDNCache, log logr.Logger, + recorder record.EventRecorder, ) *Firewall { networkMap := networkMap{} var primaryPrivateNet *firewallv2.FirewallNetwork @@ -103,6 +102,7 @@ func NewFirewall( cache: cache, enableDNS: len(cwnps.GetFQDNs()) > 0, log: log, + recorder: recorder, } } diff --git a/pkg/nftables/ratelimit_test.go b/pkg/nftables/ratelimit_test.go index 9a15e215..c7125cbc 100644 --- a/pkg/nftables/ratelimit_test.go +++ b/pkg/nftables/ratelimit_test.go @@ -81,7 +81,7 @@ func TestRateLimitRules(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - f := NewFirewall(&firewallv2.Firewall{Spec: tt.input.Spec, Status: tt.input.Status}, &firewallv1.ClusterwideNetworkPolicyList{}, nil, nil, logr.Discard()) + f := NewFirewall(&firewallv2.Firewall{Spec: tt.input.Spec, Status: tt.input.Status}, &firewallv1.ClusterwideNetworkPolicyList{}, nil, nil, logr.Discard(), nil) got := rateLimitRules(f) if !cmp.Equal(got, tt.want) { t.Errorf("rateLimitRules() diff: %v", cmp.Diff(got, tt.want)) diff --git a/pkg/nftables/rendering.go b/pkg/nftables/rendering.go index 6b428c32..00945720 100644 --- a/pkg/nftables/rendering.go +++ b/pkg/nftables/rendering.go @@ -9,6 +9,8 @@ import ( "text/template" "github.com/metal-stack/firewall-controller/v2/pkg/dns" + "github.com/metal-stack/firewall-controller/v2/pkg/helper" + "go4.org/netipx" ) // firewallRenderingData holds the data available in the nftables template @@ -35,8 +37,18 @@ func newFirewallRenderingData(f *Firewall) (*firewallRenderingData, error) { f.clusterwideNetworkPolicies.Items[ind] = u } + var serviceAllowedSet *netipx.IPSet + if len(f.firewall.Spec.AllowedNetworks.Ingress) > 0 { + // the ips for services are only checked if the accesstype is forbidden + a, err := helper.BuildNetworksIPSet(f.firewall.Spec.AllowedNetworks.Ingress) + if err != nil { + return nil, err + } + serviceAllowedSet = a + } + for _, svc := range f.services.Items { - ingress = append(ingress, serviceRules(svc, f.logAcceptedConnections)...) + ingress = append(ingress, serviceRules(svc, serviceAllowedSet, f.logAcceptedConnections, f.recorder)...) } snatRules, err := snatRules(f) diff --git a/pkg/nftables/service.go b/pkg/nftables/service.go index b9f94d63..c4c6dfb5 100644 --- a/pkg/nftables/service.go +++ b/pkg/nftables/service.go @@ -2,47 +2,28 @@ package nftables import ( "fmt" - "net" + "net/netip" "strings" + "github.com/metal-stack/firewall-controller/v2/pkg/helper" + "go4.org/netipx" corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/record" ) -func isCIDR(cidr string) bool { - _, _, err := net.ParseCIDR(cidr) - return err != nil -} - -func isIP(ip string) bool { - i := net.ParseIP(ip) - return i != nil -} - // serviceRules generates nftables rules base on a k8s service definition -func serviceRules(svc corev1.Service, logAcceptedConnections bool) nftablesRules { +func serviceRules(svc corev1.Service, allowed *netipx.IPSet, logAcceptedConnections bool, recorder record.EventRecorder) nftablesRules { if svc.Spec.Type != corev1.ServiceTypeLoadBalancer && svc.Spec.Type != corev1.ServiceTypeNodePort { return nil } from := []string{} - for _, lbsr := range svc.Spec.LoadBalancerSourceRanges { - if !isCIDR(lbsr) && !isIP(lbsr) { - continue - } - } - from = append(from, svc.Spec.LoadBalancerSourceRanges...) to := []string{} if svc.Spec.Type == corev1.ServiceTypeLoadBalancer { - if svc.Spec.LoadBalancerIP != "" { - if isIP(svc.Spec.LoadBalancerIP) { - to = append(to, svc.Spec.LoadBalancerIP) - } - } + to = appendServiceIP(to, svc, allowed, svc.Spec.LoadBalancerIP, recorder) for _, e := range svc.Status.LoadBalancer.Ingress { - if isIP(e.IP) { - to = append(to, e.IP) - } + to = appendServiceIP(to, svc, allowed, e.IP, recorder) } } @@ -81,3 +62,19 @@ func serviceRules(svc corev1.Service, logAcceptedConnections bool) nftablesRules } return uniqueSorted(rules) } + +func appendServiceIP(to []string, svc corev1.Service, allowed *netipx.IPSet, ip string, recorder record.EventRecorder) []string { + _, err := netip.ParseAddr(ip) + if err != nil { + return to + } + if allowed == nil { + return append(to, ip) + } + + // if there is an allowed-ipset restriction, we check if the given IP is contained in this set + if ok, _ := helper.ValidateCIDR(&svc, ip+"/32", allowed, recorder); ok { + to = append(to, ip) + } + return to +} diff --git a/pkg/nftables/service_test.go b/pkg/nftables/service_test.go index 589df273..9dcae4e5 100644 --- a/pkg/nftables/service_test.go +++ b/pkg/nftables/service_test.go @@ -4,10 +4,17 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/metal-stack/firewall-controller/v2/pkg/helper" + "go4.org/netipx" corev1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +func helpMustParseIPSet(ips []string) *netipx.IPSet { + res, _ := helper.BuildNetworksIPSet(ips) + return res +} + func TestServiceRules(t *testing.T) { type want struct { ingress nftablesRules @@ -15,9 +22,10 @@ func TestServiceRules(t *testing.T) { } tests := []struct { - name string - input corev1.Service - want want + name string + input corev1.Service + allowed *netipx.IPSet + want want }{ { name: "standard service type loadbalancer with restricted source IP range", @@ -89,15 +97,132 @@ func TestServiceRules(t *testing.T) { }, want: want{nil, nil}, }, + { + name: "standard service type loadbalancer with a non matching allowed IP set", + allowed: helpMustParseIPSet([]string{"182.0.0.0/8"}), + input: corev1.Service{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "test", + Name: "svc", + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeLoadBalancer, + Ports: []corev1.ServicePort{ + { + Port: 443, + TargetPort: *port(30443), + Protocol: corev1.ProtocolTCP, + }, + }, + LoadBalancerSourceRanges: []string{"185.0.0.0/16", "185.1.0.0/16"}, + }, + Status: corev1.ServiceStatus{ + LoadBalancer: corev1.LoadBalancerStatus{ + Ingress: []corev1.LoadBalancerIngress{ + { + IP: "185.0.0.1", + }, + }, + }, + }, + }, + want: want{ + ingress: nftablesRules{ + `ip saddr { 185.0.0.0/16, 185.1.0.0/16 } tcp dport { 443 } counter accept comment "accept traffic for k8s service test/svc"`, + }, + ingressAL: nftablesRules{ + `ip saddr { 185.0.0.0/16, 185.1.0.0/16 } tcp dport { 443 } log prefix "nftables-firewall-accepted: " limit rate 10/second`, + `ip saddr { 185.0.0.0/16, 185.1.0.0/16 } tcp dport { 443 } counter accept comment "accept traffic for k8s service test/svc"`, + }, + }, + }, + { + name: "standard service type loadbalancer with restricted source IP range, allow loadbalancer and status-ingress IP", + allowed: helpMustParseIPSet([]string{"185.0.1.0/30"}), + input: corev1.Service{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "test", + Name: "svc", + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeLoadBalancer, + Ports: []corev1.ServicePort{ + { + Port: 443, + TargetPort: *port(30443), + Protocol: corev1.ProtocolTCP, + }, + }, + LoadBalancerIP: "185.0.1.2", + }, + Status: corev1.ServiceStatus{ + LoadBalancer: corev1.LoadBalancerStatus{ + Ingress: []corev1.LoadBalancerIngress{ + { + IP: "185.0.1.1", + }, + }, + }, + }, + }, + want: want{ + ingress: nftablesRules{ + `ip daddr { 185.0.1.2, 185.0.1.1 } tcp dport { 443 } counter accept comment "accept traffic for k8s service test/svc"`, + }, + ingressAL: nftablesRules{ + `ip daddr { 185.0.1.2, 185.0.1.1 } tcp dport { 443 } log prefix "nftables-firewall-accepted: " limit rate 10/second`, + `ip daddr { 185.0.1.2, 185.0.1.1 } tcp dport { 443 } counter accept comment "accept traffic for k8s service test/svc"`, + }, + }, + }, + { + name: "standard service type loadbalancer with restricted source IP range, filter out loadbalancer-IP", + allowed: helpMustParseIPSet([]string{"185.0.1.0/31"}), + input: corev1.Service{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "test", + Name: "svc", + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeLoadBalancer, + Ports: []corev1.ServicePort{ + { + Port: 443, + TargetPort: *port(30443), + Protocol: corev1.ProtocolTCP, + }, + }, + LoadBalancerIP: "185.0.1.2", + }, + Status: corev1.ServiceStatus{ + LoadBalancer: corev1.LoadBalancerStatus{ + Ingress: []corev1.LoadBalancerIngress{ + { + IP: "185.0.1.1", + }, + }, + }, + }, + }, + want: want{ + ingress: nftablesRules{ + `ip daddr { 185.0.1.1 } tcp dport { 443 } counter accept comment "accept traffic for k8s service test/svc"`, + }, + ingressAL: nftablesRules{ + `ip daddr { 185.0.1.1 } tcp dport { 443 } log prefix "nftables-firewall-accepted: " limit rate 10/second`, + `ip daddr { 185.0.1.1 } tcp dport { 443 } counter accept comment "accept traffic for k8s service test/svc"`, + }, + }, + }, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - ingress := serviceRules(tt.input, false) + ingress := serviceRules(tt.input, tt.allowed, false, nil) if !cmp.Equal(ingress, tt.want.ingress) { t.Errorf("serviceRules() diff: %v", cmp.Diff(ingress, tt.want.ingress)) } - ingressAL := serviceRules(tt.input, true) + ingressAL := serviceRules(tt.input, tt.allowed, true, nil) if !cmp.Equal(ingressAL, tt.want.ingressAL) { t.Errorf("serviceRules() diff: %v", cmp.Diff(ingressAL, tt.want.ingressAL)) } diff --git a/pkg/nftables/snat_test.go b/pkg/nftables/snat_test.go index 003bd936..3f1a0eed 100644 --- a/pkg/nftables/snat_test.go +++ b/pkg/nftables/snat_test.go @@ -198,7 +198,7 @@ func TestSnatRules(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - f := NewFirewall(&firewallv2.Firewall{Spec: tt.input.Spec, Status: tt.input.Status}, &tt.cwnps, nil, nil, logr.Discard()) + f := NewFirewall(&firewallv2.Firewall{Spec: tt.input.Spec, Status: tt.input.Status}, &tt.cwnps, nil, nil, logr.Discard(), nil) got, err := snatRules(f) if (err != nil) != tt.wantErr { t.Errorf("snatRules() error = %v, wantErr %v", err, tt.err)