Add Signing with SHAKE support #23114
Conversation
All existing digests generate algorithm parameters when generating the DER data required for the signature, For shake however these parameters are all fixed values so only the RSA_PSS_SHAKE OID is needed. The RSA-PSS code has been modified so that the RSA-PSS parameter getters such as MGF1 and saltlen, will return fixed values if the hash algorithm is either SHAKE128 or SHAKE256. During RSA-PSS signature signing and verifying an error will occur if the MGF1 or saltlen are not set to the correct values if SHAKE is selected as the digest. RSA-PSS with SHAKE is described in FIPS 186-5 and RFC 8702.
|
Depends on #22684 |
|
Tests still need to be added.. |
slontis
added
hold: needs tests
github-actions
bot
added
the
severity: fips change
|
@openssl/otc Having a self-hosted runner that queues for multiple hours seems a bit problematic.. |
t8m
added
triaged: feature
|
This PR is in a state where it requires action by @openssl/otc but the last update was 30 days ago |
|
This PR is in a state where it requires action by @openssl/otc but the last update was 61 days ago |
This is a breaking change that affects using SHAKE with EVP_DigestFinal(). This should be resolved BEFORE we add support for signing with SHAKE, See (PR openssl#23114) which is currently dependant on PR openssl#22684 (Support for fixed output length SHAKE algorithms). This was going to be used by LMS also. Leaving the code as it was may allow backwards compatability, but it would not interop nicely with signatures coming from another toolkit, and would be inconsistent with the Fixed output length SHAKE algorithms. Note that the algorithms will also map back to OIDS (so having 2 names for SHAKE-256, (one that mays back to a bad output size and one that is the correct size) does not allow a nice mapping back to a single OID.
|
This PR is in a state where it requires action by @openssl/otc but the last update was 92 days ago |
|
This PR is in a state where it requires action by @openssl/otc but the last update was 123 days ago |
xnox
left a comment
xnox
left a comment
There was a problem hiding this comment.
FIPS 186-5 & RFC 8702 are in agreement and do not allow to use shake in mgf1, but only as mgf directly.
| static const RSA_PSS_PARAMS_30 shake128_RSASSA_PSS_params = { | ||
| NID_shake128, /* default hashAlgorithm */ | ||
| { | ||
| NID_mgf1, /* default maskGenAlgorithm */ |
There was a problem hiding this comment.
That does not seem right as per RFC 8702
In RSASSA-PSS with SHAKEs, the SHAKEs MUST be used natively as the MGF, instead of the MGF1 algorithm that uses the hash function in multiple iterations, as specified in Appendix B.2.1 of [RFC8017]. In other words, the MGF is defined as the SHAKE128 or SHAKE256 with input being the mgfSeed for id-RSASSA-PSS-SHAKE128 and id-RSASSA-PSS-SHAKE256, respectively.
Thus I am expecting no NID_mgf1 here.
There was a problem hiding this comment.
Could you give me your opinion on the test vectors located here
https://github.com/usnistgov/ACVP-Server/blob/master/gen-val/json-files/RSA-SigVer-FIPS186-5/internalProjection.json
Look at the SHAKE entries in particular... it seems to allow alg/maskgenalgorithm combinations such as shake-128 with shake-256 and sha3 with shakeX
The "FIPS 186-5 5.4.1 Mask Generation Functions" does not mention the CMS related changes.. So there is a mismatch between the specs here..
There was a problem hiding this comment.
Half of the test vectors look fine to me (as they use shake as the mgf function), I am confused about those that imply using MGF1.
There was a problem hiding this comment.
The test vectors that use Shake + "MGF1" seem to use shake inside the MGF1 function. (They also seem to use 1/2 the security strength currently inside the MGF1 for shake - I dont think this should be supported inside OpenSSL).
There was a problem hiding this comment.
RFC 8702 unambiguously states that when SHAKE is used in digital signature hash, SHAKE must be used as the MGF function, and not as mgf1(shake). Thus whatever ACVP server states appears to be in violation of RFC 8702.
Also reading FIPS 186 it doesn't explicitly call out SHAKE as allowed in MGF1 (section 5.4.1), in the same manner as it calls out that SHAKE is allowed for message hash (section 5.4(b)).
Imho SHAKE in MGF1 must not be supported by OpenSSL as it would violate RFC 8702. I will separately try to contact NIST about this, for them to clarify this.
In RSASSA-PSS with SHAKEs, the SHAKEs MUST be used natively as the MGF, instead of the MGF1 algorithm that uses the hash function in multiple iterations, as specified in Appendix B.2.1 of [RFC8017].
|
This PR is in a state where it requires action by @openssl/otc but the last update was 30 days ago |
|
This PR is in a state where it requires action by @openssl/otc but the last update was 61 days ago |
|
This PR is in a state where it requires action by @openssl/otc but the last update was 92 days ago |
|
@xnox I hit a roadblock with the test vectors quite a while ago. |
|
This PR is in a state where it requires action by @openssl/committers but the last update was 30 days ago |
|
This PR is in a state where it requires action by @openssl/committers but the last update was 61 days ago |
|
This PR is in a state where it requires action by @openssl/committers but the last update was 92 days ago |
|
This PR is in a state where it requires action by @openssl/committers but the last update was 123 days ago |
|
This PR is in a state where it requires action by @openssl/committers but the last update was 154 days ago |
|
This PR is in a state where it requires action by @openssl/committers but the last update was 185 days ago |
|
This PR is in a state where it requires action by @openssl/committers but the last update was 216 days ago |
|
This PR is in a state where it requires action by @openssl/committers but the last update was 247 days ago |
|
This PR is in a state where it requires action by @openssl/committers but the last update was 278 days ago |
|
This PR is in a state where it requires action by @openssl/committers but the last update was 309 days ago |
|
This PR is in a state where it requires action by @openssl/committers but the last update was 340 days ago |
|
This PR is in a state where it requires action by @openssl/committers but the last update was 371 days ago |
|
RSA-PSS does not seem to be popular. |
|
This PR is in a state where it requires action by @openssl/committers but the last update was 30 days ago |
|
This PR is in a state where it requires action by @openssl/committers but the last update was 61 days ago |
4 participants
RSA-PSS and ECDSA with SHAKE are described in FIPS 186-5 and RFC 8702.
To add support we need to add DER encoding support for SHAKE.
The RSA PSS case is a little more complicated, since all existing digests
generate algorithm parameters when generating the DER data required
for the signature, For shake however these parameters are all fixed values
so only the RSA_PSS_SHAKE OID is needed.
The RSA-PSS code has been modified so that the RSA-PSS parameter
getters such as MGF1 and saltlen, will return
fixed values if the hash algorithm is either SHAKE128 or SHAKE256.
During RSA-PSS signature signing and verifying an error will occur
if the MGF1 or saltlen are not set to the correct values if SHAKE
is selected as the digest.
Checklist