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

Initial PR to add support for IDevID and IAK #608

Merged
merged 1 commit into from
Sep 25, 2023

Conversation

Isaac-Matthews
Copy link
Contributor

@Isaac-Matthews Isaac-Matthews commented Jun 29, 2023

This is the first of a series of PRs that will enable the use of IDevIDs and IAKs as proposed in enhancement 81.
The modifications to the Keylime repositories are:

Keylime
Columns for the IDevID and IAK are added to the database
The IAK and IDevID sent by the agent are processed
The AK is verified by the IAK

Rust-Keylime
Config options to enable IDevID and IAK generation and use are added, with choices on how to generate the IDevID and IAK
The IAK and IDevID are generated according to config options
The IAK is used to certify the AK in the registration process

The purpose of this PR is to begin enabling the use of IAK and IDevID in keylime, starting with their generation. Options for the server config are not yet added so the inclusion of IAK and IDevID are left as optional for the agent.
The changes are all backwards compatible. The use of the IAK to certify and verify the AK is to act as a simple early use case in the process. Down the road this will be changed to encompass all the options listed in the enhancement, and those options will be included in the Keylime config.

N.B.
The certify in Rust-keylime currently just uses the UUID as qualifying data. This should perhaps be changed to include some temporal data at some point.

@Isaac-Matthews Isaac-Matthews marked this pull request as ready for review August 2, 2023 16:24
@THS-on THS-on requested review from aplanas and ansasaki August 3, 2023 05:31
keylime-agent.conf Show resolved Hide resolved
keylime/src/tpm.rs Show resolved Hide resolved
keylime/src/tpm.rs Show resolved Hide resolved
keylime/src/tpm.rs Show resolved Hide resolved
keylime/src/tpm.rs Show resolved Hide resolved
keylime/src/tpm.rs Show resolved Hide resolved
keylime/src/tpm.rs Show resolved Hide resolved
@stefanberger
Copy link
Contributor

Another question: Does the agent need to know (at this point in the series of changes or later?) that the new keys are supported on the server side and so they do not go into the void?

@ansasaki ansasaki added the configuration Involves changes to configuration file format label Aug 31, 2023
@ansasaki
Copy link
Contributor

I think this feature is useful, I'm looking forward for it to be finished.

I have some concerns around the addition of new configuration options. We have treated this lightly, but for me the correct would be to bump the minor version of the configuration and add the required mappings and templates for the configuration upgrades to make sure it will work in an upgrade scenario.

The new options defaults should be reflected on the mappings, similar to what was created for version 2.0 in https://github.com/keylime/keylime/tree/master/templates/2.0

I can help on this if the required changes are not clear. To start I'll write the documentation that is currently missing to explain how the configuration upgrades are supposed to be extended and maintained.

Also, it would be great if tests were added for the feature. I think the tests repository evolved quite well, and new tests can be quite easily added based on the available examples. Also the used tools are handy (although the usage for keylime could be better explained in the repo README).

@Isaac-Matthews
Copy link
Contributor Author

Another question: Does the agent need to know (at this point in the series of changes or later?) that the new keys are supported on the server side and so they do not go into the void?

My thought was that the requirements for registration and enforcement would all be set on the server side and so this doesn't need to be communicated to the agent.

@Isaac-Matthews
Copy link
Contributor Author

I have some concerns around the addition of new configuration options. We have treated this lightly, but for me the correct would be to bump the minor version of the configuration and add the required mappings and templates for the configuration upgrades to make sure it will work in an upgrade scenario.

The new options defaults should be reflected on the mappings, similar to what was created for version 2.0 in https://github.com/keylime/keylime/tree/master/templates/2.0

Makes sense. Could you please send me the docs on that and I will make the changes (here or on slack). Thanks.

Also, it would be great if tests were added for the feature. I think the tests repository evolved quite well, and new tests can be quite easily added based on the available examples. Also the used tools are handy (although the usage for keylime could be better explained in the repo README).

Definitely intend to include tests; I planned on adding them alongside the next patch that will enable proper use of this feature as they should test the full process. Let me know if that isn't OK.

@Isaac-Matthews Isaac-Matthews force-pushed the idevid_iak_initial branch 2 times, most recently from c9d5cf6 to 0789b02 Compare September 19, 2023 14:05
@ansasaki
Copy link
Contributor

/packit retest-failed

keylime-agent/src/main.rs Outdated Show resolved Hide resolved
keylime-agent/src/main.rs Outdated Show resolved Hide resolved
keylime-agent/src/main.rs Outdated Show resolved Hide resolved
keylime-agent/src/main.rs Outdated Show resolved Hide resolved
keylime-agent/src/main.rs Outdated Show resolved Hide resolved
Signed-off-by: Isaac Matthews <isaac.matthews@hpe.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
configuration Involves changes to configuration file format
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants