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

Incorrect usage of the HMAC APIs #13210

Open
mattcaswell opened this issue Oct 21, 2020 · 8 comments
Open

Incorrect usage of the HMAC APIs #13210

mattcaswell opened this issue Oct 21, 2020 · 8 comments
Labels
branch: master Merge to master branch branch: 1.1.1 Merge to OpenSSL_1_1_1-stable branch triaged: bug The issue/pr is/fixes a bug

Comments

@mattcaswell
Copy link
Member

This talk discusses a security attack which may result if the HMAC APIs are used incorrectly:

https://www.youtube.com/watch?v=hYDD_rI_8tU

The issue occurs if an application developer makes calls in this order:

HMAC_Init_ex()
HMAC_Update()
HMAC_Final()
HMAC_Update()
HMAC_Final()

i.e. if an HMAC_CTX is reused to "incrementally" calculate the HMAC of a message consisting of multiple fragments, but without calling HMAC_Init_ex() again, before calling update and then final.

As noted above this is incorrect usage of the API, but the APIs do not protect you from this foot gun. Should they?

It looks to me like we don't do a check to protect against this in the EVP_MAC APIs either (and probably not with other init/update/final APIs). Should we?

@mattcaswell mattcaswell added the triaged: bug The issue/pr is/fixes a bug label Oct 21, 2020
@paulidale
Copy link
Contributor

The paper Unintended Features of APIs: Cryptanalysis of Incremental HMAC is available on the conference site.

@kaduk
Copy link
Contributor

kaduk commented Oct 22, 2020

As noted above this is incorrect usage of the API, but the APIs do not protect you from this foot gun. Should they?

I believe this is "yes", though there are perhaps arguments to be made about API stability.
I thought that we had a couple existing APIs that did enforce single-use via Final() (maybe EVP_MD_CTX? Though the code is not a clear example), as some precedent for this being a good idea.

It looks to me like we don't do a check to protect against this in the EVP_MAC APIs either (and probably not with other init/update/final APIs). Should we?

This seems to be pretty unambiguously "yes".

@kroeckx
Copy link
Member

kroeckx commented Oct 23, 2020

Since EVP_MAC is a new and as yet unreleased API, I think we should prevent the user from doing that with EVP_MAC.

@beldmit
Copy link
Member

beldmit commented Oct 23, 2020

I think we should have some checks preventing bad order of operations in this and in some other cases.

@kroeckx kroeckx added branch: 1.1.1 Merge to OpenSSL_1_1_1-stable branch branch: master Merge to master branch labels Oct 31, 2020
@kroeckx
Copy link
Member

kroeckx commented Nov 7, 2020

APIs like EVP_VerifyFinal() and EVP_DigestVerifyFinal() document that calling Update after Final is possible. I assume they don't have the same problem.

@mattcaswell
Copy link
Member Author

APIs like EVP_VerifyFinal() and EVP_DigestVerifyFinal() document that calling Update after Final is possible. I assume they don't have the same problem.

Until I saw issue #13342 I had been thinking that we should incorporate checks everywhere for all APIs that ban this type of calling. The Verify APIs confuse this picture somewhat.

@mattcaswell
Copy link
Member Author

Until I saw issue #13342 I had been thinking that we should incorporate checks everywhere for all APIs that ban this type of calling. The Verify APIs confuse this picture somewhat.

I also wonder whether we have tests for this way of using the Verify APIs and whether it even still works with the provider model.

@levitte
Copy link
Member

levitte commented Feb 25, 2021

Something to be noted is that EVP_DigestFinal() clears the context data when it's done producing the output, at least in 1.1.1. In 3.0, we're leaving that to the provider implementation... and at least most of our implementations do that clearing (for pretty obvious reasons, the SHA3 one does not)

It seems that HMAC_Final() doesn't do this sort of clearing... and maybe it should?

EDIT: ... HMAC_Final() calls EVP_DigestFinal_ex(), so I may not have thought this through enough. It makes me wonder what kind of random output is expected from the second Update+Final call...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch: master Merge to master branch branch: 1.1.1 Merge to OpenSSL_1_1_1-stable branch triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

No branches or pull requests

6 participants