QUIC support for wolfSSL #5384

icing · 2022-07-21T15:11:34Z

Description

This PR adds QUIC support to wolfSSL.

Please see the supplied QUIC.md for a complete description.

Testing

test cases have been added in tests/quic.c and run as part of make test.
this PR has been verified in the ngtcp2 project to achieve inter-op with itself, openssl and other public QUIC implementations.

Checklist

added tests
updated/added doxygen
tbd. when changes and new API has been reviewed.
updated appropriate READMEs
Updated manual and documentation
see QUIC.md as a base for further documentation updates

wolfSSL-Bot · 2022-07-21T16:19:05Z

Can one of the wolfSSL admins verify this patch?

dgarske · 2022-07-21T16:24:33Z

OK to test

icing · 2022-07-22T17:35:05Z

I have no access to the 2 last failing test suites. So, if someone could figure out what is needed to have them pass, let me know.

dgarske · 2022-07-22T17:46:54Z

I have no access to the 2 last failing test suites. So, if someone could figure out what is needed to have them pass, let me know.

./configure --enable-sniffer && make check
FAIL: scripts/sniffer-testsuite.test

-----------------------------------------------
Detecting new files that have been added
-----------------------------------------------
added: QUIC.md
added: src/quic.c
added: tests/quic.c
added: wolfssl/quic.h
Total new files to check: 4
-----------------------------------------------
Checking if files are included in an include.am
-----------------------------------------------
./src/include.am:src_libwolfssl_la_SOURCES += src/quic.c
./tests/include.am:                  tests/quic.c \
./wolfssl/include.am:                         wolfssl/quic.h \
AUTOCONF NEW FILE DETECTOR FAILED!

icing · 2022-07-25T08:33:09Z

./configure --enable-sniffer && make check
FAIL: scripts/sniffer-testsuite.test

Thank you! The latest addition fixes that for me. Let's see what CI says.

Also as a note, if I configure

> ./configure --enable-all --enable-sniffer && make check

I get failures in ocsp-stapling and unit.tests, but after some research I found that those also happen in master. Just FYI.

dgarske

Some minor cast warnings:

./configure CC=g++ --enable-quic

In file included from tests/quic.c:29:

tests/quic.c: In function ‘int test_quic_crypt()’:
./wolfssl/wolfcrypt/types.h:508:67: error: invalid conversion from ‘void*’ to ‘uint8_t*’ {aka ‘unsigned char*’} [-fpermissive]
  508 |                 #define XMALLOC(s, h, t)     ((void)(h), (void)(t), wolfSSL_Malloc((s)))
      |                                              ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
      |                                                                   |
      |                                                                   void*
./tests/unit.h:52:49: note: in definition of macro ‘Assert’

   52 | #define Assert(test, description, result) if (!(test)) Fail(description, result)
      |                                                 ^~~~

tests/quic.c:353:9: note: in expansion of macro ‘AssertNotNull’

  353 |         AssertNotNull(encrypted = XMALLOC(enc_len, NULL, DYNAMIC_TYPE_TMP_BUFFER));
      |         ^~~~~~~~~~~~~

tests/quic.c:353:35: note: in expansion of macro ‘XMALLOC’

  353 |         AssertNotNull(encrypted = XMALLOC(enc_len, NULL, DYNAMIC_TYPE_TMP_BUFFER));
      |                                   ^~~~~~~
./wolfssl/wolfcrypt/types.h:508:67: error: invalid conversion from ‘void*’ to ‘uint8_t*’ {aka ‘unsigned char*’} [-fpermissive]
  508 |                 #define XMALLOC(s, h, t)     ((void)(h), (void)(t), wolfSSL_Malloc((s)))
      |                                              ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
      |                                                                   |
      |                                                                   void*
./tests/unit.h:52:49: note: in definition of macro ‘Assert’

   52 | #define Assert(test, description, result) if (!(test)) Fail(description, result)
      |                                                 ^~~~

tests/quic.c:354:9: note: in expansion of macro ‘AssertNotNull’

  354 |         AssertNotNull(decrypted = XMALLOC(dec_len, NULL, DYNAMIC_TYPE_TMP_BUFFER));
      |         ^~~~~~~~~~~~~

tests/quic.c:354:35: note: in expansion of macro ‘XMALLOC’

  354 |         AssertNotNull(decrypted = XMALLOC(dec_len, NULL, DYNAMIC_TYPE_TMP_BUFFER));
      |                                   ^~~~~~~

tests/quic.c: In function ‘void QuicTestContext_init(QuicTestContext*, WOLFSSL_CTX*, const char*, int)’:

