fix: canonicalize TPM EK NV fallback

[martinraumann]

Summary

• Model the TPM EK certificate NV fallback indices with the same names, key types, and order as upstream tpm2_getekcertificate.
• Stop concatenating every readable NV EK certificate into the machine identity input.
• Select the canonical default-output certificate: first readable RSA cert in upstream order, otherwise first readable non-RSA cert.
• Add regression coverage for multi-cert fallback, RSA preference over earlier ECC, ECC-only fallback, invalid cert skipping, and no-cert failure.

Root Cause

The prior fallback was intended to work around tpm2_getekcertificate failing on systems with many NV entries, but it changed identity semantics by concatenating all readable EK certs. Upstream tpm2_getekcertificate may read multiple NV certs internally, but its default stdout behavior emits only one certificate, preferring RSA for backward compatibility.

Upstream Check

Checked tpm2-tools/tools/tpm2_getekcertificate.c on upstream master: ek_index_maps uses the same index order, and process_output writes one RSA certificate first for backward compatibility. With default stdout output, additional NV certs are not concatenated.

Validation

• cargo fmt --package carbide-host-support --check — passed on Linux.
• git diff --check — passed.
• cargo test -p carbide-host-support hardware_enumeration::tpm — passed on Linux: 5 passed, 0 failed.

[copy-pr-bot]

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[coderabbitai]

Summary by CodeRabbit

• Bug Fixes
  • Improved TPM certificate fallback when the primary retrieval method fails.
  • Selects one canonical certificate instead of combining multiple certificates.
  • Prefers RSA certificates for compatibility, with ECC used as a fallback.
  • Provides clearer fallback error details when certificate retrieval fails.

Walkthrough

TPM EK certificate fallback now uses structured NV index metadata, probes all known indices, and returns one canonical certificate. RSA certificates are preferred in table order, with ECC used when no RSA certificate is readable. Tests cover both selection paths.

Changes

TPM EK certificate fallback

Layer / File(s)	Summary
NV metadata and canonical selection crates/host-support/src/hardware_enumeration/tpm.rs	Adds typed NV index metadata, records readable fallback certificates with their key types, and selects the first readable RSA certificate or first readable ECC certificate.
Fallback selection tests crates/host-support/src/hardware_enumeration/tpm.rs	Updates NV probe expectations and fixtures, and verifies RSA preference plus ECC fallback scenarios.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly captures the main change: canonicalizing TPM EK NV fallback behavior.
Description check	✅ Passed	The description is directly aligned with the TPM EK fallback changes and validation.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (2)

crates/host-support/src/hardware_enumeration/tpm.rs (2)

155-170: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use structured tracing fields instead of string interpolation.

nv_index.name and nv_index.index are interpolated into the message string rather than passed as structured fields, breaking logfmt-style parsing for this log line.

As per coding guidelines, "tracing messages should pass common values as structured fields rather than interpolating them into strings" and "Services must emit logs in logfmt syntax".

🧾 Proposed fix

                     Ok(cert) => {
                         tracing::info!(
-                            "Read TPM EK certificate {} from NV index {}",
-                            nv_index.name,
-                            nv_index.index
+                            name = nv_index.name,
+                            index = nv_index.index,
+                            "Read TPM EK certificate from NV index"
                         );
                         certs.push((*nv_index, cert));
                     }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/host-support/src/hardware_enumeration/tpm.rs` around lines 155 - 170,
Update the tracing::info! call in the TPM EK certificate enumeration loop to
pass nv_index.name and nv_index.index as structured tracing fields, while
keeping the message static and preserving the existing log content.

Sources: Coding guidelines, Path instructions

---

421-478: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a scenario with multiple readable non-RSA certs.

Current cases cover multi-RSA tie-break and single-ECC fallback, but nothing exercises first_non_rsa_cert.is_none() guarding against a second readable non-RSA cert overwriting the first (e.g., two readable ECC entries at different table positions, no RSA readable). Worth locking this branch in explicitly.

As per coding guidelines, "Use table-driven tests for functions mapping inputs to outputs, errors, or observable results."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/host-support/src/hardware_enumeration/tpm.rs` around lines 421 - 478,
Add a table-driven test alongside the existing TPM certificate selection
scenarios covering two readable ECC/non-RSA certificates at different NV indices
with no readable RSA certificate. Configure the fake runner so both reads
succeed and assert the selection yields the first ECC certificate, exercising
the first_non_rsa_cert guard without changing the existing RSA-preference cases.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@crates/host-support/src/hardware_enumeration/tpm.rs`:
- Around line 155-170: Update the tracing::info! call in the TPM EK certificate
enumeration loop to pass nv_index.name and nv_index.index as structured tracing
fields, while keeping the message static and preserving the existing log
content.
- Around line 421-478: Add a table-driven test alongside the existing TPM
certificate selection scenarios covering two readable ECC/non-RSA certificates
at different NV indices with no readable RSA certificate. Configure the fake
runner so both reads succeed and assert the selection yields the first ECC
certificate, exercising the first_non_rsa_cert guard without changing the
existing RSA-preference cases.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 1c3fe55b-837f-4ad7-8208-75db9599de6a

📥 Commits

Reviewing files that changed from the base of the PR and between ae62c4b and 0a6b663.

📒 Files selected for processing (1)

• crates/host-support/src/hardware_enumeration/tpm.rs