From f61791b6cfc51583e537bb200579916aa09bfeac Mon Sep 17 00:00:00 2001 From: bcleenders Date: Wed, 22 Jul 2026 13:35:19 +0200 Subject: [PATCH] fix(provider): validate peer chain before invoking SpiffeIdVerifier `SpiffeTrustManager` invoked the `SpiffeIdVerifier` before any cryptographic validation. The callback received an unvalidated, attacker-supplied chain despite the verifier's [API spec calling it `verifiedChain`](https://github.com/spiffe/java-spiffe/blob/55200fc8e6b01f9da5951c379e4f4187d7f920b7/java-spiffe-provider/src/main/java/io/spiffe/provider/SpiffeIdVerifier.java#L13). Validate the chain against the trust bundle first and only then invoke the verifier. This matches go-spiffe, where [the `Authorizer` only receives chains returned by `x509svid.ParseAndVerify`](https://github.com/spiffe/go-spiffe/blob/v2.8.1/spiffetls/tlsconfig/config.go#L193-L201) (`ParseAndVerify` first checks the certificates, afterwards `Authorizer` checks if the SPIFFE ID is authorized). Observable change: for a chain that is both untrusted and carries an unacceptable SPIFFE ID, the reported error is now the chain validation failure, rather than the verifier error. There is a small conflict on imports with https://github.com/spiffe/java-spiffe/pull/433 , so if you poke me after that one is merged, I'm happy to rebase. Signed-off-by: bcleenders --- .../io/spiffe/provider/SpiffeTrustManager.java | 14 +++++++------- .../spiffe/provider/SpiffeTrustManagerTest.java | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/java-spiffe-provider/src/main/java/io/spiffe/provider/SpiffeTrustManager.java b/java-spiffe-provider/src/main/java/io/spiffe/provider/SpiffeTrustManager.java index 7699e72d..8c80871f 100644 --- a/java-spiffe-provider/src/main/java/io/spiffe/provider/SpiffeTrustManager.java +++ b/java-spiffe-provider/src/main/java/io/spiffe/provider/SpiffeTrustManager.java @@ -162,19 +162,19 @@ public void checkServerTrusted(X509Certificate[] chain, final String authType, f checkServerTrusted(chain, authType); } - // Check that the SPIFFE ID in the peer's certificate is accepted and the chain can be validated with a - // root CA in the bundle source + // Check that the peer's certificate chain can be validated with a root CA in the bundle source + // and that the SPIFFE ID in the peer's certificate is accepted. private void validatePeerChain(final X509Certificate... chain) throws CertificateException { - SpiffeId spiffeId = CertificateUtils.getSpiffeId(chain[0]); try { - spiffeIdVerifier.verify(spiffeId, chain); - } catch (SpiffeVerificationException e) { + X509SvidValidator.verifyChain(Arrays.asList(chain), x509BundleSource); + } catch (BundleNotFoundException e) { throw new CertificateException(e.getMessage(), e); } + SpiffeId spiffeId = CertificateUtils.getSpiffeId(chain[0]); try { - X509SvidValidator.verifyChain(Arrays.asList(chain), x509BundleSource); - } catch (BundleNotFoundException e) { + spiffeIdVerifier.verify(spiffeId, chain); + } catch (SpiffeVerificationException e) { throw new CertificateException(e.getMessage(), e); } } diff --git a/java-spiffe-provider/src/test/java/io/spiffe/provider/SpiffeTrustManagerTest.java b/java-spiffe-provider/src/test/java/io/spiffe/provider/SpiffeTrustManagerTest.java index e13ddacd..b9979b66 100644 --- a/java-spiffe-provider/src/test/java/io/spiffe/provider/SpiffeTrustManagerTest.java +++ b/java-spiffe-provider/src/test/java/io/spiffe/provider/SpiffeTrustManagerTest.java @@ -29,6 +29,7 @@ import static io.spiffe.utils.X509CertificateTestUtils.createCertificate; import static io.spiffe.utils.X509CertificateTestUtils.createRootCA; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.Mockito.when; @@ -366,6 +367,22 @@ void checkServerTrusted_verifierResult_ThrowCertificateException() throws Bundle } } + @Test + void checkServerTrusted_untrustedChain_verifierNotInvoked() throws BundleNotFoundException { + when(bundleSource.getBundleForTrustDomain(TrustDomain.parse("example.org"))).thenReturn(bundleUnknown); + final AtomicBoolean verifierInvoked = new AtomicBoolean(false); + final SpiffeIdVerifier verifier = (spiffeId, verifiedChain) -> verifierInvoked.set(true); + + SpiffeTrustManager spiffeTrustManager = new SpiffeTrustManager(bundleSource, verifier); + try { + spiffeTrustManager.checkServerTrusted(chain, ""); + fail("CertificateException was expected"); + } catch (CertificateException e) { + assertEquals("Cert chain cannot be verified", e.getMessage()); + } + assertFalse(verifierInvoked.get(), "verifier must not be invoked when the chain cannot be verified"); + } + @Test void getAcceptedIssuers() { X509Certificate[] acceptedIssuers = spiffeTrustManager.getAcceptedIssuers();