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: when HMAC key is all zeros, still set a valid key length #24204
Conversation
By itself, this is no change in any computation. However, this will unlock enforcing minimum key lengths for NIST and FIPS 140-3 requirements. Also reading RFC8448 and RFC5869, this seems to be strictly correct too. Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@surgut.co.uk>
Hmm... given RFC5869 this is really a bug in our implementation. Apparently we do not test this case at all and we should. But the bug seems to be already in HKDF_Extract() - it should handle NULL salt properly - using the all zeros of mdlen. |
The change seems fine to me.. HKDF_Extract() calls MAC_Init() which calls HMAC_Init_ex().. So pre and post this change the same result will occur. |
I think the change should only happen on master, since it makes no difference to the result. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I did not realize that the HMAC does padding of the key implicitly.
This pull request is ready to merge |
Merged to the master branch. Thank you for your contribution. |
By itself, this is no change in any computation. However, this will unlock enforcing minimum key lengths for NIST and FIPS 140-3 requirements. Also reading RFC8448 and RFC5869, this seems to be strictly correct too. Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@surgut.co.uk> Reviewed-by: Shane Lontis <shane.lontis@oracle.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #24204)
By itself, this is no change in any computation. However, this will unlock enforcing minimum key lengths for NIST and FIPS 140-3 requirements. Also reading RFC8448 and RFC5869, this seems to be strictly correct too. Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@surgut.co.uk> Reviewed-by: Shane Lontis <shane.lontis@oracle.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#24204) (cherry picked from commit 15d6114)
By itself, this is no change in any computation. However, this will unlock enforcing minimum key lengths for NIST and FIPS 140-3 requirements.
Also reading RFC8448 and RFC5869, this seems to be strictly correct too.
Submitting this separately from #24199 because this may seem as potentially critical change.
Checklist
No tests adjusted as the expectations are to remain the same.