l***@apache.org
2018-10-28 07:22:59 UTC
Repository: mina-sshd
Updated Branches:
refs/heads/master 2714a1cd9 -> 5c7cc7137
[SSHD-854] Using a session attribute instead of a session listener updated map in CachingPublicKeyAuthenticator
Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/5c7cc713
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/5c7cc713
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/5c7cc713
Branch: refs/heads/master
Commit: 5c7cc71377ef8d720683957548e8f0c66756d5c2
Parents: 2714a1c
Author: Lyor Goldstein <***@apache.org>
Authored: Sun Oct 28 09:22:31 2018 +0200
Committer: Lyor Goldstein <***@apache.org>
Committed: Sun Oct 28 09:22:47 2018 +0200
----------------------------------------------------------------------
.../org/apache/sshd/common/AttributeStore.java | 34 ++++++++++
.../sshd/common/channel/AbstractChannel.java | 8 +++
.../common/helpers/AbstractFactoryManager.java | 8 +++
.../common/session/helpers/SessionHelper.java | 8 +++
.../pubkey/CachingPublicKeyAuthenticator.java | 65 +++++---------------
.../common/auth/SinglePublicKeyAuthTest.java | 53 ++++++++++------
6 files changed, 106 insertions(+), 70 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/5c7cc713/sshd-common/src/main/java/org/apache/sshd/common/AttributeStore.java
----------------------------------------------------------------------
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/AttributeStore.java b/sshd-common/src/main/java/org/apache/sshd/common/AttributeStore.java
index 96f6ab1..feccc11 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/AttributeStore.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/AttributeStore.java
@@ -19,6 +19,9 @@
package org.apache.sshd.common;
+import java.util.Objects;
+import java.util.function.Function;
+
/**
* Provides the capability to attach in-memory attributes to the entity
*
@@ -66,6 +69,37 @@ public interface AttributeStore {
<T> T getAttribute(AttributeKey<T> key);
/**
+ * If the specified key is not already associated with a value (or is mapped
+ * to {@code null}), attempts to compute its value using the given mapping
+ * function and enters it into this map unless {@code null}.
+ *
+ * @param <T> The generic attribute type
+ * @param key The key of the attribute; must not be {@code null}.
+ * @param resolver The (never {@code null}) mapping function to use if value
+ * not already mapped. If returns {@code null} then value is not mapped to
+ * the provided key.
+ * @return The resolved value - {@code null} if value not mapped and resolver
+ * did not return a non-{@code null} value for it
+ */
+ default <T> T computeAttributeIfAbsent(
+ AttributeKey<T> key, Function<? super AttributeKey<T>, ? extends T> resolver) {
+ Objects.requireNonNull(resolver, "No resolver provided");
+
+ T value = getAttribute(key);
+ if (value != null) {
+ return value;
+ }
+
+ value = resolver.apply(key);
+ if (value == null) {
+ return null;
+ }
+
+ setAttribute(key, value);
+ return value;
+ }
+
+ /**
* Sets a user-defined attribute.
*
* @param <T> The generic attribute type
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/5c7cc713/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java b/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java
index 0f1892c..d213d72 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java
@@ -32,6 +32,7 @@ import java.util.concurrent.CopyOnWriteArraySet;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
import java.util.function.IntUnaryOperator;
import org.apache.sshd.common.Closeable;
@@ -953,6 +954,13 @@ public abstract class AbstractChannel
}
@Override
+ @SuppressWarnings({ "unchecked", "rawtypes" })
+ public <T> T computeAttributeIfAbsent(
+ AttributeKey<T> key, Function<? super AttributeKey<T>, ? extends T> resolver) {
+ return (T) attributes.computeIfAbsent(Objects.requireNonNull(key, "No key"), (Function) resolver);
+ }
+
+ @Override
@SuppressWarnings("unchecked")
public <T> T setAttribute(AttributeKey<T> key, T value) {
return (T) attributes.put(
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/5c7cc713/sshd-core/src/main/java/org/apache/sshd/common/helpers/AbstractFactoryManager.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/helpers/AbstractFactoryManager.java b/sshd-core/src/main/java/org/apache/sshd/common/helpers/AbstractFactoryManager.java
index 2342e45..7b9a68e 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/helpers/AbstractFactoryManager.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/helpers/AbstractFactoryManager.java
@@ -27,6 +27,7 @@ import java.util.concurrent.CopyOnWriteArraySet;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
+import java.util.function.Function;
import org.apache.sshd.agent.SshAgentFactory;
import org.apache.sshd.common.Factory;
@@ -150,6 +151,13 @@ public abstract class AbstractFactoryManager extends AbstractKexFactoryManager i
}
@Override
+ @SuppressWarnings({ "unchecked", "rawtypes" })
+ public <T> T computeAttributeIfAbsent(
+ AttributeKey<T> key, Function<? super AttributeKey<T>, ? extends T> resolver) {
+ return (T) attributes.computeIfAbsent(Objects.requireNonNull(key, "No key"), (Function) resolver);
+ }
+
+ @Override
@SuppressWarnings("unchecked")
public <T> T setAttribute(AttributeKey<T> key, T value) {
return (T) attributes.put(
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/5c7cc713/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java
index d6a4bba..9dcc58c 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java
@@ -35,6 +35,7 @@ import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
import org.apache.sshd.common.FactoryManager;
import org.apache.sshd.common.NamedResource;
@@ -149,6 +150,13 @@ public abstract class SessionHelper extends AbstractKexFactoryManager implements
}
@Override
+ @SuppressWarnings({ "unchecked", "rawtypes" })
+ public <T> T computeAttributeIfAbsent(
+ AttributeKey<T> key, Function<? super AttributeKey<T>, ? extends T> resolver) {
+ return (T) attributes.computeIfAbsent(Objects.requireNonNull(key, "No key"), (Function) resolver);
+ }
+
+ @Override
@SuppressWarnings("unchecked")
public <T> T setAttribute(AttributeKey<T> key, T value) {
return (T) attributes.put(
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/5c7cc713/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/CachingPublicKeyAuthenticator.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/CachingPublicKeyAuthenticator.java b/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/CachingPublicKeyAuthenticator.java
index 383d57b..411a579 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/CachingPublicKeyAuthenticator.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/CachingPublicKeyAuthenticator.java
@@ -23,22 +23,25 @@ import java.util.Map;
import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;
+import org.apache.sshd.common.AttributeStore.AttributeKey;
import org.apache.sshd.common.RuntimeSshException;
import org.apache.sshd.common.config.keys.KeyUtils;
-import org.apache.sshd.common.session.Session;
-import org.apache.sshd.common.session.SessionListener;
import org.apache.sshd.common.util.logging.AbstractLoggingBean;
import org.apache.sshd.server.session.ServerSession;
/**
- * Caches the result per session
+ * Caches the result per session - compensates for {@code OpenSSH} behavior
+ * where it sends 2 requests with the same key (see {@code SSHD-300}).
*
* @author <a href="mailto:***@mina.apache.org">Apache MINA SSHD Project</a>
*/
-public class CachingPublicKeyAuthenticator extends AbstractLoggingBean implements PublickeyAuthenticator, SessionListener {
+public class CachingPublicKeyAuthenticator extends AbstractLoggingBean implements PublickeyAuthenticator {
+ /**
+ * The {@link AttributeKey} used to store the cached authentication results on the session instance
+ */
+ public static final AttributeKey<Map<PublicKey, Boolean>> CACHE_ATTRIBUTE = new AttributeKey<>();
protected final PublickeyAuthenticator authenticator;
- protected final Map<Session, Map<PublicKey, Boolean>> cache = new ConcurrentHashMap<>();
public CachingPublicKeyAuthenticator(PublickeyAuthenticator authenticator) {
this.authenticator = Objects.requireNonNull(authenticator, "No delegate authenticator");
@@ -46,21 +49,15 @@ public class CachingPublicKeyAuthenticator extends AbstractLoggingBean implement
@Override
public boolean authenticate(String username, PublicKey key, ServerSession session) {
- Map<PublicKey, Boolean> map = cache.get(session);
- if (map == null) {
- map = new ConcurrentHashMap<>();
- cache.put(session, map);
- session.addSessionListener(this);
- }
-
+ Map<PublicKey, Boolean> map = resolveCachedResults(username, key, session);
Boolean result = map.get(key);
if (result == null) {
try {
result = authenticator.authenticate(username, key, session);
} catch (Error e) {
log.warn("authenticate({}@{}) failed ({}) to consult delegate for {} key={}: {}",
- username, session, e.getClass().getSimpleName(),
- KeyUtils.getKeyType(key), KeyUtils.getFingerPrint(key), e.getMessage());
+ username, session, e.getClass().getSimpleName(),
+ KeyUtils.getKeyType(key), KeyUtils.getFingerPrint(key), e.getMessage());
if (log.isDebugEnabled()) {
log.debug("authenticate(" + username + "@" + session + ") delegate failure details", e);
}
@@ -69,52 +66,20 @@ public class CachingPublicKeyAuthenticator extends AbstractLoggingBean implement
}
if (log.isDebugEnabled()) {
log.debug("authenticate({}@{}) cache result={} for {} key={}",
- username, session, result, KeyUtils.getKeyType(key), KeyUtils.getFingerPrint(key));
+ username, session, result, KeyUtils.getKeyType(key), KeyUtils.getFingerPrint(key));
}
map.put(key, result);
} else {
if (log.isDebugEnabled()) {
log.debug("authenticate({}@{}) use cached result={} for {} key={}",
- username, session, result, KeyUtils.getKeyType(key), KeyUtils.getFingerPrint(key));
+ username, session, result, KeyUtils.getKeyType(key), KeyUtils.getFingerPrint(key));
}
}
return result;
}
- @Override
- public void sessionCreated(Session session) {
- // ignored
- }
-
- @Override
- public void sessionEvent(Session session, Event event) {
- // ignored
- }
-
- @Override
- public void sessionException(Session session, Throwable t) {
- if (log.isDebugEnabled()) {
- log.debug("sessionException({}) {}: {}", session, t.getClass().getSimpleName(), t.getMessage());
- }
- if (log.isTraceEnabled()) {
- log.trace("sessionException(" + session + ") details", t);
- }
- sessionClosed(session);
- }
-
- @Override
- public void sessionClosed(Session session) {
- Map<PublicKey, Boolean> map = cache.remove(session);
- if (map == null) {
- if (log.isDebugEnabled()) {
- log.debug("sessionClosed({}) not cached", session);
- }
- } else {
- if (log.isDebugEnabled()) {
- log.debug("sessionClosed({}) removed from cache", session);
- }
- }
- session.removeSessionListener(this);
+ protected Map<PublicKey, Boolean> resolveCachedResults(String username, PublicKey key, ServerSession session) {
+ return session.computeAttributeIfAbsent(CACHE_ATTRIBUTE, attr -> new ConcurrentHashMap<>());
}
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/5c7cc713/sshd-core/src/test/java/org/apache/sshd/common/auth/SinglePublicKeyAuthTest.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/test/java/org/apache/sshd/common/auth/SinglePublicKeyAuthTest.java b/sshd-core/src/test/java/org/apache/sshd/common/auth/SinglePublicKeyAuthTest.java
index 7f03ce2..4e97267 100644
--- a/sshd-core/src/test/java/org/apache/sshd/common/auth/SinglePublicKeyAuthTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/common/auth/SinglePublicKeyAuthTest.java
@@ -38,6 +38,7 @@ import org.apache.sshd.server.auth.pubkey.CachingPublicKeyAuthenticator;
import org.apache.sshd.server.auth.pubkey.PublickeyAuthenticator;
import org.apache.sshd.server.auth.pubkey.UserAuthPublicKeyFactory;
import org.apache.sshd.server.keyprovider.SimpleGeneratorHostKeyProvider;
+import org.apache.sshd.server.session.ServerSession;
import org.apache.sshd.util.test.BaseTestSupport;
import org.junit.After;
import org.junit.Before;
@@ -52,7 +53,7 @@ import org.junit.runners.MethodSorters;
public class SinglePublicKeyAuthTest extends BaseTestSupport {
private SshServer sshd;
private int port;
- private KeyPair pairRsa = createTestHostKeyProvider().loadKey(KeyPairProvider.SSH_RSA);
+ private final KeyPair pairRsaGood;
private KeyPair pairRsaBad;
private PublickeyAuthenticator delegate;
@@ -60,6 +61,7 @@ public class SinglePublicKeyAuthTest extends BaseTestSupport {
SimpleGeneratorHostKeyProvider provider = new SimpleGeneratorHostKeyProvider();
provider.setAlgorithm(KeyUtils.RSA_ALGORITHM);
pairRsaBad = provider.loadKey(KeyPairProvider.SSH_RSA);
+ pairRsaGood = createTestHostKeyProvider().loadKey(KeyPairProvider.SSH_RSA);
}
@Before
@@ -80,12 +82,12 @@ public class SinglePublicKeyAuthTest extends BaseTestSupport {
@Test
public void testPublicKeyAuthWithCache() throws Exception {
- final ConcurrentHashMap<String, AtomicInteger> count = new ConcurrentHashMap<>();
+ ConcurrentHashMap<String, AtomicInteger> count = new ConcurrentHashMap<>();
TestCachingPublicKeyAuthenticator auth = new TestCachingPublicKeyAuthenticator((username, key, session) -> {
String fp = KeyUtils.getFingerPrint(key);
- count.putIfAbsent(fp, new AtomicInteger());
- count.get(fp).incrementAndGet();
- return key.equals(pairRsa.getPublic());
+ AtomicInteger counter = count.computeIfAbsent(fp, k -> new AtomicInteger());
+ counter.incrementAndGet();
+ return key.equals(pairRsaGood.getPublic());
});
delegate = auth;
@@ -94,34 +96,37 @@ public class SinglePublicKeyAuthTest extends BaseTestSupport {
try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
session.addPublicKeyIdentity(pairRsaBad);
- session.addPublicKeyIdentity(pairRsa);
+ session.addPublicKeyIdentity(pairRsaGood);
session.auth().verify(5L, TimeUnit.SECONDS);
assertEquals("Mismatched authentication invocations count", 2, count.size());
+ Map<Session, Map<PublicKey, Boolean>> cache = auth.getCache();
+ assertEquals("Mismatched cache size", 1, cache.size());
+
String fpBad = KeyUtils.getFingerPrint(pairRsaBad.getPublic());
- String fpGood = KeyUtils.getFingerPrint(pairRsa.getPublic());
- assertTrue("Missing bad public key", count.containsKey(fpBad));
- assertTrue("Missing good public key", count.containsKey(fpGood));
- assertEquals("Mismatched bad key authentication attempts", 1, count.get(fpBad).get());
- assertEquals("Mismatched good key authentication attempts", 1, count.get(fpGood).get());
+ AtomicInteger badCounter = count.get(fpBad);
+ assertNotNull("Missing bad public key", badCounter);
+ assertEquals("Mismatched bad key authentication attempts", 1, badCounter.get());
+
+ String fpGood = KeyUtils.getFingerPrint(pairRsaGood.getPublic());
+ AtomicInteger goodCounter = count.get(fpGood);
+ assertNotNull("Missing good public key", goodCounter);
+ assertEquals("Mismatched good key authentication attempts", 1, goodCounter.get());
} finally {
client.stop();
}
}
-
- Thread.sleep(100L);
- assertTrue("Cache not empty", auth.getCache().isEmpty());
}
@Test
public void testPublicKeyAuthWithoutCache() throws Exception {
- final ConcurrentHashMap<String, AtomicInteger> count = new ConcurrentHashMap<>();
+ ConcurrentHashMap<String, AtomicInteger> count = new ConcurrentHashMap<>();
delegate = (username, key, session) -> {
String fp = KeyUtils.getFingerPrint(key);
- count.putIfAbsent(fp, new AtomicInteger());
- count.get(fp).incrementAndGet();
- return key.equals(pairRsa.getPublic());
+ AtomicInteger counter = count.computeIfAbsent(fp, k -> new AtomicInteger());
+ counter.incrementAndGet();
+ return key.equals(pairRsaGood.getPublic());
};
try (SshClient client = setupTestClient()) {
@@ -129,7 +134,7 @@ public class SinglePublicKeyAuthTest extends BaseTestSupport {
try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
session.addPublicKeyIdentity(pairRsaBad);
- session.addPublicKeyIdentity(pairRsa);
+ session.addPublicKeyIdentity(pairRsaGood);
AuthFuture auth = session.auth();
assertTrue("Failed to authenticate on time", auth.await(5L, TimeUnit.SECONDS));
@@ -146,13 +151,15 @@ public class SinglePublicKeyAuthTest extends BaseTestSupport {
assertNotNull("Missing bad RSA key", badIndex);
assertEquals("Mismatched attempt index for bad key", 1, badIndex.intValue());
- String goodFingerPrint = KeyUtils.getFingerPrint(pairRsa.getPublic());
+ String goodFingerPrint = KeyUtils.getFingerPrint(pairRsaGood.getPublic());
Number goodIndex = count.get(goodFingerPrint);
assertNotNull("Missing good RSA key", goodIndex);
assertEquals("Mismatched attempt index for good key", 2, goodIndex.intValue());
}
public static class TestCachingPublicKeyAuthenticator extends CachingPublicKeyAuthenticator {
+ private final Map<Session, Map<PublicKey, Boolean>> cache = new ConcurrentHashMap<>();
+
public TestCachingPublicKeyAuthenticator(PublickeyAuthenticator authenticator) {
super(authenticator);
}
@@ -160,5 +167,11 @@ public class SinglePublicKeyAuthTest extends BaseTestSupport {
public Map<Session, Map<PublicKey, Boolean>> getCache() {
return cache;
}
+
+ @Override
+ protected Map<PublicKey, Boolean> resolveCachedResults(String username, PublicKey key, ServerSession session) {
+ Map<PublicKey, Boolean> map = cache.computeIfAbsent(session, s -> new ConcurrentHashMap<>());
+ return session.computeAttributeIfAbsent(CACHE_ATTRIBUTE, k -> map);
+ }
}
}
\ No newline at end of file
Updated Branches:
refs/heads/master 2714a1cd9 -> 5c7cc7137
[SSHD-854] Using a session attribute instead of a session listener updated map in CachingPublicKeyAuthenticator
Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/5c7cc713
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/5c7cc713
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/5c7cc713
Branch: refs/heads/master
Commit: 5c7cc71377ef8d720683957548e8f0c66756d5c2
Parents: 2714a1c
Author: Lyor Goldstein <***@apache.org>
Authored: Sun Oct 28 09:22:31 2018 +0200
Committer: Lyor Goldstein <***@apache.org>
Committed: Sun Oct 28 09:22:47 2018 +0200
----------------------------------------------------------------------
.../org/apache/sshd/common/AttributeStore.java | 34 ++++++++++
.../sshd/common/channel/AbstractChannel.java | 8 +++
.../common/helpers/AbstractFactoryManager.java | 8 +++
.../common/session/helpers/SessionHelper.java | 8 +++
.../pubkey/CachingPublicKeyAuthenticator.java | 65 +++++---------------
.../common/auth/SinglePublicKeyAuthTest.java | 53 ++++++++++------
6 files changed, 106 insertions(+), 70 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/5c7cc713/sshd-common/src/main/java/org/apache/sshd/common/AttributeStore.java
----------------------------------------------------------------------
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/AttributeStore.java b/sshd-common/src/main/java/org/apache/sshd/common/AttributeStore.java
index 96f6ab1..feccc11 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/AttributeStore.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/AttributeStore.java
@@ -19,6 +19,9 @@
package org.apache.sshd.common;
+import java.util.Objects;
+import java.util.function.Function;
+
/**
* Provides the capability to attach in-memory attributes to the entity
*
@@ -66,6 +69,37 @@ public interface AttributeStore {
<T> T getAttribute(AttributeKey<T> key);
/**
+ * If the specified key is not already associated with a value (or is mapped
+ * to {@code null}), attempts to compute its value using the given mapping
+ * function and enters it into this map unless {@code null}.
+ *
+ * @param <T> The generic attribute type
+ * @param key The key of the attribute; must not be {@code null}.
+ * @param resolver The (never {@code null}) mapping function to use if value
+ * not already mapped. If returns {@code null} then value is not mapped to
+ * the provided key.
+ * @return The resolved value - {@code null} if value not mapped and resolver
+ * did not return a non-{@code null} value for it
+ */
+ default <T> T computeAttributeIfAbsent(
+ AttributeKey<T> key, Function<? super AttributeKey<T>, ? extends T> resolver) {
+ Objects.requireNonNull(resolver, "No resolver provided");
+
+ T value = getAttribute(key);
+ if (value != null) {
+ return value;
+ }
+
+ value = resolver.apply(key);
+ if (value == null) {
+ return null;
+ }
+
+ setAttribute(key, value);
+ return value;
+ }
+
+ /**
* Sets a user-defined attribute.
*
* @param <T> The generic attribute type
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/5c7cc713/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java b/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java
index 0f1892c..d213d72 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java
@@ -32,6 +32,7 @@ import java.util.concurrent.CopyOnWriteArraySet;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
import java.util.function.IntUnaryOperator;
import org.apache.sshd.common.Closeable;
@@ -953,6 +954,13 @@ public abstract class AbstractChannel
}
@Override
+ @SuppressWarnings({ "unchecked", "rawtypes" })
+ public <T> T computeAttributeIfAbsent(
+ AttributeKey<T> key, Function<? super AttributeKey<T>, ? extends T> resolver) {
+ return (T) attributes.computeIfAbsent(Objects.requireNonNull(key, "No key"), (Function) resolver);
+ }
+
+ @Override
@SuppressWarnings("unchecked")
public <T> T setAttribute(AttributeKey<T> key, T value) {
return (T) attributes.put(
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/5c7cc713/sshd-core/src/main/java/org/apache/sshd/common/helpers/AbstractFactoryManager.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/helpers/AbstractFactoryManager.java b/sshd-core/src/main/java/org/apache/sshd/common/helpers/AbstractFactoryManager.java
index 2342e45..7b9a68e 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/helpers/AbstractFactoryManager.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/helpers/AbstractFactoryManager.java
@@ -27,6 +27,7 @@ import java.util.concurrent.CopyOnWriteArraySet;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
+import java.util.function.Function;
import org.apache.sshd.agent.SshAgentFactory;
import org.apache.sshd.common.Factory;
@@ -150,6 +151,13 @@ public abstract class AbstractFactoryManager extends AbstractKexFactoryManager i
}
@Override
+ @SuppressWarnings({ "unchecked", "rawtypes" })
+ public <T> T computeAttributeIfAbsent(
+ AttributeKey<T> key, Function<? super AttributeKey<T>, ? extends T> resolver) {
+ return (T) attributes.computeIfAbsent(Objects.requireNonNull(key, "No key"), (Function) resolver);
+ }
+
+ @Override
@SuppressWarnings("unchecked")
public <T> T setAttribute(AttributeKey<T> key, T value) {
return (T) attributes.put(
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/5c7cc713/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java
index d6a4bba..9dcc58c 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java
@@ -35,6 +35,7 @@ import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
import org.apache.sshd.common.FactoryManager;
import org.apache.sshd.common.NamedResource;
@@ -149,6 +150,13 @@ public abstract class SessionHelper extends AbstractKexFactoryManager implements
}
@Override
+ @SuppressWarnings({ "unchecked", "rawtypes" })
+ public <T> T computeAttributeIfAbsent(
+ AttributeKey<T> key, Function<? super AttributeKey<T>, ? extends T> resolver) {
+ return (T) attributes.computeIfAbsent(Objects.requireNonNull(key, "No key"), (Function) resolver);
+ }
+
+ @Override
@SuppressWarnings("unchecked")
public <T> T setAttribute(AttributeKey<T> key, T value) {
return (T) attributes.put(
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/5c7cc713/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/CachingPublicKeyAuthenticator.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/CachingPublicKeyAuthenticator.java b/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/CachingPublicKeyAuthenticator.java
index 383d57b..411a579 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/CachingPublicKeyAuthenticator.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/CachingPublicKeyAuthenticator.java
@@ -23,22 +23,25 @@ import java.util.Map;
import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;
+import org.apache.sshd.common.AttributeStore.AttributeKey;
import org.apache.sshd.common.RuntimeSshException;
import org.apache.sshd.common.config.keys.KeyUtils;
-import org.apache.sshd.common.session.Session;
-import org.apache.sshd.common.session.SessionListener;
import org.apache.sshd.common.util.logging.AbstractLoggingBean;
import org.apache.sshd.server.session.ServerSession;
/**
- * Caches the result per session
+ * Caches the result per session - compensates for {@code OpenSSH} behavior
+ * where it sends 2 requests with the same key (see {@code SSHD-300}).
*
* @author <a href="mailto:***@mina.apache.org">Apache MINA SSHD Project</a>
*/
-public class CachingPublicKeyAuthenticator extends AbstractLoggingBean implements PublickeyAuthenticator, SessionListener {
+public class CachingPublicKeyAuthenticator extends AbstractLoggingBean implements PublickeyAuthenticator {
+ /**
+ * The {@link AttributeKey} used to store the cached authentication results on the session instance
+ */
+ public static final AttributeKey<Map<PublicKey, Boolean>> CACHE_ATTRIBUTE = new AttributeKey<>();
protected final PublickeyAuthenticator authenticator;
- protected final Map<Session, Map<PublicKey, Boolean>> cache = new ConcurrentHashMap<>();
public CachingPublicKeyAuthenticator(PublickeyAuthenticator authenticator) {
this.authenticator = Objects.requireNonNull(authenticator, "No delegate authenticator");
@@ -46,21 +49,15 @@ public class CachingPublicKeyAuthenticator extends AbstractLoggingBean implement
@Override
public boolean authenticate(String username, PublicKey key, ServerSession session) {
- Map<PublicKey, Boolean> map = cache.get(session);
- if (map == null) {
- map = new ConcurrentHashMap<>();
- cache.put(session, map);
- session.addSessionListener(this);
- }
-
+ Map<PublicKey, Boolean> map = resolveCachedResults(username, key, session);
Boolean result = map.get(key);
if (result == null) {
try {
result = authenticator.authenticate(username, key, session);
} catch (Error e) {
log.warn("authenticate({}@{}) failed ({}) to consult delegate for {} key={}: {}",
- username, session, e.getClass().getSimpleName(),
- KeyUtils.getKeyType(key), KeyUtils.getFingerPrint(key), e.getMessage());
+ username, session, e.getClass().getSimpleName(),
+ KeyUtils.getKeyType(key), KeyUtils.getFingerPrint(key), e.getMessage());
if (log.isDebugEnabled()) {
log.debug("authenticate(" + username + "@" + session + ") delegate failure details", e);
}
@@ -69,52 +66,20 @@ public class CachingPublicKeyAuthenticator extends AbstractLoggingBean implement
}
if (log.isDebugEnabled()) {
log.debug("authenticate({}@{}) cache result={} for {} key={}",
- username, session, result, KeyUtils.getKeyType(key), KeyUtils.getFingerPrint(key));
+ username, session, result, KeyUtils.getKeyType(key), KeyUtils.getFingerPrint(key));
}
map.put(key, result);
} else {
if (log.isDebugEnabled()) {
log.debug("authenticate({}@{}) use cached result={} for {} key={}",
- username, session, result, KeyUtils.getKeyType(key), KeyUtils.getFingerPrint(key));
+ username, session, result, KeyUtils.getKeyType(key), KeyUtils.getFingerPrint(key));
}
}
return result;
}
- @Override
- public void sessionCreated(Session session) {
- // ignored
- }
-
- @Override
- public void sessionEvent(Session session, Event event) {
- // ignored
- }
-
- @Override
- public void sessionException(Session session, Throwable t) {
- if (log.isDebugEnabled()) {
- log.debug("sessionException({}) {}: {}", session, t.getClass().getSimpleName(), t.getMessage());
- }
- if (log.isTraceEnabled()) {
- log.trace("sessionException(" + session + ") details", t);
- }
- sessionClosed(session);
- }
-
- @Override
- public void sessionClosed(Session session) {
- Map<PublicKey, Boolean> map = cache.remove(session);
- if (map == null) {
- if (log.isDebugEnabled()) {
- log.debug("sessionClosed({}) not cached", session);
- }
- } else {
- if (log.isDebugEnabled()) {
- log.debug("sessionClosed({}) removed from cache", session);
- }
- }
- session.removeSessionListener(this);
+ protected Map<PublicKey, Boolean> resolveCachedResults(String username, PublicKey key, ServerSession session) {
+ return session.computeAttributeIfAbsent(CACHE_ATTRIBUTE, attr -> new ConcurrentHashMap<>());
}
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/5c7cc713/sshd-core/src/test/java/org/apache/sshd/common/auth/SinglePublicKeyAuthTest.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/test/java/org/apache/sshd/common/auth/SinglePublicKeyAuthTest.java b/sshd-core/src/test/java/org/apache/sshd/common/auth/SinglePublicKeyAuthTest.java
index 7f03ce2..4e97267 100644
--- a/sshd-core/src/test/java/org/apache/sshd/common/auth/SinglePublicKeyAuthTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/common/auth/SinglePublicKeyAuthTest.java
@@ -38,6 +38,7 @@ import org.apache.sshd.server.auth.pubkey.CachingPublicKeyAuthenticator;
import org.apache.sshd.server.auth.pubkey.PublickeyAuthenticator;
import org.apache.sshd.server.auth.pubkey.UserAuthPublicKeyFactory;
import org.apache.sshd.server.keyprovider.SimpleGeneratorHostKeyProvider;
+import org.apache.sshd.server.session.ServerSession;
import org.apache.sshd.util.test.BaseTestSupport;
import org.junit.After;
import org.junit.Before;
@@ -52,7 +53,7 @@ import org.junit.runners.MethodSorters;
public class SinglePublicKeyAuthTest extends BaseTestSupport {
private SshServer sshd;
private int port;
- private KeyPair pairRsa = createTestHostKeyProvider().loadKey(KeyPairProvider.SSH_RSA);
+ private final KeyPair pairRsaGood;
private KeyPair pairRsaBad;
private PublickeyAuthenticator delegate;
@@ -60,6 +61,7 @@ public class SinglePublicKeyAuthTest extends BaseTestSupport {
SimpleGeneratorHostKeyProvider provider = new SimpleGeneratorHostKeyProvider();
provider.setAlgorithm(KeyUtils.RSA_ALGORITHM);
pairRsaBad = provider.loadKey(KeyPairProvider.SSH_RSA);
+ pairRsaGood = createTestHostKeyProvider().loadKey(KeyPairProvider.SSH_RSA);
}
@Before
@@ -80,12 +82,12 @@ public class SinglePublicKeyAuthTest extends BaseTestSupport {
@Test
public void testPublicKeyAuthWithCache() throws Exception {
- final ConcurrentHashMap<String, AtomicInteger> count = new ConcurrentHashMap<>();
+ ConcurrentHashMap<String, AtomicInteger> count = new ConcurrentHashMap<>();
TestCachingPublicKeyAuthenticator auth = new TestCachingPublicKeyAuthenticator((username, key, session) -> {
String fp = KeyUtils.getFingerPrint(key);
- count.putIfAbsent(fp, new AtomicInteger());
- count.get(fp).incrementAndGet();
- return key.equals(pairRsa.getPublic());
+ AtomicInteger counter = count.computeIfAbsent(fp, k -> new AtomicInteger());
+ counter.incrementAndGet();
+ return key.equals(pairRsaGood.getPublic());
});
delegate = auth;
@@ -94,34 +96,37 @@ public class SinglePublicKeyAuthTest extends BaseTestSupport {
try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
session.addPublicKeyIdentity(pairRsaBad);
- session.addPublicKeyIdentity(pairRsa);
+ session.addPublicKeyIdentity(pairRsaGood);
session.auth().verify(5L, TimeUnit.SECONDS);
assertEquals("Mismatched authentication invocations count", 2, count.size());
+ Map<Session, Map<PublicKey, Boolean>> cache = auth.getCache();
+ assertEquals("Mismatched cache size", 1, cache.size());
+
String fpBad = KeyUtils.getFingerPrint(pairRsaBad.getPublic());
- String fpGood = KeyUtils.getFingerPrint(pairRsa.getPublic());
- assertTrue("Missing bad public key", count.containsKey(fpBad));
- assertTrue("Missing good public key", count.containsKey(fpGood));
- assertEquals("Mismatched bad key authentication attempts", 1, count.get(fpBad).get());
- assertEquals("Mismatched good key authentication attempts", 1, count.get(fpGood).get());
+ AtomicInteger badCounter = count.get(fpBad);
+ assertNotNull("Missing bad public key", badCounter);
+ assertEquals("Mismatched bad key authentication attempts", 1, badCounter.get());
+
+ String fpGood = KeyUtils.getFingerPrint(pairRsaGood.getPublic());
+ AtomicInteger goodCounter = count.get(fpGood);
+ assertNotNull("Missing good public key", goodCounter);
+ assertEquals("Mismatched good key authentication attempts", 1, goodCounter.get());
} finally {
client.stop();
}
}
-
- Thread.sleep(100L);
- assertTrue("Cache not empty", auth.getCache().isEmpty());
}
@Test
public void testPublicKeyAuthWithoutCache() throws Exception {
- final ConcurrentHashMap<String, AtomicInteger> count = new ConcurrentHashMap<>();
+ ConcurrentHashMap<String, AtomicInteger> count = new ConcurrentHashMap<>();
delegate = (username, key, session) -> {
String fp = KeyUtils.getFingerPrint(key);
- count.putIfAbsent(fp, new AtomicInteger());
- count.get(fp).incrementAndGet();
- return key.equals(pairRsa.getPublic());
+ AtomicInteger counter = count.computeIfAbsent(fp, k -> new AtomicInteger());
+ counter.incrementAndGet();
+ return key.equals(pairRsaGood.getPublic());
};
try (SshClient client = setupTestClient()) {
@@ -129,7 +134,7 @@ public class SinglePublicKeyAuthTest extends BaseTestSupport {
try (ClientSession session = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
session.addPublicKeyIdentity(pairRsaBad);
- session.addPublicKeyIdentity(pairRsa);
+ session.addPublicKeyIdentity(pairRsaGood);
AuthFuture auth = session.auth();
assertTrue("Failed to authenticate on time", auth.await(5L, TimeUnit.SECONDS));
@@ -146,13 +151,15 @@ public class SinglePublicKeyAuthTest extends BaseTestSupport {
assertNotNull("Missing bad RSA key", badIndex);
assertEquals("Mismatched attempt index for bad key", 1, badIndex.intValue());
- String goodFingerPrint = KeyUtils.getFingerPrint(pairRsa.getPublic());
+ String goodFingerPrint = KeyUtils.getFingerPrint(pairRsaGood.getPublic());
Number goodIndex = count.get(goodFingerPrint);
assertNotNull("Missing good RSA key", goodIndex);
assertEquals("Mismatched attempt index for good key", 2, goodIndex.intValue());
}
public static class TestCachingPublicKeyAuthenticator extends CachingPublicKeyAuthenticator {
+ private final Map<Session, Map<PublicKey, Boolean>> cache = new ConcurrentHashMap<>();
+
public TestCachingPublicKeyAuthenticator(PublickeyAuthenticator authenticator) {
super(authenticator);
}
@@ -160,5 +167,11 @@ public class SinglePublicKeyAuthTest extends BaseTestSupport {
public Map<Session, Map<PublicKey, Boolean>> getCache() {
return cache;
}
+
+ @Override
+ protected Map<PublicKey, Boolean> resolveCachedResults(String username, PublicKey key, ServerSession session) {
+ Map<PublicKey, Boolean> map = cache.computeIfAbsent(session, s -> new ConcurrentHashMap<>());
+ return session.computeAttributeIfAbsent(CACHE_ATTRIBUTE, k -> map);
+ }
}
}
\ No newline at end of file