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

Fb 001 #4

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Fb 001 #4

wants to merge 9 commits into from

Conversation

B3K7
Copy link

@B3K7 B3K7 commented Jan 14, 2022

Changes

  • forced tls1.2+
  • forced perfect forward secrecy

B3K7 added 4 commits January 11, 2022 09:41
enabled TLSv1_2 and TLSv1_3
enabled perfect forward secrecy ciphers
bumped version to 0.8.8
Changes
- Force TLS1.2+
- Force perfect forward secrecy
@B3K7
Copy link
Author

B3K7 commented Jan 14, 2022

addresses open issue #3

B3K7 added 2 commits January 14, 2022 10:50
removed change to _query method to simplify code review
To work around missing SSLContext.minimum_version logic.
@B3K7
Copy link
Author

B3K7 commented Jan 14, 2022

@aaront This looks like a false positive. Can you please review and/or update the 2.7 test suite?

@B3K7
Copy link
Author

B3K7 commented Jan 14, 2022

Hi @aaront. Your test case appears to be failing trying to establish an SSLv3 session. This is a false positive. The test should fail. Please advise.

B3K7 added 2 commits January 14, 2022 17:00
Reduced the number of touched files.
Downgraded from P-521 to P-384.
  secp521r1 : NIST/SECG curve over a 521 bit prime field
  secp384r1 : NIST/SECG curve over a 384 bit prime field
@B3K7
Copy link
Author

B3K7 commented Jan 14, 2022

@aaront. I could be wrong but it looks like the test suite is still borken.

requests.exceptions.SSLError: HTTPSConnectionPool(host='my.geotab.com', port=443): Max retries exceeded with url: /apiv1 (Caused by SSLError(SSLError(1, '[SSL: SSLV3_ALERT_HANDSHAKE_FAILURE] sslv3 alert handshake failure (_ssl.c:1131)')))

@aaront
Copy link
Contributor

aaront commented Jan 15, 2022

@B3K7 not sure it's the test suite, I made some changes that work in c0f71cd and seems to handle the recommendation from the Python SSL docs to not specify a version of TLS, but rather use ssl.PROTOCOL_TLS instead which uses the highest protocol that both the server and client machine support.

I'm a bit hesitant specifying ciphers in the API call itself. It was under my impression that the server and client will use the best available, and maybe by no longer specifying TLS 1.2 as the SSL protocol, it may be more strict if you're using TLS 1.3.

Why what was enabled/disabled.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants