Problem Description

The XML Security Library (xmlsec1) is a binary that is required for Mattermost.  The xmlsec1 binary provides the encryption/decryption and digital signing functionality required for the implementation of Security Assertion Mark-up Language (SAML), which is used by Mattermost to support single sign-on (SSO).

There are a couple issues with using xmlsec1.  First of all, it is an external binary that must be installed separately by the administrator. Because of the many different potential OS, we cannot reasonably ship xmlsec1 with our deployment. Therefore, it has to be installed separately. In addition, there was recently an issue where the version of xmlsec that was installed didn’t support self-signed certificates. This was a very difficult problem to track down (but that’s a separate issue).

Second, based on this Jira ticket filed under security it is a security risk.
“Parsing the XML in-process and validating the signature via another process is hazardous. It takes way too much thought for me to convince myself there are no remaining signature wrapping attacks.”

The way it is currently used is for the software to dump the XML document to a file, shell out to xmlsec1, then read the resulting file from the OS. Although it works, it is not the cleanest of implementations.

Potential Solutions

There are a few different Go Libraries that we could us in order to provide this functionality. In a pure go environment.

GoLang Signing LIbraries
https://github.com/crewjam/go-xmlsec
https://github.com/russellhaering/goxmldsig

I didn’t actually investigate any of these libraries specifically, because there are also Go libraries for SAML and two of the above libraries were written for the SAML libraries.

GoLang SAML Libraries

A quick investigation of the version from RobotsAndPencils revealed that it used the xmlsec1 binary.  OUT!

After spending some time with the crewjam version, it appears to be designed to take control at the HTTP stack and control Authentication to your “protected” pages.  A bit too heavy for what we are looking for. Although, it has to have the code to do what we need, it may require more work to make it work for our use case. SKIP (for now).

The version from russellhaering seems to be what we are looking for. It uses another russellhaering library for decryption and digital signing. Looking at the sample code, it appears to be similar to our implementation.

Proof of Concept

In order to evaluate the library, I started with their demo. I had to make a few changes but had it working against my Okta developer account in a couple hours.

Short story is it took about a day and a half to replace our SAML implementation with the SAML2 library. This is simply a proof of concept a uses mostly hard-coded values.  But it worked and was fairly easy to implement and allowed me to perform a Feature Comparison below.

The file “enterprise/saml/saml.go” needed rewritten. But that was the only file that required any changes. After the change, the rest of the files in this directory can be removed.

Feature Comparison

Does it support the same features we require? Short answer is NOT all of them, but it does support a more complete feature set than our implementation.

Features Not Supported

Scoping Element
Although we don’t fully support the scoping element, we add information to this element in order to support a feature of SAML Azure. It appears only the backend implemented as the UI ticket was closed as “won’t fix”.  Also it appears we did this for a prospect. Whether or not support for this feature is required is unknown. However, if we had to implement it, as we have it currently implemented, it would only take a couple of days.

https://techcommunity.microsoft.com/t5/Azure-Active-Directory-Identity/Using-Azure-AD-to-land-users-on-their-custom-login-page-from/ba-p/243900

Digest Algorithm/Signing Algorithm

A recently added feature to Mattermost allows the user to select which algorithms to use for Signature and Digest algorithms. These are separate settings as implemented. The SAML2 library allows them to be set, however, they both get set to the same “bit” algorithm. If SHA1 is selected then a SHA1 algorithm is selected for both the Signature and Digest Algorithms. Our implementation is due to release in 5.18, so we can simply match our support to this before releasing.  Then we would have feature parity here. Should take less than a day.

Features Supported

The following features are supported by the SAML2 library. Currently, our implementation does not support these features. If supporting these features is desired, we would only need to implement UI and configuration settings. We would get the backend implementation for free.

Signature Validation

Although we verify signatures, we don’t check the std error, so we don’t stop if signature fails verification.
We don’t verify the Assertion signature

Validates Audience URI
We don’t set, nor validate.

SigningKeyStore

Optional signing key, Encryption key is default signing key.

NameIdFormat

Tells the IdP the NameId to return in its response Assertion. (Need to investigate further, but may eliminate the need for IdAttribute?

ValidateEncryptionCert

Checks validity period of certificate, our implementation always checks.

AllowMissingAttributes

Valid even if NO Attributes are returned. Our implementation requires at least an “ID” attribute and any attributes who is used for a user property.

Compatibility

GoSAML2 has been tested against the following libraries.

• Okta

• Auth0

• Shibboleth

• Ipsilon

• OneLogin

As part of my investigation, I tested the POC agains Okta and OneLogin.

Licensing

GoSAML2 is licensed under Apache 2.0 License

Next Steps

• Test with other IdPs that we support or know our large customers support. 

• Determine if we need to support Scoping Element - This is a YES based on comment above.

• Break down tasks and estimate work - Started

Phased Rollout

Making a major library change can be extremely risky and should be done with care. Although local testing will occur, testing with every possible SAML Identity Provider isn’t feasible and unexpected issues can be discovered in the wild. It is important to mitigate the risk involved with such a change.  Therefore, a phased approach will be used. 

1. The first phase will deploy both the current xmlSec1 library and the new goSAML library. A configuration setting will allow an administrator to switch to the goSAML library. This will allow us to some initial testing with the Mattermost instance, then ask other users to switch. The administrators would need to OPT-IN in order to user the new goSAML library.

2. After making any necessary fixes and have a greater confidence level. The new library will become the default. The administrators would have to OPT-OUT in order to continue to use the old xmlSec1 library. This allows a safety valve in case there is an installation that still doesn’t work. This stage should flush out any issues with any installations.

3. The final stage is to completely remove the xmlsec1 implementation. Ideally, this would happen on a Major version change.

Preliminary Tasks Breakdown (Mana?)

1. Error handling/logging - 2

2. Use configuration settings - 1

3. Implement GetMetadata() - 2

4. Reimplement userFromSamlResponse() - 1

5. Review/Implement Certificate Loading - 4

6. Update for new libraries (go.mod) - 1

7. Unit tests - 4

8. Performance Tests - ? not sure how much infrastrure we have.

9. Potential Build Issues (dependencies only in Enterprise) ??

10. Comparison Testing (old vs new response) ???

11. Add config Switch for Experiment/Testing(?)

12. Implement Scoping Element in goSAML2 - 8
    Create Fork other admin tasks associated

13. Remove Digest setting from 5.18, set based on Signature Algorithm bit. - 2 

14. Remove old SAML implementation - 2