Add PSA to block host field in probe/lifecycle handlers #4942
Conversation
|
Skipping CI for Draft Pull Request. |
k8s-ci-robot
added
do-not-merge/work-in-progress
k8s-ci-robot
added
the
kind/kep
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: tssurya The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
sig/auth
Signed-off-by: Surya Seetharaman <[email protected]>
tssurya
force-pushed
the
psa-host-field
branch
from
3cd9662
to
feb1a37
Compare
|
not familiar with the mechanics of this admission plugin, the trickiest part seems to handle the roll out of the feature, as some pods will be impacted by the new policy on upgrade. Things like a rolling update on a deployment comes to mind, where valid pods will created and will be impacted, despite the existing ones running will not. From the network perspective +1 from me, most people should not use the host field, although there are case that is used for polling external endpoints and now it can not be removed without breaking existing workloads, but seems correct to warn these users about the risk they are running by doing that. cc: @tallclair @liggitt |
| ## Motivation | ||
|
|
||
| **Probe-based Blind SSRF Attacks**: The current definition of TCP and | ||
| HTTP probes allows the user to specify an alternative hostname/IP to | ||
| connect to rather than the pod IP. (The expected use is for sending a | ||
| probe via a HostPort, NodePort, or LoadBalancer IP.) But this allows a | ||
| "blind SSRF" (Server-Side Request Forgery) attack, in which a pod can | ||
| trick kubelet into sending an HTTP GET request to an arbitrary URL (or | ||
| portscanning TCP ports on arbitrary hosts). ([kubernetes #99425]) |
There was a problem hiding this comment.
No need to repeat this; you just said all that in the summary.
But you should keep the Goals/Non-Goals section, even if there's only a single Goal.
|
|
||
| There is a long term plan to deprecate the existing TCP and HTTP probe | ||
| types in the API to replace them with ones with slightly different semantics. | ||
| See the [KEP](https://github.com/kubernetes/enhancements/pull/4558) for more |
There was a problem hiding this comment.
| See the [KEP](https://github.com/kubernetes/enhancements/pull/4558) for more | |
| See [KEP-4559](https://github.com/kubernetes/enhancements/pull/4558) for more |
| Meanwhile, the older API is never going to go away. So we also want to | ||
| add PSA to allow admins to be able to restrict users from creating | ||
| probes with the Host field set when using the (about to be deprecated) API. | ||
| Here is the [draft PR](https://github.com/kubernetes/kubernetes/pull/125271) |
There was a problem hiding this comment.
(again I'd mention the PR number in the text. "This is implemented by [kubernetes #125271]" or something)
|
|
||
| Consider including folks who also work outside the SIG or subproject. | ||
| --> | ||
| N/A |
There was a problem hiding this comment.
You can remove the HTML comments after you fill the section in.
Anyway, I think you could say there's a risk, which is that some people may be depending on this functionality, and the mitigation for that is that the admin doesn't have to block it. But there's no way to make the existing feature safe, so we think blocking it is the right thing to do.
| <!-- | ||
| Based on reviewers feedback describe what additional tests need to be added prior | ||
| implementing this enhancement to ensure the enhancements have also solid foundations. | ||
| --> |
| heartbeats, leader election, etc.) | ||
| --> | ||
|
|
||
| TBD |
| [existing SLIs/SLOs]: https://git.k8s.io/community/sig-scalability/slos/slos.md#kubernetes-slisslos | ||
| --> | ||
|
|
||
| TBD |
|
|
||
| ###### How does this feature react if the API server and/or etcd is unavailable? | ||
|
|
||
| TBD |
There was a problem hiding this comment.
N/A; it's an apiserver feature
| # The following PRR answers are required at alpha release | ||
| # List the feature gate name and the components for which it must be enabled | ||
| feature-gates: | ||
| - name: N/A We decided to go with PSA versioning |
There was a problem hiding this comment.
don't put freeform text in kep.yaml; some scripts try to interpret it. Just leave this empty.
|
|
||
| # The following PRR answers are required at beta release | ||
| metrics: | ||
| - my_feature_metric |
There was a problem hiding this comment.
(remove the fake metric)
.hostfield from ProbeHandler and LifecycleHandler #4940