diff --git a/plugin/pkg/auth/authorizer/node/graph.go b/plugin/pkg/auth/authorizer/node/graph.go index 501e1d298ce..4951cf15473 100644 --- a/plugin/pkg/auth/authorizer/node/graph.go +++ b/plugin/pkg/auth/authorizer/node/graph.go @@ -17,6 +17,7 @@ limitations under the License. package node import ( + "fmt" "sync" "time" @@ -79,7 +80,8 @@ func (e *destinationEdge) DestinationID() int { return e.Destination.ID() } // pvc <- pv // pv <- secret type Graph struct { - lock sync.RWMutex + lock sync.RWMutex + // Calling graph.SetEdge is restricted to the addEdgeLocked method of Graph. graph *simple.DirectedAcyclicGraph // vertices is a map of type -> namespace -> name -> vertex vertices map[vertexType]namespaceVertexMapping @@ -138,6 +140,20 @@ var vertexTypes = map[vertexType]string{ serviceAccountVertexType: "serviceAccount", } +// vertexTypeWithAuthoritativeIndex indicates which types of vertices can hold +// a destination edge index that is authoritative i.e. if the index exists, +// then it always stores all of the Nodes that are reachable from that vertex +// in the graph. +var vertexTypeWithAuthoritativeIndex = map[vertexType]bool{ + configMapVertexType: true, + sliceVertexType: true, + podVertexType: true, + pvcVertexType: true, + resourceClaimVertexType: true, + vaVertexType: true, + serviceAccountVertexType: true, +} + // must be called under a write lock func (g *Graph) getOrCreateVertexLocked(vertexType vertexType, namespace, name string) *namedVertex { if vertex, exists := g.getVertexRLocked(vertexType, namespace, name); exists { @@ -348,7 +364,8 @@ func (g *Graph) AddPod(pod *corev1.Pod) { g.deleteVertexLocked(podVertexType, pod.Namespace, pod.Name) podVertex := g.getOrCreateVertexLocked(podVertexType, pod.Namespace, pod.Name) nodeVertex := g.getOrCreateVertexLocked(nodeVertexType, "", pod.Spec.NodeName) - g.graph.SetEdge(newDestinationEdge(podVertex, nodeVertex, nodeVertex)) + // Edge adds must be handled by addEdgeLocked instead of direct g.graph.SetEdge calls. + g.addEdgeLocked(podVertex, nodeVertex, nodeVertex) // Short-circuit adding edges to other resources for mirror pods. // A node must never be able to create a pod that grants them permissions on other API objects. @@ -363,24 +380,21 @@ func (g *Graph) AddPod(pod *corev1.Pod) { // ref https://github.com/kubernetes/kubernetes/issues/58790 if len(pod.Spec.ServiceAccountName) > 0 { serviceAccountVertex := g.getOrCreateVertexLocked(serviceAccountVertexType, pod.Namespace, pod.Spec.ServiceAccountName) - e := newDestinationEdge(serviceAccountVertex, podVertex, nodeVertex) - g.graph.SetEdge(e) - g.addEdgeToDestinationIndexLocked(e) + // Edge adds must be handled by addEdgeLocked instead of direct g.graph.SetEdge calls. + g.addEdgeLocked(serviceAccountVertex, podVertex, nodeVertex) } podutil.VisitPodSecretNames(pod, func(secret string) bool { secretVertex := g.getOrCreateVertexLocked(secretVertexType, pod.Namespace, secret) - e := newDestinationEdge(secretVertex, podVertex, nodeVertex) - g.graph.SetEdge(e) - g.addEdgeToDestinationIndexLocked(e) + // Edge adds must be handled by addEdgeLocked instead of direct g.graph.SetEdge calls. + g.addEdgeLocked(secretVertex, podVertex, nodeVertex) return true }) podutil.VisitPodConfigmapNames(pod, func(configmap string) bool { configmapVertex := g.getOrCreateVertexLocked(configMapVertexType, pod.Namespace, configmap) - e := newDestinationEdge(configmapVertex, podVertex, nodeVertex) - g.graph.SetEdge(e) - g.addEdgeToDestinationIndexLocked(e) + // Edge adds must be handled by addEdgeLocked instead of direct g.graph.SetEdge calls. + g.addEdgeLocked(configmapVertex, podVertex, nodeVertex) return true }) @@ -393,9 +407,8 @@ func (g *Graph) AddPod(pod *corev1.Pod) { } if claimName != "" { pvcVertex := g.getOrCreateVertexLocked(pvcVertexType, pod.Namespace, claimName) - e := newDestinationEdge(pvcVertex, podVertex, nodeVertex) - g.graph.SetEdge(e) - g.addEdgeToDestinationIndexLocked(e) + // Edge adds must be handled by addEdgeLocked instead of direct g.graph.SetEdge calls. + g.addEdgeLocked(pvcVertex, podVertex, nodeVertex) } } @@ -407,12 +420,34 @@ func (g *Graph) AddPod(pod *corev1.Pod) { // was created and never will be because it isn't needed. if err == nil && claimName != nil { claimVertex := g.getOrCreateVertexLocked(resourceClaimVertexType, pod.Namespace, *claimName) - e := newDestinationEdge(claimVertex, podVertex, nodeVertex) - g.graph.SetEdge(e) - g.addEdgeToDestinationIndexLocked(e) + // Edge adds must be handled by addEdgeLocked instead of direct g.graph.SetEdge calls. + g.addEdgeLocked(claimVertex, podVertex, nodeVertex) } } } + +// Must be called under a write lock. +// All edge adds must be handled by that method rather than by calling +// g.graph.SetEdge directly. +// Note: if "from" belongs to vertexTypeWithAuthoritativeIndex, then +// "destination" must be non-nil. +func (g *Graph) addEdgeLocked(from, to, destination *namedVertex) { + if destination != nil { + e := newDestinationEdge(from, to, destination) + g.graph.SetEdge(e) + g.addEdgeToDestinationIndexLocked(e) + return + } + + // We must not create edges without a Node label from a vertex that is + // supposed to hold authoritative destination edge index only. + // Entering this branch would mean there's a bug in the Node authorizer. + if vertexTypeWithAuthoritativeIndex[from.vertexType] { + panic(fmt.Sprintf("vertex of type %q must have destination edges only", vertexTypes[from.vertexType])) + } + g.graph.SetEdge(simple.Edge{F: from, T: to}) +} + func (g *Graph) DeletePod(name, namespace string) { start := time.Now() defer func() { @@ -444,11 +479,13 @@ func (g *Graph) AddPV(pv *corev1.PersistentVolume) { pvVertex := g.getOrCreateVertexLocked(pvVertexType, "", pv.Name) // since we don't know the other end of the pvc -> pod -> node chain (or it may not even exist yet), we can't decorate these edges with kubernetes node info - g.graph.SetEdge(simple.Edge{F: pvVertex, T: g.getOrCreateVertexLocked(pvcVertexType, pv.Spec.ClaimRef.Namespace, pv.Spec.ClaimRef.Name)}) + // Edge adds must be handled by addEdgeLocked instead of direct g.graph.SetEdge calls. + g.addEdgeLocked(pvVertex, g.getOrCreateVertexLocked(pvcVertexType, pv.Spec.ClaimRef.Namespace, pv.Spec.ClaimRef.Name), nil) pvutil.VisitPVSecretNames(pv, func(namespace, secret string, kubeletVisible bool) bool { // This grants access to the named secret in the same namespace as the bound PVC if kubeletVisible { - g.graph.SetEdge(simple.Edge{F: g.getOrCreateVertexLocked(secretVertexType, namespace, secret), T: pvVertex}) + // Edge adds must be handled by addEdgeLocked instead of direct g.graph.SetEdge calls. + g.addEdgeLocked(g.getOrCreateVertexLocked(secretVertexType, namespace, secret), pvVertex, nil) } return true }) @@ -482,7 +519,8 @@ func (g *Graph) AddVolumeAttachment(attachmentName, nodeName string) { if len(nodeName) > 0 { vaVertex := g.getOrCreateVertexLocked(vaVertexType, "", attachmentName) nodeVertex := g.getOrCreateVertexLocked(nodeVertexType, "", nodeName) - g.graph.SetEdge(newDestinationEdge(vaVertex, nodeVertex, nodeVertex)) + // Edge adds must be handled by addEdgeLocked instead of direct g.graph.SetEdge calls. + g.addEdgeLocked(vaVertex, nodeVertex, nodeVertex) } } func (g *Graph) DeleteVolumeAttachment(name string) { @@ -513,7 +551,8 @@ func (g *Graph) AddResourceSlice(sliceName, nodeName string) { if len(nodeName) > 0 { sliceVertex := g.getOrCreateVertexLocked(sliceVertexType, "", sliceName) nodeVertex := g.getOrCreateVertexLocked(nodeVertexType, "", nodeName) - g.graph.SetEdge(newDestinationEdge(sliceVertex, nodeVertex, nodeVertex)) + // Edge adds must be handled by addEdgeLocked instead of direct g.graph.SetEdge calls. + g.addEdgeLocked(sliceVertex, nodeVertex, nodeVertex) } } func (g *Graph) DeleteResourceSlice(sliceName string) { diff --git a/plugin/pkg/auth/authorizer/node/graph_test.go b/plugin/pkg/auth/authorizer/node/graph_test.go index de755deb854..d7335bf956a 100644 --- a/plugin/pkg/auth/authorizer/node/graph_test.go +++ b/plugin/pkg/auth/authorizer/node/graph_test.go @@ -52,7 +52,7 @@ func TestDeleteEdgesLocked(t *testing.T) { g.getOrCreateVertexLocked(configMapVertexType, "namespace1", "configmap2") nodeVertex := g.getOrCreateVertexLocked(nodeVertexType, "", "node1") configmapVertex := g.getOrCreateVertexLocked(configMapVertexType, "namespace1", "configmap1") - g.graph.SetEdge(newDestinationEdge(configmapVertex, nodeVertex, nodeVertex)) + g.addEdgeLocked(configmapVertex, nodeVertex, nodeVertex) return g }(), expect: func() *Graph { @@ -74,8 +74,8 @@ func TestDeleteEdgesLocked(t *testing.T) { nodeVertex1 := g.getOrCreateVertexLocked(nodeVertexType, "", "node1") nodeVertex2 := g.getOrCreateVertexLocked(nodeVertexType, "", "node2") configmapVertex := g.getOrCreateVertexLocked(configMapVertexType, "namespace1", "configmap1") - g.graph.SetEdge(newDestinationEdge(configmapVertex, nodeVertex1, nodeVertex1)) - g.graph.SetEdge(newDestinationEdge(configmapVertex, nodeVertex2, nodeVertex2)) + g.addEdgeLocked(configmapVertex, nodeVertex1, nodeVertex1) + g.addEdgeLocked(configmapVertex, nodeVertex2, nodeVertex2) return g }(), expect: func() *Graph { @@ -83,7 +83,7 @@ func TestDeleteEdgesLocked(t *testing.T) { nodeVertex1 := g.getOrCreateVertexLocked(nodeVertexType, "", "node1") g.getOrCreateVertexLocked(nodeVertexType, "", "node2") configmapVertex := g.getOrCreateVertexLocked(configMapVertexType, "namespace1", "configmap1") - g.graph.SetEdge(newDestinationEdge(configmapVertex, nodeVertex1, nodeVertex1)) + g.addEdgeLocked(configmapVertex, nodeVertex1, nodeVertex1) return g }(), }, @@ -344,3 +344,558 @@ func TestIndex(t *testing.T) { "serviceAccount:ns/sa1": {"node:node1=1", "node:node2=1", "node:node3=1"}, }) } + +func TestIndex2(t *testing.T) { + NewTestGraph := func() *Graph { + g := NewGraph() + g.destinationEdgeThreshold = 3 + return g + } + + pod := func(podName, nodeName, saName string, volumes []corev1.Volume, resourceClaims []corev1.PodResourceClaim) *corev1.Pod { + p := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: podName, Namespace: "ns", UID: types.UID(fmt.Sprintf("pod%suid", podName))}, + Spec: corev1.PodSpec{ + NodeName: nodeName, + }, + } + if saName != "" { + p.Spec.ServiceAccountName = saName + } + if volumes != nil { + p.Spec.Volumes = volumes + } + if resourceClaims != nil { + p.Spec.ResourceClaims = resourceClaims + } + return p + } + + podWithSAAndCMs := func(podName, nodeName string) *corev1.Pod { + cm := func(name string) corev1.Volume { + return corev1.Volume{Name: name, VolumeSource: corev1.VolumeSource{ConfigMap: &corev1.ConfigMapVolumeSource{LocalObjectReference: corev1.LocalObjectReference{Name: name}}}} + } + return pod(podName, nodeName, "sa1", []corev1.Volume{ + cm("cm1"), + cm("cm2"), + cm("cm3"), + }, nil) + } + + podWithSecrets := func(podName, nodeName string) *corev1.Pod { + secret := func(name string) corev1.Volume { + return corev1.Volume{Name: name, VolumeSource: corev1.VolumeSource{Secret: &corev1.SecretVolumeSource{SecretName: name}}} + } + return pod(podName, nodeName, "", []corev1.Volume{ + secret("s1"), + secret("s2"), + secret("s3"), + }, nil) + } + + podWithPVCs := func(podName, nodeName string) *corev1.Pod { + pvc := func(name string) corev1.Volume { + return corev1.Volume{Name: name, VolumeSource: corev1.VolumeSource{PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ClaimName: name}}} + } + return pod(podName, nodeName, "", []corev1.Volume{ + pvc("pvc1"), + pvc("pvc2"), + pvc("pvc3"), + }, nil) + } + + podWithResourceClaims := func(podName, nodeName string) *corev1.Pod { + rc := func(name string) corev1.PodResourceClaim { + return corev1.PodResourceClaim{ResourceClaimName: &name} + } + return pod(podName, nodeName, "", nil, []corev1.PodResourceClaim{ + rc("rc1"), + rc("rc2"), + rc("rc3"), + }) + } + + pv := func(pvName, pvcName, secretName string) *corev1.PersistentVolume { + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: pvName, UID: types.UID(fmt.Sprintf("pv%suid", pvName))}, + Spec: corev1.PersistentVolumeSpec{ + ClaimRef: &corev1.ObjectReference{ + Name: pvcName, + Namespace: "ns", + }, + }, + } + if secretName != "" { + pv.Spec.PersistentVolumeSource = corev1.PersistentVolumeSource{ + CSI: &corev1.CSIPersistentVolumeSource{ + NodePublishSecretRef: &corev1.SecretReference{ + Name: secretName, + Namespace: "ns", + }, + }, + } + } + return pv + } + + toString := func(g *Graph, id int) string { + for _, namespaceName := range g.vertices { + for _, nameVertex := range namespaceName { + for _, vertex := range nameVertex { + if vertex.id == id { + return vertex.String() + } + } + } + } + return "" + } + expectGraph := func(t *testing.T, g *Graph, expect map[string][]string) { + t.Helper() + actual := map[string][]string{} + for _, node := range g.graph.Nodes() { + sortedTo := []string{} + for _, to := range g.graph.From(node) { + sortedTo = append(sortedTo, toString(g, to.ID())) + } + sort.Strings(sortedTo) + actual[toString(g, node.ID())] = sortedTo + } + if !reflect.DeepEqual(expect, actual) { + e, _ := json.MarshalIndent(expect, "", " ") + a, _ := json.MarshalIndent(actual, "", " ") + t.Errorf("expected graph:\n%s\ngot:\n%s", string(e), string(a)) + } + } + expectIndex := func(t *testing.T, g *Graph, expect map[string][]string) { + t.Helper() + actual := map[string][]string{} + for from, to := range g.destinationEdgeIndex { + sortedValues := []string{} + for member, count := range to.members { + sortedValues = append(sortedValues, fmt.Sprintf("%s=%d", toString(g, member), count)) + } + sort.Strings(sortedValues) + actual[toString(g, from)] = sortedValues + } + if !reflect.DeepEqual(expect, actual) { + e, _ := json.MarshalIndent(expect, "", " ") + a, _ := json.MarshalIndent(actual, "", " ") + t.Errorf("expected index:\n%s\ngot:\n%s", string(e), string(a)) + } + } + + cases := []struct { + desc string + startingGraph *Graph + graphTransformer func(*Graph) + expectedGraph map[string][]string + expectedIndex map[string][]string + }{ + { + desc: "empty graph", + startingGraph: NewTestGraph(), + graphTransformer: func(_ *Graph) {}, + expectedGraph: map[string][]string{}, + expectedIndex: map[string][]string{}, + }, + { + desc: "outdeg below destination edge index threshold", + startingGraph: NewTestGraph(), + graphTransformer: func(g *Graph) { + g.AddPod(podWithSAAndCMs("pod1", "node1")) + g.AddPod(podWithSAAndCMs("pod2", "node2")) + }, + expectedGraph: map[string][]string{ + "node:node1": {}, + "node:node2": {}, + "pod:ns/pod1": {"node:node1"}, + "pod:ns/pod2": {"node:node2"}, + "configmap:ns/cm1": {"pod:ns/pod1", "pod:ns/pod2"}, + "configmap:ns/cm2": {"pod:ns/pod1", "pod:ns/pod2"}, + "configmap:ns/cm3": {"pod:ns/pod1", "pod:ns/pod2"}, + "serviceAccount:ns/sa1": {"pod:ns/pod1", "pod:ns/pod2"}, + }, + expectedIndex: map[string][]string{}, + }, + { + desc: "index built for configmaps and serviceaccounts", + startingGraph: func() *Graph { + g := NewTestGraph() + g.AddPod(podWithSAAndCMs("pod1", "node1")) + g.AddPod(podWithSAAndCMs("pod2", "node2")) + return g + }(), + graphTransformer: func(g *Graph) { + g.AddPod(podWithSAAndCMs("pod3", "node3")) + }, + expectedGraph: map[string][]string{ + "node:node1": {}, + "node:node2": {}, + "node:node3": {}, + "pod:ns/pod1": {"node:node1"}, + "pod:ns/pod2": {"node:node2"}, + "pod:ns/pod3": {"node:node3"}, + "configmap:ns/cm1": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3"}, + "configmap:ns/cm2": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3"}, + "configmap:ns/cm3": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3"}, + "serviceAccount:ns/sa1": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3"}, + }, + expectedIndex: map[string][]string{ + "configmap:ns/cm1": {"node:node1=1", "node:node2=1", "node:node3=1"}, + "configmap:ns/cm2": {"node:node1=1", "node:node2=1", "node:node3=1"}, + "configmap:ns/cm3": {"node:node1=1", "node:node2=1", "node:node3=1"}, + "serviceAccount:ns/sa1": {"node:node1=1", "node:node2=1", "node:node3=1"}, + }, + }, + { + desc: "no index for configmaps and serviceaccounts - dropping below threshold", + startingGraph: func() *Graph { + g := NewTestGraph() + g.AddPod(podWithSAAndCMs("pod1", "node1")) + g.AddPod(podWithSAAndCMs("pod2", "node2")) + g.AddPod(podWithSAAndCMs("pod3", "node3")) + return g + }(), + graphTransformer: func(g *Graph) { + g.DeletePod("pod1", "ns") + }, + expectedGraph: map[string][]string{ + "node:node2": {}, + "node:node3": {}, + "pod:ns/pod2": {"node:node2"}, + "pod:ns/pod3": {"node:node3"}, + "configmap:ns/cm1": {"pod:ns/pod2", "pod:ns/pod3"}, + "configmap:ns/cm2": {"pod:ns/pod2", "pod:ns/pod3"}, + "configmap:ns/cm3": {"pod:ns/pod2", "pod:ns/pod3"}, + "serviceAccount:ns/sa1": {"pod:ns/pod2", "pod:ns/pod3"}, + }, + expectedIndex: map[string][]string{}, + }, + { + desc: "removing pod but staying above threshold", + startingGraph: func() *Graph { + g := NewTestGraph() + g.AddPod(podWithSAAndCMs("pod1", "node1")) + g.AddPod(podWithSAAndCMs("pod2", "node2")) + g.AddPod(podWithSAAndCMs("pod3", "node3")) + g.AddPod(podWithSAAndCMs("pod4", "node1")) + return g + }(), + graphTransformer: func(g *Graph) { + g.DeletePod("pod1", "ns") + }, + expectedGraph: map[string][]string{ + "node:node1": {}, + "node:node2": {}, + "node:node3": {}, + "pod:ns/pod2": {"node:node2"}, + "pod:ns/pod3": {"node:node3"}, + "pod:ns/pod4": {"node:node1"}, + "configmap:ns/cm1": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"}, + "configmap:ns/cm2": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"}, + "configmap:ns/cm3": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"}, + "serviceAccount:ns/sa1": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"}, + }, + expectedIndex: map[string][]string{ + "configmap:ns/cm1": {"node:node1=1", "node:node2=1", "node:node3=1"}, + "configmap:ns/cm2": {"node:node1=1", "node:node2=1", "node:node3=1"}, + "configmap:ns/cm3": {"node:node1=1", "node:node2=1", "node:node3=1"}, + "serviceAccount:ns/sa1": {"node:node1=1", "node:node2=1", "node:node3=1"}, + }, + }, + { + desc: "index built for secrets", + startingGraph: func() *Graph { + g := NewTestGraph() + g.AddPod(podWithSecrets("pod1", "node1")) + g.AddPod(podWithSecrets("pod2", "node2")) + return g + }(), + graphTransformer: func(g *Graph) { + g.AddPod(podWithSecrets("pod3", "node3")) + }, + expectedGraph: map[string][]string{ + "node:node1": {}, + "node:node2": {}, + "node:node3": {}, + "pod:ns/pod1": {"node:node1"}, + "pod:ns/pod2": {"node:node2"}, + "pod:ns/pod3": {"node:node3"}, + "secret:ns/s1": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3"}, + "secret:ns/s2": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3"}, + "secret:ns/s3": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3"}, + }, + expectedIndex: map[string][]string{ + "secret:ns/s1": {"node:node1=1", "node:node2=1", "node:node3=1"}, + "secret:ns/s2": {"node:node1=1", "node:node2=1", "node:node3=1"}, + "secret:ns/s3": {"node:node1=1", "node:node2=1", "node:node3=1"}, + }, + }, + { + desc: "no index for secrets - dropping below threshold", + startingGraph: func() *Graph { + g := NewTestGraph() + g.AddPod(podWithSecrets("pod1", "node1")) + g.AddPod(podWithSecrets("pod2", "node2")) + g.AddPod(podWithSecrets("pod3", "node3")) + return g + }(), + graphTransformer: func(g *Graph) { + g.DeletePod("pod1", "ns") + }, + expectedGraph: map[string][]string{ + "node:node2": {}, + "node:node3": {}, + "pod:ns/pod2": {"node:node2"}, + "pod:ns/pod3": {"node:node3"}, + "secret:ns/s1": {"pod:ns/pod2", "pod:ns/pod3"}, + "secret:ns/s2": {"pod:ns/pod2", "pod:ns/pod3"}, + "secret:ns/s3": {"pod:ns/pod2", "pod:ns/pod3"}, + }, + expectedIndex: map[string][]string{}, + }, + { + desc: "index built for pvcs", + startingGraph: func() *Graph { + g := NewTestGraph() + g.AddPod(podWithPVCs("pod1", "node1")) + g.AddPod(podWithPVCs("pod2", "node2")) + return g + }(), + graphTransformer: func(g *Graph) { + g.AddPod(podWithPVCs("pod3", "node3")) + }, + expectedGraph: map[string][]string{ + "node:node1": {}, + "node:node2": {}, + "node:node3": {}, + "pod:ns/pod1": {"node:node1"}, + "pod:ns/pod2": {"node:node2"}, + "pod:ns/pod3": {"node:node3"}, + "pvc:ns/pvc1": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3"}, + "pvc:ns/pvc2": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3"}, + "pvc:ns/pvc3": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3"}, + }, + expectedIndex: map[string][]string{ + "pvc:ns/pvc1": {"node:node1=1", "node:node2=1", "node:node3=1"}, + "pvc:ns/pvc2": {"node:node1=1", "node:node2=1", "node:node3=1"}, + "pvc:ns/pvc3": {"node:node1=1", "node:node2=1", "node:node3=1"}, + }, + }, + { + desc: "no index for pvcs - dropping below threshold", + startingGraph: func() *Graph { + g := NewTestGraph() + g.AddPod(podWithPVCs("pod1", "node1")) + g.AddPod(podWithPVCs("pod2", "node2")) + g.AddPod(podWithPVCs("pod3", "node3")) + return g + }(), + graphTransformer: func(g *Graph) { + g.DeletePod("pod1", "ns") + }, + expectedGraph: map[string][]string{ + "node:node2": {}, + "node:node3": {}, + "pod:ns/pod2": {"node:node2"}, + "pod:ns/pod3": {"node:node3"}, + "pvc:ns/pvc1": {"pod:ns/pod2", "pod:ns/pod3"}, + "pvc:ns/pvc2": {"pod:ns/pod2", "pod:ns/pod3"}, + "pvc:ns/pvc3": {"pod:ns/pod2", "pod:ns/pod3"}, + }, + expectedIndex: map[string][]string{}, + }, + { + desc: "index built for resourceclaims", + startingGraph: NewTestGraph(), + graphTransformer: func(g *Graph) { + g.AddPod(podWithResourceClaims("pod1", "node1")) + g.AddPod(podWithResourceClaims("pod2", "node2")) + g.AddPod(podWithResourceClaims("pod3", "node3")) + }, + expectedGraph: map[string][]string{ + "node:node1": {}, + "node:node2": {}, + "node:node3": {}, + "pod:ns/pod1": {"node:node1"}, + "pod:ns/pod2": {"node:node2"}, + "pod:ns/pod3": {"node:node3"}, + "resourceclaim:ns/rc1": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3"}, + "resourceclaim:ns/rc2": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3"}, + "resourceclaim:ns/rc3": {"pod:ns/pod1", "pod:ns/pod2", "pod:ns/pod3"}, + }, + expectedIndex: map[string][]string{ + "resourceclaim:ns/rc1": {"node:node1=1", "node:node2=1", "node:node3=1"}, + "resourceclaim:ns/rc2": {"node:node1=1", "node:node2=1", "node:node3=1"}, + "resourceclaim:ns/rc3": {"node:node1=1", "node:node2=1", "node:node3=1"}, + }, + }, + { + desc: "no index for resourceclaims - dropping below threshold", + startingGraph: func() *Graph { + g := NewTestGraph() + g.AddPod(podWithResourceClaims("pod1", "node1")) + g.AddPod(podWithResourceClaims("pod2", "node2")) + g.AddPod(podWithResourceClaims("pod3", "node3")) + return g + }(), + graphTransformer: func(g *Graph) { + g.DeletePod("pod1", "ns") + }, + expectedGraph: map[string][]string{ + "node:node2": {}, + "node:node3": {}, + "pod:ns/pod2": {"node:node2"}, + "pod:ns/pod3": {"node:node3"}, + "resourceclaim:ns/rc1": {"pod:ns/pod2", "pod:ns/pod3"}, + "resourceclaim:ns/rc2": {"pod:ns/pod2", "pod:ns/pod3"}, + "resourceclaim:ns/rc3": {"pod:ns/pod2", "pod:ns/pod3"}, + }, + expectedIndex: map[string][]string{}, + }, + { + desc: "resourceslices adding", + startingGraph: NewTestGraph(), + graphTransformer: func(g *Graph) { + g.AddResourceSlice("s1", "node1") + g.AddResourceSlice("s2", "node2") + g.AddResourceSlice("s3", "node3") + }, + expectedGraph: map[string][]string{ + "node:node1": {}, + "node:node2": {}, + "node:node3": {}, + "resourceslice:s1": {"node:node1"}, + "resourceslice:s2": {"node:node2"}, + "resourceslice:s3": {"node:node3"}, + }, + expectedIndex: map[string][]string{}, + }, + { + desc: "resourceslices deleting", + startingGraph: func() *Graph { + g := NewTestGraph() + g.AddResourceSlice("s1", "node1") + g.AddResourceSlice("s2", "node2") + g.AddResourceSlice("s3", "node3") + return g + }(), + graphTransformer: func(g *Graph) { + g.DeleteResourceSlice("s1") + }, + expectedGraph: map[string][]string{ + "node:node2": {}, + "node:node3": {}, + "resourceslice:s2": {"node:node2"}, + "resourceslice:s3": {"node:node3"}, + }, + expectedIndex: map[string][]string{}, + }, + { + desc: "volumeattachments adding", + startingGraph: NewTestGraph(), + graphTransformer: func(g *Graph) { + g.AddVolumeAttachment("va1", "node1") + g.AddVolumeAttachment("va2", "node2") + g.AddVolumeAttachment("va3", "node3") + }, + expectedGraph: map[string][]string{ + "node:node1": {}, + "node:node2": {}, + "node:node3": {}, + "volumeattachment:va1": {"node:node1"}, + "volumeattachment:va2": {"node:node2"}, + "volumeattachment:va3": {"node:node3"}, + }, + expectedIndex: map[string][]string{}, + }, + { + desc: "volumeattachments deleting", + startingGraph: func() *Graph { + g := NewTestGraph() + g.AddVolumeAttachment("va1", "node1") + g.AddVolumeAttachment("va2", "node2") + g.AddVolumeAttachment("va3", "node3") + return g + }(), + graphTransformer: func(g *Graph) { + g.DeleteVolumeAttachment("va1") + }, + expectedGraph: map[string][]string{ + "node:node2": {}, + "node:node3": {}, + "volumeattachment:va2": {"node:node2"}, + "volumeattachment:va3": {"node:node3"}, + }, + expectedIndex: map[string][]string{}, + }, + { + desc: "persistentvolumes adding", + startingGraph: NewTestGraph(), + graphTransformer: func(g *Graph) { + g.AddPV(pv("pv1", "pvc1", "")) + g.AddPV(pv("pv2", "pvc2", "")) + g.AddPV(pv("pv3", "pvc3", "")) + }, + expectedGraph: map[string][]string{ + "pv:pv1": {"pvc:ns/pvc1"}, + "pv:pv2": {"pvc:ns/pvc2"}, + "pv:pv3": {"pvc:ns/pvc3"}, + "pvc:ns/pvc1": {}, + "pvc:ns/pvc2": {}, + "pvc:ns/pvc3": {}, + }, + expectedIndex: map[string][]string{}, + }, + { + desc: "persistentvolumes deleting", + startingGraph: func() *Graph { + g := NewTestGraph() + g.AddPV(pv("pv1", "pvc1", "")) + g.AddPV(pv("pv2", "pvc2", "")) + g.AddPV(pv("pv3", "pvc3", "")) + return g + }(), + graphTransformer: func(g *Graph) { + g.DeletePV("pv1") + }, + expectedGraph: map[string][]string{ + "pv:pv2": {"pvc:ns/pvc2"}, + "pv:pv3": {"pvc:ns/pvc3"}, + "pvc:ns/pvc2": {}, + "pvc:ns/pvc3": {}, + }, + expectedIndex: map[string][]string{}, + }, + { + desc: "persistentvolumes with secrets", + startingGraph: NewTestGraph(), + graphTransformer: func(g *Graph) { + g.AddPV(pv("pv1", "pvc1", "s1")) + g.AddPV(pv("pv2", "pvc2", "s2")) + g.AddPV(pv("pv3", "pvc3", "s3")) + }, + expectedGraph: map[string][]string{ + "pv:pv1": {"pvc:ns/pvc1"}, + "pv:pv2": {"pvc:ns/pvc2"}, + "pv:pv3": {"pvc:ns/pvc3"}, + "pvc:ns/pvc1": {}, + "pvc:ns/pvc2": {}, + "pvc:ns/pvc3": {}, + "secret:ns/s1": {"pv:pv1"}, + "secret:ns/s2": {"pv:pv2"}, + "secret:ns/s3": {"pv:pv3"}, + }, + expectedIndex: map[string][]string{}, + }, + } + + for _, tc := range cases { + t.Run(tc.desc, func(t *testing.T) { + tc.graphTransformer(tc.startingGraph) + expectGraph(t, tc.startingGraph, tc.expectedGraph) + expectIndex(t, tc.startingGraph, tc.expectedIndex) + }) + } +} diff --git a/plugin/pkg/auth/authorizer/node/node_authorizer.go b/plugin/pkg/auth/authorizer/node/node_authorizer.go index d68bad01d92..93747786b0b 100644 --- a/plugin/pkg/auth/authorizer/node/node_authorizer.go +++ b/plugin/pkg/auth/authorizer/node/node_authorizer.go @@ -458,9 +458,17 @@ func (r *NodeAuthorizer) hasPathFrom(nodeName string, startingType vertexType, s return false, fmt.Errorf("node '%s' cannot get unknown %s %s/%s", nodeName, vertexTypes[startingType], startingNamespace, startingName) } - // Fast check to see if we know of a destination edge - if r.graph.destinationEdgeIndex[startingVertex.ID()].has(nodeVertex.ID()) { - return true, nil + if index, indexExists := r.graph.destinationEdgeIndex[startingVertex.ID()]; indexExists { + // Fast check to see if we know of a destination edge + if index.has(nodeVertex.ID()) { + return true, nil + } + // For some types of vertices, the destination edge index is authoritative + // (as long as it exists), hence we can fail-fast here instead of running + // a potentially costly DFS below. + if vertexTypeWithAuthoritativeIndex[startingType] { + return false, fmt.Errorf("node '%s' cannot get %s %s/%s, no relationship to this object was found in the node authorizer graph", nodeName, vertexTypes[startingType], startingNamespace, startingName) + } } found := false diff --git a/plugin/pkg/auth/authorizer/node/node_authorizer_test.go b/plugin/pkg/auth/authorizer/node/node_authorizer_test.go index 8ddc41ae0cf..0c5836c065e 100644 --- a/plugin/pkg/auth/authorizer/node/node_authorizer_test.go +++ b/plugin/pkg/auth/authorizer/node/node_authorizer_test.go @@ -996,6 +996,50 @@ func BenchmarkWriteIndexMaintenance(b *testing.B) { }) } +func BenchmarkUnauthorizedRequests(b *testing.B) { + + // Run with: + // go test ./plugin/pkg/auth/authorizer/node -benchmem -bench BenchmarkUnauthorizedRequests -run None + + opts := &sampleDataOpts{ + nodes: 5000, + namespaces: 1, + podsPerNode: 1, + sharedConfigMapsPerPod: 1, + } + nodes, pods, pvs, attachments, slices := generate(opts) + + // Create an additional Node that doesn't have access to a shared ConfigMap + // that all the other Nodes are authorized to read. + additionalNodeName := "nodeX" + nsName := "ns0" + additionalNode := &user.DefaultInfo{Name: fmt.Sprintf("system:node:%s", additionalNodeName), Groups: []string{"system:nodes"}} + pod := &corev1.Pod{} + pod.Name = "pod-X" + pod.Namespace = nsName + pod.Spec.NodeName = additionalNodeName + pod.Spec.ServiceAccountName = "svcacct-X" + pods = append(pods, pod) + + g := NewGraph() + populate(g, nodes, pods, pvs, attachments, slices) + + identifier := nodeidentifier.NewDefaultNodeIdentifier() + authz := NewAuthorizer(g, identifier, bootstrappolicy.NodeRules()) + + attrs := authorizer.AttributesRecord{User: additionalNode, ResourceRequest: true, Verb: "get", Resource: "configmaps", Name: "configmap0-shared", Namespace: nsName} + + b.SetParallelism(500) + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + decision, _, _ := authz.Authorize(context.Background(), attrs) + if decision != authorizer.DecisionNoOpinion { + b.Errorf("expected %v, got %v", authorizer.DecisionNoOpinion, decision) + } + } + }) +} + func BenchmarkAuthorization(b *testing.B) { g := NewGraph()