eworm-de / mkinitcpio-ykfde

Full disk encryption with Yubikey (Yubico key)
GNU General Public License v3.0
109 stars 26 forks source link

Insecure random generator used #27

Closed HacKanCuBa closed 5 years ago

HacKanCuBa commented 5 years ago

Generating random data from system time is a bad idea. Implement getrandom() instead.

https://github.com/eworm-de/mkinitcpio-ykfde/blob/46c8a3e214ee092b99a30fab8ed91b6f033287a9/bin/ykfde.c#L202

eworm-de commented 5 years ago

I think glibc did not have getrandom() when I started this. However I agree that seeding from system time is a bad idea. ;)

Wondering whether or not we should call getrandom() with GRND_RANDOM... If we do we have to make sure we get enough bytes and/or do not block.

eworm-de commented 5 years ago

Just pushed c33a043cb6a9ff5431d050a095f7900d9303a6e1, which uses getrandom() to seed.

I thought about filling the challenge with getrandom(), but I would like to keep printable characters in the challenge.

So are you happy with this?

HacKanCuBa commented 5 years ago

Hi! thanks for looking at this. Sorry about the quite cold writing at the top, I was tired then.

I was thinking about something along the lines of removing rand() completely. Getting "randomness" from /dev/urandom is more than enough and cryptographically secure.

If the urandom source has been initialized, reads of up to 256 bytes will always return as many bytes as requested and will not be interrupted by signals. If the urandom source has not yet been initialized, then getrandom() will block.

So this means that we are good to go to generate ints. You don't need to pass any flag to getrandom, use 0.

Sorry that my C is very rusted (about 15years rusted), so I can't write the code. For C++11, it's as easy as: https://en.cppreference.com/w/cpp/numeric/random/uniform_int_distribution

For C, I couldn't find an example with getrandom so I guess I have to take your solution, but I would use 0 as flag and sizeof(unsigned int) instead of hardcoded 4. Maybe you can get the idea with my python code (based on other's) to generate random ints from a random buffer: randbelow() which uses randint().

Cheers!

eworm-de commented 5 years ago

I have not been happy either. ;) Have a look at 25d3aaf2a9e5ee9fc461fd49b8a00af4e50096da, that should be fine for anybody.

eworm-de commented 5 years ago

Feel free to comment on closed issue... Are you happy with this change?

HacKanCuBa commented 5 years ago

Yeah, much better :) Thanks for the effort!