Can't specify both fingerprint and SSLContext for client request

aio-libs / aiohttp

Asynchronous HTTP client/server framework for asyncio and Python

https://docs.aiohttp.org

Other

14.94k stars 1.99k forks source link

Can't specify both fingerprint and SSLContext for client request #3679

Open jeremy-hiatt opened 5 years ago

jeremy-hiatt commented 5 years ago

Long story short

It appears that 4.0 will remove all the SSL parameters from the TCPConnector class except for a single ssl parameter (#2626). This appears to be quite limiting: for example, it would no longer be possible to supply a client certificate and simultaneously pin the expected server certificate. The former requires you to pass the SSLContext configured with the client certificate as the ssl parameter, but for the latter you would have to pass a Fingerprint. Is there a recommendation for how this could be achieved?

Expected behaviour

Should be possible to make a request with BOTH a client certificate and a pinned server certificate.

Actual behaviour

It appears this will be removed with no alternative offered in 4.0.

Steps to reproduce

N/A

Your environment

client

webknjaz commented 5 years ago

It looks like those were dropped in #3548

webknjaz commented 5 years ago

@asvetlov it looks like an API design regression: we should probably separate ssl context and fingerprint.

Another thought: one arg to rule them all smells like a godobject antipattern. Maybe it's a wrong decision after all?

jeremy-hiatt commented 5 years ago

Following up on this...

I see a couple of options here:

Revert back to a pattern where SSL is configured through multiple non-orthogonal parameters.
Find a way to (ab)use inheritance to allow a single parameter to specify the fingerprint and configure the SSLContext as desired.

I'm happy to submit a patch if we can reach agreement on the best path forward.

webknjaz commented 5 years ago

Yeah, Andrew and I discussed this privately and thought that it probably needs to be done on the stdlib side and is probably in the scope of https://www.python.org/dev/peps/pep-0543/ (which I notified @tiran about, I think he wanted to add come hook for an extra validation callback).

Personally, I think that it's probably mostly fine to extend SSLContext but Andrew seems to dislike this idea.

jeremy-hiatt commented 5 years ago

Thanks for the update. I did some experimentation with the multiple inheritance route; unfortunately it doesn't look easy to create an object that looks like both a Fingerprint and an SSLContext. I think Andrew's instincts were correct there.

For now I can work around the issue by implementing a subclass of TCPConnector and extending _get_fingerprint() to check an additional attribute.

tiran commented 5 years ago

It's currently not possible to implement cert pinning with Python's ssl module. To implement it correctly, the pinned cert must be verified during the handshake. In case of an error, the handshake must be aborted before a client cert is verified and error must be signalled with a bad_certificate alert.

webknjaz commented 5 years ago

@tiran is that because the client cert may contain sensitive data?