This ticket is for Drupal 8

This ticket is discussing Drupal 8.

Problem / Motivation

This issue applies to any project that has installed this module prior to Nov 6, 2019 9 am EST, which is when simplesamlphp v1.17.7 was released.

Background:

See CVE https://security-tracker.debian.org/tracker/CVE-2019-3465

Original notice was from https://groups.google.com/forum/#!msg/simplesamlphp-announce/2odMqz63z7k...

We have been made aware of a security issue affecting all SimpleSAMLphp instances deployed as a service provider (basically, using SimpleSAMLphp to protect access to your application). This issue has been deemed critical, and will therefore need an urgent update. We will be releasing SimpleSAMLphp 1.17.7 during next Wednesday the 6th of November, at a time yet to be determined. We urge all SimpleSAMLphp users to make sure they are running the current stable version, so that upgrading to the new release doesn’t have any side effects, and to be prepared to upgrade their deployments as soon as the new stable release is published.

The details of the issue are embargoed for the time being, but will be made public after the bugfix release has been published. CVE 2019-3465 has been assigned to this issue.

Nov 5 a fix was released for simplesamlphp and the underlying xmlseclibs library, see:

Proposed Resolution

For any installs of this module after Nov 6, 2019 9 am EST no action needs taken. Composer will pull in the latest version of dependencies.

For installs of this module prior to that date, follow the actions below to update dependencies.

There isn't a single library with a fix. Several libraries have been updated. To ensure you are pulling in all updates related to this module, do the following:

$ composer update drupal/simplesamlphp_auth --with-dependencies
...some output....
Package operations: 0 installs, 13 updates, 0 removals
Gathering patches for root package.
Gathering patches for dependencies. This might take a minute.
  - Updating symfony/event-dispatcher (v3.4.31 => v3.4.33): Loading from cache
  - Updating symfony/filesystem (v3.4.31 => v3.4.33): Loading from cache
  - Updating symfony/yaml (v3.4.31 => v3.4.33): Loading from cache
  - Updating symfony/dependency-injection (v3.4.31 => v3.4.33): Loading from cache
  - Updating symfony/http-foundation (v3.4.31 => v3.4.33): Loading from cache
  - Updating psr/log (1.1.0 => 1.1.2): Loading from cache
  - Updating symfony/http-kernel (v3.4.31 => v3.4.33): Loading from cache
  - Updating symfony/config (v3.4.31 => v3.4.33): Loading from cache
  - Updating symfony/routing (v3.4.31 => v3.4.33): Loading from cache
  - Updating gettext/gettext (v4.6.3 => v4.8.0): Loading from cache
  - Updating robrichards/xmlseclibs (3.0.3 => 3.0.4): Loading from cache
  - Updating simplesamlphp/saml2 (v3.4.1 => v3.4.2): Loading from cache
  - Updating simplesamlphp/simplesamlphp (v1.17.6 => v1.17.7): Loading from cache
Package egeloen/http-adapter is abandoned, you should avoid using it. Use php-http/httplug instead.
Package jaimeperez/twig-configurable-i18n is abandoned, you should avoid using it. Use simplesamlphp/twig-configurable-i18n instead.
Package phpunit/phpunit-mock-objects is abandoned, you should avoid using it. No replacement was suggested.
...more output....

To double check, let's make sure we have the right versions installed:

$ composer show --installed | grep xmlseclibs
robrichards/xmlseclibs                         3.0.4               A PHP library for XML Security
$ composer show --installed | grep "simplesamlphp/simplesamlphp"
simplesamlphp/simplesamlphp                    v1.17.7             A PHP implementation of a SAML 2.0 service provider and identity provider, also compatible with Shibboleth 1.3 and 2.0.
$ composer show --installed | grep saml2
simplesamlphp/saml2                            v3.4.2              SAML2 PHP library from SimpleSAMLphp

Great! We have xmlseclibs 3.0.4, simplesamlphp v1.17.7, saml2 v3.4.2 which is what we want. There might be other libraries with CVE fixes, but we know at least these 3 libs should be updated.

Can't update to simplesamlphp v1.17?

(This for instance applies to D7 versions of the module.)

From the looks of it, xmlseclibs 3.0.4 (which contains the fix for the vulnerability) is a drop-in replacement for xmlseclibs 3.0.3 without disadvantages or things that need to be tested. Please update your xmlseclibs library.

Comments

josephdpurcell created an issue. See original summary.

juangong’s picture

juangong commented

Will there be a patch to this for the Drupal 7 version of this module?

matthensley’s picture

matthensley commented

The issue may not apply to 1.16.x

