SA-2010-007 Checksum vulnerabilities (CVE-2010-1324 and others)

Fix multiple checksum handling bugs, as described in:
  CVE-2010-1324
  CVE-2010-1323
  CVE-2010-4020
  CVE-2010-4021

* Return the correct (keyed) checksums as the mandatory checksum type
  for DES enctypes.
* Restrict simplified-profile checksums to their corresponding etypes.
* Add internal checks to reduce the risk of stream ciphers being used
  with simplified-profile key derivation or other algorithms relying
  on the block encryption primitive.
* Use the mandatory checksum type for the PKINIT KDC signature,
  instead of the first-listed keyed checksum.
* Use the mandatory checksum type when sending KRB-SAFE messages by
  default, instead of the first-listed keyed checksum.
* Use the mandatory checksum type for the t_kperf test program.
* Use the mandatory checksum type (without additional logic) for the
  FAST request checksum.
* Preserve the existing checksum choices (unkeyed checksums for DES
  enctypes) for the authenticator checksum, using explicit logic.
* Ensure that SAM checksums received from the KDC are keyed.
* Ensure that PAC checksums are keyed.

ticket: 6827

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

12 files changed, 82 insertions, 81 deletions

diff --git a/src/lib/crypto/crypto_tests/t_kperf.c b/src/lib/crypto/crypto_tests/t_kperf.c
index 8c36e90..a07a364 100644
--- a/src/lib/crypto/crypto_tests/t_kperf.c
+++ b/src/lib/crypto/crypto_tests/t_kperf.c
@@ -49,9 +49,8 @@ main(int argc, char **argv)
     krb5_keyblock kblock;
     krb5_key key;
     krb5_enctype enctype;
-    krb5_cksumtype cktype, *cktypelist;
+    krb5_cksumtype cktype;
     int blocksize, num_blocks, intf, op, i;
-    unsigned int count;
     size_t outlen, cklen;
     krb5_data block;
     krb5_enc_data outblock;
@@ -69,11 +68,6 @@ main(int argc, char **argv)
     blocksize = atoi(argv[3]);
     num_blocks = atoi(argv[4]);
 
-    /* Pick the first available keyed checksum type. */
-    krb5_c_keyed_checksum_types(NULL, enctype, &count, &cktypelist);
-    assert(count > 0);
-    cktype = cktypelist[0];
-
     block.data = "notrandom";
     block.length = 9;
     krb5_c_random_seed(NULL, &block);
@@ -89,6 +83,7 @@ main(int argc, char **argv)
     outblock.ciphertext.length = outlen;
     outblock.ciphertext.data = calloc(1, outlen);
 
+    krb5int_c_mandatory_cksumtype(NULL, enctype, &cktype);
     krb5_c_checksum_length(NULL, cktype, &cklen);
     sum.checksum_type = cktype;
     sum.length = cklen;
