Don't permit login if we find multiple accounts with the same credentials

If there are two or more account entities with the same preferred
email address and the same public key registered its ambiguous as
to which account we should use.  Instead of chosing at random we
deny the login attempt.

Signed-off-by: Shawn O. Pearce <sop@google.com>

appjar/src/main/java/com/google/gerrit/server/ssh/DatabasePubKeyAuth.java

@@ -27,7 +27,6 @@ import java.security.NoSuchProviderException;
 import java.security.PublicKey;
 import java.security.spec.InvalidKeySpecException;
 import java.util.Collections;
-import java.util.List;
 
 /**
  * Authenticates by public key through {@link AccountSshKey} entities.
@@ -45,28 +44,53 @@ class DatabasePubKeyAuth implements PublickeyAuthenticator {
 
   public boolean hasKey(final String username, final PublicKey inkey,
       final ServerSession session) {
-    final List<AccountSshKey> keyList = SshUtil.keysFor(schema, username);
+    AccountSshKey matched = null;
-    for (final AccountSshKey k : keyList) {
+
-      try {
+    for (final AccountSshKey k : SshUtil.keysFor(schema, username)) {
-        if (SshUtil.parse(k).equals(inkey)) {
+      if (match(username, k, inkey)) {
-          updateLastUsed(k);
+        if (matched == null) {
-          session.setAttribute(SshUtil.CURRENT_ACCOUNT, k.getAccount());
+          matched = k;
-          return true;
+
+        } else if (!matched.getAccount().equals(k.getAccount())) {
+          // Don't permit keys to authenticate to different accounts
+          // that have the same username and public key.
+          //
+          // We'd have to pick one at random, yielding unpredictable
+          // behavior for the end-user.
+          //
+          return false;
         }
-      } catch (NoSuchAlgorithmException e) {
-        markInvalid(k);
-      } catch (InvalidKeySpecException e) {
-        markInvalid(k);
-      } catch (NoSuchProviderException e) {
-        markInvalid(k);
-      } catch (RuntimeException e) {
-        markInvalid(k);
       }
     }
+
+    if (matched != null) {
+      updateLastUsed(matched);
+      session.setAttribute(SshUtil.CURRENT_ACCOUNT, matched.getAccount());
+      return true;
+    }
     return false;
   }
 
-  private void markInvalid(final AccountSshKey k) {
+  private boolean match(final String username, final AccountSshKey k,
+      final PublicKey inkey) {
+    try {
+      return SshUtil.parse(k).equals(inkey);
+    } catch (NoSuchAlgorithmException e) {
+      markInvalid(username, k);
+      return false;
+    } catch (InvalidKeySpecException e) {
+      markInvalid(username, k);
+      return false;
+    } catch (NoSuchProviderException e) {
+      markInvalid(username, k);
+      return false;
+    } catch (RuntimeException e) {
+      markInvalid(username, k);
+      return false;
+    }
+  }
+
+  private void markInvalid(final String username, final AccountSshKey k) {
     try {
       final ReviewDb db = schema.open();
       try {
@@ -77,6 +101,8 @@ class DatabasePubKeyAuth implements PublickeyAuthenticator {
       }
     } catch (OrmException e) {
       // TODO log mark invalid failure
+    } finally {
+      SshUtil.invalidate(username);
     }
   }
 

appjar/src/main/java/com/google/gerrit/server/ssh/SshUtil.java

@@ -146,12 +146,17 @@ public class SshUtil {
   /** Invalidate all cached keys for the given account. */
   public static void invalidate(final Account acct) {
     if (acct != null) {
-      synchronized (keys) {
+      invalidate(acct.getPreferredEmail());
-        keys.remove(acct.getPreferredEmail());
-      }
     }
   }
 
+  /** Invalidate all cached keys for the given account. */
+  public static void invalidate(final String username){
+    synchronized (keys) {
+      keys.remove(username);
+    }    
+  }
+
   /** Locate keys for the requested account whose email matches the name given. */
   public static List<AccountSshKey> keysFor(final SchemaFactory<ReviewDb> rdf,
       final String username) {