From 76f79bc36c7c9130550459ace957703f1511b08d Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Sat, 25 Jul 2020 10:20:29 +0200 Subject: 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 --- .../internal/transport/sshd/JGitClientSession.java | 124 +++++++++++++++++++++ .../internal/transport/sshd/JGitSshClient.java | 68 +++++++---- .../transport/sshd/PasswordProviderWrapper.java | 69 ++++++++---- .../sshd/RepeatingFilePasswordProvider.java | 41 ------- .../jgit/transport/sshd/SshdSessionFactory.java | 18 +-- 5 files changed, 225 insertions(+), 95 deletions(-) delete mode 100644 org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/RepeatingFilePasswordProvider.java (limited to 'org.eclipse.jgit.ssh.apache') 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 getAttribute(AttributeKey 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 getAttribute(AttributeKey key) { + return delegate.getAttribute(Objects.requireNonNull(key)); + } + + @Override + public Collection> attributeKeys() { + return delegate.attributeKeys(); + } + + @Override + public T resolveAttribute(AttributeKey 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> 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 getProperties() { + Map 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 and others + * Copyright (C) 2018, 2020 Thomas Wolf 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 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, 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 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 and others + * Copyright (C) 2018, 2020 Thomas Wolf 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 STATE = new AttributeKey<>(); - private Map counts = new ConcurrentHashMap<>(); + private static class PerSessionState { + + Map 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 factory; + + /** + * Creates a new {@link PasswordProviderWrapper}. + * + * @param factory + * to use to create per-session {@link KeyPasswordProvider}s + */ + public PasswordProviderWrapper( + @NonNull Supplier 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 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 and others + * Copyright (C) 2018, 2020 Thomas Wolf 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 providerFactory) { + return new PasswordProviderWrapper(providerFactory); } /** -- cgit v1.2.3