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

New key wrapping API #642

Merged
merged 5 commits into from
Aug 10, 2023
Merged

Conversation

kaczmarczyck
Copy link
Collaborator

Allows key wrapping to be different between persistent and server-side storage.

To accomplish this, the PrivateKey now always stores the fully reconstructed key, and has different methods to serialize it for the respective use case.

Allows key wrapping to be different between persistent and server-side
storage.

To accomplish this, the PrivateKey now always stores the fully
reconstructed key, and has different methods to serialize it for the
respective use case.
@kaczmarczyck
Copy link
Collaborator Author

This PR adds 600 B of binary size.

@kaczmarczyck kaczmarczyck marked this pull request as draft August 9, 2023 14:43
This is a backwards incompatible change. This PR already introduces
backwards incompatible new credential parsing, and therefore we can also
remove all other legacy parsing.
@kaczmarczyck kaczmarczyck marked this pull request as ready for review August 9, 2023 15:35
@kaczmarczyck kaczmarczyck requested a review from ia0 August 9, 2023 15:35
@kaczmarczyck kaczmarczyck assigned zhalvorsen and unassigned zhalvorsen Aug 9, 2023
@kaczmarczyck
Copy link
Collaborator Author

This PR is a breaking change for all credentials created with OpenSK before: both server-side and resident keys.

We can retroactively change that, if we want to be compatible with stable: just revert the second commit. If we also want to be backwards compatible with a some intermediate develop stages, we would have to parse the CBOR in PrivateKey differently: If the byte array is longer, assume that it is encrypted and decrypt. Otherwise just use it as is.

As upgradability was only introduced in develop, there shouldn't yet be users that don't actively pull develop themselves that run into this problem. If you trigger this backwards incompatibility, you notice that your credentials get rejected for server-side, or don't appear to exist for resident keys.

However, the credentials are not actually gone! Flashing a firmware before this PR should solve the problem. You'd lose access to the freshly generated credentials from the new firmware though, as the change is neither backwards not forward compatible.

@coveralls
Copy link

coveralls commented Aug 9, 2023

Coverage Status

coverage: 96.34% (+0.007%) from 96.333% when pulling 25f8c12 on kaczmarczyck:key-wrapping into 96af5e8 on google:develop.

ia0
ia0 previously approved these changes Aug 10, 2023
@kaczmarczyck kaczmarczyck merged commit 87f0711 into google:develop Aug 10, 2023
24 checks passed
@kaczmarczyck kaczmarczyck deleted the key-wrapping branch August 10, 2023 12:20
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.

4 participants