Update to rustls-ffi 0.10.0 #10865

jsha · 2023-03-29T21:04:47Z

This brings in version 0.21.0 of the upstream rustls implementation, which notable includes support for IP address certificates. This allows enabling many of the unittests that relied on IP address certificates to communicate with localhost.

jsha · 2023-03-29T21:42:37Z

Interesting. The test failure is under address_sanitizer, but appears to be a failed assertion indicating that a connection was reused more than expected (one connect vs two).

=================================== FAILURES ===================================
_________________ TestDownload.test_02_07_download_reuse[0-h2] _________________

self = <test_02_download.TestDownload object at 0x7f[29](https://github.com/curl/curl/actions/runs/4557958324/jobs/8040275432?pr=10865#step:24:30)b0feab90>
env = <testenv.env.Env object at 0x7f29b0e3d720>
httpd = <testenv.httpd.Httpd object at 0x7f29b0ffeb90>
nghttpx = <testenv.nghttpx.Nghttpx object at 0x7f29b0ffd1b0>, repeat = 0
proto = 'h2'

    @pytest.mark.parametrize("proto", ['h2', 'h3'])
    def test_02_07_download_reuse(self, env: Env,
                                  httpd, nghttpx, repeat, proto):
        if proto == 'h3' and not env.have_h3():
            pytest.skip("h3 not supported")
        count = 200
        curl = CurlClient(env=env)
        urln = f'https://{env.authority_for(env.domain1, proto)}/data.json?[0-{count-1}]'
        r = curl.http_download(urls=[urln], alpn_proto=proto,
                               with_stats=True, extra_args=[
            '--parallel', '--parallel-max', '200'
        ])
        r.check_exit_code(0)
        r.check_stats(count=count, exp_status=200)
        # should have used 2 connections only (test servers allow 100 req/conn)
>       assert r.total_connects == 2, "h2 should use fewer connections here"
E       AssertionError: h2 should use fewer connections here
E       assert 1 == 2
E        +  where 1 = ExecResult[code=0, exception=None, args=['/home/runner/work/curl/curl/src/curl', '-s', '--path-as-is', '-v', '--http2'... done: 0\n', '* Connection #0 to host one.http.curl.se left intact\n', '* Expire cleared (transfer 0x6220001[30](https://github.com/curl/curl/actions/runs/4557958324/jobs/8040275432?pr=10865#step:24:31)908)\n']].total_connects

tests/http/test_02_download.py:163: AssertionError
=========================== short test summary info ============================
FAILED tests/http/test_02_download.py::TestDownload::test_02_07_download_reuse[0-h2] - AssertionError: h2 should use fewer connections here
assert 1 == 2
 +  where 1 = ExecResult[code=0, exception=None, args=['/home/runner/work/curl/curl/src/curl', '-s', '--path-as-is', '-v', '--http2'... done: 0\n', '* Connection #0 to host one.http.curl.se left intact\n', '* Expire cleared (transfer 0x622000130908)\n']].total_connects
============= 1 failed, 67 passed, [52](https://github.com/curl/curl/actions/runs/4557958324/jobs/8040275432?pr=10865#step:24:53) skipped in 62.67s (0:01:02) ==============

jsha · 2023-03-29T23:05:24Z

@icing I see you've been working on the pytests. Any idea why this would fail just on address_sanitizer?

icing · 2023-03-30T06:45:53Z

@icing I see you've been working on the pytests. Any idea why this would fail just on address_sanitizer?

First guess: the curl client becomes so slow, that apache answers the requests before the first connection runs out of space. So, a second one is never needed.

Hmm...we could change the assert to <= 2.

Added #10870 for exactly this.

bagder · 2023-03-30T21:53:29Z

Closed by mistake, sorry

pheiduck · 2023-04-03T20:24:47Z

This tests are still fail:

TESTFAIL: These test cases failed: 400 401 403 406 407 408 409 987 988 989 1112

bagder

Tests are failing

jsha · 2023-04-26T23:24:40Z

Yep, have been meaning to look into this more. Apologies for the delay.

jsha · 2023-06-23T19:07:38Z

I'm still working on diagnosing why those tests are failing but it turns out to not be IP address support. I've reverted the diffs to DISABLED so this now just bumps the ruslts-ffi dependency.

This brings in version 0.21.0 of the upstream rustls implementation, which notable includes support for IP address certificates.

jsha · 2023-07-11T22:20:49Z

There are a handful of CI jobs that I see consistently failing on this branch, but I think I didn't touch anything related:

c-ares
LibreSSL http2
cmake clang LibreSSL
cmake gcc-12 LibreSSL

Here's some example output from cmake clang LibreSSL:

ld: warning: dylib (/usr/local/lib/libssh2.dylib) was built for newer macOS version (12.0) than being linked (10.15)
Undefined symbols for architecture x86_64:
  "_EVP_PKEY_get_bn_param", referenced from:
      _Curl_ossl_certchain in unity_0_c.c.o
  "_EVP_PKEY_get_id", referenced from:
      _Curl_ossl_certchain in unity_0_c.c.o
      _cert_stuff in unity_0_c.c.o

Only 1 connection will be used when curl is slow, happens when address-sanitized in CI, for example Closes curl#10865

bagder · 2023-07-22T21:38:56Z

The libressl problems were due to a home brew problem. Rebase and push again and they should be fixed.

bagder · 2023-07-22T21:41:19Z

(never mind) Thanks!

This brings in version 0.21.0 of the upstream rustls implementation, which notable includes support for IP address certificates. Closes curl#10865

github-actions bot added CI Continuous Integration tests labels Mar 29, 2023

bagder closed this in 041cf77 Mar 30, 2023

bagder reopened this Mar 30, 2023

jsha force-pushed the rustls-ffi-0.10.0 branch from 1373bf1 to 008db1f Compare April 3, 2023 19:33

bagder added the TLS label Apr 10, 2023

bagder requested changes Apr 26, 2023

View reviewed changes

jsha force-pushed the rustls-ffi-0.10.0 branch from 008db1f to 342b7c5 Compare June 23, 2023 19:04

jsha force-pushed the rustls-ffi-0.10.0 branch from 342b7c5 to 13da05e Compare June 28, 2023 16:48

jsha force-pushed the rustls-ffi-0.10.0 branch from 13da05e to ae5ee34 Compare July 11, 2023 21:17

Update to rustls-ffi 0.10.0

e17a338

This brings in version 0.21.0 of the upstream rustls implementation, which notable includes support for IP address certificates.

jsha force-pushed the rustls-ffi-0.10.0 branch from ae5ee34 to e17a338 Compare July 11, 2023 21:29

bch pushed a commit to bch/curl that referenced this pull request Jul 19, 2023

tests/http: relax connection check in test_07_02

8399f42

Only 1 connection will be used when curl is slow, happens when address-sanitized in CI, for example Closes curl#10865

bagder closed this in 69c536b Jul 22, 2023

ptitSeb pushed a commit to wasix-org/curl that referenced this pull request Sep 25, 2023

rustls: update rustls-ffi 0.10.0

95294eb

This brings in version 0.21.0 of the upstream rustls implementation, which notable includes support for IP address certificates. Closes curl#10865

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to rustls-ffi 0.10.0 #10865

Update to rustls-ffi 0.10.0 #10865

jsha commented Mar 29, 2023

jsha commented Mar 29, 2023

jsha commented Mar 29, 2023

icing commented Mar 30, 2023 •

edited

bagder commented Mar 30, 2023

pheiduck commented Apr 3, 2023

bagder left a comment

jsha commented Apr 26, 2023

jsha commented Jun 23, 2023

jsha commented Jul 11, 2023

bagder commented Jul 22, 2023 •

edited

bagder commented Jul 22, 2023

Update to rustls-ffi 0.10.0 #10865

Update to rustls-ffi 0.10.0 #10865

Conversation

jsha commented Mar 29, 2023

jsha commented Mar 29, 2023

jsha commented Mar 29, 2023

icing commented Mar 30, 2023 • edited

bagder commented Mar 30, 2023

pheiduck commented Apr 3, 2023

bagder left a comment

Choose a reason for hiding this comment

jsha commented Apr 26, 2023

jsha commented Jun 23, 2023

jsha commented Jul 11, 2023

bagder commented Jul 22, 2023 • edited

bagder commented Jul 22, 2023

icing commented Mar 30, 2023 •

edited

bagder commented Jul 22, 2023 •

edited