[gnome-keyring/gnome-3-28] ssh-agent: Make public key parsing robuster



commit 65e20169b8dfc935c907ca65e54c1c6e2a365388
Author: Daiki Ueno <dueno src gnome org>
Date:   Mon Apr 30 18:03:19 2018 +0200

    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

 daemon/ssh-agent/gkd-ssh-agent-util.c      |    6 ++++++
 daemon/ssh-agent/test-gkd-ssh-agent-util.c |   24 +++++++++++++++---------
 pkcs11/ssh-store/fixtures/identity.pub     |    1 +
 3 files changed, 22 insertions(+), 9 deletions(-)
---
diff --git a/daemon/ssh-agent/gkd-ssh-agent-util.c b/daemon/ssh-agent/gkd-ssh-agent-util.c
index f0934b5..07bae1f 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 b74ccf4..2fab297 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 0000000..64f0092
--- /dev/null
+++ b/pkcs11/ssh-store/fixtures/identity.pub
@@ -0,0 +1 @@
+2048 65537 
24441362561658402203833950446201855344021432187363502135808306611576487614688480997306138918965620875403280637443583435371977323903269172664531450582985454519164856071918921465780900234446084102021505343856321702549268936741458077623280683630954129182173904049746029183391563757599728238653932509919193037170161958394083453355955428766928687086643512804493287104481453268463186578945838089020355657752804348513609803384640933423383046464181883782074377289700592413463972335302845592904197813072122904513007851760502305134812488745575825428292149365221963416490618044666955554129518253460898036309102204336193271317821
 miles@centos7


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]