m-click / requests_pkcs12

Add PKCS#12 support to the Python requests library in a clean way, without monkey patching or temporary files
ISC License
123 stars 33 forks source link

cert now not being read as x509 instance #30

Closed jordanmurray35 closed 3 years ago

jordanmurray35 commented 3 years ago

Just last week, the implementation that I used to make requests with pkcs12 file format was working perfectly using a file with .p12 extension. This morning, it is now not able to verify my cert as an x509 instance even though I am able to extract the certificate from the .p12 file and verify that the cert is an OpenSSL.crypto.X509 object. Whenever I run the following: p12 = crypto.load_pkcs12(open(path_to_cert, 'rb').read(), password) crt = p12.get_certificate() print(crt)

The type of crt is X509.

I am making a get request and arguments include pkcs12_filename with corresponding .p12 file and pkcs12_password with corresponding password. The below is the error that is returning:

File "/opt/python39/lib/python3.9/site-packages/requests_pkcs12.py", line 132, in get return request('get', *args, **kwargs) File "/opt/python39/lib/python3.9/site-packages/requests_pkcs12.py", line 118, in request pkcs12_adapter = Pkcs12Adapter( File "/opt/python39/lib/python3.9/site-packages/requests_pkcs12.py", line 95, in __init__ self.ssl_context = create_pyopenssl_sslcontext(pkcs12_data, pkcs12_password_bytes, ssl_protocol) File "/opt/python39/lib/python3.9/site-packages/requests_pkcs12.py", line 45, in create_pyopenssl_sslcontext ssl_context._ctx.use_certificate(cert) File "/opt/python39/lib/python3.9/site-packages/OpenSSL/SSL.py", line 861, in use_certificate raise TypeError("cert must be an X509 instance") TypeError: cert must be an X509 instance

Again, it was working just last week with the same exact .p12 file and I have not made any changes at all. I am curious if this is the cause of a change in this repo or not. If anyone is able to replicate this, please comment below. Thank you.

sbromberger commented 3 years ago

I'm seeing the same thing on 1.12. Downgrading to v1.10 fixed things.

vog commented 3 years ago

@ao-landoo Do you see the same issue?

(I'm asking because you verified successfully in #27, and I'd like to know if your formerly successful test case is affected as well.)

jordanmurray35 commented 3 years ago

@sbromberger I downgraded to v1.10 as well and it resolves the issue that I was having.

vog commented 3 years ago

I just released a new release 1.13 which adds the missing conversions between OpenSSL and cryptography objects. Moreover, I added a small selftest that should be sufficient to prevent regressions like this in the future.

@jordanmurray35 @sbromberger Does release 1.13 work for both of you?

jordanmurray35 commented 3 years ago

@vog yes release 1.13 works for me now. Thank you

sbromberger commented 3 years ago

Sorry for the delay - I'll be able to test next week. Thank you for the prompt response!

vog commented 3 years ago

Okay, I think two independent testes (@jordanmurray35 and myself) as well as the automated selftest will be sufficient to close this ticket.

@sbromberger Please do your testing nevertheless. If you find anything, you can always open a new issue.

sbromberger commented 3 years ago

Just wanted to report in that the new version seems to have fixed the problem for me as well. Thanks again!

vog commented 3 years ago

@sbromberger Thanks for your additional testing. This gives us further assurance that release 1.13 does indeed solve all remaining issues.