TT-1322-Fix SAML vuln and broken tests

[jlucktay]

Description

Dependencies

• updated crewjam/saml from v0.4.0 to v.0.4.5 to address a vulnerability (33d812c)
• updated vendored code (0e0ceb6)

configuration package (41633ee)

• TestOverrideConfigWithEnvVars was looking for a non-existent config file, so added one to its testdata subdirectory

data_loader package (e1f3016)

• MongoLoader.Flush: simplified type assertion in switch statement
• TestCreateDataMongoLoader:
  • depends on an external running instance of MongoDB, so it is now behind a test_mongo tag; run this test with something like go test -tags=test_mongo instead of just go test
  • simplified type assertion

providers package (4fd5888)

• TestProxyProvider_GoodRegex was looking for a string that wasn't there any more

Related Issue

https://www.bleepingcomputer.com/news/security/critical-golang-xml-parser-bugs-can-cause-saml-authentication-bypass/

Motivation and Context

Security vulnerabilities are bad!

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

• Make sure you are requesting to pull a topic/feature/bugfix branch (right side). If pulling from your own
  fork, don't request your master!
• Make sure you are making a pull request against the master branch (left side). Also, you should start
  your branch off our latest master.
• My change requires a change to the documentation.
  • If you've changed APIs, describe what needs to be updated in the documentation.
  • If new config option added, ensure that it can be set via ENV variable
• I have updated the documentation accordingly.
• Modules and vendor dependencies have been updated; run go mod tidy && go mod vendor
• When updating library version must provide reason/explanation for this update.
• I have added tests to cover my changes.
• All new and existing tests passed.
• Check your code additions will not fail linting checks:
  • go fmt -s 366a9db
  • go vet

[sredxny]

LGTM!

Just for reference, this issue was fixed by integrating github.com/mattermost/xml-roundtrip-validator into the SAML project. Currently there's no native fix in the go language.

[sredxny]

@matiasinsaurralde please review 😬 ...We will need a release of Tib, and then include this release in dashboard (for embeded tib users)

[jlucktay]

Just for reference, this issue was fixed by integrating github.com/mattermost/xml-roundtrip-validator into the SAML project. Currently there's no native fix in the go language.

There's a good writeup from Mattermost here, and I think this quote is the most salient:

The Go security team has chosen this path after determining that the encoding/xml package and its data structures were not designed to make safe round-trips possible. The new API allows them to lower developers’ expectations to a more manageable level, as the support for round-trips in the most problematic cases can be explicitly dropped.

There was plenty of discussion on this proposal but ultimately it didn't go ahead, and I found another good quote to summarise there:

It's not being fixed in a security release because round-trip stability is not a currently supported security property of encoding/xml, and we don't believe these fixes would be sufficient to reliably guarantee it in the future.