Skip to content

session: add support for urllib3 2.0 #5326

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 5, 2023
Merged

session: add support for urllib3 2.0 #5326

merged 1 commit into from
May 5, 2023

Conversation

bastimeyer
Copy link
Member

@bastimeyer bastimeyer commented May 4, 2023 •
edited
Loading

  • Bump max version constraint from <2 to <3
  • Fix _PERCENT_RE constant name in URL percent encoding override
  • Fix StreamlinkOptions._set_http_disable_dh(), remove the override of the old urllib3.util._ssl.DEFAULT_CIPHERS list and instead add streamlink.plugin.api.http_session.TLSNoDHAdapter which reads the ciphers returned by Python's default SSLContext, adds :!DH to the updated ciphers list, and finally replaces the HTTP session's https:// adapter
  • Use urllib3.util.create_urllib3_context() for creating SSLContext objects in custom HTTPAdapters

Resolves #5324

urllib3 migration guide here:
https://urllib3.readthedocs.io/en/stable/v2-migration-guide.html

@bastimeyer bastimeyer added dependencies WIP Work in process labels May 4, 2023
@bastimeyer bastimeyer force-pushed the build/urllib3-2.0 branch from 3c77437 to 8342d40 Compare May 4, 2023 17:19
@bastimeyer bastimeyer requested a review from mkbloke May 4, 2023 17:21
@bastimeyer
Copy link
Member Author

@mkbloke could you please help test the TLSNoDHAdapter? I haven't found a site to test this with yet. https://badssl.com is of no use here.

@bastimeyer
Copy link
Member Author 

$ streamlink --stdout 'httpstream://https://dh1024.badssl.com/' best >/dev/null
[cli][info] Found matching plugin http for URL httpstream://https://dh1024.badssl.com/
[cli][info] Available streams: live (worst, best)
[cli][info] Opening stream: live (http)
[cli][error] Try 1/1: Could not open stream <HTTPStream ['http', 'https://dh1024.badssl.com/']> (Could not open stream: Unable to open URL: https://dh1024.badssl.com/ (HTTPSConnectionPool(host='dh1024.badssl.com', port=443): Max retries exceeded with url: / (Caused by SSLError(SSLError(1, '[SSL: DH_KEY_TOO_SMALL] dh key too small (_ssl.c:1002)')))))
error: Could not open stream <HTTPStream ['http', 'https://dh1024.badssl.com/']>, tried 1 times, exiting

 streamlink --http-disable-dh --stdout 'httpstream://https://dh1024.badssl.com/' best >/dev/null
[cli][info] Found matching plugin http for URL httpstream://https://dh1024.badssl.com/
[cli][info] Available streams: live (worst, best)
[cli][info] Opening stream: live (http)
[cli][error] Try 1/1: Could not open stream <HTTPStream ['http', 'https://dh1024.badssl.com/']> (Could not open stream: Unable to open URL: https://dh1024.badssl.com/ (HTTPSConnectionPool(host='dh1024.badssl.com', port=443): Max retries exceeded with url: / (Caused by SSLError(SSLError(1, '[SSL: SSLV3_ALERT_HANDSHAKE_FAILURE] sslv3 alert handshake failure (_ssl.c:1002)')))))
error: Could not open stream <HTTPStream ['http', 'https://dh1024.badssl.com/']>, tried 1 times, exiting

@bastimeyer bastimeyer force-pushed the build/urllib3-2.0 branch 4 times, most recently from a5ace12 to 5941e7c Compare May 4, 2023 18:00
@bastimeyer bastimeyer removed the WIP Work in process label May 4, 2023
@bastimeyer
Copy link
Member Author

I don't see any further urllib3 incompabilities.

  • Override for URL percent encoding
    Fixed
  • Override for content length verification in streaming mode
    Always enabled now in 2.0.0
  • --interface:
    Working, no changes required.
  • --ipv4 / --ipv6:
    Working, no changes required.
  • --http-disable-dh
    Not sure if it's still working, but that's probably because of recent OpenSSL versions. I don't know enough about that.
    The old implementation updated the DEFAULT_CIPHERS list which was used when creating the SSLContext. The new HTTPAdapter which I've added basically does the same, but it gets the default ciphers from the SSLContext created from the same method (both pre and post 2.0) and it updates the ciphers list again afterwards. The adapter then replaces the default adapter for the https:// scheme
  • Other --http-* options:
    Using the APIs of requests, so no changes required here.

urllib3 2.0.0 however now sets TLS1.2 as the min SSL/TLS version, up from TLS1.0:
https://urllib3.readthedocs.io/en/stable/v2-migration-guide.html#modern-security-by-default
It's possible that some plugins will break due to this. I don't see a problem with that though. However, we already have the TLSSecLevel1Adapter, which at least one plugin is using (filmon), and that adapter downgrades to SSLv3. So if there are any plugin issues where we don't want to remove the plugin, a new adapter could be added or the existing one could be changed with a customizable level for bringing back support for TLS1.2 (level=4).
https://www.openssl.org/docs/man3.0/man3/SSL_CTX_set_security_level.html
But IIRC, the downgrades do only work on py39 and below anyway, because py310 prevents this.

@bastimeyer
session: add support for urllib3 2.0
1b30ad9 
- Bump max version constraint from `<2` to `<3`
- Fix `_PERCENT_RE` constant name in URL percent encoding override
- Fix `StreamlinkOptions._set_http_disable_dh()`, remove the override of
  the old `urllib3.util._ssl.DEFAULT_CIPHERS` list and instead add
  `streamlink.plugin.api.http_session.TLSNoDHAdapter` which reads
  the ciphers returned by Python's default SSLContext, adds `:!DH` to
  the updated ciphers list, and finally replaces the HTTP session's
  `https://` adapter
- Use `urllib3.util.create_urllib3_context()` for creating `SSLContext`
  objects in custom `HTTPAdapter`s
@bastimeyer bastimeyer force-pushed the build/urllib3-2.0 branch from 5941e7c to 1b30ad9 Compare May 4, 2023 19:05
@bastimeyer bastimeyer changed the title build: add support for urllib3 2.0 session: add support for urllib3 2.0 May 4, 2023
@bastimeyer bastimeyer mentioned this pull request May 4, 2023
Merged
@bastimeyer bastimeyer merged commit fac79a5 into streamlink:master May 5, 2023
@bastimeyer bastimeyer deleted the build/urllib3-2.0 branch May 5, 2023 03:54
@mkbloke
Copy link
Member

mkbloke commented May 5, 2023

I did spend quite a bit of time last night looking at this, but have not as yet found anything too helpful.

@bastimeyer bastimeyer mentioned this pull request May 7, 2023
Closed
4 tasks
@cgomesu cgomesu mentioned this pull request May 11, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkbloke mkbloke Awaiting requested review from mkbloke

Assignees
No one assigned
Labels
dependencies
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AttributeError: module 'urllib3.util.url' has no attribute 'PERCENT_RE'
2 participants
@bastimeyer @mkbloke