wolfSSL / wolfssl

The wolfSSL library is a small, fast, portable implementation of TLS/SSL for embedded devices to the cloud. wolfSSL supports up to TLS 1.3 and DTLS 1.3!
https://www.wolfssl.com
GNU General Public License v2.0
2.37k stars 834 forks source link

[Bug]: src/ssl.c:ProcessBuffer() different/broken behavior for same private key depending on PEM or ASN1 format #7668

Open gstrauss opened 5 months ago

gstrauss commented 5 months ago

Contact Details

No response

Version

v5.7.0-stable

Description

src/ssl.c:ProcessBuffer() fails if dilithium_level5_entity_key.pem is parsed by application into DER and passed to wolfSSL_CTX_use_PrivateKey_buffer() with format WOLFSSL_FILETYPE_ASN1, but works if the PEM buffer is passed to wolfSSL_CTX_use_PrivateKey_buffer() with format WOLFSSL_FILETYPE_PEM

Separate bug: wolfSSL crashes with SIGSEGV if the above private key (incorrectly) is passed as PEM buffer to wolfSSL_CTX_use_PrivateKey_buffer() with format WOLFSSL_FILETYPE_ASN1. Note the mismatched format. Still, wolfSSL should not crash, as the buffer and buffer length are parameters.

Reproduction steps

Ask @anhu for instructions for Post-Quantum Algorithms in cURL using lighttpd and wolfssl (and please give him feedback if the demo could be simpler)

Alternatively, generate dilithium_level5_entity_key.pem using oqs/generate_dilithium_chains.sh from wolfSSL osp repo. https://github.com/wolfSSL/osp

Read the file into memory and pass to wolfSSL_CTX_use_PrivateKey_buffer(). Then, parse the PEM into DER and call wolfSSL_CTX_use_PrivateKey_buffer().

Relevant log output

More details in https://wolfssl.zendesk.com/hc/requests/18173
anhu commented 4 months ago

Hi,

I built with the following configuration: $ ./configure --enable-experimental --with-liboqs CFLAGS=-DNO_FILESYSTEM

I then made the following quick and temporary changes:

