Ticket History #2545: AFS string_to_key broken for passwords > 8 chars

From krb5-bugs-incoming-bounces@mit.edu Thu Apr 29 18:29:20 2004
Received: from pch.mit.edu (PCH.MIT.EDU [18.7.21.90]) by krbdev.mit.edu (8.9.3p2) with ESMTP
id SAA05074; Thu, 29 Apr 2004 18:29:19 -0400 (EDT)
Received: from pch.mit.edu (localhost [127.0.0.1])
by pch.mit.edu (8.12.8p2/8.12.8) with ESMTP id i3TMTJos012983
for <krb5-send-pr@krbdev.mit.edu>; Thu, 29 Apr 2004 18:29:19 -0400 (EDT)
Received: from fort-point-station.mit.edu (FORT-POINT-STATION.MIT.EDU
[18.7.7.76])
by pch.mit.edu (8.12.8p2/8.12.8) with ESMTP id i3RMbjos014847
for <krb5-bugs-incoming@PCH.mit.edu>;
Tue, 27 Apr 2004 18:37:45 -0400 (EDT)
Received: from mailgate02.slac.stanford.edu (mailgate02.slac.stanford.edu
[134.79.18.92])i3RMbiZW012897
for <krb5-bugs@mit.edu>; Tue, 27 Apr 2004 18:37:44 -0400 (EDT)
Received: from telemark.slac.stanford.edu (telemark.slac.stanford.edu
[134.79.24.241])i3RMbhgR023819
for <krb5-bugs@mit.edu>; Tue, 27 Apr 2004 15:37:43 -0700 (PDT)
(envelope-from bbense@slac.stanford.edu)
Received: (from bbense@localhost)i3RMbhD8031760;
Tue, 27 Apr 2004 15:37:43 -0700
Date: Tue, 27 Apr 2004 15:37:43 -0700
From: Booker Bense <bbense@slac.stanford.edu>
Message-Id: <200404272237.i3RMbhD8031760@telemark.slac.stanford.edu>
To: krb5-bugs@mit.edu
X-send-pr-version: 3.99
X-Mailman-Approved-At: Thu, 29 Apr 2004 17:36:14 -0400
X-BeenThere: krb5-bugs-incoming@mit.edu
X-Mailman-Version: 2.1
Precedence: list
Reply-To: bbense@slac.stanford.edu
Sender: krb5-bugs-incoming-bounces@mit.edu
Errors-To: krb5-bugs-incoming-bounces@mit.edu



System: Linux telemark 2.4.21-9.0.1.EL #1 Mon Feb 9 22:44:14 EST 2004 i686 i686 i386 GNU/Linux
Architecture: i686


The function mit_des_string_to_key_int contains this work around
for use with converted AFS keys in a Heimdal KDC.

