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

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

Open
gstrauss opened this issue Jun 20, 2024 · 23 comments
Assignees
Labels

Comments

@gstrauss
Copy link
Contributor

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
Copy link
Member

anhu commented Jul 4, 2024

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: wolfSSL/wolfssl-examples#443

Warm regards, Anthony

@gstrauss
Copy link
Contributor Author

gstrauss commented Jul 5, 2024

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
Copy link
Member

anhu commented Jul 5, 2024

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

@anhu
Copy link
Member

anhu commented Jul 8, 2024

Hi @gstrauss ,

Have you tried with the MLDSA keys?

@gstrauss
Copy link
Contributor Author

gstrauss commented Jul 9, 2024

@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
Copy link
Member

anhu commented Jul 11, 2024

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

@anhu
Copy link
Member

anhu commented Jul 11, 2024

I have tested the instructions on my slides.

@gstrauss
Copy link
Contributor Author

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
Copy link
Member

anhu commented Jul 12, 2024

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 anhu closed this as completed Jul 12, 2024
@gstrauss
Copy link
Contributor Author

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 dgarske self-assigned this Jul 15, 2024
@dgarske dgarske reopened this Jul 15, 2024
@dgarske
Copy link
Contributor

dgarske commented Jul 15, 2024

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
Copy link
Contributor Author

gstrauss commented Jul 16, 2024

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
Copy link
Contributor

dgarske commented Jul 16, 2024

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
Copy link
Contributor Author

gstrauss commented Jul 16, 2024

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)

@dgarske
Copy link
Contributor

dgarske commented Jul 16, 2024

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

@gstrauss
Copy link
Contributor Author

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.

@dgarske dgarske assigned ColtonWilley and unassigned dgarske Jul 22, 2024
@gstrauss
Copy link
Contributor Author

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

@ColtonWilley
Copy link
Contributor

@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
Copy link
Contributor

@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
Copy link
Contributor Author

(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
Copy link
Contributor

@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
Copy link
Contributor Author

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.

@ColtonWilley
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants