TLS support on IBM JDKs

SSLContext.getInstance("TLS") by default behaves differently on IBM
JDK than on Oracle or OpenJDK.[1] On IBM JDK one gets sockets that
have only TLSv1 enabled, which makes HTTPS connections fail since most
servers refuse this old protocol version. On Oracle JDK/OpenJDK, one
gets sockets with all available protocol versions enabled.

Explicitly enable all available TLS protocol versions to make
HTTPS connections work also on IBM JDK.

[1] https://www.ibm.com/support/knowledgecenter/en/SSYKE2_8.0.0/com.ibm.java.security.component.80.doc/security-component/jsse2Docs/matchsslcontext_tls.html#matchsslcontext_tls

Bug: 558709
Change-Id: I5ffc57a78e67a6239b9dad54840a49a8ed28930a
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

7 files changed, 253 insertions, 7 deletions

diff --git a/org.eclipse.jgit.http.apache/META-INF/MANIFEST.MF b/org.eclipse.jgit.http.apache/META-INF/MANIFEST.MF
index f19031f896..e68959ae52 100644
--- a/org.eclipse.jgit.http.apache/META-INF/MANIFEST.MF
+++ b/org.eclipse.jgit.http.apache/META-INF/MANIFEST.MF
@@ -19,10 +19,12 @@ Import-Package: org.apache.http;version="[4.3.0,5.0.0)",
  org.apache.http.conn.scheme;version="[4.3.0,5.0.0)",
  org.apache.http.conn.socket;version="[4.3.0,5.0.0)",
  org.apache.http.conn.ssl;version="[4.3.0,5.0.0)",
+ org.apache.http.conn.util;version="[4.3.0,5.0.0)",
  org.apache.http.entity;version="[4.3.0,5.0.0)",
  org.apache.http.impl.client;version="[4.3.0,5.0.0)",
  org.apache.http.impl.conn;version="[4.3.0,5.0.0)",
  org.apache.http.params;version="[4.3.0,5.0.0)",
+ org.apache.http.ssl;version="[4.3.0,5.0.0)",
  org.eclipse.jgit.annotations;version="[5.7.0,5.8.0)",
  org.eclipse.jgit.nls;version="[5.7.0,5.8.0)",
  org.eclipse.jgit.transport.http;version="[5.7.0,5.8.0)",
diff --git a/org.eclipse.jgit.http.apache/src/org/eclipse/jgit/transport/http/apache/HttpClientConnection.java b/org.eclipse.jgit.http.apache/src/org/eclipse/jgit/transport/http/apache/HttpClientConnection.java
index 9d9e2f882d..61afaaef50 100644
--- a/org.eclipse.jgit.http.apache/src/org/eclipse/jgit/transport/http/apache/HttpClientConnection.java
+++ b/org.eclipse.jgit.http.apache/src/org/eclipse/jgit/transport/http/apache/HttpClientConnection.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013 Christian Halstrick <christian.halstrick@sap.com>
+ * Copyright (C) 2013, 2020 Christian Halstrick <christian.halstrick@sap.com>
  * and other copyright owners as documented in the project's IP log.
  *
  * This program and the accompanying materials are made available
@@ -69,6 +69,7 @@ import java.util.stream.Collectors;
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.KeyManager;
 import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLSocket;
 import javax.net.ssl.TrustManager;
 
 import org.apache.http.Header;
@@ -89,14 +90,18 @@ import org.apache.http.config.Registry;
 import org.apache.http.config.RegistryBuilder;
 import org.apache.http.conn.socket.ConnectionSocketFactory;
 import org.apache.http.conn.socket.PlainConnectionSocketFactory;
+import org.apache.http.conn.ssl.DefaultHostnameVerifier;
 import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
+import org.apache.http.conn.util.PublicSuffixMatcherLoader;
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.http.impl.client.HttpClients;
 import org.apache.http.impl.client.SystemDefaultCredentialsProvider;
 import org.apache.http.impl.conn.BasicHttpClientConnectionManager;
+import org.apache.http.ssl.SSLContexts;
 import org.eclipse.jgit.annotations.NonNull;
 import org.eclipse.jgit.transport.http.HttpConnection;
 import org.eclipse.jgit.transport.http.apache.internal.HttpApacheText;
+import org.eclipse.jgit.util.HttpSupport;
 import org.eclipse.jgit.util.TemporaryBuffer;
 import org.eclipse.jgit.util.TemporaryBuffer.LocalFile;
 
