Skip to content

fix(tpm): preserve signature algorithm metadata - #149

Merged
imran-siddique merged 3 commits into
agentrust-io:mainfrom
noah-ing:fix/tpm-signature-algorithm-binding
Sep 6, 2026
Merged

fix(tpm): preserve signature algorithm metadata#149
imran-siddique merged 3 commits into
agentrust-io:mainfrom
noah-ing:fix/tpm-signature-algorithm-binding

Conversation

@noah-ing

@noah-ing noah-ing commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

What

Preserve the parsed TPMT_SIGNATURE scheme and digest when cA2A delegates TPM
report verification to Agent Manifest. The report path now passes
ParsedSignature instead of stripping it to bare signature bytes.

The lower-level compatibility API now distinguishes its two accepted forms
explicitly: ParsedSignature carries envelope metadata, while bytes means the
historical bare-signature form. Bare bytes are tagged as ECDSA/SHA-256 or
RSASSA/SHA-256 according to the AK public-key type before delegation, rather
than being classified from their arbitrary first two bytes.

The regression coverage exercises both direct APIs and the complete
serialize_channel_offer -> parse_channel_offer -> verify_offer -> tpm_verifier
path.

Why

cA2A already parsed sig_alg and hash_alg, but verify_tpm_report() passed
only parsed.signature onward. Agent Manifest therefore applied its legacy
bare-signature defaults instead of the operation declared by the TPM envelope.
For RSA reports, that produced the following inconsistent outcomes on current
main:

Vector Current main This change
genuine RSAPSS/SHA-384 rejected accepted
RSASSA/SHA-256 falsely declared RSAPSS accepted rejected
RSASSA/SHA-256 falsely declared SHA-384 accepted rejected

The signature still had to be cryptographically valid under the trusted AK, so
this was not arbitrary-signature acceptance. It was algorithm confusion: the
reported scheme/digest and the operation actually verified could disagree.

Review also exposed a separate ambiguity in the legacy lower-level bytes
form. A valid 256-byte RSASSA/SHA-256 signature can begin 00 16; treating
those arbitrary bytes as a possible marshalled envelope mistakes 0x0016 for
TPM_ALG_RSAPSS and rejects valid evidence while parsing. The frozen synthetic
regression fixture contains such a genuine signature and no private key. cA2A
now makes its documented bare-byte interpretation explicit before delegation.

Agent Manifest 0.11.1 already supports ParsedSignature, and cA2A's current
dependency floor is already agent-manifest>=0.11.1, so no dependency change
is needed. This is a follow-up on the TPM signature-parser consolidation noted
in #77; it does not close #77's separate Azure certificate-chain limitation.

Security impact

This changes a security-critical offline attestation path. The signature scheme
and digest declared in TPMT_SIGNATURE now select the verification operation,
so relabelling either field without producing a matching signature fails.

The algorithm fields are not themselves inside the AK-signed TPMS_ATTEST
bytes. This change establishes consistency between those envelope declarations
and the cryptographic operation; it does not add an algorithm-strength policy.
It does not alter AK-chain trust, quote type/magic checks, PCR appraisal,
qualifying-data binding, key residency, or nonce policy.

The bare-byte prefix collision was a fail-closed rejection of valid evidence,
not a bypass or forged-evidence acceptance. A caller holding a marshalled
TPMT_SIGNATURE must parse it first or use the report API; bytes at the cA2A
lower-level boundary now unambiguously means a legacy bare signature.

The new non-default algorithm vectors are self-consistent synthetic RSA/AK
fixtures. The previously documented live Azure vTPM observation remains
RSASSA/SHA-256; this PR does not claim a live hardware RSAPSS/SHA-384 run.

Test plan

  • pytest tests/unit/ tests/conformance/ -v --tb=short --cov=src — 545
    passed, 2 pre-existing hardware-fixture skips, 90.24% coverage
  • Exact published dependency floor (agent-manifest==0.11.1) — 48 focused
    TPM tests passed, including the frozen 00 16 bare-RSA vector
  • ruff check src/ tests/
  • ruff format --check src/ tests/
  • mypy src/ca2a_runtime/ src/ca2a_verify/ — 44 source files
  • bandit -r src/ -c pyproject.toml
  • pip-audit — no known dependency vulnerabilities; local unpublished
    project skipped as expected
  • Wheel/sdist build, strict Twine validation, forbidden-content checks, and
    fresh Python 3.12 wheel and sdist install/import/CLI smokes
  • MkDocs build using the repository's deployment assembly. Strict mode is
    not claimed: the existing project-wide config emits unrelated
    SPONSORS.md/LICENSE warnings.
  • Manual hardware test — not performed for the new PSS/SHA-384 case; the
    synthetic-vs-live boundary is documented above

DCO sign-off

Signed-off-by: Noah Ingwers <98993329+noah-ing@users.noreply.github.com>
@noah-ing
noah-ing requested a review from a team as a code owner September 3, 2026 22:28
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

🟡 Contributor Check: MEDIUM

Check Result
Profile MEDIUM
Credential LOW
Overall MEDIUM

Automated check by AgenTrust Contributor Check.

@github-actions github-actions Bot added the needs-review:MEDIUM Contributor check flagged MEDIUM risk label Sep 3, 2026
@codecov-commenter

codecov-commenter commented Sep 3, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/ca2a_verify/tpm.py 92.30% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@devdiv07

devdiv07 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Ran this against f82cea3 rather than just reading the diff. On the frozen synthetic RSASSA/SHA-256 vector I had:

Path Result
verify_tpm_report(...) with the full TPMT_SIGNATURE PASS
verify_tpm_quote(..., ParsedSignature) PASS
verify_tpm_quote(..., bare RSA bytes) FAIL

