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

hkdf: automatically switch between Hmac and SimpleHmac #96

Closed
wants to merge 2 commits into from

Conversation

newpavlov
Copy link
Member

@newpavlov newpavlov commented Jun 16, 2024

With this change the implementation automatically selects Hmac for "eager" hashes and SimpleHmac for "lazy" ones.

The main disadvantage of this approach is that new Hkdf will work only for hash functions defined using CoreWrapper. Ideally, we would use specialization here, but we probably can work around this by tweaking digest and hmac. We need to distinguish between eager hashes which can return hash "core" and other hashes. Plus, it should be done in a way which allows compiler to see that implementations in this crate do not overlap.

@newpavlov newpavlov requested a review from tarcieri June 16, 2024 21:45
Comment on lines +25 to +28
impl<C, K> Sealed for CoreWrapper<C>
where
H: EagerHash + OutputSizeUser,
K: HmacKind<Self> + BufferKind,
C: BufferKindUser<BufferKind = K> + OutputSizeUser,
Copy link
Member

Choose a reason for hiding this comment

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

Seems like this would preclude switching to newtypes of CoreWrapper for the hash crates.

Could this blanket impl be based around the requisite traits instead?

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 don't think it's possible to do automatically without specialization. Alternatively, we could introduce yet another trait which would regulate switching between those two HMAC implementations, i.e. we would move this burden to all hash implementation crates.

Copy link
Member

Choose a reason for hiding this comment

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

Then I would personally rather have newtypes than this automatic switching.

Have you benchmarked how much SimpleHmac actually degrades performance for the short inputs in HKDF usage?

Copy link
Member Author

Choose a reason for hiding this comment

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

We already have the SimpleHkdf type aliases, which do the job just fine.

It's more about structure size, depending on hash, SimpleHmac can be twice bigger, especially if the reset feature is enabled for hmac.

@newpavlov newpavlov closed this Jun 19, 2024
@newpavlov newpavlov deleted the hkdf/generic_simple branch June 19, 2024 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.

2 participants