From 9e0b46a2b0e64190c55d8c309b3588ba1bcd6904 Mon Sep 17 00:00:00 2001
From: Noah Stride <noah.stride@goteleport.com>
Date: Mon, 7 Oct 2024 14:57:11 +0100
Subject: [PATCH] Fix Workload Attestation failure regression (#47261)

* Tweak Workload Attestation failure handling

* Add future TODO

* Tidy up flow a little more explicitly
---
 lib/tbot/service_spiffe_workload_api.go  | 55 +++++++++++++++++-------
 lib/tbot/spiffe/workloadattest/attest.go | 11 +++--
 2 files changed, 47 insertions(+), 19 deletions(-)

diff --git a/lib/tbot/service_spiffe_workload_api.go b/lib/tbot/service_spiffe_workload_api.go
index 3747efc0f53b2..71e52698e542a 100644
--- a/lib/tbot/service_spiffe_workload_api.go
+++ b/lib/tbot/service_spiffe_workload_api.go
@@ -484,33 +484,56 @@ func filterSVIDRequests(
 	return filtered
 }
 
-func (s *SPIFFEWorkloadAPIService) authenticateClient(ctx context.Context) (*slog.Logger, workloadattest.Attestation, error) {
-	// The zero value of the attestation is equivalent to no attestation.
-	var att workloadattest.Attestation
-
+func (s *SPIFFEWorkloadAPIService) authenticateClient(
+	ctx context.Context,
+) (*slog.Logger, workloadattest.Attestation, error) {
 	p, ok := peer.FromContext(ctx)
 	if !ok {
-		return nil, att, trace.BadParameter("peer not found in context")
+		return nil, workloadattest.Attestation{}, trace.BadParameter("peer not found in context")
 	}
 	log := s.log
-	authInfo, ok := p.AuthInfo.(uds.AuthInfo)
 
-	if ok && authInfo.Creds != nil {
-		var err error
-		att, err = s.attestor.Attest(ctx, authInfo.Creds.PID)
-		if err != nil {
-			return nil, att, trace.Wrap(err, "performing workload attestation")
-		}
-		log = log.With(
-			"workload", slog.LogValuer(att),
-		)
-	}
 	if p.Addr.String() != "" {
 		log = log.With(
 			slog.String("remote_addr", p.Addr.String()),
 		)
 	}
 
+	authInfo, ok := p.AuthInfo.(uds.AuthInfo)
+	// We expect Creds to be nil/unset if the client is connecting via TCP and
+	// therefore there is no workload attestation that can be completed.
+	if !ok || authInfo.Creds == nil {
+		return log, workloadattest.Attestation{}, nil
+	}
+
+	// For a UDS, sometimes we are unable to determine the PID of the calling
+	// workload. This can happen if the caller is calling from another process
+	// namespace. In this case, Creds will be non-nil but the PID will be 0.
+	//
+	// We should fail softly here as there could be SVIDs that do not require
+	// workload attestation.
+	if authInfo.Creds.PID == 0 {
+		log.DebugContext(
+			ctx, "Failed to determine the PID of the calling workload. TBot may be running in a different process namespace to the workload. Workload attestation will not be completed.")
+		return log, workloadattest.Attestation{}, nil
+	}
+
+	att, err := s.attestor.Attest(ctx, authInfo.Creds.PID)
+	if err != nil {
+		// Fail softly as there may be SVIDs configured that don't require any
+		// workload attestation and we should still issue those.
+		log.ErrorContext(
+			ctx,
+			"Workload attestation failed",
+			"error", err,
+			"pid", authInfo.Creds.PID,
+		)
+		return log, workloadattest.Attestation{}, nil
+	}
+	log = log.With(
+		"workload", slog.LogValuer(att),
+	)
+
 	return log, att, nil
 }
 
diff --git a/lib/tbot/spiffe/workloadattest/attest.go b/lib/tbot/spiffe/workloadattest/attest.go
index cd3d87cf65dc1..a50721b847ca1 100644
--- a/lib/tbot/spiffe/workloadattest/attest.go
+++ b/lib/tbot/spiffe/workloadattest/attest.go
@@ -27,6 +27,9 @@ import (
 
 // Attestation holds the results of the attestation process carried out on a
 // PID by the attestor.
+//
+// The zero value of this type indicates that no attestation was performed or
+// was successful.
 type Attestation struct {
 	Unix       UnixAttestation
 	Kubernetes KubernetesAttestation
@@ -82,10 +85,12 @@ func NewAttestor(log *slog.Logger, cfg Config) (*Attestor, error) {
 
 func (a *Attestor) Attest(ctx context.Context, pid int) (Attestation, error) {
 	a.log.DebugContext(ctx, "Starting workload attestation", "pid", pid)
-	defer a.log.DebugContext(ctx, "Finished workload attestation complete", "pid", pid)
+	defer a.log.DebugContext(ctx, "Finished workload attestation", "pid", pid)
 
-	att := Attestation{}
-	var err error
+	var (
+		att Attestation
+		err error
+	)
 
 	// We always perform the unix attestation first
 	att.Unix, err = a.unix.Attest(ctx, pid)