[release-4.16] OCPBUGS-86724: Prevent SSRF via FQDN-typed EndpointSlices

[MrSanketkumar]

The OpenShift Router previously did not validate backend destinations resolved from FQDN-typed EndpointSlices. This allowed the usage of invalid EndpointSlices to target hostnames that resolves to restricted IPs (like the cloud metadata service at 169.254.169.254).

This commit disables the usage of EndpointSlices of type FQDN, and add validations on Endpoints to check if a restricted IP is being used before adding them to HAProxy endpoints Backend.

The implementation and disabling the usage of FQDN-backed endpoints is based on the following:

• RFE-2832 to implement support for ExternalName was considered and rejected due to security concerns, so OpenShift today does not support officially the usage of FQDN-based endpoints on router
• OCPBUGS-55506 relates to a mistake caused by the customer that caused unavailability and not the need to support FQDN-based names
• The behavior of an endpointslice of type FQDN is deprecated on Kubernetes
• The behavior of an endpointslice using an address that is not IPv4 or IPv6 (eg.: hostname) is unespecified by Kubernetes and has no usage.
• The hostname field on endpointslice is not used on router

This way, there is a common understanding that the usage of FQDN based addresses on Router was a mistake, and disabling it is the right fix.

Additional validations of the IP address on the generated endpointnt array is added to guarantee that no invalid nor restricted IP is used.

Backported : #835

Summary by CodeRabbit

• Bug Fixes
  • Endpoint processing now supports IPv4 and IPv6 address types while safely skipping unsupported types.
  • Invalid, restricted, multicast, loopback, link-local, unspecified, and cloud metadata addresses are filtered from endpoint updates.
  • Endpoint validation behavior now consistently follows the configured route-validation setting.
• Tests
  • Expanded coverage for address validation, endpoint filtering, EndpointSlice conversion, and deletion handling.

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

[openshift-ci-robot]

@MrSanketkumar: This pull request references Jira Issue OCPBUGS-86724, which is invalid:

• expected dependent Jira Issue OCPBUGS-86725 to be in one of the following states: VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA), but it is POST instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

The OpenShift Router previously did not validate backend destinations resolved from FQDN-typed EndpointSlices. This allowed the usage of invalid EndpointSlices to target hostnames that resolves to restricted IPs (like the cloud metadata service at 169.254.169.254).

This commit disables the usage of EndpointSlices of type FQDN, and add validations on Endpoints to check if a restricted IP is being used before adding them to HAProxy endpoints Backend.

The implementation and disabling the usage of FQDN-backed endpoints is based on the following:

• RFE-2832 to implement support for ExternalName was considered and rejected due to security concerns, so OpenShift today does not support officially the usage of FQDN-based endpoints on router
• OCPBUGS-55506 relates to a mistake caused by the customer that caused unavailability and not the need to support FQDN-based names
• The behavior of an endpointslice of type FQDN is deprecated on Kubernetes
• The behavior of an endpointslice using an address that is not IPv4 or IPv6 (eg.: hostname) is unespecified by Kubernetes and has no usage.
• The hostname field on endpointslice is not used on router

This way, there is a common understanding that the usage of FQDN based addresses on Router was a mistake, and disabling it is the right fix.

Additional validations of the IP address on the generated endpointnt array is added to guarantee that no invalid nor restricted IP is used.

Backported : #835

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: fa98675e-592a-4cbf-9f39-9869ea38ba78

📥 Commits

Reviewing files that changed from the base of the PR and between 94888b6 and 44de1b0.

⛔ Files ignored due to path filters (22)

