Handle ssh keys as objects, not strings, and improve the ls and rm key comm...

James Moger

2014-03-22 bcc8a015ae552726742b4f437b2cb9e809270f96

Handle ssh keys as objects, not strings, and improve the ls and rm key commands

"gitblit keys ls" now defaults to showing an indexed list of fingerprints which almost matches the output of "sshadd -l".  The indexes are useful specifying key(s) to remove using "gitblit keys rm <index>".  This is an important improvement for key management.

 src/main/java/com/gitblit/manager/AuthenticationManager.java

@@ -17,7 +17,6 @@

import java.nio.charset.Charset;
import java.security.Principal;
import java.security.PublicKey;
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.HashMap;
@@ -48,6 +47,7 @@
import com.gitblit.auth.WindowsAuthProvider;
import com.gitblit.models.TeamModel;
import com.gitblit.models.UserModel;
import com.gitblit.transport.ssh.SshKey;
import com.gitblit.utils.Base64;
import com.gitblit.utils.HttpUtils;
import com.gitblit.utils.StringUtils;
@@ -160,7 +160,7 @@
        }
        return this;
    }
	

    public void addAuthenticationProvider(AuthenticationProvider prov) {
        authenticationProviders.add(prov);
    }
@@ -301,7 +301,7 @@
     * @return a user object or null
     */
    @Override
    public UserModel authenticate(String username, PublicKey key) {
    public UserModel authenticate(String username, SshKey key) {
        if (username != null) {
            if (!StringUtils.isEmpty(username)) {
                UserModel user = userManager.getUserModel(username);
@@ -391,14 +391,14 @@
                }
            }
        }
		

        // could not authenticate locally or with a provider
        return null;
    }
	

    /**
     * Returns a UserModel if local authentication succeeds.
     * 
     *
     * @param user
     * @param password
     * @return a UserModel if local authentication succeeds, null otherwise

 src/main/java/com/gitblit/manager/GitblitManager.java

@@ -22,7 +22,6 @@
import java.io.InputStream;
import java.io.InputStreamReader;
import java.lang.reflect.Type;
import java.security.PublicKey;
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.Collection;
@@ -72,6 +71,7 @@
import com.gitblit.models.UserModel;
import com.gitblit.tickets.ITicketService;
import com.gitblit.transport.ssh.IPublicKeyManager;
import com.gitblit.transport.ssh.SshKey;
import com.gitblit.utils.ArrayUtils;
import com.gitblit.utils.HttpUtils;
import com.gitblit.utils.JsonUtils;
@@ -670,7 +670,7 @@
    }

    @Override
    public UserModel authenticate(String username, PublicKey key) {
    public UserModel authenticate(String username, SshKey key) {
        return authenticationManager.authenticate(username, key);
    }


 src/main/java/com/gitblit/manager/IAuthenticationManager.java

@@ -15,13 +15,12 @@
 */
package com.gitblit.manager;

import java.security.PublicKey;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import com.gitblit.models.TeamModel;
import com.gitblit.models.UserModel;
import com.gitblit.transport.ssh.SshKey;

public interface IAuthenticationManager extends IManager {

@@ -36,13 +35,13 @@
    UserModel authenticate(HttpServletRequest httpRequest);

    /**
     * Authenticate a user based on a public key.
     * Authenticate a user based on a ssh public key.
     *
     * @param username
     * @param key
     * @return a user object or null
     */
    UserModel authenticate(String username, PublicKey key);
    UserModel authenticate(String username, SshKey key);

