Consider guarding classloading of ECNamedCurveTable

Description

At Spring Security, we get occasional reports like the following: https://github.com/spring-projects/spring-security/issues/14656

This is due to, it appears, a couple of places in OpenSAML where there is a direct reference to BouncyCastle APIs that aren’t replicated in both BC jdk18on and fips. Of course, perhaps there is more to it.

One possibility that could simplify applications using a different BC version would be to use a classpath guard to avoid referencing ECNamedCurveTable if it’s not on the classpath; for example, AbstractNamedCurve#buildParameterSpec could proceed to its fallback strategy.

If it’s permitted, I’d be happy to contribute a PR along these lines.

Environment

None

Activity

Scott Cantor 
October 29, 2024 at 12:27 PM

I don’t think it matters fundamentally what the XML Enc spec happens to mandate. For one thing, almost nobody even implements EC-DH, at least we did. Having a separate module for people that care to isolate the BC issue seems pretty reasonable to me if that’s the only way we can isolate it.

Josh Cummings 
October 28, 2024 at 9:54 PM

Thanks, that addition to the document makes it very clear! And thanks for the link to the cryptacular ticket. Feel free to close this ticket at this point.

Brent Putman 
October 28, 2024 at 8:37 PM

Yes, aside from the stuff Scott already mentioned (cryptacular, various ASN.1 usage), there’s other compile-time dependencies on BC which aren’t in the BC FIPS version.

Off-hand, in addition the the ECNamedCurveTable we also use BC’s impl of ConcatKDF for ECDH encryption. That’s actually the mandatory-to-support one in the XML Encryption spec, making that tricky to put in a optional module. There may be other stuff I’ve forgotten as well.

Dropping BC entirely is going to be hard. For some things like cert parsing there may be acceptable options in the JDK now. But I don’t think that’s true for more esoteric crypto things like the EC curves and ECDH stuff. I don’t know what the path forward is going to be for all that, short of just importing the BC code into our project and maintaining it, which doesn’t fill me with joy. For a long time BC has been the one and only open source crypto library, so there really aren’t other options that I know of.

Scott Cantor 
October 28, 2024 at 8:09 PM

I will say it’s plausible once the cryptacular issue is dealt with (as that’s too core to the library) that we could probably try and move some of the other offending code into a separate Maven module if it’s not practical to fix as easily.

Up to now, that’s been moot to consider because of the scope of the issue.

Scott Cantor 
October 28, 2024 at 8:03 PM

https://shibboleth.atlassian.net/browse/OSJ-410

I’d defer to Brent, I don’t know that I’m correct about those assertions so I’d let him close the issue if he doesn’t think there’s anything helpful to be done.

The place to point people that ask about FIPS is here:

https://shibboleth.atlassian.net/wiki/spaces/DEV/pages/1159627167/FIPS

I will add something to the effect that unless/until that dependency is gone, there is no possible way to make the situation any better.

Won't Do

Details

Assignee

Reporter

Components

Created October 28, 2024 at 6:45 PM
Updated October 29, 2024 at 12:27 PM
Resolved October 29, 2024 at 12:27 PM