TransportHttp: shared SSLContext during fetch or push

TransportHttp makes several HTTP requests. The SSLContext and socket
factory must be shared over these requests, otherwise authentication
information may not be propagated correctly from one request to the
next. This is important for authentication mechanisms that rely on
client-side state, like NEGOTIATE (either NTLM, if the underlying HTTP
library supports it, or Kerberos). In particular, SPNEGO cannot
authenticate on a POST request; the authentication must come from the
initial GET request, which implies that the POST request must use the
same SSLContext and socket factory that was used for the GET.

Change the way HTTPS connections are configured. Introduce the concept
of a GitSession, which is a client-side HTTP session over several HTTPS
requests. TransportHttp creates such a session and uses it to configure
all HTTP requests during that session (fetch or push). This gives a way
to abstract away the differences between JDK and Apache HTTP connections
and to configure SSL setup outside.

A GitSession can maintain state and thus give all HTTP requests in a
session the same socket factory.

Introduce an extension interface HttpConnectionFactory2 that adds a
method to obtain a new GitSession. Implement this for both existing
HTTP connection factories. Change TransportHttp to use the new
GitSession to configure HTTP connections.

The old methods for disabling SSL verification still exist to support
possibly external connection and connection factory implementations
that do not make use of the new GitSession yet.

Bug: 535850
Change-Id: Iedf67464e4e353c1883447c13c86b5a838e678f1
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

3 files changed, 107 insertions, 19 deletions

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 ed05f0a8d8..90348f54b9 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
@@ -57,9 +57,7 @@ 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;
@@ -103,7 +101,11 @@ public class HttpClientConnection implements HttpConnection {
 
 	private HostnameVerifier hostnameverifier;
 
-	SSLContext ctx;
+	private SSLContext ctx;
+
+	private SSLConnectionSocketFactory socketFactory;
+
+	private boolean usePooling = true;
 
 	private HttpClient getClient() {
 		if (client == null) {
@@ -125,11 +127,18 @@ public class HttpClientConnection implements HttpConnection {
 				configBuilder
 						.setRedirectsEnabled(followRedirects.booleanValue());
 			}
-			SSLConnectionSocketFactory sslConnectionFactory = getSSLSocketFactory();
+			boolean pooled = true;
+			SSLConnectionSocketFactory sslConnectionFactory;
+			if (socketFactory != null) {
+				pooled = usePooling;
+				sslConnectionFactory = socketFactory;
+			} else {
+				// Legacy implementation.
+				pooled = (hostnameverifier == null);
+				sslConnectionFactory = getSSLSocketFactory();
+			}
 			clientBuilder.setSSLSocketFactory(sslConnectionFactory);
-			if (hostnameverifier != null) {
-				// Using a custom verifier: we don't want pooled connections
-				// with this.
+			if (!pooled) {
 				Registry<ConnectionSocketFactory> registry = RegistryBuilder
 						.<ConnectionSocketFactory> create()
 						.register("https", sslConnectionFactory)
@@ -147,14 +156,19 @@ public class HttpClientConnection implements HttpConnection {
 		return client;
 	}
 
+	void setSSLSocketFactory(@NonNull SSLConnectionSocketFactory factory,
+			boolean isDefault) {
+		socketFactory = factory;
+		usePooling = isDefault;
+	}
+
 	private SSLConnectionSocketFactory getSSLSocketFactory() {
 		HostnameVerifier verifier = hostnameverifier;
 		SSLContext context;
 		if (verifier == null) {
 			// Use defaults
-			context = SSLContexts.createDefault();
-			verifier = new DefaultHostnameVerifier(
-					PublicSuffixMatcherLoader.getDefault());
+			context = SSLContexts.createSystemDefault();
+			verifier = SSLConnectionSocketFactory.getDefaultHostnameVerifier();
 		} else {
 			// Using a custom verifier. Attention: configure() must have been
 			// called already, otherwise one gets a "context not initialized"
diff --git a/org.eclipse.jgit.http.apache/src/org/eclipse/jgit/transport/http/apache/HttpClientConnectionFactory.java b/org.eclipse.jgit.http.apache/src/org/eclipse/jgit/transport/http/apache/HttpClientConnectionFactory.java
index 3c05cdef8c..4de3e470f6 100644
--- a/org.eclipse.jgit.http.apache/src/org/eclipse/jgit/transport/http/apache/HttpClientConnectionFactory.java
+++ b/org.eclipse.jgit.http.apache/src/org/eclipse/jgit/transport/http/apache/HttpClientConnectionFactory.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013 Christian Halstrick <christian.halstrick@sap.com> and others
+ * Copyright (C) 2013, 2020 Christian Halstrick <christian.halstrick@sap.com> and others
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Distribution License v. 1.0 which is available at
@@ -12,27 +12,100 @@ package org.eclipse.jgit.transport.http.apache;
 import java.io.IOException;
 import java.net.Proxy;
 import java.net.URL;
+import java.security.GeneralSecurityException;
+import java.text.MessageFormat;
 
+import javax.net.ssl.HostnameVerifier;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLSocket;
+import javax.net.ssl.TrustManager;
+
+import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
 import org.eclipse.jgit.transport.http.HttpConnection;
-import org.eclipse.jgit.transport.http.HttpConnectionFactory;
+import org.eclipse.jgit.transport.http.HttpConnectionFactory2;
+import org.eclipse.jgit.transport.http.NoCheckX509TrustManager;
+import org.eclipse.jgit.transport.http.apache.internal.HttpApacheText;
+import org.eclipse.jgit.util.HttpSupport;
 
 /**
- * A factory returning instances of
- * {@link org.eclipse.jgit.transport.http.apache.HttpClientConnection}
+ * A factory returning instances of {@link HttpClientConnection}.
  *
  * @since 3.3
  */
-public class HttpClientConnectionFactory implements HttpConnectionFactory {
-	/** {@inheritDoc} */
+public class HttpClientConnectionFactory implements HttpConnectionFactory2 {
+
 	@Override
 	public HttpConnection create(URL url) throws IOException {
 		return new HttpClientConnection(url.toString());
 	}
 
-	/** {@inheritDoc} */
 	@Override
-	public HttpConnection create(URL url, Proxy proxy)
-			throws IOException {
+	public HttpConnection create(URL url, Proxy proxy) throws IOException {
 		return new HttpClientConnection(url.toString(), proxy);
 	}
+
+	@Override
+	public GitSession newSession() {
+		return new HttpClientSession();
+	}
+
+	private static class HttpClientSession implements GitSession {
+
+		private SSLContext securityContext;
+
+		private SSLConnectionSocketFactory socketFactory;
+
+		private boolean isDefault;
+
+		@Override
+		public HttpClientConnection configure(HttpConnection connection,
+				boolean sslVerify)
+				throws IOException, GeneralSecurityException {
+			if (!(connection instanceof HttpClientConnection)) {
+				throw new IllegalArgumentException(MessageFormat.format(
+						HttpApacheText.get().httpWrongConnectionType,
+						HttpClientConnection.class.getName(),
+						connection.getClass().getName()));
+			}
+			HttpClientConnection conn = (HttpClientConnection) connection;
+			String scheme = conn.getURL().getProtocol();
+			if (!"https".equals(scheme)) { //$NON-NLS-1$
+				return conn;
+			}
+			if (securityContext == null || isDefault != sslVerify) {
+				isDefault = sslVerify;
+				HostnameVerifier verifier;
+				if (sslVerify) {
+					securityContext = SSLContext.getDefault();
+					verifier = SSLConnectionSocketFactory
+							.getDefaultHostnameVerifier();
+				} else {
+					securityContext = SSLContext.getInstance("TLS");
+					TrustManager[] trustAllCerts = {
+							new NoCheckX509TrustManager() };
+					securityContext.init(null, trustAllCerts, null);
+					verifier = (name, session) -> true;
+				}
+				socketFactory = new SSLConnectionSocketFactory(securityContext,
+						verifier) {
+
+					@Override
+					protected void prepareSocket(SSLSocket socket)
+							throws IOException {
+						super.prepareSocket(socket);
+						HttpSupport.configureTLS(socket);
+					}
+				};
+			}
+			conn.setSSLSocketFactory(socketFactory, isDefault);
+			return conn;
+		}
+
+		@Override
+		public void close() {
+			securityContext = null;
+			socketFactory = null;
+		}
+
+	}
 }
diff --git a/org.eclipse.jgit.http.apache/src/org/eclipse/jgit/transport/http/apache/internal/HttpApacheText.java b/org.eclipse.jgit.http.apache/src/org/eclipse/jgit/transport/http/apache/internal/HttpApacheText.java
index 907ab98cc8..677d7d792b 100644
--- a/org.eclipse.jgit.http.apache/src/org/eclipse/jgit/transport/http/apache/internal/HttpApacheText.java
+++ b/org.eclipse.jgit.http.apache/src/org/eclipse/jgit/transport/http/apache/internal/HttpApacheText.java
@@ -27,5 +27,6 @@ public class HttpApacheText extends TranslationBundle {
 	}
 
 	// @formatter:off
+	/***/ public String httpWrongConnectionType;
 	/***/ public String unexpectedSSLContextException;
 }