-
-
Notifications
You must be signed in to change notification settings - Fork 9.9k
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
rsa no xof #24387
rsa no xof #24387
Conversation
FIPS 186-5, RFC 8692, RFC 8702 all agree and specify that Shake shall be used directly as MGF (not as a hash in MGF1). Add tests that try to specify shake hash as MGF1 to ensure that fails. Separately the above standards specify how to use SHAKE as a message digest with either fixed or minimum output lengths. However, currently shake is not part of allowed hashes. Note that rsa_setup_md()/rsa_setup_mgf1_md() call ossl_digest_rsa_sign_get_md_nid() -> ossl_digest_get_approved_nid_with_sha1() -> ossl_digest_get_approved_nid() which only contain sha1/sha2/sha3 digests without XOF. The digest test case will need to be replace if/when shake with minimum output lengths is added to ossl_digest_get_approved_nid().
NIST SP 800-56 rev2 only allows using approved hash algorithms in OAEP. Unlike FIPS 186-5 it doesn't have text allowing to use XOF SHAKE functions. Maybe future revisions of SP 800-56 will adopt similar text to FIPS 186-5 and allow XOF as MD and MGF (not MGF1). RFC documents do not specify if SHAKE is allowed or blocked for usage (i.e. there is no equivalent of RFC 8692 or RFC 8702 for OAEP). Status quo allows their usage. Add test cases for SHAKE in RSA-OAEP as allowed in default provider, and blocked in fips.
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.
This is an example of functionality where I do not expect any real-world use-case breakage. So IMO OK as is.
Yes this is way too new and way too niche to be out there in practice. And many modules that did submit with shake support, did caught this and blocked it already. (stuff pre 186-5 being published). And I don't see anybody implementing 186-5 stuff yet. I.e. I don't see any libraries using shake as MGF directly - maybe they are all waiting for OpenSSL to implement it =) |
This will require an OTC decision since it is a breaking change. |
OTC: should we condone this break in the API or not? In the past we've been conservative with respect to potential real world usage. This PR is asking for the opposite. |
Should this have a CHANGES.md entry? |
OTC: We are OK with this not requiring a way to enable in FIPS MODULE. The reason is that it is unlikely to be used by applications and for the eventual rare use-case as this is non-approved the fallback is to use the default provider. |
@t8m should label "approval:review pending" be replaced with "approval:done" given the above comment? |
No, it still needs a second review. OTC decisions and reviews are independent generally. |
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.
LGTM
This pull request is ready to merge |
Merged to the master branch. Thank you for your contribution. |
FIPS 186-5, RFC 8692, RFC 8702 all agree and specify that Shake shall be used directly as MGF (not as a hash in MGF1). Add tests that try to specify shake hash as MGF1 to ensure that fails. Separately the above standards specify how to use SHAKE as a message digest with either fixed or minimum output lengths. However, currently shake is not part of allowed hashes. Note that rsa_setup_md()/rsa_setup_mgf1_md() call ossl_digest_rsa_sign_get_md_nid() -> ossl_digest_get_approved_nid_with_sha1() -> ossl_digest_get_approved_nid() which only contain sha1/sha2/sha3 digests without XOF. The digest test case will need to be replace if/when shake with minimum output lengths is added to ossl_digest_get_approved_nid(). Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #24387)
NIST SP 800-56 rev2 only allows using approved hash algorithms in OAEP. Unlike FIPS 186-5 it doesn't have text allowing to use XOF SHAKE functions. Maybe future revisions of SP 800-56 will adopt similar text to FIPS 186-5 and allow XOF as MD and MGF (not MGF1). RFC documents do not specify if SHAKE is allowed or blocked for usage (i.e. there is no equivalent of RFC 8692 or RFC 8702 for OAEP). Status quo allows their usage. Add test cases for SHAKE in RSA-OAEP as allowed in default provider, and blocked in fips. Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #24387)
is this now breaking all mainline builds? seems like everything is red =( will check. |
These were added as a POC in openssl#24387. However, such combinations are no longer unusable since openssl#24105 got merged. This should unbreak all build failures on mainline. Partially reverts: 1bfc8d1 (rsa-oaep: block SHAKE usage in FIPS mode, 2024-05-13) Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@surgut.co.uk>
These were added as a POC in #24387. However, such combinations are no longer unusable since #24105 got merged. This should unbreak all build failures on mainline. Partially reverts: 1bfc8d1 (rsa-oaep: block SHAKE usage in FIPS mode, 2024-05-13) Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@surgut.co.uk> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #24463)
FIPS 186-5, RFC 8692, RFC 8702 all agree and specify that Shake shall be used directly as MGF (not as a hash in MGF1). Add tests that try to specify shake hash as MGF1 to ensure that fails. Separately the above standards specify how to use SHAKE as a message digest with either fixed or minimum output lengths. However, currently shake is not part of allowed hashes. Note that rsa_setup_md()/rsa_setup_mgf1_md() call ossl_digest_rsa_sign_get_md_nid() -> ossl_digest_get_approved_nid_with_sha1() -> ossl_digest_get_approved_nid() which only contain sha1/sha2/sha3 digests without XOF. The digest test case will need to be replace if/when shake with minimum output lengths is added to ossl_digest_get_approved_nid(). Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#24387)
NIST SP 800-56 rev2 only allows using approved hash algorithms in OAEP. Unlike FIPS 186-5 it doesn't have text allowing to use XOF SHAKE functions. Maybe future revisions of SP 800-56 will adopt similar text to FIPS 186-5 and allow XOF as MD and MGF (not MGF1). RFC documents do not specify if SHAKE is allowed or blocked for usage (i.e. there is no equivalent of RFC 8692 or RFC 8702 for OAEP). Status quo allows their usage. Add test cases for SHAKE in RSA-OAEP as allowed in default provider, and blocked in fips. Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#24387)
These were added as a POC in openssl#24387. However, such combinations are no longer unusable since openssl#24105 got merged. This should unbreak all build failures on mainline. Partially reverts: 1bfc8d1 (rsa-oaep: block SHAKE usage in FIPS mode, 2024-05-13) Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@surgut.co.uk> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#24463)
rsa-pss: add tests checking for SHAKE usage in RSA-PSS
FIPS 186-5, RFC 8692, RFC 8702 all agree and specify that Shake shall
be used directly as MGF (not as a hash in MGF1). Add tests that try to
specify shake hash as MGF1 to ensure that fails.
Separately the above standards specify how to use SHAKE as a message
digest with either fixed or minimum output lengths. However, currently
shake is not part of allowed hashes.
Note that rsa_setup_md()/rsa_setup_mgf1_md() call
ossl_digest_rsa_sign_get_md_nid() ->
ossl_digest_get_approved_nid_with_sha1() ->
ossl_digest_get_approved_nid() which only contain sha1/sha2/sha3
digests without XOF.
The digest test case will need to be replace if/when shake with
minimum output lengths is added to ossl_digest_get_approved_nid().
rsa-oaep: block SHAKE usage in FIPS mode
NIST SP 800-56 rev2 only allows using approved hash algorithms in
OAEP. Unlike FIPS 186-5 it doesn't have text allowing to use XOF SHAKE
functions. Maybe future revisions of SP 800-56 will adopt similar text
to FIPS 186-5 and allow XOF as MD and MGF (not MGF1).
RFC documents do not specify if SHAKE is allowed or blocked for usage
(i.e. there is no equivalent of RFC 8692 or RFC 8702 for OAEP). Status
quo allows their usage.
Add test cases for SHAKE in RSA-OAEP as allowed in default provider,
and blocked in fips.