From 8c764bfbbaa9c2504c2e03e6424a289c967d238c Mon Sep 17 00:00:00 2001 From: NopeCharon <115794254+NopeCharon@users.noreply.github.com> Date: Wed, 28 Oct 2026 02:42:15 +0530 Subject: [PATCH 1/2] fix(bundle): wrap unsupported JWKS key types in JwtBundleException on parse JwtBundle.parse documents JwtBundleException for bundles that cannot be parsed. If the JWKS contained a key whose type is neither EC nor RSA (for example an OKP/Ed25519 or an "oct" key), JwtSignatureAlgorithm.Family.parse threw an IllegalArgumentException that escaped unwrapped. JwtBundle.load already catches IllegalArgumentException for the same input, so the two entry points disagreed. The leak was most harmful on the Workload API watch path. StreamObservers' JWT bundles observer only catches JwtBundleException from GrpcConversionUtils.toJwtBundleSet, so the IllegalArgumentException was thrown out of StreamObserver.onNext. gRPC reacts by cancelling the call ("CANCELLED: Failed to read message."). CANCELLED is treated as non-retryable, so the watch ended permanently and later bundle updates (rotations) were never delivered. With the exception wrapped, the observer reports "Error processing JWT bundles update" to the watcher and keeps the stream open for the next update. Catch IllegalArgumentException in JwtBundle.parse as load does, and mention unsupported key types in the Javadoc. Tests cover JwtBundle.parse and GrpcConversionUtils.toJwtBundleSet with a JWKS that contains an OKP key. Signed-off-by: NopeCharon <115794254+NopeCharon@users.noreply.github.com> --- .../io/spiffe/bundle/jwtbundle/JwtBundle.java | 5 +++-- .../spiffe/bundle/jwtbundle/JwtBundleTest.java | 15 +++++++++++++++ .../workloadapi/GrpcConversionUtilsTest.java | 16 ++++++++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/java-spiffe-core/src/main/java/io/spiffe/bundle/jwtbundle/JwtBundle.java b/java-spiffe-core/src/main/java/io/spiffe/bundle/jwtbundle/JwtBundle.java index 7af75780..eacc5ec2 100644 --- a/java-spiffe-core/src/main/java/io/spiffe/bundle/jwtbundle/JwtBundle.java +++ b/java-spiffe-core/src/main/java/io/spiffe/bundle/jwtbundle/JwtBundle.java @@ -87,7 +87,8 @@ public static JwtBundle load(TrustDomain trustDomain, * @param trustDomain a {@link TrustDomain} * @param bundleBytes an array of bytes representing the JWT bundle. * @return an instance of a {@link JwtBundle} - * @throws JwtBundleException if there is an error reading or parsing the file, or if a keyId is empty + * @throws JwtBundleException if there is an error reading or parsing the file, if a keyId is empty, + * or if the bundle contains a key type that is not supported */ public static JwtBundle parse( TrustDomain trustDomain, @@ -99,7 +100,7 @@ public static JwtBundle parse( try { JWKSet jwkSet = JWKSet.parse(new String(bundleBytes)); return toJwtBundle(trustDomain, jwkSet); - } catch (ParseException | JOSEException | KeyException e) { + } catch (IllegalArgumentException | ParseException | JOSEException | KeyException e) { throw new JwtBundleException("Could not parse bundle from bytes", e); } } diff --git a/java-spiffe-core/src/test/java/io/spiffe/bundle/jwtbundle/JwtBundleTest.java b/java-spiffe-core/src/test/java/io/spiffe/bundle/jwtbundle/JwtBundleTest.java index 875304b6..6c8dee69 100644 --- a/java-spiffe-core/src/test/java/io/spiffe/bundle/jwtbundle/JwtBundleTest.java +++ b/java-spiffe-core/src/test/java/io/spiffe/bundle/jwtbundle/JwtBundleTest.java @@ -253,6 +253,21 @@ void testParse_MissingKid_Fails() throws URISyntaxException, IOException { } } + @Test + void testParse_InvalidKeyType_ThrowsJwtBundleException() throws URISyntaxException, IOException { + Path path = Paths.get(toUri("testdata/jwtbundle/jwks_invalid_keytype.json")); + byte[] bundleBytes = Files.readAllBytes(path); + TrustDomain trustDomain = TrustDomain.parse("domain.test"); + + try { + JwtBundle.parse(trustDomain, bundleBytes); + fail("should have thrown exception"); + } catch (JwtBundleException e) { + assertEquals("Could not parse bundle from bytes", e.getMessage()); + assertEquals("Unsupported JWT family algorithm: OKP", e.getCause().getMessage()); + } + } + @Test void testParseInvalidJson() throws KeyException { try { diff --git a/java-spiffe-core/src/test/java/io/spiffe/workloadapi/GrpcConversionUtilsTest.java b/java-spiffe-core/src/test/java/io/spiffe/workloadapi/GrpcConversionUtilsTest.java index 668380e6..cd52f44d 100644 --- a/java-spiffe-core/src/test/java/io/spiffe/workloadapi/GrpcConversionUtilsTest.java +++ b/java-spiffe-core/src/test/java/io/spiffe/workloadapi/GrpcConversionUtilsTest.java @@ -53,6 +53,22 @@ void test_toJwtBundleSet_emtpyResponse() { } } + @Test + void test_toJwtBundleSet_unsupportedKeyType() throws URISyntaxException, IOException { + byte[] bundleBytes = Files.readAllBytes(Paths.get(toUri("testdata/jwtbundle/jwks_invalid_keytype.json"))); + Workload.JWTBundlesResponse response = Workload.JWTBundlesResponse + .newBuilder() + .putBundles("example.org", ByteString.copyFrom(bundleBytes)) + .build(); + + try { + GrpcConversionUtils.toJwtBundleSet(response); + fail("should have thrown exception"); + } catch (JwtBundleException e) { + assertEquals("Could not parse bundle from bytes", e.getMessage()); + } + } + @Test void test_parseX509Bundle_corruptedBytes() { try { From d4133b025e17b85e0632d8b96dc3bdc9ff913782 Mon Sep 17 00:00:00 2001 From: NopeCharon <115794254+NopeCharon@users.noreply.github.com> Date: Thu, 29 Oct 2026 23:08:52 +0530 Subject: [PATCH 2/2] fix(workloadapi): keep JWT bundle watch alive after rejected updates DefaultJwtSource and CachedJwtSource threw a WatcherException from the JWT bundles watcher's onError. When the error came from a rejected update, the exception escaped StreamObserver.onNext and gRPC cancelled the call, so the watch ended and later bundle rotations were never applied. Only treat errors before the first update as initialization failures: the watcher records the error and init throws it, which also makes newSource fail instead of returning a source without bundles when the first update is rejected asynchronously. After initialization, errors are logged and the current bundles are kept, so the stream stays open and the next valid update replaces them. Signed-off-by: NopeCharon <115794254+NopeCharon@users.noreply.github.com> --- .../spiffe/workloadapi/CachedJwtSource.java | 19 +++++-- .../spiffe/workloadapi/DefaultJwtSource.java | 19 +++++-- .../workloadapi/CachedJwtSourceTest.java | 15 +++++ .../workloadapi/DefaultJwtSourceTest.java | 55 ++++++++++++++++++- .../workloadapi/WorkloadApiClientStub.java | 3 + 5 files changed, 102 insertions(+), 9 deletions(-) diff --git a/java-spiffe-core/src/main/java/io/spiffe/workloadapi/CachedJwtSource.java b/java-spiffe-core/src/main/java/io/spiffe/workloadapi/CachedJwtSource.java index e5ddbd3c..702cb027 100644 --- a/java-spiffe-core/src/main/java/io/spiffe/workloadapi/CachedJwtSource.java +++ b/java-spiffe-core/src/main/java/io/spiffe/workloadapi/CachedJwtSource.java @@ -19,6 +19,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; import java.util.logging.Logger; @@ -284,7 +285,8 @@ private boolean isTokenPastHalfLifetime(JwtSvid jwtSvid) { private void init(final Duration timeout) throws TimeoutException { CountDownLatch done = new CountDownLatch(1); - setJwtBundlesWatcher(done); + AtomicReference initError = new AtomicReference<>(); + setJwtBundlesWatcher(done, initError); boolean success; if (timeout.isZero()) { @@ -296,9 +298,13 @@ private void init(final Duration timeout) throws TimeoutException { if (!success) { throw new TimeoutException("Timeout waiting for JWT bundles update"); } + Throwable error = initError.get(); + if (error != null) { + throw new WatcherException("Error fetching JwtBundleSet", error); + } } - private void setJwtBundlesWatcher(final CountDownLatch done) { + private void setJwtBundlesWatcher(final CountDownLatch done, final AtomicReference initError) { workloadApiClient.watchJwtBundles(new Watcher() { @Override public void onUpdate(final JwtBundleSet update) { @@ -310,8 +316,13 @@ public void onUpdate(final JwtBundleSet update) { @Override public void onError(final Throwable error) { log.log(Level.SEVERE, "Error in JwtBundleSet watcher", error); - done.countDown(); - throw new WatcherException("Error fetching JwtBundleSet", error); + // An error before the first update fails the initialization. After that, the current bundles + // are kept: throwing here would escape the gRPC stream observer and cancel the watch, so later + // bundle updates would never be applied. + if (done.getCount() > 0) { + initError.compareAndSet(null, error); + done.countDown(); + } } }); } diff --git a/java-spiffe-core/src/main/java/io/spiffe/workloadapi/DefaultJwtSource.java b/java-spiffe-core/src/main/java/io/spiffe/workloadapi/DefaultJwtSource.java index ed5699fb..f0f8fc1b 100644 --- a/java-spiffe-core/src/main/java/io/spiffe/workloadapi/DefaultJwtSource.java +++ b/java-spiffe-core/src/main/java/io/spiffe/workloadapi/DefaultJwtSource.java @@ -15,6 +15,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; import java.util.logging.Logger; @@ -188,7 +189,8 @@ public void close() { private void init(final Duration timeout) throws TimeoutException { CountDownLatch done = new CountDownLatch(1); - setJwtBundlesWatcher(done); + AtomicReference initError = new AtomicReference<>(); + setJwtBundlesWatcher(done, initError); boolean success; if (timeout.isZero()) { @@ -200,9 +202,13 @@ private void init(final Duration timeout) throws TimeoutException { if (!success) { throw new TimeoutException("Timeout waiting for JWT bundles update"); } + Throwable error = initError.get(); + if (error != null) { + throw new WatcherException("Error fetching JwtBundleSet", error); + } } - private void setJwtBundlesWatcher(final CountDownLatch done) { + private void setJwtBundlesWatcher(final CountDownLatch done, final AtomicReference initError) { workloadApiClient.watchJwtBundles(new Watcher() { @Override public void onUpdate(final JwtBundleSet update) { @@ -214,8 +220,13 @@ public void onUpdate(final JwtBundleSet update) { @Override public void onError(final Throwable error) { log.log(Level.SEVERE, "Error in JwtBundleSet watcher", error); - done.countDown(); - throw new WatcherException("Error fetching JwtBundleSet", error); + // An error before the first update fails the initialization. After that, the current bundles + // are kept: throwing here would escape the gRPC stream observer and cancel the watch, so later + // bundle updates would never be applied. + if (done.getCount() > 0) { + initError.compareAndSet(null, error); + done.countDown(); + } } }); } diff --git a/java-spiffe-core/src/test/java/io/spiffe/workloadapi/CachedJwtSourceTest.java b/java-spiffe-core/src/test/java/io/spiffe/workloadapi/CachedJwtSourceTest.java index 72b1ac2c..27031857 100644 --- a/java-spiffe-core/src/test/java/io/spiffe/workloadapi/CachedJwtSourceTest.java +++ b/java-spiffe-core/src/test/java/io/spiffe/workloadapi/CachedJwtSourceTest.java @@ -4,7 +4,9 @@ import com.nimbusds.jose.jwk.Curve; import com.nimbusds.jwt.JWTClaimsSet; import io.spiffe.bundle.jwtbundle.JwtBundle; +import io.spiffe.bundle.jwtbundle.JwtBundleSet; import io.spiffe.exception.BundleNotFoundException; +import io.spiffe.exception.JwtBundleException; import io.spiffe.exception.JwtSourceException; import io.spiffe.exception.JwtSvidException; import io.spiffe.exception.SocketEndpointAddressException; @@ -100,6 +102,19 @@ void testGetBundleForTrustDomain_SourceIsClosed_ThrowsIllegalStateException() th } } + @Test + void testJwtBundlesWatcherError_afterInit_keepsBundlesAndAppliesLaterUpdates() throws BundleNotFoundException { + TrustDomain trustDomain = TrustDomain.parse("example.org"); + JwtBundle initialBundle = jwtSource.getBundleForTrustDomain(trustDomain); + + workloadApiClient.jwtBundlesWatcher.onError(new JwtBundleException("Error processing JWT bundles update")); + assertSame(initialBundle, jwtSource.getBundleForTrustDomain(trustDomain)); + + JwtBundle updatedBundle = new JwtBundle(trustDomain); + workloadApiClient.jwtBundlesWatcher.onUpdate(JwtBundleSet.of(Collections.singleton(updatedBundle))); + assertSame(updatedBundle, jwtSource.getBundleForTrustDomain(trustDomain)); + } + @Test void testFetchJwtSvidWithSubject() { try { diff --git a/java-spiffe-core/src/test/java/io/spiffe/workloadapi/DefaultJwtSourceTest.java b/java-spiffe-core/src/test/java/io/spiffe/workloadapi/DefaultJwtSourceTest.java index ee6a8060..bf256634 100644 --- a/java-spiffe-core/src/test/java/io/spiffe/workloadapi/DefaultJwtSourceTest.java +++ b/java-spiffe-core/src/test/java/io/spiffe/workloadapi/DefaultJwtSourceTest.java @@ -1,25 +1,37 @@ package io.spiffe.workloadapi; import com.google.common.collect.Sets; +import com.google.protobuf.ByteString; +import io.grpc.stub.StreamObserver; +import io.grpc.testing.GrpcCleanupRule; import io.spiffe.bundle.jwtbundle.JwtBundle; +import io.spiffe.bundle.jwtbundle.JwtBundleSet; import io.spiffe.exception.BundleNotFoundException; +import io.spiffe.exception.JwtBundleException; import io.spiffe.exception.JwtSourceException; import io.spiffe.exception.JwtSvidException; import io.spiffe.exception.SocketEndpointAddressException; import io.spiffe.spiffeid.SpiffeId; import io.spiffe.spiffeid.TrustDomain; import io.spiffe.svid.jwtsvid.JwtSvid; +import io.spiffe.workloadapi.grpc.SpiffeWorkloadAPIGrpc; +import io.spiffe.workloadapi.grpc.Workload; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import uk.org.webcompere.systemstubs.environment.EnvironmentVariables; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; import java.time.Duration; +import java.util.Collections; import java.util.List; +import static io.spiffe.utils.TestUtils.toUri; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -80,6 +92,40 @@ void testGetBundleForTrustDomain_SourceIsClosed_ThrowsIllegalStateException() th } } + @Test + void testJwtBundlesWatcherError_afterInit_keepsBundlesAndAppliesLaterUpdates() throws BundleNotFoundException { + TrustDomain trustDomain = TrustDomain.parse("example.org"); + JwtBundle initialBundle = jwtSource.getBundleForTrustDomain(trustDomain); + + workloadApiClient.jwtBundlesWatcher.onError(new JwtBundleException("Error processing JWT bundles update")); + assertSame(initialBundle, jwtSource.getBundleForTrustDomain(trustDomain)); + + JwtBundle updatedBundle = new JwtBundle(trustDomain); + workloadApiClient.jwtBundlesWatcher.onUpdate(JwtBundleSet.of(Collections.singleton(updatedBundle))); + assertSame(updatedBundle, jwtSource.getBundleForTrustDomain(trustDomain)); + } + + @Test + void testWatchJwtBundles_unsupportedKeyTypeUpdate_laterUpdatesAreApplied() throws Exception { + byte[] validBundle = Files.readAllBytes(Paths.get(toUri("testdata/workloadapi/bundle.json"))); + byte[] unsupportedKeyTypeBundle = Files.readAllBytes(Paths.get(toUri("testdata/jwtbundle/jwks_invalid_keytype.json"))); + SpiffeWorkloadAPIGrpc.SpiffeWorkloadAPIImplBase fakeWorkloadApi = new SpiffeWorkloadAPIGrpc.SpiffeWorkloadAPIImplBase() { + @Override + public void fetchJWTBundles(Workload.JWTBundlesRequest request, StreamObserver responseObserver) { + responseObserver.onNext(newJwtBundlesResponse("example.org", validBundle)); + responseObserver.onNext(newJwtBundlesResponse("example.org", unsupportedKeyTypeBundle)); + responseObserver.onNext(newJwtBundlesResponse("domain.test", validBundle)); + } + }; + DefaultWorkloadApiClient client = WorkloadApiClientTestUtil.create(fakeWorkloadApi, new GrpcCleanupRule()); + JwtSourceOptions options = JwtSourceOptions.builder().workloadApiClient(client).build(); + + try (JwtSource source = DefaultJwtSource.newSource(options)) { + JwtBundle bundle = source.getBundleForTrustDomain(TrustDomain.parse("domain.test")); + assertEquals(TrustDomain.parse("domain.test"), bundle.getTrustDomain()); + } + } + @Test void testFetchJwtSvidWithSubject() { try { @@ -281,4 +327,11 @@ void newSource_noSocketAddress() throws Exception { } }); } -} \ No newline at end of file + + private static Workload.JWTBundlesResponse newJwtBundlesResponse(String trustDomain, byte[] bundle) { + return Workload.JWTBundlesResponse + .newBuilder() + .putBundles(trustDomain, ByteString.copyFrom(bundle)) + .build(); + } +} diff --git a/java-spiffe-core/src/test/java/io/spiffe/workloadapi/WorkloadApiClientStub.java b/java-spiffe-core/src/test/java/io/spiffe/workloadapi/WorkloadApiClientStub.java index b6d51d87..13985c8c 100644 --- a/java-spiffe-core/src/test/java/io/spiffe/workloadapi/WorkloadApiClientStub.java +++ b/java-spiffe-core/src/test/java/io/spiffe/workloadapi/WorkloadApiClientStub.java @@ -39,6 +39,8 @@ public class WorkloadApiClientStub implements WorkloadApiClient { int fetchJwtSvidCallCount = 0; + Watcher jwtBundlesWatcher; + boolean closed; Clock clock = Clock.systemDefaultZone(); @@ -106,6 +108,7 @@ public JwtSvid validateJwtSvid(String token, String audience) throws JwtSvidExce @Override public void watchJwtBundles(Watcher watcher) { + jwtBundlesWatcher = watcher; JwtBundleSet jwtBundleSet = generateJwtBundleSet(); watcher.onUpdate(jwtBundleSet); }