feat: select algorithm to use for signing SAML requests and responses

.changeset/swift-waves-argue.md

@@ -0,0 +1,7 @@
+---
+'@rocket.chat/core-typings': minor
+'@rocket.chat/i18n': minor
+'@rocket.chat/meteor': minor
+---
+
+Adds a new setting to select which algorithm to use when signing SAML requests and responses

apps/meteor/app/meteor-accounts-saml/server/definition/IServiceProviderOptions.ts

@@ -1,3 +1,5 @@
+import type { SAMLSignatureAlgorithm } from '@rocket.chat/core-typings';
+
 export interface IServiceProviderOptions {
 	provider: string;
 	entryPoint: string;
@@ -6,6 +8,7 @@ export interface IServiceProviderOptions {
 	cert: string;
 	privateCert: string;
 	privateKey: string;
+	signatureAlgorithm: SAMLSignatureAlgorithm;
 	customAuthnContext: string;
 	authnContextComparison: string;
 	defaultUserRole: string;

apps/meteor/app/meteor-accounts-saml/server/lib/ServiceProvider.ts

@@ -18,6 +18,13 @@ import { LogoutRequestParser } from './parsers/LogoutRequest';
 import { LogoutResponseParser } from './parsers/LogoutResponse';
 import { ResponseParser } from './parsers/Response';
 
+const signatureAlgorithms = {
+	'RSA-SHA1': 'http://www.w3.org/2000/09/xmldsig#rsa-sha1',
+	'RSA-SHA256': 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256',
+	'RSA-SHA384': 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha384',
+	'RSA-SHA512': 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha512',
+} as const;
+
 export class SAMLServiceProvider {
 	serviceProviderOptions: IServiceProviderOptions;
 
@@ -29,10 +36,35 @@ export class SAMLServiceProvider {
 		this.serviceProviderOptions = serviceProviderOptions;
 	}
 
-	private signRequest(xml: string): string {
-		const signer = crypto.createSign('RSA-SHA1');
+	private getSignatureAlgorithm(): keyof typeof signatureAlgorithms {
+		const algorithm = `RSA-${this.serviceProviderOptions.signatureAlgorithm}`;
+
+		if (algorithm in signatureAlgorithms) {
+			return algorithm as keyof typeof signatureAlgorithms;
+		}
+
+		return 'RSA-SHA256';
+	}
+
+	private maybeSignRequest(samlObject: Record<string, any>): Record<string, any> {
+		if (!this.serviceProviderOptions.privateKey) {
+			return samlObject;
+		}
+
+		const algorithm = this.getSignatureAlgorithm();
+
+		const alg = signatureAlgorithms[algorithm];
+		const xml = querystring.stringify({ ...samlObject, SigAlg: alg });
+
+		const signer = crypto.createSign(algorithm);
 		signer.update(xml);
-		return signer.sign(this.serviceProviderOptions.privateKey, 'base64');
+		const signature = signer.sign(this.serviceProviderOptions.privateKey, 'base64');
+
+		return {
+			...samlObject,
+			SigAlg: alg,
+			Signature: signature,
+		};
 	}
 
 	public generateAuthorizeRequest(credentialToken: string): string {
@@ -78,15 +110,10 @@ export class SAMLServiceProvider {
 				// TBD. We should really include a proper RelayState here
 				const relayState = Meteor.absoluteUrl();
 
-				const samlResponse: Record<string, any> = {
+				const samlResponse = this.maybeSignRequest({
 					SAMLResponse: base64,
 					RelayState: relayState,
-				};
-
-				if (this.serviceProviderOptions.privateCert) {
-					samlResponse.SigAlg = 'http://www.w3.org/2000/09/xmldsig#rsa-sha1';
-					samlResponse.Signature = this.signRequest(querystring.stringify(samlResponse));
-				}
+				});
 
 				target += querystring.stringify(samlResponse);
 
@@ -127,15 +154,10 @@ export class SAMLServiceProvider {
 				relayState = this.serviceProviderOptions.provider;
 			}
 
-			const samlRequest: Record<string, any> = {
+			const samlRequest = this.maybeSignRequest({
 				SAMLRequest: base64,
 				RelayState: relayState,
-			};
-
-			if (this.serviceProviderOptions.privateCert) {
-				samlRequest.SigAlg = 'http://www.w3.org/2000/09/xmldsig#rsa-sha1';
-				samlRequest.Signature = this.signRequest(querystring.stringify(samlRequest));
-			}
+			});
 
 			target += querystring.stringify(samlRequest);
 

apps/meteor/app/meteor-accounts-saml/server/lib/settings.ts

@@ -46,6 +46,7 @@ const getSamlConfigs = function (service: string): SAMLConfiguration {
 			publicCert: settings.get(`${service}_public_cert`),
 			// 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: SAMLUtils.normalizeCert((settings.get(`${service}_cert`) as string) || ''),
+			algorithm: settings.get(`${service}_signature_algorithm`) || 'SHA1',
 		},
 		signatureValidationType: settings.get(`${service}_signature_validation_type`),
 		userDataFieldMap: settings.get(`${service}_user_data_fieldmap`),
@@ -89,6 +90,7 @@ const configureSamlService = function (samlConfigs: Record<string, any>): IServi
 		cert: samlConfigs.secret.cert,
 		privateCert,
 		privateKey,
+		signatureAlgorithm: samlConfigs.secret.algorithm,
 		customAuthnContext: samlConfigs.customAuthnContext,
 		authnContextComparison: samlConfigs.authnContextComparison,
 		defaultUserRole: samlConfigs.defaultUserRole,
@@ -213,6 +215,18 @@ export const addSettings = async function (name: string): Promise<void> {
 						i18nLabel: 'SAML_Custom_Private_Key',
 						secret: true,
 					});
+					await this.add(`SAML_Custom_${name}_signature_algorithm`, 'SHA1', {
+						type: 'select',
+						values: [
+							{ key: 'SHA1', i18nLabel: 'SHA1' },
+							{ key: 'SHA256', i18nLabel: 'A256' },
+							{ key: 'SHA384', i18nLabel: 'A384' },
+							{ key: 'SHA512', i18nLabel: 'A512' },
+						],
+						i18nLabel: 'SAML_Custom_Signature_Algorithm',
+						i18nDescription: 'SAML_Custom_Signature_Algorithm_description',
+						secret: true,
+					});
 				});
 			},
 		);

apps/meteor/tests/e2e/containers/saml/config/simplesamlphp/config.php

@@ -107,5 +107,5 @@
 	'metadata.sign.privatekey_pass' => null,
 	'metadata.sign.certificate' => null,
 	'proxy' => null,
-	'trusted.url.domains' => array(),
+	'trusted.url.domains' => array('localhost:3000'),
 );

apps/meteor/tests/e2e/containers/saml/config/simplesamlphp/saml20-sp-remote.php

@@ -21,4 +21,11 @@
 		),
 	),
 	'NameIDFormat' => 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress',
+	// Other settings that may be useful for testing, but are not compatible with the existing e2e tests
+	// 'privatekey' => 'server.pem',
+	// 'certificate' => 'server.crt',
+	// 'redirect.sign' => true,
+	// 'redirect.validate' => true,
+	// 'validate.logout' => true,
+	// 'assertion.encryption' => true,
 );

apps/meteor/tests/unit/app/meteor-accounts-saml/data.ts

@@ -1,11 +1,14 @@
-export const serviceProviderOptions = {
+import type { IServiceProviderOptions } from '../../../../app/meteor-accounts-saml/server/definition/IServiceProviderOptions';
+
+export const serviceProviderOptions: IServiceProviderOptions = {
 	provider: '[test-provider]',
 	entryPoint: '[entry-point]',
 	idpSLORedirectURL: '[idpSLORedirectURL]',
 	issuer: '[issuer]',
 	cert: '',
 	privateCert: '',
 	privateKey: '',
+	signatureAlgorithm: 'SHA256',
 	customAuthnContext: 'Password',
 	authnContextComparison: 'Whatever',
 	defaultUserRole: 'user',

packages/core-typings/src/ILoginServiceConfiguration.ts

@@ -62,6 +62,8 @@ export type CASConfiguration = {
 	autoclose: boolean;
 };
 
+export type SAMLSignatureAlgorithm = 'SHA1' | 'SHA256' | 'SHA384' | 'SHA512';
+
 export type SAMLConfiguration = {
 	buttonLabelText: string;
 	buttonLabelColor: string;
@@ -84,6 +86,7 @@ export type SAMLConfiguration = {
 		privateKey: string;
 		publicCert: string;
 		cert: string;
+		algorithm: SAMLSignatureAlgorithm;
 	};
 	signatureValidationType: 'All' | 'Response' | 'Assertion' | 'Either';
 	userDataFieldMap: string;

packages/i18n/src/locales/en.i18n.json

@@ -4399,6 +4399,8 @@
   "SAML_Custom_Private_Key": "Private Key Contents",
   "SAML_Custom_Provider": "Custom Provider",
   "SAML_Custom_Public_Cert": "Public Cert Contents",
+  "SAML_Custom_Signature_Algorithm": "Signature Algorithm",
+  "SAML_Custom_Signature_Algorithm_description": "What Algorithm to use when signing requests and responses from Rocket.Chat",
   "SAML_Custom_Username_Field": "Username field name",
   "SAML_Custom_Username_Normalize": "Normalize username",
   "SAML_Custom_Username_Normalize_Lowercase": "To Lowercase",