diff --git examples/server/server.c examples/server/server.c
index cca853a14..b65e75434 100644
--- examples/server/server.c
+++ examples/server/server.c
@@ -2720,8 +2720,14 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args)
     #endif /* HAVE_PK_CALLBACKS && TEST_PK_PRIVKEY */
     ) {
     #ifdef NO_FILESYSTEM
-        if (wolfSSL_CTX_use_PrivateKey_buffer(ctx, server_key_der_2048,
-            sizeof_server_key_der_2048, SSL_FILETYPE_ASN1) != WOLFSSL_SUCCESS)
+
+byte dilithium_buffer[8000];
+int dilithium_sz;
+FILE * f = fopen("/path/to/wolfssl-examples/certs/mldsa87_server_key.der", "r");
+dilithium_sz = fread(dilithium_buffer, 1, 8000, f);
+fclose(f);
+        if (wolfSSL_CTX_use_PrivateKey_buffer(ctx, dilithium_buffer,
+            dilithium_sz, SSL_FILETYPE_ASN1) != WOLFSSL_SUCCESS)
             err_sys_ex(catastrophic, "can't load server private key buffer");
     #elif !defined(TEST_LOAD_BUFFER)
         if (SSL_CTX_use_PrivateKey_file(ctx, ourKey, WOLFSSL_FILETYPE_PEM)

I ran examples/server/server and no error occurred.

I then use this change:

diff --git examples/server/server.c examples/server/server.c
index cca853a14..ae3246c39 100644
--- examples/server/server.c
+++ examples/server/server.c
@@ -2720,8 +2720,14 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args)
     #endif /* HAVE_PK_CALLBACKS && TEST_PK_PRIVKEY */
     ) {
     #ifdef NO_FILESYSTEM
-        if (wolfSSL_CTX_use_PrivateKey_buffer(ctx, server_key_der_2048,
-            sizeof_server_key_der_2048, SSL_FILETYPE_ASN1) != WOLFSSL_SUCCESS)
+
+byte dilithium_buffer[8000];
+int dilithium_sz;
+FILE * f = fopen("/path/to/wolfssl-examples/certs/mldsa87_server_key.der", "r");
+dilithium_sz = fread(dilithium_buffer, 1, 8000, f);
+fclose(f);
+        if (wolfSSL_CTX_use_PrivateKey_buffer(ctx, dilithium_buffer,
+            dilithium_sz, SSL_FILETYPE_PEM) != WOLFSSL_SUCCESS)
             err_sys_ex(catastrophic, "can't load server private key buffer");
     #elif !defined(TEST_LOAD_BUFFER)
         if (SSL_CTX_use_PrivateKey_file(ctx, ourKey, WOLFSSL_FILETYPE_PEM)

As expected, I got the following error message: wolfSSL error: can't load server private key buffer

I then used this change:

diff --git examples/server/server.c examples/server/server.c
index cca853a14..88f8b1b56 100644
--- examples/server/server.c
+++ examples/server/server.c
@@ -2720,8 +2720,14 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args)
     #endif /* HAVE_PK_CALLBACKS && TEST_PK_PRIVKEY */
     ) {
     #ifdef NO_FILESYSTEM
-        if (wolfSSL_CTX_use_PrivateKey_buffer(ctx, server_key_der_2048,
-            sizeof_server_key_der_2048, SSL_FILETYPE_ASN1) != WOLFSSL_SUCCESS)
+
+byte dilithium_buffer[11000];
+int dilithium_sz;
+FILE * f = fopen("/path/to/wolfssl-examples/certs/mldsa87_server_key.pem", "r");
+dilithium_sz = fread(dilithium_buffer, 1, 11000, f);
+fclose(f);
+        if (wolfSSL_CTX_use_PrivateKey_buffer(ctx, dilithium_buffer,
+            dilithium_sz, SSL_FILETYPE_PEM) != WOLFSSL_SUCCESS)
             err_sys_ex(catastrophic, "can't load server private key buffer");
     #elif !defined(TEST_LOAD_BUFFER)
         if (SSL_CTX_use_PrivateKey_file(ctx, ourKey, WOLFSSL_FILETYPE_PEM)

I ran examples/server/server and no error occurred.

I then used this change:

diff --git examples/server/server.c examples/server/server.c
index cca853a14..ae2599a85 100644
--- examples/server/server.c
+++ examples/server/server.c
@@ -2720,8 +2720,14 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args)
     #endif /* HAVE_PK_CALLBACKS && TEST_PK_PRIVKEY */
     ) {
     #ifdef NO_FILESYSTEM
-        if (wolfSSL_CTX_use_PrivateKey_buffer(ctx, server_key_der_2048,
-            sizeof_server_key_der_2048, SSL_FILETYPE_ASN1) != WOLFSSL_SUCCESS)
+
+byte dilithium_buffer[11000];
+int dilithium_sz;
+FILE * f = fopen("/path/to/wolfssl-examples/certs/mldsa87_server_key.pem", "r");
+dilithium_sz = fread(dilithium_buffer, 1, 11000, f);
+fclose(f);
+        if (wolfSSL_CTX_use_PrivateKey_buffer(ctx, dilithium_buffer,
+            dilithium_sz, SSL_FILETYPE_ASN1) != WOLFSSL_SUCCESS)
             err_sys_ex(catastrophic, "can't load server private key buffer");
     #elif !defined(TEST_LOAD_BUFFER)
         if (SSL_CTX_use_PrivateKey_file(ctx, ourKey, WOLFSSL_FILETYPE_PEM)

As expected, I got the following error message: wolfSSL error: can't load server private key buffer

Note, you'll need the following wolfssl-examples PR to get the keys: https://github.com/wolfSSL/wolfssl-examples/pull/443

Warm regards, Anthony

gstrauss commented 4 months ago

lighttpd base64-decodes the key from a .pem file into a buffer which contains the DER. Passing this value to wolfSSL_CTX_use_PrivateKey_buffer() failed with dilithium keys. Using lighttpd mod_openssl instead of lighttpd mod_wolfssl works fine with the same keys, so the base64-decoding is working.

lighttpd does not use stdio because lighttpd takes steps to wipe the memory containing sensitive keys, something that you can not do portably with internal stdio buffers unless you employ setvbuf() with a custom-allocated buffer.

anhu commented 4 months ago

We have recently be moving to newer versions of the algorithm. You might need to use mldsa when generating the certificates.

anhu commented 4 months ago

Hi @gstrauss ,

Have you tried with the MLDSA keys?

gstrauss commented 4 months ago

@anhu Have you tested with v5.7.0-stable, as specified in my original post at the top of this page and in https://wolfssl.zendesk.com/hc/requests/18173 and in your slides?

v5.7.2-stable is tagged yesterday (8 July 2024). Please test with that tag, too.

anhu commented 4 months ago

I have tested against v5.7.2-stable and I get the same results as noted above.

anhu commented 4 months ago

I have tested the instructions on my slides.

gstrauss commented 4 months ago

I have tested the instructions on my slides.

And??? Did you test with the original instructions where you needed to patch lighttpd mod_wolfssl due to this bug in wolfssl? Were your slides modified? If so, how? Were your slides updated to use MLDSA keys? Were you slides updated to remove any additional patches to lighttpd? What version of lighttpd are you using? Can you confirm that you do not need any patches to lighttpd mod_wolfssl?

anhu commented 4 months ago

Instructions on the slide remain the same. No need for changes to the slides.
If there is no objection, I will now close this ticket.

gstrauss commented 4 months ago

I object.

Instructions on the slide remain the same. No need for changes to the slides. If there is no objection, I will now close this ticket.

@dgarske would you please explain to @anhu that a) he did not answer my questions, and b) "If there is no objection, I will now close this ticket." and then immediately closing the ticket is rude since there were open questions (which he curtly replied vaguely, without actually answering the questions.)

