rryder commented 10 years ago

nwipe 0.16 when built for x86_64 on RHEL 6.5 wipes with repeated patterns of PRNG data alternated with zeroes, instead of all PRNG data as expected. When built for i686 it works as expected with all data being overwritten with PRNG data. This goes back to at least 0.14 I haven't tested earlier versions.

Run as: nwipe --autonuke -m prng --noblank --nowait --nogui /dev/sda /dev/sdb

nwipe x86_64: nwipe-actual x86_64 nwipe i686: nwipe-expected i686

xambroz commented 9 years ago

Issue is the same with 0.17 on Fedora x86_64. Steps to reproduce the issue in test environment: 1) Create image file to wipe with a known pattern of data: dcfldd of=/tmp/test.img pattern=baadf00d bs=1M count=10

2) Add a loop device for the image file so we can nuke it

losetup -d /dev/loop0

losetup /dev/loop0 /tmp/test.img

3) check the pattern before nuking xxd /dev/loop0 hexdump -C /dev/loop

4) nuke it nwipe --autonuke -p mersenne --noblank --nogui --nowait /dev/loop0

5) see the pattern after nuking xxd /dev/loop0 hexdump -C /dev/loop

xambroz commented 9 years ago

I believe the source issue starts with the definition of u32 as unsigned long in nwipe.h. On 64bit platform this is 8 bytes long and not just 4 bytes as expected.

xambroz commented 9 years ago

Proposed patch:

As the size of random value delivered by twister_genrand_int32 is not derived from the size of the 
unsigned long on given platform, but it is always only 0x00000000 - 0xFFFFFFFF we need as special 
function to print it to a byte stream buffer byte by byte.
Issue reported upstream in : https://github.com/martijnvanbrummelen/nwipe/issues/7

diff -ru nwipe-8a9a71822148cb9c82d971030dafd8d374fd1f48/src/prng.c nwipe-8a9a71822148cb9c82d971030dafd8d374fd1f48.new/src/prng.c
--- nwipe-8a9a71822148cb9c82d971030dafd8d374fd1f48/src/prng.c   2014-10-19 21:49:22.000000000 +0200
+++ nwipe-8a9a71822148cb9c82d971030dafd8d374fd1f48.new/src/prng.c   2015-06-22 23:54:38.111837575 +0200
@@ -25,6 +25,7 @@
 #include "mt19937ar-cok.h"
 #include "isaac_rand.h"

+
 nwipe_prng_t nwipe_twister =
 {
    "Mersenne Twister (mt19937ar-cok)",
@@ -40,6 +41,25 @@
 };

+/* Print given number of bytes from unsigned integer number to a byte stream buffer starting with low-endian*/
+int nwipe_u32tobuffer(u8 *buffer, u32 rand, int len)
+{
+   int i;
+   u8 c;   //single char
+   if (len > sizeof(u32))
+   {
+       nwipe_log( NWIPE_LOG_FATAL, "Tried to print longer number than the value passed." );
+       len = sizeof(u32);
+   }
+
+   for (i=0 ; i < len; i++)
+   {
+       c=rand & 0xFFUL;
+       rand = rand >> 8;
+       buffer[i]=c;
+   }
+   return 0;
+}

 int nwipe_twister_init( NWIPE_PRNG_INIT_SIGNATURE )
 {
@@ -54,23 +74,23 @@

 int nwipe_twister_read( NWIPE_PRNG_READ_SIGNATURE )
 {
+   u32 i=0;
    u32 ii;
-   u32 words = count / sizeof( u32 );
-   u32 remain = count % sizeof( u32 );
+   u32 words = count / SIZE_OF_TWISTER ;  // the values of twister_genrand_int32 is strictly 4 bytes
+   u32 remain = count % SIZE_OF_TWISTER ; // the values of twister_genrand_int32 is strictly 4 bytes

-   /* Twister returns 4-bytes per call, so cast the buffer into words. */
+   /* Twister returns 4-bytes per call, so progress by 4 bytes. */
    for( ii = 0; ii < words; ++ii )
    {
-       ((u32*)buffer)[ii] = twister_genrand_int32( (twister_state_t*)*state );
+       nwipe_u32tobuffer((u8*)(buffer+i), twister_genrand_int32( (twister_state_t*)*state ), SIZE_OF_TWISTER) ;
+       i = i + SIZE_OF_TWISTER;
    }

-   /* Fill the buffer tail if the count is not evenly divided by the size of u32. */
-   for( ii = 1; ii <= remain; ++ii )
+   /* If there is some remainder copy only relevant number of bytes to not overflow the buffer. */
+   if ( remain > 0 )
    {
-       /* Notice how three bytes are discarded by doing this. */
-       ((u8*)buffer)[count-ii] = twister_genrand_int32( (twister_state_t*)*state );
+       nwipe_u32tobuffer((u8*)(buffer+i), twister_genrand_int32( (twister_state_t*)*state ), remain) ;
    }
-    
    return 0;
 }

diff -ru nwipe-8a9a71822148cb9c82d971030dafd8d374fd1f48/src/prng.h nwipe-8a9a71822148cb9c82d971030dafd8d374fd1f48.new/src/prng.h
--- nwipe-8a9a71822148cb9c82d971030dafd8d374fd1f48/src/prng.h   2014-10-19 21:49:22.000000000 +0200
+++ nwipe-8a9a71822148cb9c82d971030dafd8d374fd1f48.new/src/prng.h   2015-06-22 23:43:46.578397378 +0200
@@ -51,6 +51,10 @@
 int nwipe_isaac_init( NWIPE_PRNG_INIT_SIGNATURE );
 int nwipe_isaac_read( NWIPE_PRNG_READ_SIGNATURE );

+/* Size of the twister is not derived from the architecture, but it is strictly 4 bytes */
+#define SIZE_OF_TWISTER 4
+
+
 #endif /* PRNG_H_ */

 /* eof */

martijnvanbrummelen commented 9 years ago

Thanks for the patch. I need to setup a test environment and do some testing/reviewing.

xambroz commented 9 years ago

Ping?

martijnvanbrummelen commented 9 years ago

Pong I was busy with other stuff. But I am starting to get some more spare time in a bit. Sorry for the delay.