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.

Sign up for GitHub

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

Jump to bottom

ek, ak abstractions: allow to specficy exact key type #414

Merged
merged 1 commit into from
Jun 20, 2023
Merged

ek, ak abstractions: allow to specficy exact key type #414

merged 1 commit into from
Jun 20, 2023

Conversation

THS-on
Copy link
Contributor

@THS-on THS-on commented Jun 11, 2023

This adds support other keys than RSA2048 and ECC Nist P256.

@THS-on THS-on mentioned this pull request Jun 11, 2023
Closed
@THS-on THS-on force-pushed the feature/algorithmselection branch from ad49441 to d909944 Compare June 11, 2023 13:16
@THS-on THS-on force-pushed the feature/algorithmselection branch 4 times, most recently from 9ba573f to db71335 Compare June 12, 2023 12:56
@THS-on THS-on changed the title ek abstractions: allow to specficy exact key type ek, ak abstractions: allow to specficy exact key type Jun 12, 2023
@THS-on THS-on force-pushed the feature/algorithmselection branch 2 times, most recently from 2630900 to f0575ed Compare June 12, 2023 13:03
@THS-on
Copy link
Contributor Author

THS-on commented Jun 12, 2023

@ionut-arm the Original AsymmetricAlgorithm is consumed by also by the AK abstraction layer. So I now moved AsymmetricAlgorithmSelection into algorithms and then changed the other functions also to it.

The major change introduced is that now create_ak, also requires a key algorithm specified.

ionut-arm
ionut-arm reviewed Jun 17, 2023
View reviewed changes
Copy link
Member

@ionut-arm ionut-arm left a comment

Choose a reason for hiding this comment

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

Thanks for the new and improved patch!

tss-esapi/src/abstraction/ak.rs Show resolved Hide resolved
tss-esapi/src/interface_types/algorithm.rs Outdated Show resolved Hide resolved
tss-esapi/src/interface_types/algorithm.rs Outdated Show resolved Hide resolved
@THS-on THS-on force-pushed the feature/algorithmselection branch from f0575ed to 309e699 Compare June 18, 2023 10:32
ionut-arm
ionut-arm approved these changes Jun 18, 2023
View reviewed changes
Copy link
Member

@ionut-arm ionut-arm left a comment

Choose a reason for hiding this comment

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

Thanks for all the changes! LGTM now, just left a nit comment, but otherwise happy to merge.

Also, I think we can merge despite the MSRV-related failure, we'll have to decide what to do with log.

tss-esapi/src/interface_types/algorithm.rs Outdated Show resolved Hide resolved
@THS-on THS-on force-pushed the feature/algorithmselection branch from 309e699 to 7bb9f79 Compare June 18, 2023 10:59
wiktor-k
wiktor-k approved these changes Jun 19, 2023
View reviewed changes
Copy link
Collaborator

@wiktor-k wiktor-k left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@Superhepper
Copy link
Collaborator

Superhepper commented Jun 19, 2023 •
edited
Loading

I am not against this.

But is it not possible to achieve the same thing using the KeyCustomization trait? And if it is should we really have two ways of doing the same thing?

@THS-on
Copy link
Contributor Author

THS-on commented Jun 19, 2023

But is it not possible to achieve the same thing using the KeyCustomization trait?

@Superhepper for create_ek_public_from_default_template(..) and create_ak_public(..) this should be also doable with the KeyCustomization trait. The issue is with the other functions:

  • retrieve_ek_pubcert(..) we have different NV indexes for different key types, so we need some kind of selection enum.
  • create_ak(..) hard codes RSA 2048 because it derives the key type from the signing algorithm.

I think that changing the key type is a common enough (for example swtpm uses for ECC Nist P384) to provide an simple way to specify it.

@THS-on
ek, ak abstractions: allow to specficy exact key type
04a42ea 
This adds support other keys than RSA2048 and ECC Nist P256.

Signed-off-by: Thore Sommer <[email protected]>
@THS-on THS-on force-pushed the feature/algorithmselection branch from 7bb9f79 to 04a42ea Compare June 19, 2023 16:25
@ionut-arm ionut-arm merged commit 52194ff into parallaxsecond:main Jun 20, 2023
@THS-on
Copy link
Contributor Author

THS-on commented Sep 19, 2024

@ionut-arm @Superhepper would it possible to backport this to 7.X?

This is currently the main blocker for us to enable ECC fully in the rust Keylime agent, see: keylime/rust-keylime#513

@Superhepper
Copy link
Collaborator

Yeah, I am not sure if I have the time to do it any time soon but it is just a matter of writing a PR with changes for the 7.y.x branch instead.

@Superhepper
Copy link
Collaborator

@ionut-arm @Superhepper would it possible to backport this to 7.X?

This is currently the main blocker for us to enable ECC fully in the rust Keylime agent, see: keylime/rust-keylime#513

I tried to adapt your code to get it to work under the 7.x.y branch. #546
please try it out and see if it works or if it causes some weird issues.

@wiktor-k wiktor-k mentioned this pull request Sep 23, 2024
Open
Superhepper added a commit to Superhepper/rust-tss-esapi that referenced this pull request Oct 4, 2024
@THS-on @Superhepper @ionut-arm
This adds support other keys than RSA2048 and ECC Nist P256.
01ac482 
This takes the following PRs and from the main
branch and adapts them so that they can be merged
into the 7.x.y branch:

\parallaxsecond#464 (By Ionut Mihalcea <[email protected]>)
\parallaxsecond#414 (By Thore Sommer <[email protected]>)

Co-authored-by: Jesper Brynolf <[email protected]>
Co-authored-by: Thore Sommer <[email protected]>
Co-authored-by: Ionut Mihalcea <[email protected]>
Signed-off-by: Jesper Brynolf <[email protected]>
Superhepper added a commit to Superhepper/rust-tss-esapi that referenced this pull request Oct 4, 2024
@THS-on @Superhepper @ionut-arm
This adds support other keys than RSA2048 and ECC Nist P256.
1e8633f 
This takes the following PRs and from the main
branch and adapts them so that they can be merged
into the 7.x.y branch:

\parallaxsecond#464 (By Ionut Mihalcea <[email protected]>)
\parallaxsecond#414 (By Thore Sommer <[email protected]>)

Co-authored-by: Jesper Brynolf <[email protected]>
Co-authored-by: Thore Sommer <[email protected]>
Co-authored-by: Ionut Mihalcea <[email protected]>
Signed-off-by: Jesper Brynolf <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Superhepper Superhepper Superhepper approved these changes

@wiktor-k wiktor-k wiktor-k approved these changes

@ionut-arm ionut-arm ionut-arm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@THS-on @Superhepper @wiktor-k @ionut-arm