Bouncy Castle FIPS Issue

Brent Putman putmanb at georgetown.edu
Thu Jul 15 23:46:32 UTC 2021 

Hi,

I don't know that the XML Encryption authors were deliberately trying 
to preclude FIPS, but I also don't think they were suggesting a 
required monolithic approach either. You are down in the weeds on the 
diffs between SP 800-56A and RFC 6090, and I can't really evaluate any 
of that, I don't have any of that detail swapped in.   But in my 
experience with these XML specs, if they really intended the key 
agreement and key derivation to be a monolithic op, I might have 
expected the the algorithm URI would indicate that: instead of a single 
"#ECDH" URI, they'd have multiple like "#ECDHWithConcatKDFAndSHA256", 
etc.  They didn't. Key derivation has its own section, its own element 
and its own algorithm URIs, strongly implying to me that it is a first 
class thing.  But I suppose that's subject to interpretation.

A couple of practical reasons for our design are: 1) separation of 
concerns, unit testing etc, of key derivation separate from agreement 
(the key derivation was by far the most complex thing here), and 2) JDK 
support.  The standard JDK provider set does not support what you are 
describing, it only has KeyAgreement for "ECDH".  So it would be 
impossible to implement what you are advocating without a hard 
dependency on a third-party provider (and use of non-standard algorithm 
names and AlgorithmParameterSpec types).

Regarding HSMs and leaking info:  From a practical standpoint I think 
there's very little real world concern about the ephemeral agreement 
secret being handled in memory on the host. It's not stored 
persistently, but it is there as a local method var, so will exist on 
the heap prior to GC.  But even more practically, we simply don't 
target the kind of high security use case for HSMs. We're not sure it's 
even possible to actually use an HSM with say the Shib IdP, and 
probably zero users actually trying to do this via PKCS#11. (If someone 
is please let us know!).

So realistically private keys are loaded from disk, they are in-memory, 
and that's where the real world risk is.  And so any concern about the 
ephemeral agreement secret is trivial and moot by comparison. That 
might not be "great" in some absolute sense, but it's simply the 
reality of modern online services, and we're just part of that world.

Thanks,
Brent


On 7/15/21 2:52 PM, David Hook wrote:
>
> Hi Brent,
>
> I'm not entirely sure about this. I'm assuming we're talking about 
> https://www.w3.org/TR/xmlenc-core1.
>
> The KeyDerivationMethod appears inside the AgreementMethod. This 
> would be consistent with the approach taken in SP 800-56A but 
> inconsistent with not combining the operations as it would require a 
> FIPS compliant implementation of the key agreement algorithm to 
> expose the agreed value from the initial calculation performed by the 
> EC agreement keys, which in regards to the use of a KDF it 
> specifically should not do.  It's possible that the authors meant to 
> preclude the use of a FIPS module but my overall impression from:
>
> https://www.w3.org/TR/2013/NOTE-xmlenc-core1-explain-20130411/#sec-EllipticCurveAlgorithm
>
> is that the move from SP 800-56A to RFC 6090 for describing the EC 
> Diffie-Hellman primitive was more about restricting the EC curves 
> used for Diffie-Hellman to only those with a cofactor of 1 rather 
> than excluding implementations that are compliant with SP 800-56A. 
> The context of the discussion still appears to be about the EC 
> primitive supporting the key agreement method though, rather than the 
> key agreement method implementation.
>
> There's nothing wrong with using a KDF in the manner described below 
> either, and it's clearly a gap in our APIs we need to investigate, 
> but there's a reason people use HSMs to protect private keys, and 
> leaking information about private values through exposing 
> intermediate values in key agreement calculations is not one of them. 
> The commonality between AgreementMethod and DerivedKey is the 
> parameters used, not the way they are meant to be applied - the XML 
> is only describing data.
>
> Regards,
>
> David
>
> PS. One other thing, the API does specifically reject any curve with 
> cofactor that's not 1 for EC diffie-hellman doesn't it, or has some 
> other limitation? I haven't checked, but I did note in my wanderings 
> that the KeyAgreement algorithm was hard-coded to "ECDH" (the 
> primitive described in RFC 6090), rather than "ECCDH" (the primitive 
> described in SP 800-56A).
> On 15/7/21 3:00 pm, Brent Putman wrote:
>>
>> Hi,
>>
>> The secret byte[] over which which the key derivation is performed 
>> is literally a runtime argument.  It's in the method signature of 
>> the derive(...) method of the our KeyDerivation interface:
>>
>> http://git.shibboleth.net/view/?p=java-opensaml.git;a=blob;f=opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/derivation/KeyDerivation.java;hb=refs/heads/main
>>
>> The ConcatKDF under discussion is just a concrete impl of that.
>>
>> The XML Encryption model here - and reflected in our design - is 
>> that key derivation is a first class concern.  It intrinsically has 
>> nothing to do with key agreement.  For example, one can also produce 
>> a symmetric key from a pre-shared secret + a KDF, which the XML spec 
>> calls a DerivedKey.
>>
>> So any approach that conflates/combines key agreement and key 
>> derivation into a single monolithic operation is a non-starter for 
>> this.  That's the main point I was trying to make.
>>
>> Thanks,
>> Brent
>>
>>
>> On 7/14/21 9:28 PM, David Hook wrote:
>>>
>>> Hi Brent,
>>>
>>> I think I just need some more information.
>>>
>>> When you say output byte[] of the KeyAgreement op, where does that 
>>> come from? As in what code is producing that? Would you point me at 
>>> the file that's calling the KDFConcat derive() method - referring 
>>> to the source is generally the least ambiguous way of sorting these 
>>> things out.
>>>
>>> Thanks,
>>>
>>> David
>>>
>>> On 15/7/21 10:55 am, Brent Putman wrote:
>>>>
>>>> Hi,
>>>>
>>>> Either I don't think I understand, or maybe you misread our code.  
>>>> AFAIK the KeyAgreement service interface requires the 2 Keys over 
>>>> which the key agreement is being performed. For example:
>>>>
>>>> keyAgreement = KeyAgreement.getInstance(agreementAlgo);
>>>> keyAgreement.init(privateKey);
>>>> keyAgreement.doPhase(publicKey, true);
>>>>
>>>> Our KeyDerivation impls like ConcatKDF do not have access to any 
>>>> of the key agreement key material.  They are about key derivation 
>>>> from an input byte[] only (i.e. the output byte[] of the 
>>>> KeyAgreement op).  The do not and can not have access to any key 
>>>> agreement key material, because it's out-of-scope for what they 
>>>> do.  So I don't see how we can re-implement our ConcatKDF 
>>>> "SecretKey derive(...)" method using the KeyAgreement service 
>>>> interface as you suggest.
>>>>
>>>> Please clarify if I am missing something here.
>>>>
>>>> Thanks,
>>>> Brent
>>>>
>>>>
>>>> On 7/14/21 8:21 PM, David Hook wrote:
>>>>>
>>>>> Hi Brent,
>>>>>
>>>>> It will work. The naming convention is also to allow mixing and 
>>>>> matching, you just need to add:
>>>>>
>>>>> org.bouncycastle.jcajce.spec.UserKeyingMaterialSpec
>>>>>
>>>>> as a parameter to the KeyAgreement. It's unfortunate that the JCE 
>>>>> doesn't define this parameter, so I am not surprised you are not 
>>>>> aware of it, but it's impossible to use key agreement properly 
>>>>> without it, and it exists in both BC and BCFIPS specifically for 
>>>>> situations like this.
>>>>>
>>>>> You use the UserKeyingMaterialSpec to pass in otherInfo, which I 
>>>>> can see is being constructed in accordance with SP 800-56A which 
>>>>> is the FIPS standard. There is some BC support for otherInfo but 
>>>>> we are using the ASN.1 construction, SP 800-56A allows for 
>>>>> several others, and I'm not sure what the XML standard settled 
>>>>> on. You are probably better off sticking to what you are doing now.
>>>>>
>>>>> For our part I think we'd just need to add some additional KDF 
>>>>> support for the non-NIST digests. In the case of the SAML code it 
>>>>> would just be a matter of replacing SecretKey derive() I think 
>>>>> and allowing for the UserKeyingMaterialSpec construction.
>>>>>
>>>>> Concerning  your comments about BC and BCFIPS. Yes the low level 
>>>>> libraries are different, they have to be, but we've been able to 
>>>>> keep the JCE layer in sync since 1.58. The upside of this is it 
>>>>> means if people swap in BCFIPS rather than BC they can be 
>>>>> confident the code is then FIPS compliant. If you interested in 
>>>>> finding out why the difference exists, there is actually a document
>>>>>
>>>>> https://www.bouncycastle.org/fips/BCFipsDescription.pdf
>>>>>
>>>>> which provides a summary. I would recommend that over reading the 
>>>>> FIPS IG and the 30 or so associated documents that go with it 
>>>>> unless you have a specific interest. The same applies for Common 
>>>>> Criteria.
>>>>>
>>>>> I don't think there will be any end-user problems as the end 
>>>>> result will be a massive broadening of what the OpenSAML project 
>>>>> can be used for.
>>>>>
>>>>> Any further questions or issues, please let me know.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> David
>>>>>
>>>>> On 15/7/21 8:59 am, Brent Putman wrote:
>>>>>>
>>>>>> Hi David,
>>>>>>
>>>>>> I took a look and unfortunately what you propose won't work, for 
>>>>>> at least 2 reasons.
>>>>>>
>>>>>> First (the main and show-stopping technical issue), those 
>>>>>> provider-based algorithms you mention are apparently for the 
>>>>>> KeyAgreement service of the Java security framework. It seems 
>>>>>> those combine the key agreement op with the KDF op all in one go?
>>>>>>
>>>>>> That won't work for our design because we're modeling the 
>>>>>> requirements of XML Encryption 1.1 where the KA and KDF ops are 
>>>>>> conceptually completely separate. They are defined separately, 
>>>>>> have distinct XML representations, etc, so one can mix-and-match 
>>>>>> KA algo and KDF algo.
>>>>>>
>>>>>> So in OpenSAML we have distinct KA and KDF interfaces and then 
>>>>>> various impls for each. For example we have interface 
>>>>>> org.opensaml.xmlsec.derivation.KeyDerivation.  The ConcatKDF 
>>>>>> impl of that in question that threw a NoClassDefFoundError with 
>>>>>> FIPS was our org.opensaml.xmlsec.derivation.impl.ConcatKDF, 
>>>>>> which imports these KDF-related classes (along with some needed 
>>>>>> BC digest classes):
>>>>>>
>>>>>> import 
>>>>>> org.bouncycastle.crypto.agreement.kdf.ConcatenationKDFGenerator;
>>>>>> import org.bouncycastle.crypto.params.KDFParameters;
>>>>>>
>>>>>> It was specifically throwing on not finding 
>>>>>> org.bouncycastle.crypto.DerivationParameters, which is the 
>>>>>> interface implemented by the above BC KDFParameters.  For 
>>>>>> reference that source is here:
>>>>>>
>>>>>> http://git.shibboleth.net/view/?p=java-opensaml.git;a=blob;f=opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/derivation/impl/ConcatKDF.java;hb=refs/heads/main
>>>>>>
>>>>>> By contrast, for PBKDF2 we are using the SecretKeyFactory 
>>>>>> service with the JDK-provided algorithm support for 
>>>>>> "PBKDF2With*" (completed with the appropriate PRF algo ID).  So 
>>>>>> if the BC provider had a SecretKeyFactory impl for ConcatKDF by 
>>>>>> itself, that would be closer to our needs. But....
>>>>>>
>>>>>> Second, it's unlikely we could introduce a deployment 
>>>>>> requirement for people to add the BC provider declaratively in 
>>>>>> java.security and also questionable for us to add the BC 
>>>>>> provider programmatically by default.  As a library OpenSAML 
>>>>>> probably shouldn't muck with people's environments like that.
>>>>>>
>>>>>> And in XML Encryption 1.1, ConcatKDF is the 
>>>>>> mandatory-to-implement KDF (PBKDF2 is optional).  For that 
>>>>>> reason it is our default KDF for ECDH, and it needs to work 
>>>>>> out-of-the-box.  So hopefully you can see that adding a 
>>>>>> requirement on configuring a third-party provider would be an 
>>>>>> issue for us.  Our team can discuss further the idea of 
>>>>>> automagically/programmatically adding in BC.  I personally am 
>>>>>> not a fan of the idea.  But unless/until BC has a provider-based 
>>>>>> impl of the ConcatKDF by itself, it's essentially a moot question.
>>>>>>
>>>>>> I guess what I would ask about the FIPS version is: If the 
>>>>>> provider there fundamentally supports ConcatKDF via the 
>>>>>> KeyAgreement algorithms you mention, then why aren't the KDF 
>>>>>> interfaces/impls there as well? Probably they are there but just 
>>>>>> with different package and/or class names, etc?  If they are, 
>>>>>> then you know, that means the FIPS version really isn't a 
>>>>>> drop-in replacement for the regular library, so it's going to be 
>>>>>> challenging for us as a downstream consumer to overcome that.
>>>>>>
>>>>>> Thanks,
>>>>>> Brent
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 7/13/21 10:30 PM, David Hook wrote:
>>>>>>> Hi Brent,
>>>>>>>
>>>>>>> I'd like to suggest that it gets changed to use the BC provider 
>>>>>>> rather than the BC low-level API.
>>>>>>>
>>>>>>> For some reason I can't connect to any of the nabble 
>>>>>>> references, so I can't easily find the source file concerned, 
>>>>>>> but from what I understand V4.1 added support for the ConcatKDF 
>>>>>>> key derivation function with ECDH. When I checked the code this 
>>>>>>> looked like something the BCFIPS provider supports already (and 
>>>>>>> the BC provider) for the JCE. The agreement algorithms ending 
>>>>>>> in CKDF use the concat KDF function.
>>>>>>>
>>>>>>> The following ones are currently available in both providers:
>>>>>>>
>>>>>>> ECCDHWITHSHA1CKDF
>>>>>>> ECCDHWITHSHA256CKDF
>>>>>>> ECCDHWITHSHA384CKDF
>>>>>>> ECCDHWITHSHA512CKDF
>>>>>>>
>>>>>>> I have subscribed to the dev list if you would prefer to 
>>>>>>> continue the discussion there. If you would include the link to 
>>>>>>> the source file either way we should be able to work something 
>>>>>>> out that will work for both providers (as I have commit access 
>>>>>>> to both, you'd hope so!).
>>>>>>>
>>>>>>> Let me know,
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> David
>>>>>>>
>>>>>>> On 14/7/21 7:17 am, Brent Putman wrote:
>>>>>>>>
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> Sorry David (Hook) that I didn't get back to you when I 
>>>>>>>> returned from vacation a couple of weeks ago, I unfortunately 
>>>>>>>> overlooked that email todo.
>>>>>>>>
>>>>>>>> I'm interested to hear what you had in mind.  If possible it 
>>>>>>>> would be great to have this discussion over on the 
>>>>>>>> Shibboleth/OpenSAML developers list, so that other members of 
>>>>>>>> the team and community can be aware and participate:
>>>>>>>>
>>>>>>>> https://shibboleth.net/mailman/listinfo/dev
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Brent
>>>>>>>>
>>>>>>>>
>>>>>>>> On 7/9/21 11:54 AM, David Castro wrote:
>>>>>>>>> Brent,
>>>>>>>>>
>>>>>>>>> Just wanted to follow up Re: the following thread on the FIPS 
>>>>>>>>> issue we were having.
>>>>>>>>>
>>>>>>>>> https://shibboleth.1660669.n2.nabble.com/Bouncy-Castle-FIPS-Issue-td7649455.html#a7649653 
>>>>>>>>> <https://shibboleth.1660669.n2.nabble.com/Bouncy-Castle-FIPS-Issue-td7649455.html#a7649653>
>>>>>>>>>
>>>>>>>>> We spoke with David Hook from Bouncy Castle (included in this 
>>>>>>>>> thread) and he had some ideas on resolving the issue. Scott 
>>>>>>>>> Cantor recommended you as the point of contact, but let us 
>>>>>>>>> know if there is someone else we should reach out to.
>>>>>>>>>
>>>>>>>>> We are also available to assist as needed.
>>>>>>>>>
>>>>>>>>> Cheers,
>>>>>>>>> David
>>>>>>>>>
>>>>>>>>> -- 
>>>>>>>>> CONFIDENTIALITY NOTICE: This transmission, and any 
>>>>>>>>> attachments, may contain CONFIDENTIAL, PRIVILEGED or 
>>>>>>>>> PROPRIETARY information of HRworx, LLC (dba Intelliworx) that 
>>>>>>>>> is protected from disclosure under applicable laws. If you 
>>>>>>>>> are not the intended recipient, any disclosure, copying, 
>>>>>>>>> distribution, or use of any of the information contained in 
>>>>>>>>> or attached to this transmission is STRICTLY PROHIBITED. If 
>>>>>>>>> you have received this communication in error, please notify 
>>>>>>>>> the sender, by reply e-mail, and delete the original message 
>>>>>>>>> with any attachments. Thank you for your cooperation.
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://shibboleth.net/pipermail/dev/attachments/20210715/9ab64b9b/attachment-0001.htm>

More information about the dev mailing list