From 8611e2f035af5f218490687893cb7915b5929836 Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Mon, 14 Jan 2019 21:19:38 +0000
Subject: [PATCH] Add tests of the CRC compensation detector.

I remembered the existence of that module while I was changing the API
of the CRC functions. It's still quite possibly the only code in PuTTY
not written specifically _for_ PuTTY, so it definitely deserves a bit
of a test suite.

In order to expose it through the ptrlen-centric testcrypt system,
I've added some missing 'const' in the detector module itself, but
otherwise I've left the detector code as it was.
---
 Recipe             |  4 ++--
 ssh.h              |  5 ++--
 sshcrcda.c         | 14 ++++++-----
 test/cryptsuite.py | 59 ++++++++++++++++++++++++++++++++++++++++++++++
 testcrypt.c        | 13 ++++++++++
 testcrypt.h        |  1 +
 6 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/Recipe b/Recipe
index 7eff4e04..b2e64802 100644
--- a/Recipe
+++ b/Recipe
@@ -254,10 +254,10 @@ ARITH    = mpint ecc
 SSHCRYPTO = ARITH sshmd5 sshsha sshsh256 sshsh512
 	 + sshrsa sshdss sshecc
          + sshdes sshblowf sshaes sshccp ssharcf
-         + sshdh sshcrc
+         + sshdh sshcrc sshcrcda
 SSHCOMMON = sshcommon sshrand SSHCRYPTO
          + sshverstring
-         + sshcrcda sshpubk sshzlib
+         + sshpubk sshzlib
          + sshmac marshal nullplug
          + sshgssc pgssapi wildcard ssh1censor ssh2censor ssh2bpp
 	 + ssh2transport ssh2transhk ssh2connection portfwd x11fwd
diff --git a/ssh.h b/ssh.h
index 70e74c18..56d3a4d3 100644
--- a/ssh.h
+++ b/ssh.h
@@ -520,8 +520,9 @@ uint32_t crc32_update(uint32_t crc_input, ptrlen data);
 struct crcda_ctx;
 struct crcda_ctx *crcda_make_context(void);
 void crcda_free_context(struct crcda_ctx *ctx);
-bool detect_attack(struct crcda_ctx *ctx, unsigned char *buf, uint32_t len,
-                   unsigned char *IV);
+bool detect_attack(struct crcda_ctx *ctx,
+                   const unsigned char *buf, uint32_t len,
+                   const unsigned char *IV);
 
 /*
  * SSH2 RSA key exchange functions
diff --git a/sshcrcda.c b/sshcrcda.c
index 35d20ccc..bfebf73a 100644
--- a/sshcrcda.c
+++ b/sshcrcda.c
@@ -75,10 +75,11 @@ static void crc_update(uint32_t *a, const void *b)
 }
 
 /* detect if a block is used in a particular pattern */
-static bool check_crc(uint8_t *S, uint8_t *buf, uint32_t len, uint8_t *IV)
+static bool check_crc(const uint8_t *S, const uint8_t *buf,
+                      uint32_t len, const uint8_t *IV)
 {
     uint32_t crc;
-    uint8_t *c;
+    const uint8_t *c;
 
     crc = 0;
     if (IV && !CMP(S, IV)) {
@@ -98,13 +99,14 @@ static bool check_crc(uint8_t *S, uint8_t *buf, uint32_t len, uint8_t *IV)
 }
 
 /* Detect a crc32 compensation attack on a packet */
-bool detect_attack(
-    struct crcda_ctx *ctx, uint8_t *buf, uint32_t len, uint8_t *IV)
+bool detect_attack(struct crcda_ctx *ctx,
+                   const unsigned char *buf, uint32_t len,
+                   const unsigned char *IV)
 {
     register uint32_t i, j;
     uint32_t l;
-    register uint8_t *c;
-    uint8_t *d;
+    register const uint8_t *c;
+    const uint8_t *d;
 
     assert(!(len > (SSH_MAXBLOCKS * SSH_BLOCKSIZE) ||
              len % SSH_BLOCKSIZE != 0));
diff --git a/test/cryptsuite.py b/test/cryptsuite.py
index 1e448f67..92831946 100755
--- a/test/cryptsuite.py
+++ b/test/cryptsuite.py
@@ -966,6 +966,65 @@ class crypt(MyTestBase):
             # we're at it!
             self.assertEqual(shift8(i ^ prior), exp)
 
+    def testCRCDA(self):
+        def pattern(badblk, otherblks, pat):
+            # Arrange copies of the bad block in a pattern
+            # corresponding to the given bit string.
+            retstr = ""
+            while pat != 0:
+                retstr += (badblk if pat & 1 else next(otherblks))
+                pat >>= 1
+            return retstr
+
+        def testCases(pat):
+            badblock = b'muhahaha' # the block we'll maliciously repeat
+
+            # Various choices of the other blocks, including all the
+            # same, all different, and all different but only in the
+            # byte at one end.
+            for otherblocks in [
+                    itertools.repeat(b'GoodData'),
+                    (struct.pack('>Q', i) for i in itertools.count()),
+                    (struct.pack('<Q', i) for i in itertools.count())]:
+                yield pattern(badblock, otherblocks, pat)
+
+        def positiveTest(pat):
+            for data in testCases(pat):
+                self.assertTrue(crcda_detect(data, ""))
+                self.assertTrue(crcda_detect(data[8:], data[:8]))
+
+        def negativeTest(pat):
+            for data in testCases(pat):
+                self.assertFalse(crcda_detect(data, ""))
+                self.assertFalse(crcda_detect(data[8:], data[:8]))
+
+        # Tests of successful attack detection, derived by taking
+        # multiples of the CRC polynomial itself.
+        #
+        # (The CRC32 polynomial is usually written as 0xEDB88320.
+        # That's in bit-reversed form, but then, that's the form we
+        # need anyway for these patterns. But it's also missing the
+        # leading term - really, 0xEDB88320 is the value you get by
+        # reducing X^32 modulo the real poly, i.e. the value you put
+        # back in to the CRC to compensate for an X^32 that's just
+        # been shifted out. If you put that bit back on - at the
+        # bottom, because of the bit-reversal - you get the less
+        # familiar-looking 0x1db710641.)
+        positiveTest(0x1db710641) # the CRC polynomial P itself
+        positiveTest(0x26d930ac3) # (X+1) * P
+        positiveTest(0xbdbdf21cf) # (X^3+X^2+X+1) * P
+        positiveTest(0x3a66a39b653f6889d)
+        positiveTest(0x170db3167dd9f782b9765214c03e71a18f685b7f3)
+        positiveTest(0x1751997d000000000000000000000000000000001)
+        positiveTest(0x800000000000000000000000000000000f128a2d1)
+
+        # Tests of non-detection.
+        negativeTest(0x1db711a41)
+        negativeTest(0x3a66a39b453f6889d)
+        negativeTest(0x170db3167dd9f782b9765214c03e71b18f685b7f3)
+        negativeTest(0x1751997d000000000000000000000001000000001)
+        negativeTest(0x800000000000002000000000000000000f128a2d1)
+
 class standard_test_vectors(MyTestBase):
     def testAES(self):
         def vector(cipher, key, plaintext, ciphertext):
diff --git a/testcrypt.c b/testcrypt.c
index 277bbfa7..88093649 100644
--- a/testcrypt.c
+++ b/testcrypt.c
@@ -827,6 +827,19 @@ strbuf *des_decrypt_xdmauth_wrapper(ptrlen key, ptrlen data)
 }
 #define des_decrypt_xdmauth des_decrypt_xdmauth_wrapper
 
+bool crcda_detect(ptrlen packet, ptrlen iv)
+{
+    if (iv.len != 0 && iv.len != 8)
+        fatal_error("crcda_detect: iv must be empty or 8 bytes long");
+    if (packet.len % 8 != 0)
+        fatal_error("crcda_detect: packet must be a multiple of 8 bytes");
+    struct crcda_ctx *ctx = crcda_make_context();
+    bool toret = detect_attack(ctx, packet.ptr, packet.len,
+                               iv.len ? iv.ptr : NULL);
+    crcda_free_context(ctx);
+    return toret;
+}
+
 #define return_void(out, expression) (expression)
 
 #define VALTYPE_TYPEDEF(n,t,f)                  \
diff --git a/testcrypt.h b/testcrypt.h
index 61fc639f..b0c209bc 100644
--- a/testcrypt.h
+++ b/testcrypt.h
@@ -221,6 +221,7 @@ 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)
+FUNC2(boolean, crcda_detect, val_string_ptrlen, val_string_ptrlen)
 
 /*
  * These functions aren't part of PuTTY's own API, but are additions