sshd: store per-session data on the sshd session object

Don't store session properties on the client but in a dedicated
per-session object that is attached to the sshd session.

Also make sure that each sshd session gets its own instance of
IdentityPasswordProvider that asks for passphrases of encrypted
private keys, and also store it on the session itself.

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

5 files changed, 225 insertions, 95 deletions

diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitClientSession.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitClientSession.java
index 420a1d16eb..bf891742b7 100644
--- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitClientSession.java
+++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitClientSession.java
@@ -18,16 +18,22 @@ import java.nio.charset.StandardCharsets;
 import java.security.GeneralSecurityException;
 import java.security.PublicKey;
 import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 
 import org.apache.sshd.client.ClientFactoryManager;
 import org.apache.sshd.client.config.hosts.HostConfigEntry;
 import org.apache.sshd.client.keyverifier.ServerKeyVerifier;
 import org.apache.sshd.client.session.ClientSessionImpl;
+import org.apache.sshd.common.AttributeRepository;
 import org.apache.sshd.common.FactoryManager;
+import org.apache.sshd.common.PropertyResolver;
 import org.apache.sshd.common.PropertyResolverUtils;
 import org.apache.sshd.common.SshException;
 import org.apache.sshd.common.config.keys.KeyUtils;
@@ -419,4 +425,122 @@ public class JGitClientSession extends ClientSessionImpl {
 		return b.toString();
 	}
 
+	@Override
+	public <T> T getAttribute(AttributeKey<T> key) {
+		T value = super.getAttribute(key);
+		if (value == null) {
+			IoSession ioSession = getIoSession();
+			if (ioSession != null) {
+				Object obj = ioSession.getAttribute(AttributeRepository.class);
+				if (obj instanceof AttributeRepository) {
+					AttributeRepository sessionAttributes = (AttributeRepository) obj;
+					value = sessionAttributes.resolveAttribute(key);
+				}
+			}
+		}
+		return value;
+	}
+
+	@Override
+	public PropertyResolver getParentPropertyResolver() {
+		IoSession ioSession = getIoSession();
+		if (ioSession != null) {
+			Object obj = ioSession.getAttribute(AttributeRepository.class);
+			if (obj instanceof PropertyResolver) {
+				return (PropertyResolver) obj;
+			}
+		}
+		return super.getParentPropertyResolver();
+	}
+
+	/**
+	 * An {@link AttributeRepository} that chains together two other attribute
+	 * sources in a hierarchy.
+	 */
+	public static class ChainingAttributes implements AttributeRepository {
+
+		private final AttributeRepository delegate;
+
+		private final AttributeRepository parent;
+
+		/**
+		 * Create a new {@link ChainingAttributes} attribute source.
+		 *
+		 * @param self
+		 *            to search for attributes first
+		 * @param parent
+		 *            to search for attributes if not found in {@code self}
+		 */
+		public ChainingAttributes(AttributeRepository self,
+				AttributeRepository parent) {
+			this.delegate = self;
+			this.parent = parent;
+		}
+
+		@Override
+		public int getAttributesCount() {
+			return delegate.getAttributesCount();
+		}
+
+		@Override
+		public <T> T getAttribute(AttributeKey<T> key) {
+			return delegate.getAttribute(Objects.requireNonNull(key));
+		}
+
+		@Override
+		public Collection<AttributeKey<?>> attributeKeys() {
+			return delegate.attributeKeys();
+		}
+
+		@Override
+		public <T> T resolveAttribute(AttributeKey<T> key) {
+			T value = getAttribute(Objects.requireNonNull(key));
+			if (value == null) {
+				return parent.getAttribute(key);
+			}
+			return value;
+		}
+	}
+
+	/**
+	 * A {@link ChainingAttributes} repository that doubles as a
+	 * {@link PropertyResolver}. The property map can be set via the attribute
+	 * key {@link SessionAttributes#PROPERTIES}.
+	 */
+	public static class SessionAttributes extends ChainingAttributes
+			implements PropertyResolver {
+
+		/** Key for storing a map of properties in the attributes. */
+		public static final AttributeKey<Map<String, Object>> PROPERTIES = new AttributeKey<>();
+
+		private final PropertyResolver parentProperties;
+
+		/**
+		 * Creates a new {@link SessionAttributes} attribute and property
+		 * source.
+		 *
+		 * @param self
+		 *            to search for attributes first
+		 * @param parent
+		 *            to search for attributes if not found in {@code self}
+		 * @param parentProperties
+		 *            to search for properties if not found in {@code self}
+		 */
+		public SessionAttributes(AttributeRepository self,
+				AttributeRepository parent, PropertyResolver parentProperties) {
+			super(self, parent);
+			this.parentProperties = parentProperties;
+		}
+
+		@Override
+		public PropertyResolver getParentPropertyResolver() {
+			return parentProperties;
+		}
+
+		@Override
+		public Map<String, Object> getProperties() {
+			Map<String, Object> props = getAttribute(PROPERTIES);
+			return props == null ? Collections.emptyMap() : props;
+		}
+	}
 }
diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitSshClient.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitSshClient.java
index b8dd60fb10..1825fb37b2 100644
--- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitSshClient.java
+++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitSshClient.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2018, Thomas Wolf <thomas.wolf@paranor.ch> and others
+ * Copyright (C) 2018, 2020 Thomas Wolf <thomas.wolf@paranor.ch> 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
@@ -23,12 +23,16 @@ import java.nio.file.Paths;
 import java.security.GeneralSecurityException;
 import java.security.KeyPair;
 import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.Objects;
 import java.util.stream.Collectors;
 
+import org.apache.sshd.client.ClientAuthenticationManager;
 import org.apache.sshd.client.SshClient;
 import org.apache.sshd.client.config.hosts.HostConfigEntry;
 import org.apache.sshd.client.future.ConnectFuture;
@@ -45,6 +49,8 @@ import org.apache.sshd.common.keyprovider.KeyIdentityProvider;
 import org.apache.sshd.common.session.SessionContext;
 import org.apache.sshd.common.session.helpers.AbstractSession;
 import org.apache.sshd.common.util.ValidateUtils;
+import org.eclipse.jgit.internal.transport.sshd.JGitClientSession.ChainingAttributes;
+import org.eclipse.jgit.internal.transport.sshd.JGitClientSession.SessionAttributes;
 import org.eclipse.jgit.internal.transport.sshd.proxy.HttpClientConnector;
 import org.eclipse.jgit.internal.transport.sshd.proxy.Socks5ClientConnector;
 import org.eclipse.jgit.transport.CredentialsProvider;
@@ -52,6 +58,7 @@ import org.eclipse.jgit.transport.SshConstants;
 import org.eclipse.jgit.transport.sshd.KeyCache;
 import org.eclipse.jgit.transport.sshd.ProxyData;
 import org.eclipse.jgit.transport.sshd.ProxyDataFactory;
+import org.eclipse.jgit.util.StringUtils;
 
 /**
  * Customized {@link SshClient} for JGit. It creates specialized
@@ -100,35 +107,55 @@ public class JGitSshClient extends SshClient {
 		int port = hostConfig.getPort();
 		ValidateUtils.checkTrue(port > 0, "Invalid port: %d", port); //$NON-NLS-1$
 		String userName = hostConfig.getUsername();
+		AttributeRepository attributes = chain(context, this);
 		InetSocketAddress address = new InetSocketAddress(host, port);
 		ConnectFuture connectFuture = new DefaultConnectFuture(
 				userName + '@' + address, null);
 		SshFutureListener<IoConnectFuture> listener = createConnectCompletionListener(
 				connectFuture, userName, address, hostConfig);
-		// sshd needs some entries from the host config already in the
-		// constructor of the session. Put those as properties on this client,
-		// where it will find them. We can set the host config only once the
-		// session object has been created.
-		copyProperty(
-				hostConfig.getProperty(SshConstants.PREFERRED_AUTHENTICATIONS,
-						getAttribute(PREFERRED_AUTHENTICATIONS)),
-				PREFERRED_AUTHS);
-		setAttribute(HOST_CONFIG_ENTRY, hostConfig);
-		setAttribute(ORIGINAL_REMOTE_ADDRESS, address);
+		attributes = sessionAttributes(attributes, hostConfig, address);
 		// Proxy support
 		ProxyData proxy = getProxyData(address);
 		if (proxy != null) {
 			address = configureProxy(proxy, address);
 			proxy.clearPassword();
 		}
-		connector.connect(address, this, localAddress).addListener(listener);
+		connector.connect(address, attributes, localAddress)
+				.addListener(listener);
 		return connectFuture;
 	}
 
-	private void copyProperty(String value, String key) {
-		if (value != null && !value.isEmpty()) {
-			getProperties().put(key, value);
+	private AttributeRepository chain(AttributeRepository self,
+			AttributeRepository parent) {
+		if (self == null) {
+			return Objects.requireNonNull(parent);
+		}
+		if (parent == null || parent == self) {
+			return self;
+		}
+		return new ChainingAttributes(self, parent);
+	}
+
+	private AttributeRepository sessionAttributes(AttributeRepository parent,
+			HostConfigEntry hostConfig, InetSocketAddress originalAddress) {
+		// sshd needs some entries from the host config already in the
+		// constructor of the session. Put those into a dedicated
+		// AttributeRepository for the new session where it will find them.
+		// We can set the host config only once the session object has been
+		// created.
+		Map<AttributeKey<?>, Object> data = new HashMap<>();
+		data.put(HOST_CONFIG_ENTRY, hostConfig);
+		data.put(ORIGINAL_REMOTE_ADDRESS, originalAddress);
+		String preferredAuths = hostConfig.getProperty(
+				SshConstants.PREFERRED_AUTHENTICATIONS,
+				resolveAttribute(PREFERRED_AUTHENTICATIONS));
+		if (!StringUtils.isEmptyOrNull(preferredAuths)) {
+			data.put(SessionAttributes.PROPERTIES,
+					Collections.singletonMap(PREFERRED_AUTHS, preferredAuths));
 		}
+		return new SessionAttributes(
+				AttributeRepository.ofAttributesMap(data),
+				parent, this);
 	}
 
 	private ProxyData getProxyData(InetSocketAddress remoteAddress) {
@@ -219,11 +246,6 @@ public class JGitSshClient extends SshClient {
 		int numberOfPasswordPrompts = getNumberOfPasswordPrompts(hostConfig);
 		session.getProperties().put(PASSWORD_PROMPTS,
 				Integer.valueOf(numberOfPasswordPrompts));
-		FilePasswordProvider passwordProvider = getFilePasswordProvider();
-		if (passwordProvider instanceof RepeatingFilePasswordProvider) {
-			((RepeatingFilePasswordProvider) passwordProvider)
-					.setAttempts(numberOfPasswordPrompts);
-		}
 		List<Path> identities = hostConfig.getIdentities().stream()
 				.map(s -> {
 					try {
@@ -237,6 +259,7 @@ public class JGitSshClient extends SshClient {
 				.collect(Collectors.toList());
 		CachingKeyPairProvider ourConfiguredKeysProvider = new CachingKeyPairProvider(
 				identities, keyCache);
+		FilePasswordProvider passwordProvider = getFilePasswordProvider();
 		ourConfiguredKeysProvider.setPasswordFinder(passwordProvider);
 		if (hostConfig.isIdentitiesOnly()) {
 			session.setKeyIdentityProvider(ourConfiguredKeysProvider);
@@ -265,9 +288,7 @@ public class JGitSshClient extends SshClient {
 			log.warn(format(SshdText.get().configInvalidPositive,
 					SshConstants.NUMBER_OF_PASSWORD_PROMPTS, prompts));
 		}
-		// Default for NumberOfPasswordPrompts according to
-		// https://man.openbsd.org/ssh_config
-		return 3;
+		return ClientAuthenticationManager.DEFAULT_PASSWORD_PROMPTS;
 	}
 
 	/**
@@ -408,6 +429,5 @@ public class JGitSshClient extends SshClient {
 
 			};
 		}
-
 	}
 }
diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/PasswordProviderWrapper.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/PasswordProviderWrapper.java
index 5b48a8cf99..078e411f29 100644
--- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/PasswordProviderWrapper.java
+++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/PasswordProviderWrapper.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2018, Thomas Wolf <thomas.wolf@paranor.ch> and others
+ * Copyright (C) 2018, 2020 Thomas Wolf <thomas.wolf@paranor.ch> 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
@@ -16,8 +16,12 @@ import java.util.Arrays;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Supplier;
 
+import org.apache.sshd.client.ClientAuthenticationManager;
+import org.apache.sshd.common.AttributeRepository.AttributeKey;
 import org.apache.sshd.common.NamedResource;
+import org.apache.sshd.common.config.keys.FilePasswordProvider;
 import org.apache.sshd.common.session.SessionContext;
 import org.eclipse.jgit.annotations.NonNull;
 import org.eclipse.jgit.transport.CredentialsProvider;
@@ -25,39 +29,61 @@ import org.eclipse.jgit.transport.URIish;
 import org.eclipse.jgit.transport.sshd.KeyPasswordProvider;
 
 /**
- * A bridge from sshd's {@link RepeatingFilePasswordProvider} to our
+ * A bridge from sshd's {@link FilePasswordProvider} to our per-session
  * {@link KeyPasswordProvider} API.
  */
