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

Regression in SecureTransport #10227

Closed
hjmallon opened this issue Jan 4, 2023 · 1 comment
Closed

Regression in SecureTransport #10227

hjmallon opened this issue Jan 4, 2023 · 1 comment
Assignees

Comments

@hjmallon
Copy link

hjmallon commented Jan 4, 2023

I did this

This is reproduceable with curl and libcurl. I originally saw it in libcurl but I bisected it in curl.

curl https://google.co.uk/

# Sometimes you will get (with 7.87)
curl: (35) Internal SSL engine error encountered during the SSL handshake

# Sometimes you will get the correct response (with 7.87)
...301 Moved...

# However with curl 7.47 it works every time.

I expected the following

It to work every time like 7.47 does.

curl/libcurl version

% ./src/curl -V
curl 7.87.0-DEV (x86_64-apple-darwin22.2.0) libcurl/7.87.0-DEV SecureTransport zlib/1.2.11 brotli/1.0.9 zstd/1.5.2 libidn2/2.3.4 nghttp2/1.51.0 librtmp/2.3
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher gophers http https imap imaps ldap ldaps mqtt pop3 pop3s rtmp rtsp smb smbs smtp smtps telnet tftp
Features: alt-svc AsynchDNS brotli HSTS HTTP2 HTTPS-proxy IDN IPv6 Largefile libz NTLM NTLM_WB SSL threadsafe UnixSockets zstd

operating system

Darwin pixmac570 22.2.0 Darwin Kernel Version 22.2.0: Fri Nov 11 02:08:47 PST 2022; root:xnu-8792.61.2~4/RELEASE_X86_64 x86_64

git bisect

I have done a git bisect with

autoreconf -fi
./configure --with-secure-transport --without-openssl
make -j10
./src/curl https://google.co.uk/

and it says the first bad commit it

commit 55807e6c056f27846d70cec70ee6ac3f0e5b3bbe
Author: Stefan Eissing <stefan@eissing.org>
Date:   Fri Nov 25 14:06:43 2022 +0100

    tls: backends use connection filters for IO, enabling HTTPS-proxy
    
     - OpenSSL (and compatible)
     - BearSSL
     - gnutls
     - mbedtls
     - rustls
     - schannel
     - secure-transport
     - wolfSSL (v5.0.0 and newer)
    
     This leaves only the following without HTTPS-proxy support:
     - gskit
     - nss
     - wolfSSL (versions earlier than v5.0.0)
    
    Closes #9962

 CMakeLists.txt       |   5 +-
 configure.ac         |  13 ++-
 lib/cfilters.c       |  31 +++++-
 lib/cfilters.h       | 115 ++++++++++++----------
 lib/connect.c        |  10 --
 lib/ftp.c            |   2 +-
 lib/http_proxy.c     |   8 --
 lib/imap.c           |   2 +-
 lib/pop3.c           |   2 +-
 lib/smtp.c           |   2 +-
 lib/vtls/bearssl.c   |  26 ++---
 lib/vtls/gtls.c      |  97 ++++++++++--------
 lib/vtls/mbedtls.c   |  50 ++++++++--
 lib/vtls/openssl.c   | 270 +++++++++++++++++++++++++++++++++++++++++----------
 lib/vtls/rustls.c    |  76 ++++++++++-----
 lib/vtls/schannel.c  |  44 ++++-----
 lib/vtls/sectransp.c | 151 +++++++++-------------------
 lib/vtls/vtls.c      | 104 +++++++++-----------
 lib/vtls/vtls.h      |  12 ---
 lib/vtls/vtls_int.h  |  13 ++-
 lib/vtls/wolfssl.c   | 179 +++++++++++++++++++++++++++++++++-
 m4/curl-wolfssl.m4   |   9 ++
 22 files changed, 796 insertions(+), 425 deletions(-)
icing added a commit to icing/curl that referenced this issue Jan 5, 2023
SecureTransport expects result code errSSLWouldBlock when the requested length
could not be sent/recieved in full. The previous code returned noErr, which let
SecureTransport to believe that the IO had terminated prematurely.
@icing
Copy link
Contributor

icing commented Jan 5, 2023

@hjmallon would you be able to check the change in #10235? In my local tests, this solves the problem.

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

Successfully merging a pull request may close this issue.

3 participants