Race Condition allows Bypass of Trust Restrictions

@@ -21,9 +21,6 @@
 import java.net.Proxy;
 import java.net.URL;
 import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
 import java.util.logging.Logger;
 
 import javax.ws.rs.client.Client;
@@ -290,16 +287,12 @@ public interface ConnectionFactory {
          * @throws java.io.IOException in case the connection cannot be provided.
          */
         default HttpURLConnection getConnection(URL url, Proxy proxy) throws IOException {
-            synchronized (this){
-                return (proxy == null) ? getConnection(url) : (HttpURLConnection) url.openConnection(proxy);
-            }
+            return (proxy == null) ? getConnection(url) : (HttpURLConnection) url.openConnection(proxy);
         }
     }
 
     private static class DefaultConnectionFactory implements ConnectionFactory {
 
-        private final ConcurrentHashMap<URL, Lock> locks = new ConcurrentHashMap<>();
-
         @Override
         public HttpURLConnection getConnection(final URL url) throws IOException {
             return connect(url, null);
@@ -311,13 +304,7 @@ public HttpURLConnection getConnection(URL url, Proxy proxy) throws IOException
         }
 
         private HttpURLConnection connect(URL url, Proxy proxy) throws IOException {
-            Lock lock = locks.computeIfAbsent(url, u -> new ReentrantLock());
-            lock.lock();
-            try {
-                return (proxy == null) ? (HttpURLConnection) url.openConnection() : (HttpURLConnection) url.openConnection(proxy);
-            } finally {
-                lock.unlock();
-            }
+            return (proxy == null) ? (HttpURLConnection) url.openConnection() : (HttpURLConnection) url.openConnection(proxy);
         }
     }
 

