Skip to content

Verify actor JWTs in the credential exchange through OIDC discovery - #3447

Merged
Tim Hockin (thockin) merged 7 commits into
agent-substrate:mainfrom
aramase:aramase/f/exchange_oidc_discovery
Nov 3, 2026
Merged

Tim Hockin (thockin) merged 7 commits into
agent-substrate:mainfrom
aramase:aramase/f/exchange_oidc_discovery

Conversation

@aramase

@aramase Anish Ramasekar (aramase) commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

Part of #3095. The objectstore-plugin exchange read actor JWT keys from a file, a stopgap from before ate-idp-server. #3358 needs an issuer instead, so its pkg/ API takes no key file (thread).

  • The exchange fetches keys through <issuer>/.well-known/openid-configuration. --actor-jwt-jwks-file, oidcjwt.FileKeySource and the actor-id-jwt-jwks volume are removed.
  • The sidecar verifies the issuer's certificate against the system roots plus --actor-jwt-issuer-ca-file, which is new and defaults to the service DNS trust bundle. The default ate-idp-server issuer verifies with or without an explicit :443, and a custom issuer can use a public certificate. ate-setup writes no CA setting for it. The sidecar re-reads the bundle when it changes and keeps the last good pool if a re-read fails. ate-api-server's JWTProvider CA files use the same reloading client and still replace the system roots there.
  • Nothing reads the pool Secret's jwks key now, so its writers are removed: the admin commands of kubectl-ate, ate-setup, PublicJWKS and ConcretePool.JWKS. Existing Secrets keep the stale key.
  • The sidecar fetches keys once at start. It exits on a certificate error and only warns on other errors, so an unreachable issuer fails exchanges with UNAVAILABLE until a fetch succeeds. Cached keys refresh after 5m, a failed refresh retries after 1m, and a token with an unknown key ID triggers at most one fetch per 1m. Until a fetch completes, including one the caller canceled, such a token gets UNAVAILABLE, so a newly rotated key ID is retried.

Upgrade: drop --actor-jwt-jwks-file from custom sidecar configs. Set --actor-jwt-issuer-ca-file only if the issuer uses a private CA other than the service DNS one.

Tests pass for oidcjwt, credbundle, objectstoreplugin, cmd/objectstore-plugin, cmd/ate-setup and apiauthn. The snapshotcredentials e2e needed no change. It compiles with -tags=e2e but has not run, as it needs GCS.

  • Tests pass
  • Docs updated

@aramase

Copy link
Copy Markdown
Collaborator Author

cc ILLUM1N0X

@aramase
Anish Ramasekar (aramase) marked this pull request as ready for review October 29, 2026 19:39

@ILLUM1N0X ILLUM1N0X left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

admin.go:266 and cmd/kubectl-ate/README.md:335 still describe a verifier reading jwks. Could those change in this PR?

Comment thread internal/oidcjwt/oidcjwt.go Resolved
Comment thread cmd/ate-setup/internal/steps/apienv.go Resolved
@aramase
Anish Ramasekar (aramase) force-pushed the aramase/f/exchange_oidc_discovery branch from 3d87410 to f1640f9 Compare October 30, 2026 00:58
@aramase

Copy link
Copy Markdown
Collaborator Author

ILLUM1N0X on admin.go:266 and the kubectl-ate README: fixed in f1640f9. Since nothing reads jwks anymore, I removed the writers instead of rewording them: the admin commands and ate-setup only write pool, and PublicJWKS and ConcretePool.JWKS() are gone. The README is now a pointer, so the text was in docs/reference/kubectl-ate.md, which now describes only pool. Existing Secrets keep their stale jwks key, which nothing reads.

@aramase

Copy link
Copy Markdown
Collaborator Author

ILLUM1N0X Max Thompson (@thompsonmax) the fixes for your comments are in. Could you take another look? Benjamin Elder (@BenTheElder) Dmitry Berkovich (@dberkov) #3358 is on M3 and waits on this one, so could this get M3 too?

