xDS: identity cert never refreshes when the CA root provider is a separate `file_watcher` instance
- Dominant language
- Java
- Stars
- 12.1k
- Forks
- 4k
- Avg merge
- 2d 17h
- Merged PRs (30d)
- 37
Description
### What version of gRPC-Java are you using?
1.82.4 - also present on `master`
### What is your environment?
- JDK: 17+ (any)
- OS: Linux
### What did you expect to see?
When the identity cert file on disk rotates, the client's `SslContext` is rebuilt with the new cert, and new connections succeed.
### What did you see instead?
After the identity cert's on-disk file rotates once, all subsequent rotations are silently ignored. New connections keep using the original (now expired) identity cert and fail the TLS handshake with an expired-certificate error, even though the correct new cert is present on disk and the `file_watcher` provider is actively polling it without errors.
### Steps to reproduce the bug
- Uses gRPC xDS (`XdsChannelCredentials`) with two separate `file_watcher` certificate provider instances in the bootstrap file:
- one for the client identity cert + key (short refresh interval, e.g. `30s`)
- one for the CA trust bundle (separate instance name), which never changes on disk.
Example `certificate_providers` bootstrap config:
```json
{
"certificate_providers": {
"identity_cert_provider": {
"plugin_name": "file_watcher",
"config": {
"certificate_file": "/etc/certs/client/cert.pem",
"private_key_file": "/etc/certs/client/key.pem",
"refresh_interval": "30s"
}
},
"ca_provider": {
"plugin_name": "file_watcher",
"config": {
"ca_certificate_file": "/etc/certs/ca.pem",
"refresh_interval": "600s"
}
}
}
}
```
The CDS `CommonTlsContext` references these as two different `CertificateProviderInstance` names (`tls_certificate_certificate_provider_instance` = `identity_cert_provider`, `validation_context.ca_certificate_provider_instance` = `ca_provider`). This is not the "system root certs" case — an explicit CA provider instance is configured.
### Root cause
In `CertProviderSslContextProvider`:
```java
private void updateSslContextWhenReady() {
if (isMtls()) {
if (savedKey != null && (savedTrustedRoots != null || savedSpiffeTrustMap != null)) {
updateSslContext();
clearKeysAndCerts();
}
} else if (isRegularTlsAndClientSide()) {
...
}
}
private void clearKeysAndCerts() {
savedKey = null;
if (!isUsingSystemRootCerts) {
savedTrustedRoots = null;
savedSpiffeTrustMap = null;
}
savedCertChain = null;
}
```
`clearKeysAndCerts()` nulls `savedTrustedRoots` after every successful `updateSslContext()` call, unless `isUsingSystemRootCerts` is true. `isUsingSystemRootCerts` is only true when no CA provider instance is configured at all (client falls back to the OS trust store). It is `false` whenever an explicit `CaCertificateProviderInstance` is configured — as in the setup above.
Sequence of events:
1. Startup: identity `file_watcher` fires `updateCertificate()` → `savedKey`/`savedCertChain` set. CA `file_watcher` fires `updateTrustedRoots()` → `savedTrustedRoots` set. The guard passes, `updateSslContext()` builds the first `SslContext`, then `clearKeysAndCerts()` nulls `savedTrustedRoots` (and `savedKey`/`savedCertChain`).
2. The CA bundle file never changes again, so the CA `file_watcher` never calls `updateTrustedRoots()` again. `savedTrustedRoots` stays `null` forever.
3. 24h later, the identity cert file rotates. The identity `file_watcher` calls `updateCertificate()` → sets `savedKey`/`savedCertChain`. `updateSslContextWhenReady()` checks `savedTrustedRoots != null` — false — so `updateSslContext()` is never called again.
4. `DynamicSslContextProvider.sslContextAndTrustManager` keeps serving the original, now-expired `SslContext` to every new connection indefinitely, via `addCallback()`.
This reproduces deterministically any time an identity cert and its CA bundle are served by two separate `file_watcher` provider instances with different refresh cadences, and the CA bundle doesn't happen to change again after the first successful build.
### Related prior fix
#12340 ("xds: SslContext updates handling when using system root certs") fixed the same underlying pattern, but only for the `isUsingSystemRootCerts == true` branch (no CA provider instance configured, using OS trust store). A reviewer on that PR flagged the general case as "tangential" and deferred it:
> A bit tangential, but in `updateSslContextWhenReady()`, are we missing a check for `isUsingSystemRootCerts` in the `isClientSideTls()` block? ... (If that needs fixing, it can be done in a separate PR)
That follow-up was never filed. This issue is that follow-up.
### Suggested fix
`clearKeysAndCerts()` should not discard `savedTrustedRoots`/`savedSpiffeTrustMap` just because a rebuild happened to be triggered by the identity-cert side. The trust roots are still valid and simply haven't received a new update from their own (independent) watcher. Options:
- Only clear `savedTrustedRoots`/`savedSpiffeTrustMap` when the *root* provider itself pushed the current values being cleared, not on every `updateSslContext()` call triggered by any provider.
- Or, track "have we received at least one root update" state instead of nulling it after each rebuild, so a rebuild triggered solely by an identity-cert update reuses the last known-good trust roots.
Happy to submit a PR with a fix and a regression test if that's useful.
### Reproduction Test
Expand
`xds/src/test/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertRotationBugReproTest.java`
```java
package io.grpc.xds.internal.security.certprovider;
import static com.google.common.truth.Truth.assertThat;
import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.CA_PEM_FILE;
import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.CLIENT_KEY_FILE;
import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.CLIENT_PEM_FILE;
import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.SERVER_1_KEY_FILE;
import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.SERVER_1_PEM_FILE;
import com.google.common.collect.ImmutableList;
import io.grpc.testing.TlsTesting;
import io.grpc.xds.EnvoyServerProtoData;
import io.grpc.xds.client.Bootstrapper;
import io.grpc.xds.client.EnvoyProtoData;
import io.grpc.xds.internal.security.CommonTlsContextTestsUtil;
import io.grpc.xds.internal.security.CommonTlsContextTestsUtil.TestCallback;
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.util.HashMap;
import java.util.Map;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
/**
* Reproduction for the bug where an mTLS client/server's identity cert stops being refreshed
* once its CA/root-of-trust {@code file_watcher} provider instance is a *separate* instance
* that stops sending updates (e.g. because the CA bundle file on disk essentially never
* changes again after the first read).
*
*
This mirrors a real xDS bootstrap where two independent {@code file_watcher} certificate
* provider instances are configured: one for the client identity cert (rotated frequently by a
* sidecar), and a separate one for the CA trust bundle (rotated rarely, if ever). Unlike the
* existing {@link CertProviderClientSslContextProviderTest}, this test uses the *real*
* {@link FileWatcherCertificateProvider} (via {@link FileWatcherCertificateProviderProvider})
* against real files on disk, polling on a real {@code refresh_interval}, instead of a mocked
* {@link CertificateProvider.Watcher}.
*/
@RunWith(JUnit4.class)
public class FileWatcherCertRotationBugReproTest {
private static final String IDENTITY_INSTANCE = "identity_cert_provider";
private static final String CA_INSTANCE = "ca_provider";
private static final String REFRESH_INTERVAL = "1s";
private CertificateProviderStore certificateProviderStore;
private CertProviderClientSslContextProviderFactory factory;
@Before
public void setUp() {
CertificateProviderRegistry certificateProviderRegistry = new CertificateProviderRegistry();
certificateProviderRegistry.register(new FileWatcherCertificateProviderProvider());
certificateProviderStore = new CertificateProviderStore(certificateProviderRegistry);
factory = new CertProviderClientSslContextProviderFactory(certificateProviderStore);
}
/**
* Demonstrates: once the CA {@code file_watcher} instance stops sending updates (because its
* file never changes again after the initial read), a *later* identity-cert rotation on the
* separate identity {@code file_watcher} instance is silently dropped. New connections keep
* getting the SslContext built from the very first (now potentially expired) identity cert.
*
*
This test currently PASSES against grpc-java {@code master}, which demonstrates the bug:
* a correct implementation would make the final assertion fail (the SslContext should have
* been rebuilt with the rotated cert).
*/
@Test
public void identityCertRotation_isSilentlyDropped_onceCaProviderStopsUpdating()
throws Exception {
Path identityCertFile = newTempFileFrom(CLIENT_PEM_FILE);
Path identityKeyFile = newTempFileFrom(CLIENT_KEY_FILE);
Path caFile = newTempFileFrom(CA_PEM_FILE);
// The file_watcher config schema always requires certificate_file/private_key_file, even
// for an instance that is only ever consulted for its CA bundle: CertProviderSslContextProvider
// wraps this instance's watcher in an IgnoreUpdatesWatcher(ignoreRootCertUpdates=false), which
// silently discards any updateCertificate() calls coming from it. Their content is therefore
// irrelevant to this test; any valid cert/key pair works.
Path caInstanceDummyCertFile = newTempFileFrom(CLIENT_PEM_FILE);
Path caInstanceDummyKeyFile = newTempFileFrom(CLIENT_KEY_FILE);
// The identity instance's own ca_certificate_file value is likewise required by the config
// schema but never consulted: CertProviderSslContextProvider wraps its watcher in an
// IgnoreUpdatesWatcher(ignoreRootCertUpdates=true), which silently discards any
// updateTrustedRoots() calls coming from it. Reusing caFile here is just a placeholder.
Bootstrapper.BootstrapInfo bootstrapInfo =
buildTwoFileWatcherInstanceBootstrap(
IDENTITY_INSTANCE, identityCertFile, identityKeyFile, caFile,
CA_INSTANCE, caInstanceDummyCertFile, caInstanceDummyKeyFile, caFile);
EnvoyServerProtoData.UpstreamTlsContext upstreamTlsContext =
CommonTlsContextTestsUtil.buildUpstreamTlsContextForCertProviderInstance(
IDENTITY_INSTANCE,
"cert-default",
CA_INSTANCE,
"root-default",
/* alpnProtocols= */ null,
/* staticCertValidationContext= */ null);
CertProviderClientSslContextProvider provider =
(CertProviderClientSslContextProvider)
factory.getProvider(
upstreamTlsContext,
bootstrapInfo.node().toEnvoyProtoNode(),
bootstrapInfo.certProviders());
// The real file_watcher pollers run on their own ScheduledExecutorService; wait for both
// to complete their first poll (scheduled with a 0s initial delay) and for the first
// SslContext to be built from the files already on disk.
awaitTrue(() -> provider.getSslContextAndTrustManager() != null, "initial SslContext build");
TestCallback firstCallback = CommonTlsContextTestsUtil.getValueThruCallback(provider);
assertThat(firstCallback.updatedSslContext).isNotNull();
// Simulate a 24h identity-cert rotation performed by a sidecar (e.g. emissary): only the
// identity cert/key files change. The CA bundle file is never touched again, exactly like a
// rarely-rotated CA bundle in production.
overwrite(identityCertFile, SERVER_1_PEM_FILE);
overwrite(identityKeyFile, SERVER_1_KEY_FILE);
// Give the real file_watcher poller (refresh_interval=1s) several chances to observe the
// change and rebuild the SslContext.
Thread.sleep(4000);
TestCallback afterRotationCallback = CommonTlsContextTestsUtil.getValueThruCallback(provider);
// BUG: the SslContext is never rebuilt. A real new connection created at this point would
// still present the *original* identity cert, even though the correct rotated cert has been
// on disk (and observed error-free by the file_watcher poller) for several refresh cycles.
assertThat(afterRotationCallback.updatedSslContext)
.isSameInstanceAs(firstCallback.updatedSslContext);
}
private static Path newTempFileFrom(String resourceName) throws IOException {
File tempFile = File.createTempFile("repro-" + resourceName.replace('/', '_'), ".pem");
tempFile.deleteOnExit();
try (java.io.InputStream in = TlsTesting.loadCert(resourceName)) {
Files.copy(in, tempFile.toPath(), StandardCopyOption.REPLACE_EXISTING);
}
return tempFile.toPath();
}
private static void overwrite(Path target, String resourceName) throws IOException {
try (java.io.InputStream in = TlsTesting.loadCert(resourceName)) {
Files.copy(in, target, StandardCopyOption.REPLACE_EXISTING);
}
}
private static Bootstrapper.BootstrapInfo buildTwoFileWatcherInstanceBootstrap(
String identityInstanceName, Path identityCert, Path identityKey, Path identityCa,
String caInstanceName, Path caInstanceCert, Path caInstanceKey, Path caInstanceCa) {
Map certProviders = new HashMap<>();
certProviders.put(
identityInstanceName,
Bootstrapper.CertificateProviderInfo.create(
"file_watcher", fileWatcherConfig(identityCert, identityKey, identityCa)));
certProviders.put(
caInstanceName,
Bootstrapper.CertificateProviderInfo.create(
"file_watcher", fileWatcherConfig(caInstanceCert, caInstanceKey, caInstanceCa)));
return Bootstrapper.BootstrapInfo.builder()
.servers(ImmutableList.of())
.node(EnvoyProtoData.Node.newBuilder().build())
.certProviders(certProviders)
.build();
}
private static Map fileWatcherConfig(Path cert, Path key, Path ca) {
Map config = new HashMap<>();
config.put("certificate_file", cert.toString());
config.put("private_key_file", key.toString());
config.put("ca_certificate_file", ca.toString());
config.put("refresh_interval", REFRESH_INTERVAL);
return config;
}
private interface BooleanSupplierWithException {
boolean get() throws Exception;
}
private static void awaitTrue(BooleanSupplierWithException condition, String what)
throws Exception {
long deadline = System.currentTimeMillis() + 5000;
while (System.currentTimeMillis() < deadline) {
if (condition.get()) {
return;
}
Thread.sleep(50);
}
throw new AssertionError("Timed out waiting for: " + what);
}
}
```
Contributor guide
Research direction
Start in xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderSslContextProvider.java, focusing on updateSslContextWhenReady() and clearKeysAndCerts(). Run xds/src/test/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertRotationBugReproTest.java first; done means a later identity certificate rotation rebuilds the SslContext while reusing the unchanged CA provider state.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- backend, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100