openwrt / packages

Community maintained packages for OpenWrt. Documentation for submitting pull requests is in CONTRIBUTING.md
GNU General Public License v2.0
3.88k stars 3.39k forks source link

curl: upstream backports for mbedtls #24414

Closed Ra2-IFV closed 1 week ago

Ra2-IFV commented 1 week ago

Maintainer: @krant Compile tested: aarch64 531b3f667c40405581a398a9648a0e6c27909a87 Run tested: aarch64 531b3f667c40405581a398a9648a0e6c27909a87, just simple connection test

Description: tlsv1.3 support is broken in curl 8.8.0 with mbedtls 3.6.0.
See https://github.com/curl/curl/issues/13653 and https://github.com/Mbed-TLS/mbedtls/issues/9210 for more details.
A workaround was implemented in upsteam codes, see https://github.com/curl/curl/commit/0c4b4c1e93c8e869af230090f32346fdfd548f21 and https://github.com/curl/curl/commit/5f9017d4e28096b4589c4d5ee4f18cae086ba777
This commit includes patches generated from upstream commits.

Fixes #24365 #24386

Ra2-IFV commented 1 week ago
root@OpenWrt:~# curl -vSL https://raw.githubusercontent.com/just_for_test
> GET /just_for_test HTTP/2
> Host: raw.githubusercontent.com
> User-Agent: curl/8.9.0-20240618
> Accept: */*
>
< HTTP/2 400
< content-security-policy: default-src 'none'; style-src 'unsafe-inline'; sandbox
< strict-transport-security: max-age=31536000
< x-content-type-options: nosniff
< x-frame-options: deny
< x-xss-protection: 1; mode=block
< content-type: text/plain; charset=utf-8
< x-github-request-id: C1D0:2A8DB3:283E3C:2EE34E:66720D77
< accept-ranges: bytes
< date: Tue, 18 Jun 2024 22:43:04 GMT
< via: 1.1 varnish
< x-served-by: cache-nrt-rjtf7700064-NRT
< x-cache: MISS
< x-cache-hits: 0
< x-timer: S1718750584.917578,VS0,VE303
< vary: Authorization,Accept-Encoding,Origin
< access-control-allow-origin: *
< cross-origin-resource-policy: cross-origin
< x-fastly-request-id: b66d70ce95c51f125c6f9f1ef7fa1521e273e96c
< expires: Tue, 18 Jun 2024 22:48:04 GMT
< content-length: 20
<
400: Invalid requestroot@OpenWrt:~#
root@OpenWrt:~# curl -V
curl 8.9.0-20240618 (aarch64-openwrt-linux-gnu) libcurl/8.9.0-20240618 mbedTLS/3.6.0 zlib/1.3.1 zstd/1.5.6 nghttp2/1.62.1
Release-Date: 2024-06-18
Protocols: file ftp ftps http https ipfs ipns mqtt
Features: alt-svc HSTS HTTP2 HTTPS-proxy IPv6 Largefile libz SSL threadsafe UnixSockets zstd
root@OpenWrt:~#
krant commented 1 week ago

Thank you for this. My only concern is the possibility next curl could be 8.8.1 - in this case our version ordering will be broke.

Ra2-IFV commented 1 week ago

The future version number was manually set in the snapshots, so it *should* be fine
Check it out http://curl.se/snapshots/

krant commented 1 week ago

Makes sense. LGTM.

1715173329 commented 1 week ago

You should back port those fixes then, not bump to a unstable version.

Ra2-IFV commented 1 week ago

I tried to patch it but build fails. At least unstable is better than totally broken...

1715173329 commented 1 week ago
diff --git a/net/curl/Makefile b/net/curl/Makefile
index d62712a2c3..4e2406d915 100644
--- a/net/curl/Makefile
+++ b/net/curl/Makefile
@@ -10,7 +10,7 @@ include $(INCLUDE_DIR)/nls.mk

 PKG_NAME:=curl
 PKG_VERSION:=8.8.0
-PKG_RELEASE:=1
+PKG_RELEASE:=2

 PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.xz
 PKG_SOURCE_URL:=https://github.com/curl/curl/releases/download/curl-$(subst .,_,$(PKG_VERSION))/ \
diff --git a/net/curl/patches/010-mbedtls-check-version-for-cipher-id.patch b/net/curl/patches/010-mbedtls-check-version-for-cipher-id.patch
new file mode 100644
index 0000000000..4bdf7c550b
--- /dev/null
+++ b/net/curl/patches/010-mbedtls-check-version-for-cipher-id.patch
@@ -0,0 +1,48 @@
+From 0c4b4c1e93c8e869af230090f32346fdfd548f21 Mon Sep 17 00:00:00 2001
+From: Stefan Eissing <stefan@eissing.org>
+Date: Wed, 22 May 2024 14:44:56 +0200
+Subject: [PATCH] mbedtls: check version for cipher id
+
+mbedtls_ssl_get_ciphersuite_id_from_ssl() seems to have been added in
+mbedtls 3.2.0. Check for that version.
+
+Closes #13749
+---
+ lib/vtls/mbedtls.c | 19 ++++++++++++-------
+ 1 file changed, 12 insertions(+), 7 deletions(-)
+
+--- a/lib/vtls/mbedtls.c
++++ b/lib/vtls/mbedtls.c
+@@ -902,8 +902,6 @@ mbed_connect_step2(struct Curl_cfilter *
+     (struct mbed_ssl_backend_data *)connssl->backend;
+   struct ssl_primary_config *conn_config = Curl_ssl_cf_get_primary_config(cf);
+   const mbedtls_x509_crt *peercert;
+-  char cipher_str[64];
+-  uint16_t cipher_id;
+ #ifndef CURL_DISABLE_PROXY
+   const char * const pinnedpubkey = Curl_ssl_cf_is_proxy(cf)?
+     data->set.str[STRING_SSL_PINNEDPUBLICKEY_PROXY]:
+@@ -932,11 +930,18 @@ mbed_connect_step2(struct Curl_cfilter *
+     return CURLE_SSL_CONNECT_ERROR;
+   }
+ 
+-  cipher_id = (uint16_t)
+-              mbedtls_ssl_get_ciphersuite_id_from_ssl(&backend->ssl);
+-  mbed_cipher_suite_get_str(cipher_id, cipher_str, sizeof(cipher_str), true);
+-  infof(data, "mbedTLS: Handshake complete, cipher is %s", cipher_str);
+-
++#if MBEDTLS_VERSION_NUMBER >= 0x03020000
++  {
++    char cipher_str[64];
++    uint16_t cipher_id;
++    cipher_id = (uint16_t)
++                mbedtls_ssl_get_ciphersuite_id_from_ssl(&backend->ssl);
++    mbed_cipher_suite_get_str(cipher_id, cipher_str, sizeof(cipher_str), true);
++    infof(data, "mbedTLS: Handshake complete, cipher is %s", cipher_str);
++  }
++#else
++  infof(data, "mbedTLS: Handshake complete");
++#endif
+   ret = mbedtls_ssl_get_verify_result(&backend->ssl);
+ 
+   if(!conn_config->verifyhost)
diff --git a/net/curl/patches/020-mbedtls-v3.6.0-workarounds.patch b/net/curl/patches/020-mbedtls-v3.6.0-workarounds.patch
new file mode 100644
index 0000000000..15966fb5ad
--- /dev/null
+++ b/net/curl/patches/020-mbedtls-v3.6.0-workarounds.patch
@@ -0,0 +1,134 @@
+From 5f9017d4e28096b4589c4d5ee4f18cae086ba777 Mon Sep 17 00:00:00 2001
+From: Stefan Eissing <stefan@eissing.org>
+Date: Fri, 31 May 2024 13:01:17 +0200
+Subject: [PATCH] mbedtls: v3.6.0 workarounds
+
+- add special sauce to disable unwanted peer verification by mbedtls
+  when negotiating TLS v1.3
+- add special sauce for MBEDTLS_ERR_SSL_RECEIVED_NEW_SESSION_TICKET
+  return code on *writing* TLS data. We assume the data had not been
+  written and EAGAIN.
+- return correct Curl error code when peer verification failed.
+- disable test_08_05 with 50 HTTP/1.1 connections, as mbedtls reports a
+  memory allocation failed during handshake.
+- bump CI mbedtls version to 3.6.0
+
+Fixes #13653
+Closes #13838
+---
+ .github/workflows/linux.yml   |  2 +-
+ lib/vtls/mbedtls.c            | 52 +++++++++++++++++++++++++++++++----
+ tests/http/test_08_caddy.py   |  3 ++
+ tests/http/test_17_ssl_use.py |  5 ++--
+ 4 files changed, 52 insertions(+), 10 deletions(-)
+
+--- a/lib/vtls/mbedtls.c
++++ b/lib/vtls/mbedtls.c
+@@ -482,6 +482,20 @@ mbed_set_selected_ciphers(struct Curl_ea
+   return CURLE_OK;
+ }
+ 
++#ifdef TLS13_SUPPORT
++static int mbed_no_verify(void *udata, mbedtls_x509_crt *crt,
++                          int depth, uint32_t *flags)
++{
++  (void)udata;
++  (void)crt;
++  (void)depth;
++  /* we clear any faults the mbedtls' own verification found.
++   * See <https://github.com/Mbed-TLS/mbedtls/issues/9210> */
++  *flags = 0;
++  return 0;
++}
++#endif
++
+ static CURLcode
+ mbed_connect_step1(struct Curl_cfilter *cf, struct Curl_easy *data)
+ {
+@@ -737,6 +751,16 @@ mbed_connect_step1(struct Curl_cfilter *
+     failf(data, "mbedTLS: ssl_config failed");
+     return CURLE_SSL_CONNECT_ERROR;
+   }
++#ifdef TLS13_SUPPORT
++  if(!verifypeer) {
++    /* Default verify behaviour changed in mbedtls v3.6.0 with TLS v1.3.
++     * On 1.3 connections, the handshake fails by default without trust
++     * anchors. We override this questionable change by installing our
++     * own verify callback that clears all errors. */
++    mbedtls_ssl_conf_verify(&backend->config, mbed_no_verify, cf);
++  }
++#endif
++
+ 
+   mbedtls_ssl_init(&backend->ssl);
+ 
+@@ -922,10 +946,16 @@ mbed_connect_step2(struct Curl_cfilter *
+     connssl->connecting_state = ssl_connect_2_writing;
+     return CURLE_OK;
+   }
++  else if(ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED) {
++    failf(data, "peer certificate could not be verified");
++    return CURLE_PEER_FAILED_VERIFICATION;
++  }
+   else if(ret) {
+     char errorbuf[128];
++    CURL_TRC_CF(data, cf, "TLS version %04X",
++                mbedtls_ssl_get_version_number(&backend->ssl));
+     mbedtls_strerror(ret, errorbuf, sizeof(errorbuf));
+-    failf(data, "ssl_handshake returned - mbedTLS: (-0x%04X) %s",
++    failf(data, "ssl_handshake returned: (-0x%04X) %s",
+           -ret, errorbuf);
+     return CURLE_SSL_CONNECT_ERROR;
+   }
+@@ -1146,8 +1176,13 @@ static ssize_t mbed_send(struct Curl_cfi
+   ret = mbedtls_ssl_write(&backend->ssl, (unsigned char *)mem, len);
+ 
+   if(ret < 0) {
+-    *curlcode = (ret == MBEDTLS_ERR_SSL_WANT_WRITE) ?
+-      CURLE_AGAIN : CURLE_SEND_ERROR;
++    CURL_TRC_CF(data, cf, "mbedtls_ssl_write(len=%zu) -> -0x%04X",
++                len, -ret);
++    *curlcode = ((ret == MBEDTLS_ERR_SSL_WANT_WRITE)
++#ifdef TLS13_SUPPORT
++      || (ret == MBEDTLS_ERR_SSL_RECEIVED_NEW_SESSION_TICKET)
++#endif
++      )? CURLE_AGAIN : CURLE_SEND_ERROR;
+     ret = -1;
+   }
+ 
+@@ -1203,16 +1238,21 @@ static ssize_t mbed_recv(struct Curl_cfi
+ 
+   ret = mbedtls_ssl_read(&backend->ssl, (unsigned char *)buf,
+                          buffersize);
+-
+   if(ret <= 0) {
++    CURL_TRC_CF(data, cf, "mbedtls_ssl_read(len=%zu) -> -0x%04X",
++                buffersize, -ret);
+     if(ret == MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY)
+       return 0;
+-
+     *curlcode = ((ret == MBEDTLS_ERR_SSL_WANT_READ)
+ #ifdef TLS13_SUPPORT
+               || (ret == MBEDTLS_ERR_SSL_RECEIVED_NEW_SESSION_TICKET)
+ #endif
+     ) ? CURLE_AGAIN : CURLE_RECV_ERROR;
++    if(*curlcode != CURLE_AGAIN) {
++      char errorbuf[128];
++      mbedtls_strerror(ret, errorbuf, sizeof(errorbuf));
++      failf(data, "ssl_read returned: (-0x%04X) %s", -ret, errorbuf);
++    }
+     return -1;
+   }
+ 
+--- a/tests/http/test_08_caddy.py
++++ b/tests/http/test_08_caddy.py
+@@ -151,6 +151,9 @@ class TestCaddy:
+             pytest.skip("h3 not supported in curl")
+         if proto == 'h3' and env.curl_uses_lib('msh3'):
+             pytest.skip("msh3 itself crashes")
++        if proto == 'http/1.1' and env.curl_uses_lib('mbedtls'):
++            pytest.skip("mbedtls 3.6.0 fails on 50 connections with: "\
++                "ssl_handshake returned: (-0x7F00) SSL - Memory allocation failed")
+         count = 50
+         curl = CurlClient(env=env)
+         urln = f'https://{env.domain1}:{caddy.port}/data10.data?[0-{count-1}]'

I don't use mbedtls, you can test if this patch works.

Ra2-IFV commented 1 week ago

looks great.
I did almost the same thing with git format-patch but curl fails with Segmentation fault(lmao). Perhaps I didn't clean the code before patching it.
so what's next, should I open a new pull request?

1715173329 commented 1 week ago

No, simply update this PR.

Ra2-IFV commented 1 week ago

Okay.

Ra2-IFV commented 1 week ago

Force pushed, updated title and description.

1715173329 commented 1 week ago

seems like only second patch is necessary

Ra2-IFV commented 1 week ago

tested and looks good.

1715173329 commented 1 week ago

Please refresh this patch https://openwrt.org/docs/guide-developer/toolchain/use-patches-with-buildsystem#refreshing_patches

Ra2-IFV commented 1 week ago

Refreshed. I don't know there is a target to refresh patches. I use make prepare QUILT=1 and refresh them manually 🥲