envoyproxy / envoyproxy/envoy

SDS: a new secret provider for an already-subscribed secret never receives it, times out, and the listener goes active without a certificate

Open
#47,309 0 comments 1 reaction 0 assignees View on GitHub
Dominant language
C++
Stars
28.9k
Forks
5.6k
Avg merge
1d 20h
Merged PRs (30d)
437

Description

*Description*:

Envoy keeps one SDS provider per secret, keyed by the secret name plus the bytes of its `ConfigSource`. If a listener update changes those bytes (for example the control plane adds or removes `initial_fetch_timeout`), Envoy creates a new provider for a secret it already has. The same happens when a warming provider is created for a name that a non-warming (on-demand prefetch) provider already holds.

The new provider never gets the secret. The old provider still holds the watch on the same name in the shared ADS mux, so the new watch is folded into the existing interest set. With delta or unified xDS no request goes out at all (`WatchMap::findAdditions`, `watch_map.cc:313`). With legacy SotW, `GrpcMuxImpl::addWatch` does queue a request, but it carries the same resource names and version as before, so the server has nothing new to respond to. Either way, after `initial_fetch_timeout` (15s by default) the provider gives up, `SdsApi::onConfigUpdateFailed` marks the listener ready anyway (`sds_api.cc:183`), the new listener goes active with no certificate, and the old listener that had a working certificate is drained. All TLS handshakes fail. Readiness stays green. It only recovers if the server happens to push that exact secret again, or the proxy restarts. Destroying the old listener does not help: its watch is removed but the name stays subscribed through the new watch, so nothing is re-requested. With `initial_fetch_timeout: 0s` the new listener instead warms forever.

The proxy already holds the secret in memory. It should reuse it, not fail open on a fetch it never sent.

EDS had the same problem and fixed it with `EdsResourcesCache`: on `FetchTimedout`, `source/extensions/clusters/eds/eds.cc:452-479` falls back to the cached resource. SDS has no equivalent. Note the EDS fallback only runs on timeout, so a straight copy would not cover `initial_fetch_timeout: 0s`. For SDS the secret should be reused when the new provider is created, either from a per-name cache or from the live provider that still holds the watch (the secret manager already tracks every live provider). Happy to work on it if there is agreement on the direction.

*Repro steps*:

Reported and reproduced with release binaries in https://github.com/envoyproxy/gateway/issues/9918 (an Envoy Gateway 1.9.0 to 1.9.1 upgrade removed `initial_fetch_timeout: 0s` from the SDS `ConfigSource`; every warm proxy lost TLS). https://github.com/envoyproxy/gateway/issues/9519 has the same end state.

Minimal shape:

1. Run Envoy with ADS (delta, unified, or SotW) and a TLS listener whose secret comes from SDS over ADS. Let it warm.
2. Push the same listener with one byte changed inside `sds_config`, and no change to the secret.
3. No Secret request goes out (delta/unified), or an identical one is re-sent (SotW). After 15s:

```
gRPC config: initial fetch timed out for type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret
```

4. `total_listeners_warming` is 0, `sds..init_fetch_timeout` is 1, `downstream_context_secrets_not_ready` climbs, `/ready` returns 200, TLS handshakes fail.
5. Rotate that one secret so the server pushes it: that listener recovers, nothing else does.

*Config*:

Any listener with `tls_certificate_sds_secret_configs[].sds_config` set to `{ads: {}}`. Verified on `main` and v1.38.1.

Contributor guide

Open the contributing guide

Research direction

Start with SdsApi::onConfigUpdateFailed in sds_api.cc and compare the timeout fallback in source/extensions/clusters/eds/eds.cc:452-479. Reproduce the ConfigSource-change scenario with ADS, including initial_fetch_timeout: 0s, and trace how the secret manager and shared ADS mux handle the old and new providers. Done means a new provider reuses the already-held secret, avoids failing open or warming forever, and preserves TLS readiness and handshakes.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, grpc
Domain
infrastructure, networking, security
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.