tests/quic.c:432:66: error: narrowing conversion of ‘-1’ from ‘int’ to ‘byte’ {aka ‘unsigned char’} [-Wnarrowing]
  432 |     static const byte tp_params_s[] = {7, 6, 5, 4, 3, 2, 1, 0, -1};
      |                                                                  ^

tests/quic.c: In function ‘int ctx_set_encryption_secrets(WOLFSSL*, WOLFSSL_ENCRYPTION_LEVEL, const uint8_t*, const uint8_t*, size_t)’:

tests/quic.c:476:48: error: invalid conversion from ‘void*’ to ‘QuicTestContext*’ [-fpermissive]
  476 |     QuicTestContext *ctx = wolfSSL_get_app_data(ssl);
      |                            ~~~~~~~~~~~~~~~~~~~~^~~~~
      |                                                |
      |                                                void*

tests/quic.c: In function ‘int ctx_add_handshake_data(WOLFSSL*, WOLFSSL_ENCRYPTION_LEVEL, const uint8_t*, size_t)’:

tests/quic.c:495:48: error: invalid conversion from ‘void*’ to ‘QuicTestContext*’ [-fpermissive]
  495 |     QuicTestContext *ctx = wolfSSL_get_app_data(ssl);
      |                            ~~~~~~~~~~~~~~~~~~~~^~~~~
      |                                                |
      |                                                void*

tests/quic.c:505:31: error: invalid conversion from ‘void*’ to ‘OutputBuffer*’ [-fpermissive]
  505 |             out->next = calloc(1, sizeof(OutputBuffer));
      |                         ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
      |                               |
      |                               void*

tests/quic.c: In function ‘int ctx_flush_flight(WOLFSSL*)’:

tests/quic.c:525:48: error: invalid conversion from ‘void*’ to ‘QuicTestContext*’ [-fpermissive]
  525 |     QuicTestContext *ctx = wolfSSL_get_app_data(ssl);
      |                            ~~~~~~~~~~~~~~~~~~~~^~~~~
      |                                                |
      |                                                void*

tests/quic.c: In function ‘int ctx_send_alert(WOLFSSL*, WOLFSSL_ENCRYPTION_LEVEL, uint8_t)’:

tests/quic.c:534:48: error: invalid conversion from ‘void*’ to ‘QuicTestContext*’ [-fpermissive]
  534 |     QuicTestContext *ctx = wolfSSL_get_app_data(ssl);
      |                            ~~~~~~~~~~~~~~~~~~~~^~~~~
      |                                                |
      |                                                void*

make[2]: *** [Makefile:6497: tests/unit_test-quic.o] Error 1

make[2]: *** Waiting for unfinished jobs....

In file included from ./wolfssl/wolfcrypt/misc.h:32,
                 from ./wolfcrypt/src/misc.c:37,
                 from src/quic.c:34:

src/quic.c: In function ‘QuicRecord* quic_record_make(WOLFSSL*, WOLFSSL_ENCRYPTION_LEVEL, const uint8_t*, size_t)’:
./wolfssl/wolfcrypt/types.h:508:67: error: invalid conversion from ‘void*’ to ‘QuicRecord*’ [-fpermissive]
  508 |                 #define XMALLOC(s, h, t)     ((void)(h), (void)(t), wolfSSL_Malloc((s)))
      |                                              ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
      |                                                                   |
      |                                                                   void*

src/quic.c:76:10: note: in expansion of macro ‘XMALLOC’

   76 |     qr = XMALLOC(sizeof(*qr), ssl->heap, DYNAMIC_TYPE_TMP_BUFFER);
      |          ^~~~~~~
./wolfssl/wolfcrypt/types.h:508:67: error: invalid conversion from ‘void*’ to ‘uint8_t*’ {aka ‘unsigned char*’} [-fpermissive]
  508 |                 #define XMALLOC(s, h, t)     ((void)(h), (void)(t), wolfSSL_Malloc((s)))
      |                                              ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
      |                                                                   |
      |                                                                   void*

src/quic.c:89:20: note: in expansion of macro ‘XMALLOC’

   89 |         qr->data = XMALLOC(qr->capacity, ssl->heap, DYNAMIC_TYPE_TMP_BUFFER);
      |                    ^~~~~~~

src/quic.c: In function ‘int quic_record_append(WOLFSSL*, QuicRecord*, const uint8_t*, size_t, size_t*)’:
./wolfssl/wolfcrypt/types.h:514:61: error: invalid conversion from ‘void*’ to ‘uint8_t*’ {aka ‘unsigned char*’} [-fpermissive]
  514 |                 #define XREALLOC(p, n, h, t) wolfSSL_Realloc((p), (n))
      |                                              ~~~~~~~~~~~~~~~^~~~~~~~~~
      |                                                             |
      |                                                             void*

