-
Notifications
You must be signed in to change notification settings - Fork 8
DRAFT: trustee: Configure reference values based on all possible PCR combinations #116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
39cefcc to
0c7a05f
Compare
|
@bgartzi because the recent changes to compute-pcrs-lib are incompatible, I am ignoring it for dependabot or it keeps updating (see #121). Hoping that none of us forget it, reminder for you and myself to unignore it after this PR ( |
Minimum rust version was set to 1.85. Fedora is way above that threshold at the moment. Future EL releases will be above that as well. While on it, fix some of the linter errors that arise from the minimum version update. Signed-off-by: Beñat Gartzia Arruabarrena <bgartzia@redhat.com>
And update those crates that were making use of it. Later we will also use it in some other crates. Signed-off-by: Beñat Gartzia Arruabarrena <bgartzia@redhat.com>
I broke some of the compute-pcrs' lib APIs, so apart from just pointing into a newer commit, we also need to tweak a couple of things here and there. Signed-off-by: Beñat Gartzia Arruabarrena <bgartzia@redhat.com>
trustee tests were relying on some values that didn't hold any TPMEvent or any PCR value that related to them. This commit turns those dummy values into something that are closer to something we could expect in reality. The main reason to do this is that the compute-pcrs logic does not like empty event PCRs, as it's based on that to reconstruct them. This goes against the purpose of a unit test, as we should actually mock that external part so as not to rely on it, but decided this was simpler at least as an initial proposal. Signed-off-by: Beñat Gartzia Arruabarrena <bgartzia@redhat.com>
Update trustee reference values with all possible combinations between approved PCR values, not just those that come directly from image PCRs. This computes reference values for possible stages during node updates in which a node could be booting some components from image A and some other from image B. This commit also adds a test to check that pcr4 is well covered in front of bootloader and kernel updates. The test is closer to an integration test than to an unit-test as it relies on values that are close to real values, and how the compute-pcrs lib's combine_images processes them. Signed-off-by: Beñat Gartzia Arruabarrena <bgartzia@redhat.com>
This is an edge case that the compute-pcrs library should be able to
handle by itself.
A PR with a fix has been opened, but not merged yet
trusted-execution-clusters/compute-pcrs#56
Once fixed and merged, I will remove this workaround.
0c7a05f to
69d8614
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: bgartzi The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@bgartzi: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Now the compute-pcrs library supports computing PCR value combinations. In other words, given all events from image A and image B (where some of the components got updated), it computes all possible PCRs of the intermediate states that a node could go through during an update from image A to image B.
While implementing this I found a compute-pcrs bug that this PR relies on: trusted-execution-clusters/compute-pcrs#56
For now, I added a workaround commit that is not signed-off-by me. Once the compute-pcrs library gets merged, I will update this PR.
Another discussion topic that comes to my mind is how unit tests are implemented to face this new integration. They sure could be way more isolated from compute-pcrs, which I will update if relevant.