[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: arcfour
From: |
Ralf Wildenhues |
Subject: |
Re: arcfour |
Date: |
Fri, 14 Oct 2005 09:10:24 +0200 |
User-agent: |
Mutt/1.5.11 |
Hi Simon,
* Simon Josefsson wrote on Fri, Oct 14, 2005 at 01:12:59AM CEST:
> How about this?
Some style comments, if I may..
> Index: lib/arcfour.c
> ===================================================================
> RCS file: lib/arcfour.c
> diff -N lib/arcfour.c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ lib/arcfour.c 13 Oct 2005 23:12:01 -0000
*snip*
> +
> +#ifdef HAVE_CONFIG_H
> +# include <config.h>
> +#endif
> +
> +#include "arcfour.h"
> +
> +void
> +arcfour_stream (arcfour_context * context, const char *inbuf, char *outbuf,
> + size_t length)
> +{
> + register size_t i = context->idx_i;
> + register size_t j = context->idx_j;
> + register unsigned char *sbox = context->sbox;
> + register unsigned char t;
> +
> + while (length--)
> + {
> + i++;
> + i = i & 255; /* The and-op seems to be faster than the mod-op. */
Yes (for less-than-commonly-optimizing compilers), but you could also be
consistent in notation here, and two lines down the road.
Furthermore, if I may suggest not to sprinkle hard-coded numbers all
over the place. How about something like this
#define ARCFOUR_BLOCKBITS 8
#define ARCFOUR_BLOCKSIZE (1 << ARCFOUR_BLOCKBITS)
#define ARCFOUR_BLOCKMASK (ARCFOUR_BLOCKSIZE - 1)
in the header, and then use the latter two consistently?
If you don't like long names, maybe s/BLOCK//g.
> + j += sbox[i];
> + j &= 255;
> + t = sbox[i];
> + sbox[i] = sbox[j];
> + sbox[j] = t;
> + *outbuf++ = *inbuf++ ^ sbox[(sbox[i] + sbox[j]) & 255];
> + }
> +
> + context->idx_i = i;
> + context->idx_j = j;
> +}
> +
> +void
> +arcfour_setkey (arcfour_context * context, const char *key, size_t keylen)
> +{
> + int i, j;
> + unsigned char karr[256];
> +
> + context->idx_i = context->idx_j = 0;
> + for (i = 0; i < 256; i++)
> + context->sbox[i] = i;
> + for (i = 0; i < 256; i++)
> + karr[i] = key[i % keylen];
If this were time-critical, you could hand-maintain another index k and
omit the modulo. I also wonder whether merging the karr loop into the
next one (and thus eliding karr completely) isn't beneficial. Like
this (untested):
for (i = j = k = 0; i < ARCFOUR_BLOCKSIZE; i++)
{
int t;
j = (j + context->sbox[i] + key[k]) & ARCFOUR_BLOCKMASK;
t = context->sbox[i];
context->sbox[i] = context->sbox[j];
context->sbox[j] = t;
if (++k == keylen)
k = 0;
}
}
Furthermore, I guess it would be beneficial if you would test this and
the other modules you recently added with test strings that are longer
than respective block sizes. :)
> + for (i = j = 0; i < 256; i++)
> + {
> + int t;
> + j = (j + context->sbox[i] + karr[i]) % 256;
> + t = context->sbox[i];
> + context->sbox[i] = context->sbox[j];
> + context->sbox[j] = t;
> + }
> +}
*snip*
Cheers,
Ralf
- arcfour, Simon Josefsson, 2005/10/13
- Re: arcfour,
Ralf Wildenhues <=
- Re: arcfour, Simon Josefsson, 2005/10/14
- Re: arcfour, Simon Josefsson, 2005/10/14
- Re: arcfour, Stepan Kasal, 2005/10/14
- Re: arcfour, Simon Josefsson, 2005/10/14
- Re: arcfour, Paul Eggert, 2005/10/14
- Re: arcfour, Bruno Haible, 2005/10/14