Use available PCR bank for Clevis TPM2 encrypt/decrypt tests (bugfix) - #2734
Open
pieqq wants to merge 16 commits into
Open
Use available PCR bank for Clevis TPM2 encrypt/decrypt tests (bugfix)#2734pieqq wants to merge 16 commits into
pieqq wants to merge 16 commits into
Conversation
Split the linear code into functions, and use argparse for argument parsing. The output remains the same.
Modify the script to add a `--resource-pcr-banks` option that spits out
pcr_bank resource records for each supported PCR bank.
For example:
pcr_bank: sha256
pcr_bank: sha384
For each PCR bank identified with the resource job, a separate job is
created to test the encryption against the specific PCR bank.
Since it's standard practice to have the same value for hashing
algorithm and PCR bank, the same {pcr_bank} value is used when running
the clevis-encrypt-tpm2 for both `hash` and `pcr_bank`.
The clevis-automated test plan is updated to use the template jobs
instead of the older ones.
They have been superseded by their template jobs equivalent and are not used anywhere in our generic providers.
Add sample data from a desktop device with SHA256 PCR banks and hash, and use it in some unit tests.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2734 +/- ##
=======================================
Coverage 60.03% 60.03%
=======================================
Files 487 487
Lines 48935 48935
Branches 8765 8765
=======================================
Hits 29377 29377
- Misses 18636 18639 +3
+ Partials 922 919 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the TPM2 Clevis encrypt/decrypt provider tests to dynamically use whatever PCR bank(s) the DUT exposes (e.g. sha384-only systems), instead of hardcoding sha256. It does this by adding a PCR-bank resource job and converting the RSA/ECC encrypt/decrypt jobs into templates instantiated per available PCR bank.
Changes:
- Add a
tpm2_pcr_banksresource job and wire it into relevant test plans (including canary bootstrapping). - Convert Clevis RSA/ECC encrypt/decrypt jobs into templates keyed by
pcr_bank, and update test plan inclusion accordingly. - Extend
tpm2_capabilities.pywith explicit--resource/--resource-pcr-banksmodes and add unit tests + sampletpm2_getcapoutputs.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| providers/tpm2/units/test-plan.pxu | Updates the Clevis automated plan to include templated RSA/ECC jobs and bootstrap the PCR bank resource. |
| providers/tpm2/units/resource.pxu | Adds a new tpm2_pcr_banks resource unit and updates capability resource invocation. |
| providers/tpm2/units/clevis.pxu | Converts RSA/ECC jobs into templates parameterized by {pcr_bank}. |
| providers/tpm2/bin/tpm2_capabilities.py | Refactors capability probing into functions and adds argparse-driven modes for resource outputs. |
| providers/tpm2/tests/test_tpm2_capabilities.py | Adds unit tests for parsing/probing logic and CLI behaviors. |
| providers/tpm2/tests/test_data/tpm2_getcap_pcrs.txt | Adds sample PCR capability output fixture. |
| providers/tpm2/tests/test_data/tpm2_getcap_algorithms.txt | Adds sample algorithms capability output fixture. |
| providers/base/units/canary/test-plan.yaml | Adjusts canary to bootstrap PCR-bank resource and include all templated Clevis RSA/ECC jobs. |
Comments suppressed due to low confidence (1)
providers/tpm2/units/clevis.pxu:71
- The ECC capabilities detection job is still hardcoded to require the sha256 PCR bank and hash, which breaks on TPMs that only provide sha384 PCRs. Since the actual encrypt/decrypt jobs are now templated per available PCR bank, this job should not require sha256 specifically.
tpm2_capabilities.py hash sha256 && \
tpm2_capabilities.py pcr_banks sha256 && \
tpm2_capabilities.py assymetric ecc
unit: template
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No more hardcoded sha256 expectations, and the capabilities tests are still valuable in case some devices in the future have a specific PCR bank that does not match any hashing algorithm.
This provider includes a lot of external Shell scripts that should not be tested, but it's still important to run the unit tests.
These are still hosted in an external provider and it creates problems in our CI/CD.
There's no need for the FullLoader, since this is used only to parse some config-like strings.
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.
WARNING: This modifies com.canonical.certification::sru-server
Description
Until now, the Clevis-based TPM2 encrypt/decrypt tests were hardcoded to use the sha256 PCR bank and hashing algorithm. Newer devices come with support for sha384 and drop support for sha256, so these tests don't work anymore.
The proposed solution is to create an additional resource job that outputs the supported
pcr_bank, and turn the RSA and ECC encrypt/decrypt tests into templates. The value for PCR bank is used to fill bothpcr_bankandhashin the clevis command since it's a standard practice to do so and I didn't want to create a lot of different iterations on the same test.The old jobs are removed.
Resolved issues
Fix #2549
Fix https://warthogs.atlassian.net/browse/CHECKBOX-2265
Documentation
Tests
sha256only)clevis-automatedtest plan and making sure the aforementioned templated jobs get executed: