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>

org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitClientSession.java

@@ -18,16 +18,22 @@
 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 @@ private static String escapeControls(String s) {
 		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;
+		}
+	}
 }

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.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.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.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 ConnectFuture connect(HostConfigEntry hostConfig,
 		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
+		attributes = sessionAttributes(attributes, hostConfig, address);
-		// 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);
 		// 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) {
+	private AttributeRepository chain(AttributeRepository self,
-		if (value != null && !value.isEmpty()) {
+			AttributeRepository parent) {
-			getProperties().put(key, value);
+		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 @@ private JGitClientSession createSession(IoSession ioSession,
 		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 @@ private JGitClientSession createSession(IoSession ioSession,
 				.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 @@ private int getNumberOfPasswordPrompts(HostConfigEntry hostConfig) {
 			log.warn(format(SshdText.get().configInvalidPositive,
 					SshConstants.NUMBER_OF_PASSWORD_PROMPTS, prompts));
 		}
-		// Default for NumberOfPasswordPrompts according to
+		return ClientAuthenticationManager.DEFAULT_PASSWORD_PROMPTS;
-		// https://man.openbsd.org/ssh_config
-		return 3;
 	}
 
 	/**
@@ -408,6 +429,5 @@ public KeyPair next() {
 
 			};
 		}
-
 	}
 }

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.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.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;
+
+	}
+
+	private final Supplier<KeyPasswordProvider> factory;
 
 	/**
-	 * @param delegate
+	 * Creates a new {@link PasswordProviderWrapper}.
+	 *
+	 * @param factory
+	 *            to use to create per-session {@link KeyPasswordProvider}s
 	 */
-	public PasswordProviderWrapper(@NonNull KeyPasswordProvider delegate) {
+	public PasswordProviderWrapper(
-		this.delegate = delegate;
+			@NonNull Supplier<KeyPasswordProvider> factory) {
+		this.factory = factory;
 	}
 
-	@Override
+	private PerSessionState getState(SessionContext context) {
-	public void setAttempts(int numberOfPasswordPrompts) {
+		PerSessionState state = context.getAttribute(STATE);
-		delegate.setAttempts(numberOfPasswordPrompts);
+		if (state == null) {
-	}
+			state = new PerSessionState();
-
+			state.delegate = factory.get();
-	@Override
+			Integer maxNumberOfAttempts = context
-	public int getAttempts() {
+					.getInteger(ClientAuthenticationManager.PASSWORD_PROMPTS);
-		return delegate.getAttempts();
+			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 ResourceDecodeResult handleDecodeAttemptResult(
 			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;

org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/RepeatingFilePasswordProvider.java

@@ -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;
-	}
-
-}

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.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 SshdSession getSession(URIish uri,
 						home, sshDir);
 				KeyIdentityProvider defaultKeysProvider = toKeyIdentityProvider(
 						getDefaultKeys(sshDir));
-				KeyPasswordProvider passphrases = createKeyPasswordProvider(
-						credentialsProvider);
 				SshClient client = ClientBuilder.builder()
 						.factory(JGitSshClient::new)
-						.filePasswordProvider(
+						.filePasswordProvider(createFilePasswordProvider(
-								createFilePasswordProvider(passphrases))
+								() -> createKeyPasswordProvider(
+										credentialsProvider)))
 						.hostConfigEntryResolver(configFile)
 						.serverKeyVerifier(new JGitServerKeyVerifier(
 								getServerKeyDatabase(home, sshDir)))
@@ -536,14 +536,14 @@ protected KeyPasswordProvider createKeyPasswordProvider(
 	/**
 	 * Creates a {@link FilePasswordProvider} for a new session.
 	 *
-	 * @param provider
+	 * @param providerFactory
-	 *            the {@link KeyPasswordProvider} to delegate to
+	 *            providing the {@link KeyPasswordProvider} to delegate to
 	 * @return a new {@link FilePasswordProvider}
 	 */
 	@NonNull
 	private FilePasswordProvider createFilePasswordProvider(
-			KeyPasswordProvider provider) {
+			Supplier<KeyPasswordProvider> providerFactory) {
-		return new PasswordProviderWrapper(provider);
+		return new PasswordProviderWrapper(providerFactory);
 	}
 
 	/**