If someone gains the ability to create static pods, they might try to use that
ability to run code which gets access to the resources associated with some
existing claim which was previously allocated for some other pod. Such an
attempt already fails because the claim status tracks which pods are allowed to
use the claim, the static pod is not in that list, the node is not authorized
to add it, and the kubelet checks that list before starting the pod in
195803cde5/pkg/kubelet/cm/dra/manager.go (L218-L222).
Even if the pod were started, DRA drivers typically manage node-local resources
which can already be accessed via such an attack without involving DRA. DRA
drivers which manage non-node-local resources have to consider access by a
compromised node as part of their threat model.
Nonetheless, it is better to not accept static pods which reference
ResourceClaims or ResourceClaimTemplates in the first place because there
is no valid use case for it.
This is done at different levels for defense in depth:
- configuration validation in the kubelet
- admission checking of node restrictions
- API validation
Co-authored-by: Jordan Liggitt <liggitt@google.com>
Code changes by Jordan, with one small change (resourceClaims -> resourceclaims).
Unit tests by Patrick.
This is needed to make declaratve validation clean. Past me thought
this was clever (pointer versioned, non-pointer internal) but it is just
confusing.
Ignore pre-existing bad IP/CIDR values in:
- pod.spec.podIP(s)
- pod.spec.hostIP(s)
- service.spec.externalIPs
- service.spec.clusterIP(s)
- service.spec.loadBalancerSourceRanges (and corresponding annotation)
- service.status.loadBalancer.ingress[].ip
- endpoints.subsets
- endpointslice.endpoints
- networkpolicy.spec.{ingress[].from[],egress[].to[]}.ipBlock
- ingress.status.loadBalancer.ingress[].ip
In the Endpoints and EndpointSlice case, if *any* endpoint IP is
changed, then the entire object must be valid; invalid IPs are only
allowed to remain in place for updates that don't change any IPs.
(e.g., changing the labels or annotations).
In most of the other cases, when the invalid IP is part of an array,
it can be moved around within the array without triggering
revalidation.
Add validation.IsValidIPForLegacyField and
validation.IsValidCIDRForLegacyField, which validate "legacy" IP/CIDR
fields correctly. Use them for all such fields (indirectly, via a
wrapper in pkg/apis/core/validation that handles the
StrictIPCIDRValidation feature gate correctly).
Change IsValidIP and IsValidCIDR to require strict parsing and
canonical form, and update the IPAddr, ServiceCIDR, and
NetworkDeviceData validation to make use of them.
Fix some incorrect test case names.
Use t.Run() in a few more places (to facilitate using
SetFeatureGateDuringTest later).
Clarify TestPodIPsValidation/TestHostIPsValidation (and fix
weird indentation).
There is not a single definition of "non-special IP" that makes sense
in all contexts. Rename ValidateNonSpecialIP to ValidateEndpointIP and
clarify that it shouldn't be used for other validations.
Also add a few more unit tests.
Graduate the feature to beta, by:
- Allowing `subPath`/`subPathExpr` for image volumes
- Modifying the CRI to pass down the (resolved) sub path
- Adding metrics which are outlined in the KEP
Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
I discovered this by changing the validation in a way that SHOULD fail
(by allowing something it should not). But it didn't. A different
error happens which totally masks the non-failure I expected. New test
is much more explicit about what failures are expected.
This does not focus on adding test coverage, just making sure the test
is not terrible.
Remove unnecessary duplicate checks for pod.spec.podIPs /
pod.spec.hostIPs / node.spec.podCIDRs. (A list that is known to
contain exactly 2 values, where one is IPv4 and the other is IPv6,
cannot possibly contain duplicates.)
Fix a bad CIDR in the NetworkPolicy validation tests.
Fix some comment typos.
a) Rename the type and drop the constructor
b) Make MatchErrors() into a Test() method
For followup:
c) Consider making ByType() assumed
d) Consider making ByField() assumed and handle nil as "don't care"
e) Consider making ByValue() assumed and handle nil as "don't care"
I fixed up the TestValidateEndpointsCreate path to show the matcher
instead of manual origin checking.
I picked TestValidateTopologySpreadConstraints because it was the last
failing test on my screen when I changed on of the commonly hard-coded
error strings. I fixed exactly those validation errors that were needed
to make this test pass. Some of the Origin values can be debated.
The `field/testing.Matcher` interface allows tests to configure the
criteria by which they want to match expected and actual errors. The
hope is that everyone will use Origin for Invalid errors.
There's some collateral impact for tests which use exact-comparisons and
don't expect origins. These are all candidates for using the matcher.
Update ValidateEndpointsCreate validation tests to use the new Origin field for more precise error comparisons. It leverage the Origin field instead of detailed error messages, improving test robustness and readability.
Co-authored-by: Tim Hockin <thockin@google.com>
