[release-4.20] OCPBUGS-86728: Prevent SSRF via FQDN-typed EndpointSlices - #819
Open
MrSanketkumar wants to merge 2 commits into
Open
[release-4.20] OCPBUGS-86728: Prevent SSRF via FQDN-typed EndpointSlices#819MrSanketkumar wants to merge 2 commits into
MrSanketkumar wants to merge 2 commits into
Conversation
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.
openshift-ci-robot
added
jira/severity-important
Contributor
|
@MrSanketkumar: This pull request references Jira Issue OCPBUGS-86728, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
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. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughChangesEndpoint and route validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant RouterTemplate
participant ExtendedValidator
participant EndpointSliceConverter
participant NextPlugin
RouterTemplate->>ExtendedValidator: construct with extended validation flag
EndpointSliceConverter->>ExtendedValidator: provide converted endpoints
ExtendedValidator->>ExtendedValidator: filter restricted endpoint addresses
ExtendedValidator->>NextPlugin: forward filtered endpoint event
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
|
@MrSanketkumar: This pull request references Jira Issue OCPBUGS-86728, which is invalid:
Comment DetailsIn response to this:
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/router/template/plugin_test.go (1)
776-776: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the disabled route-validation path.
This test now exercises only
true. Add a companion case usingNewExtendedValidator(..., false)with an invalid route to verify that unconditional wrapping does not accidentally enable extended route validation when the option is disabled.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/router/template/plugin_test.go` at line 776, Add a companion test alongside the existing NewExtendedValidator case using false for the route-validation option and an invalid route, then assert the route is not rejected by extended validation. Ensure the test specifically verifies that wrapping remains inactive when the option is disabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pkg/router/template/plugin_test.go`:
- Line 776: Add a companion test alongside the existing NewExtendedValidator
case using false for the route-validation option and an invalid route, then
assert the route is not rejected by extended validation. Ensure the test
specifically verifies that wrapping remains inactive when the option is
disabled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b1a8e14c-26d7-4826-b041-7a127429d86d
📒 Files selected for processing (8)
pkg/cmd/infra/router/template.gopkg/router/controller/endpointsubset/converter.gopkg/router/controller/endpointsubset/converter_test.gopkg/router/controller/extended_validator.gopkg/router/controller/extended_validator_test.gopkg/router/controller/factory/factory_endpointslices_test.gopkg/router/router_test.gopkg/router/template/plugin_test.go
Contributor
|
@MrSanketkumar: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
Cluster Versionoc get clusterversionOutputSSRF Protection Validation - EndpointSlice Verification ResultsTest Environment SetupCreate test namespace and serviceoc create namespace ssrf-test
oc -n ssrf-test create service clusterip metadata-svc --tcp=80:80
oc -n ssrf-test patch svc metadata-svc --type=json \
-p='[{"op":"remove","path":"/spec/selector"}]'Create routeCLUSTER_DOMAIN=$(oc get ingresses.config cluster -o jsonpath='{.spec.domain}')
oc -n ssrf-test expose svc metadata-svc \
--hostname=ssrf-test.${CLUSTER_DOMAIN}Save URLTEST_URL="http://ssrf-test.${CLUSTER_DOMAIN}/latest/meta-data/"Test 1: FQDN EndpointSlice (Layer 1)EndpointSlice cat <<EOF | oc apply -f -
apiVersion: discovery.k8s.io/v1
kind: EndpointSlice
metadata:
name: test-fqdn
namespace: ssrf-test
labels:
kubernetes.io/service-name: metadata-svc
addressType: FQDN
endpoints:
- addresses:
- "metadata.google.internal"
ports:
- port: 80
EOF
Warning: spec.addressType: FQDN endpoints are deprecated
endpointslice.discovery.k8s.io/test-fqdn created
Verificationcurl -s -o /dev/null -w "%{http_code}" $TEST_URL
503
oc -n openshift-ingress logs deployment/router-default | grep "unsupported address type"pe"
Found 2 pods, using pod/router-default-5f59648476-n5jvv
I0717 06:18:42.188302 1 converter.go:18] "msg"="Skipping EndpointSlice with unsupported address type" "addressType"="FQDN" "logger"="endpointsubset" "name"="test-fqdn" "namespace"="ssrf-test"Test 2: IPv6 Hex Metadata IP (Layer 2 Pass 2)EndpointSlicecat <<EOF | oc apply -f -
apiVersion: discovery.k8s.io/v1
kind: EndpointSlice
metadata:
name: test-ipv6-metadata-hex
namespace: ssrf-test
labels:
kubernetes.io/service-name: metadata-svc
addressType: IPv6
endpoints:
- addresses:
- "::a9fe:a9fe"
ports:
- port: 80
EOF
endpointslice.discovery.k8s.io/test-ipv6-metadata-hex createdVerificationcurl -s -o /dev/null -w "%{http_code}" $TEST_URL
503
oc -n openshift-ingress logs deployment/router-default | grep "restricted" | grep "a9fe"fe"
**Found 2 pods, using pod/router-default-5f59648476-n5jvv
E0717 06:22:45.976337 1 extended_validator.go:70] "msg"="Skipping endpoint address with restricted or invalid IP" "error"="IP address 169.254.169.254 is a restricted link-local IP" "address"="::a9fe:a9fe" "logger"="controller"**Test 3: IPv6 Hex Loopback (Layer 2 Pass 2)EndpointSlicecat <<EOF | oc apply -f -
apiVersion: discovery.k8s.io/v1
kind: EndpointSlice
metadata:
name: test-ipv6-loopback-hex
namespace: ssrf-test
labels:
kubernetes.io/service-name: metadata-svc
addressType: IPv6
endpoints:
- addresses:
- "::7f00:1"
ports:
- port: 80
EOF
endpointslice.discovery.k8s.io/test-ipv6-loopback-hex createdVerificationcurl -s -o /dev/null -w "%{http_code}" $TEST_URL
503
oc -n openshift-ingress logs deployment/router-default | grep "restricted" | grep "7f00"
Found 2 pods, using pod/router-default-5f59648476-n5jvv
E0717 06:25:35.243202 1 extended_validator.go:70] "msg"="Skipping endpoint address with restricted or invalid IP" "error"="IP address 127.0.0.1 is a restricted loopback IP" "address"="::7f00:1" "logger"="controller"Test 4: AWS IPv6 IMDS (Layer 2 Pass 1)EndpointSlicecat <<EOF | oc apply -f -
apiVersion: discovery.k8s.io/v1
kind: EndpointSlice
metadata:
name: test-aws-ipv6-imds
namespace: ssrf-test
labels:
kubernetes.io/service-name: metadata-svc
addressType: IPv6
endpoints:
- addresses:
- "fd00:ec2::254"
ports:
- port: 80
EOF
endpointslice.discovery.k8s.io/test-aws-ipv6-imds createdVerificationcurl -s -o /dev/null -w "%{http_code}" $TEST_URL
503
oc -n openshift-ingress logs deployment/router-default | grep "restricted" | grep "fd00"
Found 2 pods, using pod/router-default-5f59648476-n5jvv
E0717 06:27:50.255380 1 extended_validator.go:70] "msg"="Skipping endpoint address with restricted or invalid IP" "error"="IP address fd00:ec2::254 is a restricted cloud metadata IP" "address"="fd00:ec2::254" "logger"="controller"Test 5: IPv6 Loopback (Layer 2 Pass 1)EndpointSliceapiVersion: discovery.k8s.io/v1
kind: EndpointSlice
metadata:
name: test-ipv6-loopback
namespace: ssrf-test
labels:
kubernetes.io/service-name: metadata-svc
addressType: IPv6
endpoints:
- addresses:
- "::1"
ports:
- port: 80Verificationoc apply -f test-ipv6-loopback.yaml
curl -s -o /dev/null -w "%{http_code}" $TEST_URL
oc -n openshift-ingress logs deployment/router-default | grep "restricted" | grep "::1"OutputTest 6: IPv6 Link-Local (Layer 2 Pass 1)EndpointSliceapiVersion: discovery.k8s.io/v1
kind: EndpointSlice
metadata:
name: test-ipv6-linklocal
namespace: ssrf-test
labels:
kubernetes.io/service-name: metadata-svc
addressType: IPv6
endpoints:
- addresses:
- "fe80::1"
ports:
- port: 80Verificationoc apply -f test-ipv6-linklocal.yaml
curl -s -o /dev/null -w "%{http_code}" $TEST_URL
oc -n openshift-ingress logs deployment/router-default | grep "restricted" | grep "fe80"OutputTest 7: IPv6 Multicast (Layer 2 Pass 1)EndpointSliceapiVersion: discovery.k8s.io/v1
kind: EndpointSlice
metadata:
name: test-ipv6-multicast
namespace: ssrf-test
labels:
kubernetes.io/service-name: metadata-svc
addressType: IPv6
endpoints:
- addresses:
- "ff02::1"
ports:
- port: 80Verificationoc apply -f test-ipv6-multicast.yaml
curl -s -o /dev/null -w "%{http_code}" $TEST_URL
oc -n openshift-ingress logs deployment/router-default | grep "restricted" | grep "ff02"OutputTest 8: IPv6 Unspecified (Layer 2 Pass 1)EndpointSliceapiVersion: discovery.k8s.io/v1
kind: EndpointSlice
metadata:
name: test-ipv6-unspecified
namespace: ssrf-test
labels:
kubernetes.io/service-name: metadata-svc
addressType: IPv6
endpoints:
- addresses:
- "::"
ports:
- port: 80Verificationoc apply -f test-ipv6-unspecified.yaml
curl -s -o /dev/null -w "%{http_code}" $TEST_URL
oc -n openshift-ingress logs deployment/router-default | grep "restricted" | grep "unspecified"OutputTest 9: Valid IPv6 Pass-ThroughEndpointSlicecat <<EOF | oc apply -f -
apiVersion: discovery.k8s.io/v1
kind: EndpointSlice
metadata:
name: test-valid-ipv6
namespace: ssrf-test
labels:
kubernetes.io/service-name: metadata-svc
addressType: IPv6
endpoints:
- addresses:
- "2001:db8::1"
ports:
- port: 80
EOF
endpointslice.discovery.k8s.io/test-valid-ipv6 createdVerificationoc -n openshift-ingress logs deployment/router-default | grep "restricted" | grep "2001"
Found 2 pods, using pod/router-default-5f59648476-n5jvv (EXPECTED)
Expected Results:
No log line found regarding this address being restricted.
The router accepts it as a valid backend.
Test 10: Normal Application Regression Testoc -n ssrf-test new-app --image=openshift/hello-openshift --name=hello
oc -n ssrf-test expose svc hello --hostname=hello-test.${CLUSTER_DOMAIN}
# Wait for pod to be ready
oc -n ssrf-test wait --for=condition=ready pod -l app=hello --timeout=60s
--> Found container image 7af3297 (8 years old) from Docker Hub for "openshift/hello-openshift"
* An image stream tag will be created as "hello:latest" that will track this image
--> Creating resources ...
imagestream.image.openshift.io "hello" created
deployment.apps "hello" created
service "hello" created
--> Success
Application is not exposed. You can expose services to the outside world by executing one or more of the commands below:
'oc expose service/hello'
Run 'oc status' to view your app.
route.route.openshift.io/hello exposed
Verificationcurl -s http://hello-test.${CLUSTER_DOMAIN}
Hello OpenShift!
NoteKubernetes/OpenShift prevent the creation of EndpointSlices that reference special-purpose IP address ranges, including loopback (127.0.0.0/8, ::1/128), unspecified (0.0.0.0, ::), multicast (224.0.0.0/4, ff00::/8), and other non-routable address ranges such as link-local addresses (169.254.0.0/16, fe80::/10) because of the built-in validation implemented by Kubernetes/OpenShift. Tests 5–8 are blocked by Kubernetes/OpenShift built-in EndpointSlice validation before reaching the router SSRF validation logic. /verified by ci |
openshift-ci-robot
added
the
verified
Contributor
|
@UdayYendva: This PR has been marked as verified by DetailsIn response to this:
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
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 : #818
Summary by CodeRabbit
New Features
Bug Fixes