Skip Menu |
 

History Show all quoted text — Show full headers
# Wed Aug 06 13:09:04 2003 Cesar Garcia <Cesar.Garcia@morganstanley.com> - Ticket created
Date: Wed, 6 Aug 2003 13:08:58 -0400
From: Cesar Garcia <Cesar.Garcia@morganstanley.com>
To: krb5-bugs@mit.edu
Subject: starttime marshalling bug on 64bit platforms in krb524d
Download (untitled) / with headers
text/plain 955B
krb524d uses krb524int_krb_create_ticket(), which when it populates
the K4 ticket starttime in the tkt->dat, is using memcpy to copy 4
from an 8 byte long (8 bytes on 64bit platforms).

The result is a starttime of zero, rather than the actual start time
which is held in the lower order bytes (assuming big endian).

marshalling the 64bit starttime is implemented in krb_create_ticket
(src/lib/krb4/cr_tkt.c), but appears to have been overlooked in
krb524int_krb_create_ticket (src/krb524/cnv_tkt_skey.c).

Attached is a patch to 1.3.1 for your review. I've tested this on:
* 64bit solaris 8 (big endian)
* 32bit solaris 8 (big endian)
* 32bit red hat linux AS 2.1 (little endian)

However the krb_create_ticket code assumes big endian, so this patch
is an adaptation of the changes made to krb_create_ticket. I'm not
aware of any predefined macros that can be used to determine
endianness, but the intent of this patch should be clear.

Regards,
Cesar
Download cnv_tkt_skey.c.patch.2
text/plain 806B
*** cnv_tkt_skey.c.orig Tue Aug 5 19:23:21 2003
--- cnv_tkt_skey.c Wed Aug 6 12:25:14 2003
***************
*** 33,38 ****
--- 33,40 ----
#include <krb.h>
#include "krb524d.h"

+ #define KRB524_LITTLE_ENDIAN (1 != htonl(1))
+
static int krb524d_debug = 0;

static int
***************
*** 359,366 ****
data += 8;
*(data++) = (char) life;
/* issue time */
! memcpy(data, (char *) &time_sec, 4);
! data += 4;
(void) strcpy(data, sname);
data += 1 + strlen(sname);
(void) strcpy(data, sinstance);
--- 361,368 ----
data += 8;
*(data++) = (char) life;
/* issue time */
! KRB4_PUT32(data, time_sec, (KRB524_LITTLE_ENDIAN));
!
(void) strcpy(data, sname);
data += 1 + strlen(sname);
(void) strcpy(data, sinstance);
# Thu Aug 07 18:26:42 2003 tlyu (Taylor Yu) - Correspondence added
To: rt@krbdev.mit.edu
Subject: Re: [krbdev.mit.edu #1714] starttime marshalling bug on 64bit platforms in krb524d
From: Tom Yu <tlyu@mit.edu>
Date: Thu, 07 Aug 2003 18:26:39 -0400
RT-Send-Cc:
Download (untitled) / with headers
text/plain 171B
This is partially due to a failure to duplicate code changes made to
libkrb4... we'll probably end up re-syncing the relevant code, which
should fix this problem.

---Tom
# Thu Aug 07 18:48:17 2003 Cesar Garcia <Cesar.Garcia@morganstanley.com> - Comments added
Date: Thu, 7 Aug 2003 18:48:14 -0400
From: Cesar Garcia <Cesar.Garcia@morganstanley.com>
To: rt-comment@krbdev.mit.edu
Cc: Cesar.Garcia@morganstanley.com, krb5-prs@mit.edu
Subject: Re: [krbdev.mit.edu #1714] starttime marshalling bug on 64bit platforms in krb524d
RT-Send-Cc:
Download (untitled) / with headers
text/plain 528B
Hmm, not totally. Would appear the code in libkrb4 assumes big
endian. (as explained in original mail - code did not work on little
endian platforms - e.g., RedHat AS 2.1 on intel, patch addresses this
by determining big or little endian and handling accordingly).

Show quoted text
>>>>> "Tom" == Tom Yu via <RT" <rt-comment@krbdev.mit.edu>> writes:

Show quoted text
Tom> This is partially due to a failure to duplicate code changes made
Tom> to libkrb4... we'll probably end up re-syncing the relevant code,
Tom> which should fix this problem.

Show quoted text
Tom> ---Tom
# Thu Aug 07 19:04:50 2003 tlyu (Taylor Yu) - Correspondence added
To: rt@krbdev.mit.edu
Subject: Re: [krbdev.mit.edu #1714] starttime marshalling bug on 64bit platforms in krb524d
From: Tom Yu <tlyu@mit.edu>
Date: Thu, 07 Aug 2003 19:04:49 -0400
RT-Send-Cc:
Download (untitled) / with headers
text/plain 580B
Show quoted text
>>>>> "Cesar" == Cesar Garcia via RT <rt-comment@krbdev.mit.edu> writes:

Show quoted text
Cesar> Hmm, not totally. Would appear the code in libkrb4 assumes big
Cesar> endian. (as explained in original mail - code did not work on
Cesar> little endian platforms - e.g., RedHat AS 2.1 on intel, patch
Cesar> addresses this by determining big or little endian and handling
Cesar> accordingly).

Not exactly... in krb5-1.3, libkrb4 has been modified to
unconditionally emit big-endian encodings, regardless of the
endianness of the machine, but will accept either little-endian or
big-endian.

---Tom
# Thu Aug 07 19:55:50 2003 Cesar Garcia <Cesar.Garcia@morganstanley.com> - Comments added
Date: Thu, 7 Aug 2003 19:55:47 -0400
From: Cesar Garcia <Cesar.Garcia@morganstanley.com>
To: rt-comment@krbdev.mit.edu
Cc: Cesar.Garcia@morganstanley.com, krb5-prs@mit.edu
Subject: Re: [krbdev.mit.edu #1714] starttime marshalling bug on 64bit platforms in krb524d
RT-Send-Cc:
Download (untitled) / with headers
text/plain 2.5KiB
Are we looking at the same thing?

===> from lib/krb4/cr_tkt.c:

memcpy(data, session, 8);
data += 8;
*data++ = life;
/* issue time */
KRB4_PUT32BE(data, time_sec);

memcpy(data, sname, snamelen);
data += snamelen;
memcpy(data, sinstance, sinstlen);
data += sinstlen;

====> KRB4_PUT32BE is the relevent macro, from include/kerberosIV/prot.h:

#define KRB4_PUT32BE(p, val) \
do { \
(p)[0] = ((KRB_UINT32)(val) >> 24) & 0xff; \
(p)[1] = ((KRB_UINT32)(val) >> 16) & 0xff; \
(p)[2] = ((KRB_UINT32)(val) >> 8) & 0xff; \
(p)[3] = (KRB_UINT32)(val) & 0xff; \
(p) += 4; \
} while (0)

This is equivalent to casting an 8byte or 4byte quantities (val) to a 4
byte quantity and memcpy'ing the 4 bytes.

If val is big endian, it writes the (casted) 4 bytes as would a
memcpy of 4 bytes, resulting in network byte order (big endian).

If val is little endian, this again, copies bytes as would a
mempcy, resulting in little endian.

With 1.3.1 krb524d running on linux - with code borrowed from libkrb4
as shown above, start time is:

1060266366 (decimal), or

7E61323F (hex, in host byte order, which on my linux box is little
endian).

Would be 3F32617E (hex, if converted to big endian)

These corresponds to: Thu Aug 7 10:26:06 EDT 2003

However, when 7E61323F is interpreted as big endian (212029907 in
decimal), by AFS server, it corresponds to Tue Mar 10 06:57:51 EDT
2037.

AFS error message:

afs: Tokens for user of AFS id 4843 for cell q.ny.ms.com are discarded (rxkad error=19270407)

(don't have code off hand to convert this error code to string).

In any case, this code did not run correctly on linux. When I applied
patch that was submitted, starttime is correctly issued. Without
patch, starttime was postdated and tokens discarded by AFS fileserver.


Show quoted text
>>>>> "Tom" == Tom Yu via <RT" <rt-comment@krbdev.mit.edu>> writes:

Show quoted text
>>>>> "Cesar" == Cesar Garcia via RT <rt-comment@krbdev.mit.edu> writes:
Show quoted text
Cesar> Hmm, not totally. Would appear the code in libkrb4 assumes big
Cesar> endian. (as explained in original mail - code did not work on
Cesar> little endian platforms - e.g., RedHat AS 2.1 on intel, patch
Cesar> addresses this by determining big or little endian and handling
Cesar> accordingly).

Show quoted text
Tom> Not exactly... in krb5-1.3, libkrb4 has been modified to
Tom> unconditionally emit big-endian encodings, regardless of the
Tom> endianness of the machine, but will accept either little-endian
Tom> or big-endian.

Show quoted text
Tom> ---Tom
# Thu Aug 07 22:36:02 2003 tlyu (Taylor Yu) - Correspondence added
To: rt@krbdev.mit.edu
Subject: Re: [krbdev.mit.edu #1714] starttime marshalling bug on 64bit platforms in krb524d
From: Tom Yu <tlyu@mit.edu>
Date: Thu, 07 Aug 2003 22:36:00 -0400
RT-Send-Cc:
Download (untitled) / with headers
text/plain 2.6KiB
Show quoted text
>>>>> "Cesar" == Cesar Garcia via RT <rt-comment@krbdev.mit.edu> writes:

Show quoted text
Cesar> Are we looking at the same thing?

Yes...

Show quoted text
Cesar> ====> KRB4_PUT32BE is the relevent macro, from include/kerberosIV/prot.h:

Show quoted text
Cesar> #define KRB4_PUT32BE(p, val) \
Cesar> do { \
Cesar> (p)[0] = ((KRB_UINT32)(val) >> 24) & 0xff; \
Cesar> (p)[1] = ((KRB_UINT32)(val) >> 16) & 0xff; \
Cesar> (p)[2] = ((KRB_UINT32)(val) >> 8) & 0xff; \
Cesar> (p)[3] = (KRB_UINT32)(val) & 0xff; \
Cesar> (p) += 4; \
Cesar> } while (0)

Show quoted text
Cesar> This is equivalent to casting an 8byte or 4byte quantities (val) to a 4
Cesar> byte quantity and memcpy'ing the 4 bytes.

It is not. It is equivalent to truncating a 64-bit or 32-bit quantity
(or whatever) to 32 bits and unconditionally encoding as big-endian.
The cast operates on the value, not on its representation. The
sequence of shifts always produces a big-endian encoding, hence the
name of the macro.

If there were a cast of a _pointer_ to a 64-bit integer to a pointer
to a 32-bit integer, you would have gotten all zeroes on a big-endian
machine when shifting out the value.

Show quoted text
Cesar> If val is big endian, it writes the (casted) 4 bytes as would a
Cesar> memcpy of 4 bytes, resulting in network byte order (big endian).

Show quoted text
Cesar> If val is little endian, this again, copies bytes as would a
Cesar> mempcy, resulting in little endian.

The endianness of val is irrelevant here, since we're not accessing it
with a char pointer, or doing any other sort of pointer casting.

Show quoted text
Cesar> With 1.3.1 krb524d running on linux - with code borrowed from libkrb4
Cesar> as shown above, start time is:

Show quoted text
Cesar> 1060266366 (decimal), or

Show quoted text
Cesar> 7E61323F (hex, in host byte order, which on my linux box is little
Cesar> endian).

Ah. Assuming you copied only the call to the KRB4_PUT32BE macro, and
not the other parts of the cr_tkt, you may have wound up with an
inconsistent packet. There is a flag at the beginning of the packet
that indicates what byte order it has. If you did not adjust the
setting of this flag to match the byte order used to encode the
timestamp, the receiving server would decide incorrectly whether to
byte-swap.

Show quoted text
Cesar> Would be 3F32617E (hex, if converted to big endian)

Show quoted text
Cesar> These corresponds to: Thu Aug 7 10:26:06 EDT 2003

Show quoted text
Cesar> However, when 7E61323F is interpreted as big endian (212029907 in
Cesar> decimal), by AFS server, it corresponds to Tue Mar 10 06:57:51 EDT
Cesar> 2037.

No, it would have gotten 0x3F32617E in big-endian, but incorrectly
byte-swapped it because the setting of the endianness flag would
correspond to little-endian.

---Tom
# Fri Aug 08 07:41:04 2003 Cesar Garcia <Cesar.Garcia@morganstanley.com> - Comments added
Date: Fri, 8 Aug 2003 07:41:01 -0400
From: Cesar Garcia <Cesar.Garcia@morganstanley.com>
To: rt-comment@krbdev.mit.edu
Cc: Cesar.Garcia@morganstanley.com, krb5-prs@mit.edu
Subject: Re: [krbdev.mit.edu #1714] starttime marshalling bug on 64bit platforms in krb524d
RT-Send-Cc:
Download (untitled) / with headers
text/plain 473B
Show quoted text
Tom> Ah. Assuming you copied only the call to the KRB4_PUT32BE macro,
Tom> and not the other parts of the cr_tkt, you may have wound up with
Tom> an inconsistent packet. There is a flag at the beginning of the
Tom> packet that indicates what byte order it has. If you did not
Tom> adjust the setting of this flag to match the byte order used to
Tom> encode the timestamp, the receiving server would decide
Tom> incorrectly whether to byte-swap.

That's it. Many thanks.
# Mon Aug 11 22:19:01 2003 tlyu (Taylor Yu) - Tags pullup added
# Mon Aug 11 22:19:01 2003 tlyu (Taylor Yu) - Status changed from 'new' to 'resolved'
# Mon Aug 11 22:19:02 2003 tlyu (Taylor Yu) - Version_reported 1.3.1 added
# Mon Aug 11 22:19:02 2003 tlyu (Taylor Yu) - Target_Version 1.3.2 added
# Mon Aug 11 22:19:03 2003 tlyu (Taylor Yu) - Component krb5-kdc added
# Mon Aug 11 22:19:03 2003 tlyu (Taylor Yu) - Given to tlyu (Taylor Yu)
# Mon Aug 11 22:19:03 2003 tlyu (Taylor Yu) - Correspondence added
From: tlyu@mit.edu
Subject: CVS Commit
Download (untitled) / with headers
text/plain 415B
* cnv_tkt_skey.c (krb524_convert_tkt_skey): Call krb_create_ticket
instead of local version. Remove local version of
krb_create_ticket. This file no longer gets included into a
krb524 library, so accessing internal functions isn't that big of
an issue anymore.


To generate a diff of this commit:



cvs diff -r1.132 -r1.133 krb5/src/krb524/ChangeLog
cvs diff -r1.30 -r1.31 krb5/src/krb524/cnv_tkt_skey.c
# Thu Aug 14 10:39:57 2003 Cesar Garcia <Cesar.Garcia@morganstanley.com> - Comments added
Date: Thu, 14 Aug 2003 10:39:19 -0400
From: Cesar Garcia <Cesar.Garcia@morganstanley.com>
To: rt-comment@krbdev.mit.edu
Cc: Cesar.Garcia@morganstanley.com, krb5-prs@mit.edu
Subject: Re: [krbdev.mit.edu #1714] CVS Commit
RT-Send-Cc:
Download (untitled) / with headers
text/plain 654B
Thanks.

How would one go about getting CVS access (I was under the impression
there was no public CVS access).

Thanks again.

Show quoted text
>>>>> "Tom" == Tom Yu via <RT" <rt-comment@krbdev.mit.edu>> writes:

Show quoted text
Tom> * cnv_tkt_skey.c (krb524_convert_tkt_skey): Call krb_create_ticket
Tom> instead of local version. Remove local version of
Tom> krb_create_ticket. This file no longer gets included into a
Tom> krb524 library, so accessing internal functions isn't that big of
Tom> an issue anymore.


Show quoted text
Tom> To generate a diff of this commit:



Show quoted text
Tom> cvs diff -r1.132 -r1.133 krb5/src/krb524/ChangeLog
Tom> cvs diff -r1.30 -r1.31 krb5/src/krb524/cnv_tkt_skey.c
# Tue Aug 19 19:44:41 2003 tlyu (Taylor Yu) - Version_Fixed 1.3.2 added
# Tue Aug 19 19:44:41 2003 tlyu (Taylor Yu) - Correspondence added
From: tlyu@mit.edu
Subject: CVS Commit
Download (untitled) / with headers
text/plain 181B
pullup from trunk


To generate a diff of this commit:



cvs diff -r1.122.2.7 -r1.122.2.8 krb5/src/krb524/ChangeLog
cvs diff -r1.28.2.2 -r1.28.2.3 krb5/src/krb524/cnv_tkt_skey.c
# Tue Aug 19 19:47:18 2003 tlyu (Taylor Yu) - Correspondence added
To: Cesar Garcia <Cesar.Garcia@morganstanley.com>
Cc: rt@krbdev.mit.edu
Subject: Re: [krbdev.mit.edu #1714] CVS Commit
From: Tom Yu <tlyu@mit.edu>
Date: Tue, 19 Aug 2003 19:47:16 -0400
RT-Send-Cc:
Download (untitled) / with headers
text/plain 415B
Show quoted text
>>>>> "Cesar" == Cesar Garcia <Cesar.Garcia@morganstanley.com> writes:

Show quoted text
Cesar> Thanks.
Cesar> How would one go about getting CVS access (I was under the impression
Cesar> there was no public CVS access).

We do not offer direct CVS access to the public, but you can get daily
snapshots of the latest source from our CVS repository. Please see
the description of the krb5-current snapshots on our web page.

---Tom
# Wed Dec 16 18:02:41 2015 tlyu (Taylor Yu) - CustomField pullup deleted