Repository navigation
atenet: Router checks the routed worker's SPIFFE ID on atunnel's cert - #2678
Conversation
afd6568 to
5f1f173
Compare
Eitan Yarmush (EItanya)
left a comment
Eitan Yarmush (EItanya)
left a comment
There was a problem hiding this comment.
Overall makes sense to me
| - serviceaccounts | ||
| verbs: | ||
| - create | ||
| - patch |
There was a problem hiding this comment.
Greg Castle (@destijl) do you think these new permissions will be a security issue?
There was a problem hiding this comment.
ate-controller already has CRUD on pods cluster-wide. These don't seem scarier than that.
There was a problem hiding this comment.
Sounds good, just checking in
| // applyWorkerServiceAccount creates the default ServiceAccount worker pods run | ||
| // as. The pools in a namespace share it, so none of them owns it. | ||
| func (r *WorkerPoolReconciler) applyWorkerServiceAccount(ctx context.Context, namespace string) error { | ||
| saAC := corev1ac.ServiceAccount(installdefaults.WorkerServiceAccount, namespace) |
There was a problem hiding this comment.
It seems like the serviceAccount should be something set on the WorkerPool CRD --- there's nothing that says it needs to be the same service account in each namespace or across all workerpools.
But that can be a followup.
There was a problem hiding this comment.
You already can set the ServiceAccount, but the current meaning is an existing one, not create. So we'd just need to figure out the semantic is all
There was a problem hiding this comment.
not sure I follow.
This function is called only if you dont have a serviceaccount set see (line 322 above)
if wp.Spec.Template == nil || wp.Spec.Template.ServiceAccountName == nil {
if err := r.applyWorkerServiceAccount(ctx, wp.Namespace); err != nil {
return err
}
}So if you set up your service account - great, if you dont, we set it up for you.
| - serviceaccounts | ||
| verbs: | ||
| - create | ||
| - patch |
There was a problem hiding this comment.
ate-controller already has CRUD on pods cluster-wide. These don't seem scarier than that.
3be940d to
7c22343
Compare
…ificate export NODE_EXTRA_CA_CERTS=/etc/ssl/cert.pem
7c22343 to
3c2231a
Compare
Part of #2078 (Envoy dataplane, Eitan Yarmush (@EItanya) John Howard (@howardjohn) for agw)
The router accepted any podidentity cert in the trust domain as atunnel, so any pod in the cluster could pose as a worker. It now requires the exact SPIFFE ID of the worker each request is routed to.
worker_spiffe_id, built from the pod's namespace and ServiceAccount), and ateapi copies it into the actor's worker assignment. ateapi rejects a value that isn't a valid SPIFFE ID.--upstream-spiffe-prefixis removed; the cluster still checks the trust domain.ateworkerServiceAccount the controller creates, instead ofdefault, so the worker's ID isn't shared with every other pod in the namespace.Remaining gap: another pod running as the same ServiceAccount in the same namespace can still pose as the worker. Closing it needs a signer that puts the pod in the worker's SPIFFE ID; atecontroller would then record that ID, with no router change.
Breaking change
spec.template.serviceAccountNameneed a restart. Upgrade atecontroller and ate-api-server before atenet-router.ateworkerServiceAccount by default instead ofdefault. RBAC, Workload Identity bindings and imagePullSecrets attached todefaultno longer apply to them; setserviceAccountName: defaultor move those toateworker.--upstream-spiffe-prefix.