(WIP) ateom/actor ID split: Ateom uses MintAteomActorCertificate - Agentgateway support

cmd/ateapi/internal/controlapi/actor.go

@@ -704,9 +704,6 @@ func (s *RPCService) MintActorCertificate(ctx context.Context, req *ateapipb.Min
 		return nil, status.Errorf(codes.InvalidArgument, "Failed to verify CSR signature")
 	}
 
-	// TODO(identity): Atunnel certificates should probably have a separate RPC,
-	// since different callers will be authorized to get atunnel certificates vs
-	// actor self-identity certificates.
 	var template *x509.Certificate
 	switch req.GetPurpose() {
 	case ateapipb.ActorCertificatePurpose_ACTOR_CERTIFICATE_PURPOSE_ATUNNEL:

cmd/atelet/ateomsupport.go

@@ -31,8 +31,7 @@ import (
 
 type ateomSupportServer struct {
 	ateletpb.UnimplementedAteomSupportServer
-	controlClient ateapipb.ControlClient
-	workers       ateapipb.WorkerServiceClient
+	workers ateapipb.WorkerServiceClient
 }
 
 func (b *ateomSupportServer) MintActorCertificate(ctx context.Context, req *ateletpb.MintActorCertificateRequest) (*ateletpb.MintActorCertificateResponse, error) {
@@ -46,14 +45,13 @@ func (b *ateomSupportServer) MintActorCertificate(ctx context.Context, req *atel
 	// requested actor?  ate-api-server will further check that we (the atelet)
 	// are allowed to request a certificate for the actor.
 
-	resp, err := b.controlClient.MintActorCertificate(ctx, &ateapipb.MintActorCertificateRequest{
+	resp, err := b.workers.MintAteomActorCertificate(ctx, &ateapipb.MintAteomActorCertificateRequest{
 		Actor: &ateapipb.ObjectRef{
 			Atespace: req.GetActorAtespace(),
 			Name:     req.GetActorName(),
 		},
 		ActorUid:                  req.GetActorUid(),
 		CertificateSigningRequest: req.GetCertificateSigningRequest(),
-		Purpose:                   ateapipb.ActorCertificatePurpose_ACTOR_CERTIFICATE_PURPOSE_ATUNNEL,
 	})
 	if err != nil {
 		return nil, fmt.Errorf("mint actor certificate: %w", err)

cmd/atelet/ateomsupport_test.go

@@ -88,8 +88,10 @@ func workerCertificate(t *testing.T, podUID, nodeName string) *x509.Certificate
 type fakeWorkerService struct {
 	ateapipb.WorkerServiceClient
 
-	got []*ateapipb.SetWorkerCapacityRequest
-	err error
+	got      []*ateapipb.SetWorkerCapacityRequest
+	mintGot  []*ateapipb.MintAteomActorCertificateRequest
+	mintResp *ateapipb.MintAteomActorCertificateResponse
+	err      error
 }
 
 func (s *fakeWorkerService) SetWorkerCapacity(_ context.Context, in *ateapipb.SetWorkerCapacityRequest, _ ...grpc.CallOption) (*ateapipb.SetWorkerCapacityResponse, error) {
@@ -100,6 +102,17 @@ func (s *fakeWorkerService) SetWorkerCapacity(_ context.Context, in *ateapipb.Se
 	return &ateapipb.SetWorkerCapacityResponse{}, nil
 }
 
+func (s *fakeWorkerService) MintAteomActorCertificate(_ context.Context, in *ateapipb.MintAteomActorCertificateRequest, _ ...grpc.CallOption) (*ateapipb.MintAteomActorCertificateResponse, error) {
+	if s.err != nil {
+		return nil, s.err
+	}
+	s.mintGot = append(s.mintGot, in)
+	if s.mintResp != nil {
+		return s.mintResp, nil
+	}
+	return &ateapipb.MintAteomActorCertificateResponse{}, nil
+}
+
 func TestSetWorkerCapacityRecordsWhatTheWorkerSays(t *testing.T) {
 	workers := &fakeWorkerService{}
 	svc := &ateomSupportServer{workers: workers}
@@ -167,3 +180,57 @@ func TestSetWorkerCapacitySurfacesRejection(t *testing.T) {
 		t.Error("a rejected report returned success, so the worker would not retry")
 	}
 }
+
+func TestMintActorCertificateForwardsToWorkerService(t *testing.T) {
+	wantCerts := [][]byte{[]byte("cert-der-bytes")}
+	workers := &fakeWorkerService{
+		mintResp: &ateapipb.MintAteomActorCertificateResponse{
+			ActorCertificates: wantCerts,
+		},
+	}
+	svc := &ateomSupportServer{workers: workers}
+
+	ctx := workerContext(t, "pod-a")
+	resp, err := svc.MintActorCertificate(ctx, &ateletpb.MintActorCertificateRequest{
+		ActorAtespace:             "team-a",
+		ActorName:                 "actor-1",
+		ActorUid:                  "actor-uid-1",
+		CertificateSigningRequest: []byte("csr-bytes"),
+	})
+	if err != nil {
+		t.Fatalf("MintActorCertificate() failed: %v", err)
+	}
+
+	want := []*ateapipb.MintAteomActorCertificateRequest{{
+		Actor: &ateapipb.ObjectRef{
+			Atespace: "team-a",
+			Name:     "actor-1",
+		},
+		ActorUid:                  "actor-uid-1",
+		CertificateSigningRequest: []byte("csr-bytes"),
+	}}
+	if diff := cmp.Diff(want, workers.mintGot, protocmp.Transform()); diff != "" {
+		t.Errorf("forwarded mint request mismatch (-want +got):\n%s", diff)
+	}
+	if diff := cmp.Diff(wantCerts, resp.GetActorCertificates()); diff != "" {
+		t.Errorf("returned certificates mismatch (-want +got):\n%s", diff)
+	}
+}
+
+func TestMintActorCertificateRequiresCertificate(t *testing.T) {
+	workers := &fakeWorkerService{}
+	svc := &ateomSupportServer{workers: workers}
+
+	_, err := svc.MintActorCertificate(context.Background(), &ateletpb.MintActorCertificateRequest{
+		ActorAtespace:             "team-a",
+		ActorName:                 "actor-1",
+		ActorUid:                  "actor-uid-1",
+		CertificateSigningRequest: []byte("csr-bytes"),
+	})
+	if status.Code(err) != codes.Unauthenticated {
+		t.Errorf("unauthenticated request returned %v, want Unauthenticated", err)
+	}
+	if len(workers.mintGot) != 0 {
+		t.Errorf("unauthenticated request still forwarded %v", workers.mintGot)
+	}
+}

cmd/atelet/main.go

@@ -368,8 +368,7 @@ func main() {
 	ateomFacingSrv := grpc.NewServer(grpc.Creds(credentials.NewTLS(ateomFacingTLS)))
 
 	ateletpb.RegisterAteomSupportServer(ateomFacingSrv, &ateomSupportServer{
-		controlClient: ateapipb.NewControlClient(ateapiConn),
-		workers:       ateapipb.NewWorkerServiceClient(ateapiConn),
+		workers: ateapipb.NewWorkerServiceClient(ateapiConn),
 	})
 	go func() {
 		if err := ateomFacingSrv.Serve(ateomFacingLis); err != nil {

cmd/atenet/internal/router/egress/credentials.go

@@ -80,7 +80,7 @@ func (h *Handler) applyEffects(ctx context.Context, ref resources.ActorRef, dest
 	// the same form the CONNECT leg verified and shared as filter state. The
 	// provider authenticates this gateway and trusts its assertion; see
 	// pkg/proto/credproviderpb.
-	actorSpiffeID := resources.ActorSPIFFEID(ref).String()
+	actorSpiffeID := resources.AteomForActorSPIFFEID(ref).String()
 
 	setHeaders := make([]*corev3.HeaderValueOption, 0, len(injections))
 	for _, inj := range injections {

cmd/atenet/internal/router/egress/egress.go

@@ -34,7 +34,6 @@ import (
 	"log/slog"
 	"net"
 	"net/url"
-	"slices"
 	"strconv"
 	"strings"
 	"time"
@@ -48,7 +47,6 @@ import (
 	"github.com/agent-substrate/substrate/cmd/atenet/internal/router/extproc"
 	"github.com/agent-substrate/substrate/internal/egresspolicy"
 	"github.com/agent-substrate/substrate/internal/resources"
-	"github.com/agent-substrate/substrate/internal/substratex509"
 	"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
 	"github.com/agent-substrate/substrate/pkg/proto/credproviderpb"
 )
@@ -63,10 +61,11 @@ const (
 	// client sends under this name is discarded and replaced by Envoy's own
 	// value.
 	//
-	// This is the only channel that can carry a whole certificate to ext_proc:
-	// the CEL request attributes Envoy exposes (subject, SANs, SHA-256 digest)
-	// cannot express the custom ActorIdentity X.509 extension this gateway
-	// authorizes on.
+	// This is the only channel that can carry a whole certificate to ext_proc
+	// so the gateway can verify the chain, key usages, and ateom SPIFFE URI.
+	//
+	// TODO(identity): Audit that this cannot be stomped by a header sent by the
+	// actor.
 	forwardedClientCertHeader = "x-forwarded-client-cert"
 	// xfccChainKey is the x-forwarded-client-cert key holding the URL-encoded
 	// PEM of the full presented chain, leaf first.
@@ -157,7 +156,7 @@ func (h *Handler) handleConnect(ctx context.Context, md *extproc.RequestMetadata
 			"egress unavailable: no actor-identity CA configured")
 	}
 
-	identity, err := h.authenticateActorCertificate(md)
+	actorRef, err := h.authenticateActorCertificate(md)
 	if err != nil {
 		// The body stays generic on purpose: an actor that fails authentication
 		// has not proven it is anyone, so it gets no detail about why. The
@@ -168,14 +167,11 @@ func (h *Handler) handleConnect(ctx context.Context, md *extproc.RequestMetadata
 			"egress denied: invalid actor certificate")
 	}
 
-	if err := validateIdentity(identity); err != nil {
-		return extproc.Result{}, err
-	}
-	if err := h.validateActor(ctx, identity); err != nil {
+	if err := h.validateActor(ctx, actorRef); err != nil {
 		return extproc.Result{}, err
 	}
 
-	ref := resources.ActorRef{Atespace: identity.Atespace, Name: identity.ActorName}
+	ref := resources.ActorRef{Atespace: actorRef.Atespace, Name: actorRef.Name}
 
 	// atunnel always sends the address the actor's kernel dialed, never a
 	// name. Refuse a name here, where there is still a response to do it with.
@@ -194,7 +190,6 @@ func (h *Handler) handleConnect(ctx context.Context, md *extproc.RequestMetadata
 	decision := policy.Evaluate(dest)
 	attrs := []any{
 		slog.Any("actor", ref),
-		slog.String("actorUid", identity.ActorUid),
 		slog.String("leg", leg),
 		slog.String("destination", md.Host),
 		slog.Int("rule", decision.RuleIndex),
@@ -265,93 +260,61 @@ func (h *Handler) lookupPolicy(ctx context.Context, leg string, ref resources.Ac
 	return policy, nil
 }
 
-// validateIdentity checks that the identity a verified actor certificate
-// carries names an actor that could exist at all, before it is used as a
-// control-plane lookup key.
-func validateIdentity(identity *substratex509.ActorIdentity) error {
-	// The CA only ever mints these from control-plane state, so a name that is
-	// not a legal resource name means the CA or its inputs are compromised.
-	if !resources.IsValidResourceName(identity.Atespace) || !resources.IsValidResourceName(identity.ActorName) {
-		return extproc.NewReqError(envoy_type.StatusCode_Forbidden,
-			"egress denied: invalid actor identity %q/%q", identity.Atespace, identity.ActorName)
-	}
-	return nil
-}
-
-// validateActor checks the identity a certificate certifies against the control
-// plane's current view of that actor: it still exists, it is the actor the
-// certificate was issued to, and it is running. Every error it returns is
-// already a client-facing ext_proc denial.
-func (h *Handler) validateActor(ctx context.Context, identity *substratex509.ActorIdentity) error {
-	atespace := identity.Atespace
-	actorName := identity.ActorName
-	actorUID := identity.ActorUid
-
-	// Confirm the certified actor still exists. The name is only a lookup key
-	// here; the UID below is what actually authorizes.
+// validateActor checks the actor certified by the certificate against the control
+// plane's current view of that actor: it still exists and it is running. Every
+// error it returns is already a client-facing ext_proc denial.
+func (h *Handler) validateActor(ctx context.Context, actorRef resources.ActorRef) error {
+	// Confirm the certified actor still exists.
 	// TODO: this can cause heavy load on ate api server. Change it based on https://github.com/agent-substrate/substrate/issues/592.
 	actor, err := h.apiClient.GetActor(ctx, &ateapipb.GetActorRequest{
-		Actor: &ateapipb.ObjectRef{Atespace: atespace, Name: actorName},
+		Actor: &ateapipb.ObjectRef{Atespace: actorRef.Atespace, Name: actorRef.Name},
 	})
 	if err != nil {
-		return mapEgressIdentityError(atespace, actorName, err)
-	}
-
-	// Authorize on the UID, not the name. The UID the CA certified has to match the UID the
-	// control plane holds right now.
-	if uid := actor.GetMetadata().GetUid(); uid != actorUID {
-		slog.WarnContext(ctx, "egress denied: actor UID mismatch",
-			slog.String("atespace", atespace),
-			slog.String("actor", actorName),
-			slog.String("certificateActorUid", actorUID),
-			slog.String("currentActorUid", uid))
-		return extproc.NewReqError(envoy_type.StatusCode_Forbidden,
-			"egress denied: actor %q/%q is not the actor this certificate was issued to", atespace, actorName)
+		return mapEgressIdentityError(actorRef.Atespace, actorRef.Name, err)
 	}
 
 	// The actor performing egress must actually be running.
 	if actor.GetStatus().GetState() != ateapipb.ActorState_ACTOR_STATE_RUNNING {
 		return extproc.NewReqError(envoy_type.StatusCode_Forbidden,
-			"egress denied: actor %q/%q is %s, not running", atespace, actorName, actor.GetStatus().GetState())
+			"egress denied: actor %q/%q is %s, not running", actorRef.Atespace, actorRef.Name, actor.GetStatus().GetState())
 	}
 	return nil
 }
 
 // authenticateActorCertificate turns the mTLS peer certificate Envoy recorded
-// on the request into a verified ActorIdentity, or an error describing why it
+// on the request into a verified ActorRef, or an error describing why it
 // cannot be trusted.
-func (h *Handler) authenticateActorCertificate(md *extproc.RequestMetadata) (*substratex509.ActorIdentity, error) {
+func (h *Handler) authenticateActorCertificate(md *extproc.RequestMetadata) (resources.ActorRef, error) {
 	if certificate := md.Attribute(agentgatewayClientCertificateAttribute); certificate != "" {
 		chain, err := parseCertificateChainPEM([]byte(certificate))
 		if err != nil {
-			return nil, err
+			return resources.ActorRef{}, err
 		}
 		return h.verifyActorCertificate(chain)
 	}
 	header := md.Header(forwardedClientCertHeader)
 	if header == "" {
-		return nil, fmt.Errorf("request carries no %s header", forwardedClientCertHeader)
+		return resources.ActorRef{}, fmt.Errorf("request carries no %s header", forwardedClientCertHeader)
 	}
 	chain, err := parseXFCCChain(header)
 	if err != nil {
-		return nil, err
+		return resources.ActorRef{}, err
 	}
 	return h.verifyActorCertificate(chain)
 }
 
 // verifyActorCertificate checks that chain[0] is a live, non-CA, client-auth
-// actor certificate issued by the actor-identity CA, and returns the single
-// ActorIdentity it carries.
+// ateom actor certificate issued by the actor-identity CA, and returns the
+// ActorRef from its SPIFFE URI.
 //
 // The chain is verified here even though Envoy already did it at the handshake
 // (require_client_certificate with the actor-identity CA as trusted_ca). We have
-// to parse the certificate anyway to read the ActorIdentity extension, which
-// Envoy cannot see, and trusting a parsed-but-unverified certificate is a
-// well-worn source of CVEs. It also keeps the handler safe if the Envoy config
-// is ever loosened, and costs one signature check per CONNECT rather than per
-// request. The IsCA, ClientAuth-EKU, and purpose checks below have no Envoy-side
-// equivalent at all.
-func (h *Handler) verifyActorCertificate(chain []*x509.Certificate) (*substratex509.ActorIdentity, error) {
+// to parse the certificate anyway to inspect its SPIFFE URI and usages, and
+// trusting a parsed-but-unverified certificate is a well-worn source of CVEs.
+// It also keeps the handler safe if the Envoy config is ever loosened, and costs
+// one signature check per CONNECT rather than per request. The IsCA, ClientAuth-EKU,
+// and ateom SPIFFE URI checks below have no Envoy-side equivalent at all.
+func (h *Handler) verifyActorCertificate(chain []*x509.Certificate) (resources.ActorRef, error) {
 	leaf := chain[0]
 	intermediates := x509.NewCertPool()
 	for _, cert := range chain[1:] {
@@ -360,59 +323,35 @@ func (h *Handler) verifyActorCertificate(chain []*x509.Certificate) (*substratex
 
 	now := time.Now()
 	if now.Before(leaf.NotBefore) || !now.Before(leaf.NotAfter) {
-		return nil, fmt.Errorf("actor certificate is outside its validity period (%s..%s)",
+		return resources.ActorRef{}, fmt.Errorf("actor certificate is outside its validity period (%s..%s)",
 			leaf.NotBefore.Format(time.RFC3339), leaf.NotAfter.Format(time.RFC3339))
 	}
 	// An actor certificate is an end-entity credential. Refusing IsCA here stops
 	// a leaked or mis-issued CA certificate from being replayed as a leaf: chain
 	// verification alone would happily accept one.
 	if leaf.IsCA {
-		return nil, fmt.Errorf("actor certificate is a CA certificate")
-	}
-	// Require ClientAuth explicitly rather than relying on VerifyOptions.KeyUsages:
-	// an empty ExtKeyUsage means "any usage" to crypto/x509 and would pass. This
-	// mirrors the check atunnel makes on the certificate when it mints it.
-	if !slices.Contains(leaf.ExtKeyUsage, x509.ExtKeyUsageClientAuth) {
-		return nil, fmt.Errorf("actor certificate cannot authenticate a TLS client")
+		return resources.ActorRef{}, fmt.Errorf("actor certificate is a CA certificate")
 	}
+
 	if _, err := leaf.Verify(x509.VerifyOptions{
 		Roots:         h.actorIdentityRoots,
 		Intermediates: intermediates,
 		CurrentTime:   now,
 		KeyUsages:     []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth},
 	}); err != nil {
-		return nil, fmt.Errorf("actor certificate is not signed by the actor-identity CA: %w", err)
+		return resources.ActorRef{}, fmt.Errorf("actor certificate is not signed by the actor-identity CA: %w", err)
 	}
 
-	// ActorIdentityFromCertificate returns (nil, nil) when the extension is
-	// absent, an error when there is more than one or when its contents are
-	// malformed, empty, or carry a purpose other than atunnel.
-	identity, err := substratex509.ActorIdentityFromCertificate(leaf)
+	// Check that this is an ateom certificate --- the SPIFFE URI should be of
+	// the form `spiffe://${trustdomain}/ateom-for-actor/${atespace}/${actor}`.
+	if len(leaf.URIs) != 1 {
+		return resources.ActorRef{}, fmt.Errorf("actor certificate has %d URI SANs, want 1", len(leaf.URIs))
+	}
+	ref, err := resources.ActorRefFromAteomForActorSPIFFEURL(leaf.URIs[0])
 	if err != nil {
-		return nil, fmt.Errorf("actor certificate has no single valid ActorIdentity extension: %w", err)
-	}
-	if identity == nil {
-		return nil, fmt.Errorf("actor certificate has no ActorIdentity extension")
-	}
-	// Restate what ActorIdentityFromCertificate enforces. The gateway is the
-	// component that gets hurt if that helper ever loosens, and "reject anything
-	// not scoped to atunnel" is the property this endpoint depends on: a
-	// certificate minted for some future purpose must not open a tunnel.
-	if identity.Atespace == "" || identity.ActorName == "" || identity.ActorUid == "" {
-		return nil, fmt.Errorf("actor certificate identity is incomplete")
-	}
-	if identity.Purpose != substratex509.ActorIdentityPurposeAtunnel {
-		return nil, fmt.Errorf("actor certificate purpose %q is not %q",
-			identity.Purpose, substratex509.ActorIdentityPurposeAtunnel)
-	}
-	// The CONNECT authenticates on the extension; the request legs attribute
-	// traffic to the URI SAN, via Envoy's filter state. ateapi mints both from
-	// one actor; check it rather than assume it.
-	want := resources.ActorSPIFFEID(resources.ActorRef{Atespace: identity.Atespace, Name: identity.ActorName}).String()
-	if len(leaf.URIs) != 1 || leaf.URIs[0].String() != want {
-		return nil, fmt.Errorf("actor certificate URI SANs %v do not name the actor in its ActorIdentity extension (%s)", leaf.URIs, want)
-	}
-	return identity, nil
+		return resources.ActorRef{}, fmt.Errorf("while parsing actor from SPIFFE ID: %w", err)
+	}
+	return ref, nil
 }
 
 // parseXFCCChain extracts the presented certificate chain, leaf first, from an