Is this based on some inside knowledge / theorizing that it's a problem introduced in 1.17.x?

geekwisdom’s picture

geekwisdom commented

I have been notified directly from the maintainers of SimpleSAMLphp. The issue is focused on SimpleSAMLphp running in SP mode. It's not an issue with the simplesamlphp_auth module itself.

josephdpurcell’s picture

josephdpurcell commented

Sorry for any confusion. This ticket is to track the fact the CVE exists and how it impacts those who use the simplesamlphp_auth Drupal module.

@geekwisdom is correct that the CVE applies to the third party library, https://packagist.org/packages/simplesamlphp/simplesamlphp. It does not apply to this module's code base. However, that library is used by this module. Thus, anyone using this module should be aware of that CVE.

As I follow the roll-out of the fix I'll update this ticket with my own findings on impact and steps to upgrade the library.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

> The issue may not apply to 1.16.x

Thanks for the heads up. This is indeed quite important, as the 7.x version, which I don't maintain, doesn't support 1.17 yet. So updating D7 sites is going to be trickier.

I guess this can't be disclosed yet in advance. As a note to all D8 users, make sure you update to 3.1, which (only) supports the 1.17 version.

pmeulen’s picture

pmeulen commented

SimpleSAMLphp 1.17.7, with the fix for CVE 2019-3465, has just been released. The issue is in the xmlseclibs library used by SimpleSAMLphp. SimpleSAMLphp versions that use this library, including 1.16.x, are affected. If you can't update SimpleSAMLphp, you can patch xmlseclibs manually.

matthensley’s picture

matthensley commented

Do we know what the highest supported version for D7 is? Anything 1.16.x?

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Right, but there is no fix in SimpleSAMLphp itself, it its release just updates the bundled dependencies? And if you install it through composer, which is the recommended approach IMHO on D8 (if your infrastructure can handle that), the you just need to update that specific dependency, I think?

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

> Do we know what the highest supported version for D7 is? Anything 1.16.x?

I think so. See #2953991: Warning because of changes in SimpleSAMLphp

pmeulen’s picture

pmeulen commented

Yes, the fix in SimpleSAMLphp was to update its xmlseclibs dependency which is included via the simplesamlphp/saml2 library.

For composer projects adding "robrichards/xmlseclibs": "^3.0.4", to the required packages in the composer.json of your project and then doing a "composer update" to update composer lockfile should force an updated xmlseclibs to be used, even when using an older SimpleSAMLphp.

The best fix is to ensure that you are using a current (i.e. 1.17.x) simplesamlphp IMO.

josephdpurcell’s picture

josephdpurcell commented
Issue summary: View changes

Looking at the diff between simplesamlphp library v1.17.6 and v1.17.7 https://github.com/simplesamlphp/simplesamlphp/compare/v1.17.6...v1.17.7... I can see it just sets "robrichards/xmlseclibs" to "^3.0.4".

The changelog references says:

Resolved a security issue that allows to bypass signature validation. See [SSPSA 201911-01](https://simplesamlphp.org/security/201911-01).

Looking at xmlseclibs we can see the change there: https://github.com/robrichards/xmlseclibs/compare/3.0.3...3.0.4

I've updated the description with a temporary workaround, i.e. directly upgrading simplesamlphp and xmlseclibs libraries.

For a longer term solution, should this module require simplesamlphp/simplesamlphp "~1.17.7"?

josephdpurcell’s picture

josephdpurcell commented
Issue summary: View changes
josephdpurcell’s picture

josephdpurcell commented
Issue summary: View changes
josephdpurcell’s picture

josephdpurcell commented
Title: Headsup - Critical SimpleSAMLphp issue release announcement for Wed 6th CVE 2019-3465 » Critical security vulnerability for SimpleSAMLphp as service provider CVE-2019-3465
lahoosascoots’s picture

lahoosascoots commented

Any idea how critical this is? Could users with no login credentials exploit this?

partyka’s picture

partyka commented

According to https://groups.google.com/forum/#!msg/simplesamlphp-announce/J2U2Z0vFY6U... the details are embargoed until 2019-11-07 at "around 2 CET".. If that's Central European Time .. I think that means 13:00 UTC. We just did our annual clock changes in the US so not sure if any other offsets are needed.

All I needed to do to update was this:

composer require 'robrichards/xmlseclibs:3.0.4'
composer require 'simplesamlphp/simplesamlphp:1.17.7'

Note .. This doesn't change the constraints in this module's composer.json file.

absoludo’s picture

absoludo commented

Simply updating this module and its dependencies was enough for me.

composer update drupal/simplesamlphp_auth --with-dependencies

Adding additional requirement is unnecessary, they are managed by the module itself.

josephdpurcell’s picture

josephdpurcell commented

Regarding comment #18, this would introduce the technical debt of having specific versions pinned that you would need to unpin at a future date to upgrade (or do another require statement to pull in the newer version).

Regarding comment #19, I found that update --with-dependencies updated more than the specific libraries. This may be undesirable if you only want to pull in the specific fix for this CVE.

Those are valid workarounds if the consequences are acceptable. The description of this ticket clarifies a path to update those libraries without those two consequences. Specifically:

$ composer update simplesamlphp/simplesamlphp robrichards/xmlseclibs

The long term fix is still TBD. I suspect it would involve updating this module's composer.json to require v1.17.7 or newer of simplesamlphp.

absoludo’s picture

absoludo commented

Hi Joseph,

You are correct, but the composer.json of the module also does not limit the upper version, so a clean install would also install the latest version of all the modules requirement.
Overlapping module requirements with a limited upper version however, will limit it.

I don't see the problems with my solution.

However, only updating the packages, as you mentioned, does introduce issues since, robrichards/xmlseclibs is a requirement of simplesamlphp/saml2 which is a dependency of simplesamlphp/simplesamlphp, so I'm missing the full update in your solution.

josephdpurcell’s picture

josephdpurcell commented
Issue summary: View changes

Great point about a fresh install of this module is not affected. I'll note that in the description.

Sorry, I didn't mean to say the solution in #19 has a problem, just wanted to note it will update all dependencies and not just the two specific libraries: simplesamlphp and xmlseclibs.

But, looking at simplesamlphp/saml2 I see it has an update as well that goes beyond just using the latest version of xmlseclibs. For example, it adds an assertion: https://github.com/simplesamlphp/saml2/compare/v3.4.1...v3.4.2#diff-6744...

I'm thinking the solution in #19 is probably the safest overall. Going the path I suggested would require specifying every single library that has a change for this CVE.

I'm updating the description to use that approach.

josephdpurcell’s picture

josephdpurcell commented
Issue summary: View changes
partyka’s picture

partyka commented

Regarding my comment earlier -- absolutely it would need to be unpinned later on.

josephdpurcell’s picture

josephdpurcell commented
Issue summary: View changes
Status: Active » Needs review

I updated the description since it sounds like the temporary solution is actually the permanent fix for existing installs of this module.

Submitting this ticket for review / approval by the maintainer. Again, this ticket is to clarify what actions need taken by those using this module as it relates to CVE-2019-3465.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Category: Bug report » Task
Status: Needs review » Active

The maintainer still thinks the the only thing that you have to update is robrichards/xmlseclibs, so composer update robrichards/xmlseclibs. all other changes in simplesamlphp/simplesamlphp and simplesamlphp/saml2 are unrelated bugfixes that ended up in those releases. You can see that all other changes in saml2 are from august/july for example.

Require ^1.17.7 and doing a new release is an option if someone wants to write a patch, but it's not required. Unlike simplesamlphp/simplesamlphp, we don't offer any bundled downloads that contain the vulnerable dependency and sites are responsible to update their composer dependencies.

https://www.drupal.org/psa-2019-09-04 explains that pretty clearly :)

I'm setting this back to active in case someone wants to propose a patch with that, but it won't help *that* much unless I would also release it as a security fix, which needs to be vetted by the security team. And that in turn wouldn't help those that don't have the dependency installed with composer :)

roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
commented

This is not my issue to play with, but could we delete the following prominent-ish information near the top of the original issue message, which feels misleading?

The issue may not apply to 1.16.x

We have every reason to believe that it does apply to 1.16.x because

  • It's in xmlseclibs, not simplesamlphp.
  • The SimpleSAMLphp maintainers, in their original pre-announcement two days ago, encouraged everyone to 1.17.6 in order to be able to patch quickly. They wouldn't have done that if 1.16 wasn't vulnerable, I think.
  • No info have been given to support the above statement.
berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Absolutely, please remove it :)

roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
commented
Issue summary: View changes

OK.

I made a crude addition to the bottom of the original message. Feel free to add more info because I'm not testing an upgrade path. My issue is I'm not a user of the simplesamlphp_auth module, I just came here from a related issue.

I hope the D7 version did already pull in v3.0.3 of the xmlseclibs library, because then the changes to v3.0.4 (https://github.com/robrichards/xmlseclibs/commits/master) look really innocuous and people should just update the library.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Category: Task » Plan
Status: Active » Fixed

We raised the min version now in #3097283: Allow updating to SimpleSAMLphp 1.18 and higher to fix security problem 201911-02

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.