Domain-based AOKI implementation - #852
Merged
Merged
Conversation
Fix incorrect private key usage in setup command Rename menu Endpoints back to Devices
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the AOKI onboarding flow to support a domain-based DevOwnerID lookup (CA-pinning) as a fallback when an IDevID-specific DevOwnerID is not present, and updates the demo/test tooling and UI to reflect/display this new reference type.
Changes:
- Add domain-based DevOwnerID credential resolution in CMP authorization (fallback to domain CA pinning).
- Extend test environment setup and certificate generation to include domain-based DevOwnerID artifacts.
- Update UI templates to rename navigation (“Devices”) and to display either IDevID or pinned-CA fingerprints.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| trustpoint/templates/trustpoint/base.html | Renames navigation label from “Endpoints” to “Devices”. |
| trustpoint/templates/devices/zero_touch_credentials/details.html | Displays a new fingerprint “display” field that supports domain-based references. |
| trustpoint/request/authorization/base.py | Adds fallback owner credential resolution using the request domain. |
| trustpoint/pki/models/credential.py | Adds parsing/display helpers for domain-based dev-owner:ca: references. |
| trustpoint/help_pages/devices_help_views.py | Adds domain-based DevOwnerID artifacts to the help/demo certificate listing. |
| trustpoint/aoki/views.py | Implements domain-based DevOwnerID lookup by domain CA fingerprint. |
| trustpoint/aoki/tests/cmp_client.py | Updates CMP client verification to accept domain-based DevOwnerID matching. |
| trustpoint/aoki/management/commands/aoki_setup_idevid_test_env.py | Extends test env setup to create/store both IDevID-based and domain-based DevOwnerIDs. |
| trustpoint/aoki/management/commands/aoki_gen_test_certs.py | Refactors test cert generation to create an Owner CA and use it to sign DevOwnerIDs. |
Comments suppressed due to low confidence (1)
trustpoint/aoki/tests/cmp_client.py:226
- The caught exception variable
eis unused here. Dropping the binding avoids an unused-variable lint issue and keeps the intent clear.
try:
self._verify_matches_idevid_cert(owner_id_cert, idevid_cert)
except AokiClientOwnerIdCertVerificationError as e:
self._verify_matches_domain_ca_cert(owner_id_cert)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
126
to
128
| idevid_sha256_fingerprint = idevid_cert.fingerprint(hashes.SHA256()).hex() | ||
| # Build URI string "dev-owner:cert:<idevid_subj_sn>_<idevid_sha256_fingerprint>" | ||
| # If the IDevID Subject Serial Number is not present, '' shall be used as a placeholder |
Comment on lines
+109
to
+111
| self.log_and_stdout(f' Domain: {domain.unique_name}') | ||
| self.log_and_stdout(f' Owner Credential: {OWNER_CRED_UNIQUE_NAME}') | ||
| self.log_and_stdout(f' Domain-based Owner Cred.: {DOMAIN_BASED_OWNER_CRED_UNIQUE_NAME}') |
| def _verify_matches_domain_ca_cert(self, owner_id_cert: x509.Certificate) -> None: | ||
| """Verify the Owner ID certificate is valid for the domain issuing CA certificate.""" | ||
| print('Verifying Owner ID certificate matches Domain CA certificate') | ||
| candidate_ca_certs = self._load_certificates(CERTS_DIR / 'full_chain.pem') |
❌ 13 Tests Failed:
View the full list of 13 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (4)
trustpoint/aoki/management/commands/aoki_setup_idevid_test_env.py:180
_get_or_create_domain_ca_owner_id_certunconditionally regenerates and overwrites the domain-based DevOwnerID cert/key on every run, and it readsOWNER_CA_KEY_PATHwithout ensuring it exists. This can break repeatable test environments (DB credential may no longer match regenerated files) and can crash if only the owner CA cert exists but the key is missing.
def _get_or_create_domain_ca_owner_id_cert(self, domain_ca: CaModel) -> None:
"""Generates a domain-based DevOwnerID, pinning the certificate of the domain CA."""
domain_ca_cert = domain_ca.get_certificate()
owner_ca_cert = x509.load_pem_x509_certificate(OWNER_CA_CERT_PATH.read_bytes())
owner_ca_key = load_pem_private_key(OWNER_CA_KEY_PATH.read_bytes(), password=None)
trustpoint/pki/models/credential.py:839
domain_ca_sha256_fingerprintdocstring is incorrect (it references “Third dot-separated segment” anddev-owner:), and thetry/except IndexErroris dead code becauseremoveprefix()cannot raiseIndexError. This makes the parsing rules fordev-owner:ca:<fingerprint>unclear.
Third dot-separated segment after stripping the ``dev-owner:`` prefix.
"""
if not self.idevid_ref.startswith('dev-owner:ca:'):
return ''
try:
trustpoint/aoki/management/commands/aoki_gen_test_certs.py:49
handle()includes a large commented-out call for domain-based DevOwnerID generation and uses a single long line forgenerate_owner_id_cert(...). The commented block is dead code and makes it unclear where domain-based certs should be generated.
idevid_cert = AokiTestCertGenerator.generate_idevid_pki()
owner_ca_cert, owner_ca_key = AokiTestCertGenerator.generate_owner_id_ca()
AokiTestCertGenerator.generate_owner_id_cert(idevid_cert, owner_ca_cert=owner_ca_cert, owner_ca_key=owner_ca_key)
# AokiTestCertGenerator.generate_domain_ca_owner_id_cert(
# domain_ca_cert=idevid_cert, owner_ca_cert=owner_ca_cert, owner_ca_key=owner_ca_key
# )
trustpoint/aoki/tests/cmp_client.py:226
- The caught exception is bound to
ebut never used. This triggers unused-variable lint and makes the intent less clear.
try:
self._verify_matches_idevid_cert(owner_id_cert, idevid_cert)
except AokiClientOwnerIdCertVerificationError as e:
self._verify_matches_domain_ca_cert(owner_id_cert)
Aircoookie
marked this pull request as ready for review
August 4, 2026 11:40
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
trustpoint/aoki/management/commands/aoki_setup_idevid_test_env.py:186
_get_or_create_domain_ca_owner_id_cert()readsOWNER_CA_KEY_PATHwithout first ensuring it exists. Ifownerid_ca.pemis present butownerid_ca_pk.pemis missing (e.g., from an older test-certs set), this will crash withFileNotFoundErrorbefore the command can self-heal.
domain_ca_cert = domain_ca.get_certificate()
owner_ca_cert = x509.load_pem_x509_certificate(OWNER_CA_CERT_PATH.read_bytes())
owner_ca_key = load_pem_private_key(OWNER_CA_KEY_PATH.read_bytes(), password=None)
AokiTestCertGenerator.generate_domain_ca_owner_id_cert(
trustpoint/aoki/tests/cmp_client.py:225
- The exception variable
eis unused here; this will trigger anF841/unused-variable lint error.
except AokiClientOwnerIdCertVerificationError as e:
trustpoint/pki/models/credential.py:848
- New parsing/display behavior for
dev-owner:ca:<sha256>refs (domain_ca_sha256_fingerprint/sha256_fingerprint_display) isn’t covered by the existing IDevIDReferenceModel property tests. Adding a couple of pytest cases would prevent regressions for the new domain-based AOKI flow.
@property
def sha256_fingerprint_display(self) -> str:
"""Returns the SHA256 Fingerprint for display purposes.
This property returns the SHA256 Fingerprint of the IDevID if available, otherwise that of the domain CA.
"""
if fingerprint := self.idevid_sha256_fingerprint:
return fingerprint
if domain_fp := self.domain_ca_sha256_fingerprint:
return f'Pinned CA: {domain_fp}'
return ''
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (5)
trustpoint/aoki/management/commands/aoki_setup_idevid_test_env.py:185
- This method loads
ownerid_ca_pk.pembut_ensure_certs_exist()doesn’t require it, so an environment created with an olderaoki_gen_test_certs(or partial files) can pass the initial check and then fail here with FileNotFoundError. Add an explicit existence check with a clear error (or trigger regeneration) before attempting to read the key.
domain_ca_cert = domain_ca.get_certificate()
owner_ca_cert = x509.load_pem_x509_certificate(OWNER_CA_CERT_PATH.read_bytes())
owner_ca_key = load_pem_private_key(OWNER_CA_KEY_PATH.read_bytes(), password=None)
trustpoint/aoki/tests/cmp_client.py:226
- The exception variable is unused here; use
as _to avoid lint failures (and to make it clear the exception is intentionally ignored).
except AokiClientOwnerIdCertVerificationError as e:
self._verify_matches_domain_ca_cert(owner_id_cert)
trustpoint/aoki/management/commands/aoki_gen_test_certs.py:49
- Leaving a commented-out call block in the command makes it unclear how domain-based DevOwnerID certs are expected to be generated and invites accidental re-enabling. Either remove it or replace it with a short explanatory comment pointing to the command that actually generates these certs.
# AokiTestCertGenerator.generate_domain_ca_owner_id_cert(
# domain_ca_cert=idevid_cert, owner_ca_cert=owner_ca_cert, owner_ca_key=owner_ca_key
# )
trustpoint/aoki/management/commands/aoki_setup_idevid_test_env.py:191
- There should be a blank line between the end of this method and the next method definition; without it, formatters/linters typically flag the file.
)
def _get_or_create_idevid_truststore(self) -> TruststoreModel:
trustpoint/templates/devices/zero_touch_credentials/define_cert_content_est.html:64
- The example SHA-256 fingerprint is much shorter than a real SHA-256 hex digest (64 hex chars), which can mislead users when forming URIs. Use a full-length example or explicitly mark it as truncated.
<code>dev-owner:cert:<IDevID_Subject_SN>_<IDevID_SHA256_Fingerprint></code> or<br>
<code>dev-owner:uri:urn:uuid:<Device UUID></code><br>
Example: <code>dev-owner:cert:SN-12345678_8a58abcdef123456789</code>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (5)
trustpoint/aoki/management/commands/aoki_setup_idevid_test_env.py:120
_ensure_certs_exist()does not includeOWNER_CA_KEY_PATHinrequired_files, but_get_or_create_domain_ca_owner_id_cert()reads that key to generate the domain-based DevOwnerID. If the CA key file is missing while the other files exist, the command will skip regeneration and then fail later when trying to read the key.
def _ensure_certs_exist(self) -> None:
"""Run ``aoki_gen_test_certs`` if any required certificate file is missing."""
required_files = [
IDEVID_CA_CERT_PATH,
OWNER_ID_CERT_PATH,
OWNER_ID_KEY_PATH,
OWNER_CA_CERT_PATH,
]
trustpoint/aoki/tests/cmp_client.py:116
_verify_matches_domain_ca_cert()currently treats every certificate infull_chain.pemas a potential domain CA and will also crash with an unhandled exception if the OwnerID cert has no SAN extension. Filter to CA certificates (BasicConstraints.ca == True) and handle missing SAN explicitly so the verification cannot produce false positives or unexpected crashes.
def _verify_matches_domain_ca_cert(self, owner_id_cert: x509.Certificate) -> None:
"""Verify the Owner ID certificate is valid for the domain issuing CA certificate."""
print('Verifying Owner ID certificate matches Domain CA certificate')
candidate_ca_certs = self._load_certificates(CERTS_DIR / 'full_chain.pem')
for domain_ca_cert in candidate_ca_certs:
trustpoint/templates/devices/zero_touch_credentials/define_cert_content_est.html:64
- The example
dev-owner:cert:URI shows a truncated SHA-256 fingerprint, but the actual value produced byCertificate.fingerprint(...).hex()is 64 hex characters. This example may lead users to enter an invalid reference; use a clear placeholder (or a full-length example) to avoid confusion.
One URI per line. The Zero-Touch Credential profile requires at least one URI
referencing an IDevID in the format:<br>
<code>dev-owner:cert:<IDevID_Subject_SN>_<IDevID_SHA256_Fingerprint></code> or<br>
<code>dev-owner:uri:urn:uuid:<Device UUID></code><br>
Example: <code>dev-owner:cert:SN-12345678_8a58abcdef123456789</code>
trustpoint/request/authorization/base.py:234
- The new domain-based fallback (
get_domain_based_owner_credential(context.domain)) changes the authorization behavior for AOKI CMP requests but is not covered by tests. Please add request-layer tests that assert: (1) direct DevOwnerID match is preferred when present, (2) domain-based credential is accepted when direct match is missing, and (3) missing/invalid domain information does not accidentally authorize.
owner_credential = (
AokiServiceMixin.get_owner_credential(client_cert)
or AokiServiceMixin.get_domain_based_owner_credential(context.domain)
)
trustpoint/aoki/management/commands/aoki_gen_test_certs.py:49
- The commented-out
generate_domain_ca_owner_id_cert(...)call leaves dead code in a management command. If the domain-based DevOwnerID is intentionally generated elsewhere (e.g., inaoki_setup_idevid_test_env), replace this commented block with a short explanatory comment (or remove it) to avoid confusion about the intended workflow.
idevid_cert = AokiTestCertGenerator.generate_idevid_pki()
owner_ca_cert, owner_ca_key = AokiTestCertGenerator.generate_owner_id_ca()
AokiTestCertGenerator.generate_owner_id_cert(idevid_cert, owner_ca_cert=owner_ca_cert, owner_ca_key=owner_ca_key)
# AokiTestCertGenerator.generate_domain_ca_owner_id_cert(
# domain_ca_cert=idevid_cert, owner_ca_cert=owner_ca_cert, owner_ca_key=owner_ca_key
# )
print('Certificates generated successfully.')
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (5)
trustpoint/aoki/management/commands/aoki_setup_idevid_test_env.py:184
- CaModel.get_certificate() can return None. If the issuing/domain CA certificate is missing, this will pass None into generate_domain_ca_owner_id_cert(), causing an attribute error when fingerprint() is called. This should fail fast with a clear error message.
domain_ca_cert = domain_ca.get_certificate()
owner_ca_cert = x509.load_pem_x509_certificate(OWNER_CA_CERT_PATH.read_bytes())
trustpoint/aoki/tests/cmp_client.py:226
- The caught exception variable "e" is unused here. With ruff enabled in this module, this will trigger F841 (local variable assigned but never used).
try:
self._verify_matches_idevid_cert(owner_id_cert, idevid_cert)
except AokiClientOwnerIdCertVerificationError as e:
self._verify_matches_domain_ca_cert(owner_id_cert)
trustpoint/aoki/management/commands/aoki_gen_test_certs.py:161
- This comment says the URI is built from an "idevid" fingerprint, but it is actually pinning the domain CA certificate fingerprint. The mismatch is confusing when debugging SAN formats.
# Build URI string "dev-owner:ca:<idevid_sha256_fingerprint>"
ca_san_uri = f'dev-owner:ca:{ca_sha256_fingerprint}'
trustpoint/aoki/views.py:96
- Domain-based DevOwnerID lookup is newly introduced here, but there are no unit tests covering the fallback behavior (e.g., when no IDevID-specific DevOwnerID exists, the domain CA pinning reference is used). Adding tests would prevent regressions in the lookup/ref resolution logic.
ca_sha256_fingerprint = domain_ca_cert.fingerprint(hashes.SHA256()).hex()
owner_cred_ref = IDevIDReferenceModel.objects.filter(idevid_ref=f'dev-owner:ca:{ca_sha256_fingerprint}').first()
if not owner_cred_ref:
return None
owner_cred = owner_cred_ref.dev_owner_id
trustpoint/pki/models/credential.py:847
- sha256_fingerprint_display() returns a user-facing string with a hard-coded English prefix ("Pinned CA:"). Since the UI uses Django i18n elsewhere, this should be translated (or the label should be moved into the template where {% trans %} can be used).
if domain_fp := self.domain_ca_sha256_fingerprint:
return f'Pinned CA: {domain_fp}'
|
FHatCSW
approved these changes
Aug 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of changes
Notes
Legal