Expose CRC32 to testcrypt, and add tests for it.

Finding even semi-official test vectors for this CRC implementation
was hard, because it turns out not to _quite_ match any of the well
known ones catalogued on the web. Its _polynomial_ is well known, but
the combination of details that go alongside it (starting state,
post-hashing transformation) are not quite the same as any other hash
I know of.

After trawling catalogue websites for a while I finally worked out
that SSH-1's CRC and RFC 1662's CRC are basically the same except for
different choices of starting value and final adjustment. And RFC
1662's CRC is common enough that there _are_ test vectors.

So I've renamed the previous crc32_compute function to crc32_ssh1,
reflecting that it seems to be its own thing unlike any other CRC;
implemented the RFC 1662 CRC as well, as an alternative tiny wrapper
on the inner crc32_update function; and exposed all three functions to
testcrypt. That lets me run standard test vectors _and_ directed tests
of the internal update routine, plus one check that crc32_ssh1 itself
does what I expect.

While I'm here, I've also modernised the code to use uint32_t in place
of unsigned long, and ptrlen instead of separate pointer,length
arguments. And I've removed the general primer on CRC theory from the
header comment, in favour of the more specifically useful information
about _which_ CRC this is and how it matches up to anything else out
there.

(I've bowed to inevitability and put the directed CRC tests in the
'crypt' class in cryptsuite.py. Of course this is a misnomer, since
CRC isn't cryptography, but it falls into the same category in terms
of the role it plays in SSH-1, and I didn't feel like making a new
pointedly-named 'notreallycrypt' container class just for this :-)

Recipe

@@ -254,9 +254,9 @@ ARITH    = mpint ecc
 SSHCRYPTO = ARITH sshmd5 sshsha sshsh256 sshsh512
 	 + sshrsa sshdss sshecc
          + sshdes sshblowf sshaes sshccp ssharcf
-         + sshdh
+         + sshdh sshcrc
 SSHCOMMON = sshcommon sshrand SSHCRYPTO
-         + sshverstring sshcrc
+         + sshverstring
          + sshcrcda sshpubk sshzlib
          + sshmac marshal nullplug
          + sshgssc pgssapi wildcard ssh1censor ssh2censor ssh2bpp

ssh.h

@@ -512,8 +512,9 @@ int rsa_ssh1_public_blob_len(ptrlen data);
 void freersapriv(RSAKey *key);
 void freersakey(RSAKey *key);
 
-unsigned long crc32_compute(const void *s, size_t len);
-unsigned long crc32_update(unsigned long crc_input, const void *s, size_t len);
+uint32_t crc32_rfc1662(ptrlen data);
+uint32_t crc32_ssh1(ptrlen data);
+uint32_t crc32_update(uint32_t crc_input, ptrlen data);
 
 /* SSH CRC compensation attack detector */
 struct crcda_ctx;

ssh1bpp.c

