INT-3796: Configure SFTP Unknown Host Behavior

JIRA: https://jira.spring.io/browse/INT-3796

Previously, the `DefaultSftpSessionFactory` unconditionally accepted keys
from unknown hosts, or when a host key changed.

Permit this behavior to be configured with `allowUnknownHosts` and/or a custom
`UserInfo` properties in the factory.

`allowUknownHosts` is false by default (since 4.2) and true by default in 4.1.x for
backwards compatibility.

INT-3796: Fix Tests

INT-3796: Polishing

Conflicts:
	src/reference/asciidoc/sftp.adoc
	src/reference/asciidoc/whats-new.adoc

Fix `SftpSessionFactoryTests` according to the `allowUnknownKeys = true` in the `4.1.x`

spring-integration-sftp/src/main/java/org/springframework/integration/sftp/session/DefaultSftpSessionFactory.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,9 +16,14 @@
 
 package org.springframework.integration.sftp.session;
 
+import java.util.Arrays;
 import java.util.Properties;
+import java.util.concurrent.locks.ReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
 import org.springframework.beans.factory.BeanCreationException;
 import org.springframework.core.io.Resource;
 import org.springframework.integration.file.remote.session.SessionFactory;
@@ -43,11 +48,22 @@ import com.jcraft.jsch.UserInfo;
  * @author Gunnar Hillert
  * @author Gary Russell
  * @author David Liu
