diff options
5 files changed, 39 insertions, 7 deletions
diff --git a/releases.moxie b/releases.moxie index 12b82e7a..207209cd 100644 --- 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 c2fc91c1..ab44854a 100644 --- 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 588770f4..4b1d6b8f 100644 --- 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 53033d3d..da58584c 100644 --- 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 dbe4bce8..23e61795 100644 --- a/src/test/java/com/gitblit/tests/SshKeysDispatcherTest.java +++ b/src/test/java/com/gitblit/tests/SshKeysDispatcherTest.java @@ -103,6 +103,20 @@ public class SshKeysDispatcherTest extends SshUnitTest { } @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); assertTrue(StringUtils.isEmpty(keys.get(0).getComment())); |