* quad_cksum.c (des_quad_cksum): Add comments. Force 32-bit arithmetic just to be careful

git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@13148 dc483132-0cff-0310-8789-dd5450dbe970

2 files changed, 41 insertions, 5 deletions

diff --git a/src/lib/des425/ChangeLog b/src/lib/des425/ChangeLog
index 2cc9e3f..3274772 100644
--- a/src/lib/des425/ChangeLog
+++ b/src/lib/des425/ChangeLog
@@ -1,3 +1,8 @@
+2001-04-09  Ken Raeburn  <raeburn@mit.edu>
+
+	* quad_cksum.c (des_quad_cksum): Add comments.  Force 32-bit
+	arithmetic just to be careful.
+
 2001-04-07  Ken Raeburn  <raeburn@mit.edu>
 
 	* t_quad.c: New file.
diff --git a/src/lib/des425/quad_cksum.c b/src/lib/des425/quad_cksum.c
index 4ae542f..e9340bd 100644
--- a/src/lib/des425/quad_cksum.c
+++ b/src/lib/des425/quad_cksum.c
@@ -23,7 +23,10 @@
  * M.I.T. makes no representations about the suitability of
  * this software for any purpose.  It is provided "as is" without express
  * or implied warranty.
- * 
+ *
+ *
+ * This routine does not implement:
+ *
  *
  * Quadratic Congruential Manipulation Dectection Code
  *
@@ -35,8 +38,7 @@
  * This routine, part of the Athena DES library built for the Kerberos
  * authentication system, calculates a manipulation detection code for
  * a message.  It is a much faster alternative to the DES-checksum
- * method. No guarantees are offered for its security.	Refer to the
- * paper noted above for more information
+ * method. No guarantees are offered for its security.
  *
  * Implementation for 4.2bsd
  * by S.P. Miller	Project Athena/MIT
@@ -82,6 +84,22 @@
  *	cant get at the carry or high order results from multiply,
  *	but nontheless is 64 bit arithmetic.
  */
+/*
+ * This code purports to implement the above algorithm, but fails.
+ *
+ * First of all, there was an implicit mod 2**32 being done on the
+ * machines where this was developed because of their word sizes, and
+ * for compabitility this has to be done on machines with 64-bit
+ * words, so we make it explicit.
+ *
+ * Second, in the squaring operation, I really doubt the carry-over
+ * from the low 31-bit half of the accumulator is being done right,
+ * and using a modulus of 0x7fffffff on the low half of the
+ * accumulator seems completely wrong.  And I challenge anyone to
+ * explain where the number 83653421 comes from.
+ *
+ * --Ken Raeburn  2001-04-06
+ */
 
 
 /* System include files */
@@ -144,6 +162,10 @@ des_quad_cksum(in,out,length,out_count,c_seed)
 	len = length;
 	p = in;
 	while (len) {
+	    /*
+	     * X = Z + Input ... sort of.  Carry out from low half
+	     * isn't done, so we're using all 32 bits of x now.
+	     */
 	    if (len > 1) {
 		x = (z + vaxtohs(p));
 		p += 2;
@@ -154,8 +176,17 @@ des_quad_cksum(in,out,length,out_count,c_seed)
 		len = 0;
 	    }
 	    x2 = z2;
-	    z  = ((x * x) + (x2 * x2)) % 0x7fffffff;
-	    z2 = (x * (x2+83653421))   % 0x7fffffff; /* modulo */
+	    /*
+	     * I think this is supposed to be a squaring operation.
+	     * What it really is, I haven't figured out yet.
+	     *
+	     * Explicit mod 2**32 is for backwards compatibility.  Why
+	     * mod 0x7fffffff and not 0x80000000 on the low half of
+	     * the (supposed) accumulator?  And where does the number
+	     * 83653421 come from??
+	     */
+	    z  = (((x * x) + (x2 * x2)) & 0xffffffff) % 0x7fffffff;
+	    z2 = ((x * (x2+83653421)) & 0xffffffff) % 0x7fffffff; /* modulo */
 #ifdef DEBUG
 	    if (des_debug & 8)
 		printf("%d %d\n",z,z2);