-public class PasswordProviderWrapper implements RepeatingFilePasswordProvider {
+public class PasswordProviderWrapper implements FilePasswordProvider {
 
-	private final KeyPasswordProvider delegate;
+	private static final AttributeKey<PerSessionState> STATE = new AttributeKey<>();
 
-	private Map<String, AtomicInteger> counts = new ConcurrentHashMap<>();
+	private static class PerSessionState {
+
+		Map<String, AtomicInteger> counts = new ConcurrentHashMap<>();
+
+		KeyPasswordProvider delegate;
 
-	/**
-	 * @param delegate
-	 */
-	public PasswordProviderWrapper(@NonNull KeyPasswordProvider delegate) {
-		this.delegate = delegate;
 	}
 
-	@Override
-	public void setAttempts(int numberOfPasswordPrompts) {
-		delegate.setAttempts(numberOfPasswordPrompts);
+	private final Supplier<KeyPasswordProvider> factory;
+
+	/**
+	 * Creates a new {@link PasswordProviderWrapper}.
+	 *
+	 * @param factory
+	 *            to use to create per-session {@link KeyPasswordProvider}s
+	 */
+	public PasswordProviderWrapper(
+			@NonNull Supplier<KeyPasswordProvider> factory) {
+		this.factory = factory;
 	}
 
-	@Override
-	public int getAttempts() {
-		return delegate.getAttempts();
+	private PerSessionState getState(SessionContext context) {
+		PerSessionState state = context.getAttribute(STATE);
+		if (state == null) {
+			state = new PerSessionState();
+			state.delegate = factory.get();
+			Integer maxNumberOfAttempts = context
+					.getInteger(ClientAuthenticationManager.PASSWORD_PROMPTS);
+			if (maxNumberOfAttempts != null
+					&& maxNumberOfAttempts.intValue() > 0) {
+				state.delegate.setAttempts(maxNumberOfAttempts.intValue());
+			} else {
+				state.delegate.setAttempts(
+						ClientAuthenticationManager.DEFAULT_PASSWORD_PROMPTS);
+			}
+			context.setAttribute(STATE, state);
+		}
+		return state;
 	}
 
 	@Override
 	public String getPassword(SessionContext session, NamedResource resource,
 			int attemptIndex) throws IOException {
 		String key = resource.getName();
-		int attempt = counts
+		PerSessionState state = getState(session);
+		int attempt = state.counts
 				.computeIfAbsent(key, k -> new AtomicInteger()).get();
-		char[] passphrase = delegate.getPassphrase(toUri(key), attempt);
+		char[] passphrase = state.delegate.getPassphrase(toUri(key), attempt);
 		if (passphrase == null) {
 			return null;
 		}
@@ -74,18 +100,19 @@ public class PasswordProviderWrapper implements RepeatingFilePasswordProvider {
 			String password, Exception err)
 			throws IOException, GeneralSecurityException {
 		String key = resource.getName();
-		AtomicInteger count = counts.get(key);
+		PerSessionState state = getState(session);
+		AtomicInteger count = state.counts.get(key);
 		int numberOfAttempts = count == null ? 0 : count.incrementAndGet();
 		ResourceDecodeResult result = null;
 		try {
-			if (delegate.keyLoaded(toUri(key), numberOfAttempts, err)) {
+			if (state.delegate.keyLoaded(toUri(key), numberOfAttempts, err)) {
 				result = ResourceDecodeResult.RETRY;
 			} else {
 				result = ResourceDecodeResult.TERMINATE;
 			}
 		} finally {
 			if (result != ResourceDecodeResult.RETRY) {
-				counts.remove(key);
+				state.counts.remove(key);
 			}
 		}
 		return result;
diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/RepeatingFilePasswordProvider.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/RepeatingFilePasswordProvider.java
deleted file mode 100644
index 86f0fe7b60..0000000000
--- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/RepeatingFilePasswordProvider.java
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- * Copyright (C) 2018, Thomas Wolf <thomas.wolf@paranor.ch> 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
- * https://www.eclipse.org/org/documents/edl-v10.php.
- *
- * SPDX-License-Identifier: BSD-3-Clause
- */
-package org.eclipse.jgit.internal.transport.sshd;
-
-import org.apache.sshd.common.config.keys.FilePasswordProvider;
-
-/**
- * A {@link FilePasswordProvider} augmented to support repeatedly asking for
- * passwords.
- *
- */
-public interface RepeatingFilePasswordProvider extends FilePasswordProvider {
-
-	/**
-	 * Define the maximum number of attempts to get a password that should be
-	 * attempted for one identity resource through this provider.
-	 *
-	 * @param numberOfPasswordPrompts
-	 *            number of times to ask for a password;
-	 *            {@link IllegalArgumentException} may be thrown if <= 0
-	 */
-	void setAttempts(int numberOfPasswordPrompts);
-
-	/**
-	 * Gets the maximum number of attempts to get a password that should be
-	 * attempted for one identity resource through this provider.
-	 *
-	 * @return the maximum number of attempts to try, always >= 1.
-	 */
-	default int getAttempts() {
-		return 1;
-	}
-
-}
diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/SshdSessionFactory.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/SshdSessionFactory.java
index bb4e49be8e..0f7ab849f5 100644
--- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/SshdSessionFactory.java
+++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/SshdSessionFactory.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2018, 2019 Thomas Wolf <thomas.wolf@paranor.ch> and others
+ * Copyright (C) 2018, 2020 Thomas Wolf <thomas.wolf@paranor.ch> 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
@@ -25,6 +25,7 @@ import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
 import org.apache.sshd.client.ClientBuilder;
@@ -194,12 +195,11 @@ public class SshdSessionFactory extends SshSessionFactory implements Closeable {
 						home, sshDir);
 				KeyIdentityProvider defaultKeysProvider = toKeyIdentityProvider(
 						getDefaultKeys(sshDir));
-				KeyPasswordProvider passphrases = createKeyPasswordProvider(
-						credentialsProvider);
 				SshClient client = ClientBuilder.builder()
 						.factory(JGitSshClient::new)
-						.filePasswordProvider(
-								createFilePasswordProvider(passphrases))
+						.filePasswordProvider(createFilePasswordProvider(
+								() -> createKeyPasswordProvider(
+										credentialsProvider)))
 						.hostConfigEntryResolver(configFile)
 						.serverKeyVerifier(new JGitServerKeyVerifier(
 								getServerKeyDatabase(home, sshDir)))
@@ -536,14 +536,14 @@ public class SshdSessionFactory extends SshSessionFactory implements Closeable {
 	/**
 	 * Creates a {@link FilePasswordProvider} for a new session.
 	 *
-	 * @param provider
-	 *            the {@link KeyPasswordProvider} to delegate to
+	 * @param providerFactory
+	 *            providing the {@link KeyPasswordProvider} to delegate to
 	 * @return a new {@link FilePasswordProvider}
 	 */
 	@NonNull
 	private FilePasswordProvider createFilePasswordProvider(
-			KeyPasswordProvider provider) {
-		return new PasswordProviderWrapper(provider);
+			Supplier<KeyPasswordProvider> providerFactory) {
+		return new PasswordProviderWrapper(providerFactory);
 	}
 
 	/**