src/quic.c:131:30: note: in expansion of macro ‘XREALLOC’

  131 |             uint8_t *ndata = XREALLOC(qr->data, qr->len, ssl->head, DYNAMIC_TYPE_TMP_BUFFER);
      |                              ^~~~~~~~

src/quic.c: In function ‘word32 quic_record_transfer(QuicRecord*, byte*, word32)’:

src/quic.c:186:44: error: enumeral and non-enumeral type in conditional expression [-Werror=extra]
  186 |         rlen = (qr->len <= MAX_RECORD_SIZE)? qr->len : MAX_RECORD_SIZE;
      |                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~

In file included from ./wolfssl/wolfcrypt/misc.h:32,
                 from ./wolfcrypt/src/misc.c:37,
                 from src/quic.c:34:

src/quic.c: In function ‘const QuicTransportParam* QuicTransportParam_new(const uint8_t*, size_t, void*)’:
./wolfssl/wolfcrypt/types.h:508:67: error: invalid conversion from ‘void*’ to ‘QuicTransportParam*’ [-fpermissive]
  508 |                 #define XMALLOC(s, h, t)     ((void)(h), (void)(t), wolfSSL_Malloc((s)))
      |                                              ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
      |                                                                   |
      |                                                                   void*

src/quic.c:212:10: note: in expansion of macro ‘XMALLOC’

  212 |     tp = XMALLOC(sizeof(*tp), heap, DYNAMIC_TYPE_TLSX);
      |          ^~~~~~~
./wolfssl/wolfcrypt/types.h:508:67: error: invalid conversion from ‘void*’ to ‘const uint8_t*’ {aka ‘const unsigned char*’} [-fpermissive]
  508 |                 #define XMALLOC(s, h, t)     ((void)(h), (void)(t), wolfSSL_Malloc((s)))
      |                                              ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
      |                                                                   |
      |                                                                   void*

src/quic.c:214:16: note: in expansion of macro ‘XMALLOC’

  214 |     tp->data = XMALLOC(len, heap, DYNAMIC_TYPE_TLSX);
      |                ^~~~~~~

src/quic.c: In function ‘const QuicTransportParam* QuicTransportParam_dup(const QuicTransportParam*, void*)’:
./wolfssl/wolfcrypt/types.h:508:67: error: invalid conversion from ‘void*’ to ‘QuicTransportParam*’ [-fpermissive]
  508 |                 #define XMALLOC(s, h, t)     ((void)(h), (void)(t), wolfSSL_Malloc((s)))
      |                                              ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
      |                                                                   |
      |                                                                   void*

src/quic.c:227:11: note: in expansion of macro ‘XMALLOC’

  227 |     tp2 = XMALLOC(sizeof(*tp2), heap, DYNAMIC_TYPE_TLSX);
      |           ^~~~~~~
./wolfssl/wolfcrypt/types.h:508:67: error: invalid conversion from ‘void*’ to ‘const uint8_t*’ {aka ‘const unsigned char*’} [-fpermissive]
  508 |                 #define XMALLOC(s, h, t)     ((void)(h), (void)(t), wolfSSL_Malloc((s)))
      |                                              ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
      |                                                                   |
      |                                                                   void*

src/quic.c:229:17: note: in expansion of macro ‘XMALLOC’

  229 |     tp2->data = XMALLOC(tp->len, heap, DYNAMIC_TYPE_TLSX);
      |                 ^~~~~~~

src/quic.c: In function ‘int wolfSSL_quic_send_internal(WOLFSSL*)’:

src/quic.c:740:27: error: invalid conversion from ‘int’ to ‘WOLFSSL_ENCRYPTION_LEVEL’ {aka ‘wolfssl_encryption_level_t’} [-fpermissive]
  740 |                 ssl->quic.output_rec_level, (const uint8_t*)output, len);
      |                 ~~~~~~~~~~^~~~~~~~~~~~~~~~
      |                           |
      |                           int

configure.ac

src/QUIC.md

tests/quic.c

SparkiDev

80 characters per line thanks. Noticeably quic.c.

We are using lots of OpenSSL type in new public APIs.
Is this to fit in with other products?

src/quic.c

src/tls.c

src/tls13.c

icing · 2022-07-27T08:07:20Z

Some minor cast warnings:

Should all be resolved now.

icing · 2022-07-27T08:24:49Z

80 characters per line thanks. Noticeably quic.c.

