From aaecd8f2a36d2c0d780b42425aa57725fe708551 Mon Sep 17 00:00:00 2001
From: James Moger <james.moger@gitblit.com>
Date: Thu, 10 Apr 2014 18:58:08 -0400
Subject: [PATCH] Move cache to IKeyManager and implement isStale() in FileKeyManager
---
src/main/java/com/gitblit/transport/ssh/commands/AddKeyCommand.java | 1
src/main/java/com/gitblit/transport/ssh/commands/SetAccountCommand.java | 1
src/main/java/com/gitblit/transport/ssh/FileKeyManager.java | 52 +++++++++++-
src/main/java/com/gitblit/transport/ssh/IKeyManager.java | 65 ++++++++++++---
src/main/java/com/gitblit/transport/ssh/commands/RemoveKeyCommand.java | 1
src/main/java/com/gitblit/transport/ssh/SshKeyAuthenticator.java | 46 ++--------
src/main/java/com/gitblit/transport/ssh/NullKeyManager.java | 19 +++-
7 files changed, 119 insertions(+), 66 deletions(-)
diff --git a/src/main/java/com/gitblit/transport/ssh/FileKeyManager.java b/src/main/java/com/gitblit/transport/ssh/FileKeyManager.java
index 1eb470b..ae0bc9c 100644
--- a/src/main/java/com/gitblit/transport/ssh/FileKeyManager.java
+++ b/src/main/java/com/gitblit/transport/ssh/FileKeyManager.java
@@ -21,6 +21,8 @@
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
import org.apache.commons.codec.binary.Base64;
import org.apache.sshd.common.util.Buffer;
@@ -38,12 +40,15 @@
* @author James Moger
*
*/
-public class FileKeyManager implements IKeyManager {
+public class FileKeyManager extends IKeyManager {
protected final IRuntimeManager runtimeManager;
+ protected final Map<File, Long> lastModifieds;
+
public FileKeyManager(IRuntimeManager runtimeManager) {
this.runtimeManager = runtimeManager;
+ this.lastModifieds = new ConcurrentHashMap<File, Long>();
}
@Override
@@ -68,15 +73,34 @@
}
@Override
- public List<PublicKey> getKeys(String username) {
+ protected boolean isStale(String username) {
+ File keystore = getKeystore(username);
+ if (!keystore.exists()) {
+ // keystore may have been deleted
+ return true;
+ }
+
+ if (lastModifieds.containsKey(keystore)) {
+ // compare modification times
+ long lastModified = lastModifieds.get(keystore);
+ return lastModified != keystore.lastModified();
+ }
+
+ // assume stale
+ return true;
+ }
+
+ @Override
+ protected List<PublicKey> getKeysImpl(String username) {
try {
- File keys = getKeystore(username);
- if (!keys.exists()) {
+ log.info("loading keystore for {}", username);
+ File keystore = getKeystore(username);
+ if (!keystore.exists()) {
return null;
}
- if (keys.exists()) {
+ if (keystore.exists()) {
List<PublicKey> list = new ArrayList<PublicKey>();
- for (String entry : Files.readLines(keys, Charsets.ISO_8859_1)) {
+ for (String entry : Files.readLines(keystore, Charsets.ISO_8859_1)) {
if (entry.trim().length() == 0) {
// skip blanks
continue;
@@ -93,6 +117,8 @@
if (list.isEmpty()) {
return null;
}
+
+ lastModifieds.put(keystore, keystore.lastModified());
return list;
}
} catch (IOException e) {
@@ -140,6 +166,9 @@
// write keystore
String content = Joiner.on("\n").join(lines).trim().concat("\n");
Files.write(content, keystore, Charsets.ISO_8859_1);
+
+ lastModifieds.remove(keystore);
+ keyCache.invalidate(username);
return true;
} catch (IOException e) {
throw new RuntimeException("Cannot add ssh key", e);
@@ -183,6 +212,9 @@
String content = Joiner.on("\n").join(lines).trim().concat("\n");
Files.write(content, keystore, Charsets.ISO_8859_1);
}
+
+ lastModifieds.remove(keystore);
+ keyCache.invalidate(username);
return true;
}
} catch (IOException e) {
@@ -193,7 +225,13 @@
@Override
public boolean removeAllKeys(String username) {
- return getKeystore(username).delete();
+ File keystore = getKeystore(username);
+ if (keystore.delete()) {
+ lastModifieds.remove(keystore);
+ keyCache.invalidate(username);
+ return true;
+ }
+ return false;
}
protected File getKeystore(String username) {
diff --git a/src/main/java/com/gitblit/transport/ssh/IKeyManager.java b/src/main/java/com/gitblit/transport/ssh/IKeyManager.java
index cb32a02..12fce3d 100644
--- a/src/main/java/com/gitblit/transport/ssh/IKeyManager.java
+++ b/src/main/java/com/gitblit/transport/ssh/IKeyManager.java
@@ -16,26 +16,63 @@
package com.gitblit.transport.ssh;
import java.security.PublicKey;
+import java.text.MessageFormat;
import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
/**
- *
+ *
* @author James Moger
*
*/
-public interface IKeyManager {
+public abstract class IKeyManager {
- IKeyManager start();
-
- boolean isReady();
-
- IKeyManager stop();
-
- List<PublicKey> getKeys(String username);
-
- boolean addKey(String username, String data);
-
- boolean removeKey(String username, String data);
+ protected final Logger log = LoggerFactory.getLogger(getClass());
- boolean removeAllKeys(String username);
+ protected final LoadingCache<String, List<PublicKey>> keyCache = CacheBuilder
+ .newBuilder().
+ expireAfterAccess(15, TimeUnit.MINUTES).
+ maximumSize(100)
+ .build(new CacheLoader<String, List<PublicKey>>() {
+ @Override
+ public List<PublicKey> load(String username) {
+ return getKeysImpl(username);
+ }
+ });
+
+ public abstract IKeyManager start();
+
+ public abstract boolean isReady();
+
+ public abstract IKeyManager stop();
+
+ public final List<PublicKey> getKeys(String username) {
+ try {
+ if (isStale(username)) {
+ keyCache.invalidate(username);
+ }
+ return keyCache.get(username);
+ } catch (ExecutionException e) {
+ log.error(MessageFormat.format("failed to retrieve keys for {0}", username), e);
+ }
+ return null;
+ }
+
+ protected abstract boolean isStale(String username);
+
+ protected abstract List<PublicKey> getKeysImpl(String username);
+
+ public abstract boolean addKey(String username, String data);
+
+ public abstract boolean removeKey(String username, String data);
+
+ public abstract boolean removeAllKeys(String username);
}
diff --git a/src/main/java/com/gitblit/transport/ssh/NullKeyManager.java b/src/main/java/com/gitblit/transport/ssh/NullKeyManager.java
index 454d3cf..c76728d 100644
--- a/src/main/java/com/gitblit/transport/ssh/NullKeyManager.java
+++ b/src/main/java/com/gitblit/transport/ssh/NullKeyManager.java
@@ -20,15 +20,15 @@
/**
* Rejects all SSH key management requests.
- *
+ *
* @author James Moger
*
*/
-public class NullKeyManager implements IKeyManager {
+public class NullKeyManager extends IKeyManager {
public NullKeyManager() {
}
-
+
@Override
public String toString() {
return getClass().getSimpleName();
@@ -38,19 +38,24 @@
public NullKeyManager start() {
return this;
}
-
+
@Override
public boolean isReady() {
return true;
}
-
+
@Override
public NullKeyManager stop() {
return this;
}
@Override
- public List<PublicKey> getKeys(String username) {
+ protected boolean isStale(String username) {
+ return false;
+ }
+
+ @Override
+ protected List<PublicKey> getKeysImpl(String username) {
return null;
}
@@ -58,7 +63,7 @@
public boolean addKey(String username, String data) {
return false;
}
-
+
@Override
public boolean removeKey(String username, String data) {
return false;
diff --git a/src/main/java/com/gitblit/transport/ssh/SshKeyAuthenticator.java b/src/main/java/com/gitblit/transport/ssh/SshKeyAuthenticator.java
index 3631922..922f25a 100644
--- a/src/main/java/com/gitblit/transport/ssh/SshKeyAuthenticator.java
+++ b/src/main/java/com/gitblit/transport/ssh/SshKeyAuthenticator.java
@@ -18,8 +18,6 @@
import java.security.PublicKey;
import java.util.List;
import java.util.Locale;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeUnit;
import org.apache.sshd.server.PublickeyAuthenticator;
import org.apache.sshd.server.session.ServerSession;
@@ -28,10 +26,6 @@
import com.gitblit.manager.IAuthenticationManager;
import com.gitblit.models.UserModel;
-import com.google.common.cache.Cache;
-import com.google.common.cache.CacheBuilder;
-import com.google.common.cache.CacheLoader;
-import com.google.common.cache.LoadingCache;
/**
*
@@ -45,17 +39,6 @@
protected final IKeyManager keyManager;
protected final IAuthenticationManager authManager;
-
- LoadingCache<String, List<PublicKey>> sshKeyCache = CacheBuilder
- .newBuilder().
- expireAfterAccess(15, TimeUnit.MINUTES).
- maximumSize(100)
- .build(new CacheLoader<String, List<PublicKey>>() {
- @Override
- public List<PublicKey> load(String username) {
- return keyManager.getKeys(username);
- }
- });
public SshKeyAuthenticator(IKeyManager keyManager, IAuthenticationManager authManager) {
this.keyManager = keyManager;
@@ -74,23 +57,20 @@
}
username = username.toLowerCase(Locale.US);
- try {
- List<PublicKey> keys = sshKeyCache.get(username);
- if (keys == null || keys.isEmpty()) {
- log.info("{} has not added any public keys for ssh authentication", username);
- return false;
- }
+ List<PublicKey> keys = keyManager.getKeys(username);
+ if (keys == null || keys.isEmpty()) {
+ log.info("{} has not added any public keys for ssh authentication", username);
+ return false;
+ }
- for (PublicKey key : keys) {
- if (key.equals(suppliedKey)) {
- UserModel user = authManager.authenticate(username, key);
- if (user != null) {
- client.setUser(user);
- return true;
- }
+ for (PublicKey key : keys) {
+ if (key.equals(suppliedKey)) {
+ UserModel user = authManager.authenticate(username, key);
+ if (user != null) {
+ client.setUser(user);
+ return true;
}
}
- } catch (ExecutionException e) {
}
log.warn("could not authenticate {} for SSH using the supplied public key", username);
@@ -99,9 +79,5 @@
public IKeyManager getKeyManager() {
return keyManager;
- }
-
- public Cache<String, List<PublicKey>> getKeyCache() {
- return sshKeyCache;
}
}
diff --git a/src/main/java/com/gitblit/transport/ssh/commands/AddKeyCommand.java b/src/main/java/com/gitblit/transport/ssh/commands/AddKeyCommand.java
index 69c4fec..35bb1bb 100644
--- a/src/main/java/com/gitblit/transport/ssh/commands/AddKeyCommand.java
+++ b/src/main/java/com/gitblit/transport/ssh/commands/AddKeyCommand.java
@@ -49,6 +49,5 @@
keyManager.addKey(username, key);
log.info("added SSH public key for {}", username);
}
- authenticator.getKeyCache().invalidate(username);
}
}
diff --git a/src/main/java/com/gitblit/transport/ssh/commands/RemoveKeyCommand.java b/src/main/java/com/gitblit/transport/ssh/commands/RemoveKeyCommand.java
index 0d49164..90e7041 100644
--- a/src/main/java/com/gitblit/transport/ssh/commands/RemoveKeyCommand.java
+++ b/src/main/java/com/gitblit/transport/ssh/commands/RemoveKeyCommand.java
@@ -57,6 +57,5 @@
log.info("removed SSH public key from {}", username);
}
}
- authenticator.getKeyCache().invalidate(username);
}
}
diff --git a/src/main/java/com/gitblit/transport/ssh/commands/SetAccountCommand.java b/src/main/java/com/gitblit/transport/ssh/commands/SetAccountCommand.java
index a22ca85..1f0d902 100644
--- a/src/main/java/com/gitblit/transport/ssh/commands/SetAccountCommand.java
+++ b/src/main/java/com/gitblit/transport/ssh/commands/SetAccountCommand.java
@@ -65,7 +65,6 @@
if (!deleteSshKeys.isEmpty()) {
deleteSshKeys(deleteSshKeys);
}
- authenticator.getKeyCache().invalidate(user);
}
private void addSshKeys(List<String> sshKeys) throws UnloggedFailure,
--
Gitblit v1.9.1