[OpenAFS-devel] bg-fcrypt

[Derrick J Brashear]

On Mon, 4 Dec 2006, Marcus Watts wrote:

> While tracking down the aix rxkad weirdness, I couldn't
> help but notice two implementations of fcrypt:
> 	src/rxkad/domestic/fcrypt.c
> 	src/rxkad/bg-fcrypt.c
> It appears the latter came from kth, and was put
> briefly into openafs builds between about Oct & December 2002.
> Apparently it was pulled because of unexplained runtime
> problems.  Unfortunately, nobody seems to have documented
> exactly what those runtime problems were.

I suspect the stuff from Zephyr never made it into any ticket.

Garry was looking for me the evening of December 9th and my recollection 
is staying up late after he found me early in the morning of the 10th 
identifying a problem he had as the bg-fcrypt patch, and then reverting 
it. However none of that discussion was logged, and so I lost it.

Anyway,

> I experimented a bit with this, and as best I can tell,
> the "runtime" problem is that there's a hairy ifdef
> around line 553 in bg-fcrypt.c that starts off with:
> 	#if ((1ul << 31) << 1) && defined(ULONG_MAX) && ((ULONG_MAX >> 55) != 0) && ((1ul << 55) != 0)
> 		unsigned long k;
> and if this succeeds it assumes it can store 56 bits into
> a long.  With gcc 3.3.1 on intel i386, this case gets tripped,
> but since longs on that platform are 32-bits, things don't work.

Hm. My zlog suggests we discussed this on November 15, 2002, but I never 
committed anything. Sigh. In particular, gcc warned us:

../afs/bg-fcrypt.c:518: warning: left shift count >= width of type

that line is
   ROT56R64(k, 11);

#define ROT56R64(k, n) { \
   k = (k >> n) | ((k & ((1<<n) - 1)) << (56-n)); }

and some time later I said:

the bg-fcrypt issue is dealt with on linux ( i should check in patches)

and then never did.

> In general, I don't believe it's safe to assume that cpp math
> will work exactly like gcc, especially in regards to word sizes,
> sign expansion, etc.  Fortunately, there's a simplier alternative
> 	#ifdef AFS_64BIT_ENV
> 	    afs_uint64 k;
> which should be safe because this also conditions other 64-bit logic
> inside of afs/stds.h .

Fair that. Open a ticket with a patch and I'll make sure we deal with it 
for 1.4.3 and 1.5.13.

> Apparently bg-fcrypt key schedule loses on i386 because it does lots
> of "ntohl" byte swapping.

Do we care?

Actually, "should" we care?