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

Pad to multiples of 128 bytes, rather than to a random length. #58

Merged
merged 1 commit into from
Mar 24, 2021

Conversation

rfk
Copy link
Contributor

@rfk rfk commented Mar 24, 2021

This is one of the padding techniques suggested in the RFC, and while
we can't choose a default scheme that will work well for all applications,
this one at least seems easier to reason about.

Fixes #54.

This is one of the padding techniques suggested in the RFC, and while
we can't choose a default scheme that will work well for all applications,
this one at least seems easier to reason about.

Fixes #54.
@rfk rfk requested a review from jrconlin March 24, 2021 07:28
@rfk
Copy link
Contributor Author

rfk commented Mar 24, 2021

@jrconlin thank you for your review on #59! Just a friendly /cc to make sure you saw this one as well, as this PR needs to land first to avoid conflicting changes.

@@ -14,7 +14,7 @@ const ECE_AES128GCM_HEADER_LENGTH: usize = 21;
// The max AES128GCM Key ID Length is 255 octets. We use far less of that because we use
// the "key_id" to store the exchanged public key since we don't cache the key_ids.
// Code fails if the key_id is not a public key length field.
const ECE_AES128GCM_PAD_SIZE: usize = 1;
pub(crate) const ECE_AES128GCM_PAD_SIZE: usize = 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

part of me REALLY wants this to be just ECE_PAD_SIZE and kinda force the point by requiring the package if you want to use both (e.g. aes128gcm.ECE_PAD_SIZE), but that might lead to unintended bugs.

@rfk rfk merged commit dc1c446 into main Mar 24, 2021
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.

Pad to multiples of a fixed size, rather than padding randomly
2 participants