KantaraInitiative / SAMLprofiles

SAML interoperability and deployment profiles
Other
11 stars 4 forks source link

(Issue 5) MGF hash sha1/sha256 function? #129

Closed ghost closed 5 years ago

ghost commented 5 years ago

I'm trying to understand the RSA-OAEP encryption requirements for IdPs / SPs.

The following default digest algorithm MUST be used in conjunction with the above key transport algorithms (the default mask generation function, MGF1 with SHA1, MUST be used):

http://www.w3.org/2001/04/xmlenc#sha256 [XMLEnc]

It seems most IdPs use SHA1 for both the MFG1 and digest? So, this profile requires you to use SHA1 for the MFG1 and SHA256 for the digest. Any reason why it is not SHA256 for both?

Also, why not require MGF1 with SHA256: http://www.w3.org/2009/xmlenc11#mgf1sha256 as algorithm identifier? Now it is not clear that SHA256 was used for the digest?

Probably I am missing something here...

judielaine commented 5 years ago

I read the parenthesized reference to the default mask generation function to be a reiteration of a requirement stated elsewhere, particularly XMLEnc's §5.4.2 statement that "As described in the EME-OAEP-ENCODE function RFC 2437 [PKCS1, section 9.1.1.1], .... using the mask generator function MGF1 (with SHA1) specified in RFC 2437."

If i am correct, i wonder if rewording as follows would be more clear

Key Transport (the default mask generation function, MGF1 with SHA1, MUST be used)

      http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p [XMLEnc]
       http://www.w3.org/2009/xmlenc11#rsa-oaep [XMLEnc]

The following default digest algorithm MUST be used in conjunction with the above key transport algorithms :

http://www.w3.org/2001/04/xmlenc#sha256 [XMLEnc]

scantor commented 5 years ago

There is definitely clarification needed, it reads very badly now...but most IdPs have long since stopped using SHA-1 for general usage, the MGF1 case is an exception and was left as is for interoperability. It's not that unusual for libraries to lack support for any MGF pluggability. If there are security implications for use of SHA-1 there, I'm not aware of them.

judielaine commented 5 years ago

Included as issue 5 at https://kantarainitiative.org/confluence/display/fiwg/SAML2int+v2.0

scantor commented 5 years ago

I misinterpreted in part...I think it is true that the norm for OAEP digests is SHA-1, and I just verified Shibboleth has continued that also. So I think we should probably just open up that requirement altogether and would agree with the original suggestion.

vedrnes commented 5 years ago

The writing as is right now is a problem for those using cryptographic hardware modules when decrypting content with RSA-OAEP. Namely, most vendors out there restrict the RSA-OAEP implementation to using the same digest function as both the MGF digest and message digest. So a combination of MGF1withSHA1 + SHA256 is problematic while MGF1withSHA1 + SHA1 (aka the default settings for mgf1p), MGF1withSHA256 + SHA256, MGF1withSHA384 + SHA384 and MGF1withSHA512 + SHA256 work well. Given concerns about SHA1 it may also be wise to recommend the MGF1withSHA256 + SHA256 as the default choice. I would propose this be added to the profile - matching the digests is a small price to pay for greatly enhancing the ability to use native RSA-OAEP implementations on hardware modules.

scantor commented 5 years ago

I was considering either enumerating the SHA1+SHA1 and SHA2+SHA2 combinations or leaving it at SHA1+SHA1 to address the problem.

I don't have a strong opinion either way since I don't know the security implications of SHA-1 in this context, it's always been more about hedging against regulatory moves to bar SHA-1 from implementations used in particular contexts, and I don't think that's happened to this point.

vedrnes commented 5 years ago

I would propose edits along the following lines: 1) For encrypting, MGF1 digest and message digest algorithms MUST match (this provides broad support on HSM based implementations) 2) For encrypting, implementations SHOULD use SHA-256, SHA-384 and SHA-512 (this gives a nudge in the direction of avoiding use of SHA-1) 3) For decrypting, implementations MUST support SHA-1, SHA-256, SHA-384 and SHA-512 (this gives interoperability in the terms of accepting SHA-1 on the decryption side)

scantor commented 5 years ago

This is about deployment only, not what implementations need to do. It doesn't have to cover cases that extend to interoperability outside the profile's own boundaries like a conformance document for a standard would.

I checked the implementation profile precursor to this work and it was written as MUST support SHA-256 and SHOULD support SHA-1. That would suggest we probably need to go with SHA-2/SHA-2 as the language here. I suspect given the GCM issue that the SHA-2 usage for OAEP will be the least of the problems with finding implementations that will work.

vedrnes commented 5 years ago

I guess a MUST for SHA-2 and SHOULD for SHA-1 is OK. However, including the digest match statement is important. For example, even though both SHA-256 and SHA-384 can be denoted as "SHA-2", MGF1/message digest combination SHA-256 + SHA-256 is OK, but SHA-256 + SHA-384 is not .

scantor commented 5 years ago

SHA-256 is what was stipulated, I'm just using SHA-2 as shorthand here. But your issue here is with the implementation profile, which is done, not this deployment profile. We can simply mandate SHA-256 for both and clean up the text here.

vedrnes commented 5 years ago

I agree, mandating SHA-256/SHA-256 will work.

scantor commented 5 years ago

Did some testing and was able to flip Shibboleth over to SHA2 and MGF1-SHA2 and successfully tested Shibboleth SPs and one or two others. As expected most other impls fail on it. Will discuss with group tomorrow but given GCM already being in this profile, I doubt this should give anybody pause.

scantor commented 5 years ago

After discusssion by group, it was felt that absent a compelling security issue with using SHA-1 in this area we should leave it be and match up with the standard usage in this area. I'll include a note about possibly revisiting if there's a reason to.