Prevent adding empty or invalid SSH public keys

diff --git a/releases.moxie b/releases.moxie
index 12b82e7ae879a4f9b48cd8c0ad3d55d0fbcf023a..207209cd15d34f5a6929d2fc71e4fd58c763b4d8 100644 (file)
--- a/releases.moxie
+++ b/releases.moxie
@@ -12,6 +12,7 @@ r23: {
     fixes:
     - Fix subdirectory links in pages servlet (issue-411)
     - Fix subdirectory navigation in pages servlet (issue-412)
+    - Fix bug in adding invalid or empty SSH keys (ticket-50)
     changes:
     - improve French translation (pr-176)
     - simplify current plugin release detection and ignore the currentRelease registry field 
@@ -23,6 +24,7 @@ r23: {
     - Julien Kirch
     - Ralph Hoffman
     - Olivier Rouits
+    - Owen Nelson
 }
 
 #

diff --git a/src/main/java/com/gitblit/transport/ssh/SshKey.java b/src/main/java/com/gitblit/transport/ssh/SshKey.java
index c2fc91c10c6fb028614145c18c75b7a171af1f34..ab44854a5cf5acf4858bd4f808e9f583fea36272 100644 (file)
--- a/src/main/java/com/gitblit/transport/ssh/SshKey.java
+++ b/src/main/java/com/gitblit/transport/ssh/SshKey.java
@@ -72,7 +72,7 @@ public class SshKey implements Serializable {
                        try {
                                publicKey = new Buffer(bin).getRawPublicKey();
                        } catch (SshException e) {
-                               e.printStackTrace();
+                               throw new RuntimeException(e);
                        }
                }
                return publicKey;

diff --git a/src/main/java/com/gitblit/transport/ssh/keys/BaseKeyCommand.java b/src/main/java/com/gitblit/transport/ssh/keys/BaseKeyCommand.java
index 588770f40fc2e2107b843682144d862232d7b8ad..4b1d6b8ff6370d26b229efeb195772fef8a94002 100644 (file)
--- a/src/main/java/com/gitblit/transport/ssh/keys/BaseKeyCommand.java
+++ b/src/main/java/com/gitblit/transport/ssh/keys/BaseKeyCommand.java
@@ -37,17 +37,20 @@ abstract class BaseKeyCommand extends SshCommand {
                        throws UnsupportedEncodingException, IOException {
                int idx = -1;
                if (sshKeys.isEmpty() || (idx = sshKeys.indexOf("-")) >= 0) {
-                       String sshKey = "";
+                       String content = "";
                        BufferedReader br = new BufferedReader(new InputStreamReader(
                                        in, Charsets.UTF_8));
                        String line;
                        while ((line = br.readLine()) != null) {
-                               sshKey += line + "\n";
+                               content += line + "\n";
                        }
-                       if (idx == -1) {
-                               sshKeys.add(sshKey.trim());
-                       } else {
-                               sshKeys.set(idx, sshKey.trim());
+                       final String sshKey = content.trim();
+                       if (!sshKey.isEmpty()) {
+                               if (idx == -1) {
+                                       sshKeys.add(sshKey);
+                               } else {
+                                       sshKeys.set(idx, sshKey);
+                               }
                        }
                }
                return sshKeys;

diff --git a/src/main/java/com/gitblit/transport/ssh/keys/KeysDispatcher.java b/src/main/java/com/gitblit/transport/ssh/keys/KeysDispatcher.java
index 53033d3dd2627ef6e25c4ed94aa7a0534a766037..da58584c955f457ce9f0bc2091be4cdbbbe0ec4b 100644 (file)
--- a/src/main/java/com/gitblit/transport/ssh/keys/KeysDispatcher.java
+++ b/src/main/java/com/gitblit/transport/ssh/keys/KeysDispatcher.java
@@ -79,8 +79,21 @@ public class KeysDispatcher extends DispatchCommand {
                public void run() throws IOException, Failure {
                        String username = getContext().getClient().getUsername();
                        List<String> keys = readKeys(addKeys);
+                       if (keys.isEmpty()) {
+                               throw new UnloggedFailure("No public keys were read from STDIN!");
+                       }
                        for (String key : keys) {
                                SshKey sshKey = parseKey(key);
+                               try {
+                                       // this method parses the rawdata and produces a public key
+                                       // if it fails it will throw a Buffer.BufferException
+                                       // the null check is a QC verification on top of that
+                                       if (sshKey.getPublicKey() == null) {
+                                               throw new RuntimeException();
+                                       }
+                               } catch (RuntimeException e) {
+                                       throw new UnloggedFailure("The data read from SDTIN can not be parsed as an SSH public key!");
+                               }
                                if (!StringUtils.isEmpty(permission)) {
                                        AccessPermission ap = AccessPermission.fromCode(permission);
                                        if (ap.exceeds(AccessPermission.NONE)) {

diff --git a/src/test/java/com/gitblit/tests/SshKeysDispatcherTest.java b/src/test/java/com/gitblit/tests/SshKeysDispatcherTest.java
index dbe4bce860768531315d0df1d0825fb02cb372a2..23e6179526b5f6e98b8140a0912071c78e0461ea 100644 (file)
--- a/src/test/java/com/gitblit/tests/SshKeysDispatcherTest.java
+++ b/src/test/java/com/gitblit/tests/SshKeysDispatcherTest.java
@@ -102,6 +102,20 @@ public class SshKeysDispatcherTest extends SshUnitTest {
                assertEquals(sb.toString(), result);
        }
 
+       @Test
+       public void testKeysAddBlankCommand() throws Exception {
+               testSshCommand("keys add --permission R", "\n");
+               List<SshKey> keys = getKeyManager().getKeys(username);
+               assertEquals(String.format("There are %d keys!", keys.size()), 2, keys.size());
+       }
+
+       @Test
+       public void testKeysAddInvalidCommand() throws Exception {
+               testSshCommand("keys add --permission R", "My invalid key\n");
+               List<SshKey> keys = getKeyManager().getKeys(username);
+               assertEquals(String.format("There are %d keys!", keys.size()), 2, keys.size());
+       }
+
        @Test
        public void testKeysCommentCommand() throws Exception {
                List<SshKey> keys = getKeyManager().getKeys(username);