This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch coheigea/url-procols in repository https://gitbox.apache.org/repos/asf/cxf.git
commit 13f284100d55be8eed51308758901738ba709c06 Author: Colm O hEigeartaigh <[email protected]> AuthorDate: Fri May 8 12:21:40 2026 +0100 Restrict valid URL protocols in TLSParameterJaxBUtils and URIResolver --- .../configuration/jsse/TLSParameterJaxBUtils.java | 23 +++++- .../java/org/apache/cxf/resource/URIResolver.java | 39 +++++++++ .../jsse/TLSParameterJaxBUtilsTest.java | 93 ++++++++++++++++++++++ .../org/apache/cxf/resource/URIResolverTest.java | 45 +++++++++++ 4 files changed, 198 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/apache/cxf/configuration/jsse/TLSParameterJaxBUtils.java b/core/src/main/java/org/apache/cxf/configuration/jsse/TLSParameterJaxBUtils.java index 9dc81b1236c..45e734e2e06 100644 --- a/core/src/main/java/org/apache/cxf/configuration/jsse/TLSParameterJaxBUtils.java +++ b/core/src/main/java/org/apache/cxf/configuration/jsse/TLSParameterJaxBUtils.java @@ -69,6 +69,9 @@ public final class TLSParameterJaxBUtils { private static final Logger LOG = LogUtils.getL7dLogger(TLSParameterJaxBUtils.class); + private static final java.util.Set<String> ALLOWED_KEYSTORE_SCHEMES = + java.util.Collections.unmodifiableSet(new java.util.HashSet<>(java.util.Arrays.asList("file", "https"))); + private TLSParameterJaxBUtils() { // empty } @@ -163,7 +166,7 @@ public final class TLSParameterJaxBUtils { keyStore.load(is, password); } } else if (kst.isSetUrl()) { - keyStore.load(new URL(kst.getUrl()).openStream(), password); + keyStore.load(openKeystoreUrl(kst.getUrl()), password); } else { final String loc; if (trustStore) { @@ -215,11 +218,27 @@ public final class TLSParameterJaxBUtils { return createTrustStore(is, type); } if (pst.isSetUrl()) { - return createTrustStore(new URL(pst.getUrl()).openStream(), type); + return createTrustStore(openKeystoreUrl(pst.getUrl()), type); } throw new IllegalArgumentException("Could not create KeyStore based on information in CertStoreType"); } + /** + * Opens a URL for keystore loading after validating that its scheme is in the + * allowlist ({@code file}, {@code https}). Schemes such as {@code ftp}, + * {@code http}, {@code jar}, or {@code jndi} are rejected to prevent SSRF. + */ + private static InputStream openKeystoreUrl(String urlString) throws IOException { + URL url = new URL(urlString); + String scheme = url.getProtocol(); + if (!ALLOWED_KEYSTORE_SCHEMES.contains(scheme)) { + throw new IllegalArgumentException( + "Keystore URL scheme '" + scheme + "' is not permitted. " + + "Allowed schemes: " + ALLOWED_KEYSTORE_SCHEMES); + } + return url.openStream(); + } + private static InputStream getResourceAsStream(String resource) { InputStream is = ClassLoaderUtils.getResourceAsStream(resource, TLSParameterJaxBUtils.class); if (is == null) { diff --git a/core/src/main/java/org/apache/cxf/resource/URIResolver.java b/core/src/main/java/org/apache/cxf/resource/URIResolver.java index 30bc6a682fe..fe3fb1014a5 100644 --- a/core/src/main/java/org/apache/cxf/resource/URIResolver.java +++ b/core/src/main/java/org/apache/cxf/resource/URIResolver.java @@ -32,8 +32,13 @@ import java.net.URLDecoder; import java.nio.file.Files; import java.security.AccessController; import java.security.PrivilegedAction; +import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; +import java.util.Locale; import java.util.Map; +import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; @@ -56,6 +61,10 @@ import org.apache.cxf.helpers.LoadingByteArrayOutputStream; */ public class URIResolver implements AutoCloseable { private static final Logger LOG = LogUtils.getLogger(URIResolver.class); + private static final String ALLOWED_URL_SCHEMES_PROPERTY = + "org.apache.cxf.resource.uriresolver.allowedSchemes"; + private static final Set<String> DEFAULT_ALLOWED_URL_SCHEMES = + Collections.unmodifiableSet(new HashSet<>(Arrays.asList("file", "http", "https", "jar", "zip", "wsjar"))); private Map<String, LoadingByteArrayOutputStream> cache = new HashMap<>(); private File file; @@ -168,6 +177,7 @@ public class URIResolver implements AutoCloseable { if (relative.isAbsolute()) { uri = relative; url = relative.toURL(); + checkAllowedScheme(url); try { HttpURLConnection huc = createInputStream(); @@ -258,6 +268,7 @@ public class URIResolver implements AutoCloseable { } private HttpURLConnection createInputStream() throws IOException { + checkAllowedScheme(url); HttpURLConnection huc = (HttpURLConnection)url.openConnection(); String host = SystemPropertyAction.getPropertyOrNull("http.proxyHost"); @@ -344,6 +355,7 @@ public class URIResolver implements AutoCloseable { } url = new URL(uriStr); + checkAllowedScheme(url); try { is = url.openStream(); try { @@ -409,6 +421,7 @@ public class URIResolver implements AutoCloseable { try { LoadingByteArrayOutputStream bout = cache.get(uriStr); url = new URL(uriStr); + checkAllowedScheme(url); uri = new URI(url.toString()); if (bout == null) { URLConnection connection = url.openConnection(); @@ -424,6 +437,32 @@ public class URIResolver implements AutoCloseable { } } + private static void checkAllowedScheme(URL targetUrl) throws IOException { + String scheme = targetUrl.getProtocol(); + if (scheme == null) { + return; + } + scheme = scheme.toLowerCase(Locale.ROOT); + if (!getAllowedSchemes().contains(scheme)) { + throw new IOException("URL scheme '" + scheme + "' is not permitted for URIResolver. " + + "Allowed schemes: " + getAllowedSchemes()); + } + } + + private static Set<String> getAllowedSchemes() { + Set<String> allowed = new HashSet<>(DEFAULT_ALLOWED_URL_SCHEMES); + String configured = SystemPropertyAction.getPropertyOrNull(ALLOWED_URL_SCHEMES_PROPERTY); + if (configured != null) { + for (String entry : configured.split(",")) { + String scheme = entry.trim().toLowerCase(Locale.ROOT); + if (!scheme.isEmpty()) { + allowed.add(scheme); + } + } + } + return allowed; + } + public URI getURI() { return uri; } diff --git a/core/src/test/java/org/apache/cxf/configuration/jsse/TLSParameterJaxBUtilsTest.java b/core/src/test/java/org/apache/cxf/configuration/jsse/TLSParameterJaxBUtilsTest.java new file mode 100644 index 00000000000..49706cdc58e --- /dev/null +++ b/core/src/test/java/org/apache/cxf/configuration/jsse/TLSParameterJaxBUtilsTest.java @@ -0,0 +1,93 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.cxf.configuration.jsse; + +import java.io.IOException; +import java.io.OutputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.security.KeyStore; + +import org.apache.cxf.configuration.security.KeyStoreType; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.fail; + +/** + * Unit tests for TLSParameterJaxBUtils keystore loading. + * + * <p>CXF-SSRF-001 — {@link TLSParameterJaxBUtils#getKeyStore(KeyStoreType, boolean)} passes the + * configured {@code url} attribute directly to {@code new URL(kst.getUrl()).openStream()} without + * validating the scheme. This allows any protocol understood by the JDK URL handler (including + * {@code ftp:}, {@code jar:}, {@code jndi:}, …) to be used as the keystore source, creating a + * Server-Side Request Forgery (SSRF) surface.</p> + */ +public class TLSParameterJaxBUtilsTest { + + @Test + public void testFileBasedJksKeystoreIsLoaded() throws Exception { + final String password = "changeit"; + final Path tempJks = Files.createTempFile("cxf-filestore-", ".jks"); + + try { + KeyStore source = KeyStore.getInstance("JKS"); + source.load(null, password.toCharArray()); + try (OutputStream os = Files.newOutputStream(tempJks)) { + source.store(os, password.toCharArray()); + } + + KeyStoreType kst = new KeyStoreType(); + kst.setType("JKS"); + kst.setPassword(password); + kst.setFile(tempJks.toString()); + + KeyStore loaded = TLSParameterJaxBUtils.getKeyStore(kst, false); + assertNotNull(loaded); + assertEquals(0, loaded.size()); + } finally { + Files.deleteIfExists(tempJks); + } + } + + /** + * Verifies that {@code getKeyStore()} rejects an {@code ftp://} URL with an + * {@link IllegalArgumentException}. Prior to the fix, the URL was passed directly + * to {@code new URL(…).openStream()} without scheme validation, allowing any JDK-supported + * protocol (ftp, jar, jndi, …) to be used as a keystore source (SSRF). + */ + @Test + public void testFtpUrlAllowedErroneouslyInKeystoreUrl() throws Exception { + KeyStoreType kst = new KeyStoreType(); + kst.setUrl("ftp://127.0.0.1:12345/keystore.jks"); + + try { + TLSParameterJaxBUtils.getKeyStore(kst, false); + fail("Expected IllegalArgumentException for ftp:// keystore URL — scheme not in allowlist"); + } catch (IllegalArgumentException e) { + // expected — ftp:// is correctly rejected + } catch (IOException e) { + fail("Expected IllegalArgumentException but got IOException, " + + "meaning ftp:// was accepted and an outbound connection was attempted: " + e); + } + } + +} diff --git a/core/src/test/java/org/apache/cxf/resource/URIResolverTest.java b/core/src/test/java/org/apache/cxf/resource/URIResolverTest.java index b6bb0669371..adc21742a45 100644 --- a/core/src/test/java/org/apache/cxf/resource/URIResolverTest.java +++ b/core/src/test/java/org/apache/cxf/resource/URIResolverTest.java @@ -20,7 +20,11 @@ package org.apache.cxf.resource; import java.io.InputStream; +import java.io.OutputStream; +import java.net.ServerSocket; +import java.net.Socket; import java.net.URL; +import java.nio.charset.StandardCharsets; import org.apache.cxf.helpers.IOUtils; @@ -31,6 +35,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; public class URIResolverTest { @@ -186,6 +191,46 @@ public class URIResolverTest { resolveWithCheckingClassloaderInConstructor(null, "wsdl/folder%20with%20spaces/foo.wsdl", this.getClass()); } + @Test + public void testFtpProtocolRejectedByDefault() throws Exception { + try { + new URIResolver("ftp://127.0.0.1:12345/example.wsdl"); + fail("Expected IOException for disallowed ftp:// scheme"); + } catch (java.io.IOException ex) { + assertTrue(ex.getMessage().contains("ftp")); + assertTrue(ex.getMessage().contains("not permitted")); + } + } + + @Test + public void testHttpProtocolStillAllowed() throws Exception { + checkingThreadThrowable = null; + try (ServerSocket serverSocket = new ServerSocket(0)) { + Thread t = new Thread(() -> { + try (Socket socket = serverSocket.accept(); + OutputStream os = socket.getOutputStream()) { + os.write(("HTTP/1.1 200 OK\r\n" + + "Content-Length: 4\r\n" + + "Connection: close\r\n\r\n" + + "test").getBytes(StandardCharsets.US_ASCII)); + os.flush(); + } catch (Throwable th) { + checkingThreadThrowable = th; + } + }); + t.start(); + + URIResolver resolver = new URIResolver("http://127.0.0.1:" + serverSocket.getLocalPort() + "/schema.xsd"); + assertTrue(resolver.isResolved()); + assertNotNull(resolver.getInputStream()); + resolver.close(); + + t.join(5000); + assertFalse("HTTP server thread did not finish in time", t.isAlive()); + assertNull(checkingThreadThrowable); + } + } + private void resolveWithCheckingClassloader(URIResolver resolver, String baseUriStr, String uriStr, Class<?> callingCls) throws InterruptedException { checkingThreadThrowable = null;
