This change has been marked as completed.

Describe the completed change (optional):

Pushed to release-6.x (6403e68)

Summary

Add base classes for representing certificates, bundles, and fingerprints.

Review Request #13158 — Created July 20, 2023 and submitted 1 year, 8 months ago

Information

Owner

chipx86*

Repository

Review Board

Branch

release-6.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description*

This introduces a new reviewboard.certs module, which will be used for
all SSL/TLS certificate management support going forward.

It currently contains representations for SSL/TLS certificates + keys,
CA bundles (containing root certs and intermediary certs), and
fingerprints.

We utilize cryptography's certificate support for parsing out
information from a certificate, such as validity dates and fingerprints,
in order to avoid duplication of effort. These are lazily-parsed as
needed. The goal is to avoid loading this information when just passing
around basic certificate information.

Going forward, the plans are to introduce a CertificateManager for
fetching/storing/deleting paths/data for certificates, and backends to
actually handle those operations. The default backend will manage this
in the data/ directory.

We'll then begin to build UI around this and tie the manager into auth,
repository, and WebHook operations, allowing people to begin formally
providing self-signed certificates and internal CA-signed certificates
into all Review Board communication.

Testing Done

Unit tests pass.

Commits

Summary	ID
Add base classes for representing certificates, bundles, and fingerprints. This introduces a new `reviewboard.certs` module, which will be used for all SSL/TLS certificate management support going forward. It currently contains representations for SSL/TLS certificates + keys, CA bundles (containing root certs and intermediary certs), and fingerprints. We utilize `cryptography`'s certificate support for parsing out information from a certificate, such as validity dates and fingerprints, in order to avoid duplication of effort. These are lazily-parsed as needed. The goal is to avoid loading this information when just passing around basic certificate information. Going forward, the plans are to introduce a `CertificateManager` for fetching/storing/deleting paths/data for certificates, and backends to actually handle those operations. The default backend will manage this in the `data/` directory. We'll then begin to build UI around this and tie the manager into auth, repository, and WebHook operations, allowing people to begin formally providing self-signed certificates and internal CA-signed certificates into all Review Board communication.	a70e95990f5f8cb28b437bd6d5130299c90bca6a

Summary

Add base classes for representing certificates, bundles, and fingerprints.

This introduces a new `reviewboard.certs` module, which will be used for all SSL/TLS certificate management support going forward. It currently contains representations for SSL/TLS certificates + keys, CA bundles (containing root certs and intermediary certs), and fingerprints. We utilize `cryptography`'s certificate support for parsing out information from a certificate, such as validity dates and fingerprints, in order to avoid duplication of effort. These are lazily-parsed as needed. The goal is to avoid loading this information when just passing around basic certificate information. Going forward, the plans are to introduce a `CertificateManager` for fetching/storing/deleting paths/data for certificates, and backends to actually handle those operations. The default backend will manage this in the `data/` directory. We'll then begin to build UI around this and tie the manager into auth, repository, and WebHook operations, allowing people to begin formally providing self-signed certificates and internal CA-signed certificates into all Review Board communication.

a70e95990f5f8cb28b437bd6d5130299c90bca6a

Issues

Description	From	Last Updated
Missing documentation.	maubin	1 year, 8 months ago
Do we want to say "optional" in the types here?	david	1 year, 8 months ago
There are no open issues

flake8 passed.

JSHint passed.

reviewboard/certs/cert.py (Diff revision 1)

The issue has been resolved. Show all issues

Missing documentation.

Change Summary:


Added new exceptions for file/storage-related errors.
Added support for certs/bundles/keys containing junk/metadata around PEM content, and stricter validation of the PEM content.
Added logged exception handling for all I/O operations on these core classes.
CertificateBundle now enforces bundle names as slugs.
Added missing docs.
Added missing unit tests.

Commits:

	Summary	ID
	Add base classes for representing certificates, bundles, and fingerprints. This introduces a new `reviewboard.certs` module, which will be used for all SSL/TLS certificate management support going forward. It currently contains representations for SSL/TLS certificates + keys, CA bundles (containing root certs and intermediary certs), and fingerprints. We utilize `cryptography`'s certificate support for parsing out information from a certificate, such as validity dates and fingerprints, in order to avoid duplication of effort. These are lazily-parsed as needed. The goal is to avoid loading this information when just passing around basic certificate information. Going forward, the plans are to introduce a `CertificateManager` for fetching/storing/deleting paths/data for certificates, and backends to actually handle those operations. The default backend will manage this in the `data/` directory. We'll then begin to build UI around this and tie the manager into auth, repository, and WebHook operations, allowing people to begin formally providing self-signed certificates and internal CA-signed certificates into all Review Board communication.	11812af0ccfd7f8e1993128ecfde410a24d7f540
	Add base classes for representing certificates, bundles, and fingerprints. This introduces a new `reviewboard.certs` module, which will be used for all SSL/TLS certificate management support going forward. It currently contains representations for SSL/TLS certificates + keys, CA bundles (containing root certs and intermediary certs), and fingerprints. We utilize `cryptography`'s certificate support for parsing out information from a certificate, such as validity dates and fingerprints, in order to avoid duplication of effort. These are lazily-parsed as needed. The goal is to avoid loading this information when just passing around basic certificate information. Going forward, the plans are to introduce a `CertificateManager` for fetching/storing/deleting paths/data for certificates, and backends to actually handle those operations. The default backend will manage this in the `data/` directory. We'll then begin to build UI around this and tie the manager into auth, repository, and WebHook operations, allowing people to begin formally providing self-signed certificates and internal CA-signed certificates into all Review Board communication.	a70e95990f5f8cb28b437bd6d5130299c90bca6a

Diff:

Revision 2 (+3540)

Show changes

	docs/manual/extending/coderef/index.rst
	reviewboard/certs/__init__.py
	reviewboard/certs/cert.py
	reviewboard/certs/errors.py
	reviewboard/certs/tests/__init__.py
	reviewboard/certs/tests/test_certificate.py
	reviewboard/certs/tests/test_certificate_bundle.py
	reviewboard/certs/tests/test_certificate_fingerprints.py
	1 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

reviewboard/certs/cert.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Do we want to say "optional" in the types here?
```

Status:: Completed
Change Summary:: Pushed to release-6.x (6403e68)