Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement High-Quality Random Number Generation Using AES-CTR Mode with OpenSSL and AES-NI Support. C++ variant #570

Closed
wants to merge 12 commits into from

Conversation

Knogle
Copy link
Contributor

@Knogle Knogle commented Apr 5, 2024

Ahoy. Same as #559 but with major rewrite in C++ using smart pointers to ensure memory safety.

Fabian Druschke and others added 9 commits March 23, 2024 18:48
… in experimental state. Fixed formatting, fixed AES PRNG header.
…256 as default option for 32-Bit due to performance and quality reasons.
…HER_CTX within the AES CTR PRNG C++ implementation. This ensures automatic resource release, preventing memory leaks and enhancing code safety.
@PartialVolume
Copy link
Collaborator

This branch is currently being tested, I'm pretty busy at the moment so it may be a few days before I give you an update.

@Knogle
Copy link
Contributor Author

Knogle commented Apr 8, 2024

This branch is currently being tested, I'm pretty busy at the moment so it may be a few days before I give you an update.

An issue i've found, that should be fixed in this branch now, causing buffer overflow.
I've used AES-CTR-256, which was writing into a 128-bit buffer.
Now i've altered the buffer to 256-bit.
Should effectively double the speed.

Screenshot from 2024-04-09 01-56-42

@PartialVolume
Copy link
Collaborator

Verification still fails, valgrind error still present but points to the new aes_ctr_prng_genrand_uint256_to_buf() function.

==1439456== Thread 4:
==1439456== Conditional jump or move depends on uninitialised value(s)
==1439456==    at 0x11EDDD: nwipe_random_pass (pass.c:330)
==1439456==    by 0x1279A7: nwipe_runmethod (method.c:934)
==1439456==    by 0x12734D: nwipe_random (method.c:742)
==1439456==    by 0x52AF133: start_thread (pthread_create.c:442)
==1439456==    by 0x532EA3F: clone (clone.S:100)
==1439456==  Uninitialised value was created by a stack allocation
==1439456==    at 0x124110: aes_ctr_prng_genrand_uint256_to_buf (aes_ctr_prng.cpp:126)
==1439456== 
==1439456== Syscall param write(buf) points to uninitialised byte(s)
==1439456==    at 0x531E27F: __libc_write (write.c:26)
==1439456==    by 0x531E27F: write (write.c:24)
==1439456==    by 0x11EE81: nwipe_random_pass (pass.c:349)
==1439456==    by 
```0x1279A7: nwipe_runmethod (method.c:934)
==1439456==    by 0x12734D: nwipe_random (method.c:742)
==1439456==    by 0x52AF133: start_thread (pthread_create.c:442)
==1439456==    by 0x532EA3F: clone (clone.S:100)
==1439456==  Address 0x6c57bb0 is 0 bytes inside a block of size 4,096 alloc'd
==1439456==    at 0x48455EF: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==1439456==    by 0x11EBC2: nwipe_random_pass (pass.c:268)
==1439456==    by 0x1279A7: nwipe_runmethod (method.c:934)
==1439456==    by 0x12734D: nwipe_random (method.c:742)
==1439456==    by 0x52AF133: start_thread (pthread_create.c:442)
==1439456==    by 0x532EA3F: clone (clone.S:100)
==1439456==  Uninitialised value was created by a stack allocation
==1439456==    at 0x124110: aes_ctr_prng_genrand_uint256_to_buf (aes_ctr_prng.cpp:126)

@Knogle
Copy link
Contributor Author

Knogle commented Apr 9, 2024

Now i've compiled and ran on my Ubuntu server, the result there is entirely different heh!

As we have pointed out before, Fedora uses some different linker, so it was able to build despite Ubuntu being unable to build a few weeks ago as you remember.
I think it has something to do with it.

Screenshot from 2024-04-09 11-03-07

Ok now it's the first time i'm encountering those issues as well on Fedora.

Screenshot from 2024-04-09 11-12-38

==13028== Thread 4:
==13028== Conditional jump or move depends on uninitialised value(s)
==13028==    at 0x4118F1: nwipe_random_pass (pass.c:330)
==13028==    by 0x41604C: nwipe_runmethod (method.c:934)
==13028==    by 0x417179: nwipe_random (method.c:742)
==13028==    by 0x52A1896: start_thread (in /usr/lib64/libc.so.6)
==13028==    by 0x53288C3: clone (in /usr/lib64/libc.so.6)
==13028==  Uninitialised value was created by a stack allocation
==13028==    at 0x414980: aes_ctr_prng_genrand_uint256_to_buf (aes_ctr_prng.cpp:126)
==13028== 
==13028== Syscall param write(buf) points to uninitialised byte(s)
==13028==    at 0x531BF1D: write (in /usr/lib64/libc.so.6)
==13028==    by 0x4117D0: nwipe_random_pass (pass.c:349)
==13028==    by 0x41604C: nwipe_runmethod (method.c:934)
==13028==    by 0x417179: nwipe_random (method.c:742)
==13028==    by 0x52A1896: start_thread (in /usr/lib64/libc.so.6)
==13028==    by 0x53288C3: clone (in /usr/lib64/libc.so.6)
==13028==  Address 0x64452f0 is 0 bytes inside a block of size 4,096 alloc'd
==13028==    at 0x4849E60: calloc (vg_replace_malloc.c:1595)
==13028==    by 0x41172E: nwipe_random_pass (pass.c:268)
==13028==    by 0x41604C: nwipe_runmethod (method.c:934)
==13028==    by 0x417179: nwipe_random (method.c:742)
==13028==    by 0x52A1896: start_thread (in /usr/lib64/libc.so.6)
==13028==    by 0x53288C3: clone (in /usr/lib64/libc.so.6)
==13028==  Uninitialised value was created by a stack allocation
==13028==    at 0x414980: aes_ctr_prng_genrand_uint256_to_buf (aes_ctr_prng.cpp:126)
==13028== 
==13028== Thread 8:
==13028== Conditional jump or move depends on uninitialised value(s)
==13028==    at 0x484E98E: bcmp (vg_replace_strmem.c:1229)
==13028==    by 0x4114BB: nwipe_random_verify (pass.c:198)
==13028==    by 0x416206: nwipe_runmethod (method.c:961)
==13028==    by 0x417179: nwipe_random (method.c:742)
==13028==    by 0x52A1896: start_thread (in /usr/lib64/libc.so.6)
==13028==    by 0x53288C3: clone (in /usr/lib64/libc.so.6)
==13028==  Uninitialised value was created by a stack allocation
==13028==    at 0x414980: aes_ctr_prng_genrand_uint256_to_buf (aes_ctr_prng.cpp:126)
==13028== 
==13028== Conditional jump or move depends on uninitialised value(s)
==13028==    at 0x484E965: bcmp (vg_replace_strmem.c:1229)
==13028==    by 0x4114BB: nwipe_random_verify (pass.c:198)
==13028==    by 0x416206: nwipe_runmethod (method.c:961)
==13028==    by 0x417179: nwipe_random (method.c:742)
==13028==    by 0x52A1896: start_thread (in /usr/lib64/libc.so.6)
==13028==    by 0x53288C3: clone (in /usr/lib64/libc.so.6)
==13028==  Uninitialised value was created by a stack allocation
==13028==    at 0x414980: aes_ctr_prng_genrand_uint256_to_buf (aes_ctr_prng.cpp:126)
==13028== 
==13028== Conditional jump or move depends on uninitialised value(s)
==13028==    at 0x4114BE: nwipe_random_verify (pass.c:198)
==13028==    by 0x416206: nwipe_runmethod (method.c:961)
==13028==    by 0x417179: nwipe_random (method.c:742)
==13028==    by 0x52A1896: start_thread (in /usr/lib64/libc.so.6)
==13028==    by 0x53288C3: clone (in /usr/lib64/libc.so.6)
==13028==  Uninitialised value was created by a stack allocation
==13028==    at 0x414980: aes_ctr_prng_genrand_uint256_to_buf (aes_ctr_prng.cpp:126)

@Knogle
Copy link
Contributor Author

Knogle commented Apr 9, 2024

I've been able to get a clean valgrind output now after fixing the uninitialized array.
Please test again if possible :)

Old code was:

unsigned char temp_buffer[32];  // Temporary buffer for pseudorandom output.
int outlen;

if( !EVP_EncryptUpdate( cppState->ctx.get(), temp_buffer, &outlen, temp_buffer, sizeof( temp_buffer ) ) )
{
    nwipe_log( NWIPE_LOG_ERROR, "AES-256 CTR PRNG generation failed." );
    return;
}

There are two ways to solve it.

if( !EVP_EncryptUpdate( cppState->ctx.get(), temp_buffer, &outlen, nullptr, sizeof( temp_buffer ) ) )
{
    nwipe_log( NWIPE_LOG_ERROR, "AES-256 CTR PRNG generation failed." );
    return;
}

I've opted for:

std::memset(temp_buffer, 0, sizeof(temp_buffer));

==14685== Memcheck, a memory error detector
==14685== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==14685== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info
==14685== Command: src/nwipe /dev/loop0 /dev/loop1 /dev/loop2 /dev/loop3 /dev/loop4 /dev/loop5 /dev/loop6 /dev/loop7 /dev/loop8 /dev/loop9 /dev/loop10 /dev/loop11 /dev/loop12 /dev/loop13 /dev/loop14 /dev/loop15
==14685== Parent PID: 14684
==14685== 
==14685== 
==14685== HEAP SUMMARY:
==14685==     in use at exit: 443,703 bytes in 792 blocks
==14685==   total heap usage: 27,268 allocs, 26,476 frees, 24,552,484 bytes allocated
==14685== 
==14685== LEAK SUMMARY:
==14685==    definitely lost: 496 bytes in 18 blocks
==14685==    indirectly lost: 122,115 bytes in 80 blocks
==14685==      possibly lost: 913 bytes in 12 blocks
==14685==    still reachable: 320,179 bytes in 682 blocks
==14685==         suppressed: 0 bytes in 0 blocks
==14685== Rerun with --leak-check=full to see details of leaked memory
==14685== 
==14685== For lists of detected and suppressed errors, rerun with: -s
==14685== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Screenshot from 2024-04-09 11-30-54

@Knogle
Copy link
Contributor Author

Knogle commented Apr 10, 2024

Not necessary anymore, due to successful C implementation #559

@Knogle Knogle closed this Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants