spring-attic / spring-security-saml

SAML extension for the Spring Security project
Other
420 stars 480 forks source link

SES-166: Consider using OpenSAML 2.6.4 (or above)? #140

Closed spring-projects-issues closed 4 years ago

spring-projects-issues commented 9 years ago

Thomas Maslen (Migrated from SES-166) said:

If I understand correctly, spring-security-saml2-core (both in 1.0.1.RELEASE and in master) is using OpenSAML 2.6.1 (as 1.0.0.RELEASE did).

That's not terrible, but there are a couple of fine reasons for moving to OpenSAML 2.6.4 or above (IIRC latest is 2.6.5):

[OpenSAML 3 has also been released (3.0.0, 3.1.0 and 3.1.1) and OpenSAML 2 may be headed toward legacy status, but the upgrade to 2.6.4+ is easy whereas moving to 3.* may be nontrivial].

[By the way, JIRA lists saml-1.0.0 and saml-1.0.1 under "Unreleased versions"]

spring-projects-issues commented 9 years ago

manoj pathak said:

Does this solve my decryption issue which I am facing while encrypting SAML token.

DEBUG Decrypter:631 - Attempt to decrypt EncryptedKey using credential from KEK KeyInfo resolver failed: org.opensaml.xml.encryption.DecryptionException: Probable runtime exception on decryption:unknown parameter type. at org.opensaml.xml.encryption.Decrypter.decryptKey(Decrypter.java:705) at org.opensaml.xml.encryption.Decrypter.decryptKey(Decrypter.java:628) at org.opensaml.xml.encryption.Decrypter.decryptUsingResolvedEncryptedKey(Decrypter.java:783) at org.opensaml.xml.encryption.Decrypter.decryptDataToDOM(Decrypter.java:524) at org.opensaml.xml.encryption.Decrypter.decryptDataToList(Decrypter.java:442) at org.opensaml.xml.encryption.Decrypter.decryptData(Decrypter.java:403) at org.opensaml.saml2.encryption.Decrypter.decryptData(Decrypter.java:141) at org.opensaml.saml2.encryption.Decrypter.decrypt(Decrypter.java:69) 09:21:51,120 ERROR Decrypter:639 - Failed to decrypt EncryptedKey, valid decryption key could not be resolved 09:21:51,120 DEBUG Decrypter:787 - Attempt to decrypt EncryptedData using key extracted from EncryptedKey faile

spring-projects-issues commented 9 years ago

Richard Kettelerij said:

+1, I would suggest moving to OpenSAML 3.x if possible.

spring-projects-issues commented 8 years ago

Donnchadh O Donnabhain said:

+1 for OpenSAML 3.x

spring-projects-issues commented 8 years ago

marian lopatnic said:

+1 for OpenSAML 3.x :)

spring-projects-issues commented 8 years ago

George Stanchev said:

+1 for OpenSAML 3.x

harishreddy-m commented 8 years ago

+1 for OpenSAML 3.x

manish-in-java commented 8 years ago

Please also note that OpenSAML 2.x will cease to be supported after July 2016. This necessitates a move to 3.x before that date.

kristofferpeterhansel commented 8 years ago

Not really that experienced with OpenSAML. But implementing an SP with OpenSAML 3 it looks like the API is changed quite a lot. At least not a whole lot of the code I find is compatible. So it will likely requite quite a bit of effort to update this project

csyperski commented 8 years ago

I does seem like a fair amount of work, and openSaml3 doesn't appear to have much documentation (less that OpenSaml2). Has any work started on migration to openSaml3, I don't see anything in the repo?

philvarner commented 8 years ago

SES unit tests succeed with OpenSAML 2.6.4. Unfortunately, three of them fail with 2.6.5 and 2.6.6 (jdk 1.8.0_74-b02):

testSignature_validSelfSigned(org.springframework.security.saml.metadata.MetadataManagerSignaturesTest) testSignature_validCA(org.springframework.security.saml.metadata.MetadataManagerSignaturesTest) testSignature_chain_CA(org.springframework.security.saml.metadata.MetadataManagerSignaturesTest)

The only change listed in the 2.6.5 Release Notes (http://svn.shibboleth.net/view/java-opensaml2/tags/2.6.5/doc/RELEASE-NOTES.txt?view=markup) is https://issues.shibboleth.net/jira/browse/JOST-240, which is releated to metadata providers, however that change seems entirely unrelated to the failing tests.

However, openws was also updated from 1.5.4 to 1.5.5 in the release, which itself updated xmltooling from 1.4.4 to 1.4.5 and, after experimenting with these versions that seems to be the issue. xmltooling has several changes that seem relevant to the test failures http://svn.shibboleth.net/view/java-xmltooling?view=revision&revision=847.

philvarner commented 8 years ago

It looks like the relevant change in xmltooling was done under JXT-117 in BasicX509CredentialNameEvaluator.java, which changes the behavior when the "trustedNames" (trusted certificate names) list is empty. The default behavior was to trust all certificates if this was the case, whereas the new behavior is to NOT trust any certificates if this is the case.

1.4.4:

    public boolean evaluate(X509Credential credential, Set<String> trustedNames) throws SecurityException {
        if (!isNameCheckingActive()) {
            log.debug("No trusted name options are active, skipping name evaluation");
            return true;
        } else if (trustedNames == null || trustedNames.isEmpty()) {
            log.debug("Supplied trusted names are null or empty, failing name evaluation");
            return true; <<<=====
        }

1.4.5:

  public boolean evaluate(X509Credential credential, Set<String> trustedNames) throws SecurityException {
        if (!isNameCheckingActive()) {
            log.debug("No trusted name options are active, skipping name evaluation");
            return true;
        } else if (trustedNames == null || trustedNames.isEmpty()) {
            log.debug("Supplied trusted names are null or empty, failing name evaluation");
            return false; <<<=====
        }

ExtendedMetadataDelegate defaults it's attribute metadataTrustCheck to true, so I think the correct fix for the three failing tests is to explicitly set this to false in those tests, since all three use non-CA signed certs, and were only accidentally passing before because of the bug in BasicX509CredentialNameEvaluator trust check.

jcook793 commented 8 years ago

I first came to this issue because of a bug in Apache's HttpClient 3.1 library, which gets pulled into our runtime via this dependency graph:

+- org.opensaml:opensaml:jar:2.6.0:compile
|  +- org.opensaml:openws:jar:1.5.0:compile
|  |  +- org.opensaml:xmltooling:jar:1.4.0:compile
|  |  |  \- ca.juliusdavies:not-yet-commons-ssl:jar:0.3.9:compile
|  |  \- commons-httpclient:commons-httpclient:jar:3.1:compile

HttpClient 3.x was end-of-life'd in 2011 and has some vulnerabilities which are not going to be addressed. OpenSAML 2 is also now end-of-life'd so we can't expect them to upgrade OpenWS or the HTTP client library.

Are there plans to move to OpenSAML 3? I'm worried by the age of some of these dependencies...

csyperski commented 8 years ago

So, there hasn't been much movement on Spring-Saml in a long time, is this project generally considered abandoned? I ask because I am very nervous about using it in an upcoming project if there are no plans to update it. Can anyone speak to this, are there plans to at least update the upstream dependencies (ie: OpenSaml, HttpClient, etc)?

I love spring-saml and I would be even willing to pitch in to fund the devs to continue working on it as it has made my life so much easier.

vschafer commented 8 years ago

I'll have some holidays soon, hopefully. I'm planning to spend some days on maintenance. Unless OpenSAML 3 is in shape which would allow fairly smooth transition, the project will stay at 2.6.x, unless someone is willing to sponsor the rewrite. But I will update dependencies to latest versions and walk through pull requests and issues.

gstanchev commented 8 years ago

Some of the dependency issues are chained and dependent on OpenSAML. I think it is imperative (for this project) to upgrade it. OpenSAML has reached EOL and I believe there are security issues open against the version spring-saml is currently using. I know we ran into dependencies issues with BC - our project depends on 1.5x but OpenSAML 2.6 brings and depends on 1.46 and a simple switch is not trivial. BTW thanks for all your work on this project, it is excellent component!

rmkellogg commented 8 years ago

I have started doing some research on migration to OpenSAML 3.x. Many of these classes have been simply relocated into different packages. Hopefully next week I might be able to provide a list to at least get started with the migration.

csyperski commented 8 years ago

@vschafer I can't speak for anyone else, but I would be willing to pitch in to ensure that spring-saml continues development. My company would only be able to do a meager amount as it hasn't turned a profit yet, but I feel there is real value is spring-saml, as before I tried to do the equivalent using just OpenSaml2 and failed miserably. I know your time is valuable and you are busy, so maybe a patreon/go fund me to allow multiple parties to fund the development, just a thought? I am also willing to help contribute code but I find OpenSaml3 to be almost completely undocumented (last time I looked there was less documentation than OpenSaml2) and SAML is pretty over engineered so I still have trouble wrapping my head around all aspects of it (thus my love for spring saml).

As another option, what if Spring-SAML forked OpenSaml2, updated the dependencies and absorbed it into Spring-SAML, fixing security issues with it as they come up? Is that easier that moving to OpenSAML3?

rmkellogg commented 8 years ago

I have upgraded my local deployment successfully to Bouncy Castle 1.55 and OpenSAML v2.6.4. Updates were done against Spring Security SAML 1.0.2 with Spring Framework 4.3.1 and Spring Security 4.1.1.

This includes the following key upgrades: OpenSAML WS (OpenWS) 1.5.1 to 1.5.4 (Latest 1.5.4) OpenSAML Tooling 1.4.1 to 1.4.4 (Latest 1.4.4) Apache Santuari (XMLSec) 1.5.6 to 1.5.7 (Latest 1.5.8 - Legacy version) Apache Commons Codec 1.7 to 1.9 (Latest 1.10)

No changes to the following technologies: Not Yet Commons SSL 0.3.9 (Latest 0.3.17) ESWASP Enterterprise Security API (ESAPI) 2.0.1 (Latest 2.1.0) JodaTime 2.2 (Latest 2.9.4) Apache Commons Collections 3.2.1 (Latest 3.2.2/4.1) Apache Commons Lang 2.6 (Latest 3.4 - probably safe to keep 2.6 as 3.x has breaking changes) Apache Commons HttpClient 3.1 (Later versions break API)

There is a OpenSAML 2.6.6 which fixes a few issues. Unfortunately neither 2.6.5 or 2.6.6 have been published to Maven Central for ease of integration.

rmkellogg commented 8 years ago

Question: Where did the contents of the org.opensaml.* packages come from included in the Spring Security SAML project? Are they a forked version of some code in OpenSAML? Will be helpful to think about the migration to OpenSAML 3.x.

rmkellogg commented 8 years ago

I really don't like the idea of embedding the OpenSAML 2.x code into the baseline. Then we take responsibility for all those fixes and the security auditors frown on that type of thing. Just my two cents though.

czarnyckm commented 8 years ago

The OpenSAML 2.6.6 dependencies can be resolved by this repository in your pom:

    <repositories> 
       <repository>
            <id>shib-release</id>
            <url>https://build.shibboleth.net/nexus/content/groups/public</url>
            <snapshots>
                <enabled>false</enabled>
            </snapshots>
        </repository>
        <repository>
            <id>shib-snapshot</id>
            <url>https://build.shibboleth.net/nexus/content/repositories/snapshots</url>
            <releases>
                <enabled>false</enabled>
            </releases>
        </repository>
    </repositories>

See also: https://wiki.shibboleth.net/confluence/display/DEV/Use+of+Maven+Central

bmatthews68 commented 8 years ago

The problem is that adding repositories to your POM is against the rules when publishing to Maven Central. It would be better to help the OpenSAML guys get their new artifacts published in Maven Central.

Kind regards, Brian

http://btmatthews.com http://ie.linkedin.com/in/bmatthews68 https://plus.google.com/+BrianMatthews68 http://twitter.com/bmatthews68 https://www.facebook.com/bmatthews68 http://blog.btmatthews.com/

On 31 August 2016 at 10:06, czarnyckm notifications@github.com wrote:

The OpenSAML 2.6.6 dependencies can be resolved by this repository in your pom:

<repositories>
   <repository>
        <id>shib-release</id>
        <url>https://build.shibboleth.net/nexus/content/groups/public</url>
        <snapshots>
            <enabled>false</enabled>
        </snapshots>
    </repository>
    <repository>
        <id>shib-snapshot</id>
        <url>https://build.shibboleth.net/nexus/content/repositories/snapshots</url>
        <releases>
            <enabled>false</enabled>
        </releases>
    </repository>
</repositories>

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/spring-projects/spring-security-saml/issues/140#issuecomment-243704061, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGEudLku7CkJ6BJO14Dz61otKlO_sYtks5qlUQYgaJpZM4HgcW6 .

csyperski commented 8 years ago

@bmatthews68 It seems the Shibboleth/OpenSaml devs are against publishing to Maven Central (all of a sudden?). So we might be on our own. In the short term, I am in favor of absorbing the latest 2.x branch of OpenSAML into SpringSAML, then updating the dependencies to the latest until we the spring saml team has time/resources to update against OpenSAML 3.x

Because of the inability to verify the integrity and origin of artifacts, Shibboleth product builds no longer use Maven Central. Instead, all artifacts are pulled from the Shibboleth project repository. Artifacts added to the project repository have been downloaded directly from the author, verified in the manner provided by the author and signed by the Shibboleth developers. [(https://wiki.shibboleth.net/confluence/display/DEV/Use+of+Maven+Central)]

csyperski commented 8 years ago

@rmkellogg

I really don't like the idea of embedding the OpenSAML 2.x code into the baseline. Then we take responsibility for all those fixes and the security auditors frown on that type of thing. Just my two cents though.

You don't think this would be better then using an older version of OpenSAML? I think we are taking responsibility now because we are using code that has passed EOL, right? With absorbing it, we could at least update the code to support the latest BC and httpclient, etc. We can at least remove some of that baggage, IMHO. I'm all for using OpenSAML 3, but I get the impression that the committers are short on time to make those changes.

czarnyckm commented 8 years ago

I think the embedded code make no sense. If you see how often the spring saml extension has been released in the past compared to the OpenSaml. Maybe better way to go is just release the project compiled against the last available version of the OpenSaml on Maven Central. Then the developers can easy override the OpenSaml version to the newer one in their projects as long as the API stay unchanged

rmkellogg commented 8 years ago

I concur. We should release a new release 1.0.3 or something along those lines that is restricted to updating key components (without OpenSAML 3.x). Then at a later point we tackle the OpenSAML 3.x upgrade as a more substantial update in 1.5. This would accomplish the short term and longer term goals.

csyperski commented 7 years ago

Can any devs weight in, has any process been made with 1.0.3?

ThrawnCA commented 7 years ago

BTW OpenSAML 3.1 and 3.2 are on Maven Central. Not sure why 3.3.0 isn't.

csyperski commented 7 years ago

Is anyone else nervous about using Spring SAML since there has been so little movement on this project? We are using it in production and I am worried that it has been basically abandoned. Is anyone else concerned about the longevity of Spring SAML?

philvarner commented 7 years ago

@csyperski -- I've been using this component for 6 years now in a very large application that has thousands of companies and millions of users. The lack of active development is a concern, but there's not actually much to actively develop -- the SAML 2.0 spec hasn't changed in many years and this project is very stable. Any security issues are likely to be in the OpenSAML library, for which the versions have been pretty quickly updated when vulnerabilities are found. The alternative is implementing your own support for SAML on top of the primitives in the OpenSAML library, which opens yourself up to a great risk of vulnerability.

There have been few to no compile or runtime incompatibilities with newer versions of Spring or Spring Security, so there hasn't been much need to make changes there. The way we use this library is to have a github fork of it with the poms updated to our newer versions of Spring and Spring Security, just to make sure there are not runtime incompatibilities. We also have a few light customizations on top of the library, so we'd need our own fork anyway.

There is the concern about updating the code to work with OpenSAML 3, but while OpenSAML 2 is no longer officially supported by Shibboleth, it's extremely stable, and given that the majority of OpenSAML users are still on that version, if there were a critical security issue found I think someone would patch it.

csyperski commented 7 years ago

@philvarner Thanks for the feedback, we are much smaller but I was very concerned that we bet on the wrong horse with Spring SAML

philvarner commented 7 years ago

@csyperski are there any other (Java) horses?

csyperski commented 7 years ago

@philvarner Investing the time and rolling your own based on OpenSAML (which turns out to be not so trivial)

vschafer commented 7 years ago

It's a fact that the product is fairly feature complete. There would be surely space for improving multi-tenancy support, but there's enough extension points to tailor the product to the exact need of advanced users. There haven't been new feature requests for a long time. And all typical use-cases I'm aware of are covered.

Support for new Spring features - such as Spring Boot - has been largely demonstrated in projects such as https://github.com/vdenotaris/spring-boot-security-saml-sample While I'd like to have the support directly in Spring SAML, the current 3rd party solutions work nicely and are easy-enough to use.

The major maintenance issue is indeed keeping up to date with Spring/Spring Security/OpenSAML releases. Last time I checked there was too much stuff missing from OpenSAML 3 to make a migration feasible. Things may have changed, I'll look into it.

The project isn't abandoned, you can expect updates in the future, but my time is very limited.

csyperski commented 7 years ago

@vschafer Thanks so much for the info. I have no issues with the feature set, we have customized spring SAML to support multi-tenancy without any issues. I was just more concerned about the security of the product with keeping up with upstream libs, etc. Thanks for all your work, it is a great library!

philvarner commented 7 years ago

I second that! Thanks so much @vschafer for all your work on this.

On Wed, Mar 1, 2017 at 9:33 AM, csyperski notifications@github.com wrote:

@vschafer https://github.com/vschafer Thanks so much for the info. I have no issues with the feature set, we have customized spring SAML to support multi-tenancy without any issues. I was just more concerned about the security of the product with keeping up with upstream libs, etc. Thanks for all your work, it is a great library!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/spring-projects/spring-security-saml/issues/140#issuecomment-283355829, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHoRCPN88wJS3_LKTV3Np-UsTDIO-p7ks5rhYHPgaJpZM4HgcW6 .

davidkron commented 7 years ago

Are there any new developments on this topic? We are a bit concerned about security as Sonatype reports the following vulnerabilities:

CVE-2013-4002 xerces : xercesImpl : 2.10.0 xercesImpl-2.10.0.jar Open CVE-2016-1000341 org.bouncycastle : bcprov-jdk15 : 1.46 bcprov-jdk15-1.46.jar Open CVE-2014-0107 xalan : xalan : 2.7.1 xalan-2.7.1.jar Open

venkateshkonapalli commented 7 years ago

@vschafer Is there an ETA for fixing Security Vulnerabilities (CVE) in dependent jars? https://github.com/spring-projects/spring-security-saml/issues/203

csyperski commented 7 years ago

@vschafer Do you still have any plans to update Spring SAML? It's been a bit and we haven't seen anything. I'm just curious if we can expect anything in the future?

rmkellogg commented 7 years ago

Sorry no spare cycles here. Do suggest we at least update the other dependencies I listed above. We have used them successfully for sometime. That would hopefully address the security concerns from @davidkron.

krewi1 commented 6 years ago

Hello, are there any updates about support for openSaml 3.x.x?

shruthiannappa commented 6 years ago

@vschafer : Is upgrade of spring security saml to opensaml v3 in plan anytime soon?

Regards, Shruthi

shruthiannappa commented 6 years ago

Is anyone successful in implementing SAML Websso profile using Opensaml v3

brahinets commented 6 years ago

+1 for upgrade to 3.x.x

ottenhoff commented 6 years ago

I took a look at the difficulty of migrating to use OpenSAML 3.3.x and believe it is a 120-160 hr project. Here is a look at the easy changes:

https://github.com/spring-projects/spring-security-saml/compare/master...ottenhoff:opensaml3-upgrade?expand=1

There are hours and hours of difficult changes ahead all while keeping in mind that the Shibboleth team is quite clear that OpenSAML is maintained for use by Shibboleth and stable APIs are not a primary concern of the project.

ibaskine commented 6 years ago

@rmkellogg You mentioned that you successfully upgraded your local deployment successfully to Bouncy Castle 1.55 and OpenSAML v2.6.4. Can you please share you changes to the pom?

gstanchev commented 6 years ago

Check the development branch: https://github.com/spring-projects/spring-security-saml/blob/develop/core/pom.xml

rmkellogg commented 6 years ago

I did not rebuilt the spring-security-saml project. I just upgraded the JARs in my deployment as noted above. Therefore I can attest to compatibility with them.

jasonparallel commented 6 years ago

Has anyone looked into alternatives to OpenSAML like https://github.com/onelogin/java-saml

jzheaux commented 4 years ago

This was fixed in https://github.com/spring-projects/spring-security-saml/commit/006be6763b05ba3d040da24e177d7620f81d0b2f