Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update opensaml dependency to 4.1.0 #71983

Closed
2 tasks done
jkakavas opened this issue Apr 20, 2021 · 7 comments · Fixed by #98199
Closed
2 tasks done

Update opensaml dependency to 4.1.0 #71983

jkakavas opened this issue Apr 20, 2021 · 7 comments · Fixed by #98199
Assignees
@tvernum
Labels
>non-issue :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) Team:Security Meta label for security team v8.0.0

Comments

@jkakavas
Copy link
Member

jkakavas commented Apr 20, 2021 •
edited by tvernum
Loading

opensaml 4.1.0 was released on 23/04/21. We should look into upgrading our dependencies for the SAML realm and the Identity Provider implementations

  • Investigate major API changes between 3.4.5 and 4.1.0
  • Look into release notes for bugs resolved and major functionality additions
@jkakavas jkakavas added >non-issue :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) labels Apr 20, 2021
@elasticmachine elasticmachine added the Team:Security Meta label for security team label Apr 20, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

@tvernum
Copy link
Contributor

tvernum commented May 5, 2021

OpenSAML 4.0 requires Java 11.
We cannot upgrade in the 7.x development tree.

@tvernum tvernum added the v8.0.0 label Aug 13, 2021
@tvernum tvernum self-assigned this Aug 25, 2021
@tvernum tvernum mentioned this issue Aug 30, 2021
Merged
@tvernum
Copy link
Contributor

tvernum commented Aug 31, 2021

I've started work on this in #77012

There's a few issues to work out, but most of them seem to be solvable except OpenSAML4 seems to have a hard dependency on the standard (non-FIPS) distribution of BouncyCastle.
It requires org.bouncycastle.jce.ECNamedCurveTable which doesn't exist in BCFIPS.
This makes it very difficult for us, because we want to support SAML when running in FIPS mode, and we can't do that (at least, not easily) if the BC jars are also on the classpath.

tvernum added a commit that referenced this issue Sep 22, 2021
@tvernum
Update to OpenSAML v4.0 Library (#77012)
e73d16d 
This commit switches the security and identity-provider plugins to use
v4.0.1 of the OpenSAML library (upgraded from v3.4).

In order to facilitate this upgrade the following changes are also
made:
- Common Codec is upgraded to 1.14 across all modules
- Guava is upgraded to v28.2 in the 2 affected modules

Relates: #71983
@tvernum
Copy link
Contributor

tvernum commented Sep 22, 2021 •
edited
Loading

We've upgraded to v4.0 in #77012, but could not upgrade to v4.1 due to an incompatibility with FIPS.
I'm leaving this issue open so that we can look at it again in the future.

@arteam arteam added v8.1.0 and removed v8.0.0 labels Jan 12, 2022
@tvernum tvernum removed the v8.1.0 label Jan 14, 2022
@tvernum tvernum added the v8.0.0 label Jan 4, 2023
@gwbrown
Copy link
Contributor

gwbrown commented Mar 8, 2023

A few more versions of OpenSAML are out - as of this comment they're up to 4.3.0. Perhaps we should check whether OpenSAML is still incompatible with bc-fips or if this problem has been addressed somehow.

@albertzaharovits
Copy link
Contributor

I can confirm that OpenSAML 4.3.0 still requires bouncy castle as a runtime dependency.
Here is an excerpt of the dependency graph:

|    +--- org.opensaml:opensaml-security-api:4.3.0
|    |    +--- org.opensaml:opensaml-core:4.3.0 (*)
|    |    +--- org.opensaml:opensaml-messaging-api:4.3.0 (*)
|    |    +--- commons-codec:commons-codec:1.15
|    |    +--- com.google.code.findbugs:jsr305:3.0.2
|    |    +--- com.google.guava:guava:31.1-jre (*)
|    |    +--- org.cryptacular:cryptacular:1.2.5
|    |    |    \--- org.bouncycastle:bcprov-jdk18on:1.71 -> 1.72
|    |    +--- org.bouncycastle:bcprov-jdk18on:1.72
|    |    +--- org.bouncycastle:bcpkix-jdk18on:1.72
|    |    |    +--- org.bouncycastle:bcprov-jdk18on:1.72
|    |    |    \--- org.bouncycastle:bcutil-jdk18on:1.72
|    |    |         \--- org.bouncycastle:bcprov-jdk18on:1.72

Here's a sample failure https://gradle-enterprise.elastic.co/s/26awfttvvkagg from my incomplete attempt at an upgrade #95633 . The opensaml initialization code builds a map of supported EC curve params, presumably to be later used to validate signatures.

As I understand it, we don't want to supply a runtime dependency of org.bouncycastle:bcprov-jdk18on:1.72 because we cannot claim (and truthfully so) FIPS compatibility if such a non-FIPS provider is on the classpath.

@albertzaharovits
Copy link
Contributor

PS Look for references of ECNamedCurveTable or ECNamedCurveParameterSpec in GlobalNamedCurveRegistryInitializer.

@albertzaharovits albertzaharovits mentioned this issue Aug 14, 2023
Merged
albertzaharovits added a commit that referenced this issue Aug 23, 2023
@albertzaharovits
Upgrade to OpenSAML 4.3.0 (shadowed) (#98199)
1b468a0 
This commit upgrades to OpenSAML v4.3.0

Versions of OpenSAML ≥ 4.1 have a hard dependency on the non-FIPS release of BouncyCastle.
This would prevent ES from being able to run in a JVM where BC-FIPS is configured as the security provider.

Closes: #71983

Co-authored-by: Tim Vernum tim@adjective.org
albertzaharovits added a commit to albertzaharovits/elasticsearch that referenced this issue Aug 23, 2023
@albertzaharovits
Upgrade to OpenSAML 4.3.0 (shadowed) (elastic#98199)
88b7e84 
This commit upgrades to OpenSAML v4.3.0

Versions of OpenSAML ≥ 4.1 have a hard dependency on the non-FIPS release of BouncyCastle.
This would prevent ES from being able to run in a JVM where BC-FIPS is configured as the security provider.

Closes: elastic#71983

Co-authored-by: Tim Vernum tim@adjective.org
@albertzaharovits albertzaharovits mentioned this issue Aug 23, 2023
Merged
albertzaharovits added a commit that referenced this issue Aug 23, 2023
@albertzaharovits
Upgrade to OpenSAML 4.3.0 (shadowed) (#98199) (#98785)
07aedfd 
This commit upgrades to OpenSAML v4.3.0

Versions of OpenSAML ≥ 4.1 have a hard dependency on the non-FIPS release of BouncyCastle.
This would prevent ES from being able to run in a JVM where BC-FIPS is configured as the security provider.

Backport of: #98199

Closes: #71983

Co-authored-by: Tim Vernum tim@adjective.org
@dancristiancecoi dancristiancecoi mentioned this issue Nov 18, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tvernum tvernum

Labels
>non-issue :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) Team:Security Meta label for security team v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Upgrade to OpenSAML 4.3.0 (shadowed) albertzaharovits/elasticsearch
6 participants
@gwbrown @arteam @tvernum @albertzaharovits @jkakavas @elasticmachine