Update PHP-SAML library to 2.10.0 to fix login security vulnerability

[chris001]

The SAML integration is based on a very old 2010 version of the Onelogin's SAML toolkit, which was found to be vulnerable to a signature-wrapping attack.

Issue

SAML is used to provide single sign on to web applications.

Expected Behavior

Update the SAML library to prevent login attacks.

Actual Behavior

The signature wrapping attack allows an attacker to login to a user account by signing the request multiple times.

Possible Fix

• The PHP-SAML library should be updated and use the latest version 2.10.0 or newer, which has fixed the security issue:
  https://github.com/onelogin/php-saml

• The PHP-SAML toolkit also supports composer. You can find the onelogin/php-saml package at https://packagist.org/packages/onelogin/php-saml

In order to import the latest version of the saml toolkit, run:
composer require onelogin/php-saml

After installation has completed you'll find inside the vendor/ folder a new folder named onelogin and inside, php-saml. Make sure you are including the autoloader provided by composer. It can be found at vendor/autoload.php.

Important: When using composer, the x509 certs must be stored at vendor/onelogin/php-saml/certs and settings file stored at vendor/onelogin/php-saml.

Your settings are at risk of being deleted when updating packages using composer update or similar commands. So it is highly recommended that instead of using settings files, you pass the settings as an array directly to the constructor. If you do not use this approach your settings are at risk of being deleted when updating packages using composer update or similar commands.

Steps to Reproduce

1. See PHP-SAML github page for more details on the signature wrapping hack and their fix. https://github.com/onelogin/php-saml

Update php-saml to 2.10.0 or newer, this version includes code to perform extra verification and prevent signature wrapping login attacks.
php-saml < v2.10.0 is vulnerable and allows signature wrapping!

Context

Medium-High priority. It's a serious security issue. SAML is such a popular method of login because it's fast, protects passwords, reduces password proliferation. The PHP application SuiteCRM doesn't hold any user password data, only the identity provider, such as microsoft, facebook, twitter, google, holds the user's password data. The SuiteCRM PHP application is much harder to hack when users are logging in with SAML, because the hacker would need to attack the security of the Identity Provider (social network typically) yet social networks typically run high security, are constantly audited by world class security teams, and send highly deliverable email and push notifications, to the user, of any suspicious login activity coming from new devices and new locations.

Your Environment

• SuiteCRM Version used:
• Browser name and version (e.g. Chrome Version 51.0.2704.63 (64-bit)): Mozilla Firefox 51
• Environment name and version (e.g. MySQL, PHP 7): MySQL 5.7, PHP 7.
• Operating System and version (e.g Ubuntu 16.04): Debian 8.

[shogunpol]

@chris001 , is this any chance to ask you to work on it and create some PR? looks like you did a lot of the work already.

[chris001]

@shogunpol
The existing library used by SuiteCRM is so many years out of date, that the current secure version of the SAML toolkit has changed its PHP class names, and the file you must require_once to load the toolkit library classes. A direct composer update to the new code would break the current SAML functionality with class not found errors. To make composer update work requires some changes to the existing SuiteCRM code.

Compatibility

This 2.0 version has a new library. The toolkit is still compatible.

The old code that you used in order to add SAML support gonna continue working with minor changes. You only need to load the files of the lib/Saml folder. (notice that the compatibility.php file do that).

The old-demo folder contains code from an old app that uses the old version of the toolkit (v.1). Take a look.

Sometimes the names of the classes of the old code could be a bit different and if that is your case you must change them for OneLogin_Saml_Settings, OneLogin_Saml_Response, OneLogin_Saml_AuthRequest or OneLogin_Saml_Metadata.

We recommend that you migrate the old code to the new one to be able to use the new features that the new library Saml2 carries.

[ebogaard]

I'm afraid this patch seems to lead to an unparseable saml xml. See the following error from SAMLtracer:

<parsererror>
    <sourcetext>    AssertionConsumerServiceURL=&quot;https://test.xxx.nl//index.php?action=Login&amp;module=Users"&gt;
-----------------------------------------------------------------------------------------------^</sourcetext>
</parsererror>

When properly aligned, the arrow points at the question mark after 'index.php'.
When I use the old SAMLAuthenticate, the URL is enclosed in quoted properly:

AssertionConsumerServiceURL="https://prod.xxx.nl/index.php?module=Users&action=Authenticate"

I'm not sure how to fix this (yet), but maybe someone else has an idea.

[samus-aran]

@ebogaard When do you get this error?

• Are you able to create a new github issue so we can tackle it.

[chris001]

If it only needs quotes around the URL, that's a simple fix.
For saml tracer do you mean this : https://addons.mozilla.org/en-GB/firefox/addon/saml-tracer/
Or these for chrome: https://www.samltool.com/saml_tools.php

[ebogaard]

To me it seems that the double quotes are the only problem (but I'm no SAML2.0 of xml-expert).
I've used the Firefox saml-tracer you quoted to trace the problem.
I suspect the Chrome version should show a similar parse error.

Should I create a new issue, or do we continue in this issue?

[chris001]

@ebogaard It'd probably be best to create a new issue on onelogin/php-saml at https://github.com/onelogin/php-saml/issues/new and include the link to this issue page, #2819

[ebogaard]

Okay, done: see issue #201 @ php-saml

[chris001]

@ebogaard Also could you post in a new issue here, the lines from your php log showing the failed saml2 login error, they should say the error, name of the source code file and the line where the error happens.

[ebogaard]

New issue opened: #3270