    /**
     * Authenticate a user based on HTTP request parameters.

 src/main/java/com/gitblit/transport/ssh/CachingPublicKeyAuthenticator.java

@@ -78,14 +78,14 @@
        SshDaemonClient client = session.getAttribute(SshDaemonClient.KEY);
        Preconditions.checkState(client.getUser() == null);
        username = username.toLowerCase(Locale.US);
        List<PublicKey> keys = keyManager.getKeys(username);
        List<SshKey> 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) {
        for (SshKey key : keys) {
            if (key.equals(suppliedKey)) {
                UserModel user = authManager.authenticate(username, key);
                if (user != null) {

 src/main/java/com/gitblit/transport/ssh/FileKeyManager.java

@@ -17,16 +17,11 @@

import java.io.File;
import java.io.IOException;
import java.security.PublicKey;
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;
import org.eclipse.jgit.lib.Constants;

import com.gitblit.Keys;
import com.gitblit.manager.IRuntimeManager;
@@ -92,7 +87,7 @@
    }

    @Override
    protected List<PublicKey> getKeysImpl(String username) {
    protected List<SshKey> getKeysImpl(String username) {
        try {
            log.info("loading keystore for {}", username);
            File keystore = getKeystore(username);
@@ -100,7 +95,7 @@
                return null;
            }
            if (keystore.exists()) {
                List<PublicKey> list = new ArrayList<PublicKey>();
                List<SshKey> list = new ArrayList<SshKey>();
                for (String entry : Files.readLines(keystore, Charsets.ISO_8859_1)) {
                    if (entry.trim().length() == 0) {
                        // skip blanks
@@ -110,9 +105,8 @@
                        // skip comments
                        continue;
                    }
                    final String[] parts = entry.split(" ");
                    final byte[] bin = Base64.decodeBase64(Constants.encodeASCII(parts[1]));
                    list.add(new Buffer(bin).getRawPublicKey());
                    SshKey key = new SshKey(entry);
                    list.add(key);
                }

                if (list.isEmpty()) {
@@ -133,9 +127,9 @@
     * by disregarding the comment/description field during key comparisons.
     */
    @Override
    public boolean addKey(String username, String data) {
    public boolean addKey(String username, SshKey key) {
        try {
            String newKey = stripCommentFromKey(data);
            String newKey = stripCommentFromKey(key.getRawData());

            List<String> lines = new ArrayList<String>();
            File keystore = getKeystore(username);
@@ -162,7 +156,7 @@
            }

            // add new key
            lines.add(data);
            lines.add(key.getRawData());

            // write keystore
            String content = Joiner.on("\n").join(lines).trim().concat("\n");
@@ -177,12 +171,12 @@
    }

    /**
     * Removes a key from the keystore.
     * Removes the specified key from the keystore.
     */
    @Override
    public boolean removeKey(String username, String data) {
    public boolean removeKey(String username, SshKey key) {
        try {
            String rmKey = stripCommentFromKey(data);
            String rmKey = stripCommentFromKey(key.getRawData());

            File keystore = getKeystore(username);
            if (keystore.exists()) {
@@ -244,7 +238,7 @@

    /* Strips the comment from the key data and eliminates whitespace diffs */
    protected String stripCommentFromKey(String data) {
        String [] cols = data.split(" ");
        String [] cols = data.split(" ", 3);
        String key = Joiner.on(" ").join(cols[0], cols[1]);
        return key;
    }

 src/main/java/com/gitblit/transport/ssh/IPublicKeyManager.java

@@ -15,7 +15,6 @@
 */
package com.gitblit.transport.ssh;

import java.security.PublicKey;
import java.text.MessageFormat;
import java.util.List;
import java.util.concurrent.ExecutionException;
@@ -31,7 +30,7 @@
import com.google.common.cache.LoadingCache;

/**
 * Parent class for public key managers.
 * Parent class for ssh public key managers.
 *
 * @author James Moger
 *
@@ -40,13 +39,13 @@

    protected final Logger log = LoggerFactory.getLogger(getClass());

    protected final LoadingCache<String, List<PublicKey>> keyCache = CacheBuilder
    protected final LoadingCache<String, List<SshKey>> keyCache = CacheBuilder
            .newBuilder().
            expireAfterAccess(15, TimeUnit.MINUTES).
            maximumSize(100)
            .build(new CacheLoader<String, List<PublicKey>>() {
            .build(new CacheLoader<String, List<SshKey>>() {
                @Override
                public List<PublicKey> load(String username) {
                public List<SshKey> load(String username) {
                    return getKeysImpl(username);
                }
            });
@@ -59,7 +58,7 @@
    @Override
    public abstract IPublicKeyManager stop();

    public final List<PublicKey> getKeys(String username) {
    public final List<SshKey> getKeys(String username) {
        try {
            if (isStale(username)) {
                keyCache.invalidate(username);
@@ -77,11 +76,11 @@

    protected abstract boolean isStale(String username);

    protected abstract List<PublicKey> getKeysImpl(String username);
    protected abstract List<SshKey> getKeysImpl(String username);

    public abstract boolean addKey(String username, String data);
    public abstract boolean addKey(String username, SshKey key);

    public abstract boolean removeKey(String username, String data);
    public abstract boolean removeKey(String username, SshKey key);

    public abstract boolean removeAllKeys(String username);
}

 src/main/java/com/gitblit/transport/ssh/MemoryKeyManager.java

@@ -15,7 +15,6 @@
 */
package com.gitblit.transport.ssh;

import java.security.PublicKey;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
@@ -29,10 +28,10 @@
 */
public class MemoryKeyManager extends IPublicKeyManager {

    Map<String, List<PublicKey>> keys;
    Map<String, List<SshKey>> keys;

    public MemoryKeyManager() {
        keys = new HashMap<String, List<PublicKey>>();
        keys = new HashMap<String, List<SshKey>>();
    }

    @Override
@@ -62,7 +61,7 @@
    }

    @Override
    protected List<PublicKey> getKeysImpl(String username) {
    protected List<SshKey> getKeysImpl(String username) {
        String id = username.toLowerCase();
        if (keys.containsKey(id)) {
            return keys.get(id);
@@ -71,13 +70,21 @@
    }

    @Override
    public boolean addKey(String username, String data) {
        return false;
    public boolean addKey(String username, SshKey key) {
        String id = username.toLowerCase();
        if (!keys.containsKey(id)) {
            keys.put(id, new ArrayList<SshKey>());
        }
        return keys.get(id).add(key);
    }

    @Override
    public boolean removeKey(String username, String data) {
        return false;
    public boolean removeKey(String username, SshKey key) {
        String id = username.toLowerCase();
        if (!keys.containsKey(id)) {
            return false;
        }
        return keys.get(id).remove(key);
    }

    @Override
@@ -85,14 +92,5 @@
        String id = username.toLowerCase();
        keys.remove(id.toLowerCase());
        return true;
    }

    /* Test method for populating the memory key manager */
    public void addKey(String username, PublicKey key) {
        String id = username.toLowerCase();
        if (!keys.containsKey(id)) {
            keys.put(id, new ArrayList<PublicKey>());
        }
        keys.get(id).add(key);
    }
}

 src/main/java/com/gitblit/transport/ssh/NullKeyManager.java

@@ -15,7 +15,6 @@
 */
package com.gitblit.transport.ssh;

import java.security.PublicKey;
import java.util.List;

/**
@@ -56,17 +55,17 @@
    }

    @Override
    protected List<PublicKey> getKeysImpl(String username) {
    protected List<SshKey> getKeysImpl(String username) {
        return null;
    }

    @Override
    public boolean addKey(String username, String data) {
    public boolean addKey(String username, SshKey key) {
        return false;
    }

    @Override
    public boolean removeKey(String username, String data) {
    public boolean removeKey(String username, SshKey key) {
        return false;
    }


 src/main/java/com/gitblit/transport/ssh/SshKey.java

New file
@@ -0,0 +1,156 @@
package com.gitblit.transport.ssh;

import java.io.Serializable;
import java.security.PublicKey;

import org.apache.commons.codec.binary.Base64;
import org.apache.sshd.common.SshException;
import org.apache.sshd.common.util.Buffer;
import org.eclipse.jgit.lib.Constants;

import com.gitblit.utils.StringUtils;

/**
 * Class that encapsulates a public SSH key and it's metadata.
 *
 * @author James Moger
 *
 */
public class SshKey implements Serializable {

    private static final long serialVersionUID = 1L;

    private String rawData;

    private PublicKey publicKey;

    private String comment;

    private String fingerprint;

    public SshKey(String data) {
        this.rawData = data;
    }

    public SshKey(PublicKey key) {
        this.publicKey = key;
        this.comment = "";
    }

    public PublicKey getPublicKey() {
        if (publicKey == null && rawData != null) {
            // instantiate the public key from the raw key data
            final String[] parts = rawData.split(" ", 3);
            if (comment == null && parts.length == 3) {
                comment = parts[2];
            }
            final byte[] bin = Base64.decodeBase64(Constants.encodeASCII(parts[1]));
            try {
                publicKey = new Buffer(bin).getRawPublicKey();
            } catch (SshException e) {
                e.printStackTrace();
            }
        }
        return publicKey;
    }

    public String getAlgorithm() {
        return getPublicKey().getAlgorithm();
    }

    public String getComment() {
        if (comment == null && rawData != null) {
            // extract comment from the raw data
            final String[] parts = rawData.split(" ", 3);
            if (parts.length == 3) {
                comment = parts[2];
            }
        }
        return comment;
    }

    public void setComment(String comment) {
        this.comment = comment;
    }

    public String getRawData() {
        if (rawData == null && publicKey != null) {
            // build the raw data manually from the public key
            Buffer buf = new Buffer();

            // 1: identify the algorithm
            buf.putRawPublicKey(publicKey);
            String alg = buf.getString();

            // 2: encode the key
            buf.clear();
            buf.putPublicKey(publicKey);
            String b64 = Base64.encodeBase64String(buf.getBytes());

            String c = getComment();
            rawData = alg + " " + b64 + (StringUtils.isEmpty(c) ? "" : (" " + c));
        }
        return rawData;
    }

    public String getFingerprint() {
        if (fingerprint == null) {
            StringBuilder sb = new StringBuilder();
            // TODO append the keysize
            int keySize = 0;
            if (keySize > 0) {
                sb.append(keySize).append(' ');
            }

            // append the key hash as colon-separated pairs
            String hash;
            if (rawData != null) {
                final String[] parts = rawData.split(" ", 3);
                final byte [] bin = Base64.decodeBase64(Constants.encodeASCII(parts[1]));
                hash = StringUtils.getMD5(bin);
            } else {
                // TODO get hash from publickey
                hash = "todo";
            }
            for (int i = 0; i < hash.length(); i += 2) {
                sb.append(hash.charAt(i)).append(hash.charAt(i + 1)).append(':');
            }
            sb.setLength(sb.length() - 1);

            // append the comment
            String c = getComment();
            if (!StringUtils.isEmpty(c)) {
                sb.append(' ');
                sb.append(c);
            }

            // append the algorithm
            String alg = getAlgorithm();
            if (!StringUtils.isEmpty(alg)) {
                sb.append(" (").append(alg).append(")");
            }
            fingerprint = sb.toString();
        }
        return fingerprint;
    }

    @Override
    public boolean equals(Object o) {
        if (o instanceof PublicKey) {
            return getPublicKey().equals(o);
        } else if (o instanceof SshKey) {
            return getPublicKey().equals(((SshKey) o).getPublicKey());
        }
        return false;
    }

    @Override
    public int hashCode() {
        return getPublicKey().hashCode();
    }

    @Override
    public String toString() {
        return getFingerprint();
    }
}

 src/main/java/com/gitblit/transport/ssh/gitblit/BaseKeyCommand.java

@@ -22,6 +22,7 @@
import java.util.List;

import com.gitblit.transport.ssh.IPublicKeyManager;
import com.gitblit.transport.ssh.SshKey;
import com.gitblit.transport.ssh.commands.SshCommand;
import com.google.common.base.Charsets;

@@ -55,4 +56,12 @@
    protected IPublicKeyManager getKeyManager() {
        return getContext().getGitblit().getPublicKeyManager();
    }

    protected SshKey parseKey(String rawData) throws UnloggedFailure {
        if (rawData.contains("PRIVATE")) {
            throw new UnloggedFailure(1,  "Please provide a PUBLIC key, not a PRIVATE key!");
        }
        SshKey key = new SshKey(rawData);
        return key;
    }
}

 src/main/java/com/gitblit/transport/ssh/gitblit/KeysDispatcher.java

@@ -16,18 +16,17 @@
package com.gitblit.transport.ssh.gitblit;

import java.io.IOException;
import java.security.PublicKey;
import java.util.ArrayList;
import java.util.List;

import org.apache.commons.codec.binary.Base64;
import org.apache.sshd.common.util.Buffer;
import org.kohsuke.args4j.Argument;
import org.kohsuke.args4j.Option;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.gitblit.models.UserModel;
import com.gitblit.transport.ssh.IPublicKeyManager;
import com.gitblit.transport.ssh.SshKey;
import com.gitblit.transport.ssh.commands.CommandMetaData;
import com.gitblit.transport.ssh.commands.DispatchCommand;
import com.gitblit.transport.ssh.commands.SshCommand;
@@ -61,7 +60,8 @@
            String username = getContext().getClient().getUsername();
            List<String> keys = readKeys(addKeys);
            for (String key : keys) {
                getKeyManager().addKey(username, key);
                SshKey sshKey = parseKey(key);
                getKeyManager().addKey(username, sshKey);
                log.info("added SSH public key for {}", username);
            }
        }
@@ -74,51 +74,85 @@

        private final String ALL = "ALL";

        @Argument(metaVar = "<stdin>|<KEY>|ALL", usage = "the key to remove")
        @Argument(metaVar = "-|<INDEX>|<KEY>|ALL", usage = "the key to remove", required = true)
        private List<String> removeKeys = new ArrayList<String>();

        @Override
        public void run() throws IOException, UnloggedFailure {
            String username = getContext().getClient().getUsername();
            // remove a key that has been piped to the command
            // or remove all keys

            List<SshKey> currentKeys = getKeyManager().getKeys(username);
            if (currentKeys == null || currentKeys.isEmpty()) {
                throw new UnloggedFailure(1, "There are no registered keys!");
            }

            List<String> keys = readKeys(removeKeys);
            if (keys.contains(ALL)) {
                getKeyManager().removeAllKeys(username);
                log.info("removed all SSH public keys from {}", username);
                if (getKeyManager().removeAllKeys(username)) {
                    stdout.println("Removed all keys.");
                    log.info("removed all SSH public keys from {}", username);
                } else {
                    log.warn("failed to remove all SSH public keys from {}", username);
                }
            } else {
                for (String key : keys) {
                    getKeyManager().removeKey(username, key);
                    log.info("removed SSH public key from {}", username);
                    try {
                        // remove a key by it's index (1-based indexing)
                        int index = Integer.parseInt(key);
                        if (index > keys.size()) {
                            if (keys.size() == 1) {
                                throw new UnloggedFailure(1, "Invalid index specified. There is only 1 registered key.");
                            }
                            throw new UnloggedFailure(1, String.format("Invalid index specified. There are %d registered keys.", keys.size()));
                        }
                        SshKey sshKey = currentKeys.get(index - 1);
                        if (getKeyManager().removeKey(username, sshKey)) {
                            stdout.println(String.format("Removed %s", sshKey.getFingerprint()));
                        } else {
                            throw new UnloggedFailure(1,  String.format("failed to remove #%s: %s", key, sshKey.getFingerprint()));
                        }
                    } catch (Exception e) {
                        // remove key by raw key data
                        SshKey sshKey = parseKey(key);
                        if (getKeyManager().removeKey(username, sshKey)) {
                            stdout.println(String.format("Removed %s", sshKey.getFingerprint()));
                            log.info("removed SSH public key {} from {}", sshKey.getFingerprint(), username);
                        } else {
                            log.warn("failed to remove SSH public key {} from {}", sshKey.getFingerprint(), username);
                            throw new UnloggedFailure(1,  String.format("failed to remove %s", sshKey.getFingerprint()));
                        }
                    }
                }
            }
        }
    }

    @CommandMetaData(name = "list", aliases = { "ls" }, description = "List your public keys")
    @CommandMetaData(name = "list", aliases = { "ls" }, description = "List your registered public keys")
    public static class ListKeys extends SshCommand {

        @Option(name = "-L", usage = "list complete public key parameters")
        private boolean showRaw;

        @Override
        public void run() {
            IPublicKeyManager keyManager = getContext().getGitblit().getPublicKeyManager();
            String username = getContext().getClient().getUsername();
            List<PublicKey> keys = keyManager.getKeys(username);
            if (keys == null) {
                stdout.println(String.format("%s has not added any public keys for ssh authentication", username));
            List<SshKey> keys = keyManager.getKeys(username);
            if (keys == null || keys.isEmpty()) {
                stdout.println("You have not registered any public keys for ssh authentication.");
                return;
            }
            for (PublicKey key : keys) {
                // two-steps - perhaps this could be improved
                Buffer buf = new Buffer();

                // 1: identify the algorithm
                buf.putRawPublicKey(key);
                String alg = buf.getString();

                // 2: encode the key
                buf.clear();
                buf.putPublicKey(key);
                String b64 = Base64.encodeBase64String(buf.getBytes());

                stdout.println(alg + " " + b64);
            for (int i = 0; i < keys.size(); i++) {
                if (showRaw) {
                    // output in the same format as authorized_keys
                    stdout.println(keys.get(i).getRawData());
                } else {
                    // show 1-based index numbers with the fingerprint
                    // this is useful for comparing with "ssh-add -l"
                    stdout.println("#" + (i + 1) + ": " + keys.get(i).getFingerprint());
                }
            }
        }
    }

 src/main/java/com/gitblit/transport/ssh/gitblit/SetAccountCommand.java

@@ -22,6 +22,7 @@
import org.kohsuke.args4j.Argument;
import org.kohsuke.args4j.Option;

import com.gitblit.transport.ssh.SshKey;
import com.gitblit.transport.ssh.commands.CommandMetaData;

/** Set a user's account settings. **/
@@ -66,18 +67,20 @@
        }
    }

    private void addSshKeys(List<String> sshKeys) throws UnloggedFailure,
    private void addSshKeys(List<String> keys) throws UnloggedFailure,
            IOException {
        for (String sshKey : sshKeys) {
        for (String key : keys) {
            SshKey sshKey = new SshKey(key);
            getKeyManager().addKey(user, sshKey);
        }
    }

    private void deleteSshKeys(List<String> sshKeys) {
        if (sshKeys.contains(ALL)) {
    private void deleteSshKeys(List<String> keys) {
        if (keys.contains(ALL)) {
            getKeyManager().removeAllKeys(user);
        } else {
            for (String sshKey : sshKeys) {
            for (String key : keys) {
                SshKey sshKey = new SshKey(key);
                getKeyManager().removeKey(user, sshKey);
            }
        }

 src/test/java/com/gitblit/tests/SshDaemonTest.java

@@ -35,6 +35,7 @@
import com.gitblit.Constants;
import com.gitblit.transport.ssh.IPublicKeyManager;
import com.gitblit.transport.ssh.MemoryKeyManager;
import com.gitblit.transport.ssh.SshKey;

public class SshDaemonTest extends GitblitUnitTest {

@@ -66,7 +67,7 @@
    @Before
    public void prepare() {
        MemoryKeyManager keyMgr = getKeyManager();
        keyMgr.addKey("admin", pair.getPublic());
        keyMgr.addKey("admin", new SshKey(pair.getPublic()));
    }

    @After