From 02b3b72eb4fc14037d56692be32d0fadbfa2bbc0 Mon Sep 17 00:00:00 2001
From: Timur Rakhmatullin
 <174210871+TimurRakhmatullin86@users.noreply.github.com>
Date: Sat, 5 Sep 2026 09:37:02 -0700
Subject: [PATCH] util: Fix AdvancedTlsX509KeyManager reloading credentials on
 every refresh

The UpdateResult constructor takes (success, certTime, keyTime), but
readAndUpdate constructed it as (success, newKeyTime, newCertTime), so the
returned certTime/keyTime were swapped. LoadFilePathExecution then stored each
into the oppositely named field, so on the next refresh readAndUpdate compared
the key file's mtime against the cert file's last-seen mtime and vice versa.

With two files of different mtimes (the normal case) that cross-comparison is
always true, so the scheduled reloader re-read the key and certificates and
rotated the alias on every refresh interval even when nothing changed. That
breaks the method's documented 'only update when both files changed' invariant
and continuously invalidates the per-alias key-material cache that the alias
rotation was introduced to enable.

Pass the times in the order the constructor expects. Add a test that advances a
FakeClock over several refresh intervals with the files untouched and asserts
the alias is stable; it fails before this change (the alias keeps rotating).

Signed-off-by: Timur Rakhmatullin <174210871+TimurRakhmatullin86@users.noreply.github.com>
---
 .../grpc/util/AdvancedTlsX509KeyManager.java  |  4 ++--
 .../util/AdvancedTlsX509KeyManagerTest.java   | 23 +++++++++++++++++++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/util/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java b/util/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java
index eea664f2ad4..f0bdce108d6 100644
--- a/util/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java
+++ b/util/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java
@@ -332,7 +332,7 @@ private UpdateResult readAndUpdate(File certFile, File keyFile, long oldKeyTime,
         try {
           X509Certificate[] certs = CertificateUtils.getX509Certificates(certInputStream);
           updateIdentityCredentials(certs, key);
-          return new UpdateResult(true, newKeyTime, newCertTime);
+          return new UpdateResult(true, newCertTime, newKeyTime);
         } finally {
           certInputStream.close();
         }
@@ -340,7 +340,7 @@ private UpdateResult readAndUpdate(File certFile, File keyFile, long oldKeyTime,
         keyInputStream.close();
       }
     }
-    return new UpdateResult(false, oldKeyTime, oldCertTime);
+    return new UpdateResult(false, oldCertTime, oldKeyTime);
   }
 
   /**
diff --git a/util/src/test/java/io/grpc/util/AdvancedTlsX509KeyManagerTest.java b/util/src/test/java/io/grpc/util/AdvancedTlsX509KeyManagerTest.java
index b8431d4f991..d855e77e77a 100644
--- a/util/src/test/java/io/grpc/util/AdvancedTlsX509KeyManagerTest.java
+++ b/util/src/test/java/io/grpc/util/AdvancedTlsX509KeyManagerTest.java
@@ -109,6 +109,29 @@ public void updateTrustCredentials_replacesIssuers() throws Exception {
     assertArrayEquals(serverCert0, serverKeyManager.getCertificateChain(alias4));
   }
 
+  @Test
+  public void scheduledReload_doesNotReloadWhenFilesAreUnchanged() throws Exception {
+    FakeClock fakeClock = new FakeClock();
+    AdvancedTlsX509KeyManager serverKeyManager = new AdvancedTlsX509KeyManager();
+
+    // Give the cert and key files distinct modification times, which is the normal case for two
+    // files written at different instants.
+    serverCert0File.setLastModified(TimeUnit.SECONDS.toMillis(1000));
+    serverKey0File.setLastModified(TimeUnit.SECONDS.toMillis(2000));
+
+    serverKeyManager.updateIdentityCredentials(serverCert0File, serverKey0File, 1, TimeUnit.MINUTES,
+        fakeClock.getScheduledExecutorService());
+
+    // Let one refresh cycle run; the scheduled reloader starts from a zero baseline, so it reloads
+    // once and rotates the alias.
+    fakeClock.forwardTime(1, TimeUnit.MINUTES);
+    String aliasAfterFirstCycle = serverKeyManager.chooseEngineServerAlias(null, null, null);
+
+    // Subsequent cycles with the files untouched must not reload, so the alias must stay the same.
+    fakeClock.forwardTime(5, TimeUnit.MINUTES);
+    assertEquals(aliasAfterFirstCycle, serverKeyManager.chooseEngineServerAlias(null, null, null));
+  }
+
   @Test
   public void allAliasMethods_returnNullBeforeCredentialsLoaded() {
     AdvancedTlsX509KeyManager keyManager = new AdvancedTlsX509KeyManager();