Allow k8s workload attestation when container is not ready

This change introduces a new configurable, `disable_container_selectors`
which configures the K8s Workload Attestor to only produce pod-related
selectors. This allows for workload attesation to succeed when the
attestor can positively locate the workload pod but cannot yet locate
the workload container at the time of attestation (e.g. postStart hook
is still executing).

See issue spiffe#3092 for more details.

Fixes: spiffe#3092

Signed-off-by: Andrew Harding <[email protected]>

doc/plugin_agent_workloadattestor_k8s.md

@@ -42,6 +42,7 @@ since [hostprocess](https://kubernetes.io/docs/tasks/configure-pod-container/cre
 
 | Configuration | Description |
 | ------------- | ----------- |
+| `disable_container_selectors` | If true, container selectors are not produced. This can be used to produce pod selectors when the workload pod is known but the workload container is not ready at the time of attestation. |
 | `kubelet_read_only_port` | The kubelet read-only port. This is mutually exlusive with `kubelet_secure_port`. |
 | `kubelet_secure_port` | The kubelet secure port. It defaults to `10250` unless `kubelet_read_only_port` is set. |
 | `kubelet_ca_path` | The path on disk to a file containing CA certificates used to verify the kubelet certificate. Required unless `skip_kubelet_verification` is set. Defaults to the cluster CA bundle `/run/secrets/kubernetes.io/serviceaccount/ca.crt`. |
@@ -134,4 +135,4 @@ This plugin is only supported on Unix systems.
 
 ### Known issues
 
-* This plugin may fail to correctly attest workloads in pods that use lifecycle hooks to alter pod start behavior. This includes Istio workloads when the `holdApplicationUntilProxyStarts` configurable is set to true. Please see [#3092](https://github.com/spiffe/spire/issues/3092) for more information.
+* This plugin may fail to correctly attest workloads in pods that use lifecycle hooks to alter pod start behavior. This includes Istio workloads when the `holdApplicationUntilProxyStarts` configurable is set to true. Please see [#3092](https://github.com/spiffe/spire/issues/3092) for more information. The `disable_container_selectors` configurable can be used successfully attest workloads in this situation, albeit with reduced selector granularity (i.e. pod selectors only).

pkg/agent/plugin/workloadattestor/k8s/k8s.go

@@ -46,13 +46,6 @@ func BuiltIn() catalog.BuiltIn {
 	return builtin(New())
 }
 
-type containerLookup int
-
-const (
-	containerInPod = iota
-	containerNotInPod
-)
-
 func builtin(p *Plugin) catalog.BuiltIn {
 	return catalog.MakeBuiltIn(pluginName,
 		workloadattestorv1.WorkloadAttestorPluginServer(p),
@@ -119,6 +112,13 @@ type HCLConfig struct {
 	// ReloadInterval controls how often TLS and token configuration is loaded
 	// from the disk.
 	ReloadInterval string `hcl:"reload_interval"`
+
+	// DisableContainerSelectors disables the gathering of selectors for the
+	// specific container running the workload. This allows attestation to
+	// succeed with just pod related selectors when the workload pod is known
+	// but the container may not be in a ready state at the time of attestation
+	// (e.g. when a postStart hook has yet to complete).
+	DisableContainerSelectors bool `hcl:"disable_container_selectors"`
 }
 
 // k8sConfig holds the configuration distilled from HCL
@@ -135,6 +135,7 @@ type k8sConfig struct {
 	KubeletCAPath              string
 	NodeName                   string
 	ReloadInterval             time.Duration
+	DisableContainerSelectors  bool
 
 	Client     *kubeletClient
 	LastReload time.Time
@@ -180,6 +181,7 @@ func (p *Plugin) Attest(ctx context.Context, req *workloadattestorv1.AttestReque
 	if err != nil {
 		return nil, err
 	}
+	podKnown := podUID != ""
 
 	// Not a Kubernetes pod
 	if containerID == "" {
@@ -204,21 +206,36 @@ func (p *Plugin) Attest(ctx context.Context, req *workloadattestorv1.AttestReque
 		var attestResponse *workloadattestorv1.AttestResponse
 		for _, item := range list.Items {
 			item := item
-			if isNotPod(item.UID, podUID) {
+			if podKnown && item.UID != podUID {
+				// The pod holding the container is known. Skip unrelated pods.
 				continue
 			}
 
-			lookupStatus, lookup := lookUpContainerInPod(containerID, item.Status, log)
-			switch lookup {
-			case containerInPod:
+			var selectorValues []string
+
+			containerStatus, containerFound := lookUpContainerInPod(containerID, item.Status, log)
+			switch {
+			case containerFound:
+				// The workload container was found in this pod. Add pod
+				// selectors. Only add workload container selectors if
+				// container selectors have not been disabled.
+				selectorValues = append(selectorValues, getSelectorValuesFromPodInfo(&item)...)
+				if !config.DisableContainerSelectors {
+					selectorValues = append(selectorValues, getSelectorValuesFromWorkloadContainerStatus(containerStatus)...)
+				}
+			case podKnown && config.DisableContainerSelectors:
+				// The workload container was not found (i.e. not ready yet?)
+				// but the pod is known. If container selectors have been
+				// disabled, then allow the pod selectors to be used.
+				selectorValues = append(selectorValues, getSelectorValuesFromPodInfo(&item)...)
+			}
+
+			if len(selectorValues) > 0 {
 				if attestResponse != nil {
 					log.Warn("Two pods found with same container Id")
 					return nil, status.Error(codes.Internal, "two pods found with same container Id")
 				}
-				attestResponse = &workloadattestorv1.AttestResponse{
-					SelectorValues: getSelectorValuesFromPodInfo(&item, lookupStatus),
-				}
-			case containerNotInPod:
+				attestResponse = &workloadattestorv1.AttestResponse{SelectorValues: selectorValues}
 			}
 		}
 
@@ -321,6 +338,7 @@ func (p *Plugin) Configure(ctx context.Context, req *configv1.ConfigureRequest)
 		KubeletCAPath:              config.KubeletCAPath,
 		NodeName:                   nodeName,
 		ReloadInterval:             reloadInterval,
+		DisableContainerSelectors:  config.DisableContainerSelectors,
 	}
 	if err := p.reloadKubeletClient(c); err != nil {
 		return nil, err
@@ -565,7 +583,7 @@ func (c *kubeletClient) GetPodList() (*corev1.PodList, error) {
 	return out, nil
 }
 
-func lookUpContainerInPod(containerID string, status corev1.PodStatus, log hclog.Logger) (*corev1.ContainerStatus, containerLookup) {
+func lookUpContainerInPod(containerID string, status corev1.PodStatus, log hclog.Logger) (*corev1.ContainerStatus, bool) {
 	for _, status := range status.ContainerStatuses {
 		// TODO: should we be keying off of the status or is the lack of a
 		// container id sufficient to know the container is not ready?
@@ -582,7 +600,7 @@ func lookUpContainerInPod(containerID string, status corev1.PodStatus, log hclog
 		}
 
 		if containerID == containerURL.Host {
-			return &status, containerInPod
+			return &status, true
 		}
 	}
 
@@ -602,16 +620,16 @@ func lookUpContainerInPod(containerID string, status corev1.PodStatus, log hclog
 		}
 
 		if containerID == containerURL.Host {
-			return &status, containerInPod
+			return &status, true
 		}
 	}
 
-	return nil, containerNotInPod
+	return nil, false
 }
 
-func getPodImageIdentifiers(containerStatusArray []corev1.ContainerStatus) map[string]bool {
+func getPodImageIdentifiers(containerStatuses ...corev1.ContainerStatus) map[string]struct{} {
 	// Map is used purely to exclude duplicate selectors, value is unused.
-	podImages := make(map[string]bool)
+	podImages := make(map[string]struct{})
 	// Note that for each pod image we generate *2* matching selectors.
 	// This is to support matching against ImageID, which has a SHA
 	// docker.io/envoyproxy/envoy-alpine@sha256:bf862e5f5eca0a73e7e538224578c5cf867ce2be91b5eaed22afc153c00363eb
@@ -620,36 +638,28 @@ func getPodImageIdentifiers(containerStatusArray []corev1.ContainerStatus) map[s
 	// while also maintaining backwards compatibility and allowing for dynamic workload registration (k8s operator)
 	// when the SHA is not yet known (e.g. before the image pull is initiated at workload creation time)
 	// More info here: https://github.com/spiffe/spire/issues/2026
-	for _, status := range containerStatusArray {
-		podImages[status.ImageID] = true
-		podImages[status.Image] = true
+	for _, containerStatus := range containerStatuses {
+		podImages[containerStatus.ImageID] = struct{}{}
+		podImages[containerStatus.Image] = struct{}{}
 	}
 	return podImages
 }
 
-func getSelectorValuesFromPodInfo(pod *corev1.Pod, status *corev1.ContainerStatus) []string {
-	podImageIdentifiers := getPodImageIdentifiers(pod.Status.ContainerStatuses)
-	podInitImageIdentifiers := getPodImageIdentifiers(pod.Status.InitContainerStatuses)
-	containerImageIdentifiers := getPodImageIdentifiers([]corev1.ContainerStatus{*status})
-
+func getSelectorValuesFromPodInfo(pod *corev1.Pod) []string {
 	selectorValues := []string{
 		fmt.Sprintf("sa:%s", pod.Spec.ServiceAccountName),
 		fmt.Sprintf("ns:%s", pod.Namespace),
 		fmt.Sprintf("node-name:%s", pod.Spec.NodeName),
 		fmt.Sprintf("pod-uid:%s", pod.UID),
 		fmt.Sprintf("pod-name:%s", pod.Name),
-		fmt.Sprintf("container-name:%s", status.Name),
 		fmt.Sprintf("pod-image-count:%s", strconv.Itoa(len(pod.Status.ContainerStatuses))),
 		fmt.Sprintf("pod-init-image-count:%s", strconv.Itoa(len(pod.Status.InitContainerStatuses))),
 	}
 
-	for containerImage := range containerImageIdentifiers {
-		selectorValues = append(selectorValues, fmt.Sprintf("container-image:%s", containerImage))
-	}
-	for podImage := range podImageIdentifiers {
+	for podImage := range getPodImageIdentifiers(pod.Status.ContainerStatuses...) {
 		selectorValues = append(selectorValues, fmt.Sprintf("pod-image:%s", podImage))
 	}
-	for podInitImage := range podInitImageIdentifiers {
+	for podInitImage := range getPodImageIdentifiers(pod.Status.InitContainerStatuses...) {
 		selectorValues = append(selectorValues, fmt.Sprintf("pod-init-image:%s", podInitImage))
 	}
 
@@ -664,6 +674,14 @@ func getSelectorValuesFromPodInfo(pod *corev1.Pod, status *corev1.ContainerStatu
 	return selectorValues
 }
 
+func getSelectorValuesFromWorkloadContainerStatus(status *corev1.ContainerStatus) []string {
+	selectorValues := []string{fmt.Sprintf("container-name:%s", status.Name)}
+	for containerImage := range getPodImageIdentifiers(*status) {
+		selectorValues = append(selectorValues, fmt.Sprintf("container-image:%s", containerImage))
+	}
+	return selectorValues
+}
+
 func tryRead(r io.Reader) string {
 	buf := make([]byte, 1024)
 	n, _ := r.Read(buf)

pkg/agent/plugin/workloadattestor/k8s/k8s_posix.go

@@ -175,7 +175,3 @@ func canonicalizePodUID(uid string) types.UID {
 		return r
 	}, uid))
 }
-
-func isNotPod(itemPodUID, podUID types.UID) bool {
-	return podUID != "" && itemPodUID != podUID
-}

pkg/agent/plugin/workloadattestor/k8s/k8s_posix_test.go

@@ -192,7 +192,7 @@ func (s *Suite) requireAttestFailWithDuplicateContainerID(p workloadattestor.Wor
 func (s *Suite) requireAttestSuccessWithPodSystemdCgroups(p workloadattestor.WorkloadAttestor) {
 	s.addPodListResponse(podListFilePath)
 	s.addCgroupsResponse(cgSystemdPidInPodFilePath)
-	s.requireAttestSuccess(p, testPodSelectors)
+	s.requireAttestSuccess(p, testPodAndContainerSelectors)
 }
 
 func TestGetContainerIDFromCGroups(t *testing.T) {

pkg/agent/plugin/workloadattestor/k8s/k8s_test.go

@@ -56,9 +56,6 @@ FwOGLt+I3+9beT0vo+pn9Rq0squewFYe3aJbwpkyfP2xOovQCdm4PC8y
 `))
 
 	testPodSelectors = []*common.Selector{
-		{Type: "k8s", Value: "container-image:docker-pullable://localhost/spiffe/blog@sha256:0cfdaced91cb46dd7af48309799a3c351e4ca2d5e1ee9737ca0cbd932cb79898"},
-		{Type: "k8s", Value: "container-image:localhost/spiffe/blog:latest"},
-		{Type: "k8s", Value: "container-name:blog"},
 		{Type: "k8s", Value: "node-name:k8s-node-1"},
 		{Type: "k8s", Value: "ns:default"},
 		{Type: "k8s", Value: "pod-image-count:2"},
@@ -75,6 +72,12 @@ FwOGLt+I3+9beT0vo+pn9Rq0squewFYe3aJbwpkyfP2xOovQCdm4PC8y
 		{Type: "k8s", Value: "pod-uid:2c48913c-b29f-11e7-9350-020968147796"},
 		{Type: "k8s", Value: "sa:default"},
 	}
+	testContainerSelectors = []*common.Selector{
+		{Type: "k8s", Value: "container-image:docker-pullable://localhost/spiffe/blog@sha256:0cfdaced91cb46dd7af48309799a3c351e4ca2d5e1ee9737ca0cbd932cb79898"},
+		{Type: "k8s", Value: "container-image:localhost/spiffe/blog:latest"},
+		{Type: "k8s", Value: "container-name:blog"},
+	}
+	testPodAndContainerSelectors = append(testPodSelectors, testContainerSelectors...)
 )
 
 type attestResult struct {
@@ -146,7 +149,7 @@ func (s *Suite) TestAttestWithPidInPodAfterRetry() {
 	select {
 	case result := <-resultCh:
 		s.Require().Nil(result.err)
-		s.requireSelectorsEqual(testPodSelectors, result.selectors)
+		s.requireSelectorsEqual(testPodAndContainerSelectors, result.selectors)
 	case <-time.After(time.Minute):
 		s.FailNow("timed out waiting for attest response")
 	}
@@ -262,6 +265,22 @@ func (s *Suite) TestAttestReachingKubeletViaNodeName() {
 	`))
 }
 
+func (s *Suite) TestAttestWhenContainerReadyButContainerSelectorsDisabled() {
+	s.startInsecureKubelet()
+	p := s.loadInsecurePluginWithExtra("disable_container_selectors = true")
+	s.addPodListResponse(podListFilePath)
+	s.addGetContainerResponsePidInPod()
+	s.requireAttestSuccess(p, testPodSelectors)
+}
+
+func (s *Suite) TestAttestWhenContainerNotReadyButContainerSelectorsDisabled() {
+	s.startInsecureKubelet()
+	p := s.loadInsecurePluginWithExtra("disable_container_selectors = true")
+	s.addPodListResponse(podListNotRunningFilePath)
+	s.addGetContainerResponsePidInPod()
+	s.requireAttestSuccess(p, testPodSelectors)
+}
+
 func (s *Suite) TestConfigure() {
 	s.generateCerts("")
 
@@ -607,6 +626,15 @@ func (s *Suite) loadInsecurePlugin() workloadattestor.WorkloadAttestor {
 `, s.kubeletPort()))
 }
 
+func (s *Suite) loadInsecurePluginWithExtra(extraConfig string) workloadattestor.WorkloadAttestor {
+	return s.loadPlugin(fmt.Sprintf(`
+		kubelet_read_only_port = %d
+		max_poll_attempts = 5
+		poll_retry_interval = "1s"
+		%s
+`, s.kubeletPort(), extraConfig))
+}
+
 func (s *Suite) startInsecureKubelet() {
 	s.setServer(httptest.NewServer(http.HandlerFunc(s.serveHTTP)))
 }
@@ -746,7 +774,7 @@ func (s *Suite) writeKey(path string, key *ecdsa.PrivateKey) {
 func (s *Suite) requireAttestSuccessWithPod(p workloadattestor.WorkloadAttestor) {
 	s.addPodListResponse(podListFilePath)
 	s.addGetContainerResponsePidInPod()
-	s.requireAttestSuccess(p, testPodSelectors)
+	s.requireAttestSuccess(p, testPodAndContainerSelectors)
 }
 
 func (s *Suite) requireAttestSuccess(p workloadattestor.WorkloadAttestor, expectedSelectors []*common.Selector) {
@@ -762,8 +790,11 @@ func (s *Suite) requireAttestFailure(p workloadattestor.WorkloadAttestor, code c
 }
 
 func (s *Suite) requireSelectorsEqual(expected, actual []*common.Selector) {
-	// assert the selectors (sorting for consistency)
+	// assert the selectors (non-destructively sorting for consistency)
+	actual = append([]*common.Selector(nil), actual...)
+	expected = append([]*common.Selector(nil), expected...)
 	util.SortSelectors(actual)
+	util.SortSelectors(expected)
 	s.RequireProtoListEqual(expected, actual)
 }
 

pkg/agent/plugin/workloadattestor/k8s/k8s_windows.go

@@ -45,7 +45,3 @@ func (p *Plugin) defaultTokenPath() string {
 	mountPoint := p.getenv(containerMountPointEnvVar)
 	return filepath.Join(mountPoint, defaultTokenPath)
 }
-
-func isNotPod(itemPodUID, podUID types.UID) bool {
-	return false
-}