Fix some MAC issues #8584
Fix some MAC issues #8584
Conversation
EVP_MAC_ctrl is documented to return 0 or -1 on failure. Numerous places were not getting this check correct.
See discussion in github issue openssl#8563 Fixes openssl#8563
|
1.1.1 backport in #8585. |
|
I'm not sure how "currently" this is. The shake functions just don't seem to make sense to use since they do not have a fixed output length. |
|
I could drop the word "currently" throughout if you prefer. My only reasoning for including it was allowing for the possibility that someone might in the future specify how to handle this type of digest. |
doc/man3/EVP_MAC.pod
Outdated
| @@ -202,6 +202,9 @@ For MAC implementations that use an underlying computation algorithm, | |||
| these controls set what the algorithm should be, and the engine that | |||
| implements the algorithm if needed. | |||
|
|
|||
| Note that not all algorithms may support all digests. HMAC does not currently | |||
| support SHAKE128 or SHAKE256. | |||
There was a problem hiding this comment.
Worth saying this as variable output length digests?
paulidale
added
the
approval: done
There was a problem hiding this comment.
I tested both branches #8584 and #8585 on linux using debug and release configuration and was able to verify Matt's #8563 (comment).
The "currently" should be dropped, the other suggestion is optional.
| @@ -35,6 +35,10 @@ int HMAC_Init_ex(HMAC_CTX *ctx, const void *key, int len, | |||
| return 0; | |||
| } | |||
|
|
|||
| /* We currently don't support shake128 and shake256 with HMAC */ | |||
| if ((EVP_MD_meth_get_flags(md) & EVP_MD_FLAG_XOF) != 0) | |||
| return 0; | |||
There was a problem hiding this comment.
I agree with @kroeckx that "currently" should be dropped because it is misleading. How about the following alternative formulation?
/*
* The HMAC construction is not allowed to be used with the
* extendable-output functions (XOF) shake128 and shake256.
*/(BTW: @tiran you could have told us this long time ago ;-) )
doc/man3/EVP_MAC.pod
Outdated
| @@ -202,6 +202,9 @@ For MAC implementations that use an underlying computation algorithm, | |||
| these controls set what the algorithm should be, and the engine that | |||
| implements the algorithm if needed. | |||
|
|
|||
| Note that not all algorithms may support all digests. HMAC does not currently | |||
|
Updates pushed addressing all feedback. Please reconfirm? |
|
Pushed. Thanks. |
EVP_MAC_ctrl is documented to return 0 or -1 on failure. Numerous places were not getting this check correct. Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com> (Merged from #8584)
Don't allow SHAKE128/SHAKE256 with HMAC.
Correctly check the return code of EVP_MAC_ctrl everwhere it is used
Fixes #8563.