Re: pseudo-random key generator, not bad says i

From: Tom St Denis (tom_at_securescience.net)
Date: 04/29/04 

Date: Thu, 29 Apr 2004 01:13:23 GMT

FRMook wrote:
>>It's also a coding nightmare.
>
>
> what can i say, it gets the job done.

What job is that? You haven't even said what problem this solves!!!

>>These shouldn't be global. They should be part of the "class" prng.
>
>
> i like to use struct instead of class when i have no private parts.
> is using class just a matter of writing clean code or does it have
> some importance?

Makes the code easier to read. Normally you don't put functions in
structures [well function pointers is ok]. Just because the compiler
will handle it doesn't make it good code.

>>> inline unsigned int go( unsigned int );
>>
>>Why is this marked inline?
>
>
> that's inline because it gets called a whole lot and putting things
> 'in line' makes'em go faster.

Let the compiler worry about that. In particular GCC will automatically
inline things that are appropriate.

>>>void main( void )
>>
>>main returns int, even in C++.
>
>
> nothing's calling this, so i didn't think it mattered that i return a
> value.

The startup library calls it and returns the value to the OS.

>>> cerr << "enter a seed and i'll generate 128 pseudo-random bytes\n";
>>> cout << "seed: "; cin.getline( seed, 500 );
>>
>>I hate C++ streams... but MHO aside... the second cout needs a flush.
>
>
> because of the cin right after that, it flushes automatically.

Well maybe in C++. In C printf won't. Usually something to be aware of.

>>> random.go( 3 );
>>
>>random.go() returns a value that you ignore. Either you're design is
>>flawed [e.g. should have an interface that returns void] or you are
>>calling it wrong.
>
>
> it doesn't return a value because go() works on a member of the prng
> struct, it calls it from within and later the result is output.
> well... in the next exerpt.

go() is defined as returning unsigned int.

>>> for( unsigned int i = 0; i < pwlen; i++ )
>>> cerr << hex << random.key[i];
>>
>>Technically this is not an error. It should goto cout not cerr. And
>>nitpick but "++i" is more traditional at the far right.
>
>
> i use x++ because i go from 0 to arraysize+1, if i do ++x it'll
> overflow; i think.

Nope. You're wrong.

>>>inline unsigned int prng::go( unsigned int level )
>>>{
>>> static unsigned int chr = 0;
>>> srand( clock() );
>>
>>srand() and clock() are not portable in the sense they will give the
>>same values on different machines. Furthermore clock() is often defined
>>as the time from program start. Therefore clock() will most likely
>>return zero or something close to it [e.g. no entropy].
>
>
> yeah, i know. maybe i should use time_t or something instead...

Or scrap the design...

>>> if( level )
>>
>>It's a good idea to use {} around if and for statements to avoid
>>confusing program flow. Good indentation though.
>>
>>
>>> for( unsigned int i = 0; i < pwlen; i++ )
>>> key[i] = go( level - 1 );
>>> else
>>> for( unsigned int i = 0; i < pwlen; i++ )
>>> key[i] = (rand()%256) ^ seed[i%len];
>>
>>This program allows len==0 to happen. Not only that but if pwlen<len
>>then you won't use all of the input entropy.
>
>
> thanks.
> i always use values that go from 0 to arraysize-1 so it's ok for len
> to equal 0.

i%0 is a division by zero. So no it's not ok.

> and yeah, i thought of the pwlen<len thing. i thought i might just
> lower the seed[] size from 500 bytes to 128.

Or... again think of the design carefully.

> yeah, i know that's pretty funky. and i did mess up on that. i meant
> to make a local k[128] so the key[] wouldn't be overwritten like that.
> but since the recursion is 3 layers deep and each layer only returns
> one byte that's xored against all the other bytes, and each array size
> is 128 bytes the clock would have to hit the same mark more than 2
> million times in the same way from the beginning of the program. i
> know that's still got some problems though.

The design is just bad. I'd scrap it alltogether.

>>I'd say given the platform and start time it would be very easy to find
>>the seed in the case pwlen==len. So overall, I'd say this algorithm is
>>weak, poorly implemented and not designed with any particular goal in mind.
>
>
> i got the idea and wrote the program all starting about twenty minutes
> before posting, so i did jump the gun on sticking this thing out. but
> i have written a program that has a purpose... it's a stream cipher
> that has nothing at all to do with this program. and you seem to be
> good at picking things apart, so i'll stick it up here if you can give
> me some suggestions.

No, no, no, no, definitely no.

You're clearly not qualified to design a stream cipher "for use" right
now so don't bother trying.

First and foremost take a problem and craft a solution. What problem
does this solve? Faster more secure stream ciphers already exist. So
proposing a slower less secure cipher doesn't make sense.

Second, read up on cipher design and cryptanalysis. There are tons of
papers on the subjects in the various crypto journals. Spend time
reading them.

Third, if all you're gonna do is reply quickly with anothed crapped out
design I'm [or others] are just going to pick it apart again and again.
    I won't tell you how to make it more secure so you're not going to
learn much and just make an ass of yourself.

So here's my advice in a nutshell. Scrap this design, stop trying to
make it work. Go read up [say via citeseer] on stream cipher
cryptanalysis [e.g. various linear, consistency style attacks, etc].
Then if you still want to come back to it go for it.

Tom

Relevant Pages

  • Re: Privilege-escalation attacks on NT-based Windows are unfixable
    ... Who on earth thinks that the appropriate way to allow a secure service ... of the privileged process to require a particular user interface. ... > Why is this not a serious flaw in the design of the application? ... It can in some cases be a privilege elevation attack, ...
    (comp.security.misc)
  • Re: Privilege-escalation attacks on NT-based Windows are unfixable
    ... Who on earth thinks that the appropriate way to allow a secure service ... of the privileged process to require a particular user interface. ... > Why is this not a serious flaw in the design of the application? ... It can in some cases be a privilege elevation attack, ...
    (comp.os.ms-windows.nt.admin.security)
  • Re: etc/passwd file
    ... it would still be secure. ... >effective form of security. ... the password database was known to all, ... All I can say is if that were the design criteria then they failed miserably. ...
    (comp.security.unix)
  • Re: An observation...anyone agree?
    ... meet the design critera, at a resonable material or machining cost. ... OEE is the composite measure of three things, 1) equipment availablility, ... Productivity: The number of parts that would be theoretically possible ... Scrap: ...
    (alt.machines.cnc)
  • Re: etc/passwd file
    ... it would still be secure. ... ]>effective form of security. ... ]the password database was known to all, ... ]All I can say is if that were the design criteria then they failed miserably. ...
    (comp.security.unix)
Quantcast