Prevent adding empty or invalid SSH public keys

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
 }
 
 #

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;

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;

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)) {

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);