SAML authentication bypass possibility due lack of IdP cert option

[srd90]

Current wiki.js implementation uses version of node-saml/passport-saml

wiki/package.json

Line 151 in 8dcbc18

"passport-saml": "1.3.5",

which has following issues:

---

Instances of passport-saml versions < 3.0.0 can be configured without IdP's certificate in which case the result is that passport-saml does not validate authentication responses' digital signatures. This means that anyone can post any authentication response with any content he/she chooses to. I.e. it is possible to bypass IdP login completely and impersonate anyone.

For background information see:

1. Missing cert configuration option disables signature validation silenty/without huge warning messages node-saml/passport-saml#523
2. The certificate of the IdP MUST be required node-saml/passport-saml#180
3. README file should indicate signature checking is critical (not just a good idea) node-saml/passport-saml#199

This issue was fixed to passport-saml version 3.0.0 (node-saml/passport-saml#199 (comment)).

If you are unable to upgrade passport-saml version consider to make cert option mandatory instead of optional:

wiki/server/modules/authentication/saml/authentication.js

Lines 31 to 33 in 8dcbc18

	if (!_.isEmpty(conf.cert)) {
	samlConfig.cert = _.split(conf.cert, '|')
	}

wiki/server/modules/authentication/saml/definition.yml

Lines 26 to 31 in 8dcbc18

	cert:
	type: String
	title: Certificate
	hint: (Optional) - Public PEM-encoded X.509 signing certificate. If the provider has multiple certificates that are valid, join them together using the | pipe symbol.
	multiline: true
	order: 4

---

Furthermore wiki.js'es current passport-saml version 1.3.5 uses vulnerable xmldom implementation:

• [BUG] Vulnerability dependency library xmldom  node-saml/passport-saml#628
  • 3.1.2 seems have fix for that issue.

[srd90]

@NGPixel could you take a look at this report...

SAML authentication response digital signature verification is the most important thing to validate about response (among other things like NotOnOrAfter and audience verifications). It is unfortunate that passport-saml versions prior to version 3.0.0 did not have (IdP) cert as mandatory configuration option. Instead those versions assumed that users of that library understand to provide IdP cert. It is even more unfortunate that passport-saml versions prior to 3.0.0 just silenty consume authentication responses without any warnings about disabled signature verification (due missing cert option).

wiki.js is now instructing end users that cert is optional. wiki.js should at least make cert required configuration option. Furthermore it could be good idea to issue CVE to make users of wiki.js (who are using SAML authentication) aware of this issue. Each wiki.js site which is using SAML authentication and which has not provided cert option is effectively open for anyone who chooses to initiate authenticated login session by posting forged SAML login response to callback interface.

Quote from https://docs.oasis-open.org/security/saml/v2.0/saml-sec-consider-2.0-os.pdf

6.4.3 Forged Assertion

Threat: A malicious user, or the browser user, could forge or alter a SAML assertion.
Countermeasures: The browser/POST profile requires the SAML response carrying SAML assertions tobe signed, thus providing both message integrity and authentication. The Service Provider site MUSTverify the signature and authenticate the issuer.

Another quote from https://sec.okta.com/articles/2020/05/common-pitfalls-custom-saml-implementations

The Signature Does Not Exist Or Is Not Verified

First, if the SAML signature is not implemented or is not verified at all, the identity assertions are not protected. Attackers can forge the identity information in the SAML response at will.

[NGPixel]

Please report security issues using the channels listed in https://github.com/requarks/wiki/security/policy the next time. This is not the place to get a quick response.

[srd90]

This issue was reported to security@requarks.io at January 2021.

I did not receive any answer to that email.

Forgot whole thing. Net is "full" of unsecure passport-saml configuration examples/usages and wiki.js is just one of those). Now that I remembered this project again and based on

1. no answer to first report
2. "disclaimer" at this page https://docs.requarks.io/auth/saml which gives an impression that SAML authentication module is not in active mainentenance in wiki.js
   • furthermore impression of passport-saml not been under active maintenance was based on the fact that its version had not been upgraded even though it must have been visible at some vulnerability reporting tools (due vulnerabilities at transitive dependencies and due one of its own vulnerability (CVE))
3. passport-saml certificate check issue has been more or less public since 2016 (The certificate of the IdP MUST be required node-saml/passport-saml#180 which received only partial fix and later on - at 2021 - a fix which made cert truly required option)
4. vulnerability is quite obvious (easy to spot just by code review or by reading configuration info which said IdP cert is optional which should raise immediately some eyebrows).

So I decided to give this issue one more "push" but this time publicly so that at least wiki.js community becomes somehow aware of it.

[NGPixel]

Your email must have ended in spam. I'll investigate to ensure mail is still being received correctly at that address.

v2.5.280 is currently being built and published, with the latest passport-saml dependency. The text for the cert parameter as being modified to remove the (Optional) mention.

[NGPixel]

Dependency updated in 2cb7b9f

[srd90]

There has been number of breaking changes in passport-saml between 1.3.5 and 3.2.1.

I do not remember all of them (I'm not maintainer of that project...just checking it out every now and then) but from what I remember for example:

• privateCert was deprecated (replaced with privateKey) at some 2.x and removed at 3.0.0 see release notes of passport-saml 3.0.0 especially this change https://github.com/node-saml/passport-saml/pull/569/files

So at the moment current requarks/wiki/server/modules/authentication/saml/definition.yml @ 2cb7b9f
is not in sync with passport-saml 3.2.1's options listed at: node-saml/passport-saml/README.md @ 6ba76ba aka 3.2.1