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

[Nicolas-EM]

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:

{
  "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:

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

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).
 *
 * <p>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.
   *
   * <p>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<String, Bootstrapper.CertificateProviderInfo> 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.<Bootstrapper.ServerInfo>of())
        .node(EnvoyProtoData.Node.newBuilder().build())
        .certProviders(certProviders)
        .build();
  }

  private static Map<String, String> fileWatcherConfig(Path cert, Path key, Path ca) {
    Map<String, String> 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);
  }
}