Skip to content

Adding quic support via wolfSSL#9290

Closed
icing wants to merge 2 commits intocurl:masterfrom
icing:quic-wolfssl
Closed

Adding quic support via wolfSSL#9290
icing wants to merge 2 commits intocurl:masterfrom
icing:quic-wolfssl

Conversation

@icing
Copy link
Copy Markdown
Contributor

@icing icing commented Aug 10, 2022

Adding quic support via wolfSSL (commit fa979230050dde47f6ac084c6f3b2…
fd88de6 
…32a5e729eb5).

- based on ngtcp2 PR ngtcp2/ngtcp2#505
- configure adapted to build against ngtcp2 wolfssl crypto lib
- quic code added for creation of WOLFSSL* instances
@bagder bagder added the HTTP/3 h3 or quic related label Aug 10, 2022
cvengler
cvengler reviewed Aug 10, 2022
View reviewed changes
lib/vquic/ngtcp2.c
result = Curl_gtls_verifyserver(data, conn, conn->quic->ssl, FIRSTSOCKET);
#elif defined(USE_WOLFSSL)
char *snihost = Curl_ssl_snihost(data, SSL_HOST_NAME(), NULL);
if(!snihost ||
Copy link
Copy Markdown
Contributor

@cvengler cvengler Aug 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this really be an OR? The latter expression makes less sense then.

Copy link
Copy Markdown
Contributor Author

@icing icing Aug 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took this from lib/vtls/wolfssl.c_616 as I thought QUIC and TLS should behave the same in this case. Have I mistunderstood?

Copy link
Copy Markdown
Member

@bagder bagder Aug 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I believe it is correct!

vszakats added a commit to curl/curl-for-win that referenced this pull request Aug 10, 2022
@vszakats
add WolfSSL support, quite broken/untested at this point [ci skip]
04f3a9c 
```
curl 7.84.0 (x86_64-w64-mingw32) libcurl/7.84.0 wolfSSL/5.4.0 (mbedTLS/3.2.1) (Schannel) zlib/1.2.11.zlib-ng brotli/1.0.9 zstd/1.5.2 c-ares/1.17.2 libidn2/2.3.3 libpsl/0.21.1 (+libidn2/2.3.3) nghttp2/1.48.0 libgsasl/1.10.0
Release-Date: 2022-06-27
Protocols: dict file ftp ftps gopher gophers http https imap imaps ldap ldaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: alt-svc AsynchDNS brotli gsasl HSTS HTTP2 IDN IPv6 Kerberos Largefile libz MultiSSL NTLM PSL SPNEGO SSL SSPI threadsafe UnixSockets zstd
```

Issues:
- libssh2 (next release, autotools only) and ngtcp2 (WIP) also support
  WolfSSL. These options weren't tested.

- WolfSSL CMake builds broke early in the process, so I only implemented
  autotools builds. These work fine.

- The custom option -DSIZEOF_LONG_LONG=8 seems to be necessary when
  compiling against WolfSSL, otherwise curl build breaks with:
  ```
  In file included from curl_ntlm_core.c:66:
  In file included from ../../wolfssl/x64-ucrt/usr/include/wolfssl/openssl/md5.h:32:
  In file included from ../../wolfssl/x64-ucrt/usr/include/wolfssl/wolfcrypt/hash.h:29:
  ../../wolfssl/x64-ucrt/usr/include/wolfssl/wolfcrypt/types.h:1118:10: error: "bad math long / long long settings"
          #error "bad math long / long long settings"
           ^
  ../../wolfssl/x64-ucrt/usr/include/wolfssl/wolfcrypt/types.h:1120:5: error: use of empty enum
      };
      ^
  2 errors generated.
  make: *** [curl_ntlm_core.o] Error 1
  ```
  Maybe this is Windows-specific and nobody else caught it, or I'm
  failing to set something up correctly. This is only necessary with
  Makefile.m32 and CMake builds, autotools defines SIZEOF_LONG_LONG
  automatically. Strangely, Makefile.m32 has it explicitly commented out
  in `lib/config-win32.h` since commit
  ae40cdf92fe05f49a146bbab5e7c9ac3e5c8c59c.

- Had to fix fallout caused by nghttp3/ngtcp2 left enabled without any
  OpenSSL fork enabled. curl CMake and autotools doesn't handle this
  gracefully (only Makefile.m32 does), so manage this from these scripts.
  Ref: b11e60245469646b0dd981c2eff828e51940f329

- Future fallout: Makefile.m32 assumes that nghttp3/ngtcp2 always
  comes together with an OpenSSL fork. This will stop being true
  after enabling HTTP/3 with WolfSSL. Will be fixed in a future
  commit.

- When WolfSSL is enabled together with BoringSSL (and possibly with
  other OpenSSL forks), the build fails in lib/curl_ntlm_core.c because
  the curl code is inconsistent in choosing one of them, ending up
  including both WolfSSL's OpenSSL compatibility headers and the real
  ones and colliding. With that patched, build will fail in wolfssl.c
  because it includes quic.h, which ends up including BoringSSL headers
  which then collide yet again with WolfSSL's. This applies only to
  OpenSSL forks with HTTP/3 support.

- Certain non-default WolfSSL build options had to be enable to compile
  and link with curl: --enable-des3 --enable-krb (for DES_ECB)
  --enable-opensslextra --enable-ripemd. Replaced these with
  --enable-curl.

- WolfSSL uses a unusual solution to provide its source tarball
  signature. Contrary to the majority of packages, it uses a different
  base URL for the tarball and the signature. Usually the signatures are
  the tarball URL plus a suffix. This made it necessary to add and
  maintain extra logic to deal with that.

- WolfSSL distributes its sources via its official homepage through
  secret URLs requiring browser interactions. Could not figure out how
  to automate those, but today found out that the source is also
  available on GitHub.

Ref: curl/curl#9290
@icing
Copy link
Copy Markdown
Contributor Author

icing commented Aug 12, 2022

I have a question about the handling of a client cert. I could not find it in the vtls case for wolfSSL and the vquic code in ngtcp2.c:815 is missing its wolfSSL equivalent. Any ideas?

@bagder
Copy link
Copy Markdown
Member

bagder commented Aug 14, 2022

Thanks!

@bagder bagder closed this in 8a13be2 Aug 14, 2022
@vszakats
Copy link
Copy Markdown
Member

vszakats commented Aug 15, 2022 •
edited
Loading

Bumped into the error below while testing a wolfSSL build with H3:

vquic/ngtcp2.c:418:9: error: use of undeclared identifier 'wolfssl_logging'
  (void)wolfssl_logging;
        ^
1 error generated.

Deleting the line above fixes it. Then remains this new warning:

vquic/ngtcp2.c:214:13: warning: unused function 'keylog_callback' [-Wunused-function]
static void keylog_callback(const WOLFSSL *ssl, const char *line)
            ^
1 warning generated.

@bagder
Copy link
Copy Markdown
Member

bagder commented Aug 15, 2022

My fault.

bagder added a commit that referenced this pull request Aug 15, 2022
@bagder
ngtcp2: remove leftover variable
c031ec5 
Mistake leftover from my edit before push.

Follow-up from 8a13be2
Reported-by: Viktor Szakats
Bug: #9290 (comment)
@bagder bagder mentioned this pull request Aug 15, 2022
Closed
jquepi pushed a commit to jquepi/curl.1.555 that referenced this pull request Oct 24, 2022
@bagder @jquepi
ngtcp2: remove leftover variable
a24d2db 
Mistake leftover from my edit before push.

Follow-up from 8a13be2
Reported-by: Viktor Szakats
Bug: curl/curl#9290 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bagder bagder bagder left review comments

+1 more reviewer

@cvengler cvengler cvengler left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

HTTP/3 h3 or quic related

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@icing @bagder @vszakats @cvengler