The parsed-signature change in this PR resolves the issue for the report path; what remains is the legacy bytes form.

The residual I found is in the new compatibility wording rather than the report-path change.

attestation.md now says:

verify_tpm_quote is the lower-level compatibility form taking an attest blob plus either parsed signature metadata or a legacy bare signature; a bare RSA signature retains the RSASSA/SHA-256 defaults.

I have a deterministic counterexample to that last part. A truthful TPMT_SIGNATURE declaring sig_alg=0x0014 / hash_alg=0x000b extracts to a valid 256-byte RSASSA/SHA-256 signature whose first two bytes happen to be 00 16.

0x0016 is TPM_ALG_RSAPSS, so when those already-bare bytes are passed to agent_manifest.verify_tpm_quote, the bytes-compatibility branch interprets them as a marshalled TPMT_SIGNATURE and parses them again:

TpmVerificationError: TPMT_SIGNATURE truncated inside the RSA signature

The legacy RSASSA/SHA-256 default is therefore never reached for this input. The signature itself verifies directly against the AK under PKCS#1 v1.5 / SHA-256.

This case is already inside the documented RSASSA/SHA-256 profile, so it is separate from the PSS/SHA-384 behaviour this PR is adding.

I also checked the tests. The existing bare-byte verify_tpm_quote cases use ECDSA DER signatures, which begin 0x30 and so cannot reach that branch, while the new lower-level RSA case passes a ParsedSignature. I could not find coverage for the specific condition: a valid bare RSA signature under RSASSA/SHA-256 whose first two bytes are 00 16, passed through the legacy bytes API.

I have not tried to prescribe the resolution here; the point I wanted to surface is that the unconditional bare-signature compatibility statement has at least this counterexample.

I have the frozen vector if it is useful as a regression: synthetic, offline-replayable, and containing no private key.

Scope-wise, this is a fail-closed rejection of valid evidence, not a bypass or forged-evidence acceptance.

Signed-off-by: Noah Ingwers <98993329+noah-ing@users.noreply.github.com>
@noah-ing

noah-ing commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Thank you — this is a real counterexample, and I reproduced the same dispatch
failure independently against f82cea3:

  • valid 256-byte RSASSA/SHA-256 signature beginning 00 16
  • bare bytes passed directly to the released shared verifier: rejected while
    being parsed as a truncated TPMT_SIGNATURE
  • the same signature wrapped as ParsedSignature(0x0014, 0x000b, ...): accepted

I pushed 1b2ec1e to address it at cA2A's compatibility boundary. The
lower-level cA2A API now makes its documented distinction explicit:

  • bytes is a legacy bare signature and is tagged ECDSA/SHA-256 or
    RSASSA/SHA-256 according to the AK public-key type before delegation;
  • ParsedSignature retains and applies its declared scheme and digest; and
  • callers holding a marshalled TPMT_SIGNATURE must parse it first (or use the
    report API).

That avoids a parse-failure fallback and removes the prefix heuristic from this
API without weakening the report-path algorithm binding. I also replaced the
unconditional compatibility wording and added a frozen, private-key-free
synthetic regression vector for the exact 00 16 case.

Verification after the change:

  • 545 passed, 2 pre-existing hardware-fixture skips, 90.24% coverage
  • 48 focused TPM tests passed at the published agent-manifest==0.11.1 floor
  • Ruff, formatting, mypy, Bandit, pip-audit, wheel/sdist/Twine and fresh-install
    smokes passed

Thanks for running the code and isolating the residual to the legacy form. The
scope remains as you characterized it: valid-evidence rejection, not forged-
evidence acceptance.

@devdiv07

devdiv07 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

@noah-ing Re-ran my frozen vector against 1b2ec1e — the legacy bytes path now passes, and the upstream second parse count is zero. The cA2A boundary now passes a typed ParsedSignature(0x0014, 0x000b, ...) with the original signature bytes unchanged, so the prefix no longer determines structure. Thanks for reproducing it and for adding the regression fixture.

@carloshvp carloshvp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head 1b2ec1eb7d1da5b4ba201194d1952bcf409d16cf against the published dependency floor agent-manifest==0.11.1.

The report path retains ParsedSignature metadata, and the lower-level bare-byte path uses the AK key type rather than a signature-prefix heuristic. The frozen 00 16 RSA regression passes. An independent 36-case signed wire-level matrix covering RSASSA/RSAPSS, SHA-256/384/512, and relabelled scheme/digest combinations accepted only matching declarations.

Validation: unit/conformance suite 545 passed, 2 skipped. The skips are hardware-fixture tests; these results use synthetic signatures and do not claim new hardware validation. No blocking findings. Substantive CI is green; the remaining failure is the maintainer-approval gate.

@imran-siddique imran-siddique left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed preservation and enforcement of parsed TPM signature scheme and digest metadata, including legacy bare-signature handling and the fixed RSA prefix fixture. All 58 focused TPM tests pass locally; restoring the previous verifier produces seven failures covering the digest/scheme checks and reference-wire paths.

@imran-siddique imran-siddique left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed preservation and enforcement of parsed TPM signature scheme and digest metadata, including legacy bare-signature handling and the fixed RSA prefix fixture. All 58 focused TPM tests pass locally; restoring the previous verifier produces seven failures covering the digest/scheme checks and reference-wire paths.

@imran-siddique
imran-siddique merged commit ffd2046 into agentrust-io:main Sep 6, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-review:MEDIUM Contributor check flagged MEDIUM risk

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tee: TPM key provenance is host-dependent on Azure, and the local TPMT_SIGNATURE parser is a duplicate

5 participants