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

Remove serde trait implementations for requests and replies #184

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

robin-nitrokey
Copy link
Member

Implementing Serialize and Deserialize for the request and reply structs leaks implementation details, especially when using serde_indexed. This patch removes these implementations for the request and reply structs and also for some other types that presumably only had them because they were used in these structs and that are not serialized or deserialized anywhere in the Trussed ecosystem.

Fixes: #183

@sosthene-nitrokey
Copy link
Contributor

sosthene-nitrokey commented Dec 16, 2024

Seeing the need to implement this in two third-party crates, could this lead to an increase of binary size?

@robin-nitrokey
Copy link
Member Author

Yes, in my tests this led to an increase of ~ 500 bytes. I thought that would just be acceptable. But we could also add WrappedKey to trussed::types.

@sosthene-nitrokey
Copy link
Contributor

I think we can do with 500B increase.

@robin-nitrokey
Copy link
Member Author

But as we need the type in Trussed anyway (for the CryptoClient::wrap_key implementation), we can also just make it public.

@robin-nitrokey
Copy link
Member Author

I’ve added EncryptedData to trussed_core::types. An additional benefit is that it can directly implement the conversion from reply::Encrypt and to request::Decrypt.

Implementing Serialize and Deserialize for the request and reply structs
leaks implementation details, especially when using serde_indexed.  This
patch removes these implementations for the request and reply structs
and also for some other types that presumably only had them because they
were used in these structs and that are not serialized or deserialized
anywhere in the Trussed ecosystem.

To make it easier to store encrypted data – where previously
reply::Encrypt could be used direcly – this patch adds an EncryptedData
struct that implemente Serialize and Deserialize.

Fixes: #183
@robin-nitrokey robin-nitrokey merged commit df2867d into main Dec 17, 2024
2 checks passed
@robin-nitrokey robin-nitrokey deleted the api-serde branch December 17, 2024 09:04
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.

Remove serde implementations for API requests and replies
2 participants