node authorizer: limit kubelet access to ResourceClaim objects

As discussed during the KEP and code review of dynamic resource allocation for
Kubernetes 1.26, to increase security kubelet should only get read access to
those ResourceClaim objects that are needed by a pod on the node.

This can be enforced easily by the node authorizer:
- the names of the objects are defined by the pod status
- there is no indirection as with volumes (pod -> pvc -> pv)

Normally the graph only gets updated when a pod is not scheduled yet.
Resource claim status changes can still happen after that, so they
also must trigger an update.
This commit is contained in:
Patrick Ohly 2023-03-03 13:33:20 +01:00
parent a6890b361d
commit 1db11c07ff
4 changed files with 130 additions and 24 deletions

View File

@ -22,6 +22,7 @@ import (
corev1 "k8s.io/api/core/v1" corev1 "k8s.io/api/core/v1"
"k8s.io/component-helpers/storage/ephemeral" "k8s.io/component-helpers/storage/ephemeral"
"k8s.io/dynamic-resource-allocation/resourceclaim"
pvutil "k8s.io/kubernetes/pkg/api/v1/persistentvolume" pvutil "k8s.io/kubernetes/pkg/api/v1/persistentvolume"
podutil "k8s.io/kubernetes/pkg/api/v1/pod" podutil "k8s.io/kubernetes/pkg/api/v1/pod"
"k8s.io/kubernetes/third_party/forked/gonum/graph" "k8s.io/kubernetes/third_party/forked/gonum/graph"
@ -117,6 +118,7 @@ const (
podVertexType podVertexType
pvcVertexType pvcVertexType
pvVertexType pvVertexType
resourceClaimVertexType
secretVertexType secretVertexType
vaVertexType vaVertexType
serviceAccountVertexType serviceAccountVertexType
@ -128,6 +130,7 @@ var vertexTypes = map[vertexType]string{
podVertexType: "pod", podVertexType: "pod",
pvcVertexType: "pvc", pvcVertexType: "pvc",
pvVertexType: "pv", pvVertexType: "pv",
resourceClaimVertexType: "resourceclaim",
secretVertexType: "secret", secretVertexType: "secret",
vaVertexType: "volumeattachment", vaVertexType: "volumeattachment",
serviceAccountVertexType: "serviceAccount", serviceAccountVertexType: "serviceAccount",
@ -393,6 +396,20 @@ func (g *Graph) AddPod(pod *corev1.Pod) {
g.addEdgeToDestinationIndex_locked(e) g.addEdgeToDestinationIndex_locked(e)
} }
} }
for _, podResourceClaim := range pod.Spec.ResourceClaims {
claimName, _, err := resourceclaim.Name(pod, &podResourceClaim)
// Do we have a valid claim name? If yes, add an edge that grants
// kubelet access to that claim. An error indicates that a claim
// still needs to be created, nil that intentionally no claim
// was created and never will be because it isn't needed.
if err == nil && claimName != nil {
claimVertex := g.getOrCreateVertex_locked(resourceClaimVertexType, pod.Namespace, *claimName)
e := newDestinationEdge(claimVertex, podVertex, nodeVertex)
g.graph.SetEdge(e)
g.addEdgeToDestinationIndex_locked(e)
}
}
} }
func (g *Graph) DeletePod(name, namespace string) { func (g *Graph) DeletePod(name, namespace string) {
start := time.Now() start := time.Now()

View File

@ -78,8 +78,9 @@ func (g *graphPopulator) updatePod(oldObj, obj interface{}) {
return return
} }
if oldPod, ok := oldObj.(*corev1.Pod); ok && oldPod != nil { if oldPod, ok := oldObj.(*corev1.Pod); ok && oldPod != nil {
if (pod.Spec.NodeName == oldPod.Spec.NodeName) && (pod.UID == oldPod.UID) { if (pod.Spec.NodeName == oldPod.Spec.NodeName) && (pod.UID == oldPod.UID) &&
// Node and uid are unchanged, all object references in the pod spec are immutable resourceClaimStatusesEqual(oldPod.Status.ResourceClaimStatuses, pod.Status.ResourceClaimStatuses) {
// Node and uid are unchanged, all object references in the pod spec are immutable respectively unmodified (claim statuses).
klog.V(5).Infof("updatePod %s/%s, node unchanged", pod.Namespace, pod.Name) klog.V(5).Infof("updatePod %s/%s, node unchanged", pod.Namespace, pod.Name)
return return
} }
@ -91,6 +92,29 @@ func (g *graphPopulator) updatePod(oldObj, obj interface{}) {
klog.V(5).Infof("updatePod %s/%s for node %s completed in %v", pod.Namespace, pod.Name, pod.Spec.NodeName, time.Since(startTime)) klog.V(5).Infof("updatePod %s/%s for node %s completed in %v", pod.Namespace, pod.Name, pod.Spec.NodeName, time.Since(startTime))
} }
func resourceClaimStatusesEqual(statusA, statusB []corev1.PodResourceClaimStatus) bool {
if len(statusA) != len(statusB) {
return false
}
// In most cases, status entries only get added once and not modified.
// But this cannot be guaranteed, so for the sake of correctness in all
// cases this code here has to check.
for i := range statusA {
if statusA[i].Name != statusB[i].Name {
return false
}
claimNameA := statusA[i].ResourceClaimName
claimNameB := statusB[i].ResourceClaimName
if (claimNameA == nil) != (claimNameB == nil) {
return false
}
if claimNameA != nil && *claimNameA != *claimNameB {
return false
}
}
return true
}
func (g *graphPopulator) deletePod(obj interface{}) { func (g *graphPopulator) deletePod(obj interface{}) {
if tombstone, ok := obj.(cache.DeletedFinalStateUnknown); ok { if tombstone, ok := obj.(cache.DeletedFinalStateUnknown); ok {
obj = tombstone.Obj obj = tombstone.Obj

View File

@ -30,6 +30,7 @@ import (
"k8s.io/component-base/featuregate" "k8s.io/component-base/featuregate"
coordapi "k8s.io/kubernetes/pkg/apis/coordination" coordapi "k8s.io/kubernetes/pkg/apis/coordination"
api "k8s.io/kubernetes/pkg/apis/core" api "k8s.io/kubernetes/pkg/apis/core"
resourceapi "k8s.io/kubernetes/pkg/apis/resource"
storageapi "k8s.io/kubernetes/pkg/apis/storage" storageapi "k8s.io/kubernetes/pkg/apis/storage"
"k8s.io/kubernetes/pkg/auth/nodeidentifier" "k8s.io/kubernetes/pkg/auth/nodeidentifier"
"k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac" "k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac"
@ -40,7 +41,7 @@ import (
// NodeAuthorizer authorizes requests from kubelets, with the following logic: // NodeAuthorizer authorizes requests from kubelets, with the following logic:
// 1. If a request is not from a node (NodeIdentity() returns isNode=false), reject // 1. If a request is not from a node (NodeIdentity() returns isNode=false), reject
// 2. If a specific node cannot be identified (NodeIdentity() returns nodeName=""), reject // 2. If a specific node cannot be identified (NodeIdentity() returns nodeName=""), reject
// 3. If a request is for a secret, configmap, persistent volume or persistent volume claim, reject unless the verb is get, and the requested object is related to the requesting node: // 3. If a request is for a secret, configmap, persistent volume, resource claim, or persistent volume claim, reject unless the verb is get, and the requested object is related to the requesting node:
// node <- configmap // node <- configmap
// node <- pod // node <- pod
// node <- pod <- secret // node <- pod <- secret
@ -48,6 +49,7 @@ import (
// node <- pod <- pvc // node <- pod <- pvc
// node <- pod <- pvc <- pv // node <- pod <- pvc <- pv
// node <- pod <- pvc <- pv <- secret // node <- pod <- pvc <- pv <- secret
// node <- pod <- ResourceClaim
// 4. For other resources, authorize all nodes uniformly using statically defined rules // 4. For other resources, authorize all nodes uniformly using statically defined rules
type NodeAuthorizer struct { type NodeAuthorizer struct {
graph *Graph graph *Graph
@ -72,14 +74,15 @@ func NewAuthorizer(graph *Graph, identifier nodeidentifier.NodeIdentifier, rules
} }
var ( var (
configMapResource = api.Resource("configmaps") configMapResource = api.Resource("configmaps")
secretResource = api.Resource("secrets") secretResource = api.Resource("secrets")
pvcResource = api.Resource("persistentvolumeclaims") pvcResource = api.Resource("persistentvolumeclaims")
pvResource = api.Resource("persistentvolumes") pvResource = api.Resource("persistentvolumes")
vaResource = storageapi.Resource("volumeattachments") resourceClaimResource = resourceapi.Resource("resourceclaims")
svcAcctResource = api.Resource("serviceaccounts") vaResource = storageapi.Resource("volumeattachments")
leaseResource = coordapi.Resource("leases") svcAcctResource = api.Resource("serviceaccounts")
csiNodeResource = storageapi.Resource("csinodes") leaseResource = coordapi.Resource("leases")
csiNodeResource = storageapi.Resource("csinodes")
) )
func (r *NodeAuthorizer) RulesFor(user user.Info, namespace string) ([]authorizer.ResourceRuleInfo, []authorizer.NonResourceRuleInfo, bool, error) { func (r *NodeAuthorizer) RulesFor(user user.Info, namespace string) ([]authorizer.ResourceRuleInfo, []authorizer.NonResourceRuleInfo, bool, error) {
@ -117,6 +120,8 @@ func (r *NodeAuthorizer) Authorize(ctx context.Context, attrs authorizer.Attribu
return r.authorizeGet(nodeName, pvcVertexType, attrs) return r.authorizeGet(nodeName, pvcVertexType, attrs)
case pvResource: case pvResource:
return r.authorizeGet(nodeName, pvVertexType, attrs) return r.authorizeGet(nodeName, pvVertexType, attrs)
case resourceClaimResource:
return r.authorizeGet(nodeName, resourceClaimVertexType, attrs)
case vaResource: case vaResource:
return r.authorizeGet(nodeName, vaVertexType, attrs) return r.authorizeGet(nodeName, vaVertexType, attrs)
case svcAcctResource: case svcAcctResource:

View File

@ -42,16 +42,19 @@ func TestAuthorizer(t *testing.T) {
g := NewGraph() g := NewGraph()
opts := &sampleDataOpts{ opts := &sampleDataOpts{
nodes: 2, nodes: 2,
namespaces: 2, namespaces: 2,
podsPerNode: 2, podsPerNode: 2,
attachmentsPerNode: 1, attachmentsPerNode: 1,
sharedConfigMapsPerPod: 0, sharedConfigMapsPerPod: 0,
uniqueConfigMapsPerPod: 1, uniqueConfigMapsPerPod: 1,
sharedSecretsPerPod: 1, sharedSecretsPerPod: 1,
uniqueSecretsPerPod: 1, uniqueSecretsPerPod: 1,
sharedPVCsPerPod: 0, sharedPVCsPerPod: 0,
uniquePVCsPerPod: 1, uniquePVCsPerPod: 1,
uniqueResourceClaimsPerPod: 1,
uniqueResourceClaimTemplatesPerPod: 1,
uniqueResourceClaimTemplatesWithClaimPerPod: 1,
} }
nodes, pods, pvs, attachments := generate(opts) nodes, pods, pvs, attachments := generate(opts)
populate(g, nodes, pods, pvs, attachments) populate(g, nodes, pods, pvs, attachments)
@ -117,6 +120,16 @@ func TestAuthorizer(t *testing.T) {
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "persistentvolumeclaims", Name: "pvc0-pod0-node0", Namespace: "ns0"}, attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "persistentvolumeclaims", Name: "pvc0-pod0-node0", Namespace: "ns0"},
expect: authorizer.DecisionAllow, expect: authorizer.DecisionAllow,
}, },
{
name: "allowed resource claim",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "resourceclaims", APIGroup: "resource.k8s.io", Name: "claim0-pod0-node0-ns0", Namespace: "ns0"},
expect: authorizer.DecisionAllow,
},
{
name: "allowed resource claim with template",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "resourceclaims", APIGroup: "resource.k8s.io", Name: "generated-claim-pod0-node0-ns0-0", Namespace: "ns0"},
expect: authorizer.DecisionAllow,
},
{ {
name: "allowed pv", name: "allowed pv",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "persistentvolumes", Name: "pv0-pod0-node0-ns0", Namespace: ""}, attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "persistentvolumes", Name: "pv0-pod0-node0-ns0", Namespace: ""},
@ -142,6 +155,16 @@ func TestAuthorizer(t *testing.T) {
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "persistentvolumeclaims", Name: "pvc0-pod0-node1", Namespace: "ns0"}, attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "persistentvolumeclaims", Name: "pvc0-pod0-node1", Namespace: "ns0"},
expect: authorizer.DecisionNoOpinion, expect: authorizer.DecisionNoOpinion,
}, },
{
name: "disallowed resource claim, other node",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "resourceclaims", APIGroup: "resource.k8s.io", Name: "claim0-pod0-node1-ns0", Namespace: "ns0"},
expect: authorizer.DecisionNoOpinion,
},
{
name: "disallowed resource claim with template",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "resourceclaims", APIGroup: "resource.k8s.io", Name: "pod0-node1-claimtemplate0", Namespace: "ns0"},
expect: authorizer.DecisionNoOpinion,
},
{ {
name: "disallowed pv", name: "disallowed pv",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "persistentvolumes", Name: "pv0-pod0-node1-ns0", Namespace: ""}, attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "persistentvolumes", Name: "pv0-pod0-node1-ns0", Namespace: ""},
@ -468,9 +491,12 @@ type sampleDataOpts struct {
sharedSecretsPerPod int sharedSecretsPerPod int
sharedPVCsPerPod int sharedPVCsPerPod int
uniqueSecretsPerPod int uniqueSecretsPerPod int
uniqueConfigMapsPerPod int uniqueConfigMapsPerPod int
uniquePVCsPerPod int uniquePVCsPerPod int
uniqueResourceClaimsPerPod int
uniqueResourceClaimTemplatesPerPod int
uniqueResourceClaimTemplatesWithClaimPerPod int
} }
func BenchmarkPopulationAllocation(b *testing.B) { func BenchmarkPopulationAllocation(b *testing.B) {
@ -845,6 +871,40 @@ func generatePod(name, namespace, nodeName, svcAccountName string, opts *sampleD
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ClaimName: pv.Spec.ClaimRef.Name}, PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ClaimName: pv.Spec.ClaimRef.Name},
}}) }})
} }
for i := 0; i < opts.uniqueResourceClaimsPerPod; i++ {
claimName := fmt.Sprintf("claim%d-%s-%s", i, pod.Name, pod.Namespace)
pod.Spec.ResourceClaims = append(pod.Spec.ResourceClaims, corev1.PodResourceClaim{
Name: fmt.Sprintf("claim%d", i),
Source: corev1.ClaimSource{
ResourceClaimName: &claimName,
},
})
}
for i := 0; i < opts.uniqueResourceClaimTemplatesPerPod; i++ {
claimTemplateName := fmt.Sprintf("claimtemplate%d-%s-%s", i, pod.Name, pod.Namespace)
podClaimName := fmt.Sprintf("claimtemplate%d", i)
pod.Spec.ResourceClaims = append(pod.Spec.ResourceClaims, corev1.PodResourceClaim{
Name: podClaimName,
Source: corev1.ClaimSource{
ResourceClaimTemplateName: &claimTemplateName,
},
})
}
for i := 0; i < opts.uniqueResourceClaimTemplatesWithClaimPerPod; i++ {
claimTemplateName := fmt.Sprintf("claimtemplate%d-%s-%s", i, pod.Name, pod.Namespace)
podClaimName := fmt.Sprintf("claimtemplate-with-claim%d", i)
claimName := fmt.Sprintf("generated-claim-%s-%s-%d", pod.Name, pod.Namespace, i)
pod.Spec.ResourceClaims = append(pod.Spec.ResourceClaims, corev1.PodResourceClaim{
Name: podClaimName,
Source: corev1.ClaimSource{
ResourceClaimTemplateName: &claimTemplateName,
},
})
pod.Status.ResourceClaimStatuses = append(pod.Status.ResourceClaimStatuses, corev1.PodResourceClaimStatus{
Name: podClaimName,
ResourceClaimName: &claimName,
})
}
// Choose shared pvcs randomly from shared pvcs in a namespace. // Choose shared pvcs randomly from shared pvcs in a namespace.
subset = randomSubset(opts.sharedPVCsPerPod, opts.sharedPVCsPerNamespace) subset = randomSubset(opts.sharedPVCsPerPod, opts.sharedPVCsPerNamespace)
for _, i := range subset { for _, i := range subset {