Skip to content

Consistently use message buffer type aliases#365

Merged
geonnave merged 3 commits intolake-rs:mainfrom
chrysn-pull-requests:initial-363
Jun 2, 2025
Merged

Consistently use message buffer type aliases#365
geonnave merged 3 commits intolake-rs:mainfrom
chrysn-pull-requests:initial-363

Conversation

@chrysn
Copy link
Member

@chrysn chrysn commented May 23, 2025

This cleans up some of #363.

  • The first commit only doing something similar to 4bd8715 in Various small (but semver breaking) enhancements that do not break hax #364 -- just to get the sizes there out of the way.
  • The second commit removes the concrete buffer types from the crypto API: The AES functions just had the type alias in there that they were first used and written for, and then accidentally worked because all the message types were the same size. This is cleaned up by making them take slices and returning owned buffers of a caller chosen size. (Long term I'd like to avoid buffers there completely, but one step after the other).
  • Then, all the cases found by CI TEST ONLY: Evaluate whether buffer sizes are independent #366 were fixed, ie. those where the code relied on the type aliases to all be the same type. (Cases in the C API were not fixed, but fixing that would be excessive -- there, we'll just need to stick with some defaults).

I think those are sensible enhancements at this point. They are crucial for making perfect use of the #362 typestate buffers (because to make maximum use of that, we'll need to size the buffers so that the right ones fit in each other, and we can thus tell at build time that some operations can't overflow) -- but even without that, I'd like to build on them to reduce type confusion.

This reduces the amount of necessary copying, and decouples the
functions from predefined lengths (given that they were originally
designed for a concrete type, then used with other types, which only
works while those are aliased).

The length generic being decoupled from the input length does raise a
few new cases of panicking (maybe even unsoundness, in the cryptocell
case), but at least for encrypting (where more data is written to the
output than in the input), those were already present before, and are
better tackled independently from the refactoring.

Contributes-To: lake-rs#363
The new types are all aliases of the old, but those now work even when
of all the type aliases of EdhocMessageBuffer in shared/src/lib.rs, new
explicitly different sizes are chosen. (Except for EdhocCiphertext2 and
EdhocPlaintext2 which need to be the same size lest their
encrypt_decrypt function needs duplication).
@chrysn chrysn changed the title Initial fixes for #363 Consistently use message buffer type aliases May 23, 2025
@chrysn chrysn marked this pull request as ready for review May 23, 2025 15:50
@chrysn chrysn mentioned this pull request May 23, 2025
@chrysn chrysn requested a review from geonnave May 26, 2025 09:37
pub x: BytesP256ElemLen,
pub g_y: BytesP256ElemLen,
pub plaintext_2: EdhocMessageBuffer,
pub plaintext_2: BufferPlaintext2,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason why some places are plaintext: &[u8] and others are plaintext_2: BufferPlaintext2 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't revisit yet where we can get rid of buffers and uses slices instead; that's a follow-up step.

@geonnave
Copy link
Collaborator

Went through and LGTM. 👍 when you think it's ready?

@chrysn
Copy link
Member Author

chrysn commented May 27, 2025

I think this is ready.

@geonnave geonnave merged commit 3c29239 into lake-rs:main Jun 2, 2025
37 of 65 checks passed
@chrysn chrysn deleted the initial-363 branch June 2, 2025 07:59
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.

2 participants