@@ -84,7 +84,7 @@ public class HttpUrlConnector implements Connector {
     private static final String ALLOW_RESTRICTED_HEADERS_SYSTEM_PROPERTY = "sun.net.http.allowRestrictedHeaders";
     // Avoid multi-thread uses of HttpsURLConnection.getDefaultSSLSocketFactory() because it does not implement a
     // proper lazy-initialization. See https://github.com/jersey/jersey/issues/3293
-    private static final Value<SSLSocketFactory> DEFAULT_SSL_SOCKET_FACTORY =
+    private static final LazyValue<SSLSocketFactory> DEFAULT_SSL_SOCKET_FACTORY =
             Values.lazy((Value<SSLSocketFactory>) () -> HttpsURLConnection.getDefaultSSLSocketFactory());
     // The list of restricted headers is extracted from sun.net.www.protocol.http.HttpURLConnection
     private static final String[] restrictedHeaders = {
@@ -387,6 +387,10 @@ private ClientResponse _apply(final ClientRequest request) throws IOException {
             sniUri = request.getUri();
         }
 
+        if (!DEFAULT_SSL_SOCKET_FACTORY.isInitialized() && "HTTPS".equalsIgnoreCase(sniUri.getScheme())) {
+            DEFAULT_SSL_SOCKET_FACTORY.get();
+        }
+
         proxy.ifPresent(clientProxy -> ClientProxy.setBasicAuthorizationHeader(request.getHeaders(), proxy.get()));
         uc = this.connectionFactory.getConnection(sniUri.toURL(), proxy.isPresent() ? proxy.get().proxy() : null);
         uc.setDoInput(true);

@@ -164,6 +164,24 @@
                 </http.patch.addopens>
             </properties>
         </profile>
+        <profile>
+            <id>JDK_17-</id>
+            <activation>
+                <jdk>[1.8,17)</jdk>
+            </activation>
+            <build>
+                <plugins>
+                    <plugin>
+                        <groupId>org.apache.maven.plugins</groupId>
+                        <artifactId>maven-surefire-plugin</artifactId>
+                        <configuration>
+                            <!-- The certificate is not working with JDK 11 -->
+                            <excludes><exclude>**/ConcurrentHttpsUrlConnectionTest*</exclude></excludes>
+                        </configuration>
+                    </plugin>
+                </plugins>
+            </build>
+        </profile>
     </profiles>
 
 </project>

@@ -0,0 +1,119 @@
+/*
+ * Copyright (c) 2024 Oracle and/or its affiliates. All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0, which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the
+ * Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
+ * version 2 with the GNU Classpath Exception, which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ */
+
+package org.glassfish.jersey.tests.e2e.tls.connector;
+
+import org.junit.jupiter.api.Test;
+
+import javax.ws.rs.client.ClientBuilder;
+import javax.ws.rs.client.Client;
+import javax.ws.rs.core.GenericType;
+import javax.ws.rs.core.MediaType;
+
+import java.io.InputStream;
+import java.net.URL;
+import java.security.KeyStore;
+import java.util.Random;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import javax.net.ssl.HostnameVerifier;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLSession;
+import javax.net.ssl.TrustManagerFactory;
+
+/**
+ * Jersey client seems to be not thread-safe:
+ * When the first GET request is in progress,
+ * all parallel requests from other Jersey client instances fail
+ * with SSLHandshakeException: PKIX path building failed.
+ * <p>
+ * Once the first GET request is completed,
+ * all subsequent requests work without error.
+ * <p>
+ * BUG 5749
+ */
+public class ConcurrentHttpsUrlConnectionTest {
+    private static int THREAD_NUMBER = 5;
+
+    private static volatile int responseCounter = 0;
+
+    private static SSLContext createContext() throws Exception {
+        URL url = ConcurrentHttpsUrlConnectionTest.class.getResource("keystore.jks");
+        KeyStore keyStore = KeyStore.getInstance("JKS");
+        try (InputStream is = url.openStream()) {
+            keyStore.load(is, "password".toCharArray());
+        }
+        KeyManagerFactory kmf = KeyManagerFactory.getInstance("SunX509");
+        kmf.init(keyStore, "password".toCharArray());
+        TrustManagerFactory tmf = TrustManagerFactory.getInstance("PKIX");
+        tmf.init(keyStore);
+        SSLContext context = SSLContext.getInstance("TLS");
+        context.init(kmf.getKeyManagers(), tmf.getTrustManagers(), null);
+        return context;
+    }
+
+    @Test
+    public void testSSLConnections() throws Exception {
+        if (THREAD_NUMBER == 1) {
+            System.out.println("\nThis is the working case (THREAD_NUMBER==1). Set THREAD_NUMBER > 1 to reproduce the error! \n");
+        }
+
+        final HttpsServer server = new HttpsServer(createContext());
+        Executors.newFixedThreadPool(1).submit(server);
+
+        // set THREAD_NUMBER > 1 to reproduce an issue
+        ExecutorService executorService2clients = Executors.newFixedThreadPool(THREAD_NUMBER);
+
+        final ClientBuilder builder = ClientBuilder.newBuilder().sslContext(createContext())
+                .hostnameVerifier(new HostnameVerifier() {
+                    public boolean verify(String arg0, SSLSession arg1) {
+                        return true;
+                    }
+                });
+
+        AtomicInteger counter = new AtomicInteger(0);
+
+        for (int i = 0; i < THREAD_NUMBER; i++) {
+            executorService2clients.submit(new Runnable() {
+                @Override
+                public void run() {
+                    try {
+                        Client client = builder.build();
+                        String ret = client.target("https://127.0.0.1:" + server.getPort() + "/" + new Random().nextInt())
+                                .request(MediaType.TEXT_HTML)
+                                .get(new GenericType<String>() {
+                                });
+                        System.out.print(++responseCounter + ". Server returned: " + ret);
+                    } catch (Exception e) {
+                        //get an exception here, if jersey lib is buggy and THREAD_NUMBER > 1:
+                        //jakarta.ws.rs.ProcessingException: javax.net.ssl.SSLHandshakeException: PKIX path building failed:
+                        e.printStackTrace();
+                    } finally {
+                        System.out.println(counter.incrementAndGet());
+                    }
+                }
+            });
+        }
+
+        while (counter.get() != THREAD_NUMBER) {
+            Thread.sleep(100L);
+        }
+        server.stop();
+    }
+}

@@ -0,0 +1,87 @@
+/*
+ * Copyright (c) 2024 Oracle and/or its affiliates. All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0, which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the
+ * Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
+ * version 2 with the GNU Classpath Exception, which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ */
+
+package org.glassfish.jersey.tests.e2e.tls.connector;
+
+import java.io.BufferedInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.PrintWriter;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLServerSocket;
+import javax.net.ssl.SSLSocket;
+
+class HttpsServer implements Runnable {
+    private final SSLServerSocket sslServerSocket;
+    private boolean closed = false;
+
+    public HttpsServer(SSLContext context) throws Exception {
+        sslServerSocket = (SSLServerSocket) context.getServerSocketFactory().createServerSocket(0);
+    }
+
+    public int getPort() {
+        return sslServerSocket.getLocalPort();
+    }
+
+    @Override
+    public void run() {
+        System.out.printf("Server started on port %d%n", getPort());
+        while (!closed) {
+            SSLSocket s;
+            try {
+                s = (SSLSocket) sslServerSocket.accept();
+            } catch (IOException e2) {
+                s = null;
+            }
+            final SSLSocket socket = s;
+            new Thread(new Runnable() {
+                public void run() {
+                    try {
+                        if (socket != null) {
+                            InputStream is = new BufferedInputStream(socket.getInputStream());
+                            byte[] data = new byte[2048];
+                            int len = is.read(data);
+                            if (len <= 0) {
+                                throw new IOException("no data received");
+                            }
+                            //System.out.printf("Server received: %s\n", new String(data, 0, len));
+                            PrintWriter writer = new PrintWriter(socket.getOutputStream());
+                            writer.println("HTTP/1.1 200 OK");
+                            writer.println("Content-Type: text/html");
+                            writer.println();
+                            writer.println("Hello from server!");
+                            writer.flush();
+                            writer.close();
+                            socket.close();
+                        }
+                    } catch (Exception e1) {
+                        e1.printStackTrace();
+                    }
+                }
+            }).start();
+        }
+    }
+
+    void stop() {
+        try {
+            closed = true;
+            sslServerSocket.close();
+        } catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+    }
+}

@@ -21,6 +21,9 @@
 import java.net.Proxy;
 import java.net.URL;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
 import java.util.logging.Logger;
 
 import javax.ws.rs.client.Client;
@@ -295,9 +298,26 @@ default HttpURLConnection getConnection(URL url, Proxy proxy) throws IOException
 
     private static class DefaultConnectionFactory implements ConnectionFactory {
 
+        private final ConcurrentHashMap<URL, Lock> locks = new ConcurrentHashMap<>();
+
         @Override
         public HttpURLConnection getConnection(final URL url) throws IOException {
-            return (HttpURLConnection) url.openConnection();
+            return connect(url, null);
+        }
+
+        @Override
+        public HttpURLConnection getConnection(URL url, Proxy proxy) throws IOException {
+            return connect(url, proxy);
+        }
+
+        private HttpURLConnection connect(URL url, Proxy proxy) throws IOException {
+            Lock lock = locks.computeIfAbsent(url, u -> new ReentrantLock());
+            lock.lock();
+            try {
+                return (proxy == null) ? (HttpURLConnection) url.openConnection() : (HttpURLConnection) url.openConnection(proxy);
+            } finally {
+                lock.unlock();
+            }
         }
     }