@@ -13,7 +13,7 @@ struct ssh1_bpp_state {
     int crState;
     long len, pad, biglen, length, maxlen;
     unsigned char *data;
-    unsigned long realcrc, gotcrc;
+    uint32_t realcrc, gotcrc;
     int chunk;
     PktIn *pktin;
 
@@ -164,7 +164,7 @@ static void ssh1_bpp_handle_input(BinaryPacketProtocol *bpp)
         if (s->cipher)
             ssh1_cipher_decrypt(s->cipher, s->data, s->biglen);
 
-        s->realcrc = crc32_compute(s->data, s->biglen - 4);
+        s->realcrc = crc32_ssh1(make_ptrlen(s->data, s->biglen - 4));
         s->gotcrc = GET_32BIT(s->data + s->biglen - 4);
         if (s->gotcrc != s->realcrc) {
             ssh_sw_abort(s->bpp.ssh, "Incorrect CRC received on packet");
@@ -280,7 +280,7 @@ static PktOut *ssh1_bpp_new_pktout(int pkt_type)
 static void ssh1_bpp_format_packet(struct ssh1_bpp_state *s, PktOut *pkt)
 {
     int pad, biglen, i, pktoffs;
-    unsigned long crc;
+    uint32_t crc;
     int len;
 
     if (s->bpp.logctx) {
@@ -315,8 +315,8 @@ static void ssh1_bpp_format_packet(struct ssh1_bpp_state *s, PktOut *pkt)
 
     for (i = pktoffs; i < 4+8; i++)
         pkt->data[i] = random_byte();
-    crc = crc32_compute(pkt->data + pktoffs + 4,
-                        biglen - 4); /* all ex len */
+    crc = crc32_ssh1(
+        make_ptrlen(pkt->data + pktoffs + 4, biglen - 4)); /* all ex len */
     PUT_32BIT(pkt->data + pktoffs + 4 + biglen - 4, crc);
     PUT_32BIT(pkt->data + pktoffs, len);
 

sshcrc.c

@@ -1,72 +1,38 @@
 /*
- * CRC32 implementation.
+ * CRC32 implementation, as used in SSH-1.
  *
- * The basic concept of a CRC is that you treat your bit-string
- * abcdefg... as a ludicrously long polynomial M=a+bx+cx^2+dx^3+...
- * over Z[2]. You then take a modulus polynomial P, and compute the
- * remainder of M on division by P. Thus, an erroneous message N
- * will only have the same CRC if the difference E = M-N is an
- * exact multiple of P. (Note that as we are working over Z[2], M-N
- * = N-M = M+N; but that's not very important.)
+ * This particular form of the CRC uses the polynomial
+ * P(x) = x^32+x^26+x^23+x^22+x^16+x^12+x^11+x^10+x^8+x^7+x^5+x^4+x^2+x^1+1
+ * and represents polynomials in bit-reversed form, so that the x^0
+ * coefficient (constant term) appears in the bit with place value
+ * 2^31, and the x^31 coefficient in the bit with place value 2^0. In
+ * this representation, (x^32 mod P) = 0xEDB88320, so multiplying the
+ * current state by x is done by shifting right by one bit, and XORing
+ * that constant into the result if the bit shifted out was 1.
  *
- * What makes the CRC good is choosing P to have good properties:
+ * There's a bewildering array of subtly different variants of CRC out
+ * there, using different polynomials, both bit orders, and varying
+ * the start and end conditions. There are catalogue websites such as
+ * http://reveng.sourceforge.net/crc-catalogue/ , which generally seem
+ * to have the convention of indexing CRCs by their 'check value',
+ * defined as whatever you get if you hash the 9-byte test string
+ * "123456789".
  *
- *  - If its first and last terms are both nonzero then it cannot
- *    be a factor of any single term x^i. Therefore if M and N
- *    differ by exactly one bit their CRCs will guaranteeably
- *    be distinct.
+ * The crc32_rfc1662() function below, which starts off the CRC state
+ * at 0xFFFFFFFF and complements it after feeding all the data, gives
+ * the check value 0xCBF43926, and matches the hash function that the
+ * above catalogue refers to as "CRC-32/ISO-HDLC"; among other things,
+ * it's also the "FCS-32" checksum described in RFC 1662 section C.3
+ * (hence the name I've given it here).
  *
- *  - If it has a prime (irreducible) factor with three terms then
- *    it cannot divide a polynomial of the form x^i(1+x^j).
- *    Therefore if M and N differ by exactly _two_ bits they will
- *    have different CRCs.
- *
- *  - If it has a factor (x+1) then it cannot divide a polynomial
- *    with an odd number of terms. Therefore if M and N differ by
- *    _any odd_ number of bits they will have different CRCs.
- *
- *  - If the error term E is of the form x^i*B(x) where B(x) has
- *    order less than P (i.e. a short _burst_ of errors) then P
- *    cannot divide E (since no polynomial can divide a shorter
- *    one), so any such error burst will be spotted.
- *
- * The CRC32 standard polynomial is
- *   x^32+x^26+x^23+x^22+x^16+x^12+x^11+x^10+x^8+x^7+x^5+x^4+x^2+x^1+x^0
- *
- * In fact, we don't compute M mod P; we compute M*x^32 mod P.
- *
- * The concrete implementation of the CRC is this: we maintain at
- * all times a 32-bit word which is the current remainder of the
- * polynomial mod P. Whenever we receive an extra bit, we multiply
- * the existing remainder by x, add (XOR) the x^32 term thus
- * generated to the new x^32 term caused by the incoming bit, and
- * remove the resulting combined x^32 term if present by replacing
- * it with (P-x^32).
- *
- * Bit 0 of the word is the x^31 term and bit 31 is the x^0 term.
- * Thus, multiplying by x means shifting right. So the actual
- * algorithm goes like this:
- *
- *   x32term = (crcword & 1) ^ newbit;
- *   crcword = (crcword >> 1) ^ (x32term * 0xEDB88320);
- *
- * In practice, we pre-compute what will happen to crcword on any
- * given sequence of eight incoming bits, and store that in a table
- * which we then use at run-time to do the job:
- * 
- *   outgoingplusnew = (crcword & 0xFF) ^ newbyte;
- *   crcword = (crcword >> 8) ^ table[outgoingplusnew];
- *
- * where table[outgoingplusnew] is computed by setting crcword=0
- * and then iterating the first code fragment eight times (taking
- * the incoming byte low bit first).
- *
- * Note that all shifts are rightward and thus no assumption is
- * made about exact word length! (Although word length must be at
- * _least_ 32 bits, but ANSI C guarantees this for `unsigned long'
- * anyway.)
+ * The crc32_ssh1() function implements the variant form used by
+ * SSH-1, which uses the same update function, but starts the state at
+ * zero and doesn't complement it at the end of the computation. The
+ * check value for that version is 0x2DFD2D88, which that CRC
+ * catalogue doesn't list at all.
  */
 
+#include <stdint.h>
 #include <stdlib.h>
 
 #include "ssh.h"
@@ -100,15 +66,15 @@
  * This variant of the code generates the table at run-time from an
  * init function.
  */
-static unsigned long crc32_table[256];
+static uint32_t crc32_table[256];
 
 void crc32_init(void)
 {
-    unsigned long crcword;
+    uint32_t crcword;
     int i;
 
     for (i = 0; i < 256; i++) {
-	unsigned long newbyte, x32term;
+	uint32_t newbyte, x32term;
 	int j;
 	crcword = 0;
 	newbyte = i;
@@ -126,7 +92,7 @@ void crc32_init(void)
 /*
  * This variant of the code has the data already prepared.
  */
-static const unsigned long crc32_table[256] = {
+static const uint32_t crc32_table[256] = {
     0x00000000L, 0x77073096L, 0xEE0E612CL, 0x990951BAL,
     0x076DC419L, 0x706AF48FL, 0xE963A535L, 0x9E6495A3L,
     0x0EDB8832L, 0x79DCB8A4L, 0xE0D5E91EL, 0x97D2D988L,
@@ -212,18 +178,31 @@ int main(void)
 }
 #endif
 
-unsigned long crc32_update(unsigned long crcword, const void *buf, size_t len)
+uint32_t crc32_update(uint32_t crcword, ptrlen data)
 {
-    const unsigned char *p = (const unsigned char *) buf;
-    while (len--) {
-	unsigned long newbyte = *p++;
+    const uint8_t *p = (const uint8_t *)data.ptr;
+    for (size_t len = data.len; len-- > 0 ;) {
+	uint32_t newbyte = *p++;
 	newbyte ^= crcword & 0xFFL;
 	crcword = (crcword >> 8) ^ crc32_table[newbyte];
     }
     return crcword;
 }
 
-unsigned long crc32_compute(const void *buf, size_t len)
+/*
+ * The SSH-1 variant of CRC-32.
+ */
+uint32_t crc32_ssh1(ptrlen data)
 {
-    return crc32_update(0L, buf, len);
+    return crc32_update(0, data);
+}
+
+/*
+ * The official version of CRC-32. Nothing in PuTTY proper uses this,
+ * but it's useful to expose it to testcrypt so that we can implement
+ * standard test vectors.
+ */
+uint32_t crc32_rfc1662(ptrlen data)
+{
+    return crc32_update(0xFFFFFFFF, data) ^ 0xFFFFFFFF;
 }

sshcrcda.c

@@ -69,9 +69,9 @@ void crcda_free_context(struct crcda_ctx *ctx)
     }
 }
 
-static void crc_update(uint32_t *a, void *b)
+static void crc_update(uint32_t *a, const void *b)
 {
-    *a = crc32_update(*a, b, 4);
+    *a = crc32_update(*a, make_ptrlen(b, 4));
 }
 
 /* detect if a block is used in a particular pattern */

test/cryptsuite.py

@@ -3,6 +3,7 @@
 import unittest
 import struct
 import itertools
+import functools
 import contextlib
 import hashlib
 import binascii
@@ -935,6 +936,36 @@ class crypt(MyTestBase):
             for d in decryptions:
                 self.assertEqualBin(d, decryptions[0])
 
+    def testCRC32(self):
+        # Check the effect of every possible single-byte input to
+        # crc32_update. In the traditional implementation with a
+        # 256-word lookup table, this exercises every table entry; in
+        # _any_ implementation which iterates over the input one byte
+        # at a time, it should be a similarly exhaustive test. (But if
+        # a more optimised implementation absorbed _more_ than 8 bits
+        # at a time, then perhaps this test wouldn't be enough...)
+
+        # It would be nice if there was a functools.iterate() which
+        # would apply a function n times. Failing that, making shift1
+        # accept and ignore a second argument allows me to iterate it
+        # 8 times using functools.reduce.
+        shift1 = lambda x, dummy=None: (x >> 1) ^ (0xEDB88320 * (x & 1))
+        shift8 = lambda x: functools.reduce(shift1, [None]*8, x)
+
+        # A small selection of choices for the other input to
+        # crc32_update, just to check linearity.
+        test_prior_values = [0, 0xFFFFFFFF, 0x45CC1F6A, 0xA0C4ADCF, 0xD482CDF1]
+
+        for prior in test_prior_values:
+            prior_shifted = shift8(prior)
+        for i in range(256):
+            exp = shift8(i) ^ prior_shifted
+            self.assertEqual(crc32_update(prior, struct.pack("B", i)), exp)
+
+            # Check linearity of the _reference_ implementation, while
+            # we're at it!
+            self.assertEqual(shift8(i ^ prior), exp)
+
 class standard_test_vectors(MyTestBase):
     def testAES(self):
         def vector(cipher, key, plaintext, ciphertext):
@@ -1367,6 +1398,50 @@ class standard_test_vectors(MyTestBase):
                     signature = unhex(words[3])[:64]
                     vector(privkey, pubkey, message, signature)
 
+    def testCRC32(self):
+        self.assertEqual(crc32_rfc1662("123456789"), 0xCBF43926)
+        self.assertEqual(crc32_ssh1("123456789"), 0x2DFD2D88)
+
+        # Source:
+        # http://reveng.sourceforge.net/crc-catalogue/17plus.htm#crc.cat.crc-32-iso-hdlc
+        # which collected these from various sources.
+        reveng_tests = [
+            '000000001CDF4421',
+            'F20183779DAB24',
+            '0FAA005587B2C9B6',
+            '00FF55111262A032',
+            '332255AABBCCDDEEFF3D86AEB0',
+            '926B559BA2DE9C',
+            'FFFFFFFFFFFFFFFF',
+            'C008300028CFE9521D3B08EA449900E808EA449900E8300102007E649416',
+            '6173640ACEDE2D15',
+        ]
+        for vec in map(unhex, reveng_tests):
+            # Each of these test vectors can be read two ways. One
+            # interpretation is that the last four bytes are the
+            # little-endian encoding of the CRC of the rest. (Because
+            # that's how the CRC is attached to a string at the
+            # sending end.)
+            #
+            # The other interpretation is that if you CRC the whole
+            # string, _including_ the final four bytes, you expect to
+            # get the same value for any correct string (because the
+            # little-endian encoding matches the way the rest of the
+            # string was interpreted as a polynomial in the first
+            # place). That's how a receiver is intended to check
+            # things.
+            #
+            # The expected output value is listed in RFC 1662, and in
+            # the reveng.sourceforge.net catalogue, as 0xDEBB20E3. But
+            # that's because their checking procedure omits the final
+            # complement step that the construction procedure
+            # includes. Our crc32_rfc1662 function does do the final
+            # complement, so we expect the bitwise NOT of that value,
+            # namely 0x2144DF1C.
+            expected = struct.unpack("<L", vec[-4:])[0]
+            self.assertEqual(crc32_rfc1662(vec[:-4]), expected)
+            self.assertEqual(crc32_rfc1662(vec), 0x2144DF1C)
+
 if __name__ == "__main__":
     try:
         unittest.main()

testcrypt.h

@@ -218,6 +218,9 @@ FUNC2(val_wpoint, ecdsa_public, val_mpint, keyalg)
 FUNC2(val_epoint, eddsa_public, val_mpint, keyalg)
 FUNC2(val_string, des_encrypt_xdmauth, val_string_ptrlen, val_string_ptrlen)
 FUNC2(val_string, des_decrypt_xdmauth, val_string_ptrlen, val_string_ptrlen)
+FUNC1(uint, crc32_rfc1662, val_string_ptrlen)
+FUNC1(uint, crc32_ssh1, val_string_ptrlen)
+FUNC2(uint, crc32_update, uint, val_string_ptrlen)
 
 /*
  * These functions aren't part of PuTTY's own API, but are additions