+ * @author Pat Turner
  *
  * @since 2.0
  */
 public class DefaultSftpSessionFactory implements SessionFactory<LsEntry>, SharedSessionCapable {
 
+	private static final Log logger = LogFactory.getLog(DefaultSftpSessionFactory.class);
+
+	private final ReadWriteLock sharedSessionLock = new ReentrantReadWriteLock();
+
+	private final UserInfo userInfoWrapper = new UserInfoWrapper();
+
+	private final JSch jsch;
+
+	private final boolean isSharedSession;
+
 	private volatile String host;
 
 	private volatile int port = 22; // the default
@@ -80,13 +96,11 @@ public class DefaultSftpSessionFactory implements SessionFactory<LsEntry>, Share
 
 	private volatile Boolean enableDaemonThread;
 
-	private final JSch jsch;
-
-	private final boolean isSharedSession;
-
 	private volatile JSchSessionWrapper sharedJschSession;
 
-	private final ReentrantReadWriteLock sharedSessionLock = new ReentrantReadWriteLock();
+	private volatile UserInfo userInfo;
+
+	private volatile boolean allowUnknownKeys = true;
 
 
 	public DefaultSftpSessionFactory() {
@@ -159,8 +173,10 @@ public class DefaultSftpSessionFactory implements SessionFactory<LsEntry>, Share
 	}
 
 	/**
-	 * Specifies the filename that will be used to create a host key repository.
-	 * The resulting file has the same format as OpenSSH's known_hosts file.
+	 * Specifies the filename that will be used for a host key repository.
+	 * The file has the same format as OpenSSH's known_hosts file.
+	 * Required if {@link #setAllowUnknownKeys(boolean) allowUnknownKeys} is
+	 * false (default).
 	 *
 	 * @param knownHosts The known hosts.
 	 *
@@ -311,6 +327,42 @@ public class DefaultSftpSessionFactory implements SessionFactory<LsEntry>, Share
 		this.enableDaemonThread = enableDaemonThread;
 	}
 
+	/**
+	 * Provide a {@link UserInfo} which exposes control over dealing with new keys or key
+	 * changes. As Spring Integration will not normally allow user interaction, the
+	 * implementation must respond to Jsch calls in a suitable way.
+	 * <p>
+	 * Jsch calls {@link UserInfo#promptYesNo(String)} when connecting to an unknown host,
+	 * or when a known host's key has changed (see {@link #setKnownHosts(String)
+	 * knownHosts}). Generally, it should return false as returning true will accept all
+	 * new keys or key changes.
+	 * <p>
+	 * If no {@link UserInfo} is provided, the behavior is defined by
+	 * {@link #setAllowUnknownKeys(boolean) allowUnknownKeys}.
+	 * <p>
+	 * If {@link #setPassword(String) setPassword} is invoked with a non-null password, it will
+	 * override any password in the supplied {@link UserInfo}.
+	 *
+	 * @param userInfo the UserInfo.
+	 * @see com.jcraft.jsch.Session#setUserInfo(com.jcraft.jsch.UserInfo)
+	 * @since 4.1.7
+	 */
+	public void setUserInfo(UserInfo userInfo) {
+		this.userInfo = userInfo;
+	}
+
+	/**
+	 * When no {@link UserInfo} has been provided, set to true to unconditionally allow
+	 * connecting to an unknown host or when a host's key has changed (see
+	 * {@link #setKnownHosts(String) knownHosts}). Default false (since 4.2).
+	 * Set to true if a knownHosts file is not provided.
+	 *
+	 * @param allowUnknownKeys true to allow connecting to unknown hosts.
+	 * @since 4.1.7
+	 */
+	public void setAllowUnknownKeys(boolean allowUnknownKeys) {
+		this.allowUnknownKeys = allowUnknownKeys;
+	}
 
 	@Override
 	public SftpSession getSession() {
@@ -383,7 +435,7 @@ public class DefaultSftpSessionFactory implements SessionFactory<LsEntry>, Share
 		if (StringUtils.hasText(this.password)) {
 			jschSession.setPassword(this.password);
 		}
-		jschSession.setUserInfo(new OptimisticUserInfoImpl(this.password));
+		jschSession.setUserInfo(this.userInfoWrapper);
 
 		try {
 			if (proxy != null){
@@ -428,50 +480,119 @@ public class DefaultSftpSessionFactory implements SessionFactory<LsEntry>, Share
 	}
 
 	/**
-	 * this is a simple, optimistic implementation of the UserInfo interface.
-	 * It returns in the positive where possible and handles interactive authentication
-	 * (i.e. 'Please enter your password: ' prompts are dispatched automatically).
+	 * Wrapper class will delegate calls to a configured {@link UserInfo}, providing
+	 * sensible defaults if null. As the password is configured in this Factory, the
+	 * wrapper will return the factory's configured password and only delegate to the
+	 * UserInfo if null.
+	 * @since 4.1.7
 	 */
-	private static class OptimisticUserInfoImpl implements UserInfo, UIKeyboardInteractive {
+	private class UserInfoWrapper implements UserInfo, UIKeyboardInteractive {
 
-		private final String password;
+		/**
+		 * Convenience to check whether enclosing factory's UserInfo is configured.
+		 * @return true if there's a delegate.
+		 */
+		private boolean hasDelegate() {
+			return getDelegate() != null;
+		}
 
-		public OptimisticUserInfoImpl(String password) {
-			this.password = password;
+		/**
+		 * Convenience to retrieve enclosing factory's UserInfo.
+		 * @return
+		 */
+		private UserInfo getDelegate() {
+			return DefaultSftpSessionFactory.this.userInfo;
 		}
 
 		@Override
 		public String getPassphrase() {
-			return null; // pass
+			if (hasDelegate()) {
+				return getDelegate().getPassphrase();
+			}
+			else {
+				if (logger.isDebugEnabled()) {
+					logger.debug("No UserInfo provided for passphrase, returning: null");
+				}
+				return null;
+			}
 		}
 
 		@Override
 		public String getPassword() {
-			return this.password;
+			if (hasDelegate()) {
+				if (DefaultSftpSessionFactory.this.password != null) {
+					logger.debug("Password is obtained from the factory, not the supplied UserInfo");
+				}
+				else {
+					return getDelegate().getPassword();
+				}
+			}
+			return DefaultSftpSessionFactory.this.password;
 		}
 
 		@Override
-		public boolean promptPassphrase(String string) {
-			return true;
+		public boolean promptPassword(String message) {
+			if (hasDelegate()) {
+				return getDelegate().promptPassword(message);
+			}
+			else {
+				if (logger.isDebugEnabled()) {
+					logger.debug("No UserInfo provided - " + message + ", returning: true");
+				}
+				return true;
+			}
 		}
 
 		@Override
-		public boolean promptPassword(String string) {
-			return true;
+		public boolean promptPassphrase(String message) {
+			if (hasDelegate()) {
+				return getDelegate().promptPassphrase(message);
+			}
+			else {
+				if (logger.isDebugEnabled()) {
+					logger.debug("No UserInfo provided - " + message + ", returning: true");
+				}
+				return true;
+			}
 		}
 
 		@Override
-		public boolean promptYesNo(String string) {
-			return true;
+		public boolean promptYesNo(String message) {
+			if (hasDelegate()) {
+				return getDelegate().promptYesNo(message);
+			}
+			else {
+				if (logger.isDebugEnabled()) {
+					logger.debug("No UserInfo provided - " + message + ", returning:"
+							+ DefaultSftpSessionFactory.this.allowUnknownKeys);
+				}
+				return DefaultSftpSessionFactory.this.allowUnknownKeys;
+			}
 		}
 
 		@Override
-		public void showMessage(String string) {
+		public void showMessage(String message) {
+			if (hasDelegate()) {
+				getDelegate().showMessage(message);
+			}
+			else {
+				logger.debug(message);
+			}
 		}
 
 		@Override
-		public String[] promptKeyboardInteractive(String destination,
-				String name, String instruction, String[] prompt, boolean[] echo) {
+		public String[] promptKeyboardInteractive(String destination, String name, String instruction, String[] prompt,
+				boolean[] echo) {
+			if (hasDelegate()) {
+				if (getDelegate() instanceof UIKeyboardInteractive) {
+					return ((UIKeyboardInteractive) getDelegate()).promptKeyboardInteractive(destination, name,
+							instruction, prompt, echo);
+				}
+			}
+			if (logger.isDebugEnabled()) {
+				logger.debug("No UserInfo provided - " + destination + ":" + name + ":" + instruction + ":"
+						+ Arrays.asList(prompt) + ":" + Arrays.asList(echo));
+			}
 			return null;
 		}
 	}

spring-integration-sftp/src/test/java/org/springframework/integration/sftp/TestSftpServer.java

@@ -192,6 +192,7 @@ public class TestSftpServer implements InitializingBean, DisposableBean {
 		factory.setPort(this.port);
 		factory.setUser("foo");
 		factory.setPassword("foo");
+		factory.setAllowUnknownKeys(true);
 		return factory;
 	}
 

spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/INT3305Tests.java

@@ -1,105 +0,0 @@
-/*
- * Copyright 2014 the original author or authors.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.springframework.integration.sftp.session;
-
-import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.instanceOf;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
-import java.net.ConnectException;
-
-import org.apache.sshd.SshServer;
-import org.apache.sshd.server.PasswordAuthenticator;
-import org.apache.sshd.server.keyprovider.SimpleGeneratorHostKeyProvider;
-import org.apache.sshd.server.session.ServerSession;
-import org.junit.Test;
-
-import org.springframework.integration.test.util.SocketUtils;
-
-import com.jcraft.jsch.JSchException;
-
-/**
- * @author Gary Russell
- * @since 3.0.2
- *
- */
-public class INT3305Tests {
-
-	/*
-	 * Verify the socket is closed if the channel.connect() fails.
-	 */
-	@Test
-	public void testConnectFailSocketOpen() throws Exception {
-		final int port = SocketUtils.findAvailableServerSocket();
-		SshServer server = SshServer.setUpDefaultServer();
-		try {
-			server.setPasswordAuthenticator(new PasswordAuthenticator() {
-
-				@Override
-				public boolean authenticate(String arg0, String arg1, ServerSession arg2) {
-					return true;
-				}
-			});
-			server.setPort(port);
-			server.setKeyPairProvider(new SimpleGeneratorHostKeyProvider("hostkey.ser"));
-			server.start();
-
-			DefaultSftpSessionFactory f = new DefaultSftpSessionFactory();
-			f.setHost("localhost");
-			f.setPort(port);
-			f.setUser("user");
-			f.setPassword("pass");
-			int n = 0;
-			while (true) {
-				try {
-					f.getSession();
-					fail("Expected Exception");
-				}
-				catch (Exception e) {
-					if (e instanceof IllegalStateException && "failed to create SFTP Session".equals(e.getMessage())) {
-						if (e.getCause() instanceof IllegalStateException) {
-							if (e.getCause().getCause() instanceof JSchException) {
-								if (e.getCause().getCause().getCause() instanceof ConnectException) {
-									assertTrue("Server failed to start in 10 seconds", n++ < 100);
-									Thread.sleep(100);
-									continue;
-								}
-							}
-						}
-					}
-					assertThat(e, instanceOf(IllegalStateException.class));
-					assertThat(e.getCause(), instanceOf(IllegalStateException.class));
-					assertThat(e.getCause().getMessage(), equalTo("failed to connect"));
-					break;
-				}
-			}
-
-			n = 0;
-			while (n++ < 100 && server.getActiveSessions().size() > 0) {
-				Thread.sleep(100);
-			}
-
-			assertEquals(0, server.getActiveSessions().size());
-		}
-		finally {
-			server.stop(true);
-		}
-	}
-
-}

spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpServerTests.java

@@ -102,6 +102,7 @@ public class SftpServerTests {
 			f.setPort(port);
 			f.setUser("user");
 			f.setPassword("pass");
+			f.setAllowUnknownKeys(true);
 			Session<LsEntry> session = f.getSession();
 			doTest(server, session);
 		}
@@ -153,6 +154,7 @@ public class SftpServerTests {
 			f.setHost("localhost");
 			f.setPort(port);
 			f.setUser("user");
+			f.setAllowUnknownKeys(true);
 			InputStream stream = new ClassPathResource("id_rsa").getInputStream();
 			f.setPrivateKey(new ByteArrayResource(StreamUtils.copyToByteArray(stream)));
 			Session<LsEntry> session = f.getSession();

spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpSessionFactoryTests.java

@@ -0,0 +1,211 @@
+/*
+ * Copyright 2014-2015 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.integration.sftp.session;
+
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+import java.net.ConnectException;
+import java.security.PublicKey;
+import java.util.Arrays;
+
+import org.apache.sshd.SshServer;
+import org.apache.sshd.common.NamedFactory;
+import org.apache.sshd.server.Command;
+import org.apache.sshd.server.PasswordAuthenticator;
+import org.apache.sshd.server.PublickeyAuthenticator;
+import org.apache.sshd.server.keyprovider.SimpleGeneratorHostKeyProvider;
+import org.apache.sshd.server.session.ServerSession;
+import org.apache.sshd.server.sftp.SftpSubsystem;
+import org.junit.Test;
+
+import org.springframework.core.io.ClassPathResource;
+import org.springframework.core.io.Resource;
+import org.springframework.integration.test.util.SocketUtils;
+
+import com.jcraft.jsch.JSchException;
+import com.jcraft.jsch.UserInfo;
+
+/**
+ * @author Gary Russell
+ * @since 3.0.2
+ */
+public class SftpSessionFactoryTests {
+
+	/*
+	 * Verify the socket is closed if the channel.connect() fails.
+	 * INT-3305
+	 */
+	@Test
+	public void testConnectFailSocketOpen() throws Exception {
+		final int port = SocketUtils.findAvailableServerSocket();
+		SshServer server = SshServer.setUpDefaultServer();
+		try {
+			server.setPasswordAuthenticator(new PasswordAuthenticator() {
+
+				@Override
+				public boolean authenticate(String arg0, String arg1, ServerSession arg2) {
+					return true;
+				}
+			});
+			server.setPort(port);
+			server.setKeyPairProvider(new SimpleGeneratorHostKeyProvider("hostkey.ser"));
+			server.start();
+
+			DefaultSftpSessionFactory f = new DefaultSftpSessionFactory();
+			f.setHost("localhost");
+			f.setPort(port);
+			f.setUser("user");
+			f.setPassword("pass");
+			int n = 0;
+			while (true) {
+				try {
+					f.getSession();
+					fail("Expected Exception");
+				}
+				catch (Exception e) {
+					if (e instanceof IllegalStateException && "failed to create SFTP Session".equals(e.getMessage())) {
+						if (e.getCause() instanceof IllegalStateException) {
+							if (e.getCause().getCause() instanceof JSchException) {
+								if (e.getCause().getCause().getCause() instanceof ConnectException) {
+									assertTrue("Server failed to start in 10 seconds", n++ < 100);
+									Thread.sleep(100);
+									continue;
+								}
+							}
+						}
+					}
+					assertThat(e, instanceOf(IllegalStateException.class));
+					assertThat(e.getCause(), instanceOf(IllegalStateException.class));
+					assertThat(e.getCause().getMessage(), equalTo("failed to connect"));
+					break;
+				}
+			}
+
+			n = 0;
+			while (n++ < 100 && server.getActiveSessions().size() > 0) {
+				Thread.sleep(100);
+			}
+
+			assertEquals(0, server.getActiveSessions().size());
+		}
+		finally {
+			server.stop(true);
+		}
+	}
+
+	@Test
+	public void testDefaultUserInfoFalse() throws Exception {
+		SshServer server = SshServer.setUpDefaultServer();
+		try {
+			DefaultSftpSessionFactory f = createServerAndClient(server);
+			f.setAllowUnknownKeys(false);
+			expectReject(f);
+		}
+		finally {
+			server.stop(true);
+		}
+	}
+
+	@Test
+	public void testDefaultUserInfoTrue() throws Exception {
+		SshServer server = SshServer.setUpDefaultServer();
+		try {
+			DefaultSftpSessionFactory f = createServerAndClient(server);
+			f.getSession().close();
+		}
+		finally {
+			server.stop(true);
+		}
+	}
+
+	@Test
+	public void testCustomUserInfoFalse() throws Exception {
+		SshServer server = SshServer.setUpDefaultServer();
+		try {
+			DefaultSftpSessionFactory f = createServerAndClient(server);
+			UserInfo userInfo = mock(UserInfo.class);
+			when(userInfo.promptYesNo(anyString())).thenReturn(false);
+			f.setUserInfo(userInfo);
+			expectReject(f);
+		}
+		finally {
+			server.stop(true);
+		}
+	}
+
+	private void expectReject(DefaultSftpSessionFactory f) {
+		try {
+			f.getSession().close();
+			fail("Expected Exception");
+		}
+		catch (Exception e) {
+			assertThat(e, instanceOf(IllegalStateException.class));
+			assertThat(e.getCause(), instanceOf(IllegalStateException.class));
+			assertThat(e.getCause().getCause(), instanceOf(JSchException.class));
+			assertThat(e.getCause().getCause().getMessage(), containsString("reject HostKey"));
+		}
+	}
+
+	@Test
+	public void testCustomUserInfoTrue() throws Exception {
+		SshServer server = SshServer.setUpDefaultServer();
+		try {
+			DefaultSftpSessionFactory f = createServerAndClient(server);
+			UserInfo userInfo = mock(UserInfo.class);
+			when(userInfo.promptYesNo(anyString())).thenReturn(true);
+			f.setUserInfo(userInfo);
+			f.getSession().close();
+		}
+		finally {
+			server.stop(true);
+		}
+	}
+
+	@SuppressWarnings("unchecked")
+	private DefaultSftpSessionFactory createServerAndClient(SshServer server) throws IOException {
+		final int port = SocketUtils.findAvailableServerSocket();
+		server.setPublickeyAuthenticator(new PublickeyAuthenticator() {
+
+			@Override
+			public boolean authenticate(String username, PublicKey key, ServerSession session) {
+				return true;
+			}
+		});
+		server.setPort(port);
+		server.setSubsystemFactories(Arrays.<NamedFactory<Command>>asList(new SftpSubsystem.Factory()));
+		server.setKeyPairProvider(new SimpleGeneratorHostKeyProvider("hostkey.ser"));
+		server.start();
+
+		DefaultSftpSessionFactory f = new DefaultSftpSessionFactory();
+		f.setHost("localhost");
+		f.setPort(port);
+		f.setUser("user");
+		Resource privateKey = new ClassPathResource("id_rsa");
+		f.setPrivateKey(privateKey);
+		return f;
+	}
+
+}