Proxies should use proxy_ssl_context when connecting via ProxyConfig.use_forwarding_with_https = True

[sethmlarson]

This was discovered in https://github.com/urllib3/urllib3/pull/2558 and discussed with @jalopezsilva on Discord. The gist is it appears that we're using HTTPSConnection.ssl_context to connect to HTTPS proxies when using use_forwarding_with_https = True mode. This is likely caused by us treating the proxy like it's the origin when we have proxies in "forwarding" mode and previously we didn't have a forwarding mode for HTTPS.

Minimum requirements

:dollar: You can get paid to complete this issue! Please read the docs for more information.

• When ProxyConfig.use_forwarding_with_https = True and ssl_context is specified:
  • [ ] If proxy_ssl_context is specified then error out with ValueError
  • If proxy_ssl_context isn't specified:
  • [ ] On 1.26.x emit a DeprecationWarning instructing to switch to proxy_ssl_context and set the value of ssl_context to proxy_ssl_context
  • [ ] On 2.x raise a ValueError that proxy_ssl_context should be used, not ssl_context.
• [ ] Only proxy_ssl_context should be used for creating the connection to proxies in all modes (HTTP->HTTPS, HTTPS->HTTP, HTTPS->HTTPS, in forwarding/tunnel modes).
• [ ] Remove the skip to test_proxy_https_target_tls_error added in https://github.com/urllib3/urllib3/commit/9f4d05c11eb2f80682c1b7a2671b0636292ef932

[fyunusa]

Hi @sethmlarson have this issue been fixed ? I propose this patch for the 2.x version

if use_forwarding_for_https == True and proxy_ssl_context != None:
            if isinstance(proxy_ssl_context, ssl.SSLContext) != True:
                self.proxy_config = ProxyConfig(proxy_ssl_context, use_forwarding_for_https)

            elif proxy_ssl_context == ssl.SSLContext:
                raise ValueError(
                    "proxy ssl context should be used, not ssl context"
                )

[pquentin]

You will need a pull request with tests before we can review your code.

[fyunusa]

OK @pquentin noted I'll work on that also

[sethmlarson]

@umarfarouk98 this issue is already being worked on by @avi364 I think in #2651?

[fyunusa]

OK yeah, I see that

[sg3-141-592]

Hey @abebeos , I only ever got as far as adding the ValueError exception and a unit test for it. I'm not going to be finishing this change so feel free to reuse anything that's useful off my draft PR.

[alexprabhat99]

Hi @sethmlarson Is this issue fixed or still accepting PRs?