From 4a0fa90979d43a6ba93ff8fa6a5ac81cf981e8d0 Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Sat, 5 Jan 2019 08:14:32 +0000
Subject: [PATCH] Fix wrong output from ssh1_rsa_fingerprint.

I broke it last year in commit 4988fd410, when I made hash contexts
expose a BinarySink interface. I went round finding no end of long-
winded ways of pushing things into hash contexts, often reimplementing
some standard thing like the wire formatting of an mpint, and rewrote
them more concisely using one or two put_foo calls.

But I failed to notice that the hash preimage used in SSH-1 key
fingerprints is _not_ implementable by put_ssh1_mpint! It consists of
the two public-key integers encoded in multi-byte binary big-endian
form, but without any preceding length field at all. I must have
looked too hastily, 'recognised' it as just implementing an mpint
formatter yet again, and replaced it with put_ssh1_mpint. So SSH-1 key
fingerprints have been completely wrong in the snapshots for months.

Fixed now, and this time, added a comment to warn me in case I get the
urge to simplify the code again, and a regression test in cryptsuite.
---
 sshrsa.c           | 14 ++++++++++++--
 test/cryptsuite.py | 15 +++++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/sshrsa.c b/sshrsa.c
index 32842939..fac01faf 100644
--- a/sshrsa.c
+++ b/sshrsa.c
@@ -237,9 +237,19 @@ char *rsa_ssh1_fingerprint(RSAKey *key)
     strbuf *out;
     int i;
 
+    /*
+     * The hash preimage for SSH-1 key fingerprinting consists of the
+     * modulus and exponent _without_ any preceding length field -
+     * just the minimum number of bytes to represent each integer,
+     * stored big-endian, concatenated with no marker at the division
+     * between them.
+     */
+
     MD5Init(&md5c);
-    put_mp_ssh1(&md5c, key->modulus);
-    put_mp_ssh1(&md5c, key->exponent);
+    for (size_t i = (mp_get_nbits(key->modulus) + 7) / 8; i-- > 0 ;)
+        put_byte(&md5c, mp_get_byte(key->modulus, i));
+    for (size_t i = (mp_get_nbits(key->exponent) + 7) / 8; i-- > 0 ;)
+        put_byte(&md5c, mp_get_byte(key->exponent, i));
     MD5Final(digest, &md5c);
 
     out = strbuf_new();
diff --git a/test/cryptsuite.py b/test/cryptsuite.py
index c792db4b..b3b23960 100755
--- a/test/cryptsuite.py
+++ b/test/cryptsuite.py
@@ -40,6 +40,12 @@ def ssh2_mpint(x):
     bytevals = [0xFF & (x >> (8*n)) for n in range(nbits(x)//8, -1, -1)]
     return struct.pack(">L" + "B" * len(bytevals), len(bytevals), *bytevals)
 
+def rsa_bare(e, n):
+    rsa = rsa_new()
+    get_rsa_ssh1_pub(ssh_uint32(nbits(n)) + ssh1_mpint(e) + ssh1_mpint(n),
+                     rsa, 'exponent_first')
+    return rsa
+
 def find_non_square_mod(p):
     # Find a non-square mod p, using the Jacobi symbol
     # calculation function from eccref.py.
@@ -737,6 +743,15 @@ class ecc(unittest.TestCase):
             self.assertEqual(int(x), int(rGi.x))
             self.assertEqual(int(y), int(rGi.y))
 
+class crypt(unittest.TestCase):
+    def testSSH1Fingerprint(self):
+        # Example key and reference fingerprint value generated by
+        # OpenSSH 6.7 ssh-keygen
+        rsa = rsa_bare(65537, 984185866443261798625575612408956568591522723900235822424492423996716524817102482330189709310179009158443944785704183009867662230534501187034891091310377917105259938712348098594526746211645472854839799025154390701673823298369051411)
+        fp = rsa_ssh1_fingerprint(rsa)
+        self.assertEqual(
+            fp, b"768 96:12:c8:bc:e6:03:75:86:e8:c7:b9:af:d8:0c:15:75")
+
 class standard_test_vectors(unittest.TestCase):
     def testAES(self):
         def vector(cipher, key, plaintext, ciphertext):