If you think that @anhu answered my questions clearly in this ticket, please kindly point out to me where "Can you confirm that you do not need any patches to lighttpd mod_wolfssl?" was answered. Thank you.

dgarske commented 4 months ago

Hi @gstrauss ,

Thanks for your report and detail. I've read through the ticket and the issue. I couldn't find an answer to your question if patches were needed for lighttpd. I have downloaded the latest slides and will attempt to reproduce and answer the question.

Thanks, David Garske, wolfSSL

gstrauss commented 4 months ago

Thanks, @dgarske

Is a current version of the slides available? @anhu's communications were vague and I do not know the current state of them. I filed this PR after following a (previous?) version of the slides sent to me as an attachment in https://wolfssl.zendesk.com/hc/requests/18173. Using the version of wolfssl specified in those slides, I duplicated the issue for which @anhu patched lighttpd (instead of fixing the issue in wolfssl) and traced it to the wolfssl code.

The issue might already have been fixed in the recently-released wolfssl v5.7.2-stable as I see that the code in question was moved to a different file and since modified.

dgarske commented 4 months ago

Hi @gstrauss ,

Attached is the latest PDF of the slides I could find: pq_curl_07_2024.pdf. These can be public.

I didn't see any fixes in wolfSSL yet to address this, but I could have missed this. I suspect the lighttpd patch is still required. I plan to fix properly in wolfSSL when I get some time later this week.

Thanks, David Garske, wolfSSL

gstrauss commented 4 months ago

Thanks. I had given @anhu feedback about the slides and will post what I remember here:

dgarske commented 4 months ago

Thank you @gstrauss . That is all excellent feedback. I'll make sure it is taken in.

gstrauss commented 4 months ago

Some details about my testing in lighttpd before filing this issue for WolfSSL:

lighttpd reads in a .der or .pem file and converts PEM to DER in an in-memory buffer which is submitted to wolfSSL_CTX_use_PrivateKey_buffer() or wolfSSL_use_PrivateKey_buffer() for new connections based on the TLS SNI. The reason to store the DER format is to elide the need to convert PEM to DER for each and every HTTPS connection.

I tested lighttpd code which reads PEM and base64-decodes into DER. Then I took the DER and base64-encoded it, manually added PEM header and footer, and passed that to wolfSSL and it worked. Therefore, the lighttpd code reading and base64-decoding the PEM file is working. (Similarly, substituting lighttpd mod_openssl for lighttpd mod_wolfssl works with the same certficates without any changes to lighttpd.)

