wolfssl icon indicating copy to clipboard operation
wolfssl copied to clipboard

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

Open gstrauss opened this issue 1 year ago • 24 comments

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

gstrauss avatar Jun 20 '24 02:06 gstrauss

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

anhu avatar Jul 04 '24 17:07 anhu

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.

gstrauss avatar Jul 05 '24 02:07 gstrauss

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

anhu avatar Jul 05 '24 15:07 anhu

Hi @gstrauss ,

Have you tried with the MLDSA keys?

anhu avatar Jul 08 '24 21:07 anhu

@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.

gstrauss avatar Jul 09 '24 14:07 gstrauss

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

anhu avatar Jul 11 '24 20:07 anhu

I have tested the instructions on my slides.

anhu avatar Jul 11 '24 21:07 anhu

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?

gstrauss avatar Jul 12 '24 20:07 gstrauss

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.

anhu avatar Jul 12 '24 21:07 anhu

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.

gstrauss avatar Jul 13 '24 00:07 gstrauss

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

dgarske avatar Jul 15 '24 17:07 dgarske

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.

gstrauss avatar Jul 16 '24 16:07 gstrauss

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

dgarske avatar Jul 16 '24 17:07 dgarske

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

  • OpenSSL 1.1.1 is EOL and no longer maintained by the OpenSSL foundation.
  • OpenSSL 3 (or 3.1?) has native support for the commands in the slides, so building old code is not recommended
  • Building lighttpd from https://github.com/anhu/lighttpd1.4 is VERY WRONG. That is not the official lighttpd source code. The official lighttpd source code is at https://git.lighttpd.net/lighttpd/lighttpd1.4.git
  • If wolfssl is going to provide a patch to lighttpd, wolfssl should provide a patch to lighttpd which applies to a tag in the official lighttpd source code (https://git.lighttpd.net/lighttpd/lighttpd1.4.git). (If the problem in this github issue is resolved in wolfssl, then there should be no need to patch lighttpd mod_wolfssl)
  • I was able to generate dilithium keys using native openssl and liboqs-devel packages on Fedora 40, so the slides provide lots and lots and lots and lots of unnecessary "required" steps for the demo. The demo should list prerequisites, and an appendex could provide additional instructions if the system on which the demo was to be run needed to build those prerequisites.
  • The ./generate_dilithium_chains.sh script with hard-coded wolfssl CN should generate certificate with explicit "TEST" or "DEVEL" or similar in the name, and should not generate what might be mistaken at first glance to be a WolfSSL official certificate.
  • Instead of all the convoluted steps to generate dilithium or kyber keys, wolfssl might provide test certs and keys which are part of the wolfssl test suite and can be downloaded and used directly for testing.
  • The lighttpd.conf should prefer to use ssl.pemfile and ssl.privkey rather than combining key and certficate into a single .pem file.
  • The slides should be updated to refer to wolfSSL GIT Tag: v5.7.2-stable (instead of GIT Tag: v5.7.0-stable)

gstrauss avatar Jul 16 '24 17:07 gstrauss

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

dgarske avatar Jul 16 '24 17:07 dgarske

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 avatar Jul 16 '24 17:07 gstrauss

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

gstrauss avatar Jul 28 '24 23:07 gstrauss

@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 avatar Jul 29 '24 16:07 ColtonWilley

@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.

ColtonWilley avatar Jul 29 '24 20:07 ColtonWilley

(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.

gstrauss avatar Jul 29 '24 20:07 gstrauss

@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.

ColtonWilley avatar Jul 29 '24 21:07 ColtonWilley

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

  • Slide 4 is inaccurate. OQS fork of openssl application is no longer necessary with modern openssl, e.g. openssl 3.something, maybe 3.1?
  • Slides 8 and 9 should be removed.
  • Multiple slides could be removed about cloning https://github.com/wolfssl/osp if slide 14 is modified to curl or wget the test cert files directly from github, and you should suggest putting them in $path_to_lighttpd_certs instead of ./
  • Slide 16 server.document-root = "/path/to/lighttpd" would be better as server.document-root = "/path/to/lighttpd/htdocs" which should be different from the paths to logs and certs. The slides should not suggest putting logs or cert private keys in the web document-root, making them accessible to download.

gstrauss avatar Jul 29 '24 21:07 gstrauss

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.

ColtonWilley avatar Jul 29 '24 21:07 ColtonWilley

@ColtonWilley @dgarske FYI: lighttpd 1.4.77 was just released and by default now uses a limited set of Curves. The demo will need an adjustment to configure lighttpd.conf to include PQC curves in the Curves list.

https://www.lighttpd.net/2025/1/10/1.4.77/


As an aside, lighttpd 1.4.77 includes support for TLS ECH, but WolfSSL support for TLS ECH is limited to the client side. The WolfSSL server-side implementation of TLS ECH is incomplete and does not support multiple ECH keys and ECHConfigs to allow for admin-provided and rotated ECH keys and ECHConfigs.

gstrauss avatar Jan 11 '25 04:01 gstrauss