Browse Source

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.<uri>.sslVerify = false setting for the
original URI in the repo config.

For (3), we store the http.<uri>.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 <thomas.wolf@paranor.ch>
stable-4.9
Thomas Wolf 7 years ago committed by Matthias Sohn
parent
commit
d946f95c9c
  1. 82
      org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerSslTest.java
  2. 16
      org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/SimpleHttpServer.java
  3. 8
      org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
  4. 8
      org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
  5. 15
      org.eclipse.jgit/src/org/eclipse/jgit/transport/HttpConfig.java
  6. 177
      org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java

82
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<Repository> 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"));
}
}
}

16
org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/SimpleHttpServer.java

@ -69,9 +69,15 @@ public class SimpleHttpServer {
private URIish uri;
private URIish secureUri;
public SimpleHttpServer(Repository repository) {
this(repository, false);
}
public SimpleHttpServer(Repository repository, boolean withSsl) {
this.db = repository;
server = new AppServer();
server = new AppServer(0, withSsl ? 0 : -1);
}
public void start() throws Exception {
@ -79,6 +85,10 @@ public class SimpleHttpServer {
server.setUp();
final String srcName = db.getDirectory().getName();
uri = toURIish(sBasic, srcName);
int sslPort = server.getSecurePort();
if (sslPort > 0) {
secureUri = uri.setPort(sslPort).setScheme("https");
}
}
public void stop() throws Exception {
@ -89,6 +99,10 @@ public class SimpleHttpServer {
return uri;
}
public URIish getSecureUri() {
return secureUri;
}
private ServletContextHandler smart(final String path) {
GitServlet gs = new GitServlet();
gs.setRepositoryResolver(new RepositoryResolver<HttpServletRequest>() {

8
org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties

@ -607,6 +607,14 @@ sourceIsNotAWildcard=Source is not a wildcard.
sourceRefDoesntResolveToAnyObject=Source ref {0} doesn''t resolve to any object.
sourceRefNotSpecifiedForRefspec=Source ref not specified for refspec: {0}
squashCommitNotUpdatingHEAD=Squash commit -- not updating HEAD
sslFailureExceptionMessage=Secure connection to {0} could not be stablished because of SSL problems
sslFailureInfo=A secure connection to {0}\ncould not be established because the server''s certificate could not be validated.
sslFailureCause=SSL reported: {0}
sslFailureTrustExplanation=Do you want to skip SSL verification for this server?
sslTrustAlways=Always skip SSL verification for this server from now on
sslTrustForRepo=Skip SSL verification for git operations for repository {0}
sslTrustNow=Skip SSL verification for this single git operation
sslVerifyCannotSave=Could not save setting for http.sslVerify
staleRevFlagsOn=Stale RevFlags on {0}
startingReadStageWithoutWrittenRequestDataPendingIsNotSupported=Starting read stage without written request data pending is not supported
stashApplyConflict=Applying stashed changes resulted in a conflict

8
org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java

@ -666,6 +666,14 @@ public class JGitText extends TranslationBundle {
/***/ public String sourceRefDoesntResolveToAnyObject;
/***/ public String sourceRefNotSpecifiedForRefspec;
/***/ public String squashCommitNotUpdatingHEAD;
/***/ public String sslFailureExceptionMessage;
/***/ public String sslFailureInfo;
/***/ public String sslFailureCause;
/***/ public String sslFailureTrustExplanation;
/***/ public String sslTrustAlways;
/***/ public String sslTrustForRepo;
/***/ public String sslTrustNow;
/***/ public String sslVerifyCannotSave;
/***/ public String staleRevFlagsOn;
/***/ public String startingReadStageWithoutWrittenRequestDataPendingIsNotSupported;
/***/ public String stashApplyConflict;

15
org.eclipse.jgit/src/org/eclipse/jgit/transport/HttpConfig.java

@ -71,15 +71,20 @@ public class HttpConfig {
private static final String FTP = "ftp"; //$NON-NLS-1$
private static final String HTTP = "http"; //$NON-NLS-1$
/** git config section key for http settings. */
public static final String HTTP = "http"; //$NON-NLS-1$
private static final String FOLLOW_REDIRECTS_KEY = "followRedirects"; //$NON-NLS-1$
/** git config key for the "followRedirects" setting. */
public static final String FOLLOW_REDIRECTS_KEY = "followRedirects"; //$NON-NLS-1$
private static final String MAX_REDIRECTS_KEY = "maxRedirects"; //$NON-NLS-1$
/** git config key for the "maxRedirects" setting. */
public static final String MAX_REDIRECTS_KEY = "maxRedirects"; //$NON-NLS-1$
private static final String POST_BUFFER_KEY = "postBuffer"; //$NON-NLS-1$
/** git config key for the "postBuffer" setting. */
public static final String POST_BUFFER_KEY = "postBuffer"; //$NON-NLS-1$
private static final String SSL_VERIFY_KEY = "sslVerify"; //$NON-NLS-1$
/** git config key for the "sslVerify" setting. */
public static final String SSL_VERIFY_KEY = "sslVerify"; //$NON-NLS-1$
private static final String MAX_REDIRECT_SYSTEM_PROPERTY = "http.maxRedirects"; //$NON-NLS-1$

177
org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java

@ -72,6 +72,9 @@ import java.net.Proxy;
import java.net.ProxySelector;
import java.net.URISyntaxException;
import java.net.URL;
import java.security.cert.CertPathBuilderException;
import java.security.cert.CertPathValidatorException;
import java.security.cert.CertificateException;
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.Arrays;
@ -87,6 +90,8 @@ import java.util.TreeMap;
import java.util.zip.GZIPInputStream;
import java.util.zip.GZIPOutputStream;
import javax.net.ssl.SSLHandshakeException;
import org.eclipse.jgit.errors.NoRemoteRepositoryException;
import org.eclipse.jgit.errors.NotSupportedException;
import org.eclipse.jgit.errors.PackProtocolException;
@ -99,13 +104,16 @@ import org.eclipse.jgit.lib.ObjectIdRef;
import org.eclipse.jgit.lib.ProgressMonitor;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.lib.StoredConfig;
import org.eclipse.jgit.lib.SymbolicRef;
import org.eclipse.jgit.transport.HttpAuthMethod.Type;
import org.eclipse.jgit.transport.HttpConfig.HttpRedirectMode;
import org.eclipse.jgit.transport.http.HttpConnection;
import org.eclipse.jgit.util.FS;
import org.eclipse.jgit.util.HttpSupport;
import org.eclipse.jgit.util.IO;
import org.eclipse.jgit.util.RawParseUtils;
import org.eclipse.jgit.util.SystemReader;
import org.eclipse.jgit.util.TemporaryBuffer;
import org.eclipse.jgit.util.io.DisabledOutputStream;
import org.eclipse.jgit.util.io.UnionInputStream;
@ -249,7 +257,7 @@ public class TransportHttp extends HttpTransport implements WalkTransport,
private URL objectsUrl;
final HttpConfig http;
private final HttpConfig http;
private final ProxySelector proxySelector;
@ -259,12 +267,17 @@ public class TransportHttp extends HttpTransport implements WalkTransport,
private Map<String, String> headers;
private boolean sslVerify;
private boolean sslFailure = false;
TransportHttp(final Repository local, final URIish uri)
throws NotSupportedException {
super(local, uri);
setURI(uri);
http = new HttpConfig(local.getConfig(), uri);
proxySelector = ProxySelector.getDefault();
sslVerify = http.isSslVerify();
}
private URL toURL(URIish urish) throws MalformedURLException {
@ -301,6 +314,7 @@ public class TransportHttp extends HttpTransport implements WalkTransport,
setURI(uri);
http = new HttpConfig(uri);
proxySelector = ProxySelector.getDefault();
sslVerify = http.isSslVerify();
}
/**
@ -549,6 +563,9 @@ public class TransportHttp extends HttpTransport implements WalkTransport,
throw e;
} catch (TransportException e) {
throw e;
} catch (SSLHandshakeException e) {
handleSslFailure(e);
continue; // Re-try
} catch (IOException e) {
if (authMethod.getType() != HttpAuthMethod.Type.NONE) {
if (ignoreTypes == null) {
@ -569,6 +586,120 @@ public class TransportHttp extends HttpTransport implements WalkTransport,
}
}
private static class CredentialItems {
CredentialItem.InformationalMessage message;
/** Trust the server for this git operation */
CredentialItem.YesNoType now;
/**
* Trust the server for all git operations from this repository; may be
* {@code null} if the transport was created via
* {@link #TransportHttp(URIish)}.
*/
CredentialItem.YesNoType forRepo;
/** Always trust the server from now on. */
CredentialItem.YesNoType always;
public CredentialItem[] items() {
if (forRepo == null) {
return new CredentialItem[] { message, now, always };
} else {
return new CredentialItem[] { message, now, forRepo, always };
}
}
}
private void handleSslFailure(Throwable e) throws TransportException {
if (sslFailure || !trustInsecureSslConnection(e.getCause())) {
throw new TransportException(uri,
MessageFormat.format(
JGitText.get().sslFailureExceptionMessage,
currentUri.setPass(null)),
e);
}
sslFailure = true;
}
private boolean trustInsecureSslConnection(Throwable cause) {
if (cause instanceof CertificateException
|| cause instanceof CertPathBuilderException
|| cause instanceof CertPathValidatorException) {
// Certificate expired or revoked, PKIX path building not
// possible, self-signed certificate, host does not match ...
CredentialsProvider provider = getCredentialsProvider();
if (provider != null) {
CredentialItems trust = constructSslTrustItems(cause);
CredentialItem[] items = trust.items();
if (provider.supports(items)) {
boolean answered = provider.get(uri, items);
if (answered) {
// Not canceled
boolean trustNow = trust.now.getValue();
boolean trustLocal = trust.forRepo != null
&& trust.forRepo.getValue();
boolean trustAlways = trust.always.getValue();
if (trustNow || trustLocal || trustAlways) {
sslVerify = false;
if (trustAlways) {
updateSslVerify(SystemReader.getInstance()
.openUserConfig(null, FS.DETECTED),
false);
} else if (trustLocal) {
updateSslVerify(local.getConfig(), false);
}
return true;
}
}
}
}
}
return false;
}
private CredentialItems constructSslTrustItems(Throwable cause) {
CredentialItems items = new CredentialItems();
String info = MessageFormat.format(JGitText.get().sslFailureInfo,
currentUri.setPass(null));
String sslMessage = cause.getLocalizedMessage();
if (sslMessage == null) {
sslMessage = cause.toString();
}
sslMessage = MessageFormat.format(JGitText.get().sslFailureCause,
sslMessage);
items.message = new CredentialItem.InformationalMessage(info + '\n'
+ sslMessage + '\n'
+ JGitText.get().sslFailureTrustExplanation);
items.now = new CredentialItem.YesNoType(JGitText.get().sslTrustNow);
if (local != null) {
items.forRepo = new CredentialItem.YesNoType(
MessageFormat.format(JGitText.get().sslTrustForRepo,
local.getDirectory()));
}
items.always = new CredentialItem.YesNoType(
JGitText.get().sslTrustAlways);
return items;
}
private void updateSslVerify(StoredConfig config, boolean value) {
// Since git uses the original URI for matching, we must also use the
// original URI and cannot use the current URI (which might be different
// after redirects)
String uriPattern = uri.getScheme() + "://" + uri.getHost(); //$NON-NLS-1$
int port = uri.getPort();
if (port > 0) {
uriPattern += ":" + port; //$NON-NLS-1$
}
config.setBoolean(HttpConfig.HTTP, uriPattern,
HttpConfig.SSL_VERIFY_KEY, value);
try {
config.save();
} catch (IOException e) {
LOG.error(JGitText.get().sslVerifyCannotSave, e);
}
}
private URIish redirect(String location, String checkFor, int redirects)
throws TransportException {
if (location == null || location.isEmpty()) {
@ -687,7 +818,7 @@ public class TransportHttp extends HttpTransport implements WalkTransport,
final Proxy proxy = HttpSupport.proxyFor(proxySelector, u);
HttpConnection conn = connectionFactory.create(u, proxy);
if (!http.isSslVerify() && "https".equals(u.getProtocol())) { //$NON-NLS-1$
if (!sslVerify && "https".equals(u.getProtocol())) { //$NON-NLS-1$
HttpSupport.disableSslVerify(conn);
}
@ -1034,13 +1165,16 @@ public class TransportHttp extends HttpTransport implements WalkTransport,
int authAttempts = 1;
int redirects = 0;
for (;;) {
try {
// The very first time we will try with the authentication
// method used on the initial GET request. This is a hint only;
// it may fail. If so, we'll then re-try with proper 401
// handling, going through the available authentication schemes.
// method used on the initial GET request. This is a hint
// only; it may fail. If so, we'll then re-try with proper
// 401 handling, going through the available authentication
// schemes.
openStream();
if (buf != out) {
conn.setRequestProperty(HDR_CONTENT_ENCODING, ENCODING_GZIP);
conn.setRequestProperty(HDR_CONTENT_ENCODING,
ENCODING_GZIP);
}
conn.setFixedLengthStreamingMode((int) buf.length());
try (OutputStream httpOut = conn.getOutputStream()) {
@ -1054,8 +1188,9 @@ public class TransportHttp extends HttpTransport implements WalkTransport,
return;
case HttpConnection.HTTP_NOT_FOUND:
throw new NoRemoteRepositoryException(uri, MessageFormat
.format(JGitText.get().uriNotFound, conn.getURL()));
throw new NoRemoteRepositoryException(uri,
MessageFormat.format(JGitText.get().uriNotFound,
conn.getURL()));
case HttpConnection.HTTP_FORBIDDEN:
throw new TransportException(uri,
@ -1073,15 +1208,16 @@ public class TransportHttp extends HttpTransport implements WalkTransport,
// Let openResponse() issue an error
return;
}
currentUri = redirect(
conn.getHeaderField(HDR_LOCATION),
currentUri = redirect(conn.getHeaderField(HDR_LOCATION),
'/' + serviceName, redirects++);
try {
baseUrl = toURL(currentUri);
} catch (MalformedURLException e) {
throw new TransportException(uri, MessageFormat.format(
throw new TransportException(uri,
MessageFormat.format(
JGitText.get().invalidRedirectLocation,
baseUrl, currentUri), e);
baseUrl, currentUri),
e);
}
continue;
@ -1117,8 +1253,9 @@ public class TransportHttp extends HttpTransport implements WalkTransport,
// round, keeping a client stuck here.
break;
default:
// DIGEST or BASIC. Let's be sure we ignore NEGOTIATE;
// if it was available, we have tried it before.
// DIGEST or BASIC. Let's be sure we ignore
// NEGOTIATE; if it was available, we have tried it
// before.
ignoreTypes.add(HttpAuthMethod.Type.NEGOTIATE);
if (authenticator == null || authenticator
.getType() != nextMethod.getType()) {
@ -1139,8 +1276,8 @@ public class TransportHttp extends HttpTransport implements WalkTransport,
if (authAttempts > 1) {
credentialsProvider.reset(currentUri);
}
if (3 < authAttempts || !authMethod.authorize(currentUri,
credentialsProvider)) {
if (3 < authAttempts || !authMethod
.authorize(currentUri, credentialsProvider)) {
throw new TransportException(uri,
JGitText.get().notAuthorized);
}
@ -1148,10 +1285,14 @@ public class TransportHttp extends HttpTransport implements WalkTransport,
continue;
default:
// Just return here; openResponse() will report an appropriate
// error.
// Just return here; openResponse() will report an
// appropriate error.
return;
}
} catch (SSLHandshakeException e) {
handleSslFailure(e);
continue; // Re-try
}
}
}

Loading…
Cancel
Save