loadbalancer: enforce loadBalancerSourceRanges on ExternalIPs frontends#44747
Conversation
|
Hey looks reasonable! Could you also look into adding a script test for this so we have a high-level integration test for ExternalIPs that would cover this? https://github.com/cilium/cilium/blob/main/pkg/loadbalancer/tests/testdata/source-ranges-all.txtar or https://github.com/cilium/cilium/blob/main/pkg/loadbalancer/tests/testdata/loadbalancer.txtar might be close as starting point. Maybe call it Useful references: |
ExternalIPs frontends of a LoadBalancer service were not subject to loadBalancerSourceRanges filtering, allowing traffic from outside the allowed source ranges to bypass the check via the ExternalIP address. Only the LoadBalancerIP frontend was enforcing the restriction. Fix GetSourceRangesEnabled to also return true for SVCTypeExternalIPs, and extend the test coverage to include ExternalIPs with and without source ranges, and with lbSourceRangeAllTypes enabled. Fixes: cilium#44718 Signed-off-by: Azeez Syed <syedazeez337@gmail.com>
cfd8161 to
5ba2a2e
Compare
|
/test |
|
Hi @joamaki , thank you for approving the PR. The failed check appears to be Flaky, let me know if anything comes up related to my changes. |
Doesn't seem related to your changes. I'll try re-running the test (https://github.com/cilium/cilium/actions/runs/23046248348/job/67224092249). If this still fails could you please rebase the PR? |
ExternalIPs frontends of a LoadBalancer service were not subject to
loadBalancerSourceRangesfiltering. Traffic from outside the allowedsource ranges could bypass the check via the ExternalIP address, while
the LoadBalancerIP frontend correctly enforced it.
Fix
GetSourceRangesEnabledto returntrueforSVCTypeExternalIPsin addition to
SVCTypeLoadBalancer.Fixes: #44718