diff --git a/internal/testutil/fixture/azure_securitygroup.go b/internal/testutil/fixture/azure_securitygroup.go index a979fa3646..fcfce0bcf9 100644 --- a/internal/testutil/fixture/azure_securitygroup.go +++ b/internal/testutil/fixture/azure_securitygroup.go @@ -163,6 +163,41 @@ func (f *AzureFixture) DenyAllSecurityRule(ipFamily iputil.Family) *AzureDenyAll } } +func (f *AzureFixture) DenyBlockedIPRangeSecurityRule( + protocol armnetwork.SecurityRuleProtocol, + ipFamily iputil.Family, + srcPrefixes []string, + dstPorts []int32, +) *AzureDenyBlockedIPRangeSecurityRuleFixture { + name := securitygroup.GenerateDenyBlockedSecurityRuleName(protocol, ipFamily, srcPrefixes, dstPorts) + dstPortRanges := fnutil.Map(func(p int32) string { return strconv.FormatInt(int64(p), 10) }, dstPorts) + sort.Strings(dstPortRanges) + + rule := &armnetwork.SecurityRule{ + Name: ptr.To(name), + Properties: &armnetwork.SecurityRulePropertiesFormat{ + Protocol: to.Ptr(protocol), + Access: to.Ptr(armnetwork.SecurityRuleAccessDeny), + Direction: to.Ptr(armnetwork.SecurityRuleDirectionInbound), + SourcePortRange: ptr.To("*"), + DestinationPortRanges: to.SliceOfPtrs(dstPortRanges...), + Priority: ptr.To(int32(consts.IPPrefixBlockingMinimumPriority)), + }, + } + if len(dstPorts) == 0 { + rule.Properties.DestinationPortRange = ptr.To("*") + rule.Properties.DestinationPortRanges = nil + } + + if len(srcPrefixes) == 1 { + rule.Properties.SourceAddressPrefix = ptr.To(srcPrefixes[0]) + } else { + rule.Properties.SourceAddressPrefixes = to.SliceOfPtrs(srcPrefixes...) + } + + return &AzureDenyBlockedIPRangeSecurityRuleFixture{rule: rule} +} + // AzureSecurityGroupFixture is a fixture for an Azure security group. type AzureSecurityGroupFixture struct { sg *armnetwork.SecurityGroup @@ -231,3 +266,26 @@ func (f *AzureDenyAllSecurityRuleFixture) WithDestination(prefixes ...string) *A func (f *AzureDenyAllSecurityRuleFixture) Build() *armnetwork.SecurityRule { return f.rule } + +// AzureDenyBlockedIPRangeSecurityRuleFixture is a fixture for a deny blocked IP range security rule. +type AzureDenyBlockedIPRangeSecurityRuleFixture struct { + rule *armnetwork.SecurityRule +} + +func (f *AzureDenyBlockedIPRangeSecurityRuleFixture) WithPriority(p int32) *AzureDenyBlockedIPRangeSecurityRuleFixture { + f.rule.Properties.Priority = ptr.To(p) + return f +} + +func (f *AzureDenyBlockedIPRangeSecurityRuleFixture) WithDestination(prefixes ...string) *AzureDenyBlockedIPRangeSecurityRuleFixture { + if len(prefixes) == 1 { + f.rule.Properties.DestinationAddressPrefix = ptr.To(prefixes[0]) + f.rule.Properties.DestinationAddressPrefixes = nil + } else if len(prefixes) > 1 { + f.rule.Properties.DestinationAddressPrefix = nil + f.rule.Properties.DestinationAddressPrefixes = to.SliceOfPtrs(securitygroup.NormalizeSecurityRuleAddressPrefixes(prefixes)...) + } + return f +} + +func (f *AzureDenyBlockedIPRangeSecurityRuleFixture) Build() *armnetwork.SecurityRule { return f.rule } diff --git a/internal/testutil/fixture/kubernetes.go b/internal/testutil/fixture/kubernetes.go index 1a446c04b6..1b40c3abd5 100644 --- a/internal/testutil/fixture/kubernetes.go +++ b/internal/testutil/fixture/kubernetes.go @@ -96,6 +96,11 @@ func (f *KubernetesServiceFixture) WithAllowedIPRanges(parts ...string) *Kuberne return f } +func (f *KubernetesServiceFixture) WithBlockedIPRanges(parts ...string) *KubernetesServiceFixture { + f.svc.Annotations[consts.ServiceAnnotationBlockedIPRanges] = strings.Join(parts, ",") + return f +} + func (f *KubernetesServiceFixture) WithAllowedServiceTags(parts ...string) *KubernetesServiceFixture { f.svc.Annotations[consts.ServiceAnnotationAllowedServiceTags] = strings.Join(parts, ",") return f diff --git a/pkg/consts/consts.go b/pkg/consts/consts.go index 66490f08ad..6455b5a81d 100644 --- a/pkg/consts/consts.go +++ b/pkg/consts/consts.go @@ -289,6 +289,11 @@ const ( // It is compatible with both IPv4 and IPV6 CIDR formats. ServiceAnnotationAllowedIPRanges = "service.beta.kubernetes.io/azure-allowed-ip-ranges" + // ServiceAnnotationBlockedIPRanges is the annotation used on the service + // to specify a list of blocked IP Ranges separated by comma. + // It is compatible with both IPv4 and IPV6 CIDR formats. + ServiceAnnotationBlockedIPRanges = "service.beta.kubernetes.io/azure-blocked-ip-ranges" + // ServiceAnnotationDenyAllExceptLoadBalancerSourceRanges denies all traffic to the load balancer except those // within the service.Spec.LoadBalancerSourceRanges. Ref: https://github.com/kubernetes-sigs/cloud-provider-azure/issues/374. ServiceAnnotationDenyAllExceptLoadBalancerSourceRanges = "service.beta.kubernetes.io/azure-deny-all-except-load-balancer-source-ranges" @@ -355,6 +360,11 @@ const ( // TrueAnnotationValue is the true annotation value TrueAnnotationValue = "true" + // IP prefix blocking range minimum priority + IPPrefixBlockingMinimumPriority = 400 + // IP prefix blocking range maximum priority + IPPrefixBlockingMaximumPriority = 499 + // LoadBalancerMinimumPriority is the minimum priority LoadBalancerMinimumPriority = 500 // LoadBalancerMaximumPriority is the maximum priority diff --git a/pkg/provider/loadbalancer/accesscontrol.go b/pkg/provider/loadbalancer/accesscontrol.go index e95ab54f68..ac52fbdd4b 100644 --- a/pkg/provider/loadbalancer/accesscontrol.go +++ b/pkg/provider/loadbalancer/accesscontrol.go @@ -19,6 +19,7 @@ package loadbalancer import ( "fmt" "net/netip" + "slices" "strings" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v6" @@ -46,6 +47,7 @@ type AccessControl struct { // immutable pre-compute states. SourceRanges []netip.Prefix AllowedIPRanges []netip.Prefix + BlockedIPRanges []netip.Prefix AllowedServiceTags []string invalidRanges []string securityRuleDestinationPortsByProtocol map[armnetwork.SecurityRuleProtocol][]int32 @@ -95,6 +97,13 @@ func NewAccessControl(logger logr.Logger, svc *v1.Service, sg *armnetwork.Securi // Backward compatibility: no error but emit a warning event. eventEmitter(svc, v1.EventTypeWarning, "InvalidAllowedIPRanges", EventMessageOfInvalidAllowedIPRanges(invalidAllowedIPRanges)) } + blockedIPRanges, invalidBlockedIPRanges, err := BlockedIPRanges(svc) + if err != nil { + logger.Error(err, "Failed to parse BlockedIPRanges configuration") + + // Backward compatibility: no error but emit a warning event. + eventEmitter(svc, v1.EventTypeWarning, "InvalidBlockedIPRanges", EventMessageOfInvalidBlockedIPRanges(invalidBlockedIPRanges)) + } allowedServiceTags := AllowedServiceTags(svc) securityRuleDestinationPortsByProtocol, err := SecurityRuleDestinationPortsByProtocol(svc) if err != nil { @@ -115,14 +124,17 @@ func NewAccessControl(logger logr.Logger, svc *v1.Service, sg *armnetwork.Securi eventEmitter(svc, v1.EventTypeWarning, "ConflictConfiguration", EventMessageOfConflictLoadBalancerSourceRangesAndAllowedIPRanges()) } + invalidRanges := slices.Concat(invalidSourceRanges, invalidAllowedIPRanges) + return &AccessControl{ logger: logger, svc: svc, sgHelper: sgHelper, SourceRanges: sourceRanges, AllowedIPRanges: allowedIPRanges, + BlockedIPRanges: blockedIPRanges, AllowedServiceTags: allowedServiceTags, - invalidRanges: append(invalidSourceRanges, invalidAllowedIPRanges...), + invalidRanges: invalidRanges, securityRuleDestinationPortsByProtocol: securityRuleDestinationPortsByProtocol, }, nil } @@ -233,11 +245,34 @@ func (ac *AccessControl) PatchSecurityGroup(dstIPv4Addresses, dstIPv6Addresses [ armnetwork.SecurityRuleProtocolAsterisk, } + // First, add deny rules for blocked IP ranges (higher precedence) per IP family. + if len(ac.BlockedIPRanges) > 0 { + for _, protocol := range protocols { + dstPorts, found := ac.securityRuleDestinationPortsByProtocol[protocol] + if !found { + continue + } + blockedAggregated := iputil.AggregatePrefixes(ac.BlockedIPRanges) + blockedIPv4, blockedIPv6 := iputil.GroupPrefixesByFamily(blockedAggregated) + if len(blockedIPv4) > 0 && len(dstIPv4Addresses) > 0 { + if err := ac.sgHelper.AddRuleForBlockedIPRanges(blockedIPv4, protocol, dstIPv4Addresses, dstPorts); err != nil { + return fmt.Errorf("failed to add a new rule for blocked IPv4 ranges: %w", err) + } + } + if len(blockedIPv6) > 0 && len(dstIPv6Addresses) > 0 { + if err := ac.sgHelper.AddRuleForBlockedIPRanges(blockedIPv6, protocol, dstIPv6Addresses, dstPorts); err != nil { + return fmt.Errorf("failed to add a new rule for blocked IPv6 ranges: %w", err) + } + } + } + } + for _, protocol := range protocols { dstPorts, found := ac.securityRuleDestinationPortsByProtocol[protocol] if !found { continue } + if len(dstIPv4Addresses) > 0 { for _, tag := range allowedServiceTags { err := ac.sgHelper.AddRuleForAllowedServiceTag(tag, protocol, dstIPv4Addresses, dstPorts) diff --git a/pkg/provider/loadbalancer/accesscontrol_test.go b/pkg/provider/loadbalancer/accesscontrol_test.go index 6b23a23d65..e800c5025f 100644 --- a/pkg/provider/loadbalancer/accesscontrol_test.go +++ b/pkg/provider/loadbalancer/accesscontrol_test.go @@ -203,6 +203,25 @@ func TestNewAccessControl(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 1, called) }) + + t.Run("it should emit warning event if invalid azure-blocked-ip-ranges", func(t *testing.T) { + svc := k8sFx.Service(). + WithBlockedIPRanges("foo", "10.0.0.1/32", "bar"). + Build() + + called := 0 + eventEmitter := func(obj runtime.Object, eventType, reason, message string) { + called++ + assert.Equal(t, &svc, obj) + assert.Equal(t, v1.EventTypeWarning, eventType) + assert.Equal(t, "InvalidBlockedIPRanges", reason) + assert.Equal(t, EventMessageOfInvalidBlockedIPRanges([]string{"foo", "bar"}), message) + } + + _, err := NewAccessControl(log.Noop(), &svc, sg, WithEventEmitter(eventEmitter)) + assert.NoError(t, err) + assert.Equal(t, 1, called) + }) } func TestAccessControl_DenyAllExceptSourceRanges(t *testing.T) { @@ -1134,6 +1153,382 @@ func TestAccessControl_PatchSecurityGroup(t *testing.T) { ) runTest(t, svc, originalRules, dstIPv4Addresses, dstIPv6Addresses, true, expectedRules) }) + + t.Run("patch service with blockedIPRanges only (deny before internet allow)", func(t *testing.T) { + var ( + k8sFx = fixture.NewFixture().Kubernetes() + blockedIPv4Ranges = []string{"10.1.0.0/16"} + blockedIPv6Ranges = []string{"fd12:abcd::/48"} + svc = k8sFx.Service(). + WithBlockedIPRanges(append(blockedIPv4Ranges, blockedIPv6Ranges...)...). + Build() + originalRules = azureFx.NoiseSecurityRules() + dstIPv4Addresses = []string{"52.0.0.1"} + dstIPv6Addresses = []string{"2001:db8::1"} + expectedRules = testutil.CloneInJSON(originalRules) + ) + // Expected: deny blocked IPv4/IPv6 rules first, then internet allow rules + expectedRules = append(expectedRules, + // Deny rules for TCP + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, blockedIPv4Ranges, k8sFx.Service().TCPPorts()).WithPriority(400).WithDestination(dstIPv4Addresses...).Build(), + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv6, blockedIPv6Ranges, k8sFx.Service().TCPPorts()).WithPriority(401).WithDestination(dstIPv6Addresses...).Build(), + // Deny rules for UDP + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, blockedIPv4Ranges, k8sFx.Service().UDPPorts()).WithPriority(402).WithDestination(dstIPv4Addresses...).Build(), + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv6, blockedIPv6Ranges, k8sFx.Service().UDPPorts()).WithPriority(403).WithDestination(dstIPv6Addresses...).Build(), + // Allow rules + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, []string{securitygroup.ServiceTagInternet}, k8sFx.Service().TCPPorts()).WithPriority(500).WithDestination(dstIPv4Addresses...).Build(), + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv6, []string{securitygroup.ServiceTagInternet}, k8sFx.Service().TCPPorts()).WithPriority(501).WithDestination(dstIPv6Addresses...).Build(), + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, []string{securitygroup.ServiceTagInternet}, k8sFx.Service().UDPPorts()).WithPriority(502).WithDestination(dstIPv4Addresses...).Build(), + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv6, []string{securitygroup.ServiceTagInternet}, k8sFx.Service().UDPPorts()).WithPriority(503).WithDestination(dstIPv6Addresses...).Build(), + ) + runTest(t, svc, originalRules, dstIPv4Addresses, dstIPv6Addresses, true, expectedRules) + }) + + t.Run("patch service with blockedIPRanges and allowedServiceTags", func(t *testing.T) { + var ( + k8sFx = fixture.NewFixture().Kubernetes() + serviceTags = azureFx.ServiceTags(1) + blockedIPv4Ranges = []string{"10.1.0.0/16"} + svc = k8sFx.Service(). + WithBlockedIPRanges(blockedIPv4Ranges...). + WithAllowedServiceTags(serviceTags...). + Build() + originalRules = azureFx.NoiseSecurityRules() + dstIPv4Addresses = []string{"52.0.0.1"} + dstIPv6Addresses = []string{} + expectedRules = testutil.CloneInJSON(originalRules) + ) + // Expected: deny blocked IPv4 rule then service tag allow (no internet tag since service tag specified) + expectedRules = append(expectedRules, + // Deny rules for TCP + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, blockedIPv4Ranges, k8sFx.Service().TCPPorts()).WithPriority(400).WithDestination(dstIPv4Addresses...).Build(), + // Deny rules for UDP + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, blockedIPv4Ranges, k8sFx.Service().UDPPorts()).WithPriority(401).WithDestination(dstIPv4Addresses...).Build(), + // Allow rules + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, []string{serviceTags[0]}, k8sFx.Service().TCPPorts()).WithPriority(500).WithDestination(dstIPv4Addresses...).Build(), + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, []string{serviceTags[0]}, k8sFx.Service().UDPPorts()).WithPriority(501).WithDestination(dstIPv4Addresses...).Build(), + ) + runTest(t, svc, originalRules, dstIPv4Addresses, dstIPv6Addresses, true, expectedRules) + }) + + t.Run("patch service with blockedIPRanges on IPv4 only", func(t *testing.T) { + var ( + k8sFx = fixture.NewFixture().Kubernetes() + blockedIPv4Ranges = []string{"10.1.0.0/16", "172.16.0.0/12"} + svc = k8sFx.Service(). + WithBlockedIPRanges(blockedIPv4Ranges...). + Build() + originalRules = azureFx.NoiseSecurityRules() + dstIPv4Addresses = []string{"52.0.0.1", "52.0.0.2"} + dstIPv6Addresses = []string{} + expectedRules = testutil.CloneInJSON(originalRules) + ) + // Expected: deny blocked IPv4 rules first, then internet allow rules for IPv4 + expectedRules = append(expectedRules, + // Deny rules for TCP IPv4 + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, blockedIPv4Ranges, k8sFx.Service().TCPPorts()).WithPriority(400).WithDestination(dstIPv4Addresses...).Build(), + // Deny rules for UDP IPv4 + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, blockedIPv4Ranges, k8sFx.Service().UDPPorts()).WithPriority(401).WithDestination(dstIPv4Addresses...).Build(), + // Allow rules for IPv4 + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, []string{securitygroup.ServiceTagInternet}, k8sFx.Service().TCPPorts()).WithPriority(500).WithDestination(dstIPv4Addresses...).Build(), + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, []string{securitygroup.ServiceTagInternet}, k8sFx.Service().UDPPorts()).WithPriority(501).WithDestination(dstIPv4Addresses...).Build(), + ) + runTest(t, svc, originalRules, dstIPv4Addresses, dstIPv6Addresses, true, expectedRules) + }) + + t.Run("patch service with blockedIPRanges on IPv6 only", func(t *testing.T) { + var ( + k8sFx = fixture.NewFixture().Kubernetes() + blockedIPv6Ranges = []string{"fd12:abcd::/48", "fe80:1234::/32"} + svc = k8sFx.Service(). + WithBlockedIPRanges(blockedIPv6Ranges...). + Build() + originalRules = azureFx.NoiseSecurityRules() + dstIPv4Addresses = []string{} + dstIPv6Addresses = []string{"2001:db8::1", "2001:db8::2"} + expectedRules = testutil.CloneInJSON(originalRules) + ) + // Expected: deny blocked IPv6 rules first, then internet allow rules for IPv6 + expectedRules = append(expectedRules, + // Deny rules for TCP IPv6 + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv6, blockedIPv6Ranges, k8sFx.Service().TCPPorts()).WithPriority(400).WithDestination(dstIPv6Addresses...).Build(), + // Deny rules for UDP IPv6 + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv6, blockedIPv6Ranges, k8sFx.Service().UDPPorts()).WithPriority(401).WithDestination(dstIPv6Addresses...).Build(), + // Allow rules for IPv6 + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv6, []string{securitygroup.ServiceTagInternet}, k8sFx.Service().TCPPorts()).WithPriority(500).WithDestination(dstIPv6Addresses...).Build(), + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv6, []string{securitygroup.ServiceTagInternet}, k8sFx.Service().UDPPorts()).WithPriority(501).WithDestination(dstIPv6Addresses...).Build(), + ) + runTest(t, svc, originalRules, dstIPv4Addresses, dstIPv6Addresses, true, expectedRules) + }) + + t.Run("patch service with blockedIPRanges on IPv4 and IPv6", func(t *testing.T) { + var ( + k8sFx = fixture.NewFixture().Kubernetes() + blockedIPv4Ranges = []string{"10.1.0.0/16", "172.16.0.0/12"} + blockedIPv6Ranges = []string{"fd12:abcd::/48"} + svc = k8sFx.Service(). + WithBlockedIPRanges(append(blockedIPv4Ranges, blockedIPv6Ranges...)...). + Build() + originalRules = azureFx.NoiseSecurityRules() + dstIPv4Addresses = []string{"52.0.0.1"} + dstIPv6Addresses = []string{"2001:db8::1"} + expectedRules = testutil.CloneInJSON(originalRules) + ) + // Expected: deny blocked IPv4/IPv6 rules first, then internet allow rules + expectedRules = append(expectedRules, + // Deny rules for TCP + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, blockedIPv4Ranges, k8sFx.Service().TCPPorts()).WithPriority(400).WithDestination(dstIPv4Addresses...).Build(), + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv6, blockedIPv6Ranges, k8sFx.Service().TCPPorts()).WithPriority(401).WithDestination(dstIPv6Addresses...).Build(), + // Deny rules for UDP + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, blockedIPv4Ranges, k8sFx.Service().UDPPorts()).WithPriority(402).WithDestination(dstIPv4Addresses...).Build(), + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv6, blockedIPv6Ranges, k8sFx.Service().UDPPorts()).WithPriority(403).WithDestination(dstIPv6Addresses...).Build(), + // Allow rules + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, []string{securitygroup.ServiceTagInternet}, k8sFx.Service().TCPPorts()).WithPriority(500).WithDestination(dstIPv4Addresses...).Build(), + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv6, []string{securitygroup.ServiceTagInternet}, k8sFx.Service().TCPPorts()).WithPriority(501).WithDestination(dstIPv6Addresses...).Build(), + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, []string{securitygroup.ServiceTagInternet}, k8sFx.Service().UDPPorts()).WithPriority(502).WithDestination(dstIPv4Addresses...).Build(), + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv6, []string{securitygroup.ServiceTagInternet}, k8sFx.Service().UDPPorts()).WithPriority(503).WithDestination(dstIPv6Addresses...).Build(), + ) + runTest(t, svc, originalRules, dstIPv4Addresses, dstIPv6Addresses, true, expectedRules) + }) + + t.Run("patch service with blockedIPRanges and allowedIPRanges", func(t *testing.T) { + var ( + k8sFx = fixture.NewFixture().Kubernetes() + blockedIPv4Ranges = []string{"10.1.0.0/16"} + allowedIPv4Ranges = []string{"192.168.0.0/16", "20.0.0.1/32"} + svc = k8sFx.Service(). + WithBlockedIPRanges(blockedIPv4Ranges...). + WithAllowedIPRanges(allowedIPv4Ranges...). + Build() + originalRules = azureFx.NoiseSecurityRules() + dstIPv4Addresses = []string{"52.0.0.1"} + dstIPv6Addresses = []string{} + expectedRules = testutil.CloneInJSON(originalRules) + ) + // Expected: deny blocked first, then allow specific IP ranges (no internet tag since allowedIPRanges specified) + expectedRules = append(expectedRules, + // Deny rules for TCP + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, blockedIPv4Ranges, k8sFx.Service().TCPPorts()).WithPriority(400).WithDestination(dstIPv4Addresses...).Build(), + // Deny rules for UDP + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, blockedIPv4Ranges, k8sFx.Service().UDPPorts()).WithPriority(401).WithDestination(dstIPv4Addresses...).Build(), + // Allow rules for specific IP ranges + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, allowedIPv4Ranges, k8sFx.Service().TCPPorts()).WithPriority(500).WithDestination(dstIPv4Addresses...).Build(), + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, allowedIPv4Ranges, k8sFx.Service().UDPPorts()).WithPriority(501).WithDestination(dstIPv4Addresses...).Build(), + ) + runTest(t, svc, originalRules, dstIPv4Addresses, dstIPv6Addresses, true, expectedRules) + }) + + t.Run("patch service with blockedIPRanges and loadBalancerSourceRanges", func(t *testing.T) { + var ( + k8sFx = fixture.NewFixture().Kubernetes() + blockedIPv4Ranges = []string{"10.1.0.0/16"} + sourceRanges = []string{"192.168.0.0/16", "20.0.0.1/32"} + svc = k8sFx.Service(). + WithBlockedIPRanges(blockedIPv4Ranges...). + WithLoadBalancerSourceRanges(sourceRanges...). + Build() + originalRules = azureFx.NoiseSecurityRules() + dstIPv4Addresses = []string{"52.0.0.1"} + dstIPv6Addresses = []string{} + expectedRules = testutil.CloneInJSON(originalRules) + ) + // Expected: deny blocked first, then allow source ranges (no internet tag since sourceRanges specified) + expectedRules = append(expectedRules, + // Deny rules for TCP + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, blockedIPv4Ranges, k8sFx.Service().TCPPorts()).WithPriority(400).WithDestination(dstIPv4Addresses...).Build(), + // Deny rules for UDP + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, blockedIPv4Ranges, k8sFx.Service().UDPPorts()).WithPriority(401).WithDestination(dstIPv4Addresses...).Build(), + // Allow rules for source ranges + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, sourceRanges, k8sFx.Service().TCPPorts()).WithPriority(500).WithDestination(dstIPv4Addresses...).Build(), + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, sourceRanges, k8sFx.Service().UDPPorts()).WithPriority(501).WithDestination(dstIPv4Addresses...).Build(), + ) + runTest(t, svc, originalRules, dstIPv4Addresses, dstIPv6Addresses, true, expectedRules) + }) + + t.Run("patch service with invalid blockedIPRanges should only use valid ones", func(t *testing.T) { + var ( + k8sFx = fixture.NewFixture().Kubernetes() + // 10.1.0.1/16 is invalid (not network address), only 10.2.0.0/16 should be used + inputBlockedRanges = []string{"10.1.0.1/16", "10.2.0.0/16"} + validBlockedRanges = []string{"10.2.0.0/16"} + svc = k8sFx.Service(). + WithBlockedIPRanges(inputBlockedRanges...). + Build() + originalRules = azureFx.NoiseSecurityRules() + dstIPv4Addresses = []string{"52.0.0.1"} + dstIPv6Addresses = []string{} + expectedRules = testutil.CloneInJSON(originalRules) + ) + // Expected: only valid blocked IP ranges should be used + expectedRules = append(expectedRules, + // Deny rules for TCP with only valid ranges + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, validBlockedRanges, k8sFx.Service().TCPPorts()).WithPriority(400).WithDestination(dstIPv4Addresses...).Build(), + // Deny rules for UDP with only valid ranges + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, validBlockedRanges, k8sFx.Service().UDPPorts()).WithPriority(401).WithDestination(dstIPv4Addresses...).Build(), + // Allow rules + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, []string{securitygroup.ServiceTagInternet}, k8sFx.Service().TCPPorts()).WithPriority(500).WithDestination(dstIPv4Addresses...).Build(), + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, []string{securitygroup.ServiceTagInternet}, k8sFx.Service().UDPPorts()).WithPriority(501).WithDestination(dstIPv4Addresses...).Build(), + ) + runTest(t, svc, originalRules, dstIPv4Addresses, dstIPv6Addresses, true, expectedRules) + }) + + t.Run("patch service with blockedIPRanges and denyAllExceptSourceRanges", func(t *testing.T) { + var ( + k8sFx = fixture.NewFixture().Kubernetes() + blockedIPv4Ranges = []string{"10.1.0.0/16"} + allowedIPv4Ranges = []string{"192.168.0.0/16"} + svc = k8sFx.Service(). + WithBlockedIPRanges(blockedIPv4Ranges...). + WithAllowedIPRanges(allowedIPv4Ranges...). + WithDenyAllExceptLoadBalancerSourceRanges(). + Build() + originalRules = azureFx.NoiseSecurityRules() + dstIPv4Addresses = []string{"52.0.0.1"} + dstIPv6Addresses = []string{} + expectedRules = testutil.CloneInJSON(originalRules) + ) + // Expected: deny blocked first, allow specific ranges, then deny all + expectedRules = append(expectedRules, + // Deny rules for blocked IP ranges + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, blockedIPv4Ranges, k8sFx.Service().TCPPorts()).WithPriority(400).WithDestination(dstIPv4Addresses...).Build(), + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, blockedIPv4Ranges, k8sFx.Service().UDPPorts()).WithPriority(401).WithDestination(dstIPv4Addresses...).Build(), + // Allow rules for specific IP ranges + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, allowedIPv4Ranges, k8sFx.Service().TCPPorts()).WithPriority(500).WithDestination(dstIPv4Addresses...).Build(), + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, allowedIPv4Ranges, k8sFx.Service().UDPPorts()).WithPriority(501).WithDestination(dstIPv4Addresses...).Build(), + // Deny ALL at the end + azureFx.DenyAllSecurityRule(iputil.IPv4).WithPriority(4095).WithDestination(dstIPv4Addresses...).Build(), + ) + runTest(t, svc, originalRules, dstIPv4Addresses, dstIPv6Addresses, true, expectedRules) + }) + + t.Run("patch service with blockedIPRanges, allowedIPRanges and allowedServiceTags", func(t *testing.T) { + var ( + k8sFx = fixture.NewFixture().Kubernetes() + serviceTags = azureFx.ServiceTags(1) + blockedIPv4Ranges = []string{"10.1.0.0/16"} + allowedIPv4Ranges = []string{"192.168.0.0/16"} + svc = k8sFx.Service(). + WithBlockedIPRanges(blockedIPv4Ranges...). + WithAllowedIPRanges(allowedIPv4Ranges...). + WithAllowedServiceTags(serviceTags...). + Build() + originalRules = azureFx.NoiseSecurityRules() + dstIPv4Addresses = []string{"52.0.0.1"} + dstIPv6Addresses = []string{} + expectedRules = testutil.CloneInJSON(originalRules) + ) + // Expected: deny blocked first, then allow service tags and IP ranges + expectedRules = append(expectedRules, + // Deny rules for blocked IP ranges + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, blockedIPv4Ranges, k8sFx.Service().TCPPorts()).WithPriority(400).WithDestination(dstIPv4Addresses...).Build(), + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, blockedIPv4Ranges, k8sFx.Service().UDPPorts()).WithPriority(401).WithDestination(dstIPv4Addresses...).Build(), + // Allow rules for service tag + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, []string{serviceTags[0]}, k8sFx.Service().TCPPorts()).WithPriority(500).WithDestination(dstIPv4Addresses...).Build(), + // Allow rules for IP ranges + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, allowedIPv4Ranges, k8sFx.Service().TCPPorts()).WithPriority(501).WithDestination(dstIPv4Addresses...).Build(), + // UDP for service tag + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, []string{serviceTags[0]}, k8sFx.Service().UDPPorts()).WithPriority(502).WithDestination(dstIPv4Addresses...).Build(), + // UDP for IP ranges + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, allowedIPv4Ranges, k8sFx.Service().UDPPorts()).WithPriority(503).WithDestination(dstIPv4Addresses...).Build(), + ) + runTest(t, svc, originalRules, dstIPv4Addresses, dstIPv6Addresses, true, expectedRules) + }) + + t.Run("patch internal LB with blockedIPRanges and allowedIPRanges", func(t *testing.T) { + var ( + k8sFx = fixture.NewFixture().Kubernetes() + blockedIPv4Ranges = []string{"10.1.0.0/16"} + allowedIPv4Ranges = []string{"192.168.0.0/16"} + svc = k8sFx.Service(). + WithInternalEnabled(). + WithBlockedIPRanges(blockedIPv4Ranges...). + WithAllowedIPRanges(allowedIPv4Ranges...). + Build() + originalRules = azureFx.NoiseSecurityRules() + dstIPv4Addresses = []string{"10.0.0.1"} + dstIPv6Addresses = []string{} + expectedRules = testutil.CloneInJSON(originalRules) + ) + // Internal LB with blocked and allowed IP ranges + expectedRules = append(expectedRules, + // Deny rules for blocked IP ranges + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, blockedIPv4Ranges, k8sFx.Service().TCPPorts()).WithPriority(400).WithDestination(dstIPv4Addresses...).Build(), + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, blockedIPv4Ranges, k8sFx.Service().UDPPorts()).WithPriority(401).WithDestination(dstIPv4Addresses...).Build(), + // Allow rules for specific IP ranges + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, allowedIPv4Ranges, k8sFx.Service().TCPPorts()).WithPriority(500).WithDestination(dstIPv4Addresses...).Build(), + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, allowedIPv4Ranges, k8sFx.Service().UDPPorts()).WithPriority(501).WithDestination(dstIPv4Addresses...).Build(), + ) + runTest(t, svc, originalRules, dstIPv4Addresses, dstIPv6Addresses, true, expectedRules) + }) + + t.Run("patch service with multiple overlapping blockedIPRanges should aggregate", func(t *testing.T) { + var ( + k8sFx = fixture.NewFixture().Kubernetes() + // 10.0.0.0/8 contains 10.1.0.0/16, so they should be aggregated + blockedIPv4Ranges = []string{"10.0.0.0/8", "10.1.0.0/16"} + aggregatedBlockedIP = []string{"10.0.0.0/8"} + svc = k8sFx.Service(). + WithBlockedIPRanges(blockedIPv4Ranges...). + Build() + originalRules = azureFx.NoiseSecurityRules() + dstIPv4Addresses = []string{"52.0.0.1"} + dstIPv6Addresses = []string{} + expectedRules = testutil.CloneInJSON(originalRules) + ) + // Expected: overlapping ranges should be aggregated + expectedRules = append(expectedRules, + // Deny rules for TCP with aggregated ranges + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, aggregatedBlockedIP, k8sFx.Service().TCPPorts()).WithPriority(400).WithDestination(dstIPv4Addresses...).Build(), + // Deny rules for UDP with aggregated ranges + azureFx.DenyBlockedIPRangeSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, aggregatedBlockedIP, k8sFx.Service().UDPPorts()).WithPriority(401).WithDestination(dstIPv4Addresses...).Build(), + // Allow rules + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, []string{securitygroup.ServiceTagInternet}, k8sFx.Service().TCPPorts()).WithPriority(500).WithDestination(dstIPv4Addresses...).Build(), + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, []string{securitygroup.ServiceTagInternet}, k8sFx.Service().UDPPorts()).WithPriority(501).WithDestination(dstIPv4Addresses...).Build(), + ) + runTest(t, svc, originalRules, dstIPv4Addresses, dstIPv6Addresses, true, expectedRules) + }) + + t.Run("patch service with blockedIPRanges but no destination addresses for that IP family", func(t *testing.T) { + var ( + k8sFx = fixture.NewFixture().Kubernetes() + blockedIPv6Ranges = []string{"fd12:abcd::/48"} + svc = k8sFx.Service(). + WithBlockedIPRanges(blockedIPv6Ranges...). + Build() + originalRules = azureFx.NoiseSecurityRules() + dstIPv4Addresses = []string{"52.0.0.1"} // Only IPv4 destinations + dstIPv6Addresses = []string{} // No IPv6 destinations + expectedRules = testutil.CloneInJSON(originalRules) + ) + // No IPv6 deny rules should be added since there are no IPv6 destinations + // Only IPv4 allow rules should be added + expectedRules = append(expectedRules, + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv4, []string{securitygroup.ServiceTagInternet}, k8sFx.Service().TCPPorts()).WithPriority(500).WithDestination(dstIPv4Addresses...).Build(), + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv4, []string{securitygroup.ServiceTagInternet}, k8sFx.Service().UDPPorts()).WithPriority(501).WithDestination(dstIPv4Addresses...).Build(), + ) + runTest(t, svc, originalRules, dstIPv4Addresses, dstIPv6Addresses, true, expectedRules) + }) + + t.Run("patch service with blockedIPRanges but no destination addresses for that IP family (IPv4 blocked, IPv6 destinations)", func(t *testing.T) { + var ( + k8sFx = fixture.NewFixture().Kubernetes() + blockedIPv4Ranges = []string{"10.1.0.0/16"} + svc = k8sFx.Service(). + WithBlockedIPRanges(blockedIPv4Ranges...). + Build() + originalRules = azureFx.NoiseSecurityRules() + dstIPv4Addresses = []string{} // No IPv4 destinations + dstIPv6Addresses = []string{"2001:db8::1", "2002:fb8::1"} // Only IPv6 destinations + expectedRules = testutil.CloneInJSON(originalRules) + ) + // No IPv4 deny rules should be added since there are no IPv4 destinations + // Only IPv6 allow rules should be added + expectedRules = append(expectedRules, + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolTCP, iputil.IPv6, []string{securitygroup.ServiceTagInternet}, k8sFx.Service().TCPPorts()).WithPriority(500).WithDestination(dstIPv6Addresses...).Build(), + azureFx.AllowSecurityRule(armnetwork.SecurityRuleProtocolUDP, iputil.IPv6, []string{securitygroup.ServiceTagInternet}, k8sFx.Service().UDPPorts()).WithPriority(501).WithDestination(dstIPv6Addresses...).Build(), + ) + runTest(t, svc, originalRules, dstIPv4Addresses, dstIPv6Addresses, true, expectedRules) + }) } func TestAccessControl_RetainSecurityGroup(t *testing.T) { diff --git a/pkg/provider/loadbalancer/configuration.go b/pkg/provider/loadbalancer/configuration.go index d1f3d46832..a56f5358ae 100644 --- a/pkg/provider/loadbalancer/configuration.go +++ b/pkg/provider/loadbalancer/configuration.go @@ -56,9 +56,6 @@ func AllowedServiceTags(svc *v1.Service) []string { // AllowedIPRanges returns the allowed IP ranges configured by user through AKS custom annotations: // service.beta.kubernetes.io/azure-allowed-ip-ranges and service.beta.kubernetes.io/load-balancer-source-ranges func AllowedIPRanges(svc *v1.Service) ([]netip.Prefix, []string, error) { - const ( - Sep = "," - ) var ( errs []error validRanges []netip.Prefix @@ -66,24 +63,11 @@ func AllowedIPRanges(svc *v1.Service) ([]netip.Prefix, []string, error) { ) for _, key := range []string{consts.ServiceAnnotationAllowedIPRanges, v1.AnnotationLoadBalancerSourceRangesKey} { - value, found := svc.Annotations[key] - if !found { - continue - } - - var errsByKey []error - for _, p := range strings.Split(strings.TrimSpace(value), Sep) { - p = strings.TrimSpace(p) - prefix, err := iputil.ParsePrefix(p) - if err != nil { - errsByKey = append(errsByKey, err) - invalidRanges = append(invalidRanges, p) - } else { - validRanges = append(validRanges, prefix) - } - } - if len(errsByKey) > 0 { - errs = append(errs, NewErrAnnotationValue(key, value, errors.Join(errsByKey...))) + valid, invalid, err := parseIPRangesAnnotation(svc, key) + validRanges = append(validRanges, valid...) + invalidRanges = append(invalidRanges, invalid...) + if err != nil { + errs = append(errs, err) } } @@ -93,6 +77,12 @@ func AllowedIPRanges(svc *v1.Service) ([]netip.Prefix, []string, error) { return validRanges, invalidRanges, nil } +// BlockedIPRanges returns the blocked IP ranges configured by user through the AKS custom annotation: +// service.beta.kubernetes.io/azure-blocked-ip-ranges +func BlockedIPRanges(svc *v1.Service) ([]netip.Prefix, []string, error) { + return parseIPRangesAnnotation(svc, consts.ServiceAnnotationBlockedIPRanges) +} + // SourceRanges returns the allowed IP ranges configured by user through `spec.LoadBalancerSourceRanges`. func SourceRanges(svc *v1.Service) ([]netip.Prefix, []string, error) { var ( @@ -153,3 +143,40 @@ func NewErrAnnotationValue(key, value string, inner error) *ErrAnnotationValue { func (err *ErrAnnotationValue) Error() string { return fmt.Sprintf("invalid service annotation %s:%s: %s", err.AnnotationKey, err.AnnotationValue, err.Inner) } + +// parseIPRangesAnnotation parses a comma separated list of IP prefixes from the given annotation key. +// It returns the successfully parsed prefixes, a slice of invalid raw strings, and an error wrapping +// all parse failures for that annotation (annotated with ErrAnnotationValue). If the annotation is +// absent, all returned values are zero-value (nil, nil, nil). +func parseIPRangesAnnotation(svc *v1.Service, key string) ([]netip.Prefix, []string, error) { + const ( + Sep = "," + ) + + value, found := svc.Annotations[key] + if !found { + return nil, nil, nil + } + + var ( + validRanges []netip.Prefix + invalidRanges []string + errsByKey []error + ) + for _, p := range strings.Split(strings.TrimSpace(value), Sep) { + p = strings.TrimSpace(p) + prefix, err := iputil.ParsePrefix(p) + if err != nil { + errsByKey = append(errsByKey, err) + invalidRanges = append(invalidRanges, p) + } else { + validRanges = append(validRanges, prefix) + } + } + + if len(errsByKey) > 0 { + return validRanges, invalidRanges, NewErrAnnotationValue(key, value, errors.Join(errsByKey...)) + } + + return validRanges, invalidRanges, nil +} diff --git a/pkg/provider/loadbalancer/configuration_test.go b/pkg/provider/loadbalancer/configuration_test.go index 37c3df1b0e..5845c32c40 100644 --- a/pkg/provider/loadbalancer/configuration_test.go +++ b/pkg/provider/loadbalancer/configuration_test.go @@ -223,6 +223,89 @@ func TestAllowedIPRanges(t *testing.T) { }) } +func TestBlockedIPRanges(t *testing.T) { + t.Run("no annotation", func(t *testing.T) { + actual, invalid, err := BlockedIPRanges(&v1.Service{ + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeLoadBalancer, + }, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{}, + }, + }) + assert.NoError(t, err) + assert.Empty(t, actual) + assert.Empty(t, invalid) + }) + t.Run("with 1 IPv4 range in blocked ip ranges", func(t *testing.T) { + actual, invalid, err := BlockedIPRanges(&v1.Service{ + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeLoadBalancer, + }, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + consts.ServiceAnnotationBlockedIPRanges: "10.10.0.0/24", + }, + }, + }) + assert.NoError(t, err) + assert.Equal(t, []netip.Prefix{netip.MustParsePrefix("10.10.0.0/24")}, actual) + assert.Empty(t, invalid) + }) + t.Run("with 1 IPv6 range in blocked ip ranges", func(t *testing.T) { + actual, invalid, err := BlockedIPRanges(&v1.Service{ + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeLoadBalancer, + }, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + consts.ServiceAnnotationBlockedIPRanges: "2001:db8::/32", + }, + }, + }) + assert.NoError(t, err) + assert.Equal(t, []netip.Prefix{netip.MustParsePrefix("2001:db8::/32")}, actual) + assert.Empty(t, invalid) + }) + t.Run("with multiple IP ranges", func(t *testing.T) { + actual, invalid, err := BlockedIPRanges(&v1.Service{ + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeLoadBalancer, + }, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + consts.ServiceAnnotationBlockedIPRanges: " 10.10.0.0/24, 10.20.0.0/16, 2001:db8::/32, 2002:db8::/64 ", + }, + }, + }) + assert.NoError(t, err) + assert.Equal(t, []netip.Prefix{ + netip.MustParsePrefix("10.10.0.0/24"), + netip.MustParsePrefix("10.20.0.0/16"), + netip.MustParsePrefix("2001:db8::/32"), + netip.MustParsePrefix("2002:db8::/64"), + }, actual) + assert.Empty(t, invalid) + }) + t.Run("with invalid IP range", func(t *testing.T) { + _, invalid, err := BlockedIPRanges(&v1.Service{ + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeLoadBalancer, + }, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + consts.ServiceAnnotationBlockedIPRanges: "foobar,10.0.0.1/24", + }, + }, + }) + assert.Error(t, err) + + var e *ErrAnnotationValue + assert.ErrorAs(t, err, &e) + assert.Equal(t, []string{"foobar", "10.0.0.1/24"}, invalid) + }) +} + func TestSourceRanges(t *testing.T) { t.Run("not specified in spec", func(t *testing.T) { actual, invalid, err := SourceRanges(&v1.Service{ @@ -339,3 +422,105 @@ func TestAdditionalPublicIPs(t *testing.T) { assert.Equal(t, e.AnnotationKey, consts.ServiceAnnotationAdditionalPublicIPs) }) } + +func TestParseIPRangesAnnotation(t *testing.T) { + tests := []struct { + name string + annotationKey string + annotationVal string + wantPrefixes []netip.Prefix + wantInvalid []string + wantErr bool + }{ + { + name: "annotation absent", + annotationKey: consts.ServiceAnnotationAllowedIPRanges, + annotationVal: "", + wantPrefixes: nil, + wantInvalid: nil, + wantErr: false, + }, + { + name: "single valid IPv4", + annotationKey: consts.ServiceAnnotationAllowedIPRanges, + annotationVal: "10.0.0.0/24", + wantPrefixes: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/24")}, + wantInvalid: nil, + wantErr: false, + }, + { + name: "single valid IPv6", + annotationKey: consts.ServiceAnnotationAllowedIPRanges, + annotationVal: "2001:db8::/48", + wantPrefixes: []netip.Prefix{netip.MustParsePrefix("2001:db8::/48")}, + wantInvalid: nil, + wantErr: false, + }, + { + name: "multiple mixed IPv4 IPv6 with spaces", + annotationKey: consts.ServiceAnnotationAllowedIPRanges, + annotationVal: " 10.1.0.0/16 , 2001:db8:1::/64 ", + wantPrefixes: []netip.Prefix{ + netip.MustParsePrefix("10.1.0.0/16"), + netip.MustParsePrefix("2001:db8:1::/64"), + }, + wantInvalid: nil, + wantErr: false, + }, + { + name: "multiple valid IP ranges", + annotationKey: consts.ServiceAnnotationAllowedIPRanges, + annotationVal: "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16,2001:db8::/32,2001:db8:1::/48,fd00::/8", + wantPrefixes: []netip.Prefix{ + netip.MustParsePrefix("10.0.0.0/8"), + netip.MustParsePrefix("172.16.0.0/12"), + netip.MustParsePrefix("192.168.0.0/16"), + netip.MustParsePrefix("2001:db8::/32"), + netip.MustParsePrefix("2001:db8:1::/48"), + netip.MustParsePrefix("fd00::/8"), + }, + wantInvalid: nil, + wantErr: false, + }, + { + name: "invalid entries collected", + annotationKey: consts.ServiceAnnotationAllowedIPRanges, + annotationVal: "foo,10.0.0.1/24,2001:db8::/129,bar", + wantPrefixes: nil, + wantInvalid: []string{"foo", "10.0.0.1/24", "2001:db8::/129", "bar"}, + wantErr: true, + }, + { + name: "some valid some invalid", + annotationKey: consts.ServiceAnnotationAllowedIPRanges, + annotationVal: "10.2.0.0/20,foo,2001:db8:2::/64,bar", + wantPrefixes: []netip.Prefix{ + netip.MustParsePrefix("10.2.0.0/20"), + netip.MustParsePrefix("2001:db8:2::/64"), + }, + wantInvalid: []string{"foo", "bar"}, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ann := map[string]string{} + if tt.annotationVal != "" { + ann[tt.annotationKey] = tt.annotationVal + } + svc := &v1.Service{ObjectMeta: metav1.ObjectMeta{Annotations: ann}} + + prefixes, invalid, err := parseIPRangesAnnotation(svc, tt.annotationKey) + if tt.wantErr { + assert.Error(t, err) + var e *ErrAnnotationValue + assert.ErrorAs(t, err, &e) + } else { + assert.NoError(t, err) + } + assert.Equal(t, tt.wantPrefixes, prefixes) + assert.Equal(t, tt.wantInvalid, invalid) + }) + } +} diff --git a/pkg/provider/loadbalancer/k8sevent.go b/pkg/provider/loadbalancer/k8sevent.go index 5fafa316f1..a7ac27a666 100644 --- a/pkg/provider/loadbalancer/k8sevent.go +++ b/pkg/provider/loadbalancer/k8sevent.go @@ -42,6 +42,13 @@ func EventMessageOfInvalidAllowedIPRanges(allowedIPRanges []string) string { ) } +func EventMessageOfInvalidBlockedIPRanges(blockedIPRanges []string) string { + return fmt.Sprintf("Found invalid %s %q, ignoring.", + consts.ServiceAnnotationBlockedIPRanges, + blockedIPRanges, + ) +} + func EventMessageOfConflictLoadBalancerSourceRangesAndAllowedIPRanges() string { return fmt.Sprintf( "Please use annotation %s instead of spec.loadBalancerSourceRanges while using %s annotation at the same time.", diff --git a/pkg/provider/securitygroup/securitygroup.go b/pkg/provider/securitygroup/securitygroup.go index 7459b011af..f661fa1f5a 100644 --- a/pkg/provider/securitygroup/securitygroup.go +++ b/pkg/provider/securitygroup/securitygroup.go @@ -101,38 +101,43 @@ func NewSecurityGroupHelper(logger logr.Logger, sg *armnetwork.SecurityGroup) (* }, nil } -type rulePriorityPrefer string +type rulePriorityClass string const ( - rulePriorityPreferFromStart rulePriorityPrefer = "from_start" - rulePriorityPreferFromEnd rulePriorityPrefer = "from_end" + rulePriorityPreferFromStart rulePriorityClass = "from_start" + rulePriorityPreferFromEnd rulePriorityClass = "from_end" + rulePriorityIPRangeDeny rulePriorityClass = "ip_range_deny" ) // nextRulePriority returns the next available priority for a new rule. // It takes a preference for whether to start from the beginning or end of the priority range. -func (helper *RuleHelper) nextRulePriority(prefer rulePriorityPrefer) (int32, error) { - var ( - init, end = consts.LoadBalancerMinimumPriority, consts.LoadBalancerMaximumPriority - delta = 1 - ) - if prefer == rulePriorityPreferFromEnd { - init, end, delta = end-1, init-1, -1 - } - - for init != end { - p := int32(init) - if _, found := helper.priorities[p]; found { - init += delta - continue +func (helper *RuleHelper) nextRulePriority(class rulePriorityClass) (int32, error) { + switch class { + case rulePriorityIPRangeDeny: + // IP prefix blocking rules have a separate interval before any other rules + for p := int32(consts.IPPrefixBlockingMinimumPriority); p <= consts.IPPrefixBlockingMaximumPriority; p++ { + if _, found := helper.priorities[p]; !found { + return p, nil + } + } + case rulePriorityPreferFromStart: + for p := int32(consts.LoadBalancerMinimumPriority); p < consts.LoadBalancerMaximumPriority; p++ { + if _, found := helper.priorities[p]; !found { + return p, nil + } + } + case rulePriorityPreferFromEnd: + for p := int32(consts.LoadBalancerMaximumPriority - 1); p >= consts.LoadBalancerMinimumPriority; p-- { + if _, found := helper.priorities[p]; !found { + return p, nil + } } - return p, nil } - return 0, ErrSecurityRulePriorityExhausted } // getOrCreateRule returns an existing rule or create a new one if it doesn't exist. -func (helper *RuleHelper) getOrCreateRule(name string, priorityPrefer rulePriorityPrefer) (*armnetwork.SecurityRule, error) { +func (helper *RuleHelper) getOrCreateRule(name string, priorityPrefer rulePriorityClass) (*armnetwork.SecurityRule, error) { logger := helper.logger.WithName("getOrCreateRule").WithValues("rule-name", name) if rule, found := helper.rules[name]; found { @@ -293,6 +298,69 @@ func (helper *RuleHelper) AddRuleForDenyAll(dstAddresses []netip.Addr) error { return nil } +// AddRuleForBlockedIPRanges adds a deny rule for traffic originating from specified blocked source prefixes. +// The rule denies traffic for a specific protocol and destination ports to the provided destination address. +func (helper *RuleHelper) AddRuleForBlockedIPRanges( + srcIPRanges []netip.Prefix, + protocol armnetwork.SecurityRuleProtocol, + dstAddresses []netip.Addr, + dstPorts []int32, +) error { + if len(srcIPRanges) == 0 || len(dstAddresses) == 0 { + return nil + } + if !iputil.ArePrefixesFromSameFamily(srcIPRanges) { + return ErrSecurityRuleSourceAddressesNotFromSameIPFamily + } + if !iputil.AreAddressesFromSameFamily(dstAddresses) { + return ErrSecurityRuleDestinationAddressesNotFromSameIPFamily + } + if srcIPRanges[0].Addr().Is4() != dstAddresses[0].Is4() { + return ErrSecurityRuleSourceAndDestinationNotFromSameIPFamily + } + + var ( + ipFamily = iputil.FamilyOfAddr(srcIPRanges[0].Addr()) + srcPrefixes = fnutil.Map(func(p netip.Prefix) string { return p.String() }, srcIPRanges) + dstPrefixes = fnutil.Map(func(a netip.Addr) string { return a.String() }, dstAddresses) + name = GenerateDenyBlockedSecurityRuleName(protocol, ipFamily, srcPrefixes, dstPorts) + ) + helper.logger.V(4).Info("Patching a rule for blocked IP ranges", "ip-family", ipFamily, "protocol", protocol, "destination", dstAddresses) + + rule, err := helper.getOrCreateRule(name, rulePriorityIPRangeDeny) + if err != nil { + return err + } + + // Configure rule properties + rule.Properties.Protocol = to.Ptr(protocol) + rule.Properties.Access = to.Ptr(armnetwork.SecurityRuleAccessDeny) + rule.Properties.Direction = to.Ptr(armnetwork.SecurityRuleDirectionInbound) + + // Source + if len(srcPrefixes) == 1 { + rule.Properties.SourceAddressPrefix = to.Ptr(srcPrefixes[0]) + rule.Properties.SourceAddressPrefixes = nil + } else { + rule.Properties.SourceAddressPrefix = nil + rule.Properties.SourceAddressPrefixes = to.SliceOfPtrs(srcPrefixes...) + } + rule.Properties.SourcePortRange = ptr.To("*") + + // Destination + existing := ListDestinationPrefixes(rule) + SetDestinationPrefixes(rule, append(existing, dstPrefixes...)) + + if len(dstPorts) > 0 { + SetDestinationPortRanges(rule, dstPorts) + } else { + SetAsteriskDestinationPortRange(rule) + } + + helper.logger.V(4).Info("Patched a rule for blocked IP ranges", "rule-name", name) + return nil +} + // RemoveDestinationFromRules removes the given destination addresses from rules that match the given protocol and ports is in the retainDstPorts list. // It may add a new rule if the original rule needs to be split. func (helper *RuleHelper) RemoveDestinationFromRules( @@ -304,7 +372,7 @@ func (helper *RuleHelper) RemoveDestinationFromRules( logger.V(10).Info("Cleaning destination from SecurityGroup") for _, rule := range helper.rules { - if !IsManagedSecurityRule(rule) { + if !IsManagedSecurityRule(rule) && !IsManagedPrefixBlockingSecurityRule(rule) { logger.V(4).Info("Skip rule with not-managed priority", "rule-name", *rule.Name) continue } diff --git a/pkg/provider/securitygroup/securitygroup_test.go b/pkg/provider/securitygroup/securitygroup_test.go index 7600e4ec21..f0f9d20005 100644 --- a/pkg/provider/securitygroup/securitygroup_test.go +++ b/pkg/provider/securitygroup/securitygroup_test.go @@ -17,6 +17,7 @@ limitations under the License. package securitygroup_test import ( + "fmt" "net/netip" "sort" "testing" @@ -1967,3 +1968,170 @@ func TestGenerateDenyAllSecurityRuleName(t *testing.T) { assert.Equal(t, GenerateDenyAllSecurityRuleName(iputil.IPv4), "k8s-azure-lb_deny-all_IPv4") assert.Equal(t, GenerateDenyAllSecurityRuleName(iputil.IPv6), "k8s-azure-lb_deny-all_IPv6") } + +func TestSecurityGroupHelper_AddRuleForBlockedIPRanges(t *testing.T) { + fx := fixture.NewFixture() + + t.Run("should return error when priority range is exhausted (more than 100 rules)", func(t *testing.T) { + // Pre-fill all 100 priorities in the IP blocking range (400-499) + existingRules := make([]*armnetwork.SecurityRule, 100) + for i := 0; i < 100; i++ { + existingRules[i] = &armnetwork.SecurityRule{ + Name: ptr.To(fmt.Sprintf("k8s-azure-lb-blocking-rule-%d", i)), + Properties: &armnetwork.SecurityRulePropertiesFormat{ + Priority: ptr.To(int32(400 + i)), // 400-499 + Protocol: to.Ptr(armnetwork.SecurityRuleProtocolTCP), + Access: to.Ptr(armnetwork.SecurityRuleAccessDeny), + Direction: to.Ptr(armnetwork.SecurityRuleDirectionInbound), + SourceAddressPrefix: ptr.To("*"), + SourcePortRange: ptr.To("*"), + DestinationAddressPrefix: ptr.To("10.0.0.1"), + DestinationPortRange: ptr.To("443"), + }, + } + } + + sg := fx.Azure().SecurityGroup().WithRules(existingRules).Build() + helper := ExpectNewSecurityGroupHelper(t, sg) + + // Try to add another blocked IP rule - should fail because all 100 priorities (400-499) are exhausted + err := helper.AddRuleForBlockedIPRanges( + fx.RandomIPv4Prefixes(1), + armnetwork.SecurityRuleProtocolTCP, + fx.RandomIPv4Addresses(1), + []int32{443}, + ) + + assert.Error(t, err) + assert.ErrorIs(t, err, ErrSecurityRulePriorityExhausted) + }) + + t.Run("when no rule exists, it should add one", func(t *testing.T) { + cases := []struct { + TestName string + IPFamily iputil.Family + Protocol armnetwork.SecurityRuleProtocol + SrcIPRanges []netip.Prefix + DstAddrs []netip.Addr + DstPorts []int32 + }{ + { + TestName: "TCP / IPv4", + IPFamily: iputil.IPv4, + Protocol: armnetwork.SecurityRuleProtocolTCP, + SrcIPRanges: fx.RandomIPv4Prefixes(2), + DstAddrs: fx.RandomIPv4Addresses(2), + DstPorts: []int32{80, 443}, + }, + { + TestName: "TCP / IPv6", + IPFamily: iputil.IPv6, + Protocol: armnetwork.SecurityRuleProtocolTCP, + SrcIPRanges: fx.RandomIPv6Prefixes(2), + DstAddrs: fx.RandomIPv6Addresses(2), + DstPorts: []int32{80, 443}, + }, + { + TestName: "UDP / IPv4", + IPFamily: iputil.IPv4, + Protocol: armnetwork.SecurityRuleProtocolUDP, + SrcIPRanges: fx.RandomIPv4Prefixes(2), + DstAddrs: fx.RandomIPv4Addresses(2), + DstPorts: []int32{5000}, + }, + { + TestName: "ANY / IPv4 / no ports", + IPFamily: iputil.IPv4, + Protocol: armnetwork.SecurityRuleProtocolAsterisk, + SrcIPRanges: fx.RandomIPv4Prefixes(2), + DstAddrs: fx.RandomIPv4Addresses(2), + DstPorts: []int32{}, + }, + } + + for _, c := range cases { + t.Run(c.TestName, func(t *testing.T) { + var ( + rules = fx.Azure().NoiseSecurityRules() + sg = fx.Azure().SecurityGroup().WithRules(rules).Build() + helper = ExpectNewSecurityGroupHelper(t, sg) + ) + + err := helper.AddRuleForBlockedIPRanges(c.SrcIPRanges, c.Protocol, c.DstAddrs, c.DstPorts) + assert.NoError(t, err) + + out, updated, err := helper.SecurityGroup() + assert.NoError(t, err) + assert.True(t, updated) + + srcPrefixesStr := fnutil.Map(func(p netip.Prefix) string { return p.String() }, c.SrcIPRanges) + expectedRule := fx.Azure().DenyBlockedIPRangeSecurityRule(c.Protocol, c.IPFamily, srcPrefixesStr, c.DstPorts). + WithDestination(fnutil.Map(func(a netip.Addr) string { return a.String() }, c.DstAddrs)...). + Build() + + testutil.ExpectHasSecurityRules(t, out, []*armnetwork.SecurityRule{expectedRule}) + }) + } + }) + + t.Run("merge destinations on repeated call same rule set", func(t *testing.T) { + srcPrefixes := fx.RandomIPv6Prefixes(2) + dstA := fx.RandomIPv6Addresses(2) + dstB := fx.RandomIPv6Addresses(1) + protocol := armnetwork.SecurityRuleProtocolTCP + dstPorts := []int32{443} + + sg := fx.Azure().SecurityGroup().WithRules(fx.Azure().NoiseSecurityRules()).Build() + helper := ExpectNewSecurityGroupHelper(t, sg) + + err := helper.AddRuleForBlockedIPRanges(srcPrefixes, protocol, dstA, dstPorts) + assert.NoError(t, err) + err = helper.AddRuleForBlockedIPRanges(srcPrefixes, protocol, dstB, dstPorts) + assert.NoError(t, err) + + out, _, err := helper.SecurityGroup() + assert.NoError(t, err) + + merged := append(dstA, dstB...) + mergedStrs := fnutil.Map(func(a netip.Addr) string { return a.String() }, merged) + sort.Strings(mergedStrs) + + expectedRule := fx.Azure().DenyBlockedIPRangeSecurityRule(protocol, iputil.IPv6, fnutil.Map(func(p netip.Prefix) string { return p.String() }, srcPrefixes), dstPorts). + WithDestination(mergedStrs...). + Build() + + testutil.ExpectHasSecurityRules(t, out, []*armnetwork.SecurityRule{expectedRule}) + }) + + t.Run("create new rule on repeated call different rule set", func(t *testing.T) { + srcPrefixes := fx.RandomIPv6Prefixes(2) + dstA := fx.RandomIPv6Addresses(2) + dstB := fx.RandomIPv6Addresses(1) + protocolA := armnetwork.SecurityRuleProtocolTCP + protocolB := armnetwork.SecurityRuleProtocolUDP + dstPorts := []int32{443} + + sg := fx.Azure().SecurityGroup().WithRules(fx.Azure().NoiseSecurityRules()).Build() + helper := ExpectNewSecurityGroupHelper(t, sg) + + err := helper.AddRuleForBlockedIPRanges(srcPrefixes, protocolA, dstA, dstPorts) + assert.NoError(t, err) + err = helper.AddRuleForBlockedIPRanges(srcPrefixes, protocolB, dstB, dstPorts) + assert.NoError(t, err) + + out, _, err := helper.SecurityGroup() + assert.NoError(t, err) + + srcPrefixesStr := fnutil.Map(func(p netip.Prefix) string { return p.String() }, srcPrefixes) + expectedRuleA := fx.Azure().DenyBlockedIPRangeSecurityRule(protocolA, iputil.IPv6, srcPrefixesStr, dstPorts). + WithDestination(fnutil.Map(func(a netip.Addr) string { return a.String() }, dstA)...). + WithPriority(400). + Build() + expectedRuleB := fx.Azure().DenyBlockedIPRangeSecurityRule(protocolB, iputil.IPv6, srcPrefixesStr, dstPorts). + WithDestination(fnutil.Map(func(a netip.Addr) string { return a.String() }, dstB)...). + WithPriority(401). + Build() + + testutil.ExpectHasSecurityRules(t, out, []*armnetwork.SecurityRule{expectedRuleA, expectedRuleB}) + }) +} diff --git a/pkg/provider/securitygroup/securityrule.go b/pkg/provider/securitygroup/securityrule.go index 639aaf59fb..5fca3c2481 100644 --- a/pkg/provider/securitygroup/securityrule.go +++ b/pkg/provider/securitygroup/securityrule.go @@ -41,6 +41,15 @@ func IsManagedSecurityRule(r *armnetwork.SecurityRule) bool { return strings.HasPrefix(*r.Name, SecurityRuleNamePrefix) && consts.LoadBalancerMinimumPriority <= priority && priority <= consts.LoadBalancerMaximumPriority } +// IsManagedPrefixBlockingSecurityRule returns true if the security rule is a prefix blocking rule managed by the cloud provider. +func IsManagedPrefixBlockingSecurityRule(r *armnetwork.SecurityRule) bool { + if r == nil || r.Name == nil || r.Properties == nil || r.Properties.Priority == nil { + return false + } + priority := *r.Properties.Priority + return strings.HasPrefix(*r.Name, SecurityRuleNamePrefix) && consts.IPPrefixBlockingMinimumPriority <= priority && priority <= consts.IPPrefixBlockingMaximumPriority +} + // GenerateAllowSecurityRuleName returns the AllowInbound rule name based on the given rule properties. func GenerateAllowSecurityRuleName( protocol armnetwork.SecurityRuleProtocol, @@ -75,6 +84,31 @@ func GenerateDenyAllSecurityRuleName(ipFamily iputil.Family) string { return strings.Join([]string{SecurityRuleNamePrefix, "deny-all", string(ipFamily)}, SecurityRuleNameSep) } +// GenerateDenyBlockedSecurityRuleName returns the DenyInbound rule name based on the given rule properties. +func GenerateDenyBlockedSecurityRuleName( + protocol armnetwork.SecurityRuleProtocol, + ipFamily iputil.Family, + srcPrefixes []string, + dstPorts []int32, +) string { + dstPortRanges := fnutil.Map(func(p int32) string { return strconv.FormatInt(int64(p), 10) }, dstPorts) + + sort.Strings(srcPrefixes) + sort.Strings(dstPortRanges) + + v := strings.Join([]string{ + string(protocol), + strings.Join(srcPrefixes, ","), + strings.Join(dstPortRanges, ","), + }, "_") + + h := md5.New() //nolint:gosec + h.Write([]byte(v)) + ruleID := fmt.Sprintf("%x", h.Sum(nil)) + + return strings.Join([]string{SecurityRuleNamePrefix, "deny-blocked", string(ipFamily), ruleID}, SecurityRuleNameSep) +} + // NormalizeSecurityRuleAddressPrefixes normalizes the given rule address prefixes. func NormalizeSecurityRuleAddressPrefixes(vs []string) []string { // Remove redundant addresses. diff --git a/pkg/provider/securitygroup/securityrule_test.go b/pkg/provider/securitygroup/securityrule_test.go index a0c4e36f35..2f67c260bc 100644 --- a/pkg/provider/securitygroup/securityrule_test.go +++ b/pkg/provider/securitygroup/securityrule_test.go @@ -22,6 +22,8 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v6" "github.com/stretchr/testify/assert" + + "sigs.k8s.io/cloud-provider-azure/pkg/util/iputil" ) func TestSetDestinationPortRanges(t *testing.T) { @@ -164,3 +166,138 @@ func TestSetAsteriskDestinationPortRanges(t *testing.T) { }) } } + +func TestGenerateDenyBlockedSecurityRuleName(t *testing.T) { + t.Parallel() + + tests := []struct { + Name string + Protocol armnetwork.SecurityRuleProtocol + IPFamily iputil.Family + SrcPrefixes []string + DstPorts []int32 + Expected string + }{ + { + Name: "TCP with IPv4", + Protocol: armnetwork.SecurityRuleProtocolTCP, + IPFamily: iputil.IPv4, + SrcPrefixes: []string{"10.0.0.1", "10.0.0.2"}, + DstPorts: []int32{80, 443}, + Expected: "k8s-azure-lb_deny-blocked_IPv4_0070730d5e225d645e76e1c23e2cf80f", + }, + { + Name: "TCP with IPv6", + Protocol: armnetwork.SecurityRuleProtocolTCP, + IPFamily: iputil.IPv6, + SrcPrefixes: []string{"::1"}, + DstPorts: []int32{443}, + Expected: "k8s-azure-lb_deny-blocked_IPv6_7cdb7a69bd6ce190cf1c1bbdaef78abe", + }, + { + Name: "UDP with IPv4", + Protocol: armnetwork.SecurityRuleProtocolUDP, + IPFamily: iputil.IPv4, + SrcPrefixes: []string{"192.168.1.0/24"}, + DstPorts: []int32{53}, + Expected: "k8s-azure-lb_deny-blocked_IPv4_0fc4a31a46f267e7bb265f58235b91e2", + }, + { + Name: "Asterisk protocol with IPv4", + Protocol: armnetwork.SecurityRuleProtocolAsterisk, + IPFamily: iputil.IPv4, + SrcPrefixes: []string{"0.0.0.0/0"}, + DstPorts: []int32{22}, + Expected: "k8s-azure-lb_deny-blocked_IPv4_d05a837b8427e7f73f5b500ab4f42ba5", + }, + { + Name: "order-insensitive for srcPrefixes", + Protocol: armnetwork.SecurityRuleProtocolTCP, + IPFamily: iputil.IPv4, + SrcPrefixes: []string{"bar", "foo"}, + DstPorts: []int32{80}, + Expected: "k8s-azure-lb_deny-blocked_IPv4_e9f1431f8e7d696203a35c2424eb98ee", + }, + { + Name: "order-insensitive for srcPrefixes reversed", + Protocol: armnetwork.SecurityRuleProtocolTCP, + IPFamily: iputil.IPv4, + SrcPrefixes: []string{"foo", "bar"}, + DstPorts: []int32{80}, + Expected: "k8s-azure-lb_deny-blocked_IPv4_e9f1431f8e7d696203a35c2424eb98ee", + }, + { + Name: "order-insensitive for dstPorts", + Protocol: armnetwork.SecurityRuleProtocolTCP, + IPFamily: iputil.IPv4, + SrcPrefixes: []string{"test"}, + DstPorts: []int32{443, 80}, + Expected: "k8s-azure-lb_deny-blocked_IPv4_e9f186e3c1c2ae7997154387a55790e8", + }, + { + Name: "order-insensitive for dstPorts reversed", + Protocol: armnetwork.SecurityRuleProtocolTCP, + IPFamily: iputil.IPv4, + SrcPrefixes: []string{"test"}, + DstPorts: []int32{80, 443}, + Expected: "k8s-azure-lb_deny-blocked_IPv4_e9f186e3c1c2ae7997154387a55790e8", + }, + { + Name: "empty srcPrefixes", + Protocol: armnetwork.SecurityRuleProtocolTCP, + IPFamily: iputil.IPv4, + SrcPrefixes: []string{}, + DstPorts: []int32{80}, + Expected: "k8s-azure-lb_deny-blocked_IPv4_a17e1d096eba6a1f917b0d4060ab0274", + }, + { + Name: "empty dstPorts", + Protocol: armnetwork.SecurityRuleProtocolTCP, + IPFamily: iputil.IPv4, + SrcPrefixes: []string{"10.0.0.1"}, + DstPorts: []int32{}, + Expected: "k8s-azure-lb_deny-blocked_IPv4_091b9c8f2039b672941e530e6b95dccc", + }, + { + Name: "baseline for parameter change tests", + Protocol: armnetwork.SecurityRuleProtocolTCP, + IPFamily: iputil.IPv4, + SrcPrefixes: []string{"10.0.0.0/8"}, + DstPorts: []int32{80}, + Expected: "k8s-azure-lb_deny-blocked_IPv4_2e80b3cfd2a6a84924746e76e404cce0", + }, + { + Name: "changing protocol produces different name", + Protocol: armnetwork.SecurityRuleProtocolUDP, + IPFamily: iputil.IPv4, + SrcPrefixes: []string{"10.0.0.0/8"}, + DstPorts: []int32{80}, + Expected: "k8s-azure-lb_deny-blocked_IPv4_ecbcbf9c2a64eaaeca9e08da2c629f69", + }, + { + Name: "changing srcPrefixes produces different name", + Protocol: armnetwork.SecurityRuleProtocolTCP, + IPFamily: iputil.IPv4, + SrcPrefixes: []string{"172.16.0.0/12"}, + DstPorts: []int32{80}, + Expected: "k8s-azure-lb_deny-blocked_IPv4_32431d2a7416344890bba9ad9df36cf4", + }, + { + Name: "changing dstPorts produces different name", + Protocol: armnetwork.SecurityRuleProtocolTCP, + IPFamily: iputil.IPv4, + SrcPrefixes: []string{"10.0.0.0/8"}, + DstPorts: []int32{443}, + Expected: "k8s-azure-lb_deny-blocked_IPv4_a062a047507bb6d8c00fa9c4ce7515ab", + }, + } + + for _, tt := range tests { + t.Run(tt.Name, func(t *testing.T) { + t.Parallel() + + result := GenerateDenyBlockedSecurityRuleName(tt.Protocol, tt.IPFamily, tt.SrcPrefixes, tt.DstPorts) + assert.Equal(t, tt.Expected, result) + }) + } +} diff --git a/tests/e2e/network/network_security_group.go b/tests/e2e/network/network_security_group.go index d39d03cf24..dbcc54ab76 100644 --- a/tests/e2e/network/network_security_group.go +++ b/tests/e2e/network/network_security_group.go @@ -627,6 +627,235 @@ var _ = Describe("Network security group", Label(utils.TestSuiteLabelNSG), func( }) }) + When("creating a LoadBalancer service with annotation `service.beta.kubernetes.io/azure-blocked-ip-ranges`", func() { + It("should add deny rules for blocked IPs", func() { + var ( + serviceIPv4s []netip.Addr + serviceIPv6s []netip.Addr + blockedIPv4Ranges = []string{ + "10.1.0.0/16", "172.16.0.0/12", "192.168.1.0/24", "203.0.113.0/24", + } + blockedIPv6Ranges = []string{ + "fd12:abcd::/48", "fe80:1234::/32", "2001:db8:1::/48", "2001:db8:2::/48", + } + ) + + By("Creating a LoadBalancer service", func() { + var ( + labels = map[string]string{ + "app": ServiceName, + } + annotations = map[string]string{ + consts.ServiceAnnotationBlockedIPRanges: strings.Join( + append(blockedIPv4Ranges, blockedIPv6Ranges...), + ",", + ), + } + ports = []v1.ServicePort{{ + Port: serverPort, + TargetPort: intstr.FromInt32(serverPort), + }} + ) + rv := createAndExposeDefaultServiceWithAnnotation(k8sClient, azureClient.IPFamily, ServiceName, namespace.Name, labels, annotations, ports) + serviceIPv4s, serviceIPv6s = groupIPsByFamily(mustParseIPs(derefSliceOfStringPtr(rv))) + }) + logger.Info("Created a LoadBalancer service", "v4-IPs", serviceIPv4s, "v6-IPs", serviceIPv6s) + + var validator *SecurityGroupValidator + By("Getting the cluster security groups", func() { + rv, err := azureClient.GetClusterSecurityGroups() + Expect(err).NotTo(HaveOccurred()) + + validator = NewSecurityGroupValidator(rv) + }) + + By("Checking if the blocked IP range rules exist", func() { + var ( + expectedProtocol = armnetwork.SecurityRuleProtocolTCP + expectedDstPorts = []string{strconv.FormatInt(int64(serverPort), 10)} + ) + + if len(serviceIPv4s) > 0 { + actualBlockedIPv4Ranges := validator.GetBlockedIPRanges(expectedProtocol, serviceIPv4s, expectedDstPorts) + Expect(actualBlockedIPv4Ranges).NotTo(BeEmpty(), "Should have blocked IP range rules for IPv4") + Expect(sets.NewString(actualBlockedIPv4Ranges...).Equal(sets.NewString(blockedIPv4Ranges...))).To(BeTrue(), + "Blocked IPv4 ranges should match expected, got: %v, expected: %v", actualBlockedIPv4Ranges, blockedIPv4Ranges) + } + if len(serviceIPv6s) > 0 { + actualBlockedIPv6Ranges := validator.GetBlockedIPRanges(expectedProtocol, serviceIPv6s, expectedDstPorts) + Expect(actualBlockedIPv6Ranges).NotTo(BeEmpty(), "Should have blocked IP range rules for IPv6") + Expect(sets.NewString(actualBlockedIPv6Ranges...).Equal(sets.NewString(blockedIPv6Ranges...))).To(BeTrue(), + "Blocked IPv6 ranges should match expected, got: %v, expected: %v", actualBlockedIPv6Ranges, blockedIPv6Ranges) + } + }) + }) + + It("should not add blocked IP range rules when no ranges are specified", func() { + var ( + serviceIPv4s []netip.Addr + serviceIPv6s []netip.Addr + ) + + By("Creating a LoadBalancer service without blocked-ip-ranges annotation", func() { + var ( + labels = map[string]string{ + "app": ServiceName, + } + annotations = map[string]string{} + ports = []v1.ServicePort{{ + Port: serverPort, + TargetPort: intstr.FromInt32(serverPort), + }} + ) + rv := createAndExposeDefaultServiceWithAnnotation(k8sClient, azureClient.IPFamily, ServiceName, namespace.Name, labels, annotations, ports) + serviceIPv4s, serviceIPv6s = groupIPsByFamily(mustParseIPs(derefSliceOfStringPtr(rv))) + }) + logger.Info("Created a LoadBalancer service", "v4-IPs", serviceIPv4s, "v6-IPs", serviceIPv6s) + + var validator *SecurityGroupValidator + By("Getting the cluster security groups", func() { + rv, err := azureClient.GetClusterSecurityGroups() + Expect(err).NotTo(HaveOccurred()) + + validator = NewSecurityGroupValidator(rv) + }) + + By("Checking that no blocked IP range rules exist", func() { + var ( + expectedProtocol = armnetwork.SecurityRuleProtocolTCP + expectedDstPorts = []string{strconv.FormatInt(int64(serverPort), 10)} + ) + + if len(serviceIPv4s) > 0 { + actualBlockedIPv4Ranges := validator.GetBlockedIPRanges(expectedProtocol, serviceIPv4s, expectedDstPorts) + Expect(actualBlockedIPv4Ranges).To(BeEmpty(), "Should not have any blocked IP range rule for IPv4, got: %v", actualBlockedIPv4Ranges) + } + if len(serviceIPv6s) > 0 { + actualBlockedIPv6Ranges := validator.GetBlockedIPRanges(expectedProtocol, serviceIPv6s, expectedDstPorts) + Expect(actualBlockedIPv6Ranges).To(BeEmpty(), "Should not have any blocked IP range rule for IPv6, got: %v", actualBlockedIPv6Ranges) + } + }) + }) + + It("should remove deny rules when blocked IP ranges annotation is removed", func() { + var ( + serviceIPv4s []netip.Addr + serviceIPv6s []netip.Addr + blockedIPv4Ranges = []string{ + "10.1.0.0/16", "172.16.0.0/12", + } + blockedIPv6Ranges = []string{ + "fd12:abcd::/48", "fe80:1234::/32", + } + ) + + By("Creating a LoadBalancer service with blocked-ip-ranges annotation", func() { + var ( + labels = map[string]string{ + "app": ServiceName, + } + annotations = map[string]string{ + consts.ServiceAnnotationBlockedIPRanges: strings.Join( + append(blockedIPv4Ranges, blockedIPv6Ranges...), + ",", + ), + } + ports = []v1.ServicePort{{ + Port: serverPort, + TargetPort: intstr.FromInt32(serverPort), + }} + ) + rv := createAndExposeDefaultServiceWithAnnotation(k8sClient, azureClient.IPFamily, ServiceName, namespace.Name, labels, annotations, ports) + serviceIPv4s, serviceIPv6s = groupIPsByFamily(mustParseIPs(derefSliceOfStringPtr(rv))) + }) + logger.Info("Created a LoadBalancer service with blocked IP ranges", "v4-IPs", serviceIPv4s, "v6-IPs", serviceIPv6s) + + var validator *SecurityGroupValidator + By("Verifying blocked IP range rules exist", func() { + rv, err := azureClient.GetClusterSecurityGroups() + Expect(err).NotTo(HaveOccurred()) + + validator = NewSecurityGroupValidator(rv) + + var ( + expectedProtocol = armnetwork.SecurityRuleProtocolTCP + expectedDstPorts = []string{strconv.FormatInt(int64(serverPort), 10)} + ) + + if len(serviceIPv4s) > 0 { + actualBlockedIPv4Ranges := validator.GetBlockedIPRanges(expectedProtocol, serviceIPv4s, expectedDstPorts) + Expect(actualBlockedIPv4Ranges).NotTo(BeEmpty(), "Should have blocked IP range rules for IPv4") + Expect(sets.NewString(actualBlockedIPv4Ranges...).Equal(sets.NewString(blockedIPv4Ranges...))).To(BeTrue(), + "Blocked IPv4 ranges should match expected, got: %v, expected: %v", actualBlockedIPv4Ranges, blockedIPv4Ranges) + } + if len(serviceIPv6s) > 0 { + actualBlockedIPv6Ranges := validator.GetBlockedIPRanges(expectedProtocol, serviceIPv6s, expectedDstPorts) + Expect(actualBlockedIPv6Ranges).NotTo(BeEmpty(), "Should have blocked IP range rules for IPv6") + Expect(sets.NewString(actualBlockedIPv6Ranges...).Equal(sets.NewString(blockedIPv6Ranges...))).To(BeTrue(), + "Blocked IPv6 ranges should match expected, got: %v, expected: %v", actualBlockedIPv6Ranges, blockedIPv6Ranges) + } + }) + + By("Removing the blocked-ip-ranges annotation from the service", func() { + svc, err := k8sClient.CoreV1().Services(namespace.Name).Get(context.Background(), ServiceName, metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + + delete(svc.Annotations, consts.ServiceAnnotationBlockedIPRanges) + + _, err = k8sClient.CoreV1().Services(namespace.Name).Update(context.Background(), svc, metav1.UpdateOptions{}) + Expect(err).NotTo(HaveOccurred()) + }) + logger.Info("Removed blocked-ip-ranges annotation from service") + + By("Waiting for the security group rules to be reconciled", func() { + _, err := utils.WaitServiceExposureAndValidateConnectivity(k8sClient, azureClient.IPFamily, namespace.Name, ServiceName, nil) + Expect(err).NotTo(HaveOccurred()) + }) + + By("Getting the cluster security groups after removal", func() { + rv, err := azureClient.GetClusterSecurityGroups() + Expect(err).NotTo(HaveOccurred()) + + validator = NewSecurityGroupValidator(rv) + }) + + By("Verifying blocked IP range rules are removed", func() { + var ( + expectedProtocol = armnetwork.SecurityRuleProtocolTCP + expectedDstPorts = []string{strconv.FormatInt(int64(serverPort), 10)} + ) + + if len(serviceIPv4s) > 0 { + actualBlockedIPv4Ranges := validator.GetBlockedIPRanges(expectedProtocol, serviceIPv4s, expectedDstPorts) + Expect(actualBlockedIPv4Ranges).To(BeEmpty(), "Should not have any blocked IP range rule for IPv4 after removal, got: %v", actualBlockedIPv4Ranges) + } + if len(serviceIPv6s) > 0 { + actualBlockedIPv6Ranges := validator.GetBlockedIPRanges(expectedProtocol, serviceIPv6s, expectedDstPorts) + Expect(actualBlockedIPv6Ranges).To(BeEmpty(), "Should not have any blocked IP range rule for IPv6 after removal, got: %v", actualBlockedIPv6Ranges) + } + }) + + By("Verifying allow rule from Internet still exists", func() { + var ( + expectedProtocol = armnetwork.SecurityRuleProtocolTCP + expectedSrcPrefixes = []string{"Internet"} + expectedDstPorts = []string{strconv.FormatInt(int64(serverPort), 10)} + ) + + if len(serviceIPv4s) > 0 { + Expect( + validator.HasExactAllowRule(expectedProtocol, expectedSrcPrefixes, serviceIPv4s, expectedDstPorts), + ).To(BeTrue(), "Should have a rule for allowing IPv4 traffic from Internet after blocked IP ranges removal") + } + if len(serviceIPv6s) > 0 { + Expect( + validator.HasExactAllowRule(expectedProtocol, expectedSrcPrefixes, serviceIPv6s, expectedDstPorts), + ).To(BeTrue(), "Should have a rule for allowing IPv6 traffic from Internet after blocked IP ranges removal") + } + }) + }) + }) + When("creating a LoadBalancer service with annotation `service.beta.kubernetes.io/azure-allowed-ip-ranges`", func() { It("should add a rule to allow traffic from allowed-IPs only", func() { var ( @@ -1217,6 +1446,21 @@ func (v *SecurityGroupValidator) NotHasRuleForDestination(dstAddresses []netip.A return true } +// GetBlockedIPRanges returns the blocked IP ranges for the given destination addresses and ports. +func (v *SecurityGroupValidator) GetBlockedIPRanges( + protocol armnetwork.SecurityRuleProtocol, + dstAddresses []netip.Addr, + dstPorts []string, +) []string { + for i := range v.nsgs { + ranges := SecurityGroupGetBlockedIPRanges(v.nsgs[i], protocol, dstAddresses, dstPorts) + if len(ranges) > 0 { + return ranges + } + } + return nil +} + // HasDenyAllRuleForDestination checks if the security group has a rule that denies all traffic to the given destination addresses. func (v *SecurityGroupValidator) HasDenyAllRuleForDestination(dstAddresses []netip.Addr) bool { for i := range v.nsgs { @@ -1351,6 +1595,83 @@ func SecurityGroupHasAllowRuleForDestination( return true } +func SecurityGroupGetBlockedIPRanges( + nsg *armnetwork.SecurityGroup, + protocol armnetwork.SecurityRuleProtocol, + dstAddresses []netip.Addr, dstPorts []string, +) []string { + logger := GinkgoLogr.WithName("GetBlockedIPRanges"). + WithValues("nsg-name", nsg.Name) + + logger.Info("checking", + "expected-protocol", protocol, + "expected-dst-addresses", dstAddresses, + "expected-dst-ports", dstPorts, + ) + + if len(dstAddresses) == 0 { + logger.Info("skip as no destination addresses") + return nil + } + + var ( + expectedDstPorts = sets.NewString(dstPorts...) + expectedDstAddresses = sets.NewString() + ) + for _, ip := range dstAddresses { + expectedDstAddresses.Insert(ip.String()) + } + + for _, rule := range nsg.Properties.SecurityRules { + if *rule.Properties.Access != armnetwork.SecurityRuleAccessDeny || + *rule.Properties.Direction != armnetwork.SecurityRuleDirectionInbound || + *rule.Properties.Protocol != protocol || + ptr.Deref(rule.Properties.SourcePortRange, "") != "*" || + len(rule.Properties.DestinationPortRanges) != len(dstPorts) { + logger.Info("skip rule", "rule", rule) + continue + } + logger.Info("checking rule", "rule", rule) + + { + // check destination ports + actualDstPorts := sets.NewString() + for _, d := range rule.Properties.DestinationPortRanges { + actualDstPorts.Insert(*d) + } + if !actualDstPorts.Equal(expectedDstPorts) { + // skip if the rule does not match the expected destination ports + continue + } + } + + // Check if destination addresses match + remainingDstAddresses := expectedDstAddresses.Union(nil) // copy + if rule.Properties.DestinationAddressPrefix != nil { + remainingDstAddresses.Delete(*rule.Properties.DestinationAddressPrefix) + } + for _, d := range rule.Properties.DestinationAddressPrefixes { + remainingDstAddresses.Delete(*d) + } + + if remainingDstAddresses.Len() == 0 { + // Found matching rule, return the blocked IP ranges + var blockedIPRanges []string + if rule.Properties.SourceAddressPrefix != nil { + blockedIPRanges = append(blockedIPRanges, *rule.Properties.SourceAddressPrefix) + } + for _, d := range rule.Properties.SourceAddressPrefixes { + blockedIPRanges = append(blockedIPRanges, *d) + } + logger.Info("found blocked IP ranges", "ranges", blockedIPRanges) + return blockedIPRanges + } + } + + logger.Info("no blocked IP range rule for destination addresses") + return nil +} + func SecurityGroupHasDenyAllRuleForDestination(nsg *armnetwork.SecurityGroup, dstAddresses []netip.Addr) bool { logger := GinkgoLogr.WithName("HasDenyAllRuleForDestination"). WithValues("nsg-name", nsg.Name).