grpc / grpc/grpc-java

xDS: identity cert never refreshes when the CA root provider is a separate `file_watcher` instance

Open
#13,058 0 comments 0 reactions 0 assignees View on GitHub
bug
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.