[OpenAFS-devel] bg-fcrypt

Marcus Watts mdw@umich.edu
Tue, 05 Dec 2006 15:51:08 -0500


Derrick J Brashear <shadow@dementia.org> (& others) wrote:
...
> 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)); }

Yup - and other two compilers complained about stuff as well.
I was ignoring this because it was just a warning, but I'll see
if I can make it all compile warning-free on all 3 architectures.
I have a bunch of other related fixes that clean up ivec & 
key schedule data types for cleaner compilation with safer alignment
types, fewer casts, warnings, & other weirdness.  This is mostly
fcrypt, for instance,
	old:
		typedef afs_int32 fc_KeySchedule[MAXROUNDS];
	new:
		typedef struct { afs_int32 d[MAXROUNDS]; } fc_KeySchedule;
but also one change for des (here's the big alignment gotcha):
	old:
		typedef struct des_ks_struct { des_cblock _; } des_key_schedule[16];
	new:
		typedef struct des_ks_struct { int _[8/sizeof(int)]; }
			des_key_schedule[16];
(des_cblock only forces char alignment...)
(ideally I'd like to make des_key_schedule not be an array typedef
because those are easily misunderstood and also slightly broken, but
des has a much better known broken api...)

...

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

Do you still have those patches?

> > 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.

Ok.  I'll try to pull this out as a separate stand-alone patch.
Let me know if you are specifically not interested in the
typedef cleanup I described above.

> 
> > Apparently bg-fcrypt key schedule loses on i386 because it does lots
> > of "ntohl" byte swapping.
> 
> Do we care?
> 
> Actually, "should" we care?

Nope.  As  Bjorn Gronvall points out, the win on encryption more
than pays for the lose on key scheduling.  I mainly mentioned it
because the lose is only on i386; big-endian machines are faster
even for the key schedule.

				-Marcus