Summary

Add certificate support for hosting services.

Review Request #14926 — Created March 18, 2026 and updated April 10, 2026, 2:47 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.	52583d104f8826af06a7d590124c90667d79df18

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.

52583d104f8826af06a7d590124c90667d79df18

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	April 2, 2026, 1:17 p.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	April 2, 2026, 1:19 p.m.
This will end up creating a cert for http/port 80, which seems... odd. We should probably at least log something.	david	April 2, 2026, 1:22 p.m.
In the case that we get an exception from this, the hostname can be None, leaving this method doing nothing …	david	April 2, 2026, 1:24 p.m.
Should we make a mixin that includes this for setup/teardown?	david	April 8, 2026, 1:21 a.m.
It also might be nice to set this via addCleanup at the beginning of the setUp method instead of putting …	david	April 8, 2026, 1:21 a.m.
Copy/pasteo: Docstring says legacy but the implementation uses the new exception.	david	April 2, 2026, 1:39 p.m.
local variable 'e' is assigned to but never used Column: 13 Error code: F841	reviewbot	April 2, 2026, 2:02 p.m.
If trust_host == True and cert is None, this falls through silently, leaving reauth as False, and we'll proceed as …	david	April 8, 2026, 1:21 a.m.

flake8 passed.

JSHint passed.

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

The issue has been resolved. 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?

chipx86

April 2, 2026, 1:41 p.m.

Yeah, it was originally used but not anymore. I'll remove it.

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

The issue has been dropped. 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.

chipx86

April 2, 2026, 1:41 p.m.

Right, it's meant to be loud and persistent and noisy, and to try again until fixed. This represents an insecure setup requiring work. I'll add a comment to that effect.

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

The issue has been resolved. 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)

The issue has been resolved. 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)

The issue has been dropped. Show all issues

Should we make a mixin that includes this for setup/teardown?

chipx86

April 2, 2026, 1:41 p.m.

Yeah but in a separate change. I want to revisit some of this, because a few things are scattered.

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

The issue has been dropped. 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.

chipx86

April 2, 2026, 1:41 p.m.

This only ends up mattering if the test actually runs, because it's what populates state.

I have a thought on how I want to approach this in a more general way but I want to think about that separately from this change at this point.

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

Change Summary:


Checked for deprecation warnings in unit tests.
Updated for RB8.
Removed the unused request_url attribute.
Added a comment about the noisy logging around unsafe certs.
Certs can no longer be accepted for http:// URLs, and will be logged.
Exceptions when accepting a legacy cert or when failing to get a hostname are now logged.
Fixed a bad test docstring.

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
	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.	2df6c47315e47079cacff42291b0e1d2e03acd6d

Diff:

Revision 2 (+2062 -186)

Show changes

	reviewboard/certs/manager.py
	reviewboard/certs/tests/test_certificate_manager.py
	reviewboard/certs/tests/testcases.py
	reviewboard/hostingsvcs/models.py
	reviewboard/hostingsvcs/base/forms.py
	reviewboard/hostingsvcs/base/http.py
	reviewboard/hostingsvcs/tests/test_hosting_service_auth_form.py
	reviewboard/hostingsvcs/tests/test_hosting_service_http_request.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/hostingsvcs/models.py (Diff revision 2)
The issue has been resolved. Show all issues
```
local variable 'e' is assigned to but never used

Column: 13
Error code: F841
```

Change Summary:

Added the missing exception message to a log statement.

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.	2df6c47315e47079cacff42291b0e1d2e03acd6d
	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.	833aad80e66112474981e9e720790c110a4dcdce

Diff:

Revision 3 (+2062 -186)

Show changes

	reviewboard/certs/manager.py
	reviewboard/certs/tests/test_certificate_manager.py
	reviewboard/certs/tests/testcases.py
	reviewboard/hostingsvcs/models.py
	reviewboard/hostingsvcs/base/forms.py
	reviewboard/hostingsvcs/base/http.py
	reviewboard/hostingsvcs/tests/test_hosting_service_auth_form.py
	reviewboard/hostingsvcs/tests/test_hosting_service_http_request.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/hostingsvcs/base/forms.py (Diff revision 3)

The issue has been resolved. Show all issues

If trust_host == True and cert is None, this falls through silently, leaving reauth as False, and we'll proceed as if authorization succeeded.

We should probably log and reraise the error.

Change Summary:


Added logging for the implementation case where a cert instance is missing.

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.	833aad80e66112474981e9e720790c110a4dcdce
	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.	52583d104f8826af06a7d590124c90667d79df18

Diff:

Revision 4 (+2090 -186)

Show changes

	reviewboard/certs/manager.py
	reviewboard/certs/tests/test_certificate_manager.py
	reviewboard/certs/tests/testcases.py
	reviewboard/hostingsvcs/models.py
	reviewboard/hostingsvcs/base/forms.py
	reviewboard/hostingsvcs/base/http.py
	reviewboard/hostingsvcs/tests/test_hosting_service_auth_form.py
	reviewboard/hostingsvcs/tests/test_hosting_service_http_request.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```