[5017] in Kerberos
Re: IMPORTANT: SEVERE interoperability bug in Kerberos V5 beta 4
daemon@ATHENA.MIT.EDU (Theodore Ts'o)
Fri Apr 21 20:53:31 1995
Date: Fri, 21 Apr 1995 20:31:43 +0500
From: Theodore Ts'o <tytso@MIT.EDU>
To: Theodore Ts'o <tytso@MIT.EDU>
Cc: kerberos@MIT.EDU
In-Reply-To: Theodore Ts'o's message of Thu, 20 Apr 1995 23:03:06 +0500,
<9504210303.AA15130@dcl.MIT.EDU>
After more extensive analysis, a flaw was discovered in the backwards
compatibility code (i.e., if BACKWARD_BITMASK_COMPAT is defined). Due
to how bits were allocated for the KDC Options field, this could cause
the algorithm used to detect old, broken clients to fail.
Here is a fixed patch which does a better job. However, note that if
BACKWARD_BITMASK_COMPAT is defined, certain valid KDC options will be
mistakenly identified as "broken" options, and cause a bit reversal when
it shouldn't happen. With this patch, this cases proxy tickets and
rewable tickets to break. Fortunately, the standard distribution does
not use these fields, and so these seems to be an acceptable tradeoff.
However, unless you have a real installed base of broken
implementations, I urge not to define BACKWARD_BITMASK_COMPAT (but you
should still apply this patch so that you are sending the options fields
in the right order). In most cases, vendors should *not* ship with this
define turned on. (Fortunately, as far as I know most vendors did not
ship product based on the broken releases of beta 4, so need for
backwards compatibility should be relatively limited.)
I deeply apologize for the inconvenience caused by this implementation
bug. I had thought we had done sufficient testing to have caught any
problems with our hand-coded parsers, but somehow this slipped through.
- Ted
Index: ChangeLog
===================================================================
RCS file: /mit/krb5/.cvsroot/src/lib/krb5/asn.1/ChangeLog,v
retrieving revision 5.41
diff -c -r5.41 ChangeLog
*** ChangeLog 1995/04/14 03:19:17 5.41
--- ChangeLog 1995/04/22 00:11:55
***************
*** 1,3 ****
--- 1,46 ----
+ Fri Apr 21 09:45:00 1995 Theodore Y. Ts'o <tytso@lurch.mit.edu>
+
+ * asn1_k_decode.c (asn1_decode_kdc_options): Fix
+ BACKWARD_BITMASK_COMPAT so that it doesn't break
+ user-to-user authentication. Unfortunately, this breaks
+ proxy tickets (and renewable tickets continue to be
+ broken if BACKWARD_BITMASK_COMPAT is defined; nothing
+ can be done by this.) Sites should only define
+ BACKWARD_BITMASK_COMPAT if they have an installed base
+ of broken implementations.
+
+ Thu Apr 20 17:41:24 1995 Theodore Y. Ts'o (tytso@dcl)
+
+ * asn1_k_decode.c, krbasn1.h: Move the define of
+ BACKWARDS_BITMASK_COMPAT to asn1_k_decode.c, since it
+ doesn't #include krbasn1.h
+
+ Tue Apr 18 21:46:30 1995 Theodore Y. Ts'o (tytso@dcl)
+
+ * asn1_k_decode.c (asn1_decode_krb5_flags): Make the function
+ accept bit strings which are less 32 bits long.
+ (RFC-1510 makes no guarantee that the length of the bit
+ string must be 32 bits long; the old code required that
+ the length of the bit string must be exactly 32 bits.)
+
+ Flip the bits with respect to a 32-bit boundary, since
+ that's what the old ASN.1 glue code did. (The values in
+ fieldbits.h are encoded backwards, for no good reason.)
+ If BACKWARDS_BITMASK_COMPAT is defined, then only flip the
+ bits if the high 16 bits are clear and there are some bits set
+ in the low 16 bits. This preserves interoperabilty with
+ the old beta 4 distribution, which sent the bit string
+ without flipping them around.
+
+ * asn1_k_encode.c (asn1_encode_krb5_flags): Flip the bits with
+ respect to a 32-bit boundary, since that's what the
+ old ASN.1 glue code did. (The values in fieldbits.h
+ are encoded backwards, for no good reason.)
+
+ * krb_asn1.h: #define BACKWARDS_BITMASK_COMPAT. Add extern
+ declaration for asn1_swbits, which is needed for the
+ bit reversing code.
+
Thu Apr 13 20:13:38 1995 Keith Vetter (keithv@fusion.com)
* asn1_k_decode.c: fixed up 'unreferenced local variable' problems.
Index: asn1_k_decode.c
===================================================================
RCS file: /mit/krb5/.cvsroot/src/lib/krb5/asn.1/asn1_k_decode.c,v
retrieving revision 5.5
diff -c -r5.5 asn1_k_decode.c
*** asn1_k_decode.c 1995/04/14 03:19:19 5.5
--- asn1_k_decode.c 1995/04/21 23:30:02
***************
*** 21,26 ****
--- 21,43 ----
* or implied warranty.
*/
+ /*
+ * The hand-coded parser used in the Beta 4 distribution didn't
+ * reverse the order of the bit string fields. These define allows partial
+ * interoperability with the Beta 4 distribution by doing a bit reversal
+ * on bitfields which have bits set in the high 16 bits.
+ *
+ * Warning: defining this will cause proxiable tickets and renewable
+ * tickets to break. Fortunately, these aren't in common use yet....
+ * Vendors shipping product probably should NOT define this #define,
+ * unless there is an explicit need for backwards compatibility with
+ * Beta 4 implementations. (Which hopefully will be relatively rare.)
+ */
+ #define BACKWARD_BITMASK_COMPAT
+ #ifdef BACKWARD_BITMASK_COMPAT
+ int asn1_always_reverse = 0;
+ #endif
+
#include "asn1_k_decode.h"
#include "asn1_decode.h"
#include "asn1_get.h"
***************
*** 265,276 ****
cleanup();
}
asn1_error_code asn1_decode_krb5_flags(buf, val)
asn1buf * buf;
krb5_flags * val;
{
setup();
! asn1_octet o;
int i;
krb5_flags f=0;
unused_var(taglen);
--- 282,340 ----
cleanup();
}
+ static asn1_octet asn1_pad_mask[] = { 0xFF, 0x7F, 0x3F, 0x1F,
+ 0x0F, 0x07, 0x03, 0x01 };
+
+ asn1_octet asn1_swbits[256] = {
+ 0x00, 0x80, 0x40, 0xc0, 0x20, 0xa0, 0x60, 0xe0,
+ 0x10, 0x90, 0x50, 0xd0, 0x30, 0xb0, 0x70, 0xf0,
+ 0x08, 0x88, 0x48, 0xc8, 0x28, 0xa8, 0x68, 0xe8,
+ 0x18, 0x98, 0x58, 0xd8, 0x38, 0xb8, 0x78, 0xf8,
+ 0x04, 0x84, 0x44, 0xc4, 0x24, 0xa4, 0x64, 0xe4,
+ 0x14, 0x94, 0x54, 0xd4, 0x34, 0xb4, 0x74, 0xf4,
+ 0x0c, 0x8c, 0x4c, 0xcc, 0x2c, 0xac, 0x6c, 0xec,
+ 0x1c, 0x9c, 0x5c, 0xdc, 0x3c, 0xbc, 0x7c, 0xfc,
+ 0x02, 0x82, 0x42, 0xc2, 0x22, 0xa2, 0x62, 0xe2,
+ 0x12, 0x92, 0x52, 0xd2, 0x32, 0xb2, 0x72, 0xf2,
+ 0x0a, 0x8a, 0x4a, 0xca, 0x2a, 0xaa, 0x6a, 0xea,
+ 0x1a, 0x9a, 0x5a, 0xda, 0x3a, 0xba, 0x7a, 0xfa,
+ 0x06, 0x86, 0x46, 0xc6, 0x26, 0xa6, 0x66, 0xe6,
+ 0x16, 0x96, 0x56, 0xd6, 0x36, 0xb6, 0x76, 0xf6,
+ 0x0e, 0x8e, 0x4e, 0xce, 0x2e, 0xae, 0x6e, 0xee,
+ 0x1e, 0x9e, 0x5e, 0xde, 0x3e, 0xbe, 0x7e, 0xfe,
+ 0x01, 0x81, 0x41, 0xc1, 0x21, 0xa1, 0x61, 0xe1,
+ 0x11, 0x91, 0x51, 0xd1, 0x31, 0xb1, 0x71, 0xf1,
+ 0x09, 0x89, 0x49, 0xc9, 0x29, 0xa9, 0x69, 0xe9,
+ 0x19, 0x99, 0x59, 0xd9, 0x39, 0xb9, 0x79, 0xf9,
+ 0x05, 0x85, 0x45, 0xc5, 0x25, 0xa5, 0x65, 0xe5,
+ 0x15, 0x95, 0x55, 0xd5, 0x35, 0xb5, 0x75, 0xf5,
+ 0x0d, 0x8d, 0x4d, 0xcd, 0x2d, 0xad, 0x6d, 0xed,
+ 0x1d, 0x9d, 0x5d, 0xdd, 0x3d, 0xbd, 0x7d, 0xfd,
+ 0x03, 0x83, 0x43, 0xc3, 0x23, 0xa3, 0x63, 0xe3,
+ 0x13, 0x93, 0x53, 0xd3, 0x33, 0xb3, 0x73, 0xf3,
+ 0x0b, 0x8b, 0x4b, 0xcb, 0x2b, 0xab, 0x6b, 0xeb,
+ 0x1b, 0x9b, 0x5b, 0xdb, 0x3b, 0xbb, 0x7b, 0xfb,
+ 0x07, 0x87, 0x47, 0xc7, 0x27, 0xa7, 0x67, 0xe7,
+ 0x17, 0x97, 0x57, 0xd7, 0x37, 0xb7, 0x77, 0xf7,
+ 0x0f, 0x8f, 0x4f, 0xcf, 0x2f, 0xaf, 0x6f, 0xef,
+ 0x1f, 0x9f, 0x5f, 0xdf, 0x3f, 0xbf, 0x7f, 0xff,
+ };
+
+ /*
+ * NOTE!!!! for historical reasons, krb5_flags are bitreversed
+ * around a 32-bit boundary in the MIT implementation. Hence, bit #0 is
+ * really 0x80000000. There's no good reason for it, but it's too hard
+ * to change things now.....
+ *
+ * People should beware of this before using asn1_decode_krb5_flags to decode
+ * other ASN.1 bit strings, since behavior is hard coded in this function.
+ */
asn1_error_code asn1_decode_krb5_flags(buf, val)
asn1buf * buf;
krb5_flags * val;
{
setup();
! asn1_octet o, pad;
int i;
krb5_flags f=0;
unused_var(taglen);
***************
*** 279,295 ****
if(retval) return retval;
if(class != UNIVERSAL || construction != PRIMITIVE ||
tagnum != ASN1_BITSTRING) return ASN1_BAD_ID;
! if(length != 5) return ASN1_BAD_LENGTH;
! retval = asn1buf_remove_octet(buf,&o); /* # of padding bits */
if(retval) return retval; /* should be 0 */
! if(o != 0) return ASN1_BAD_FORMAT;
! for(i=0; i<4; i++){
retval = asn1buf_remove_octet(buf,&o);
if(retval) return retval;
f = (f<<8) | ((krb5_flags)o&0xFF);
}
*val = f;
return 0;
}
--- 343,366 ----
if(retval) return retval;
if(class != UNIVERSAL || construction != PRIMITIVE ||
tagnum != ASN1_BITSTRING) return ASN1_BAD_ID;
! if (length < 2 || length > 5) return ASN1_BAD_LENGTH;
! retval = asn1buf_remove_octet(buf,&pad); /* # of padding bits */
if(retval) return retval; /* should be 0 */
! if (pad > 7) return ASN1_BAD_FORMAT;
!
! length -= 2; /* -1 for #pad bits, -1 for the last byte */
! for(i=0; i<length; i++){
retval = asn1buf_remove_octet(buf,&o);
if(retval) return retval;
f = (f<<8) | ((krb5_flags)o&0xFF);
}
+ /* handle last byte separately, to mask out the padding bits */
+ retval = asn1buf_remove_octet(buf,&o);
+ if(retval) return retval;
+ f = (f<<8) | ((krb5_flags)o&asn1_pad_mask[pad]);
+
*val = f;
return 0;
}
***************
*** 297,313 ****
asn1_error_code asn1_decode_ticket_flags(buf, val)
asn1buf * buf;
krb5_flags * val;
! { return asn1_decode_krb5_flags(buf,val); }
asn1_error_code asn1_decode_ap_options(buf, val)
asn1buf * buf;
krb5_flags * val;
! { return asn1_decode_krb5_flags(buf,val); }
asn1_error_code asn1_decode_kdc_options(buf, val)
asn1buf * buf;
krb5_flags * val;
! { return asn1_decode_krb5_flags(buf,val); }
asn1_error_code asn1_decode_transited_encoding(buf, val)
asn1buf * buf;
--- 368,467 ----
asn1_error_code asn1_decode_ticket_flags(buf, val)
asn1buf * buf;
krb5_flags * val;
! {
! asn1_error_code retval;
! krb5_flags f;
!
! retval = asn1_decode_krb5_flags(buf, &f);
! if (retval)
! return retval;
!
! #ifdef BACKWARD_BITMASK_COMPAT
! if (asn1_always_reverse || (((f & 0xFFFF0000) == 0) && ((f & 0xFFFF) != 0)))
! #endif
! f = (asn1_swbits[(f & 0xff)] << 24) | (asn1_swbits[(f >> 8) & 0xff] << 16) |
! (asn1_swbits[(f >> 16) & 0xff] << 8) | asn1_swbits[(f >> 24) & 0xff];
!
! *val = f;
! return 0;
! }
asn1_error_code asn1_decode_ap_options(buf, val)
asn1buf * buf;
krb5_flags * val;
! {
! asn1_error_code retval;
! krb5_flags f;
!
! retval = asn1_decode_krb5_flags(buf, &f);
! if (retval)
! return retval;
!
! #ifdef BACKWARD_BITMASK_COMPAT
! if (asn1_always_reverse || (((f & 0xFFFF0000) == 0) && ((f & 0xFFFF) != 0)))
! #endif
! f = (asn1_swbits[(f & 0xff)] << 24) | (asn1_swbits[(f >> 8) & 0xff] << 16) |
! (asn1_swbits[(f >> 16) & 0xff] << 8) | asn1_swbits[(f >> 24) & 0xff];
!
! *val = f;
! return 0;
! }
!
!
! #ifdef BACKWARD_BITMASK_COMPAT
! #define VALID_KDC_FLAGS (KDC_OPT_FORWARDABLE | KDC_OPT_FORWARDED | \
! KDC_OPT_PROXIABLE | KDC_OPT_PROXY | \
! KDC_OPT_ALLOW_POSTDATE | KDC_OPT_POSTDATED | \
! KDC_OPT_RENEWABLE | KDC_OPT_RENEWABLE_OK | \
! KDC_OPT_ENC_TKT_IN_SKEY | KDC_OPT_RENEW | \
! KDC_OPT_VALIDATE)
! #endif
asn1_error_code asn1_decode_kdc_options(buf, val)
asn1buf * buf;
krb5_flags * val;
! {
! asn1_error_code retval;
! krb5_flags f;
! #ifdef BACKWARD_BITMASK_COMPAT
! krb5_flags r;
! #endif
!
! retval = asn1_decode_krb5_flags(buf, &f);
! if (retval)
! return retval;
!
! #ifdef BACKWARD_BITMASK_COMPAT
!
! r = ((asn1_swbits[(f & 0xff)] << 24) |
! (asn1_swbits[(f >> 8) & 0xff] << 16) |
! (asn1_swbits[(f >> 16) & 0xff] << 8) |
! asn1_swbits[(f >> 24) & 0xff]);
!
! if (asn1_always_reverse)
! *val = r;
! else if (((f & ~VALID_KDC_FLAGS) == 0) &&
! ((r & ~VALID_KDC_FLAGS) != 0))
! *val = f;
! else if (((r & ~VALID_KDC_FLAGS) == 0) &&
! ((f & ~VALID_KDC_FLAGS) != 0))
! *val = r;
! else if (f & (KDC_OPT_FORWARDABLE|
! KDC_OPT_FORWARDED|
! KDC_OPT_ENC_TKT_IN_SKEY))
! *val = f;
! else
! *val = r;
! #else
! f = ((asn1_swbits[(f & 0xff)] << 24) |
! (asn1_swbits[(f >> 8) & 0xff] << 16) |
! (asn1_swbits[(f >> 16) & 0xff] << 8) |
! asn1_swbits[(f >> 24) & 0xff]);
!
! *val = f;
! #endif
! return 0;
! }
asn1_error_code asn1_decode_transited_encoding(buf, val)
asn1buf * buf;
Index: asn1_k_encode.c
===================================================================
RCS file: /mit/krb5/.cvsroot/src/lib/krb5/asn.1/asn1_k_encode.c,v
retrieving revision 5.5
retrieving revision 5.6
diff -c -r5.5 -r5.6
*** asn1_k_encode.c 1995/04/14 00:51:42 5.5
--- asn1_k_encode.c 1995/04/19 21:49:56 5.6
***************
*** 235,248 ****
asn1_cleanup();
}
asn1_error_code asn1_encode_krb5_flags(buf, val, retlen)
asn1buf * buf;
const krb5_flags val;
int * retlen;
{
asn1_setup();
! krb5_flags valcopy = val;
int i;
for(i=0; i<4; i++){
retval = asn1buf_insert_octet(buf,(asn1_octet) (valcopy&0xFF));
--- 235,263 ----
asn1_cleanup();
}
+ /*
+ * NOTE!!!! for historical reasons, krb5_flags are bitreversed
+ * around a 32-bit boundary in the MIT implementation. Hence, bit #0 is
+ * really 0x80000000, so we need to do a 32 bit reverse operation before
+ * encoding the bit string.. There's no good reason for it, but it's too
+ * hard to change things now, since the flags are used in other places.
+ *
+ * People should beware of this before using asn1_encode_krb5_flags to encode
+ * other ASN.1 bit strings, since behavior is hard-coded into this function.
+ */
asn1_error_code asn1_encode_krb5_flags(buf, val, retlen)
asn1buf * buf;
const krb5_flags val;
int * retlen;
{
asn1_setup();
! krb5_flags valcopy;
int i;
+
+ valcopy = ((asn1_swbits[(val & 0xff)] << 24) |
+ (asn1_swbits[(val >> 8) & 0xff] << 16) |
+ (asn1_swbits[(val >> 16) & 0xff] << 8) |
+ asn1_swbits[(val >> 24) & 0xff]);
for(i=0; i<4; i++){
retval = asn1buf_insert_octet(buf,(asn1_octet) (valcopy&0xFF));
Index: krbasn1.h
===================================================================
RCS file: /mit/krb5/.cvsroot/src/lib/krb5/asn.1/krbasn1.h,v
retrieving revision 5.6
retrieving revision 5.8
diff -c -r5.6 -r5.8
*** krbasn1.h 1995/03/18 03:08:47 5.6
--- krbasn1.h 1995/04/20 21:57:59 5.8
***************
*** 39,44 ****
--- 39,46 ----
#define ASN1_TAGNUM_CEILING INT_MAX
#define ASN1_TAGNUM_MAX (ASN1_TAGNUM_CEILING-1)
+ extern asn1_octet asn1_swbits[256];
+
/* This is Kerberos Version 5 */
#define KVNO 5
