krb5: make keyed checksums mandatory where possible

Make keyed checksums mandatory when generating and verifying checksums, with
the following exceptions:

* the checksum is being generated or verified as part of encrypting data for
  a legacy (DES) encryption type

* the KRB5_CRYPTO_FLAG_ALLOW_UNKEYED_CHECKSUM flag was set on the crypto
  context, used to allow unkeyed checksums in krb5 authenticators

By making unkeyed checksums opt-in, we eliminate a class of potential
vulnerabilities where callers could pass unkeyed checksums.

Any code that uses the mandatory checksum type for a given non-legacy
encryption type should not be affected by this change. It could potentially
break, say, a client trying to do FAST with DES keys but, that should not be
supported (because FAST KDCs also support AES).

Closes: #835

kdc/krb5tgs.c

@@ -809,16 +809,8 @@ tgs_check_authenticator(krb5_context context,
 	ret = KRB5KRB_AP_ERR_INAPP_CKSUM;
 	goto out;
     }
-    /*
+
-     * according to RFC1510 it doesn't need to be keyed,
+    if (!krb5_checksum_is_collision_proof(context, auth->cksum->cksumtype)) {
-     * but according to the latest draft it needs to.
-     */
-    if (
-#if 0
-!krb5_checksum_is_keyed(context, auth->cksum->cksumtype)
-	||
-#endif
- !krb5_checksum_is_collision_proof(context, auth->cksum->cksumtype)) {
 	kdc_log(context, config, 4, "Bad checksum type in authenticator: %d",
 		auth->cksum->cksumtype);
 	ret =  KRB5KRB_AP_ERR_INAPP_CKSUM;
@@ -832,6 +824,12 @@ tgs_check_authenticator(krb5_context context,
 	krb5_free_error_message(context, msg);
 	goto out;
     }
+
+    /*
+     * RFC4120 says the checksum must be collision-proof, but it does
+     * not require it to be keyed (as the authenticator is encrypted).
+     */
+    _krb5_crypto_set_flags(context, crypto, KRB5_CRYPTO_FLAG_ALLOW_UNKEYED_CHECKSUM);
     ret = krb5_verify_checksum(context,
 			       crypto,
 			       KRB5_KU_TGS_REQ_AUTH_CKSUM,

lib/krb5/crypto.c

@@ -472,6 +472,7 @@ create_checksum_iov(krb5_context context,
 		    unsigned usage,
 		    struct krb5_crypto_iov *iov,
 		    int niov,
+		    krb5_flags flags,
 		    Checksum *result)
 {
     krb5_error_code ret;
@@ -485,6 +486,8 @@ create_checksum_iov(krb5_context context,
 	ret = get_checksum_key(context, crypto, usage, ct, &dkey);
 	if (ret)
 	    return ret;
+    } else if ((flags & KRB5_CRYPTO_FLAG_ALLOW_UNKEYED_CHECKSUM) == 0) {
+	return EINVAL;
     } else
 	dkey = NULL;
 
@@ -500,6 +503,7 @@ create_checksum (krb5_context context,
 		 unsigned usage,
 		 void *data,
 		 size_t len,
+		 krb5_flags flags,
 		 Checksum *result)
 {
     int ret;
@@ -513,7 +517,7 @@ create_checksum (krb5_context context,
     iov[0].data.length = len;
     iov[0].flags = KRB5_CRYPTO_TYPE_DATA;
 
-    return create_checksum_iov(context, ct, crypto, usage, iov, 1, result);
+    return create_checksum_iov(context, ct, crypto, usage, iov, 1, flags, result);
 }
 
 static int
@@ -523,6 +527,16 @@ arcfour_checksum_p(struct _krb5_checksum_type *ct, krb5_crypto crypto)
 	(crypto->key.key->keytype == KEYTYPE_ARCFOUR);
 }
 
+static inline krb5_flags
+crypto_flags(krb5_crypto crypto)
+{
+    /* If caller didn't specify a key, unkeyed checksums are the only option */
+    if (crypto == NULL)
+	return KRB5_CRYPTO_FLAG_ALLOW_UNKEYED_CHECKSUM;
+    else
+	return crypto->flags;
+}
+
 KRB5_LIB_FUNCTION krb5_error_code KRB5_LIB_CALL
 krb5_create_checksum(krb5_context context,
 		     krb5_crypto crypto,
@@ -557,8 +571,8 @@ krb5_create_checksum(krb5_context context,
     } else
 	keyusage = CHECKSUM_USAGE(usage);
 
-    return create_checksum(context, ct, crypto, keyusage,
+    return create_checksum(context, ct, crypto, keyusage, data, len,
-			   data, len, result);
+			   crypto_flags(crypto), result);
 }
 
 static krb5_error_code
@@ -567,6 +581,7 @@ verify_checksum_iov(krb5_context context,
 		    unsigned usage, /* not krb5_key_usage */
 		    struct krb5_crypto_iov *iov,
 		    int niov,
+		    krb5_flags flags,
 		    Checksum *cksum)
 {
     krb5_error_code ret;
@@ -595,6 +610,13 @@ verify_checksum_iov(krb5_context context,
 	ret = get_checksum_key(context, crypto, usage, ct, &dkey);
 	if (ret)
 	    return ret;
+    } else if ((flags & KRB5_CRYPTO_FLAG_ALLOW_UNKEYED_CHECKSUM) == 0) {
+	krb5_clear_error_message (context);
+	krb5_set_error_message(context, KRB5KRB_AP_ERR_BAD_INTEGRITY,
+			       N_("Unkeyed checksum type %s provided where keyed "
+				  "checksum was expected", ""), ct->name);
+
+	return KRB5KRB_AP_ERR_INAPP_CKSUM;
     } else
 	dkey = NULL;
 
@@ -642,6 +664,7 @@ verify_checksum(krb5_context context,
 		unsigned usage, /* not krb5_key_usage */
 		void *data,
 		size_t len,
+		krb5_flags flags,
 		Checksum *cksum)
 {
     struct krb5_crypto_iov iov[1];
@@ -650,7 +673,7 @@ verify_checksum(krb5_context context,
     iov[0].data.length = len;
     iov[0].flags = KRB5_CRYPTO_TYPE_DATA;
 
-    return verify_checksum_iov(context, crypto, usage, iov, 1, cksum);
+    return verify_checksum_iov(context, crypto, usage, iov, 1, flags, cksum);
 }
 
 KRB5_LIB_FUNCTION krb5_error_code KRB5_LIB_CALL
@@ -679,7 +702,7 @@ krb5_verify_checksum(krb5_context context,
 	keyusage = CHECKSUM_USAGE(usage);
 
     return verify_checksum(context, crypto, keyusage,
-			   data, len, cksum);
+			   data, len, crypto_flags(crypto), cksum);
 }
 
 KRB5_LIB_FUNCTION krb5_error_code KRB5_LIB_CALL
@@ -973,6 +996,7 @@ encrypt_internal_derived(krb5_context context,
 			  INTEGRITY_USAGE(usage),
 			  p,
 			  block_sz,
+			  0,
 			  &cksum);
     if(ret == 0 && cksum.checksum.length != checksum_sz) {
 	free_Checksum (&cksum);
@@ -1060,6 +1084,7 @@ encrypt_internal_enc_then_cksum(krb5_context context,
 			  INTEGRITY_USAGE(usage),
 			  ivc,
 			  et->blocksize + block_sz,
+			  0,
 			  &cksum);
     if(ret == 0 && cksum.checksum.length != checksum_sz) {
 	free_Checksum (&cksum);
@@ -1116,6 +1141,7 @@ encrypt_internal(krb5_context context,
 			  0,
 			  p,
 			  block_sz,
+			  KRB5_CRYPTO_FLAG_ALLOW_UNKEYED_CHECKSUM,
 			  &cksum);
     if(ret == 0 && cksum.checksum.length != checksum_sz) {
 	krb5_clear_error_message (context);
@@ -1241,6 +1267,7 @@ decrypt_internal_derived(krb5_context context,
 			  INTEGRITY_USAGE(usage),
 			  p,
 			  len,
+			  0,
 			  &cksum);
     if(ret) {
 	free(p);
@@ -1308,6 +1335,7 @@ decrypt_internal_enc_then_cksum(krb5_context context,
 			  INTEGRITY_USAGE(usage),
 			  p,
 			  et->blocksize + len,
+			  0,
 			  &cksum);
     if(ret) {
 	free(p);
@@ -1389,7 +1417,8 @@ decrypt_internal(krb5_context context,
     }
     memset(p + et->confoundersize, 0, checksum_sz);
     cksum.cksumtype = CHECKSUMTYPE(et->checksum);
-    ret = verify_checksum(context, NULL, 0, p, len, &cksum);
+    ret = verify_checksum(context, NULL, 0, p, len,
+			  KRB5_CRYPTO_FLAG_ALLOW_UNKEYED_CHECKSUM, &cksum);
     free_Checksum(&cksum);
     if(ret) {
 	free(p);
@@ -1739,6 +1768,7 @@ krb5_encrypt_iov_ivec(krb5_context context,
                               INTEGRITY_USAGE(usage),
                               sign_data.data,
                               sign_data.length,
+                              0,
                               &cksum);
 
         if(ret == 0 && cksum.checksum.length != trailersz) {
@@ -1761,6 +1791,7 @@ krb5_encrypt_iov_ivec(krb5_context context,
                               INTEGRITY_USAGE(usage),
                               data,
                               num_data,
+                              0,
                               &cksum);
         if (ret)
             goto cleanup;
@@ -1902,7 +1933,7 @@ krb5_decrypt_iov_ivec(krb5_context context,
 	cksum.cksumtype       = CHECKSUMTYPE(et->keyed_checksum);
 
 	ret = verify_checksum_iov(context, crypto, INTEGRITY_USAGE(usage),
-	                          data, num_data, &cksum);
+	                          data, num_data, 0, &cksum);
 	if(ret)
 	    goto cleanup;
     } else {
@@ -1928,6 +1959,7 @@ krb5_decrypt_iov_ivec(krb5_context context,
 			      INTEGRITY_USAGE(usage),
 			      sign_data.data,
 			      sign_data.length,
+			      0,
 			      &cksum);
 	if(ret)
 	    goto cleanup;
@@ -2022,7 +2054,7 @@ krb5_create_checksum_iov(krb5_context context,
 
     cksum.checksum = civ->data;
     ret = create_checksum_iov(context, ct, crypto, keyusage,
-			      data, num_data, &cksum);
+			      data, num_data, crypto_flags(crypto), &cksum);
 
     if (ret == 0 && type)
 	*type = cksum.cksumtype;
@@ -2081,7 +2113,8 @@ krb5_verify_checksum_iov(krb5_context context,
     } else
 	keyusage = CHECKSUM_USAGE(usage);
 
-    ret = verify_checksum_iov(context, crypto, keyusage, data, num_data, &cksum);
+    ret = verify_checksum_iov(context, crypto, keyusage, data, num_data,
+			      crypto_flags(crypto), &cksum);
 
     if (ret == 0 && type)
 	*type = cksum.cksumtype;
@@ -2563,6 +2596,7 @@ krb5_crypto_init(krb5_context context,
     (*crypto)->key.schedule = NULL;
     (*crypto)->num_key_usage = 0;
     (*crypto)->key_usage = NULL;
+    (*crypto)->flags = 0;
     return 0;
 }
 
@@ -3132,7 +3166,13 @@ krb5_crypto_fx_cf2(krb5_context context,
     return ret;
 }
 
-
+KRB5_LIB_FUNCTION void KRB5_LIB_CALL
+_krb5_crypto_set_flags(krb5_context context,
+		       krb5_crypto crypto,
+		       krb5_flags flags)
+{
+    crypto->flags |= flags;
+}
 
 #ifndef HEIMDAL_SMALLER
 

lib/krb5/crypto.h

@@ -218,6 +218,13 @@ struct krb5_crypto_data {
     HMAC_CTX *hmacctx;
     int num_key_usage;
     struct _krb5_key_usage *key_usage;
+    krb5_flags flags;
 };
 
+/*
+ * Allow generation and verification of unkeyed checksums even when
+ * key material is available.
+ */
+#define KRB5_CRYPTO_FLAG_ALLOW_UNKEYED_CHECKSUM		    0x01
+
 #endif

lib/krb5/libkrb5-exports.def.in

@@ -832,6 +832,7 @@ EXPORTS
 	_krb5_put_int
 	_krb5_s4u2self_to_checksumdata
 	_krb5_HMAC_MD5_checksum
+	_krb5_crypto_set_flags
 	_krb5_expand_path_tokens	;!
 
         ; kinit helper

lib/krb5/mk_req_ext.c

@@ -73,48 +73,37 @@ _krb5_mk_req_internal(krb5_context context,
     if (ret)
 	goto out;
 
-    /* it's unclear what type of checksum we can use.  try the best one, except:
+    /*
-     * a) if it's configured differently for the current realm, or
+     * Use the default checksum type except for some interoperability cases
-     * b) if the session key is des-cbc-crc
+     * with older MIT, DCE and Windows KDCs.
      */
-
     if (in_data) {
-	if(ac->keyblock->keytype == ETYPE_DES_CBC_CRC) {
+	krb5_crypto crypto;
-	    /* this is to make DCE secd (and older MIT kdcs?) happy */
+	krb5_cksumtype checksum_type = CKSUMTYPE_NONE;
-	    ret = krb5_create_checksum(context,
-				       NULL,
-				       0,
-				       CKSUMTYPE_RSA_MD4,
-				       in_data->data,
-				       in_data->length,
-				       &c);
-	} else if(ac->keyblock->keytype == ETYPE_ARCFOUR_HMAC_MD5 ||
-		  ac->keyblock->keytype == ETYPE_ARCFOUR_HMAC_MD5_56 ||
-		  ac->keyblock->keytype == ETYPE_DES_CBC_MD4 ||
-		  ac->keyblock->keytype == ETYPE_DES_CBC_MD5) {
-	    /* this is to make MS kdc happy */
-	    ret = krb5_create_checksum(context,
-				       NULL,
-				       0,
-				       CKSUMTYPE_RSA_MD5,
-				       in_data->data,
-				       in_data->length,
-				       &c);
-	} else {
-	    krb5_crypto crypto;
 
-	    ret = krb5_crypto_init(context, ac->keyblock, 0, &crypto);
+	if (ac->keyblock->keytype == ETYPE_DES_CBC_CRC)
-	    if (ret)
+	    checksum_type = CKSUMTYPE_RSA_MD4;
-		goto out;
+	else if (ac->keyblock->keytype == ETYPE_DES_CBC_MD4 ||
-	    ret = krb5_create_checksum(context,
+		 ac->keyblock->keytype == ETYPE_DES_CBC_MD5 ||
-				       crypto,
+		 ac->keyblock->keytype == ETYPE_ARCFOUR_HMAC_MD5 ||
-				       checksum_usage,
+		 ac->keyblock->keytype == ETYPE_ARCFOUR_HMAC_MD5_56)
-				       0,
+	    checksum_type = CKSUMTYPE_RSA_MD5;
-				       in_data->data,
+	else
-				       in_data->length,
+	    checksum_type = CKSUMTYPE_NONE;
-				       &c);
+
-	    krb5_crypto_destroy(context, crypto);
+	ret = krb5_crypto_init(context, ac->keyblock, 0, &crypto);
-	}
+	if (ret)
+	    goto out;
+
+	_krb5_crypto_set_flags(context, crypto, KRB5_CRYPTO_FLAG_ALLOW_UNKEYED_CHECKSUM);
+	ret = krb5_create_checksum(context,
+				   crypto,
+				   checksum_usage,
+				   checksum_type,
+				   in_data->data,
+				   in_data->length,
+				   &c);
+	krb5_crypto_destroy(context, crypto);
 	c_opt = &c;
     } else {
 	c_opt = NULL;

lib/krb5/rd_req.c

@@ -260,6 +260,8 @@ krb5_verify_authenticator_checksum(krb5_context context,
     ret = krb5_crypto_init(context, key, 0, &crypto);
     if (ret)
 	goto out;
+
+    _krb5_crypto_set_flags(context, crypto, KRB5_CRYPTO_FLAG_ALLOW_UNKEYED_CHECKSUM);
     ret = krb5_verify_checksum(context, crypto,
                                KRB5_KU_AP_REQ_AUTH_CKSUM,
                                data, len, authenticator->cksum);

lib/krb5/version-script.map

@@ -823,6 +823,7 @@ HEIMDAL_KRB5_2.0 {
 		_krb5_put_int;
 		_krb5_s4u2self_to_checksumdata;
 		_krb5_HMAC_MD5_checksum;
+		_krb5_crypto_set_flags;
 
 		# kinit helper
 		krb5_get_init_creds_opt_set_pkinit_user_certs;