Skip to content

Add service health endpoints and harden gRPC/HTTP client robustness#1398

Merged
fidencio merged 5 commits into
confidential-containers:mainfrom
fidencio:topic/grpc-health-and-robustness
Jun 2, 2026
Merged

Add service health endpoints and harden gRPC/HTTP client robustness#1398
fidencio merged 5 commits into
confidential-containers:mainfrom
fidencio:topic/grpc-health-and-robustness

Conversation

@fidencio

@fidencio fidencio commented Jun 1, 2026

Copy link
Copy Markdown
Member

This series makes the Trustee services more observable and more resilient
when run as long-lived deployments behind an orchestrator.

It adds standard health endpoints so liveness/readiness probes can be wired
up, and fixes a handful of robustness issues in the inter-service gRPC and
the outbound HTTP clients that can otherwise cause requests to hang
indefinitely.

These changes are independent of any particular deployment method and are
sent ahead of the Helm chart work so they can be reviewed on their own.

  1. serviceability: add health and readiness endpoints to AS, RVPS and KBS
    The attestation-service and rvps gRPC servers now register the standard
    gRPC Health service (via tonic-health) and report SERVING once ready,
    and KBS gains a lightweight GET /healthz HTTP route. This lets external
    orchestrators detect a wedged or still-initialising service and restart it.

  2. attestation-service: query RVPS from the runtime that owns the pool
    The reference-value lookup ran on a helper thread that spun up its own
    current-thread runtime, while the mobc pool and its gRPC channels live on
    the main multi-threaded runtime. Driving pool.get() from an unrelated
    runtime delivered wakeups to the wrong reactor and could hang the request
    forever. The query now runs on the handle of the runtime that owns the pool.

  3. attestation-service: recycle stale RVPS gRPC connections
    Long-lived pooled connections to RVPS could be silently torn down, making
    later queries hang or fail. The channel now uses HTTP/2 keep-alive and a
    capped idle lifetime, and each pooled connection is validated on checkout
    with a short, time-bounded health query before use.

  4. kbs: bound the lifetime of requests on the AS gRPC channel
    The KBS→AS channel had no timeout. Evidence verification can be slow
    (notably Intel TDX DCAP), but with no upper bound a wedged connection would
    block a request indefinitely. A generous 300s timeout covers the slow path
    while ensuring stuck requests fail cleanly.

  5. verifier: bound NVIDIA NRAS HTTP requests with timeouts
    The NVIDIA verifier's attestation POST and JWKS fetch used a default
    reqwest client with no timeout, so an unreachable NRAS could hang the
    verifier. Explicit timeouts are now set (60s attestation, 30s JWKS).

@fidencio fidencio requested review from Xynnn007 and fitzthum June 1, 2026 21:57
@fidencio fidencio requested a review from a team as a code owner June 1, 2026 21:57
@fitzthum

fitzthum commented Jun 1, 2026

Copy link
Copy Markdown
Member

cc @MatiasVara

@Xynnn007 Xynnn007 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @fidencio ! Nice code and nice commit messages. I only have some nits for the timeout consts. Overall LGTM

Comment thread kbs/src/attestation/coco/grpc.rs Outdated
Comment thread attestation-service/src/rvps/grpc.rs Outdated
Comment thread attestation-service/src/rvps/grpc.rs Outdated
Comment thread attestation-service/src/rvps/grpc.rs Outdated
Comment thread attestation-service/src/rvps/grpc.rs Outdated
Comment thread deps/verifier/src/nvidia/mod.rs Outdated
Comment thread deps/verifier/src/nvidia/nras_jwks.rs Outdated
@fidencio fidencio force-pushed the topic/grpc-health-and-robustness branch from 3c36050 to b2f7aaf Compare June 2, 2026 05:07

@Xynnn007 Xynnn007 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thanks @fidencio

@mythi

mythi commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Evidence verification can be slow (notably Intel TDX DCAP)

Not related to this PR but I noticed some time ago that Trustee's default config does not cache the collateral at all. PCS stopped sending Cache-Control headers which enabled caching so to force caching is to add some "verify_collateral_cache_expire_hours": <value, e.g., 168>, or get my PR merged which unconditionally sets cache and drops QCNL.

@fidencio

fidencio commented Jun 2, 2026

Copy link
Copy Markdown
Member Author

Not related to this PR but I noticed some time ago that Trustee's default config does not cache the collateral at all. PCS stopped sending Cache-Control headers which enabled caching so to force caching is to add some "verify_collateral_cache_expire_hours": <value, e.g., 168>, or get my PR merged which unconditionally sets cache and drops QCNL.

Is it okay if we land this one first, @mythi?

This is basically the set of commits that I needed in order to get the helm chart working / passing Kata Containers CI.
Once this lands, and we do the helm chart switch there, things will be less tricky to test end-to-end.

