Revert "Revert "SSHD: Prevent double authentication for the same public key""

This reverts commit c7dedf989c.

Change-Id: Ic4295ee58db8e0eb8869e526988ac4a3758370ee

gerrit-sshd/src/main/java/com/google/gerrit/sshd/CachingPublicKeyAuthenticator.java

@@ -0,0 +1,72 @@
+// Copyright (C) 2014 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.sshd;
+
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+import org.apache.sshd.common.Session;
+import org.apache.sshd.common.SessionListener;
+import org.apache.sshd.server.PublickeyAuthenticator;
+import org.apache.sshd.server.session.ServerSession;
+
+import java.security.PublicKey;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+@Singleton
+public class CachingPublicKeyAuthenticator implements PublickeyAuthenticator,
+    SessionListener {
+
+  private final PublickeyAuthenticator authenticator;
+  private final Map<ServerSession, Map<PublicKey, Boolean>> sessionCache;
+
+  @Inject
+  public CachingPublicKeyAuthenticator(DatabasePubKeyAuth authenticator) {
+    this.authenticator = authenticator;
+    this.sessionCache = new ConcurrentHashMap<>();
+  }
+
+  @Override
+  public boolean authenticate(String username, PublicKey key,
+      ServerSession session) {
+    Map<PublicKey, Boolean> m = sessionCache.get(session);
+    if (m == null) {
+      m = new HashMap<>();
+      sessionCache.put(session, m);
+      session.addListener(this);
+    }
+    if (m.containsKey(key)) {
+      return m.get(key);
+    }
+    boolean r = authenticator.authenticate(username, key, session);
+    m.put(key, r);
+    return r;
+  }
+
+  @Override
+  public void sessionCreated(Session session) {
+  }
+
+  @Override
+  public void sessionEvent(Session sesssion, Event event) {
+  }
+
+  @Override
+  public void sessionClosed(Session session) {
+    sessionCache.remove(session);
+  }
+}

gerrit-sshd/src/main/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java

@@ -14,13 +14,16 @@
 
 package com.google.gerrit.sshd;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.google.gerrit.common.FileUtil;
+import com.google.common.base.Preconditions;
 import com.google.gerrit.reviewdb.client.AccountSshKey;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.PeerDaemonUser;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.inject.Inject;
-import com.google.inject.Singleton;
 
 import org.apache.commons.codec.binary.Base64;
 import org.apache.sshd.common.KeyPairProvider;
@@ -48,7 +51,6 @@ import java.util.Set;
 /**
  * Authenticates by public key through {@link AccountSshKey} entities.
  */
-@Singleton
 class DatabasePubKeyAuth implements PublickeyAuthenticator {
   private static final Logger log =
       LoggerFactory.getLogger(DatabasePubKeyAuth.class);
@@ -92,10 +94,11 @@ class DatabasePubKeyAuth implements PublickeyAuthenticator {
     }
   }
 
-  public boolean authenticate(String username,
+  @Override
-      final PublicKey suppliedKey, final ServerSession session) {
+  public boolean authenticate(String username, PublicKey suppliedKey,
-    final SshSession sd = session.getAttribute(SshSession.KEY);
+      ServerSession session) {
-
+    SshSession sd = session.getAttribute(SshSession.KEY);
+    Preconditions.checkState(sd.getCurrentUser() == null);
     if (PeerDaemonUser.USER_NAME.equals(username)) {
       if (myHostKeys.contains(suppliedKey)
           || getPeerKeys().contains(suppliedKey)) {
@@ -112,10 +115,10 @@ class DatabasePubKeyAuth implements PublickeyAuthenticator {
       username = username.toLowerCase(Locale.US);
     }
 
-    final Iterable<SshKeyCacheEntry> keyList = sshKeyCache.get(username);
+    Iterable<SshKeyCacheEntry> keyList = sshKeyCache.get(username);
-    final SshKeyCacheEntry key = find(keyList, suppliedKey);
+    SshKeyCacheEntry key = find(keyList, suppliedKey);
     if (key == null) {
-      final String err;
+      String err;
       if (keyList == SshKeyCacheImpl.NO_SUCH_USER) {
         err = "user-not-found";
       } else if (keyList == SshKeyCacheImpl.NO_KEYS) {
@@ -133,7 +136,7 @@ class DatabasePubKeyAuth implements PublickeyAuthenticator {
     // security check to ensure there aren't two users sharing the same
     // user name on the server.
     //
-    for (final SshKeyCacheEntry otherKey : keyList) {
+    for (SshKeyCacheEntry otherKey : keyList) {
       if (!key.getAccount().equals(otherKey.getAccount())) {
         sd.authenticationError(username, "keys-cross-accounts");
         return false;

gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshModule.java

@@ -81,7 +81,7 @@ public class SshModule extends LifecycleModule {
     bind(QueueProvider.class).to(CommandExecutorQueueProvider.class).in(SINGLETON);
 
     bind(GSSAuthenticator.class).to(GerritGSSAuthenticator.class);
-    bind(PublickeyAuthenticator.class).to(DatabasePubKeyAuth.class);
+    bind(PublickeyAuthenticator.class).to(CachingPublicKeyAuthenticator.class);
 
     bind(ModuleGenerator.class).to(SshAutoRegisterModuleGenerator.class);
     bind(SshPluginStarterCallback.class);