@@ -153,10 +158,11 @@ public class HttpClientConnection implements HttpConnection {
 				configBuilder
 						.setRedirectsEnabled(followRedirects.booleanValue());
 			}
+			SSLConnectionSocketFactory sslConnectionFactory = getSSLSocketFactory();
+			clientBuilder.setSSLSocketFactory(sslConnectionFactory);
 			if (hostnameverifier != null) {
-				SSLConnectionSocketFactory sslConnectionFactory = new SSLConnectionSocketFactory(
-						getSSLContext(), hostnameverifier);
-				clientBuilder.setSSLSocketFactory(sslConnectionFactory);
+				// Using a custom verifier: we don't want pooled connections
+				// with this.
 				Registry<ConnectionSocketFactory> registry = RegistryBuilder
 						.<ConnectionSocketFactory> create()
 						.register("https", sslConnectionFactory)
@@ -174,6 +180,32 @@ public class HttpClientConnection implements HttpConnection {
 		return client;
 	}
 
+	private SSLConnectionSocketFactory getSSLSocketFactory() {
+		HostnameVerifier verifier = hostnameverifier;
+		SSLContext context;
+		if (verifier == null) {
+			// Use defaults
+			context = SSLContexts.createDefault();
+			verifier = new DefaultHostnameVerifier(
+					PublicSuffixMatcherLoader.getDefault());
+		} else {
+			// Using a custom verifier. Attention: configure() must have been
+			// called already, otherwise one gets a "context not initialized"
+			// exception. In JGit this branch is reached only when hostname
+			// verification is switched off, and JGit _does_ call configure()
+			// before we get here.
+			context = getSSLContext();
+		}
+		return new SSLConnectionSocketFactory(context, verifier) {
+
+			@Override
+			protected void prepareSocket(SSLSocket socket) throws IOException {
+				super.prepareSocket(socket);
+				HttpSupport.configureTLS(socket);
+			}
+		};
+	}
+
 	private SSLContext getSSLContext() {
 		if (ctx == null) {
 			try {
diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index c80d616859..fb1bd9f3c8 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -282,6 +282,7 @@ expectedReceivedContentType=expected Content-Type {0}; received Content-Type {1}
 expectedReportForRefNotReceived={0}: expected report for ref {1} not received
 failedAtomicFileCreation=Atomic file creation failed, number of hard links to file {0} was not 2 but {1}
 failedCreateLockFile=Creating lock file {} failed
+failedReadHttpsProtocols=Failed to read system property https.protocols, assuming it is not set
 failedToConvert=Failed to convert rest: %s
 failedToDetermineFilterDefinition=An exception occurred while determining filter definitions
 failedUpdatingRefs=failed updating refs
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index e8ee8b4050..5735060948 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -343,6 +343,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String expectedReportForRefNotReceived;
 	/***/ public String failedAtomicFileCreation;
 	/***/ public String failedCreateLockFile;
+	/***/ public String failedReadHttpsProtocols;
 	/***/ public String failedToDetermineFilterDefinition;
 	/***/ public String failedToConvert;
 	/***/ public String failedUpdatingRefs;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/JDKHttpConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/JDKHttpConnection.java
index 734b549294..fa8742fd29 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/JDKHttpConnection.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/JDKHttpConnection.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013 Christian Halstrick <christian.halstrick@sap.com>
+ * Copyright (C) 2013, 2020 Christian Halstrick <christian.halstrick@sap.com>
  * and other copyright owners as documented in the project's IP log.
  *
  * This program and the accompanying materials are made available
@@ -61,9 +61,12 @@ import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.HttpsURLConnection;
 import javax.net.ssl.KeyManager;
 import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLSocket;
 import javax.net.ssl.TrustManager;
 
 import org.eclipse.jgit.annotations.NonNull;
+import org.eclipse.jgit.transport.internal.DelegatingSSLSocketFactory;
+import org.eclipse.jgit.util.HttpSupport;
 /**
  * A {@link org.eclipse.jgit.transport.http.HttpConnection} which simply
  * delegates every call to a {@link java.net.HttpURLConnection}. This is the
@@ -265,7 +268,15 @@ public class JDKHttpConnection implements HttpConnection {
 			KeyManagementException {
 		SSLContext ctx = SSLContext.getInstance("TLS"); //$NON-NLS-1$
 		ctx.init(km, tm, random);
-		((HttpsURLConnection) wrappedUrlConnection).setSSLSocketFactory(ctx
-				.getSocketFactory());
+		((HttpsURLConnection) wrappedUrlConnection).setSSLSocketFactory(
+				new DelegatingSSLSocketFactory(ctx.getSocketFactory()) {
+
+					@Override
+					protected void configure(SSLSocket socket)
+							throws IOException {
+						HttpSupport.configureTLS(socket);
+					}
+				});
 	}
+
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/internal/DelegatingSSLSocketFactory.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/internal/DelegatingSSLSocketFactory.java
new file mode 100644
index 0000000000..d25ecd459d
--- /dev/null
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/internal/DelegatingSSLSocketFactory.java
@@ -0,0 +1,101 @@
+/*
+ * Copyright (c) 2020 Thomas Wolf <thomas.wolf@paranor.ch>
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Distribution License v. 1.0 which is available at
+ * https://www.eclipse.org/org/documents/edl-v10.php.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+package org.eclipse.jgit.transport.internal;
+
+import java.io.IOException;
+import java.net.InetAddress;
+import java.net.Socket;
+
+import javax.net.ssl.SSLSocket;
+import javax.net.ssl.SSLSocketFactory;
+
+/**
+ * An {@link SSLSocketFactory} that delegates to another factory and allows
+ * configuring the created socket via {@link #configure(SSLSocket)} before it is
+ * returned.
+ */
+public abstract class DelegatingSSLSocketFactory extends SSLSocketFactory {
+
+	private final SSLSocketFactory delegate;
+
+	/**
+	 * Creates a new {@link DelegatingSSLSocketFactory} based on the given
+	 * delegate.
+	 *
+	 * @param delegate
+	 *            {@link SSLSocketFactory} to delegate to
+	 */
+	public DelegatingSSLSocketFactory(SSLSocketFactory delegate) {
+		this.delegate = delegate;
+	}
+
+	@Override
+	public SSLSocket createSocket() throws IOException {
+		return prepare(delegate.createSocket());
+	}
+
+	@Override
+	public SSLSocket createSocket(String host, int port) throws IOException {
+		return prepare(delegate.createSocket(host, port));
+	}
+
+	@Override
+	public SSLSocket createSocket(String host, int port,
+			InetAddress localAddress, int localPort) throws IOException {
+		return prepare(
+				delegate.createSocket(host, port, localAddress, localPort));
+	}
+
+	@Override
+	public SSLSocket createSocket(InetAddress host, int port)
+			throws IOException {
+		return prepare(delegate.createSocket(host, port));
+	}
+
+	@Override
+	public SSLSocket createSocket(InetAddress host, int port,
+			InetAddress localAddress, int localPort) throws IOException {
+		return prepare(
+				delegate.createSocket(host, port, localAddress, localPort));
+	}
+
+	@Override
+	public SSLSocket createSocket(Socket socket, String host, int port,
+			boolean autoClose) throws IOException {
+		return prepare(delegate.createSocket(socket, host, port, autoClose));
+	}
+
+	@Override
+	public String[] getDefaultCipherSuites() {
+		return delegate.getDefaultCipherSuites();
+	}
+
+	@Override
+	public String[] getSupportedCipherSuites() {
+		return delegate.getSupportedCipherSuites();
+	}
+
+	private SSLSocket prepare(Socket socket) throws IOException {
+		SSLSocket sslSocket = (SSLSocket) socket;
+		configure(sslSocket);
+		return sslSocket;
+	}
+
+	/**
+	 * Configure the newly created socket.
+	 *
+	 * @param socket
+	 *            to configure
+	 * @throws IOException
+	 *             if the socket cannot be configured
+	 */
+	protected abstract void configure(SSLSocket socket) throws IOException;
+
+}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/HttpSupport.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/HttpSupport.java
index d897255062..9be04143b1 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/HttpSupport.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/HttpSupport.java
@@ -59,19 +59,29 @@ import java.security.KeyManagementException;
 import java.security.NoSuchAlgorithmException;
 import java.security.cert.X509Certificate;
 import java.text.MessageFormat;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashSet;
+import java.util.Set;
 
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLSession;
+import javax.net.ssl.SSLSocket;
 import javax.net.ssl.TrustManager;
 import javax.net.ssl.X509TrustManager;
 
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.transport.http.HttpConnection;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Extra utilities to support usage of HTTP.
  */
 public class HttpSupport {
+	private final static Logger LOG = LoggerFactory
+			.getLogger(HttpSupport.class);
+
 	/** The {@code GET} HTTP method. */
 	public static final String METHOD_GET = "GET"; //$NON-NLS-1$
 
@@ -191,6 +201,8 @@ public class HttpSupport {
 	 */
 	public static final String HDR_SET_COOKIE2 = "Set-Cookie2"; //$NON-NLS-1$
 
+	private static Set<String> configuredHttpsProtocols;
+
 	/**
 	 * URL encode a value string into an output buffer.
 	 *
@@ -359,6 +371,92 @@ public class HttpSupport {
 		}
 	}
 
+	/**
+	 * Enables all supported TLS protocol versions on the socket given. If
+	 * system property "https.protocols" is set, only protocols specified there
+	 * are enabled.
+	 * <p>
+	 * This is primarily a mechanism to deal with using TLS on IBM JDK. IBM JDK
+	 * returns sockets that support all TLS protocol versions but have only the
+	 * one specified in the context enabled. Oracle or OpenJDK return sockets
+	 * that have all available protocols enabled already, up to the one
+	 * specified.
+	 * <p>
+	 * <table>
+	 * <tr>
+	 * <td>SSLContext.getInstance()</td>
+	 * <td>OpenJDK</td>
+	 * <td>IDM JDK</td>
+	 * </tr>
+	 * <tr>
+	 * <td>"TLS"</td>
+	 * <td>Supported: TLSv1, TLSV1.1, TLSv1.2 (+ TLSv1.3)<br />
+	 * Enabled: TLSv1, TLSV1.1, TLSv1.2 (+ TLSv1.3)</td>
+	 * <td>Supported: TLSv1, TLSV1.1, TLSv1.2<br />
+	 * Enabled: TLSv1</td>
+	 * </tr>
+	 * <tr>
+	 * <td>"TLSv1.2"</td>
+	 * <td>Supported: TLSv1, TLSV1.1, TLSv1.2<br />
+	 * Enabled: TLSv1, TLSV1.1, TLSv1.2</td>
+	 * <td>Supported: TLSv1, TLSV1.1, TLSv1.2<br />
+	 * Enabled: TLSv1.2</td>
+	 * </tr>
+	 * </table>
+	 *
+	 * @param socket
+	 *            to configure
+	 * @see <a href=
+	 *      "https://www.ibm.com/support/knowledgecenter/en/SSYKE2_8.0.0/com.ibm.java.security.component.80.doc/security-component/jsse2Docs/matchsslcontext_tls.html">Behavior
+	 *      of SSLContext.getInstance("TLS") on IBM JDK</a>
+	 * @see <a href=
+	 *      "https://docs.oracle.com/javase/8/docs/technotes/guides/security/jsse/JSSERefGuide.html#InstallationAndCustomization">Customizing
+	 *      JSSE about https.protocols</a>
+	 * @since 5.7
+	 */
+	public static void configureTLS(SSLSocket socket) {
+		// 1. Enable all available TLS protocol versions
+		Set<String> enabled = new LinkedHashSet<>(
+				Arrays.asList(socket.getEnabledProtocols()));
+		for (String s : socket.getSupportedProtocols()) {
+			if (s.startsWith("TLS")) { //$NON-NLS-1$
+				enabled.add(s);
+			}
+		}
+		// 2. Respect the https.protocols system property
+		Set<String> configured = getConfiguredProtocols();
+		if (!configured.isEmpty()) {
+			enabled.retainAll(configured);
+		}
+		if (!enabled.isEmpty()) {
+			socket.setEnabledProtocols(enabled.toArray(new String[0]));
+		}
+	}
+
+	private static Set<String> getConfiguredProtocols() {
+		Set<String> result = configuredHttpsProtocols;
+		if (result == null) {
+			String configured = getProperty("https.protocols"); //$NON-NLS-1$
+			if (StringUtils.isEmptyOrNull(configured)) {
+				result = Collections.emptySet();
+			} else {
+				result = new LinkedHashSet<>(
+						Arrays.asList(configured.split("\\s*,\\s*"))); //$NON-NLS-1$
+			}
+			configuredHttpsProtocols = result;
+		}
+		return result;
+	}
+
+	private static String getProperty(String property) {
+		try {
+			return SystemReader.getInstance().getProperty(property);
+		} catch (SecurityException e) {
+			LOG.warn(JGitText.get().failedReadHttpsProtocols, e);
+			return null;
+		}
+	}
+
 	private HttpSupport() {
 		// Utility class only.
 	}