ssh-agent: Make public key parsing robuster

Previously, _gkd_ssh_agent_parse_public_key() accepted OpenSSH v1
keys, because the second component of the key line looks like a valid
base64 blob:

  2048 65537 2444136...

This patch checks that the component is really base64 encoded, by
checking the length is a multiple of 4.

Note that this solution is not perfect, as there could be a key with a
public exponent whose decimal length is multiple of 4.  More thorough
approach would be to call ssh-keygen -l on each public key.

https://bugzilla.gnome.org/show_bug.cgi?id=795699

3 files changed, 24 insertions, 11 deletions

diff --git a/daemon/ssh-agent/gkd-ssh-agent-util.c b/daemon/ssh-agent/gkd-ssh-agent-util.c
index f0934b5f..07bae1fe 100644
--- a/daemon/ssh-agent/gkd-ssh-agent-util.c
+++ b/daemon/ssh-agent/gkd-ssh-agent-util.c
@@ -154,6 +154,12 @@ _gkd_ssh_agent_parse_public_key (GBytes *input,
 	if (at == NULL)
 		at = data + n_data;
 
+	/* Check if the chunk is the base64 key */
+	if ((at - data) % 4 != 0) {
+		g_message ("SSH public key missing key data");
+		return NULL;
+	}
+
 	/* Decode the base64 key */
 	save = state = 0;
 	decoded = g_malloc (n_data * 3 / 4);
diff --git a/daemon/ssh-agent/test-gkd-ssh-agent-util.c b/daemon/ssh-agent/test-gkd-ssh-agent-util.c
index b74ccf4c..2fab297b 100644
--- a/daemon/ssh-agent/test-gkd-ssh-agent-util.c
+++ b/daemon/ssh-agent/test-gkd-ssh-agent-util.c
@@ -37,7 +37,9 @@ static struct {
 	{ SRCDIR "/pkcs11/ssh-store/fixtures/id_rsa_test.pub",
 	  "AAAAB3NzaC1yc2EAAAABIwAAAQEAoD6VKqkhay6pKHSRjAGWWfFPU8xfsi2gnOwP/B1UHDoztx3czhO+py/fTlhCnSP1jsjkrVIZcnzah2fUNFFRgS4+jROBtvbgHsS72V1E6+ZogV+mBJWWAhw0iPrmQ3Kvm38D3PByo5Y7yKO5kIG2LloYLjosJ5F4sx2xh0uz2wXNtnY1b5xhe2+VEksm9OB+FXaUkZC2fQrTNo8ZGFJQSFd8kUhIfbUDJmlYuZ+vvHM+A3Lc9rHyW4IPaRyxFQciRmb+ZQqU2uSdOXAhg17lskuX/q8yCI5Hy5eDicC222oUMdJTtYgwX4dQCU8TICWhxb3x4RCV+g7D99+tkIvv+w==" },
 	{ SRCDIR "/pkcs11/ssh-store/fixtures/id_dsa_test.pub",
-	  "AAAAB3NzaC1kc3MAAACBANHNmw2YHEodUj4Ae27i8Rm8uoLnpS68QEiCJx8bv9P1o0AaD0w55sH+TBzlo7vtAEDlAzIOBY3PMpy5WarELTIeXmFPzKfHL8tuxMbOPaN/wDkDZNnJZsqlyRwlQKStPcAlvLBNuMjA53u2ndMTVghtUHXETQzwxKhXf7TmvfLBAAAAFQDnF/Y8MgFCP0PpRC5ZAQo1dyDEwwAAAIEAr4iOpTeZx8i1QgQpRl+dmbBAtHTXbPiophzNJBge9lixqF0T3egN2B9wGGnumIXmnst9RPPjuu+cHCLfxhXHzLlW8MLwoiF6ZQOx9M8WcfWIl5oiGyr2e969woRf5OcMGQPOQBdws6MEtemRqq5gu6dqDqVl3xfhSZSP9LpqAI8AAACAUjiuQ3qGErsCz++qd0qrR++QA185XGXAPZqQEHcr4iKSlO17hSUYA03kOWtDaeRtJOlxjIjl9iLo3juKGFgxUfo2StScOSO2saTWFGjA4MybHCK1+mIYXRcYrq314yK2Tmbql/UGDWpcCCGXLWpSFHTaXTbJjPd6VL+TO9/8tFk=" }
+	  "AAAAB3NzaC1kc3MAAACBANHNmw2YHEodUj4Ae27i8Rm8uoLnpS68QEiCJx8bv9P1o0AaD0w55sH+TBzlo7vtAEDlAzIOBY3PMpy5WarELTIeXmFPzKfHL8tuxMbOPaN/wDkDZNnJZsqlyRwlQKStPcAlvLBNuMjA53u2ndMTVghtUHXETQzwxKhXf7TmvfLBAAAAFQDnF/Y8MgFCP0PpRC5ZAQo1dyDEwwAAAIEAr4iOpTeZx8i1QgQpRl+dmbBAtHTXbPiophzNJBge9lixqF0T3egN2B9wGGnumIXmnst9RPPjuu+cHCLfxhXHzLlW8MLwoiF6ZQOx9M8WcfWIl5oiGyr2e969woRf5OcMGQPOQBdws6MEtemRqq5gu6dqDqVl3xfhSZSP9LpqAI8AAACAUjiuQ3qGErsCz++qd0qrR++QA185XGXAPZqQEHcr4iKSlO17hSUYA03kOWtDaeRtJOlxjIjl9iLo3juKGFgxUfo2StScOSO2saTWFGjA4MybHCK1+mIYXRcYrq314yK2Tmbql/UGDWpcCCGXLWpSFHTaXTbJjPd6VL+TO9/8tFk=" },
+	{ SRCDIR "/pkcs11/ssh-store/fixtures/identity.pub",
+	  NULL }
 };
 
 #define COMMENT "A public key comment"
@@ -60,16 +62,20 @@ test_parse_public (void)
 		input_bytes = g_bytes_new_take (data, n_data);
 		output_bytes = _gkd_ssh_agent_parse_public_key (input_bytes, &comment);
 		g_bytes_unref (input_bytes);
-		g_assert (output_bytes);
-
-		blob = g_bytes_get_data (output_bytes, &n_data);
-		encoded = g_base64_encode (blob, n_data);
-		g_bytes_unref (output_bytes);
-		g_assert_cmpstr (encoded, ==, PUBLIC_FILES[i].encoded);
-		g_free (encoded);
-
-		g_assert_cmpstr (comment, ==, COMMENT);
-		g_free (comment);
+		if (PUBLIC_FILES[i].encoded == NULL) {
+			g_assert (output_bytes == NULL);
+		} else {
+			g_assert (output_bytes);
+
+			blob = g_bytes_get_data (output_bytes, &n_data);
+			encoded = g_base64_encode (blob, n_data);
+			g_bytes_unref (output_bytes);
+			g_assert_cmpstr (encoded, ==, PUBLIC_FILES[i].encoded);
+			g_free (encoded);
+
+			g_assert_cmpstr (comment, ==, COMMENT);
+			g_free (comment);
+		}
 	}
 }
 
diff --git a/pkcs11/ssh-store/fixtures/identity.pub b/pkcs11/ssh-store/fixtures/identity.pub
new file mode 100644
index 00000000..64f0092f
--- /dev/null
+++ b/pkcs11/ssh-store/fixtures/identity.pub
@@ -0,0 +1 @@
+2048 65537 24441362561658402203833950446201855344021432187363502135808306611576487614688480997306138918965620875403280637443583435371977323903269172664531450582985454519164856071918921465780900234446084102021505343856321702549268936741458077623280683630954129182173904049746029183391563757599728238653932509919193037170161958394083453355955428766928687086643512804493287104481453268463186578945838089020355657752804348513609803384640933423383046464181883782074377289700592413463972335302845592904197813072122904513007851760502305134812488745575825428292149365221963416490618044666955554129518253460898036309102204336193271317821 miles@centos7