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
SSL tests fail with uncaught SSL validation #173
Comments
It appears that those tests are attempting to catch the SSL Exception, but |
I see linux is also affected. |
Looks like tests passed 17 days ago. At the time, these were the deps installed:
|
Yeah... About that. It looks like different combos of I was trying to identify certain patterns but it's not usually obvious what's going on. Sometime test would behave differently under the seemingly similar env (like real macbook vs Travis CI vs Circle CI with the same version of macOS and openssl lib). So currently I'm trying to figure this out time to time when possible and then give up periodically... I also noticed that upgrade of |
Here we have one of the early failures where here are the deps installed:
|
One of the issues is that currently code catches and ignores "safe" exceptions during the initial TLS handshake (i.e. on connect) but I saw those happenning on the later stage (basically when we're trying to read from the network stream) as well in some cases. |
I tried downgrading to pyopenssl 18, but that didn't help. |
maybe some transitive dependency upgrade + it also depends on the actual openssl backend |
Oh and this "combo" issue thing is kinda complicated: the stdlib |
Installing the full set of deps from that successful build does work around the issue:
|
And PyPy has its own CFFI based wrapper for OpenSSL while CPython has that in pure C and cryptography also does pure C thing I think. |
Okay, from diff it seems that it's |
According to the changelog, cryptography 2.5 introduced this change:
Based on your other assertions above, this sounds like a likely proximate cause. |
@jaraco you can try this to check actual versions of stuff: python -m OpenSSL.debug |
I've actually made CI print it a while back: |
Really annoying that requests would raise different exceptions depending on the openssl version present in cryptography. |
I wonder if there's a way to test this behavior outside of cheroot - using just requests and cryptography versions against some known SSL endpoint with an unknown CA. |
Running with simple requests and cryptography 2.5, I see that 'requests' raises the expected
So the problem seems to have something to do with cheroot. |
Well, it's actually not requests itself, but underlying libraries behaving like this. I've integrated trustme to have our own fake CA and it's very easy to use + I'm pretty sure it works correctly. OTOH because implementation differences and a lot of env factors (like I mentioned above) it's kinda hard to cover all cases. In our tests I saw for one "logical" case of wrong setup the underlying library would sometimes return an alert from TLS or SSL, for example, which have different codes because they are from different namespaces. |
Yeah, I'm pretty sure my test above isn't accurately exercising the behavior. I'm not passing |
Well, I'm stumped. I can't tell what factors are leading to requests not wrapping the underlying error... to implicate requests, or what factors cheroot could possibly do to mitigate the error. I did try trapping both requests.exceptions.SSLError and OpenSSL.SSL.Error thus: diff --git a/cheroot/test/test_ssl.py b/cheroot/test/test_ssl.py
index 4708e23f..b628c36c 100644
--- a/cheroot/test/test_ssl.py
+++ b/cheroot/test/test_ssl.py
@@ -286,7 +286,11 @@ def test_tls_client_auth(
assert resp.text == 'Hello world!'
return
- with pytest.raises(requests.exceptions.SSLError) as ssl_err:
+ expected = (
+ requests.exceptions.SSLError,
+ OpenSSL.SSL.Error,
+ )
+ with pytest.raises(expected) as ssl_err:
make_https_request()
err_text = ssl_err.value.args[0].reason.args[0].args[0] But that didn't work as the follow-up assertions on err_text fail. I need to punt on this error as I'm trying to investigate other issues. I suggest we either mark these tests as xfail or proceed with #174 (pin cryptography) until someone has time to investigate further (or the problem is discovered and fixed elsewhere). @webknjaz Any preference on those two options ^? |
I'd go for |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
It looks like |
|
β I'm submitting a ...
π Describe the bug. What is the current behavior?
Running tests on macOS, two tests fail:
β What is the motivation / use case for changing the behavior?
Tests should pass on master.
π‘ To Reproduce
Steps to reproduce the behavior:
tox -r
on macOSπ‘ Expected behavior
Tests should pass.
π Details
Failures look like:
π Environment
π Additional context
The text was updated successfully, but these errors were encountered: