From d946f95c9c06f27de8aac6ecc3f5d49eabe6e030 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Sat, 2 Sep 2017 13:20:48 +0200 Subject: Handle SSL handshake failures in TransportHttp When a https connection could not be established because the SSL handshake was unsuccessful, TransportHttp would unconditionally throw a TransportException. Other https clients like web browsers or also some SVN clients handle this more gracefully. If there's a problem with the server certificate, they inform the user and give him a possibility to connect to the server all the same. In git, this would correspond to dynamically setting http.sslVerify to false for the server. Implement this using the CredentialsProvider to inform and ask the user. We offer three choices: 1. skip SSL verification for the current git operation, or 2. skip SSL verification for the server always from now on for requests originating from the current repository, or 3. always skip SSL verification for the server from now on. For (1), we just suppress SSL verification for the current instance of TransportHttp. For (2), we store a http..sslVerify = false setting for the original URI in the repo config. For (3), we store the http..sslVerify setting in the git user config. Adapt the SmartClientSmartServerSslTest such that it uses this mechanism instead of setting http.sslVerify up front. Improve SimpleHttpServer to enable setting it up also with HTTPS support in anticipation of an EGit SWTbot UI test verifying that cloning via HTTPS from a server that has a certificate that doesn't validate pops up the correct dialog, and that cloning subsequently proceeds successfully if the user decides to skip SSL verification. Bug: 374703 Change-Id: Ie1abada9a3d389ad4d8d52c2d5265d2764e3fb0e Signed-off-by: Thomas Wolf --- .../http/test/SmartClientSmartServerSslTest.java | 82 ++++++++++++++++++---- 1 file changed, 69 insertions(+), 13 deletions(-) (limited to 'org.eclipse.jgit.http.test/tst') diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerSslTest.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerSslTest.java index 47a84354fb..7deb0d85a0 100644 --- a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerSslTest.java +++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerSslTest.java @@ -68,6 +68,7 @@ import org.eclipse.jetty.servlet.FilterHolder; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jgit.errors.TransportException; +import org.eclipse.jgit.errors.UnsupportedCredentialItem; import org.eclipse.jgit.http.server.GitServlet; import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.junit.http.AccessEvent; @@ -78,16 +79,16 @@ import org.eclipse.jgit.lib.NullProgressMonitor; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevBlob; import org.eclipse.jgit.revwalk.RevCommit; -import org.eclipse.jgit.storage.file.FileBasedConfig; +import org.eclipse.jgit.transport.CredentialItem; +import org.eclipse.jgit.transport.CredentialsProvider; import org.eclipse.jgit.transport.HttpTransport; import org.eclipse.jgit.transport.Transport; import org.eclipse.jgit.transport.URIish; +import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider; import org.eclipse.jgit.transport.http.HttpConnectionFactory; import org.eclipse.jgit.transport.http.JDKHttpConnectionFactory; import org.eclipse.jgit.transport.http.apache.HttpClientConnectionFactory; -import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.HttpSupport; -import org.eclipse.jgit.util.SystemReader; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -97,6 +98,52 @@ import org.junit.runners.Parameterized.Parameters; @RunWith(Parameterized.class) public class SmartClientSmartServerSslTest extends HttpTestCase { + // We run these tests with a server on localhost with a self-signed + // certificate. We don't do authentication tests here, so there's no need + // for username and password. + // + // But the server certificate will not validate. We know that Transport will + // ask whether we trust the server all the same. This credentials provider + // blindly trusts the self-signed certificate by answering "Yes" to all + // questions. + private CredentialsProvider testCredentials = new CredentialsProvider() { + + @Override + public boolean isInteractive() { + return false; + } + + @Override + public boolean supports(CredentialItem... items) { + for (CredentialItem item : items) { + if (item instanceof CredentialItem.InformationalMessage) { + continue; + } + if (item instanceof CredentialItem.YesNoType) { + continue; + } + return false; + } + return true; + } + + @Override + public boolean get(URIish uri, CredentialItem... items) + throws UnsupportedCredentialItem { + for (CredentialItem item : items) { + if (item instanceof CredentialItem.InformationalMessage) { + continue; + } + if (item instanceof CredentialItem.YesNoType) { + ((CredentialItem.YesNoType) item).setValue(true); + continue; + } + return false; + } + return true; + } + }; + private URIish remoteURI; private URIish secureURI; @@ -150,16 +197,6 @@ public class SmartClientSmartServerSslTest extends HttpTestCase { src.update(master, B); src.update("refs/garbage/a/very/long/ref/name/to/compress", B); - - FileBasedConfig userConfig = SystemReader.getInstance() - .openUserConfig(null, FS.DETECTED); - userConfig.setBoolean("http", - "https://" + secureURI.getHost() + ':' + server.getSecurePort(), - "sslVerify", false); - userConfig.setBoolean("http", - "http://" + remoteURI.getHost() + ':' + server.getPort(), - "sslVerify", false); - userConfig.save(); } private ServletContextHandler addNormalContext(GitServlet gs, TestRepository src, String srcName) { @@ -241,6 +278,7 @@ public class SmartClientSmartServerSslTest extends HttpTestCase { assertFalse(dst.hasObject(A_txt)); try (Transport t = Transport.open(dst, secureURI)) { + t.setCredentialsProvider(testCredentials); t.fetch(NullProgressMonitor.INSTANCE, mirror(master)); } assertTrue(dst.hasObject(A_txt)); @@ -258,6 +296,7 @@ public class SmartClientSmartServerSslTest extends HttpTestCase { URIish cloneFrom = extendPath(remoteURI, "/https"); try (Transport t = Transport.open(dst, cloneFrom)) { + t.setCredentialsProvider(testCredentials); t.fetch(NullProgressMonitor.INSTANCE, mirror(master)); } assertTrue(dst.hasObject(A_txt)); @@ -275,6 +314,7 @@ public class SmartClientSmartServerSslTest extends HttpTestCase { URIish cloneFrom = extendPath(secureURI, "/back"); try (Transport t = Transport.open(dst, cloneFrom)) { + t.setCredentialsProvider(testCredentials); t.fetch(NullProgressMonitor.INSTANCE, mirror(master)); fail("Should have failed (redirect from https to http)"); } catch (TransportException e) { @@ -282,4 +322,20 @@ public class SmartClientSmartServerSslTest extends HttpTestCase { } } + @Test + public void testInitialClone_SslFailure() throws Exception { + Repository dst = createBareRepository(); + assertFalse(dst.hasObject(A_txt)); + + try (Transport t = Transport.open(dst, secureURI)) { + // Set a credentials provider that doesn't handle questions + t.setCredentialsProvider( + new UsernamePasswordCredentialsProvider("any", "anypwd")); + t.fetch(NullProgressMonitor.INSTANCE, mirror(master)); + fail("Should have failed (SSL certificate not trusted)"); + } catch (TransportException e) { + assertTrue(e.getMessage().contains("Secure connection")); + } + } + } -- cgit v1.2.3