diff --git a/src/lib/crypto/krb/cksumtypes.c b/src/lib/crypto/krb/cksumtypes.c
index bcd5140..a8b1c87 100644
--- a/src/lib/crypto/krb/cksumtypes.c
+++ b/src/lib/crypto/krb/cksumtypes.c
@@ -76,7 +76,7 @@ const struct krb5_cksumtypes krb5int_cksumtypes_list[] = {
 
     { CKSUMTYPE_HMAC_SHA1_DES3,
       "hmac-sha1-des3", { "hmac-sha1-des3-kd" }, "HMAC-SHA1 DES3 key",
-      NULL, &krb5int_hash_sha1,
+      &krb5int_enc_des3, &krb5int_hash_sha1,
       krb5int_dk_checksum, NULL,
       20, 20, 0 },
 
@@ -89,19 +89,19 @@ const struct krb5_cksumtypes krb5int_cksumtypes_list[] = {
 
     { CKSUMTYPE_HMAC_SHA1_96_AES128,
       "hmac-sha1-96-aes128", { 0 }, "HMAC-SHA1 AES128 key",
-      NULL, &krb5int_hash_sha1,
+      &krb5int_enc_aes128, &krb5int_hash_sha1,
       krb5int_dk_checksum, NULL,
       20, 12, 0 },
 
     { CKSUMTYPE_HMAC_SHA1_96_AES256,
       "hmac-sha1-96-aes256", { 0 }, "HMAC-SHA1 AES256 key",
-      NULL, &krb5int_hash_sha1,
+      &krb5int_enc_aes256, &krb5int_hash_sha1,
       krb5int_dk_checksum, NULL,
       20, 12, 0 },
 
     { CKSUMTYPE_MD5_HMAC_ARCFOUR,
       "md5-hmac-rc4", { 0 }, "Microsoft MD5 HMAC",
-      NULL, &krb5int_hash_md5,
+      &krb5int_enc_arcfour, &krb5int_hash_md5,
       krb5int_hmacmd5_checksum, NULL,
       16, 16, 0 },
 
diff --git a/src/lib/crypto/krb/dk/checksum_hmac.c b/src/lib/crypto/krb/dk/checksum_hmac.c
index ae51aa3..5f333af 100644
--- a/src/lib/crypto/krb/dk/checksum_hmac.c
+++ b/src/lib/crypto/krb/dk/checksum_hmac.c
@@ -39,20 +39,12 @@ krb5int_dk_checksum(const struct krb5_cksumtypes *ctp,
                     krb5_data *output)
 {
     const struct krb5_keytypes *ktp;
-    const struct krb5_enc_provider *enc;
+    const struct krb5_enc_provider *enc = ctp->enc;
     krb5_error_code ret;
     unsigned char constantdata[K5CLENGTH];
     krb5_data datain;
     krb5_key kc;
 
-    /* Use the key's enctype (more flexible than setting an enctype in ctp). */
-    ktp = find_enctype(key->keyblock.enctype);
-    if (ktp == NULL)
-        return KRB5_BAD_ENCTYPE;
-    enc = ktp->enc;
-    if (key->keyblock.length != enc->keylength)
-        return KRB5_BAD_KEYSIZE;
-
     /* Derive the key. */
     datain = make_data(constantdata, K5CLENGTH);
     store_32_be(usage, constantdata);
diff --git a/src/lib/crypto/krb/dk/derive.c b/src/lib/crypto/krb/dk/derive.c
index d309206..3e9af28 100644
--- a/src/lib/crypto/krb/dk/derive.c
+++ b/src/lib/crypto/krb/dk/derive.c
@@ -91,6 +91,8 @@ derive_random_rfc3961(const struct krb5_enc_provider *enc,
     blocksize = enc->block_size;
     keybytes = enc->keybytes;
 
+    if (blocksize == 1)
+        return KRB5_BAD_ENCTYPE;
     if (inkey->keyblock.length != enc->keylength || outrnd->length != keybytes)
         return KRB5_CRYPTO_INTERNAL;
 
diff --git a/src/lib/crypto/krb/etypes.c b/src/lib/crypto/krb/etypes.c
index 9703a93..ec9fb1d 100644
--- a/src/lib/crypto/krb/etypes.c
+++ b/src/lib/crypto/krb/etypes.c
@@ -51,7 +51,7 @@ const struct krb5_keytypes krb5int_enctypes_list[] = {
       krb5int_des_string_to_key,
       krb5int_des_prf,
       krb5int_init_state_enc, krb5int_free_state_enc,
-      CKSUMTYPE_RSA_MD5,
+      CKSUMTYPE_RSA_MD5_DES,
       ETYPE_WEAK },
     { ENCTYPE_DES_CBC_MD4,
       "des-cbc-md4", { 0 }, "DES cbc mode with RSA-MD4",
@@ -61,7 +61,7 @@ const struct krb5_keytypes krb5int_enctypes_list[] = {
       krb5int_des_string_to_key,
       krb5int_des_prf,
       krb5int_init_state_enc, krb5int_free_state_enc,
-      CKSUMTYPE_RSA_MD4,
+      CKSUMTYPE_RSA_MD4_DES,
       ETYPE_WEAK },
     { ENCTYPE_DES_CBC_MD5,
       "des-cbc-md5", { "des" }, "DES cbc mode with RSA-MD5",
@@ -71,7 +71,7 @@ const struct krb5_keytypes krb5int_enctypes_list[] = {
       krb5int_des_string_to_key,
       krb5int_des_prf,
       krb5int_init_state_enc, krb5int_free_state_enc,
-      CKSUMTYPE_RSA_MD5,
+      CKSUMTYPE_RSA_MD5_DES,
       ETYPE_WEAK },
     { ENCTYPE_DES_CBC_RAW,
       "des-cbc-raw", { 0 }, "DES cbc mode raw",
diff --git a/src/lib/crypto/krb/etypes.h b/src/lib/crypto/krb/etypes.h
index 70cb7bc..bbf2bf3 100644
--- a/src/lib/crypto/krb/etypes.h
+++ b/src/lib/crypto/krb/etypes.h
@@ -108,8 +108,8 @@ encrypt_block(const struct krb5_enc_provider *enc, krb5_key key,
 {
     krb5_crypto_iov iov;
 
-    /* Verify that block is the right length. */
-    if (block->length != enc->block_size)
+    /* Verify that this is a block cipher and block is the right length. */
+    if (block->length != enc->block_size || enc->block_size == 1)
         return EINVAL;
     iov.flags = KRB5_CRYPTO_TYPE_DATA;
     iov.data = *block;
diff --git a/src/lib/krb5/krb/fast.c b/src/lib/krb5/krb/fast.c
index cc9f899..3137195 100644
--- a/src/lib/krb5/krb/fast.c
+++ b/src/lib/krb5/krb/fast.c
@@ -211,7 +211,6 @@ krb5int_fast_prep_req(krb5_context context,
     krb5_data *encoded_fast_req = NULL;
     krb5_data *encoded_armored_req = NULL;
     krb5_data *local_encoded_result = NULL;
-    krb5_cksumtype cksumtype;
     krb5_data random_data;
     char random_buf[4];
 
@@ -247,15 +246,8 @@ krb5int_fast_prep_req(krb5_context context,
     }
     if (retval == 0)
         armored_req->armor = state->armor;
-    if (retval == 0)
-        retval = krb5int_c_mandatory_cksumtype(context,
-                                               state->armor_key->enctype,
-                                               &cksumtype);
-    /* DES enctypes have unkeyed mandatory checksums; need a keyed one. */
-    if (retval == 0 && !krb5_c_is_keyed_cksum(cksumtype))
-        cksumtype = CKSUMTYPE_RSA_MD5_DES;
     if (retval ==0)
-        retval = krb5_c_make_checksum(context, cksumtype, state->armor_key,
+        retval = krb5_c_make_checksum(context, 0, state->armor_key,
                                       KRB5_KEYUSAGE_FAST_REQ_CHKSUM,
                                       to_be_checksummed,
                                       &armored_req->req_checksum);
diff --git a/src/lib/krb5/krb/mk_req_ext.c b/src/lib/krb5/krb/mk_req_ext.c
index 3a4004d..1979b8d 100644
--- a/src/lib/krb5/krb/mk_req_ext.c
+++ b/src/lib/krb5/krb/mk_req_ext.c
@@ -81,6 +81,36 @@ generate_authenticator(krb5_context,
                        krb5_enctype *desired_etypes,
                        krb5_enctype tkt_enctype);
 
+/* Return the checksum type for the AP request, or 0 to use the enctype's
+ * mandatory checksum. */
+static krb5_cksumtype
+ap_req_cksum(krb5_context context, krb5_auth_context auth_context,
+             krb5_enctype enctype)
+{
+    /* Use the configured checksum type if one was set. */
+    if (auth_context->req_cksumtype)
+        return auth_context->req_cksumtype;
+
+    /*
+     * Otherwise choose based on the enctype.  For interoperability with very
+     * old implementations, use unkeyed MD4 or MD5 checkums for DES enctypes.
+     * (The authenticator checksum does not have to be keyed since it is
+     * contained within an encrypted blob.)
+     */
+    switch (enctype) {
+    case ENCTYPE_DES_CBC_CRC:
+    case ENCTYPE_DES_CBC_MD5:
+        return CKSUMTYPE_RSA_MD5;
+        break;
+    case ENCTYPE_DES_CBC_MD4:
+        return CKSUMTYPE_RSA_MD4;
+        break;
+    default:
+        /* Use the mandatory checksum type for the enctype. */
+        return 0;
+    }
+}
+
 krb5_error_code KRB5_CALLCONV
 krb5_mk_req_extended(krb5_context context, krb5_auth_context *auth_context,
                      krb5_flags ap_req_options, krb5_data *in_data,
@@ -169,13 +199,8 @@ krb5_mk_req_extended(krb5_context context, krb5_auth_context *auth_context,
         } else {
             krb5_enctype enctype = krb5_k_key_enctype(context,
                                                       (*auth_context)->key);
-            krb5_cksumtype cksumtype;
-            retval = krb5int_c_mandatory_cksumtype(context, enctype,
-                                                   &cksumtype);
-            if (retval)
-                goto cleanup_cksum;
-            if ((*auth_context)->req_cksumtype)
-                cksumtype = (*auth_context)->req_cksumtype;
+            krb5_cksumtype cksumtype = ap_req_cksum(context, *auth_context,
+                                                    enctype);
             if ((retval = krb5_k_make_checksum(context,
                                                cksumtype,
                                                (*auth_context)->key,
diff --git a/src/lib/krb5/krb/mk_safe.c b/src/lib/krb5/krb/mk_safe.c
index eaa3add..334701d 100644
--- a/src/lib/krb5/krb/mk_safe.c
+++ b/src/lib/krb5/krb/mk_safe.c
@@ -59,10 +59,9 @@ krb5_mk_safe_basic(krb5_context context, const krb5_data *userdata,
     krb5_checksum safe_checksum;
     krb5_data *scratch1, *scratch2;
 
-    if (!krb5_c_valid_cksumtype(sumtype))
+    if (sumtype && !krb5_c_valid_cksumtype(sumtype))
         return KRB5_PROG_SUMTYPE_NOSUPP;
-    if (!krb5_c_is_coll_proof_cksum(sumtype)
-        || !krb5_c_is_keyed_cksum(sumtype))
+    if (sumtype && !krb5_c_is_keyed_cksum(sumtype))
         return KRB5KRB_AP_ERR_INAPP_CKSUM;
 
     safemsg.user_data = *userdata;
@@ -110,6 +109,30 @@ cleanup_checksum:
     return retval;
 }
 
+/* Return the checksum type for the KRB-SAFE message, or 0 to use the enctype's
+ * mandatory checksum. */
+static krb5_cksumtype
+safe_cksumtype(krb5_context context, krb5_auth_context auth_context,
+               krb5_enctype enctype)
+{
+    krb5_error_code retval;
+    unsigned int nsumtypes, i;
+    krb5_cksumtype *sumtypes;
+
+    /* Use the auth context's safe_cksumtype if it is valid for the enctype.
+     * Otherwise return 0 for the mandatory checksum. */
+    retval = krb5_c_keyed_checksum_types(context, enctype, &nsumtypes,
+                                         &sumtypes);
+    if (retval != 0 || nsumtypes == 0)
+        return 0;
+    for (i = 0; i < nsumtypes; i++) {
+        if (auth_context->safe_cksumtype == sumtypes[i])
+            break;
+    }
+    krb5_free_cksumtypes(context, sumtypes);
+    return (i == nsumtypes) ? 0 : auth_context->safe_cksumtype;
+}
+
 krb5_error_code KRB5_CALLCONV
 krb5_mk_safe(krb5_context context, krb5_auth_context auth_context,
              const krb5_data *userdata, krb5_data *outbuf,
@@ -195,31 +218,7 @@ krb5_mk_safe(krb5_context context, krb5_auth_context auth_context,
             }
         }
 
-        {
-            krb5_enctype enctype = krb5_k_key_enctype(context, key);
-            unsigned int nsumtypes;
-            unsigned int i;
-            krb5_cksumtype *sumtypes;
-            retval = krb5_c_keyed_checksum_types (context, enctype,
-                                                  &nsumtypes, &sumtypes);
-            if (retval) {
-                CLEANUP_DONE ();
-                goto error;
-            }
-            if (nsumtypes == 0) {
-                retval = KRB5_BAD_ENCTYPE;
-                krb5_free_cksumtypes (context, sumtypes);
-                CLEANUP_DONE ();
-                goto error;
-            }
-            for (i = 0; i < nsumtypes; i++)
-                if (auth_context->safe_cksumtype == sumtypes[i])
-                    break;
-            if (i == nsumtypes)
-                i = 0;
-            sumtype = sumtypes[i];
-            krb5_free_cksumtypes (context, sumtypes);
-        }
+        sumtype = safe_cksumtype(context, auth_context, key->keyblock.enctype);
         if ((retval = krb5_mk_safe_basic(context, userdata, key, &replaydata,
                                          plocal_fulladdr, premote_fulladdr,
                                          sumtype, outbuf))) {
diff --git a/src/lib/krb5/krb/pac.c b/src/lib/krb5/krb/pac.c
index 5b6e182..983b4e8 100644
--- a/src/lib/krb5/krb/pac.c
+++ b/src/lib/krb5/krb/pac.c
@@ -533,6 +533,8 @@ k5_pac_verify_server_checksum(krb5_context context,
     checksum.checksum_type = load_32_le(p);
     checksum.length = checksum_data.length - PAC_SIGNATURE_DATA_LENGTH;
     checksum.contents = p + PAC_SIGNATURE_DATA_LENGTH;
+    if (!krb5_c_is_keyed_cksum(checksum.checksum_type))
+        return KRB5KRB_AP_ERR_INAPP_CKSUM;
 
     pac_data.length = pac->data.length;
     pac_data.data = malloc(pac->data.length);
@@ -603,6 +605,8 @@ k5_pac_verify_kdc_checksum(krb5_context context,
     checksum.checksum_type = load_32_le(p);
     checksum.length = privsvr_checksum.length - PAC_SIGNATURE_DATA_LENGTH;
     checksum.contents = p + PAC_SIGNATURE_DATA_LENGTH;
+    if (!krb5_c_is_keyed_cksum(checksum.checksum_type))
+        return KRB5KRB_AP_ERR_INAPP_CKSUM;
 
     server_checksum.data += PAC_SIGNATURE_DATA_LENGTH;
     server_checksum.length -= PAC_SIGNATURE_DATA_LENGTH;
diff --git a/src/lib/krb5/krb/preauth2.c b/src/lib/krb5/krb/preauth2.c
index 5d7d244..f4896eb 100644
--- a/src/lib/krb5/krb/preauth2.c
+++ b/src/lib/krb5/krb/preauth2.c
@@ -1283,7 +1283,9 @@ pa_sam_2(krb5_context context, krb5_kdc_req *request, krb5_pa_data *in_padata,
 
     cksum = sc2->sam_cksum;
 
-    while (*cksum) {
+    for (; *cksum; cksum++) {
+        if (!krb5_c_is_keyed_cksum((*cksum)->checksum_type))
+            continue;
         /* Check this cksum */
         retval = krb5_c_verify_checksum(context, as_key,
                                         KRB5_KEYUSAGE_PA_SAM_CHALLENGE_CKSUM,
@@ -1297,7 +1299,6 @@ pa_sam_2(krb5_context context, krb5_kdc_req *request, krb5_pa_data *in_padata,
         }
         if (valid_cksum)
             break;
-        cksum++;
     }
 
     if (!valid_cksum) {
diff --git a/src/plugins/preauth/pkinit/pkinit_srv.c b/src/plugins/preauth/pkinit/pkinit_srv.c
index 719b01c..5a7a5ad 100644
--- a/src/plugins/preauth/pkinit/pkinit_srv.c
+++ b/src/plugins/preauth/pkinit/pkinit_srv.c
@@ -691,8 +691,6 @@ pkinit_server_return_padata(krb5_context context,
     krb5_reply_key_pack *key_pack = NULL;
     krb5_reply_key_pack_draft9 *key_pack9 = NULL;
     krb5_data *encoded_key_pack = NULL;
-    unsigned int num_types;
-    krb5_cksumtype *cksum_types = NULL;
 
     pkinit_kdc_context plgctx;
     pkinit_kdc_req_context reqctx;
@@ -882,14 +880,8 @@ pkinit_server_return_padata(krb5_context context,
                 retval = ENOMEM;
                 goto cleanup;
             }
-            /* retrieve checksums for a given enctype of the reply key */
-            retval = krb5_c_keyed_checksum_types(context,
-                                                 encrypting_key->enctype, &num_types, &cksum_types);
-            if (retval)
-                goto cleanup;
 
-            /* pick the first of acceptable enctypes for the checksum */
-            retval = krb5_c_make_checksum(context, cksum_types[0],
+            retval = krb5_c_make_checksum(context, 0,
                                           encrypting_key, KRB5_KEYUSAGE_TGS_REQ_AUTH_CKSUM,
                                           req_pkt, &key_pack->asChecksum);
             if (retval) {
@@ -1033,7 +1025,6 @@ cleanup:
         krb5_free_data(context, encoded_key_pack);
     free(dh_pubkey);
     free(server_key);
-    free(cksum_types);
 
     switch ((int)padata->pa_type) {
     case KRB5_PADATA_PK_AS_REQ: