Summary

Add certificate support for hosting services.

Review Request #14926 — Created March 18, 2026 and updated March 18, 2026, 9:26 a.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-7.1.x

Bugs

Depends On

14925

Reviewers

Groups

reviewboard

People

Description

This introduces modern certificate support for all hosting services.
The work required for this is a bit different than just using a
cert-populated SSL context for urlopen(), as hosting services
historically have supported storing approved SSL certificate data as
part of the HostingServiceAccount. Those were conducted with hostname
checks turned off. That added a bit of complexity and required a
migration path.

Now, when preparing to open an HTTPS request where legacy cert data is
present, that data will be converted into a Certificate. If that
certificate's associated hostname (either the Common Name or any
Subject Alternative Names) is a match for the address, then it will be
saved as a new certificate and the legacy data removed. If there are any
problems, the legacy data will be used with a legacy SSL context
disabling hostname checks, logging warnings until the admin deals with
the problem.

For testing, some changes needed to be made to testing infrastructure.
A utility SSL context for the CertificateManager tests has been made
commonly available as CaptureSSLContext, and a new test cert has been
added. Along with this, CertificateManager no longer computes the data
directory until it needs it, allowing the test harness to switch that to
a temporary location for the test run.

Hosting services have not yet been updated to raise the modern
CertificateVerificationError. The legacy UnverifiedCertificateError
exception is still supported.

Testing Done

Unit tests passed.

Tested this with a couple of self-hosted services against self-signed
SSL certificates. Verified that I got the right error when I tried to
communicate with a server, and got the trust banner.

Tested that trusting the host added the certificate.

Tested migrating legacy SSL data from a hosting account, and verifying
it added the new certificate.

Commits

Summary	ID
Add certificate support for hosting services. This introduces modern certificate support for all hosting services. The work required for this is a bit different than just using a cert-populated SSL context for `urlopen()`, as hosting services historically have supported storing approved SSL certificate data as part of the `HostingServiceAccount`. Those were conducted with hostname checks turned off. That added a bit of complexity and required a migration path. Now, when preparing to open an HTTPS request where legacy cert data is present, that data will be converted into a `Certificate`. If that certificate's associated hostname (either the Common Name or any Subject Alternative Names) is a match for the address, then it will be saved as a new certificate and the legacy data removed. If there are any problems, the legacy data will be used with a legacy SSL context disabling hostname checks, logging warnings until the admin deals with the problem. For testing, some changes needed to be made to testing infrastructure. A utility SSL context for the `CertificateManager` tests has been made commonly available as `CaptureSSLContext`, and a new test cert has been added. Along with this, `CertificateManager` no longer computes the data directory until it needs it, allowing the test harness to switch that to a temporary location for the test run. Hosting services have not yet been updated to raise the modern `CertificateVerificationError`. The legacy `UnverifiedCertificateError` exception is still supported.	d418982060ff27bf044f932bddcd018de2f83dd6

Summary

Add certificate support for hosting services.

This introduces modern certificate support for all hosting services. The work required for this is a bit different than just using a cert-populated SSL context for `urlopen()`, as hosting services historically have supported storing approved SSL certificate data as part of the `HostingServiceAccount`. Those were conducted with hostname checks turned off. That added a bit of complexity and required a migration path. Now, when preparing to open an HTTPS request where legacy cert data is present, that data will be converted into a `Certificate`. If that certificate's associated hostname (either the Common Name or any Subject Alternative Names) is a match for the address, then it will be saved as a new certificate and the legacy data removed. If there are any problems, the legacy data will be used with a legacy SSL context disabling hostname checks, logging warnings until the admin deals with the problem. For testing, some changes needed to be made to testing infrastructure. A utility SSL context for the `CertificateManager` tests has been made commonly available as `CaptureSSLContext`, and a new test cert has been added. Along with this, `CertificateManager` no longer computes the data directory until it needs it, allowing the test harness to switch that to a temporary location for the test run. Hosting services have not yet been updated to raise the modern `CertificateVerificationError`. The legacy `UnverifiedCertificateError` exception is still supported.

d418982060ff27bf044f932bddcd018de2f83dd6

Issues

Description	From	Last Updated
This doesn't appear to be used for logging context in the implementation. Did you want to include it in log …	david	March 18, 2026, 9:26 a.m.
We're no longer setting self.data['ssl_cert'] but in the case that we get a LegacyUnverifiedCertificateError, the re-authorization in HostingServiceHTTPRequest.open checks for …	david	March 18, 2026, 9:26 a.m.
This will end up creating a cert for http/port 80, which seems... odd. We should probably at least log something.	david	March 18, 2026, 9:26 a.m.
In the case that we get an exception from this, the hostname can be None, leaving this method doing nothing …	david	March 18, 2026, 9:26 a.m.
Should we make a mixin that includes this for setup/teardown?	david	March 18, 2026, 9:26 a.m.
It also might be nice to set this via addCleanup at the beginning of the setUp method instead of putting …	david	March 18, 2026, 9:26 a.m.
Copy/pasteo: Docstring says legacy but the implementation uses the new exception.	david	March 18, 2026, 9:26 a.m.

flake8 passed.

JSHint passed.

Fix it!

reviewboard/hostingsvcs/base/http.py (Diff revision 1)

An issue was opened. Show all issues

This doesn't appear to be used for logging context in the implementation. Did you want to include it in log messages, or should it just be removed?

reviewboard/hostingsvcs/models.py (Diff revision 1)

An issue was opened. Show all issues

We're no longer setting self.data['ssl_cert'] but in the case that we get a LegacyUnverifiedCertificateError, the re-authorization in HostingServiceHTTPRequest.open checks for its presence.

reviewboard/hostingsvcs/models.py (Diff revision 1)

An issue was opened. Show all issues

This will end up creating a cert for http/port 80, which seems... odd. We should probably at least log something.

reviewboard/hostingsvcs/models.py (Diff revision 1)

An issue was opened. Show all issues

In the case that we get an exception from this, the hostname can be None, leaving this method doing nothing entirely silently.

We should probably log something if we get an exception, and maybe also log if the hostname is falsy.

reviewboard/hostingsvcs/tests/test_hosting_service_auth_form.py (Diff revision 1)
An issue was opened. Show all issues
```
Should we make a mixin that includes this for setup/teardown?
```

reviewboard/hostingsvcs/tests/test_hosting_service_auth_form.py (Diff revision 1)

An issue was opened. Show all issues

It also might be nice to set this via addCleanup at the beginning of the setUp method instead of putting it in tearDown, in case something in test setup fails.

reviewboard/hostingsvcs/tests/test_hosting_service_auth_form.py (Diff revision 1)
An issue was opened. Show all issues
```
Copy/pasteo: Docstring says legacy but the implementation uses the new exception.
```