@ILLUM1N0X

Copy link
Copy Markdown
Collaborator

Looks good. PR desc needs updating:

  • "The sidecar reads the file once at start." It now re-reads the file when it changes.
  • "Keys are fetched on the first exchange, not at sidecar start." The sidecar now also fetches once at start, and exits on a certificate error.
  • "ate-setup still writes it; removing it is a follow-up." This PR removes it.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a couple non-blocking 🤖 comments, otherwise LGTM.

Comment thread cmd/objectstore-plugin/exchange.go Resolved
Comment thread internal/oidcjwt/http.go Resolved
Comment thread internal/oidcjwt/oidcjwt.go Resolved
Comment thread cmd/objectstore-plugin/exchange.go Outdated
defer cancel()
err := verifier.Refresh(ctx, time.Now())
if certErr := (*tls.CertificateVerificationError)(nil); errors.As(err, &certErr) {
return fmt.Errorf("actor JWT issuer serves a certificate that does not verify against --actor-jwt-issuer-ca-file, or the system roots when it is empty: %w", err)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has this path run on GCS at all? On kind (S3) the exchange answers UNIMPLEMENTED before we get here. With this change, if ate-idp-server's cert doesn't verify against the servicedns trust bundle, ate-api-server won't start on any GCS install. Can we run snapshotcredentials on GCS once before merging?

I assume you don't have access, so ILLUM1N0X can you check this?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it has not run on GCS. I don't have access, and on kind the exchange answers UNIMPLEMENTED before this path. The start check exits the sidecar only on a certificate that does not verify and warns on any other error, and after b258896 the default issuer is checked against the system roots plus the servicedns trust bundle. Thanks for pinging ILLUM1N0X to run snapshotcredentials on GCS.

@ILLUM1N0X ILLUM1N0X Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran the e2e suite on our GKE cluster with this PR's build. ate-api-server started normally, the sidecar fetched the discovery document and keys from ate-idp-server with no certificate errors, and snapshotcredentials passed.

Comment thread cmd/ate-setup/internal/steps/apienv.go Resolved
Comment thread internal/oidcjwt/http.go Resolved
Comment thread internal/oidcjwt/oidcjwt.go Resolved
@aramase

Copy link
Copy Markdown
Collaborator Author

Max Thompson (@thompsonmax) ILLUM1N0X could you take another look at b258896? Since your approvals, the unknown-kid fix landed in 6d4b9eb and the CA trust change (system roots plus the servicedns bundle) in b258896, so the earlier approval no longer counts. Benjamin Elder (@BenTheElder) I can't run snapshotcredentials on GCS myself. If one of you can run it once before the tag, that covers your last ask.

@zlammerts-svg

Copy link
Copy Markdown
Collaborator

Hey Anish, could you please prioritize merging this PR as well, thanks!

…e actor JWT pool Secret

Signed-off-by: Anish Ramasekar <[email protected]>
…lable

A fetch for an unknown key ID starts a one-minute backoff even when the
caller cancels it. Within that backoff, Verify returned a plain "unknown
key ID", which the exchange maps to UNAUTHENTICATED, so a valid token with
a newly rotated key ID failed without a retry. Until a fetch completes,
wrap ErrSigningKeysUnavailable instead.

Signed-off-by: Anish Ramasekar <[email protected]>
@aramase
Anish Ramasekar (aramase) force-pushed the aramase/f/exchange_oidc_discovery branch from b258896 to 039f726 Compare November 2, 2026 19:13
@aramase

Copy link
Copy Markdown
Collaborator Author

Max Thompson (@thompsonmax) ILLUM1N0X I rebased this onto main to clear the conflict in docs/guides/authentication.md. Compared with b258896, the only change is one sentence in that file, and the code is the same. Could you take another look so this can land?

@BenTheElder

Copy link
Copy Markdown
Collaborator

Anish Ramasekar (@aramase) for some reason DCO isn't responding here

@aramase
Anish Ramasekar (aramase) force-pushed the aramase/f/exchange_oidc_discovery branch from 039f726 to a012364 Compare November 2, 2026 23:41
…le for the issuer

The sidecar verified the actor JWT issuer with a CA file that ate-setup
picked by an exact match on the default issuer, so an equivalent spelling
such as https://idp.<ns>.svc:443 fell back to the system roots and stopped
the sidecar. Verify the issuer against the system roots plus
--actor-jwt-issuer-ca-file, which defaults to the service DNS trust bundle,
and drop the ATE_API_ACTOR_JWT_ISSUER_CA_FILE ConfigMap key.

Signed-off-by: Anish Ramasekar <[email protected]>
@aramase
Anish Ramasekar (aramase) force-pushed the aramase/f/exchange_oidc_discovery branch from a012364 to 6822b56 Compare November 2, 2026 23:44
@aramase

Copy link
Copy Markdown
Collaborator Author

Anish Ramasekar (Anish Ramasekar (@aramase)) for some reason DCO isn't responding here

Benjamin Elder (@BenTheElder) Looks like the DCO app missed the event for my last force-push; I re-pushed the same tree with a re-signed top commit to retrigger it.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just left a few non-blocking comments. Recommend addressing those in a follow-up.

Comment thread internal/oidcjwt/http.go
if err != nil {
return nil, err
}
return newHTTPClient(transport), nil

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Non-blocking, security hardening: this client only fetches from ate-idp-server, which serves both documents directly, so it never needs to follow a redirect. Today it uses Go's default policy, which follows up to 10 redirects to any host, including from https:// to http://, after which the key set arrives without TLS. A custom issuer's Ingress or proxy could send one. Could this client refuse redirects?

client := newHTTPClient(transport)
client.CheckRedirect = func(*http.Request, []*http.Request) error {
	return http.ErrUseLastResponse
}
return client, nil

fetchJSON then rejects the 3xx as non-200, so a redirect fails closed with the status code in the error. Doing it here leaves apiauthn's providers on NewHTTPClient unchanged.
Separately, discoverKeysForIssuer fetches jwks_uri without checking its scheme, so an http:// value has the same effect. Requiring https there is safe for every caller. Both are fine as a follow-up.

ctx, cancel := context.WithTimeout(ctx, issuerCheckTimeout)
defer cancel()
err := verifier.Refresh(ctx, time.Now())
if certErr := (*tls.CertificateVerificationError)(nil); errors.As(err, &certErr) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Non-blocking nit, reliability improvement: a discovery document that names another issuer won't fix itself, since the issuer keeps serving the same document. But it only warns here, and then every exchange returns UNAVAILABLE and retries once a minute while the pod looks healthy. Like the certificate case above, could it fail the start? A sentinel from discoverKeysForIssuer would let this check tell it apart:

// ErrIssuerMismatch is wrapped when the discovery document names another issuer.
var ErrIssuerMismatch = errors.New("discovered issuer does not match")

// in discoverKeysForIssuer:
return nil, fmt.Errorf("%w: discovered %q, expected %q", ErrIssuerMismatch, oidcConfig.Issuer, issuer)

and here, before the warning:

if errors.Is(err, oidcjwt.ErrIssuerMismatch) {
	return fmt.Errorf("actor JWT issuer %s serves discovery for another issuer; it must match ate-idp-server's --issuer: %w", issuer, err)
}

ate-setup writes all three issuer flags from one value, so this only catches a hand-edited config, such as :443 on one side only. Recommend as a follow-up.

// issuer's keys already fetched by checkIssuerCertificate. The verifier
// trusts the system roots and caFile, re-read when it changes.
func newActorVerifier(ctx context.Context, issuer, caFile string) (*oidcjwt.Verifier, error) {
client, err := oidcjwt.NewHTTPClientWithRoots(credbundle.SystemPoolLoader(caFile))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Non-blocking, hardening: for a .svc issuer, could this trust only the service DNS bundle?

Only our service DNS signer can legitimately issue a certificate for idp.<ns>.svc, since public CAs can't issue internal names. The system roots add only CAs that should never vouch for it, including any root an operator adds, such as a TLS-inspecting proxy's CA (this transport honors HTTPS_PROXY). A certificate from any of them, plus a redirected connection, lets someone serve their own key set, forge worker-for-actor JWTs, and get storage credentials for any actor. Trusting only the bundle makes that fail closed. It breaks nothing: before b258896 the default issuer already trusted only the bundle, and no public certificate can exist for a .svc name.

Choosing by the parsed host keeps the :443 fix:

u, err := url.Parse(issuer)
if err != nil {
	return nil, fmt.Errorf("invalid --actor-jwt-issuer %q: %w", issuer, err)
}
loadRoots := credbundle.SystemPoolLoader(caFile)
if host := strings.ToLower(strings.TrimSuffix(u.Hostname(), ".")); strings.HasSuffix(host, ".svc") {
	loadRoots = credbundle.PoolLoader(caFile)
}

@thockin

Copy link
Copy Markdown
Collaborator

I am unqualified to review the security aspect of it but I approve M3 merge based on other reviewers

@thockin
Tim Hockin (thockin) added this pull request to the merge queue Nov 3, 2026
Merged via the queue into agent-substrate:main with commit 92040fe Nov 3, 2026
7 checks passed
@aramase
Anish Ramasekar (aramase) deleted the aramase/f/exchange_oidc_discovery branch November 3, 2026 03:51
Eitan Yarmush (EItanya) pushed a commit to kagent-dev/substrate that referenced this pull request Nov 3, 2026
…gent-substrate#3447)

Part of agent-substrate#3095. The objectstore-plugin exchange read actor JWT keys from
a file, a stopgap from before ate-idp-server. agent-substrate#3358 needs an issuer
instead, so its `pkg/` API takes no key file
([thread](agent-substrate#3358 (comment))).

- The exchange fetches keys through
`<issuer>/.well-known/openid-configuration`. `--actor-jwt-jwks-file`,
`oidcjwt.FileKeySource` and the `actor-id-jwt-jwks` volume are removed.
- The sidecar verifies the issuer's certificate against the system roots
plus `--actor-jwt-issuer-ca-file`, which is new and defaults to the
service DNS trust bundle. The default ate-idp-server issuer verifies
with or without an explicit `:443`, and a custom issuer can use a public
certificate. ate-setup writes no CA setting for it. The sidecar re-reads
the bundle when it changes and keeps the last good pool if a re-read
fails. ate-api-server's `JWTProvider` CA files use the same reloading
client and still replace the system roots there.
- Nothing reads the pool Secret's `jwks` key now, so its writers are
removed: the `admin` commands of kubectl-ate, ate-setup, `PublicJWKS`
and `ConcretePool.JWKS`. Existing Secrets keep the stale key.
- The sidecar fetches keys once at start. It exits on a certificate
error and only warns on other errors, so an unreachable issuer fails
exchanges with `UNAVAILABLE` until a fetch succeeds. Cached keys refresh
after 5m, a failed refresh retries after 1m, and a token with an unknown
key ID triggers at most one fetch per 1m. Until a fetch completes,
including one the caller canceled, such a token gets `UNAVAILABLE`, so a
newly rotated key ID is retried.

Upgrade: drop `--actor-jwt-jwks-file` from custom sidecar configs. Set
`--actor-jwt-issuer-ca-file` only if the issuer uses a private CA other
than the service DNS one.

Tests pass for `oidcjwt`, `credbundle`, `objectstoreplugin`,
`cmd/objectstore-plugin`, `cmd/ate-setup` and `apiauthn`. The
snapshotcredentials e2e needed no change. It compiles with `-tags=e2e`
but has not run, as it needs GCS.

- [x] Tests pass
- [x] Docs updated

---------

Signed-off-by: Anish Ramasekar <[email protected]>
(cherry picked from commit 92040fe)
Signed-off-by: Eitan Yarmush <[email protected]>
Dmitry Berkovich (dberkov) pushed a commit that referenced this pull request Nov 3, 2026
Bring `release-1.0` up to `main` at
5a5b192 with the 19 changes merged
since the previous batch in #3536. The final tree matches that commit
exactly.

**Please use Rebase and merge to preserve the individual commits and
author attribution.** All 19 applied cleanly in upstream order. Original
authors, author dates, co-author and sign-off trailers, and `-x` source
references are retained.

Upgrade notes:

- #3533: old gVisor MEMORY snapshots, including goldens, no longer
restore. Recreate them after upgrading. MicroVM MEMORY snapshots still
resume with their old mounts until recreated. `/tmp` is now always
tmpfs; ROOTFS snapshots no longer preserve its contents, and
image-provided files there are hidden. MicroVM `/tmp` counts against
actor memory and is capped at half the guest's memory. gVisor `/dev/shm`
is now a 64 MiB tmpfs. Mount a `durableDir` at either path when
persistence or more space is needed. `/run` now comes from the image
rootfs.
- #3447: remove `--actor-jwt-jwks-file` from custom credential-exchange
sidecar configs. Set `--actor-jwt-issuer-ca-file` only when the issuer
uses a private CA other than the service DNS CA.

- #3450: do not reuse a deleted atespace name. See the updated atespace
name-reuse guidance.

Includes #3505, which overlaps the open backport in #3547. GKE
provisioning and teardown now live in
[substrate-gke](https://github.com/ai-on-gke/substrate-gke) rather than
this repository.

<details>
<summary>Included PRs, in cherry-pick order</summary>

1. #3500: demos/{sandbox,jupyter,multi-template}: fix tutorial
prerequisites, walkthroughs, and counter volume
2. #3498: demos/counter: inline gVisor walkthrough and fix setup
commands
3. #3511: e2e tests: Extend the existing tests to cover tag actor.
4. #2775: Generate API reference documentation and publish it on
Substrate website
5. #3119: Document how people, pods and CI jobs should authenticate to
ate-api
6. #3505: tools: remove setup-gcp and hack/teardown.sh
7. #3147: e2e: resume fidelity after SandboxConfig version edits
8. #3370: ate-setup: reconcile ate-api-server ConfigMaps and roll out on
config changes
9. #1607: hack: reject breaking proto changes across each proto package
10. #3415: docs: update and add security diagrams
11. #3485: Resuming a microVM actor on the same node no longer reads its
rootfs upper back from disk
12. #3447: Verify actor JWTs in the credential exchange through OIDC
discovery
13. #3533: ocispec: mount tmpfs at /dev/shm, and /tmp on both runtimes
14. #3450: docs: Document that atespace names must not be reused
15. #3410: Document using more than one egress credential provider
16. #3486: atelet: register the device plugin on a new socket each time
17. #3553: ate-setup: wait for the demo namespace in delete demo
18. #3560: Fix multiple issues in claude-code-multiplex demo
19. #3517: docs: remove inaccurate early-stage architecture
documentation

</details>

Validation: every intermediate tree matches its upstream source commit,
all 19 authorship records and original attribution trailers are
preserved, and `git diff --check` passes. The original 18 backport
commits are unchanged.

AI use: AI-assisted cherry-picking, attribution checks, and review.

- [ ] Tests pass
- [x] Docs updated

---------

Signed-off-by: Mofi Rahman <[email protected]>
Signed-off-by: Eitan Yarmush <[email protected]>
Signed-off-by: Nayana Bidari <[email protected]>
Signed-off-by: Julian Gutierrez Oschmann <[email protected]>
Signed-off-by: Max Thompson <[email protected]>
Signed-off-by: Steven Shriver <[email protected]>
Signed-off-by: Grant McCloskey <[email protected]>
Signed-off-by: Shrutiya <[email protected]>
Signed-off-by: Haiyan Meng <[email protected]>
Signed-off-by: Greg Castle <[email protected]>
Signed-off-by: Benjamin Elder <[email protected]>
Signed-off-by: Anish Ramasekar <[email protected]>
Signed-off-by: Shruti Nair <[email protected]>
Signed-off-by: Haven Xia <[email protected]>
Co-authored-by: Mofi Rahman <[email protected]>
Co-authored-by: nybidari <[email protected]>
Co-authored-by: Julian Gutierrez Oschmann <[email protected]>
Co-authored-by: Max Thompson <[email protected]>
Co-authored-by: Grant McCloskey <[email protected]>
Co-authored-by: Steven Shriver <[email protected]>
Co-authored-by: shrutiyam-glitch <[email protected]>
Co-authored-by: haiyanmeng <[email protected]>
Co-authored-by: Greg Castle <[email protected]>
Co-authored-by: Benjamin Elder <[email protected]>
Co-authored-by: Anish Ramasekar <[email protected]>
Co-authored-by: Shruti Nair <[email protected]>
Co-authored-by: Haven Xia <[email protected]>
Co-authored-by: Adam Wathieu <[email protected]>
Dmitry Berkovich (dberkov) pushed a commit that referenced this pull request Nov 3, 2026
…3447)

Part of #3095. The objectstore-plugin exchange read actor JWT keys from
a file, a stopgap from before ate-idp-server. #3358 needs an issuer
instead, so its `pkg/` API takes no key file
([thread](#3358 (comment))).

- The exchange fetches keys through
`<issuer>/.well-known/openid-configuration`. `--actor-jwt-jwks-file`,
`oidcjwt.FileKeySource` and the `actor-id-jwt-jwks` volume are removed.
- The sidecar verifies the issuer's certificate against the system roots
plus `--actor-jwt-issuer-ca-file`, which is new and defaults to the
service DNS trust bundle. The default ate-idp-server issuer verifies
with or without an explicit `:443`, and a custom issuer can use a public
certificate. ate-setup writes no CA setting for it. The sidecar re-reads
the bundle when it changes and keeps the last good pool if a re-read
fails. ate-api-server's `JWTProvider` CA files use the same reloading
client and still replace the system roots there.
- Nothing reads the pool Secret's `jwks` key now, so its writers are
removed: the `admin` commands of kubectl-ate, ate-setup, `PublicJWKS`
and `ConcretePool.JWKS`. Existing Secrets keep the stale key.
- The sidecar fetches keys once at start. It exits on a certificate
error and only warns on other errors, so an unreachable issuer fails
exchanges with `UNAVAILABLE` until a fetch succeeds. Cached keys refresh
after 5m, a failed refresh retries after 1m, and a token with an unknown
key ID triggers at most one fetch per 1m. Until a fetch completes,
including one the caller canceled, such a token gets `UNAVAILABLE`, so a
newly rotated key ID is retried.

Upgrade: drop `--actor-jwt-jwks-file` from custom sidecar configs. Set
`--actor-jwt-issuer-ca-file` only if the issuer uses a private CA other
than the service DNS one.

Tests pass for `oidcjwt`, `credbundle`, `objectstoreplugin`,
`cmd/objectstore-plugin`, `cmd/ate-setup` and `apiauthn`. The
snapshotcredentials e2e needed no change. It compiles with `-tags=e2e`
but has not run, as it needs GCS.

- [x] Tests pass
- [x] Docs updated

---------

Signed-off-by: Anish Ramasekar <[email protected]>
(cherry picked from commit 92040fe)
Signed-off-by: Eitan Yarmush <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants