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

Confusing/wrong documentation for ssl #3722

Open
blueyed opened this issue May 2, 2019 · 1 comment
Open

Confusing/wrong documentation for ssl #3722

blueyed opened this issue May 2, 2019 · 1 comment
Assignees

Comments

@blueyed
Copy link
Contributor

blueyed commented May 2, 2019

The doc for TCPConnector still mentions verify_ssl:

class TCPConnector(BaseConnector):
"""TCP connector.
verify_ssl - Set to True to check ssl certifications.
fingerprint - Pass the binary sha256
digest of the expected certificate in DER format to verify
that the certificate the server presents matches. See also
https://en.wikipedia.org/wiki/Transport_Layer_Security#Certificate_pinning

The internal documentation for _get_ssl_context still refers to the removed verify_ssl:

def _get_ssl_context(self, req: 'ClientRequest') -> Optional[SSLContext]:
"""Logic to get the correct SSL context
0. if req.ssl is false, return None
1. if ssl_context is specified in req, use it
2. if _ssl_context is specified in self, use it
3. otherwise:
1. if verify_ssl is not specified in req, use self.ssl_context
(will generate a default context according to self.verify_ssl)
2. if verify_ssl is True in req, generate a default SSL context
3. if verify_ssl is False in req, generate a SSL context that
won't verify
"""

(it is also used in one example still)

However, the main issue is that I've expected ssl=True to check certificates, but it handles it as "not verified or fingerprinted" then:

if sslcontext is not None:
# not verified or fingerprinted
return self._make_ssl_context(False)

It might be that the intention here is to use fingerprinting only, but it certainly confused me, and should maybe throw an error when using ssl=True with fingerprint=None then?

@asvetlov
Copy link
Member

Definitely, I need to check this.
Sorry, I'm pretty busy now

@asvetlov asvetlov self-assigned this May 17, 2019
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

No branches or pull requests

2 participants