From 65e15082759fa14214cda6d81bb913db372a0b46 Mon Sep 17 00:00:00 2001 From: Slava Novak Date: Wed, 29 Jul 2026 14:37:22 +0000 Subject: [PATCH] ticket-777: Fix the bug when IMDS fails during auth token refresh which makes followers never reset the state --- .../auth/internal/X509FederationClient.java | 98 ++++++------ .../internal/X509FederationClientTest.java | 145 ++++++++++++++++++ 2 files changed, 198 insertions(+), 45 deletions(-) diff --git a/bmc-common/src/main/java/com/oracle/bmc/auth/internal/X509FederationClient.java b/bmc-common/src/main/java/com/oracle/bmc/auth/internal/X509FederationClient.java index f39d8b0ff2..b8024837f5 100644 --- a/bmc-common/src/main/java/com/oracle/bmc/auth/internal/X509FederationClient.java +++ b/bmc-common/src/main/java/com/oracle/bmc/auth/internal/X509FederationClient.java @@ -237,55 +237,14 @@ private String refreshAndGetSecurityTokenInner( } if (iAmTheLeader) { - LOG.info("[Leader] About to refresh security token."); - - if (refreshKeys) { - LOG.info("Refreshing session keys."); - sessionKeySupplier.refreshKeys(); - } - if (leafCertificateSupplier instanceof Refreshable) { - try { - ((Refreshable) leafCertificateSupplier).refresh(); - } catch (RefreshFailedException ex) { - throw new BmcException( - false, "Can't refresh the leaf certification!", ex, null); - } - // When using default purpose (ex, instance principals), the token request should always be signed with the same tenant id as the certificate. - // For other purposes, the tenant id can be different. - if (this.purpose.equals(DEFAULT_PURPOSE)) { - String newTenancyId = - AuthUtils.getTenantIdFromCertificate( - leafCertificateSupplier - .getCertificateAndKeyPair() - .getCertificate()); - - if (!this.tenancyId.equals(newTenancyId)) { - throw new IllegalArgumentException( - "The tenancy id should never be changed in cert file!"); - } - } - } - - for (X509CertificateSupplier supplier : intermediateCertificateSuppliers) { - if (supplier instanceof Refreshable) { - try { - ((Refreshable) supplier).refresh(); - } catch (RefreshFailedException ex) { - throw new BmcException( - false, "Can't refresh the intermediate certification!", ex, null); - } - } - } - try { - securityTokenAdapter = getSecurityTokenFromServer(); - String token = securityTokenAdapter.getSecurityToken(); + LOG.info("[Leader] About to refresh security token."); + String token = refreshSecurityToken(refreshKeys); future.complete(token); return token; - } catch (Exception e) { - LOG.error("Error refreshing security token", e); + } catch (RuntimeException e) { future.completeExceptionally(e); - throw new BmcException(false, "Error refreshing security token.", e, null); + throw e; } finally { inFlightRefresh = null; } @@ -310,6 +269,55 @@ private String refreshAndGetSecurityTokenInner( } } + private String refreshSecurityToken(boolean refreshKeys) { + if (refreshKeys) { + LOG.info("Refreshing session keys."); + sessionKeySupplier.refreshKeys(); + } + if (leafCertificateSupplier instanceof Refreshable) { + try { + ((Refreshable) leafCertificateSupplier).refresh(); + } catch (RefreshFailedException ex) { + throw new BmcException( + false, + "Unable to refresh the client certificate used to obtain an OCI security token.", + ex, + null); + } + // When using default purpose (ex, instance principals), the token request should always be signed with the same tenant id as the certificate. + // For other purposes, the tenant id can be different. + if (this.purpose.equals(DEFAULT_PURPOSE)) { + String newTenancyId = + AuthUtils.getTenantIdFromCertificate( + leafCertificateSupplier.getCertificateAndKeyPair().getCertificate()); + + if (!this.tenancyId.equals(newTenancyId)) { + throw new IllegalArgumentException( + "The tenancy id should never be changed in cert file!"); + } + } + } + + for (X509CertificateSupplier supplier : intermediateCertificateSuppliers) { + if (supplier instanceof Refreshable) { + try { + ((Refreshable) supplier).refresh(); + } catch (RefreshFailedException ex) { + throw new BmcException( + false, "Can't refresh the intermediate certification!", ex, null); + } + } + } + + try { + securityTokenAdapter = getSecurityTokenFromServer(); + return securityTokenAdapter.getSecurityToken(); + } catch (Exception e) { + LOG.error("Error refreshing security token", e); + throw new BmcException(false, "Error refreshing security token.", e, null); + } + } + /** * Gets a security token from the federation server * @return the security token, which is basically a JWT token string diff --git a/bmc-common/src/test/java/com/oracle/bmc/auth/internal/X509FederationClientTest.java b/bmc-common/src/test/java/com/oracle/bmc/auth/internal/X509FederationClientTest.java index f47a004bfe..b47f694930 100644 --- a/bmc-common/src/test/java/com/oracle/bmc/auth/internal/X509FederationClientTest.java +++ b/bmc-common/src/test/java/com/oracle/bmc/auth/internal/X509FederationClientTest.java @@ -14,9 +14,12 @@ import com.oracle.bmc.model.BmcException; import com.oracle.bmc.requests.BmcRequest; import java.security.KeyPairGenerator; +import java.security.cert.X509Certificate; import java.util.ArrayList; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -30,6 +33,8 @@ import javax.ws.rs.client.Invocation; import javax.ws.rs.core.Response; +import javax.security.auth.RefreshFailedException; +import javax.security.auth.Refreshable; import java.io.IOException; import java.net.URI; @@ -312,4 +317,144 @@ public void jacksonCanRoundTripSecurityToken() throws IOException { 1, serverCallCount.get()); } + + /** + * Regression test for: + * + * + * + *

This test simulates the Instance Metadata Service (IMDS) returning HTTP 404 while the + * leader refreshes {@code cert.pem}. The leader has already created {@code inFlightRefresh}; a + * concurrent follower therefore waits on that Future instead of starting another refresh. + * + *

The test verifies that the leader completes {@code inFlightRefresh} exceptionally before + * it returns. The follower must then receive the same error promptly, rather than waiting for + * the one-minute single-flight timeout. + */ + @Test + public void + refreshFailureBeforeTokenRequest_completesInFlightRefreshSoFollowerDoesNotWait() + throws Exception { + CountDownLatch leaderStartedCertificateRefresh = new CountDownLatch(1); + CountDownLatch failCertificateRefresh = new CountDownLatch(1); + AtomicReference leaderFailure = new AtomicReference<>(); + AtomicReference followerFailure = new AtomicReference<>(); + + class RefreshableFailingLeafCertificateSupplier + implements X509CertificateSupplier, Refreshable { + @Override + public boolean isCurrent() { + return false; + } + + @Override + public void refresh() throws RefreshFailedException { + leaderStartedCertificateRefresh.countDown(); + try { + if (!failCertificateRefresh.await(5, TimeUnit.SECONDS)) { + throw new RefreshFailedException( + "Timed out waiting to simulate cert.pem failure"); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RefreshFailedException("Interrupted while simulating cert.pem failure"); + } + throw new RefreshFailedException("IMDS GET cert.pem returned HTTP 404"); + } + + @Override + @Deprecated + public X509Certificate getCertificate() { + return null; + } + + @Override + @Deprecated + public java.security.interfaces.RSAPrivateKey getPrivateKey() { + return null; + } + + @Override + public CertificateAndPrivateKeyPair getCertificateAndKeyPair() { + return null; + } + } + + X509CertificateSupplier refreshableLeafCertificateSupplier = + new RefreshableFailingLeafCertificateSupplier(); + + X509FederationClient client = + new X509FederationClient( + "https://auth.example.com", + "testTenantId", + refreshableLeafCertificateSupplier, + mock(SessionKeySupplier.class), + Collections.emptySet(), + mock(ClientConfigurator.class), + Collections.emptyList(), + mock(CircuitBreakerConfiguration.class)); + + Thread leader = + new Thread( + () -> { + try { + client.getSecurityToken(); + } catch (Throwable e) { + leaderFailure.set(e); + } + }, + "refresh-leader"); + leader.start(); + assertEquals( + "Leader should begin the simulated cert.pem refresh", + true, + leaderStartedCertificateRefresh.await(5, TimeUnit.SECONDS)); + + Thread follower = + new Thread( + () -> { + try { + client.getSecurityToken(); + } catch (Throwable e) { + followerFailure.set(e); + } + }, + "refresh-follower"); + follower.start(); + + try { + final Object waitObject = new Object(); + long deadline = System.currentTimeMillis() + 5_000; + while (follower.getState() != Thread.State.TIMED_WAITING + && System.currentTimeMillis() < deadline) { + synchronized (waitObject) { + waitObject.wait(10); + } + } + assertEquals( + "Follower should wait on the leader's in-flight refresh before it fails", + Thread.State.TIMED_WAITING, + follower.getState()); + + failCertificateRefresh.countDown(); + leader.join(5_000); + follower.join(2_000); + + assertFalse("Leader should return the certificate refresh error", leader.isAlive()); + assertFalse( + "Follower must receive the leader failure instead of waiting for inFlightRefresh", + follower.isAlive()); + assertEquals(BmcException.class, leaderFailure.get().getClass()); + assertEquals(BmcException.class, followerFailure.get().getClass()); + } finally { + failCertificateRefresh.countDown(); + follower.interrupt(); + leader.join(5_000); + follower.join(5_000); + } + } }