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

TPM Provider: Check root key's name #751

Merged

Conversation

tgonzalezorlandoarm
Copy link
Member

Check the name of the TPM's root key, comparing it with a previously stored version

tgonzalezorlandoarm and others added 17 commits March 14, 2024 18:05
TryFrom<ProviderIdentity> for ProviderId is currently missing the
Core provider.

 * Add the Core Provider for the conversions between
   ProviderIdentity and ProviderId

This missing provider was spotted during on_disk_manager testing for
internal keys.

Signed-off-by: Tomás González <[email protected]>
* The KIMs now allow providers to identify themselves with a special
"authenticator" ID, marking them as service-internal and allowing them
to store any internal metadata required.
* The AIK has been moved to this per-provider metadata store, removing
potential conflicts with actual client applications.

Signed-off-by: Ionut Mihalcea <[email protected]>

SQLite: Fix for reading internal keys

When the database is read after reload, internal keys which were stored with
a authenticator_id row value "255" fails to find the correct Auth value.

Signed-off-by: Gowtham Suresh Kumar <[email protected]>

Use constant data associated with Auth::Internal

 * Create constants like INTERNAL_APP_NAME to fix the name of the
   applications for internally generated ApplicationIdentity.

Signed-off-by: Tomás González <[email protected]>
Keys should be marked as internal or not in the OnDiskManager when
stored.

The object that stores the Key Information (PasswordContext),
however, does not have the capacity to store the Auth information.
When a similar problem arose, a LegacyPasswordContext was created.

This commit avoids having to create a new PasswordContext and allows
not to treat internal keys like any other keys so as not to expose
them.

Signed-off-by: Tomás González <[email protected]>
The internal key mappings and the key entries in the Hash Map
should be possible to delete as it is provider specific. This
patch adds this support.

Signed-off-by: Gowtham Suresh Kumar <[email protected]>
1) While reading a pre-existing database, the key name was parsed and
   formatted in the wrong way to save the file path as the name. This
   has been rectified.
2) Bool was being stored in the internal hash-map. This has been
   repleaced to store the required KeyInfo.

Signed-off-by: Gowtham Suresh Kumar <[email protected]>
To recognize which provider created an internal key, prepend the
provider name to the internal key's file name when storing it on
disk.

Signed-off-by: Tomás González <[email protected]>
Signed-off-by: Gowtham Suresh Kumar <[email protected]>

on_disk_manager: create_and_load_internal_keys

Cases covered by the test after the changes:

 * get() should return the key info of the internal key
 * get_all() should not return the correct amount of Internal Keys.
 * get() should not return internal keys if the Client Auth is used.
 * get_all() should return 0 after internal key removal.

Signed-off-by: Tomás González <[email protected]>
The test creates internal keys for different providers, loads them
and finally deletes them to ensure internal keys are handled as
expected.

Signed-off-by: Gowtham Suresh Kumar <[email protected]>

SQLite: create_and_load_internal_keys

 * get() should return the key info of the internal key
 * get() should not return internal keys if the Client Auth is used.
 * get_all() should return 0 when all internal keys have been
   removed.

Signed-off-by: Tomás González <[email protected]>
The key info managers should not expose any internal key information
when listing keys for the clients or when listing clients.

Co-authored by: Gowtham Suresh Kumar <[email protected]>

Signed-off-by: Tomás González <[email protected]>
Signed-off by: Gowtham Suresh Kumar <[email protected]>
The Public Part of the Root Key in the TPM generated by the tss-esapi
layer in the TPM can be checked with a previously public part stored
internally.

Up until this commit, the storing of that information is not done
in the code: This commit only covers the part of checking the Public
Part, not of storing it.

Signed-off-by: Tomás González <[email protected]>
A PasswordContext with no auth_value is stored. The private part of
the KeyMaterial is empty.

Signed-off-by: Tomás González <[email protected]>
test_root_key_check checks that:

 * A TPM Provider is built, the root internal key is generated,
   and when the provider is reloaded the internal key check
   suceeds.
 * When modifying the key information, the internal key check fails

Signed-off-by: Tomás González <[email protected]>
kim-mappings are currently not being deleted.

 * Delete all mappings when switching to all providers tests instead
   of just some of them.

This is needed to avoid previous tests from corrupting the results
of the all providers tests.

Signed-off-by: Tomás González <[email protected]>
@tgonzalezorlandoarm tgonzalezorlandoarm added the enhancement New feature or request label Mar 14, 2024
@tgonzalezorlandoarm tgonzalezorlandoarm self-assigned this Mar 14, 2024
@tgonzalezorlandoarm tgonzalezorlandoarm requested a review from a team as a code owner March 14, 2024 18:14
Copy link
Contributor

@gowthamsk-arm gowthamsk-arm left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks:)

@tgonzalezorlandoarm tgonzalezorlandoarm merged commit f2a459e into parallaxsecond:main Mar 15, 2024
16 checks passed
@tgonzalezorlandoarm tgonzalezorlandoarm deleted the tg/root-key-check branch March 18, 2024 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants