2.32.5 breaks passing custom SSL context

[Tasssadar]

2.32.5 change 90fee08 breaks passing of custom ssl context using an adapter like this:

class SSLContextAdapter(requests.adapters.HTTPAdapter):
    @override
    def init_poolmanager(self, *args: Any, **kwargs: Any) -> Any:
        kwargs["ssl_context"] = ssl.create_default_context()
        return super().init_poolmanager(*args, **kwargs)  # type: ignore

ssl_adapter = SSLContextAdapter()
session.mount("https://", ssl_adapter)

Now, if verify=True, the code in

requests/src/requests/adapters.py

Lines 292 to 313 in 90fee08

	if url.lower().startswith("https") and verify:
	cert_loc = None
	# Allow self-specified cert location.
	if verify is not True:
	cert_loc = verify
	if not cert_loc:
	cert_loc = extract_zipped_paths(DEFAULT_CA_BUNDLE_PATH)
	if not cert_loc or not os.path.exists(cert_loc):
	raise OSError(
	f"Could not find a suitable TLS CA certificate bundle, "
	f"invalid path: {cert_loc}"
	)
	conn.cert_reqs = "CERT_REQUIRED"
	if not os.path.isdir(cert_loc):
	conn.ca_certs = cert_loc
	else:
	conn.ca_cert_dir = cert_loc

always sets ca_certs, which causes urllib3 to modify the ssl_context by loading more certs into it here.

EDIT: I can be fixed by overriding also cert_verify:

class SSLContextAdapter(requests.adapters.HTTPAdapter):
    @override
    def init_poolmanager(self, *args: Any, **kwargs: Any) -> Any:
        kwargs["ssl_context"] = ssl.create_default_context()
        return super().init_poolmanager(*args, **kwargs)  # type: ignore

    @override
    def cert_verify(self, *_args: Any, **_kwargs: Any) -> None:
        pass


ssl_adapter = SSLContextAdapter()
session.mount("https://", ssl_adapter)

I'd say this belongs to documentation and needs some tests, so that future changes don't break it again - will prepare a PR if I'll have time.

[MANAS225]

Thanks for the detailed investigation — this helped clarify the root cause a lot.

I understand that when a custom ssl_context is supplied via a custom HTTPAdapter, Requests still mutates it when verify=True by loading CA certs in cert_verify, which defeats the purpose of providing a fully user-controlled SSL context.

The workaround of overriding cert_verify makes sense, and I agree this behavior should be documented and covered by tests to prevent regressions.

If this sounds good, I’d be happy to:

• Add documentation explaining how to correctly use a custom SSLContext with HTTPAdapter
• Add tests ensuring that a user-supplied ssl_context is not modified by Requests

Please let me know if this approach aligns with your expectations.

[Tasssadar]

Hello, yes, that would be great - but I already prepared a PR in the meantime, #7044

[MANAS225]

Thanks for the update!

Got it — I see that PR #7044 already covers this.
Happy to review, test, or help iterate on the PR if any changes are needed.

[nateprewitt]

Thanks for opening this issue, @Tasssadar. The whole 2.32.x series has been plagued with cert issues because solving this for every use case has been remarkably problematic. The original CVE fix coupled with the caching found a lot of under-defined behaviors people have built infrastructure on.

Requests prior to 2.32.0 had the same behavior you're observing in 2.32.5. That is you can define certs but Requests will always inject the DEFAULT_CA_CERT_BUNDLE from certifi alongside those certs. This should only be additive, but is a mutation. That default has been intentional because older code that didn't explicitly define certs on the context has been built with the assumption it will be populated.

2.32.0 unintentionally stopped doing that with our cached SSLContext, but with that reverted, we're back to the original behavior. I'm not positive we want to explicitly tell people to disable cert_verify on custom adapters because I think that may have other security implications. I'm wondering if we need some middle-ground.

I'd rather we frame this as a feature request than a breakage (since it was the originally intended behavior). It seems like we could detect if there are certs available on the provided context and disable default loading. That's potentially overhead/breakage surface area we don't want though.

[Tasssadar]

Hello, thanks for the explanation! Yes, seems reasonable to have ti as a feature request then.

For context, the reason I'm reporting this is because this was actually a huge performance problem for us - we do not necessarily mind that the default certs are being injected into our context (although that seem problematic to me if I wanted to verify target server identity with cert pinning...) - our problem was that requests parse 300KB PEM file on each request, which is extremely expensive. We've had servers that can do 400 req/s spin on 100% cpu parsing the same file over and over again instead.

To me, it seems that there should be a way to say "just use this SSLContext for the request", which is what I tried to document in the PR, but I understand it's not so easy with requests codebase and history. Thanks for looking into this!

[nateprewitt]

I've been stewing on this one for a while. Our SSLContext workflow has never been documented except in a blog by a former maintainer almost a decade ago. It worked but was never really an official feature of the library. I think a big part of why we had so much unexpected impact in 2.32.x is there is not "one way" to do this. This pattern is adopted by a lot of high-usage, high-visibility tools but we don't really define how it should be done.

Adding the certs unilaterally in the first place seems wrong the more I think on it. Even if we rule out the performance piece, we're adding additional surface area to the SSLContext that broadens what's accepted. Removal of that bundle is a fail-closed posture, addition allows for Requests to succeed that maybe weren't intended.

The more I dig through how people are using this, the more I want us to be concrete in what you can do and how you should do it. That gives everyone a level field. We have a contract to maintain and if you don't follow it, that's outside of what we're intending to support. I still don't really want to encourage people to disable cert_verify in all adapters, I think that's a net negative if we document that as a path forward.

I put together this tonight. It's potentially a path forward but does break a subset of cases that probably work right now. Most popular libraries using a custom SSLContext are loading everything they need. It's the people adding their internal CA bundle and just expecting Requests to handle the rest that would be impacted.

We could try a FutureWarning about a behavior change but I think that's going to be loud for our well-behaving downstreams. Maybe a changelog entry stating in version 2.X, all adapters using a custom SSLContext MUST provide their full cert bundle with documentation on how to do it.