Skip to content

Conversation

@DurandA
Copy link
Contributor

@DurandA DurandA commented Jul 26, 2023

Description

This PR fixes the issue discussed in #5422 when connecting to a server with outdated/misconfigured SSL. This is a draft PR to discuss if a full PR would be accepted. A full PR would only add the OP_LEGACY_SERVER_CONNECT SSL context flag if the ssl_insecure switch is passed.

Checklist

  • I have updated tests where applicable.
  • I have added an entry to the CHANGELOG.

@mhils
Copy link
Member

mhils commented Jul 26, 2023

Doing this only when ssl_insecure is set sounds good to me! Some notes:

  1. This is only required for mitmproxy<->server connections, so we can move it to create_proxy_server_context.
  2. Let's add a legacy_server_connect boolean argument there and pass ssl_insecure as the value in the tlsconfig addon.
  3. Ideally OP_LEGACY_SERVER_CONNECT would be directly exposed by cryptography/pyOpenSSL. I'm happy to handle that part though once this PR is in. :)

@DurandA DurandA force-pushed the legacy-renegotiation branch from 2e35adf to fbf98ce Compare July 27, 2023 17:48
@DurandA DurandA marked this pull request as ready for review July 27, 2023 17:54
@DurandA
Copy link
Contributor Author

DurandA commented Jul 27, 2023

@mhils Thanks for the guidelines. Let me know if this is ok as implemented.

Ideally OP_LEGACY_SERVER_CONNECT would be directly exposed by cryptography/pyOpenSSL. I'm happy to handle that part though once this PR is in. :)

👍

mhils added a commit to mhils/cryptography that referenced this pull request Jul 27, 2023
This is useful to expose in pyOpenSSL so that it can be referenced downstream for `Context.set_options`. (mitmproxy/mitmproxy#6281)
@mhils
Copy link
Member

mhils commented Jul 27, 2023

Looks great, thanks! I've filed PRs with cryptography and pyOpenSSL. :)

alex pushed a commit to pyca/cryptography that referenced this pull request Jul 27, 2023
This is useful to expose in pyOpenSSL so that it can be referenced downstream for `Context.set_options`. (mitmproxy/mitmproxy#6281)
alex pushed a commit to pyca/pyopenssl that referenced this pull request Jul 29, 2023
* Expose `SSL_OP_LEGACY_SERVER_CONNECT` binding

based on pyca/cryptography#9303

refs mitmproxy/mitmproxy#6281

* Update CHANGELOG.rst
@mhils mhils enabled auto-merge (squash) July 29, 2023 16:27
@mhils mhils merged commit d3af57f into mitmproxy:main Jul 29, 2023
lasting-yang pushed a commit to lasting-yang/mitmproxy that referenced this pull request Aug 2, 2023
* Enable unsafe legacy renegotiation

* Update CHANGELOG

* Fix missing proxy_server_context argument in test

* use pyopenssl's OP_LEGACY_SERVER_CONNECT

---------

Co-authored-by: Maximilian Hils <[email protected]>
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

Successfully merging this pull request may close these issues.

2 participants