if (salt->length == SALT_TYPE_AFS_LENGTH || salt->length == (unsigned) -1) {
krb5_data salt2;
char *c;
c = strchr(salt->data, '@');
if (c != NULL) *c = '\0'; /* workaround from krb5-clients/1146 */
salt2.data = salt->data;
salt2.length = strlen (salt2.data);
/* cheat and do AFS string2key instead */
return mit_afs_string_to_key (keyblock, data, &salt2);

However, this code incorrectly computes the length of the salt2 if
the '@' is missing from the original salt data. In fact there is no
guarantee that strlen is going to return anything but garbage. There
is the potential for a buffer overflow in this code.

Unfortunately, due to the extremely brain-damaged solution of using a
length field as a data type flag, there is no quick and simple solution
to this problem.


Use a password of longer than 8 chars with an AFS string_to_key
algorithm.



Obviously, if there are other key salt type available this
problem goes away. Changing your password after conversion is one
work around. As far as fixing the actual code itself, this would require
significant reworking of the libraries. At a minimum you should
add some sanity checks before calling strlen, it also would be a
good idea to check the result of strchr as well.

One work around might be to convince the Heimdal KDC to send the
appropriate etype_info2 s2kparams to indicate that the AFS3 salt
should be used. If your KDC does this, our code should do the right
thing.

bbense@slac> _ Is there some code path I'm missing?

Yes, I believe so.

I will check next week.

Some archaeology on the evolution of this and related bugs:

* 1.0-beta6: etype-info support is added, but is only returned in the
preauth hint list (not AS replies) and is only used by the client
preauth system after a preauth-required error (not to decrypt AS
replies).

AFS string-to-key support is also added. It is communicated with the
afs3-salt padata type and a null-terminated salt. The client reacts by
setting a salt->length of -1. mit_des_string_to_key_int recognizes this
and calls mit_afs_string_to_key, which (unsafely) assumes that the salt
is null-terminated.

AFS keys do not appear to work with etype-info (no type information is
communicated), which means AFS keys probably don't work with preauth at
this point.

* 1.1: the get_init_creds system is added. The corresponding rewritten
preauth system (preauth2.c) uses the same logic to handle the hint list
and the AS reply, but appears to have no etype-info support (so it uses
pw-salt/afs3-salt in both the hint list and the AS reply).

* 1.1.1: preauth2.c gains etype-info support, preferring it over pw-
salt/afs3-salt, in order to work around a 1.0 KDC bug in the generation
of pw-salt in the preauth hint list. This is the first time etype-info
is used by the client when decrypting AS replies (but the KDC isn't
sending etype-info in AS replies, so AFS keys still mostly work in
practice).

* 1.2.6: a hack is added to truncate the salt at '@' for
interoperability with Heimdal.

* 1.3: etype-info2 support is added to the client and KDC. preauth2.c
prefers etype-info2 over etype-info. etype-info is suppressed if newer
enctypes (which basically means AES) are present in the request. etype-
info2 is returned in the AS reply as well as the hint list, but etype-
info is still never returned there. etype-info2 tags AFS keys using
s2kparams (it does not null-terminate the salt value).
krb5int_des_string_to_key recognizes the s2kparams and calls
mit_afs_string_to_key. mit_afs_string_to_key is fixed to use salt-
assumption of null-terminated salt values. AFS keys probably work with
preauth at this point (given a 1.3 KDC and client).

The old AFS trap door in mit_des_string_to_key_int still exists, still
has the '@' hack, and is still used if the AS reply contains no etype-
info2. It sets a length on the salt value passed to
mit_afs_string_to_key, but still assumes a null-terminated salt value
when doing so.

krb5_data's length element changes from int to unsigned int in this
release, and a constant SALT_TYPE_AFS_LENGTH (equal to UINT_MAX)
replaces the use of -1 in salt->length.

* 1.4.3: the KDC starts including etype-info in AS replies, if no newer
enctype is requested, in order to better conform with RFC 4120 (#3207).
Since etype-info doesn't tag AFS keys, 1.1.1 through 1.2.8 clients break
with AFS keys. This is reported in #3456 with a candidate patch to
avoid sending etype-info for AFS keys, but the patch is not taken. (It
may still be a good idea, although the interop issue is almost certainly
moot by now.)

Nothing has really changed since then. I believe AFS keys work along
all code paths assuming that afs3-salt padata values come with null
termination (true of all MIT KDC versions), but that code path on the
client is still unsafe.

I would speculate that this particular bug report pertains to a Heimdal
KDC which was sending afs3-salt padata values with (1) no null
terminator, (2) no '@' in the salt, and (3) a salt (realm) longer than
eight bytes. When the password is eight bytes or less, only the first
eight bytes of the salt are used and everything works. When the
password is more than eight bytes, the full salt is used, with some
trailing garbage due to the lack of a null terminator.

A much cleaner, but more invasive, fix would be to stop using
SALT_TYPE_AFS_LENGTH to tag AFS salts, and instead use s2kparams along all
code paths. Some care would be required to preserve the '@' hack for
afs3-salt values.

I think it should resolve the problem, yes. I realize this is probably
coming long after people stopped having AFS-salted keys to worry about.