From eb76024bd46d6bef098e07455dcbb7e2bb453bdd Mon Sep 17 00:00:00 2001
From: "Shawn O. Pearce" <sop@google.com>
Date: Wed, 31 Dec 2008 09:05:17 -0800
Subject: [PATCH] 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>
---
 .../gerrit/server/ssh/DatabasePubKeyAuth.java | 60 +++++++++++++------
 .../com/google/gerrit/server/ssh/SshUtil.java | 11 +++-
 2 files changed, 51 insertions(+), 20 deletions(-)

diff --git a/appjar/src/main/java/com/google/gerrit/server/ssh/DatabasePubKeyAuth.java b/appjar/src/main/java/com/google/gerrit/server/ssh/DatabasePubKeyAuth.java
index f3ae0029ff..a2889113cf 100644
--- a/appjar/src/main/java/com/google/gerrit/server/ssh/DatabasePubKeyAuth.java
+++ b/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);
-    for (final AccountSshKey k : keyList) {
-      try {
-        if (SshUtil.parse(k).equals(inkey)) {
-          updateLastUsed(k);
-          session.setAttribute(SshUtil.CURRENT_ACCOUNT, k.getAccount());
-          return true;
+    AccountSshKey matched = null;
+
+    for (final AccountSshKey k : SshUtil.keysFor(schema, username)) {
+      if (match(username, k, inkey)) {
+        if (matched == null) {
+          matched = k;
+
+        } 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);
     }
   }
 
diff --git a/appjar/src/main/java/com/google/gerrit/server/ssh/SshUtil.java b/appjar/src/main/java/com/google/gerrit/server/ssh/SshUtil.java
index 2cecdec9dc..ca18d692c3 100644
--- a/appjar/src/main/java/com/google/gerrit/server/ssh/SshUtil.java
+++ b/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) {
-        keys.remove(acct.getPreferredEmail());
-      }
+      invalidate(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) {