Skip to content
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

Allow setting binary measured boot log path on RPM binaries #554

Merged
merged 1 commit into from
Apr 11, 2023

Conversation

ansasaki
Copy link
Contributor

@ansasaki ansasaki commented Apr 4, 2023

Allow setting the path to the binary bios measurements log file via the TPM_BINARY_MEASUREMENTS environment variable when build with the testing feature enabled.

The binaries in the RPM are build with the testing feature enabled.

Fixes #490

@codecov
Copy link

codecov bot commented Apr 4, 2023

Codecov Report

Merging #554 (cd9c214) into master (bf33c9a) will decrease coverage by 0.01%.
The diff coverage is 50.00%.

❗ Current head cd9c214 differs from pull request most recent head 2e9a322. Consider uploading reports for the commit 2e9a322 to get more accurate results

Additional details and impacted files
Flag Coverage Δ
e2e-testsuite 58.72% <ø> (ø)
upstream-unit-tests 58.89% <50.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
keylime-agent/src/common.rs 70.18% <ø> (ø)
keylime-agent/src/main.rs 35.38% <50.00%> (+0.33%) ⬆️

Copy link
Contributor

@aplanas aplanas left a comment

Choose a reason for hiding this comment

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

I am a bit concern that this rpm/ can confuse later. It is to generate RPMs for the testing farm, and somehow unrelated with the project.

IMHO all this RPM code should be living in the test side and not in the project side, as in any case the testing farm is not really testing the released fedora nor centos package.

If this is not possible, at least we should add a README under rpm/ to clarify that "If you are looking for the reference spec file, do not look here"

rpm/fedora/keylime-agent-rust.spec Outdated Show resolved Hide resolved
rpm/fedora/keylime-agent-rust.spec Show resolved Hide resolved
rpm/fedora/rust-keylime-allow-setting-mb-log-path.patch Outdated Show resolved Hide resolved
@ansasaki
Copy link
Contributor Author

ansasaki commented Apr 5, 2023

I am a bit concern that this rpm/ can confuse later. It is to generate RPMs for the testing farm, and somehow unrelated with the project.

IMHO all this RPM code should be living in the test side and not in the project side, as in any case the testing farm is not really testing the released fedora nor centos package.

If this is not possible, at least we should add a README under rpm/ to clarify that "If you are looking for the reference spec file, do not look here"

I agree that it may be confusing at the moment, but the goal in future is to maintain the specfile here and automate downstream maintenance as well, so it will become the "reference specfile".

For now, the rpms will be used to avoid recompiling the agent for testing both in this repo, as well as in keylime/keylime. It makes easier for testing farm to have the rpm packages in Copr.

I'll add a commit to explain this in README.

@ansasaki ansasaki force-pushed the testing_mb branch 2 times, most recently from 0ac932a to 12b97d6 Compare April 6, 2023 12:52
Copy link
Contributor

@aplanas aplanas left a comment

Choose a reason for hiding this comment

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

Seems that the Centos package fails for some GPG key. Not sure why, but the changes LGTM

Allow setting the binary measured boot log path via
TPM_BINARY_MEASUREMENTS environment variable when build with the
'testing' feature.

When building the RPMs, enable the testing feature.

Signed-off-by: Anderson Toshiyuki Sasaki <[email protected]>
@ansasaki
Copy link
Contributor Author

ansasaki commented Apr 11, 2023

I added a small change to make the RPMS more usable: packit was picking the version from the tag (v0.2.0), which made RPM to not consider it an update because keylime-0.1.0 > keylime-v0.2.0 (note the v prepending the version number). I had to manually change the version to result in keylime-0.2.0 for the RPM.

I'll drop the temporary commit and push the changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make measured boot event log location configurable
4 participants