Done.

We are using lots of OpenSSL type in new public APIs. Is this to fit in with other products?

Which ones specifically? I think I used the WOLFSSL things where possible. Are your talking about the use of uint8_t? That I did to be more compatible to the quictls/openssl fork, yes. It's not exactly required and we could change.

SparkiDev · 2022-07-27T22:09:51Z

APIs like:
wolfSSL_quic_aead_is_gcm
wolfSSL_quic_get_md
wolfSSL_quic_get_hp
wolfSSL_quic_aead_encrypt
wolfSSL_quic_hkdf_expand

Use OpenSSL types.

icing · 2022-07-28T09:01:51Z

APIs like: wolfSSL_quic_aead_is_gcm wolfSSL_quic_get_md wolfSSL_quic_get_hp wolfSSL_quic_aead_encrypt wolfSSL_quic_hkdf_expand

Use OpenSSL types.

Ah, I see now what you mean. For me, anything with prefix WOLFSSL_ was a wolfSSL type and not an openssl one. But WOLFSSL_EVP_CIPHER is from wolfssl/openssl/evp.h, so for you it is an openssl type. Got you.

The motivation for this was that I tried to stay close to the quictls/openssl patch and that crypto internals are not my strength. It was not immediately obvious to me how the EVP functions could be replaced with something like wc_Tls13_HKDF_Expand_Label()without deep diving into the crypt layer.

If you think there is value in avoiding the EVP part of wolfSSL, I'd need either some guidance or someone with deeper understanding needs to write a PR on top of this one with the changes. I can't tell what the best way to approach this would be.

SparkiDev · 2022-07-28T22:48:16Z

That's fine we'll add different APIs later if needed.
Thanks!

dgarske · 2022-08-03T14:39:18Z

@icing , please resolve minor merge conflict.

dgarske

Spelling:

diff --git a/src/quic.c b/src/quic.c
index 79dae7371..91f7f257a 100644
--- a/src/quic.c
+++ b/src/quic.c
@@ -451,7 +451,7 @@ int wolfSSL_quic_add_transport_extensions(WOLFSSL* ssl, int msg_type)
     }

     if (is_resp) {
-        /* server repsonse: time to decide which version to use */
+        /* server response: time to decide which version to use */
         if (ssl->quic.transport_peer && ssl->quic.transport_peer_draft) {
             if (ssl->quic.transport_version == TLSX_KEY_QUIC_TP_PARAMS_DRAFT) {
                 ret = TLSX_QuicTP_Use(ssl,
@@ -536,7 +536,7 @@ void wolfSSL_set_quic_early_data_enabled(WOLFSSL* ssl, int enabled)
      * not started yet.
      * This function is part of the quictls/openssl API and does
      * not return any error, sadly. So we just ignore any
-     * unsuccessfull use. But we can produce some warnings.
+     * unsuccessful use. But we can produce some warnings.
      */
     if (!WOLFSSL_IS_QUIC(ssl)) {
         WOLFSSL_MSG("wolfSSL_set_quic_early_data_enabled: not a QUIC SSL");
@@ -754,7 +754,7 @@ static int wolfSSL_quic_send_internal(WOLFSSL* ssl)
             aret = ssl->quic.method->add_handshake_data(ssl,
                 ssl->quic.output_rec_level, (const uint8_t*)output, len);
             if (aret != 1) {
-                /* The application has an error. General desaster. */
+                /* The application has an error. General disaster. */
                 WOLFSSL_MSG("WOLFSSL_QUIC_SEND application failed");
                 ret = FWRITE_ERROR;
                 goto cleanup;

CC=g++ build errors:

src/quic.c:189:45: error: enumeral and non-enumeral type in conditional expression [-Werror=extra]
  189 |         rlen = (qr->len <= MAX_RECORD_SIZE) ? qr->len : MAX_RECORD_SIZE;
      |                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
tests/quic.c: In function ‘void QuicTestContext_init(QuicTestContext*, WOLFSSL_CTX*, const char*, int)’:
tests/quic.c:434:66: error: narrowing conversion of ‘-1’ from ‘int’ to ‘byte’ {aka ‘unsigned char’} [-Wnarrowing]
  434 |     static const byte tp_params_s[] = {7, 6, 5, 4, 3, 2, 1, 0, -1};
      |                                                                  ^
tests/quic.c: In function ‘int new_session_cb(WOLFSSL*, WOLFSSL_SESSION*)’:
tests/quic.c:1277:48: error: invalid conversion from ‘void*’ to ‘QuicTestContext*’ [-fpermissive]
 1277 |     QuicTestContext *ctx = wolfSSL_get_app_data(ssl);
      |                            ~~~~~~~~~~~~~~~~~~~~^~~~~
      |                                                |
      |                                                void*

icing · 2022-08-04T16:50:48Z

Should be fixed now. Thanks for reviewing.

dgarske

This all looks great!

Can you please squash merge these commits?

git checkout quic
git rebase master
git checkout master
git merge --squash quic
git branch -m quic quic_old
git checkout -b quic
git commit
git push -f

bagder · 2022-08-09T11:18:31Z

I am not in a position to review this wolfSSL code, but I'm eager to try this out as soon as this has merged - as part of the greater work by @icing, as I hope to use wolfSSL with ngtcp2 to do full fledged HTTP/3 in curl with this stack.

added numerous missing _SMALL_STACK code paths (PK objects on the stack); in settings.h, enable WOLFSSL_SMALL_STACK_STATIC by default when WOLFSSL_SMALL_STACK is defined (NO_WOLFSSL_SMALL_STACK_STATIC to override); fixes for unsafe strcat()s in tests/quic.c; fix for unsafe macro WOLFSSL_IS_QUIC(); fix to exclude quic from enable-all when enable-linuxkm (quic needs opensslextra, and opensslextra currently only works in-kernel in cryptonly builds); fix for signed/unsigned clash in wolfSSL_quic_receive().

dgarske self-assigned this Jul 21, 2022

dgarske requested changes Jul 26, 2022

View reviewed changes

configure.ac Show resolved Hide resolved

src/QUIC.md Outdated Show resolved Hide resolved

tests/quic.c Outdated Show resolved Hide resolved

dgarske assigned icing Jul 26, 2022

SparkiDev self-assigned this Jul 27, 2022

SparkiDev requested changes Jul 27, 2022

View reviewed changes

SparkiDev approved these changes Jul 28, 2022

View reviewed changes

dgarske approved these changes Aug 3, 2022

View reviewed changes

dgarske unassigned dgarske and SparkiDev Aug 3, 2022

icing force-pushed the quic branch from d335fba to a50738f Compare August 4, 2022 07:45

dgarske requested changes Aug 4, 2022

View reviewed changes

icing requested a review from dgarske August 4, 2022 16:50

dgarske approved these changes Aug 4, 2022

View reviewed changes

icing force-pushed the quic branch 2 times, most recently from 49a3847 to df44dd8 Compare August 8, 2022 09:19

add QUIC support.

4431438

icing force-pushed the quic branch from df44dd8 to 4431438 Compare August 8, 2022 11:24

dgarske merged commit fa97923 into wolfSSL:master Aug 9, 2022

douzzer mentioned this pull request Aug 9, 2022

20220809-quic-fixes #5449

Merged

douzzer mentioned this pull request Aug 17, 2022

20220817-make-quic-optional #5477

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

QUIC support for wolfSSL #5384

QUIC support for wolfSSL #5384

icing commented Jul 21, 2022

wolfSSL-Bot commented Jul 21, 2022

dgarske commented Jul 21, 2022

icing commented Jul 22, 2022

dgarske commented Jul 22, 2022

icing commented Jul 25, 2022

dgarske left a comment

SparkiDev left a comment

icing commented Jul 27, 2022

icing commented Jul 27, 2022

SparkiDev commented Jul 27, 2022

icing commented Jul 28, 2022

SparkiDev commented Jul 28, 2022

dgarske commented Aug 3, 2022 •

edited

Loading

dgarske left a comment

icing commented Aug 4, 2022

dgarske left a comment

bagder commented Aug 9, 2022

QUIC support for wolfSSL #5384

QUIC support for wolfSSL #5384

Conversation

icing commented Jul 21, 2022

Description

Testing

Checklist

wolfSSL-Bot commented Jul 21, 2022

dgarske commented Jul 21, 2022

icing commented Jul 22, 2022

dgarske commented Jul 22, 2022

icing commented Jul 25, 2022

dgarske left a comment

Choose a reason for hiding this comment

SparkiDev left a comment

Choose a reason for hiding this comment

icing commented Jul 27, 2022

icing commented Jul 27, 2022

SparkiDev commented Jul 27, 2022

icing commented Jul 28, 2022

SparkiDev commented Jul 28, 2022

dgarske commented Aug 3, 2022 • edited Loading

dgarske left a comment

Choose a reason for hiding this comment

icing commented Aug 4, 2022

dgarske left a comment

Choose a reason for hiding this comment

bagder commented Aug 9, 2022

dgarske commented Aug 3, 2022 •

edited

Loading