From 039686c54a947f166ba80d79187ba945cac77ad5 Mon Sep 17 00:00:00 2001
From: James Moger <james.moger@gitblit.com>
Date: Fri, 25 Apr 2014 14:44:25 -0400
Subject: [PATCH] Prevent adding empty or invalid SSH public keys
---
src/main/java/com/gitblit/transport/ssh/SshKey.java | 2 +-
src/test/java/com/gitblit/tests/SshKeysDispatcherTest.java | 14 ++++++++++++++
src/main/java/com/gitblit/transport/ssh/keys/BaseKeyCommand.java | 15 +++++++++------
releases.moxie | 2 ++
src/main/java/com/gitblit/transport/ssh/keys/KeysDispatcher.java | 13 +++++++++++++
5 files changed, 39 insertions(+), 7 deletions(-)
diff --git a/releases.moxie b/releases.moxie
index 12b82e7..207209c 100644
--- a/releases.moxie
+++ b/releases.moxie
@@ -12,6 +12,7 @@
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 @@
- 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 c2fc91c..ab44854 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 @@
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 588770f..4b1d6b8 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 @@
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 53033d3..da58584 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 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 dbe4bce..23e6179 100644
--- a/src/test/java/com/gitblit/tests/SshKeysDispatcherTest.java
+++ b/src/test/java/com/gitblit/tests/SshKeysDispatcherTest.java
@@ -103,6 +103,20 @@
}
@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()));
--
Gitblit v1.9.1