[FIX] SAML assertion signature enforcement (#17278)

app/meteor-accounts-saml/server/saml_rocketchat.js

@@ -62,6 +62,19 @@ Meteor.methods({
 			multiline: true,
 			i18nLabel: 'SAML_Custom_Public_Cert',
 		});
+		settings.add(`SAML_Custom_${ name }_signature_validation_type`, 'All', {
+			type: 'select',
+			values: [
+				{ key: 'Response', i18nLabel: 'SAML_Custom_signature_validation_response' },
+				{ key: 'Assertion', i18nLabel: 'SAML_Custom_signature_validation_assertion' },
+				{ key: 'Either', i18nLabel: 'SAML_Custom_signature_validation_either' },
+				{ key: 'All', i18nLabel: 'SAML_Custom_signature_validation_all' },
+			],
+			group: 'SAML',
+			section: name,
+			i18nLabel: 'SAML_Custom_signature_validation_type',
+			i18nDescription: 'SAML_Custom_signature_validation_type_description',
+		});
 		settings.add(`SAML_Custom_${ name }_private_key`, '', {
 			type: 'string',
 			group: 'SAML',
@@ -238,6 +251,7 @@ const getSamlConfigs = function(service) {
 			// People often overlook the instruction to remove the header and footer of the certificate on this specific setting, so let's do it for them.
 			cert: normalizeCert(settings.get(`${ service.key }_cert`)),
 		},
+		signatureValidationType: settings.get(`${ service.key }_signature_validation_type`),
 		userDataFieldMap: settings.get(`${ service.key }_user_data_fieldmap`),
 		allowedClockDrift: settings.get(`${ service.key }_allowed_clock_drift`),
 	};
@@ -290,6 +304,7 @@ const configureSamlService = function(samlConfigs) {
 		roleAttributeName: samlConfigs.roleAttributeName,
 		roleAttributeSync: samlConfigs.roleAttributeSync,
 		allowedClockDrift: samlConfigs.allowedClockDrift,
+		signatureValidationType: samlConfigs.signatureValidationType,
 	};
 };
 

app/meteor-accounts-saml/server/saml_utils.js

@@ -347,6 +347,10 @@ SAML.prototype.validateSignatureChildren = function(xml, cert, parent) {
 		signature = sign;
 	}
 
+	if (!signature) {
+		return false;
+	}
+
 	return this.validateSignature(xml, cert, signature);
 };
 
@@ -564,19 +568,43 @@ SAML.prototype.verifySignatures = function(response, assertion, xml) {
 		return;
 	}
 
-	debugLog('Verify Document Signature');
-	if (!this.validateResponseSignature(xml, this.options.cert, response)) {
-		debugLog('Document Signature WRONG');
-		throw new Error('Invalid Signature');
+	const signatureType = this.options.signatureValidationType;
+
+	const checkEither = signatureType === 'Either';
+	const checkResponse = signatureType === 'Response' || signatureType === 'All' || checkEither;
+	const checkAssertion = signatureType === 'Assertion' || signatureType === 'All' || checkEither;
+	let anyValidSignature = false;
+
+	if (checkResponse) {
+		debugLog('Verify Document Signature');
+		if (!this.validateResponseSignature(xml, this.options.cert, response)) {
+			if (!checkEither) {
+				debugLog('Document Signature WRONG');
+				throw new Error('Invalid Signature');
+			}
+		} else {
+			anyValidSignature = true;
+		}
+		debugLog('Document Signature OK');
+	}
+
+	if (checkAssertion) {
+		debugLog('Verify Assertion Signature');
+		if (!this.validateAssertionSignature(xml, this.options.cert, assertion)) {
+			if (!checkEither) {
+				debugLog('Assertion Signature WRONG');
+				throw new Error('Invalid Assertion signature');
+			}
+		} else {
+			anyValidSignature = true;
+		}
+		debugLog('Assertion Signature OK');
 	}
-	debugLog('Document Signature OK');
 
-	debugLog('Verify Assertion Signature');
-	if (!this.validateAssertionSignature(xml, this.options.cert, assertion)) {
-		debugLog('Assertion Signature WRONG');
-		throw new Error('Invalid Assertion signature');
+	if (checkEither && !anyValidSignature) {
+		debugLog('No Valid Signature');
+		throw new Error('No valid SAML Signature found');
 	}
-	debugLog('Assertion Signature OK');
 };
 
 SAML.prototype.getSubject = function(assertion) {

packages/rocketchat-i18n/i18n/en.i18n.json

@@ -2930,6 +2930,12 @@
   "SAML_Custom_Private_Key": "Private Key Contents",
   "SAML_Custom_Provider": "Custom Provider",
   "SAML_Custom_EMail_Field": "E-Mail field name",
+  "SAML_Custom_signature_validation_all": "Validate All Signatures",
+  "SAML_Custom_signature_validation_assertion": "Validate Assertion Signature",
+  "SAML_Custom_signature_validation_either": "Validate Either Signature",
+  "SAML_Custom_signature_validation_response": "Validate Response Signature",
+  "SAML_Custom_signature_validation_type": "Signature Validation Type",
+  "SAML_Custom_signature_validation_type_description": "This setting will be ignored if not Custom Certificate is provided.",
   "SAML_Custom_user_data_fieldmap": "User Data Field Map",
   "SAML_Custom_user_data_fieldmap_description": "Configure how user account fields (like email) are populated from a record in SAML (once found). <br/>As an example, `{\"cn\":\"name\", \"mail\":\"email\"}` will choose a person's human readable name from the cn attribute, and their email from the mail attribute.<br/>Available fields in Rocket.Chat: `name`, `email` and `username`, everything else will be saved as `customFields`.<br/>You can also use a regex to get the field value, like this: `{\"NameID\": { \"field\": \"username\", \"regex\": \"(.*)@.+$\"}, \"email\": \"email\"}`",
   "SAML_Custom_Username_Field": "Username field name",

server/startup/migrations/index.js

@@ -181,4 +181,5 @@ import './v180';
 import './v181';
 import './v182';
 import './v183';
+import './v184';
 import './xrun';

server/startup/migrations/v184.js

@@ -0,0 +1,16 @@
+import { Migrations } from '../../../app/migrations';
+import { Settings } from '../../../app/models/server';
+
+Migrations.add({
+	version: 184,
+	up() {
+		// Set SAML signature validation type to 'Either'
+		Settings.upsert({
+			_id: 'SAML_Custom_Default_signature_validation_type',
+		}, {
+			$set: {
+				value: 'Either',
+			},
+		});
+	},
+});