• vendor/github.com/pmezard/go-difflib/LICENSE is excluded by !**/vendor/**, !vendor/**
• vendor/github.com/pmezard/go-difflib/difflib/difflib.go is excluded by !**/vendor/**, !vendor/**
• vendor/github.com/stretchr/testify/LICENSE is excluded by !**/vendor/**, !vendor/**
• vendor/github.com/stretchr/testify/assert/assertion_compare.go is excluded by !**/vendor/**, !vendor/**
• vendor/github.com/stretchr/testify/assert/assertion_format.go is excluded by !**/vendor/**, !vendor/**
• vendor/github.com/stretchr/testify/assert/assertion_format.go.tmpl is excluded by !**/vendor/**, !vendor/**
• vendor/github.com/stretchr/testify/assert/assertion_forward.go is excluded by !**/vendor/**, !vendor/**
• vendor/github.com/stretchr/testify/assert/assertion_forward.go.tmpl is excluded by !**/vendor/**, !vendor/**
• vendor/github.com/stretchr/testify/assert/assertion_order.go is excluded by !**/vendor/**, !vendor/**
• vendor/github.com/stretchr/testify/assert/assertions.go is excluded by !**/vendor/**, !vendor/**
• vendor/github.com/stretchr/testify/assert/doc.go is excluded by !**/vendor/**, !vendor/**
• vendor/github.com/stretchr/testify/assert/errors.go is excluded by !**/vendor/**, !vendor/**
• vendor/github.com/stretchr/testify/assert/forward_assertions.go is excluded by !**/vendor/**, !vendor/**
• vendor/github.com/stretchr/testify/assert/http_assertions.go is excluded by !**/vendor/**, !vendor/**
• vendor/github.com/stretchr/testify/require/doc.go is excluded by !**/vendor/**, !vendor/**
• vendor/github.com/stretchr/testify/require/forward_requirements.go is excluded by !**/vendor/**, !vendor/**
• vendor/github.com/stretchr/testify/require/require.go is excluded by !**/vendor/**, !vendor/**
• vendor/github.com/stretchr/testify/require/require.go.tmpl is excluded by !**/vendor/**, !vendor/**
• vendor/github.com/stretchr/testify/require/require_forward.go is excluded by !**/vendor/**, !vendor/**
• vendor/github.com/stretchr/testify/require/require_forward.go.tmpl is excluded by !**/vendor/**, !vendor/**
• vendor/github.com/stretchr/testify/require/requirements.go is excluded by !**/vendor/**, !vendor/**
• vendor/modules.txt is excluded by !**/vendor/**, !vendor/**

📒 Files selected for processing (8)

• go.mod
• pkg/cmd/infra/router/template.go
• pkg/router/controller/endpointsubset/converter.go
• pkg/router/controller/endpointsubset/converter_test.go
• pkg/router/controller/extended_validator.go
• pkg/router/controller/extended_validator_test.go
• pkg/router/controller/factory/factory_endpointslices_test.go
• pkg/router/template/plugin_test.go

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

---

Walkthrough

Changes

The router now always installs ExtendedValidator. It filters invalid and restricted endpoint addresses, supports IPv4 and IPv6 EndpointSlices, preserves deletion events, and applies route validation according to the configured flag.

Endpoint validation

Layer / File(s)	Summary
EndpointSlice address-type conversion pkg/router/controller/endpointsubset/converter.go, pkg/router/controller/endpointsubset/converter_test.go, pkg/router/controller/factory/factory_endpointslices_test.go	Conversion accepts IPv4 and IPv6 slices and skips FQDN, unknown, and empty address types.
ExtendedValidator endpoint filtering pkg/router/controller/extended_validator.go, pkg/router/controller/extended_validator_test.go, go.mod	ExtendedValidator filters malformed, restricted, metadata, multicast, loopback, link-local, and unspecified addresses. It preserves input data and bypasses filtering for deleted events.
Validator configuration and integration pkg/cmd/infra/router/template.go, pkg/router/template/plugin_test.go	The router always installs ExtendedValidator and passes the ExtendedValidation setting to its constructor.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to 44de1

The endpoint validation changes have no remaining actionable merge-blocking risk.

Sequence Diagram(s)

sequenceDiagram
  participant EndpointSlice
  participant ExtendedValidator
  participant RouterPlugin
  participant RouteStatusRecorder
  EndpointSlice->>ExtendedValidator: provide endpoint event
  ExtendedValidator->>ExtendedValidator: convert and filter addresses
  ExtendedValidator->>RouteStatusRecorder: record restricted address rejection
  ExtendedValidator->>RouterPlugin: forward filtered endpoints

Loading

---

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name	Status	Explanation	Resolution
No-Sensitive-Data-In-Logs	❌ Error	The PR adds log.Error(..., "address", addr.IP) in filterValidAddresses. The same log also emits err, whose messages include the raw address. Endpoints values can contain arbitrary hostnames: t…	Do not log the raw endpoint address. Return and log only a non-sensitive validation category, such as invalid, loopback, link-local, multicast, or metadata address. Remove the raw address from the error messages as well as from the structur…
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 7 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (13 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the release context, linked issue, and primary security change: preventing SSRF through FQDN-typed EndpointSlices.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	PASS: The pull request adds standard Go tests only. The changed files contain no Ginkgo imports or Describe, Context, When, It, Entry, or DescribeTable calls. The added `t.Run(tc.name, ...…
Test Structure And Quality	✅ Passed	PASS: The pull request does not add or modify Ginkgo test code. All changed tests use Go's standard testing.T API, table-driven subtests, and testify; no Describe, Context, It, Eventually,…
Microshift Test Compatibility	✅ Passed	The check is not applicable. The pull request adds standard Go unit tests with Test... functions and testing, testify/assert, and testify/require. The changed tests contain no Ginkgo construct…
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The substantive PR adds or changes only standard Go unit tests (func Test...) in pkg/router/...; it adds no Ginkgo It, Describe, Context, or When e2e tests. The changed tests import `testi…
Topology-Aware Scheduling Compatibility	✅ Passed	PASS — The pull request modifies router validation and EndpointSlice conversion only. The functional diff adds no deployment manifests or scheduling constraints: no affinity, topology spread, replica-…
Ote Binary Stdout Contract	✅ Passed	PASS. The PR adds no OTE/Ginkgo suite code and no stdout write in main(), init(), TestMain(), or suite setup. Its new production output uses the repository logger in EndpointSlice and endpoint validat…
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	PASS: The pull request adds only Go unit tests under pkg/... that use testing, t.Run, and testify. Structural searches found no new Ginkgo It, Describe, Context, or When calls. The tes…
No-Weak-Crypto	✅ Passed	No weak-crypto issue was introduced. The complete PR diff adds no MD5, SHA-1, DES, 3DES, RC4, Blowfish, ECB, custom crypto, or secret/token comparisons. The new implementation uses net/netip IP pars…
Container-Privileges	✅ Passed	PASS: The PR changes Go source, tests, go.mod, and vendor files only. The PR-range diff contains no Kubernetes manifest or Dockerfile changes and no added privilege settings. The existing `deploy/rout…

Full details: Docstring Coverage

Explanation

Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 7 files. (1 skipped: 1 unsupported.)

Full details: No-Sensitive-Data-In-Logs

Explanation

The PR adds log.Error(..., "address", addr.IP) in filterValidAddresses. The same log also emits err, whose messages include the raw address. Endpoints values can contain arbitrary hostnames: the new tests exercise evil.example.com and metadata.google.internal. Therefore an internal hostname or customer endpoint value can be written to the router logs. Restricted internal IP addresses are also logged verbatim.

Resolution

Do not log the raw endpoint address. Return and log only a non-sensitive validation category, such as invalid, loopback, link-local, multicast, or metadata address. Remove the raw address from the error messages as well as from the structured address field.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign rfredette for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[MrSanketkumar]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[openshift-ci]

@MrSanketkumar: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.