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

Forbid use of cadata arg in pyOpenSSL backend #1885

Closed
MarSoft opened this issue Jun 1, 2020 · 5 comments
Closed

Forbid use of cadata arg in pyOpenSSL backend #1885

MarSoft opened this issue Jun 1, 2020 · 5 comments

Comments

@MarSoft
Copy link

MarSoft commented Jun 1, 2020

The class urllib3.contrib.pyopenssl.PyOpenSSLContext as a load_verify_locations() method. This method accepts cadata as its third parameter.
If this parameter is given then it wraps the provided certificate data in io.BytesIO() and passes to OpenSSL.SSL.Context.load_verify_locations() as cafile param.
But OpenSSL.SSL.Context.load_verify_locations does not support a file-like object for cafile param, it expects only file path.

This seems to break intentions of #1804.

@pquentin
Copy link
Member

pquentin commented Jun 2, 2020

Hmm, this is a tricky one, thanks for pointing this out. As you said, pyOpenSSL does not support cadata, and only accepts cafile (a file path) or capath (a directory path).

While we could write the data in a temporary file, I don't think urllib3 shoud write any kind of data to the filesystem. I think the best option is here is to refuse to handle the cadata argument, just like #1804 does it with Python < 2.7.9.

@sethmlarson @sigmavirus24 Thoughts?

@sigmavirus24
Copy link
Contributor

While we could write the data in a temporary file, I don't think urllib3 shoud write any kind of data to the filesystem

Agreed

I think the best option is here is to refuse to handle the cadata argument, just like #1804 does it with Python < 2.7.9.

Makes sense to me

@sethmlarson
Copy link
Member

I'm also in agreement here, no filesystem hacks instead fail loudly.

@pquentin
Copy link
Member

pquentin commented Jun 2, 2020

@MarSoft Would you be interested in working on this?

@pquentin pquentin changed the title cadata arg buggy in contrib.pyopenssl.PyOpenSSLContext.load_verify_locations Forbid use of cadata arg in pyOpenSSL backend Jun 3, 2020
@hodbn hodbn self-assigned this Jul 13, 2020
@IvanLauLinTiong
Copy link
Contributor

pyOpenSSL is deprecated and will be removed in future release version 2.x (#2691).

@sethmlarson sethmlarson closed this as not planned Won't fix, can't repro, duplicate, stale Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment