[NLB] Wrong source ranges used for backend security group on old (pre 2023) NLB without own SG if `preserveClientIP=true`

[frittentheke]

Bug Description

This bug only appears in the context of using an old (created prior to ~ 2023) AWS Network Loadbalancer.
Back then NLBs did not have Security Groups (https://aws.amazon.com/blogs/containers/network-load-balancers-now-support-security-groups/) and those CANNOT be added without replacement of the NLB:

AWS LBC will manage backend security groups for these old NLBs differently, see

aws-load-balancer-controller/pkg/gateway/model/model_build_target_group_binding_network.go

Lines 46 to 48 in 5cfeef8

	if len(builder.sgOutput.securityGroupTokens) == 0 {
	return builder.nlbNoSecurityGroups(targetPort, targetGroupSpec)
	}

when it branches off to nlbNoSecurityGroups vs. the standardBuilder!

Steps to Reproduce

• Step-by-step guide to reproduce the bug:

  • Ensure you are using an NLB which DOES NOT have security groups.
  • Create an internal NLB with preserveClientIP set to true
  • Check out backend security group does not NOT contain 0.0.0.0/0 or ::0/0 as allowed sources

• Manifests applied while reproducing the issue:

• Controller logs/error messages while reproducing the issue:

Expected Behavior
When using preserveClientIP e.g. via the

service.beta.kubernetes.io/aws-load-balancer-target-group-attributes: preserve_client_ip.enabled=true

annotation, traffic reaching the Pods originates not from the internal NLB interfaces, but from the actual client IP. This requires (by default) to allow 0.0.0.0/0 for IPv4 or ::/0 for IPv6. By default and without client ip preservation traffic originates from the internal NLB interfaces and the corresponding subnets are all that needs to be allowed in the backend security group.

Actual Behavior
It seems that the condition of isPreserveClientIP is handled the wrong way ...

1. In

   aws-load-balancer-controller/pkg/gateway/model/model_build_target_group_binding_network.go

   Line 150 in 5cfeef8

   trafficSource := loadBalancerSubnetCIDRs

   and

   aws-load-balancer-controller/pkg/gateway/model/model_build_target_group_binding_network.go

   Lines 156 to 161 in 5cfeef8

   	if isPreserveClientIP {
   	if builder.lbSourceRanges != nil {
   	trafficSource = *builder.lbSourceRanges
   	} else {
   	trafficSource = []string{}
   	}

   the lb source ranges (subnets) are added, which should only happen on isPreserveClientIP=false.

2. Then

   aws-load-balancer-controller/pkg/gateway/model/model_build_target_group_binding_network.go

   Lines 254 to 267 in 5cfeef8

   	if (preserveClientIP) && builder.lbScheme == elbv2model.LoadBalancerSchemeInternal {
   	vpcInfo, err := builder.vpcInfoProvider.FetchVPCInfo(context.Background(), builder.vpcID, networking.FetchVPCInfoWithoutCache())
   	if err != nil {
   	return nil, err
   	}
   	if tgSpec.IPAddressType == elbv2model.TargetGroupIPAddressTypeIPv4 {
   	defaultSourceRanges = vpcInfo.AssociatedIPv4CIDRs()
   	} else {
   	defaultSourceRanges = vpcInfo.AssociatedIPv6CIDRs()
   	}
   	}
   	return defaultSourceRanges, nil
   	}

   applies all VPC CIDRs as source for internal NLBs in case of preserveClientIP=true. While it's a sensible approach it's contrary what

• https://kubernetes-sigs.github.io/aws-load-balancer-controller/v3.1/guide/service/nlb/#worker-node-security-groups-rules

• https://kubernetes-sigs.github.io/aws-load-balancer-controller/v3.1/guide/service/annotations/#lb-source-ranges

documents, with "0.0.0.0/0" being the default also for internal NLB and with the recommendation given:

For enhanced security with internal network load balancers, we recommend limiting access by specifying allowed source IP ranges. This can be done using either the service.beta.kubernetes.io/load-balancer-source-ranges annotation or the spec.loadBalancerSourceRanges field.

While it somewhat makes sense to at first to allow "VPC CIDR only" for internal NLBs, this does not take into account any VPC peerings or other traffic from outside the VPC reaching the NLB. But most importantly this behavior is NOT aligned between old and new NLBs and thus somewhat surprising.

Regression
I believe yes, but I need to dig a little to find it.

Current Workarounds

Environment

• AWS Load Balancer controller version: 3.1.0
• Kubernetes version: 1.34
• Using EKS (yes/no), if so version?: yes, v1.34.4-eks-3a10415
• Using Service or Ingress: Service
• AWS region: eu-central-1
• How was the aws-load-balancer-controller installed:
  • If helm was used then please show output of helm ls -A | grep -i aws-load-balancer-controller
  • If helm was used then please show output of helm -n <controllernamespace> get values <helmreleasename>

mostly default:

USER-SUPPLIED VALUES:
aws-load-balancer-controller:
  clusterName: mycluster
  serviceAccount:
    annotations:
      eks.amazonaws.com/role-arn: REDACTED
  serviceMonitor:
    enabled: true

• kubectl -n <appnamespace> describe svc <servicename>

apiVersion: v1
kind: Service
metadata:
  annotations:
    meta.helm.sh/release-name: istio-ingressgateway
    meta.helm.sh/release-namespace: istio-ingress
    service.beta.kubernetes.io/aws-load-balancer-attributes: load_balancing.cross_zone.enabled=true,
      dns_record.client_routing_policy=availability_zone_affinity
    service.beta.kubernetes.io/aws-load-balancer-healthcheck-healthy-threshold: "2"
    service.beta.kubernetes.io/aws-load-balancer-healthcheck-interval: "5"
    service.beta.kubernetes.io/aws-load-balancer-healthcheck-timeout: "2"
    service.beta.kubernetes.io/aws-load-balancer-healthcheck-unhealthy-threshold: "2"
    service.beta.kubernetes.io/aws-load-balancer-manage-backend-security-group-rules: "true"
    service.beta.kubernetes.io/aws-load-balancer-nlb-target-type: ip
    service.beta.kubernetes.io/aws-load-balancer-scheme: internet-facing
    service.beta.kubernetes.io/aws-load-balancer-target-group-attributes: preserve_client_ip.enabled=true,
      proxy_protocol_v2.enabled=true, deregistration_delay.timeout_seconds=120, deregistration_delay.connection_termination.enabled=true
    service.beta.kubernetes.io/aws-load-balancer-type: external
  creationTimestamp: "2022-05-19T11:17:40Z"
  finalizers:
  - service.kubernetes.io/load-balancer-cleanup
  - service.k8s.aws/resources
  labels:
    app: istio-ingressgateway
    app.kubernetes.io/instance: istio-ingressgateway
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: istio-ingressgateway
    app.kubernetes.io/part-of: istio
    app.kubernetes.io/version: 1.29.1
    helm.sh/chart: gateway-1.29.1
    istio: ingressgateway
    istio.io/dataplane-mode: none
  name: istio-ingressgateway
  namespace: istio-ingress
spec:
  allocateLoadBalancerNodePorts: true
  clusterIP: REDACTED
  clusterIPs:
  - REDACTED
  externalTrafficPolicy: Cluster
  internalTrafficPolicy: Cluster
  ipFamilies:
  - IPv4
  ipFamilyPolicy: SingleStack
  ports:
  - name: http2
    nodePort: 30649
    port: 80
    protocol: TCP
    targetPort: 80
  - name: https
    nodePort: 32766
    port: 443
    protocol: TCP
    targetPort: 443
  selector:
    app: istio-ingressgateway
    istio: ingressgateway
  sessionAffinity: None
  type: LoadBalancer

Possible Solution (Optional)

Contribution Intention (Optional)

• Yes, I'm willing to submit a PR to fix this issue
• No, I cannot work on a PR at this time

Additional Context

• It seems that Security Group Inbound is automatically set to open 0.0.0.0 #3706 by @Rishabh-Hupr is exactly about the usual behavior of setting "0.0.0.0/0" also for internal NLBs, which then Fix docs for source ranges for internal NLB #4057 documented more specifically.

[zac-nixon]

Ouch, thanks for letting us know. We'll fix this in our next release.

[frittentheke]

@zac-nixon thanks for the quick response - we are looking forward to the next release then :-)

Another thing we noticed when debugging this issue ...

1. If one has got an internal and an external NLB there is a "0.0.0.0/0" rule added for the external NLB, "overlaying" this issue somewhat.
2. But, if one scales the targets (e.g. Ingress Controller, Istio Gateways, ...) of the external to "0", the security group is updated / the "0.0.0.0/0" from the external NLB is removed.

Is it intentional, that the security group is updated depending on targets existing (empty target group) or not?
To me, the SG rules are more about a configuration matching what Service type LB has been deployed, then about the actual existence of targets in the target group?

[zac-nixon]

@frittentheke I cross-checked this behavior against an earlier release (v2.7.2) and confirmed that the behavior has remained consistent from v2.7.2 through v3.1.0, which suggests this is a documentation gap rather than a regression. I apologize for the delayed response.

To verify this, I reviewed the code from approximately two years ago and confirmed it was injecting the VPC CIDR into the Security Groups: https://github.com/kubernetes-sigs/aws-load-balancer-controller/blame/356c90487eaa153f56bee2f358233a3dea2b9d65/pkg/service/model_build_target_group.go#L599-L617

I recommend the following actions:

1. Update the documentation to accurately describe how Security Group rules are generated.
2. For more complex network topologies (such as peered VPCs or transit gateways), use the source ranges field to override the default behavior.

Regarding #3706, I've identified that the user was using an internal NLB without preserve client IP enabled. The requirements are clear (atleast in code): you must use either UDP or preserve client IP AND an internal NLB. We should have reproduced this issue before updating the documentation. That was an oversight on our part.

To answer your question:

Is it intentional, that the security group is updated depending on targets existing (empty target group) or not?

While "intentional" may not be the precise term—as this behavior predates my involvement with the project—there are valid reasons for this design. The primary goal is to minimize network access to the cluster. When no entities in the cluster are providing the service, it's reasonable to remove the corresponding Security Group rules.

[frittentheke]

@frittentheke I cross-checked this behavior against an earlier release (v2.7.2) and confirmed that the behavior has remained consistent from v2.8.0 through v3.1.0, which suggests this is a documentation gap rather than a regression. I apologize for the delayed response.

Thanks @zac-nixon for going out of your way to triage this bug report!

But event with the fact that this is consistent behavior now clarified, isn't the behavior between "old" and "new" NLBs still inconsistent? Even if this was now documented and also not ignoring the fact that "old" NLBs might die out at some point, it seems that aligning the way SG are configured (VPC CIDR vs. any) still might make sense? I just ended up reporting this, because I noticed the difference between old and new NLBs, not a change (which you determined did not happen) in the behavior of the AWS LBC over time.

[zac-nixon]

It's hard to make the behavior match, tbh. The way that ALB and NLB (w/ SG) works is that we attach a singleton SG to the load balancer, and then allow ingress from that SG into the cluster nodes.

When you have an NLB w/ no SG we obviously can't do that, so we have to use other networking metadata.