@mythi

mythi commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

I don't mind this goes first, the PRs are not coupled. Just a comment that if you plan to make some changes to sgx_default_qcnl.conf files, they become obsolete after my PR gets merged.

Comment thread attestation-service/src/rvps/grpc.rs Outdated
match tokio::time::timeout(RVPS_HEALTH_CHECK_TIMEOUT, c.query_reference_value(req)).await {
Ok(Ok(_)) => Ok(c),
Ok(Err(e)) => {
warn!("RVPS connection health check failed, reconnecting: {e}");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we don't attempt a reconnect here, so I would move the log entry to the place where we perform retries. see also the Err arm

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@mkulke, please, give a second pass.

fidencio added 5 commits June 2, 2026 12:28
Until now none of the Trustee services exposed a way for an external
orchestrator to tell whether they were actually up and serving traffic.
This makes it impossible for an environment such as Kubernetes to drive
liveness and readiness probes against them, which in turn means a wedged
or still-initialising service cannot be detected and restarted.

Wire up the standard gRPC Health service (via tonic-health) into both
the attestation-service and the rvps gRPC servers so that they report
SERVING once they are ready to answer requests, and add a small GET
/healthz route to the KBS HTTP API that returns 200 OK while the server
is alive. The tonic-health crate is added as a workspace dependency and
enabled through the attestation-service grpc-bin feature and the rvps
crate.

Signed-off-by: Fabiano Fidêncio <ffidencio@nvidia.com>
Assisted-by: Cursor <cursoragent@cursor.com>
The reference-value lookup performed during token generation runs on a
dedicated helper thread, and that thread used to spin up its own
current-thread tokio runtime to drive the asynchronous RVPS query. The
mobc connection pool and the tonic gRPC channels it manages are created
on the main, multi-threaded runtime, so executing pool.get() and the
channel I/O from an unrelated runtime delivered their futures and
wakeups to a reactor that never owned the underlying resources. The
practical effect was that pool.get() could block forever and the whole
attestation request would hang.

Capture the handle of the currently running runtime before the helper
thread is spawned and use it to block_on() the query, keeping the pool
and its connections inside the runtime context where they were created.

Signed-off-by: Fabiano Fidêncio <ffidencio@nvidia.com>
Assisted-by: Cursor <cursoragent@cursor.com>
Connections handed out by the RVPS connection pool are long lived, and
in practice they can be torn down underneath us by the peer or by an
intermediary without the client noticing. When that happens the next
query made over such a connection either hangs or fails, even though a
healthy RVPS is sitting on the other end.

Make the pooled gRPC client defensive about this. The channel now
enables HTTP/2 keep-alive and caps the idle lifetime of a connection so
that dead connections are surfaced and dropped, and every connection
handed out by the pool is first validated with a short, time-bounded
health query (an unknown key, for which RVPS simply returns Ok(None)).
If that probe does not succeed quickly the connection is discarded and a
fresh one is established instead of risking a stale one.

Signed-off-by: Fabiano Fidêncio <ffidencio@nvidia.com>
Assisted-by: Cursor <cursoragent@cursor.com>
The channel KBS uses to talk to the attestation-service had no timeout
configured. Verifying TEE evidence can legitimately take a long time,
most notably for Intel TDX where DCAP has to fetch collateral over the
network, so the channel must tolerate slow responses; but with no upper
bound at all a connection that has silently wedged would keep a request
blocked indefinitely with no way to recover.

Give the channel a generous 300 second timeout, which comfortably covers
even the slow DCAP path while ensuring a genuinely stuck request
eventually fails and returns an error instead of hanging forever.

Signed-off-by: Fabiano Fidêncio <ffidencio@nvidia.com>
Assisted-by: Cursor <cursoragent@cursor.com>
The NVIDIA verifier talks to the NVIDIA Remote Attestation Service over
HTTP both to submit evidence for attestation and to fetch the JWKS used
to validate the response. Both requests were issued with a default
reqwest client that has no timeout, so a slow or unreachable NRAS
endpoint could leave the verifier blocked indefinitely with no way out.

Build explicit reqwest clients with request timeouts for these calls: 60
seconds for the attestation request and 30 seconds for the JWKS fetch,
so a misbehaving endpoint results in a clean error rather than a hang.

Signed-off-by: Fabiano Fidêncio <ffidencio@nvidia.com>
Assisted-by: Cursor <cursoragent@cursor.com>
@fidencio fidencio force-pushed the topic/grpc-health-and-robustness branch from b2f7aaf to 76bcab9 Compare June 2, 2026 10:38
@fidencio fidencio merged commit d9b9542 into confidential-containers:main Jun 2, 2026
64 of 65 checks passed
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.

5 participants