Please add a new instance variable annotation for this.

Can you put this in the doc comment next to the definition? It makes more sense there.

HTTPSHandler.__init__() sets self._context = None, clobbering the context you saved above. https_open() resets it too.
If we really want to be able to override this, we should store it separately and apply it (like I tried to do in /r/15185/). That said, the only use of the context parameter was removed in this change (hostingsvcs/base/http.py line 582), so we don't use it anymore. I'd say we just get rid of the parameter.

This applies check_hostname=False and the legacy cadata to all hosts, even ones reached via a redirect. This isn't a new regression (the old single-context approach was essentially the same thing), but it's not too ugly to tighten up.
In HostingServiceHTTPRequest.open(), you were previously passing in the request URL and got rid of that in this change. I say let's bring it back for a different job:

urlopen_handlers.append(CertificateVerificationHTTPHandler(
    local_site=hosting_account.local_site,
    legacy_cert_hostname=urlparse(url).hostname,
    **ssl_kwargs,
))


Then the constructor above can save it, and in here:

if hostname:
    context = self._cert_manager.build_ssl_context(...)

    if hostname == self._legacy_cert_hostname:
        context.check_hostname = self._rb_check_hostname

        if extra_cert_data := self._extra_cert_data:
            context.load_verify_locations(cadata=extra_cert_data)



test_with_http_to_https_redirect_without_certs is already testing a redirect. We can use that as a prototype for a new test that does something similar with a handler that creates the handler with legacy_cert_hostname='example.com' and then check that the resulting SSL contexts include two items, and that the first one has check_hostname=False and the second check_hostname=True.

Can we catch SSLError here and reraise a CertificateVerificationError?

CaptureSSLMixin calls a bunch of kgb stuff, which will flag if its just typed as TestCase. Can we do this instead?

class TestCaseMixinBase(kgb.SpyAgency, TestCase):
    pass

Needs a trailing comma

This (and the associated url attribute) never actually work. AbstractHTTPHandler.do_open() overwrites the response URL, so any URL passed to this will always get overwritten with the actual response URL.
That makes the assignment in test_open_with_legacy_ssl_cert_hostname_mismatch a no-op. It happens to pass because the mocked URL is the same as the response URL, but that's passing for the wrong reason.
Probably the best thing is to just get rid of url and geturl() here.

We should add a note in this docstring that users of the mixin will need to also mix in kgb.SpyAgency ahead of this.

Typo: certificats -> certificates

This reads a little odd. Can we say "The mock HTTP responses to serve, in order"?

This is currently not customizable. It gets set in setUp() and then immediately passed to SpyOpReturnInOrder, which creates an internal copy of the list. All the test cases which want to manipulate this are unspying/respying.

This constructs a fresh CertificateManager, while stuff in test_hosting_service_http_request.py and test_hosting_service_auth_form.py do from reviewboard.certs.manager import cert_manager. The tests mutate state through the global but then read paths off the instance. It works because they're both resolving to the same storage path, but it seems fragile.
Can we have this mixin just use the singleton?

Copy/pasto

The TestCase instance will get discarded after teardown, so it seems like this is all just a noop?

The docstring for this function needs to be updated.

It's kind of weird to reuse cert_data as a success value.
Can we create a separate migrated flag that gets set to True here, and then below instead of if cert_data we can do if not migrated:?

This is now unused. And once you get rid of this declaration, I think the ssl import will also be unused.

These seem unused now.

This seems unused now.

This docstring was incorrect before, and is also incorrect now.

There's an extra #: in this line from rewrapping.
Should also be a deprecation warning, not error.

_process_ssl_error calls Certificate.create_from_server(), opening a real socket with a 10s timeout and then raising an error with the server's certificate, but load_verify_locations failing has nothing to do with the server, it's just when the locally stored blob is unparseable.
This means a corrupt legacy blob gets a 10s block and a "certificate verification failed, do you want to trust this host?" prompt for an unrelated cert.
I think we should just log and skip here.

This is still freezing things at setUp time, because SpyOpMatchInOrder materializes its arg into a list, consuming the generator.
It's not pretty, but we could read the list lazily instead:

self._next_http_response = 0

@self.spy_for(HTTPConnection.getresponse, owner=HTTPConnection)
def _getresponse(_self) -> MockHTTPResponse:
    i = self._next_http_response
    self._next_http_response += 1

    return self.mock_http_responses[i]

Summary line here is an incorrect copy/paste.

Ship It!