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

Resolve "Default Ciphers" Issue #16

Closed
wants to merge 2 commits into from
Closed

Conversation

ehlewis
Copy link

@ehlewis ehlewis commented Nov 25, 2023

Description

urllib3>2 does not contain the attribute urllib3.util.ssl_.DEFAULT_CIPHERS anymore
In lieu of limiting the requirements file to urllib3<2, a code change was requested that would work with all versions
The solution is to add an SSL context adapter to the requests session that specifies the cipher suites that we would like to allow

There were two options, some details about them can be found here, the first is that we could use requests.packages.urllib3.util.ssl_.create_urllib3_context
However, we already have some code (which happens to be the code that is breaking) that applies logic in case the module requests.packages.urllib3.util.ssl_ doesn't exist. As such, we shouldn't use this in case it doesn't exist. A side benefit is that we can remove the logic which was added to account for this
Luckily, we have an analogous function ssl.create_default_context upon which we can enforce our custom cipher suite list and then mount it into the requests session. Some similar usage can be found in this issue this module does require importing ssl, but this is a built-in and does not require extra dependencies be installed

As for the cipher suite list, it was copied from the original source code in urllib3 and was appended with the extra suites in this project that were addressing the 'dh key too small' error

Fixes #7

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Tested examples in Readme for application functionality

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

@rfc-st
Copy link
Owner

rfc-st commented Nov 25, 2023

Hi, @ehlewis

Thanks for your contribution!: I have to test it, and see if it solves the problem with current versions of urllib3, request, etc while maintaining compatibility with previous versions of these libraries.

I'll check it as soon as I can and let you know.

Have a nice weekend!.

Best regards

@rfc-st
Copy link
Owner

rfc-st commented Jan 13, 2024

Hi,

I am closing this PR as I have had to make several adaptations on the code you suggest. Thank you, of course, for your time.

I will mention you in the "Acknowledgements" section of the README.

Best regards,

@rfc-st rfc-st closed this Jan 13, 2024
rfc-st added a commit that referenced this pull request Jan 13, 2024
@rfc-st rfc-st added bug Something isn't working enhancement New feature or request labels Oct 23, 2024
@rfc-st rfc-st self-assigned this Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AttributeError: module 'urllib3.util.ssl_' has no attribute 'DEFAULT_CIPHERS'
2 participants