I traced the apparent difference in behavior to the code underlying wolfSSL_use_PrivateKey_buffer() which appears to perform extra steps if DER is passed instead of PEM. I had expected PEM to be base64-decoded to DER and then for the DER data to be processed, but the data buffer below the code block which processes PEM or DER is differently sized depending on whether PEM data was passed into the function or DER data was passed into the function.

gstrauss commented 4 months ago

@ColtonWilley have you had a chance to review https://wolfssl.zendesk.com/hc/requests/18173 and the notes here?

ColtonWilley commented 4 months ago

@gstrauss Yes I have reviewed everything related to this issue. I had actually begun working on this last week but unfortunately fell ill. I was not able to reproduce this issue myself, but looking at the PEM/DER handling in wolfssl and lighttpd I see some potential issues there. I will be looking at recreating the underlying failure of wolfSSL_use_PrivateKey_buffer() to accept the dilithium DER.

ColtonWilley commented 4 months ago

@gstrauss I followed the process described as: "Alternatively, generate dilithium_level5_entity_key.pem using oqs/generate_dilithium_chains.sh Read the file into memory and pass to wolfSSL_CTX_use_PrivateKey_buffer(). Then, parse the PEM into DER and call wolfSSL_CTX_use_PrivateKey_buffer()."

I did this on 5.7.2-stable and was not able to reproduce.

I also tried to reproduce the segmentation fault described here: "wolfSSL crashes with SIGSEGV if the above private key (incorrectly) is passed as PEM buffer to wolfSSL_CTX_use_PrivateKey_buffer() with format WOLFSSL_FILETYPE_ASN1"

I tried this on 5.7.2-stable and got a graceful error as expected.

Can you please verify that these issues still persist for you when using 5.7.2-stable? If you are still getting an error is it possible for you to provide the failing input DER buffer and length?

Thanks, Colton Willey, wolfSSL.

gstrauss commented 4 months ago

(I hope you're feeling better.)

The origin of me filing this issue was @anhu modifying lighttpd in his slide demo for dilithium keys.

As noted in this bug report, I had used v5.7.0, as did @anhu.

I noted that there were changes in the code between then and v5.7.2. Since you already have a working test environment, I would appreciate if you could test v5.7.0. I would have to rebuild a test environment from scratch to do so.

If wolfssl v5.7.2 no longer requires patching lighttpd to use dilithium keys, then this issue can be closed.

If patching lighttpd is still required, I would like to know why, so that it can be fixed.

ColtonWilley commented 4 months ago

@gstrauss I was able to recreate the underlying failure of wolfSSL_CTX_use_PrivateKey_buffer() with v5.7.0 and the patch from the slides, a valid dilithium DER is rejected. The same calling code works for unmodified v5.7.2-stable, which lines up with the code changes.

The slides have been reworked here https://docs.google.com/presentation/d/1OtVDn0JqQUmiyz2U60YS4_eKDx0KmIczcv8W4G4DbA4/edit?usp=sharing to not use liboqs anymore, use pre-generated certs, and using 5.7.2-stable.

It seems 5.7.0 with the patch simply wont work as is for dilithium DER, so 5.7.2 will be required.

"If wolfssl v5.7.2 no longer requires patching lighttpd to use dilithium keys, then this issue can be closed." It is my understanding that unmodified lighttpd should be able to use dilithium keys successfully with unmodified wolfssl 5.7.2-stable.

Please let me know if you are still encountering any issues, or if this issue has been resolved for you. Thanks, Colton Willey, wolfSSL.

gstrauss commented 4 months ago

Yes, this issue is resolved and can be closed since lighttpd works unmodified with wolfssl v5.7.2. Thank you for verifying.

Regarding the slides: https://docs.google.com/presentation/d/1OtVDn0JqQUmiyz2U60YS4_eKDx0KmIczcv8W4G4DbA4/edit?usp=sharing

ColtonWilley commented 4 months ago

Thank you for your thorough reviews of the slide materials, wolfSSL appreciates the time you spend on these issues. Special thanks for recommendation on slide 16, I definitely dont know lighttpd well enough to catch that. I will keep this issue open until I have updated the slides appropriately.