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

@@ -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
 }
 

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)