Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v16] Fix Workload Attestation failure regression #47274

Merged
merged 3 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 39 additions & 16 deletions lib/tbot/service_spiffe_workload_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
11 changes: 8 additions & 3 deletions lib/tbot/spiffe/workloadattest/attest.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Loading