be more paranoid about what fields we are checksumming

2 files changed, 180 insertions, 162 deletions

diff --git a/lib/ZCkZAut.c b/lib/ZCkZAut.c
index 46e6472..0e5df8b 100644
--- a/lib/ZCkZAut.c
+++ b/lib/ZCkZAut.c
@@ -34,6 +34,20 @@ static const char rcsid_ZCheckAuthentication_c[] =
 Code_t ZCheckZcodeAuthentication(ZNotice_t *notice,
 				 struct sockaddr_in *from)
 {	
+#ifdef HAVE_KRB5
+    krb5_error_code result;
+    krb5_creds *creds;
+    krb5_keyblock *keyblock;
+    krb5_enctype enctype;
+    krb5_cksumtype cksumtype;
+    krb5_data cksumbuf;
+    int valid;
+    char *cksum0_base, *cksum1_base, *cksum2_base;
+    char *x;
+    unsigned char *asn1_data, *key_data;
+    int asn1_len, key_len, cksum0_len = 0, cksum1_len = 0, cksum2_len = 0;
+#endif
+    
     /* If the value is already known, return it. */
     if (notice->z_checked_auth != ZAUTH_UNSET)
         return (notice->z_checked_auth);
@@ -44,145 +58,139 @@ Code_t ZCheckZcodeAuthentication(ZNotice_t *notice,
     if (!notice->z_ascii_checksum)
         return (ZAUTH_NO);
 
-#ifdef HAVE_KRB5
-    {
-        krb5_error_code result;
-        krb5_creds *creds;
-        krb5_keyblock *keyblock;
-        krb5_enctype enctype;
-        krb5_cksumtype cksumtype;
-        krb5_data cksumbuf;
-	int valid;
-        char *cksum0_base, *cksum1_base, *cksum2_base;
-        char *x;
-        unsigned char *asn1_data, *key_data;
-        int asn1_len, key_len, cksum0_len, cksum1_len, cksum2_len;
-
-	result = ZGetCreds(&creds);
-
-        if (result)
-            return (ZAUTH_NO);
-        /* HOLDING: creds */
-
-        /* Figure out what checksum type to use */
-	keyblock = Z_credskey(creds);
-	key_data = Z_keydata(keyblock);
-	key_len = Z_keylen(keyblock);
-	result = Z_ExtractEncCksum(keyblock, &enctype, &cksumtype);
-        if (result) {
-	    krb5_free_creds(Z_krb5_ctx, creds);
-	    return (ZAUTH_FAILED);
-        }
-        /* HOLDING: creds */
 
-        /* Assemble the things to be checksummed */
-        /* first part is from start of packet through z_default_format:
-         * - z_version
-         * - z_num_other_fields
-         * - z_kind
-         * - z_uid
-         * - z_port
-         * - z_auth
-         * - z_authent_len
-         * - z_ascii_authent
-         * - z_class
-         * - z_class_inst
-         * - z_opcode
-         * - z_sender
-         * - z_recipient
-         * - z_default_format
-         */
-        cksum0_base = notice->z_packet;
-        x           = notice->z_default_format;
-        cksum0_len  = x + strlen(x) + 1 - cksum0_base;
-        /* second part is from z_multinotice through other fields:
-         * - z_multinotice
-         * - z_multiuid
-         * - z_other_fields[]
-         */
-        cksum1_base = notice->z_multinotice;
-        if (notice->z_num_other_fields)
+#ifdef HAVE_KRB5
+    result = ZGetCreds(&creds);
+    
+    if (result)
+	return (ZAUTH_NO);
+    /* HOLDING: creds */
+    
+    /* Figure out what checksum type to use */
+    keyblock = Z_credskey(creds);
+    key_data = Z_keydata(keyblock);
+    key_len = Z_keylen(keyblock);
+    result = Z_ExtractEncCksum(keyblock, &enctype, &cksumtype);
+    if (result) {
+	krb5_free_creds(Z_krb5_ctx, creds);
+	return (ZAUTH_FAILED);
+    }
+    /* HOLDING: creds */
+    
+    /* Assemble the things to be checksummed */
+    /* first part is from start of packet through z_default_format:
+     * - z_version
+     * - z_num_other_fields
+     * - z_kind
+     * - z_uid
+     * - z_port
+     * - z_auth
+     * - z_authent_len
+     * - z_ascii_authent
+     * - z_class
+     * - z_class_inst
+     * - z_opcode
+     * - z_sender
+     * - z_recipient
+     * - z_default_format
+     */
+    cksum0_base = notice->z_packet;
+    x           = notice->z_default_format;
+    cksum0_len  = x + strlen(x) + 1 - cksum0_base;
+    /* second part is from z_multinotice through other fields:
+     * - z_multinotice
+     * - z_multiuid
+     * - z_sender_(sock)addr
+     * - z_charset
+     * - z_other_fields[]
+     */
+    if (notice->z_num_hdr_fields > 15 ) {
+	cksum1_base = notice->z_multinotice;
+	if (notice->z_num_other_fields)
 	    x = notice->z_other_fields[notice->z_num_other_fields - 1];
-        else  {
-	    /* see also server/kstuff.c:ZCheckRealmAuthentication */
+	else  {
+	    /* see also server/kstuff.c:ZCheck{Realm,}Authentication */
 	    /* XXXXXXXXXXXXXXXXXXXXXXX */
-	    x = cksum1_base + strlen(cksum1_base) + 1; /* multinotice */
+	    if (notice->z_num_hdr_fields > 16)
+		x = cksum1_base + strlen(cksum1_base) + 1; /* multinotice */
 	    if (notice->z_num_hdr_fields > 17)
 		x = x + strlen(x) + 1; /* multiuid */
 	    if (notice->z_num_hdr_fields > 18)
 		x = x + strlen(x) + 1; /* sender */
 	}
 	cksum1_len  = x + strlen(x) + 1 - cksum1_base; /* charset / extra field */
-
-        /* last part is the message body */
-        cksum2_base = notice->z_message;
-        cksum2_len  = notice->z_message_len;
-
-	/* The following code checks for old-style checksums, which will go
-	   away once Kerberos 4 does. */
-        if ((!notice->z_ascii_checksum || *notice->z_ascii_checksum != 'Z') &&
-            key_len == 8 &&
-            (enctype == ENCTYPE_DES_CBC_CRC ||
-             enctype == ENCTYPE_DES_CBC_MD4 ||
-             enctype == ENCTYPE_DES_CBC_MD5)) {
-	  /* try old-format checksum (covers cksum0 only) */
-
-            ZChecksum_t our_checksum;
-
-            our_checksum = z_quad_cksum((unsigned char *)cksum0_base, NULL, cksum0_len, 0,
-					key_data);
-            if (our_checksum == notice->z_checksum) {
-                krb5_free_creds(Z_krb5_ctx, creds);
-                return ZAUTH_YES;
-            }
-        }
-        /* HOLDING: creds */
-
-        cksumbuf.length = cksum0_len + cksum1_len + cksum2_len;
-        cksumbuf.data = malloc(cksumbuf.length);
-        if (!cksumbuf.data) {
+    }
+    
+    /* last part is the message body */
+    cksum2_base = notice->z_message;
+    cksum2_len  = notice->z_message_len;
+    
+    /* The following code checks for old-style checksums, which will go
+       away once Kerberos 4 does. */
+    if ((!notice->z_ascii_checksum || *notice->z_ascii_checksum != 'Z') &&
+	key_len == 8 &&
+	(enctype == ENCTYPE_DES_CBC_CRC ||
+	 enctype == ENCTYPE_DES_CBC_MD4 ||
+	 enctype == ENCTYPE_DES_CBC_MD5)) {
+	/* try old-format checksum (covers cksum0 only) */
+	
+	ZChecksum_t our_checksum;
+	
+	our_checksum = z_quad_cksum((unsigned char *)cksum0_base, NULL, cksum0_len, 0,
+				    key_data);
+	if (our_checksum == notice->z_checksum) {
 	    krb5_free_creds(Z_krb5_ctx, creds);
-	    return ZAUTH_NO;
-        }
-        /* HOLDING: creds, cksumbuf.data */
-
-        memcpy(cksumbuf.data, cksum0_base, cksum0_len);
-        memcpy(cksumbuf.data + cksum0_len, cksum1_base, cksum1_len);
-        memcpy(cksumbuf.data + cksum0_len + cksum1_len,
-               cksum2_base, cksum2_len);
-
-        /* decode zcoded checksum */
-        /* The encoded form is always longer than the original */
-        asn1_len = strlen(notice->z_ascii_checksum) + 1;
-        asn1_data = malloc(asn1_len);
-        if (!asn1_data) {
-            krb5_free_creds(Z_krb5_ctx, creds);
-            free(cksumbuf.data);
-            return ZAUTH_FAILED;
-        }
-        /* HOLDING: creds, asn1_data, cksumbuf.data */
-        result = ZReadZcode((unsigned char *)notice->z_ascii_checksum,
-                            asn1_data, asn1_len, &asn1_len);
-        if (result != ZERR_NONE) {
-            krb5_free_creds(Z_krb5_ctx, creds);
-            free(asn1_data);
-            free(cksumbuf.data);
-            return ZAUTH_FAILED;
-        }
-        /* HOLDING: creds, asn1_data, cksumbuf.data */
-
-	valid = Z_krb5_verify_cksum(keyblock, &cksumbuf, cksumtype,
-				    asn1_data, asn1_len);
-
-        free(asn1_data);
-        krb5_free_creds(Z_krb5_ctx, creds);
-        free(cksumbuf.data);
-
-	if (valid)
-	  return ZAUTH_YES;
-	else
-	  return ZAUTH_FAILED;
+	    return ZAUTH_YES;
+	}
+    }
+    /* HOLDING: creds */
+    
+    cksumbuf.length = cksum0_len + cksum1_len + cksum2_len;
+    cksumbuf.data = malloc(cksumbuf.length);
+    if (!cksumbuf.data) {
+	krb5_free_creds(Z_krb5_ctx, creds);
+	return ZAUTH_NO;
     }
+    /* HOLDING: creds, cksumbuf.data */
+    
+    memcpy(cksumbuf.data, cksum0_base, cksum0_len);
+    if (cksum1_len)
+	memcpy(cksumbuf.data + cksum0_len, cksum1_base, cksum1_len);
+    memcpy(cksumbuf.data + cksum0_len + cksum1_len,
+	   cksum2_base, cksum2_len);
+    
+    /* decode zcoded checksum */
+    /* The encoded form is always longer than the original */
+    asn1_len = strlen(notice->z_ascii_checksum) + 1;
+    asn1_data = malloc(asn1_len);
+    if (!asn1_data) {
+	krb5_free_creds(Z_krb5_ctx, creds);
+	free(cksumbuf.data);
+	return ZAUTH_FAILED;
+    }
+    /* HOLDING: creds, asn1_data, cksumbuf.data */
+    result = ZReadZcode((unsigned char *)notice->z_ascii_checksum,
+			asn1_data, asn1_len, &asn1_len);
+    if (result != ZERR_NONE) {
+	krb5_free_creds(Z_krb5_ctx, creds);
+	free(asn1_data);
+	free(cksumbuf.data);
+	return ZAUTH_FAILED;
+    }
+    /* HOLDING: creds, asn1_data, cksumbuf.data */
+    
+    valid = Z_krb5_verify_cksum(keyblock, &cksumbuf, cksumtype,
+				asn1_data, asn1_len);
+    
+    free(asn1_data);
+    krb5_free_creds(Z_krb5_ctx, creds);
+    free(cksumbuf.data);
+    
+    if (valid)
+	return ZAUTH_YES;
+    else
+	return ZAUTH_FAILED;
 #endif /* HAVE_KRB5 */
+
     return (notice->z_auth ? ZAUTH_YES : ZAUTH_NO);
 }
diff --git a/server/kstuff.c b/server/kstuff.c
index 8f283b5..7311727 100644
--- a/server/kstuff.c
+++ b/server/kstuff.c
@@ -265,7 +265,7 @@ ZCheckRealmAuthentication(ZNotice_t *notice,
     char *x; 
     unsigned char *asn1_data;
     unsigned char *key_data; 
-    int asn1_len, key_len, cksum0_len, cksum1_len, cksum2_len; 
+    int asn1_len, key_len, cksum0_len = 0, cksum1_len = 0, cksum2_len = 0; 
 #ifdef KRB5_AUTH_CON_GETAUTHENTICATOR_TAKES_DOUBLE_POINTER
     krb5_authenticator *authenticator;
 #define KRB5AUTHENT authenticator
@@ -438,21 +438,25 @@ ZCheckRealmAuthentication(ZNotice_t *notice,
      * - z_sender_(sock)addr
      * - z_charset
      * - z_other_fields[] 
-     */ 
-    cksum1_base = notice->z_multinotice; 
-    if (notice->z_num_other_fields) 
-        x = notice->z_other_fields[notice->z_num_other_fields - 1]; 
-    else {
-	/* see also ZCheckAuthentication and lib/ZCkZaut.c:ZCheckZcodeAuthentication */
-	/* XXXXXXXXXXXXXXXXXXXXXXX */
-        x = cksum1_base + strlen(cksum1_base) + 1; /* multinotice */
-	if (notice->z_num_hdr_fields > 17)
-	    x = x + strlen(x) + 1; /* multiuid */
-	if (notice->z_num_hdr_fields > 18)
-	    x = x + strlen(x) + 1; /* sender */
-    }
-    cksum1_len  = x + strlen(x) + 1 - cksum1_base; /* charset / extra field */
- 
+     */
+    if (notice->z_num_hdr_fields > 15 ) {
+	cksum1_base = notice->z_multinotice; 
+	if (notice->z_num_other_fields) 
+	    x = notice->z_other_fields[notice->z_num_other_fields - 1]; 
+	else {
+	    /* see also ZCheckAuthentication and
+	       lib/ZCkZaut.c:ZCheckZcodeAuthentication */
+	    /* XXXXXXXXXXXXXXXXXXXXXXX */
+	    if (notice->z_num_hdr_fields > 16) 
+		x = cksum1_base + strlen(cksum1_base) + 1; /* multinotice */
+	    if (notice->z_num_hdr_fields > 17)
+		x = x + strlen(x) + 1; /* multiuid */
+	    if (notice->z_num_hdr_fields > 18)
+		x = x + strlen(x) + 1; /* sender */
+	}
+	cksum1_len  = x + strlen(x) + 1 - cksum1_base; /* charset / extra field */
+    }
+    
     /* last part is the message body */ 
     cksum2_base = notice->z_message; 
     cksum2_len  = notice->z_message_len; 
@@ -490,8 +494,9 @@ ZCheckRealmAuthentication(ZNotice_t *notice,
     } 
     /* HOLDING: authctx, authenticator, cksumbuf.data */ 
  
-    memcpy(cksumbuf.data, cksum0_base, cksum0_len); 
-    memcpy(cksumbuf.data + cksum0_len, cksum1_base, cksum1_len); 
+    memcpy(cksumbuf.data, cksum0_base, cksum0_len);
+    if (cksum1_len)
+	memcpy(cksumbuf.data + cksum0_len, cksum1_base, cksum1_len); 
     memcpy(cksumbuf.data + cksum0_len + cksum1_len, 
            cksum2_base, cksum2_len); 
  
@@ -558,7 +563,7 @@ ZCheckAuthentication(ZNotice_t *notice,
     char *cksum0_base, *cksum1_base, *cksum2_base; 
     char *x; 
     unsigned char *asn1_data, *key_data; 
-    int asn1_len, key_len, cksum0_len, cksum1_len, cksum2_len; 
+    int asn1_len, key_len, cksum0_len = 0, cksum1_len = 0, cksum2_len = 0; 
 #ifdef KRB5_AUTH_CON_GETAUTHENTICATOR_TAKES_DOUBLE_POINTER
     krb5_authenticator *authenticator;
 #define KRB5AUTHENT authenticator
@@ -732,19 +737,23 @@ ZCheckAuthentication(ZNotice_t *notice,
      * - z_multiuid 
      * - z_other_fields[] 
      */ 
-    cksum1_base = notice->z_multinotice; 
-    if (notice->z_num_other_fields) 
-        x = notice->z_other_fields[notice->z_num_other_fields - 1]; 
-    else {
-	/* see also ZCheckRealmAuthentication and lib/ZCkZaut.c:ZCheckZcodeAuthentication  */
-	/* XXXXXXXXXXXXXXXXXXXXXXX */
-        x = cksum1_base + strlen(cksum1_base) + 1; /* multinotice */
-	if (notice->z_num_hdr_fields > 17)
-	    x = x + strlen(x) + 1; /* multiuid */
-	if (notice->z_num_hdr_fields > 18)
-	    x = x + strlen(x) + 1; /* sender */
-    }
-    cksum1_len  = x + strlen(x) + 1 - cksum1_base; /* charset / extra field */
+    if (notice->z_num_hdr_fields > 15 ) {
+	cksum1_base = notice->z_multinotice; 
+	if (notice->z_num_other_fields) 
+	    x = notice->z_other_fields[notice->z_num_other_fields - 1]; 
+	else {
+	    /* see also ZCheckRealmAuthentication
+	       and lib/ZCkZaut.c:ZCheckZcodeAuthentication  */
+	    /* XXXXXXXXXXXXXXXXXXXXXXX */
+	    if (notice->z_num_hdr_fields > 16)
+		x = cksum1_base + strlen(cksum1_base) + 1; /* multinotice */
+	    if (notice->z_num_hdr_fields > 17)
+		x = x + strlen(x) + 1; /* multiuid */
+	    if (notice->z_num_hdr_fields > 18)
+		x = x + strlen(x) + 1; /* sender */
+	}
+	cksum1_len  = x + strlen(x) + 1 - cksum1_base; /* charset / extra field */
+    }
  
     /* last part is the message body */ 
     cksum2_base = notice->z_message; 
@@ -785,8 +794,9 @@ ZCheckAuthentication(ZNotice_t *notice,
     } 
     /* HOLDING: authctx, authenticator, cksumbuf.data */ 
  
-    memcpy(cksumbuf.data, cksum0_base, cksum0_len); 
-    memcpy(cksumbuf.data + cksum0_len, cksum1_base, cksum1_len); 
+    memcpy(cksumbuf.data, cksum0_base, cksum0_len);
+    if (cksum1_len)
+	memcpy(cksumbuf.data + cksum0_len, cksum1_base, cksum1_len); 
     memcpy(cksumbuf.data + cksum0_len + cksum1_len, 
            cksum2_base, cksum2_len);