From b56d7b8eaad9c8cc222f274bbb0d3d6fe1e8a750 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paola=20Estefan=C3=ADa=20de=20Campos?= <84341268+Paola3stefania@users.noreply.github.com> Date: Mon, 15 Dec 2025 19:40:08 -0300 Subject: [PATCH] feat(saml): validate SAML crypto algorithms during initial phase (#6785) --- docs/content/docs/plugins/sso.mdx | 45 ++++ packages/sso/src/index.ts | 9 + packages/sso/src/routes/sso.ts | 6 +- packages/sso/src/saml/algorithms.test.ts | 205 ++++++++++++++++++ packages/sso/src/saml/algorithms.ts | 259 +++++++++++++++++++++++ packages/sso/src/saml/index.ts | 9 + packages/sso/src/types.ts | 15 ++ 7 files changed, 547 insertions(+), 1 deletion(-) create mode 100644 packages/sso/src/saml/algorithms.test.ts create mode 100644 packages/sso/src/saml/algorithms.ts create mode 100644 packages/sso/src/saml/index.ts diff --git a/docs/content/docs/plugins/sso.mdx b/docs/content/docs/plugins/sso.mdx index e261a0275d..b76a2733c8 100644 --- a/docs/content/docs/plugins/sso.mdx +++ b/docs/content/docs/plugins/sso.mdx @@ -1066,6 +1066,39 @@ sso({ - **"SAML assertion has expired"** — Current time is after the `NotOnOrAfter` timestamp (plus clock skew) - **"SAML assertion missing required timestamp conditions"** — Assertion has no timestamps and `requireTimestamps` is enabled +### Algorithm Validation + +Better Auth validates SAML cryptographic algorithms and warns about deprecated ones (SHA-1, RSA 1.5, 3DES) by default. + +```ts title="auth.ts" +sso({ + saml: { + algorithms: { + // "warn" (default) | "reject" | "allow" + onDeprecated: "warn", + }, + }, +}) +``` + +| Value | Behavior | +|-------|----------| +| `"warn"` | Log warning, allow authentication (default) | +| `"reject"` | Throw error, block authentication | +| `"allow"` | Silent, no validation | + +For strict security (production): + +```ts title="auth.ts" +sso({ + saml: { + algorithms: { + onDeprecated: "reject", + }, + }, +}) +``` + ## Domain verification Domain verification allows your application to automatically trust a new SSO provider @@ -1358,6 +1391,18 @@ If you want to allow account linking for specific trusted providers, enable the type: "boolean", default: false, }, + algorithms: { + description: "Algorithm validation options.", + type: "object", + properties: { + onDeprecated: { + description: "Behavior for deprecated algorithms (SHA-1, RSA 1.5, 3DES).", + type: "string", + enum: ["reject", "warn", "allow"], + default: "warn", + }, + }, + }, }, }, modelName: { diff --git a/packages/sso/src/index.ts b/packages/sso/src/index.ts index 4e80aea8ff..2497fc4a4a 100644 --- a/packages/sso/src/index.ts +++ b/packages/sso/src/index.ts @@ -29,6 +29,15 @@ export { validateSAMLTimestamp, } from "./routes/sso"; +export { + type AlgorithmValidationOptions, + DataEncryptionAlgorithm, + type DeprecatedAlgorithmBehavior, + DigestAlgorithm, + KeyEncryptionAlgorithm, + SignatureAlgorithm, +} from "./saml"; + import type { OIDCConfig, SAMLConfig, SSOOptions, SSOProvider } from "./types"; export type { SAMLConfig, OIDCConfig, SSOOptions, SSOProvider }; diff --git a/packages/sso/src/routes/sso.ts b/packages/sso/src/routes/sso.ts index b167c4b2af..0542666f65 100644 --- a/packages/sso/src/routes/sso.ts +++ b/packages/sso/src/routes/sso.ts @@ -30,8 +30,8 @@ import { discoverOIDCConfig, mapDiscoveryErrorToAPIError, } from "../oidc"; +import { validateSAMLAlgorithms } from "../saml"; import type { OIDCConfig, SAMLConfig, SSOOptions, SSOProvider } from "../types"; - import { safeJsonParse, validateEmailDomain } from "../utils"; const AUTHN_REQUEST_KEY_PREFIX = "saml-authn-request:"; @@ -1808,6 +1808,8 @@ export const callbackSSOSAML = (options?: SSOOptions) => { const { extract } = parsedResponse!; + validateSAMLAlgorithms(parsedResponse, options?.saml?.algorithms); + validateSAMLTimestamp((extract as any).conditions, { clockSkew: options?.saml?.clockSkew, requireTimestamps: options?.saml?.requireTimestamps, @@ -2246,6 +2248,8 @@ export const acsEndpoint = (options?: SSOOptions) => { const { extract } = parsedResponse!; + validateSAMLAlgorithms(parsedResponse, options?.saml?.algorithms); + validateSAMLTimestamp((extract as any).conditions, { clockSkew: options?.saml?.clockSkew, requireTimestamps: options?.saml?.requireTimestamps, diff --git a/packages/sso/src/saml/algorithms.test.ts b/packages/sso/src/saml/algorithms.test.ts new file mode 100644 index 0000000000..c5bea923d4 --- /dev/null +++ b/packages/sso/src/saml/algorithms.test.ts @@ -0,0 +1,205 @@ +/* cspell:ignore xenc */ +import { afterEach, describe, expect, it, vi } from "vitest"; +import * as alg from "./algorithms"; + +const encryptedAssertionXml = ` + + + + + + + + + + + + +`; + +const deprecatedEncryptionXml = ` + + + + + + + + + + + + +`; + +const plainAssertionXml = ` + + + test + + +`; + +describe("validateSAMLAlgorithms", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + describe("signature validation", () => { + it("should accept secure signature algorithms", () => { + expect(() => + alg.validateSAMLAlgorithms({ + sigAlg: alg.SignatureAlgorithm.RSA_SHA256, + samlContent: plainAssertionXml, + }), + ).not.toThrow(); + }); + + it("should warn by default for deprecated signature algorithms", () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + expect(() => + alg.validateSAMLAlgorithms({ + sigAlg: alg.SignatureAlgorithm.RSA_SHA1, + samlContent: plainAssertionXml, + }), + ).not.toThrow(); + + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining("SAML Security Warning"), + ); + }); + + it("should reject deprecated signature with onDeprecated: reject", () => { + expect(() => + alg.validateSAMLAlgorithms( + { + sigAlg: alg.SignatureAlgorithm.RSA_SHA1, + samlContent: plainAssertionXml, + }, + { onDeprecated: "reject" }, + ), + ).toThrow(/deprecated/i); + }); + + it("should silently allow deprecated with onDeprecated: allow", () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + expect(() => + alg.validateSAMLAlgorithms( + { + sigAlg: alg.SignatureAlgorithm.RSA_SHA1, + samlContent: plainAssertionXml, + }, + { onDeprecated: "allow" }, + ), + ).not.toThrow(); + + expect(warnSpy).not.toHaveBeenCalled(); + }); + + it("should enforce custom signature allow-list", () => { + expect(() => + alg.validateSAMLAlgorithms( + { + sigAlg: alg.SignatureAlgorithm.RSA_SHA256, + samlContent: plainAssertionXml, + }, + { allowedSignatureAlgorithms: [alg.SignatureAlgorithm.RSA_SHA512] }, + ), + ).toThrow(/not in allow-list/i); + }); + + it("should pass null/undefined sigAlg without error", () => { + expect(() => + alg.validateSAMLAlgorithms({ + sigAlg: null, + samlContent: plainAssertionXml, + }), + ).not.toThrow(); + }); + + it("should reject unknown signature algorithms", () => { + expect(() => + alg.validateSAMLAlgorithms({ + sigAlg: "http://example.com/unknown-algo", + samlContent: plainAssertionXml, + }), + ).toThrow(/not recognized/i); + }); + }); + + describe("encryption validation", () => { + it("should accept secure encryption algorithms", () => { + expect(() => + alg.validateSAMLAlgorithms({ + sigAlg: alg.SignatureAlgorithm.RSA_SHA256, + samlContent: encryptedAssertionXml, + }), + ).not.toThrow(); + }); + + it("should warn by default for deprecated encryption algorithms", () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + expect(() => + alg.validateSAMLAlgorithms({ + sigAlg: alg.SignatureAlgorithm.RSA_SHA256, + samlContent: deprecatedEncryptionXml, + }), + ).not.toThrow(); + + expect(warnSpy).toHaveBeenCalled(); + }); + + it("should reject deprecated encryption with onDeprecated: reject", () => { + expect(() => + alg.validateSAMLAlgorithms( + { + sigAlg: alg.SignatureAlgorithm.RSA_SHA256, + samlContent: deprecatedEncryptionXml, + }, + { onDeprecated: "reject" }, + ), + ).toThrow(/deprecated/i); + }); + + it("should skip encryption validation for plain assertions", () => { + expect(() => + alg.validateSAMLAlgorithms({ + sigAlg: alg.SignatureAlgorithm.RSA_SHA256, + samlContent: plainAssertionXml, + }), + ).not.toThrow(); + }); + + it("should handle malformed XML gracefully", () => { + expect(() => + alg.validateSAMLAlgorithms({ + sigAlg: alg.SignatureAlgorithm.RSA_SHA256, + samlContent: "not valid xml", + }), + ).not.toThrow(); + }); + }); +}); + +describe("algorithm constants", () => { + it("should export signature algorithm constants", () => { + expect(alg.SignatureAlgorithm.RSA_SHA256).toBe( + "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256", + ); + expect(alg.SignatureAlgorithm.RSA_SHA1).toBe( + "http://www.w3.org/2000/09/xmldsig#rsa-sha1", + ); + }); + + it("should export encryption algorithm constants", () => { + expect(alg.KeyEncryptionAlgorithm.RSA_OAEP).toBe( + "http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p", + ); + expect(alg.DataEncryptionAlgorithm.AES_256_GCM).toBe( + "http://www.w3.org/2009/xmlenc11#aes256-gcm", + ); + }); +}); diff --git a/packages/sso/src/saml/algorithms.ts b/packages/sso/src/saml/algorithms.ts new file mode 100644 index 0000000000..4aae3105d7 --- /dev/null +++ b/packages/sso/src/saml/algorithms.ts @@ -0,0 +1,259 @@ +import { APIError } from "better-auth/api"; +import { XMLParser } from "fast-xml-parser"; + +export const SignatureAlgorithm = { + 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", + ECDSA_SHA256: "http://www.w3.org/2001/04/xmldsig-more#ecdsa-sha256", + ECDSA_SHA384: "http://www.w3.org/2001/04/xmldsig-more#ecdsa-sha384", + ECDSA_SHA512: "http://www.w3.org/2001/04/xmldsig-more#ecdsa-sha512", +} as const; + +export const DigestAlgorithm = { + SHA1: "http://www.w3.org/2000/09/xmldsig#sha1", + SHA256: "http://www.w3.org/2001/04/xmlenc#sha256", + SHA384: "http://www.w3.org/2001/04/xmldsig-more#sha384", + SHA512: "http://www.w3.org/2001/04/xmlenc#sha512", +} as const; + +export const KeyEncryptionAlgorithm = { + RSA_1_5: "http://www.w3.org/2001/04/xmlenc#rsa-1_5", + RSA_OAEP: "http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p", + RSA_OAEP_SHA256: "http://www.w3.org/2009/xmlenc11#rsa-oaep", +} as const; + +export const DataEncryptionAlgorithm = { + TRIPLEDES_CBC: "http://www.w3.org/2001/04/xmlenc#tripledes-cbc", + AES_128_CBC: "http://www.w3.org/2001/04/xmlenc#aes128-cbc", + AES_192_CBC: "http://www.w3.org/2001/04/xmlenc#aes192-cbc", + AES_256_CBC: "http://www.w3.org/2001/04/xmlenc#aes256-cbc", + AES_128_GCM: "http://www.w3.org/2009/xmlenc11#aes128-gcm", + AES_192_GCM: "http://www.w3.org/2009/xmlenc11#aes192-gcm", + AES_256_GCM: "http://www.w3.org/2009/xmlenc11#aes256-gcm", +} as const; + +const DEPRECATED_SIGNATURE_ALGORITHMS: readonly string[] = [ + SignatureAlgorithm.RSA_SHA1, +]; + +const DEPRECATED_KEY_ENCRYPTION_ALGORITHMS: readonly string[] = [ + KeyEncryptionAlgorithm.RSA_1_5, +]; + +const DEPRECATED_DATA_ENCRYPTION_ALGORITHMS: readonly string[] = [ + DataEncryptionAlgorithm.TRIPLEDES_CBC, +]; + +const SECURE_SIGNATURE_ALGORITHMS: readonly string[] = [ + SignatureAlgorithm.RSA_SHA256, + SignatureAlgorithm.RSA_SHA384, + SignatureAlgorithm.RSA_SHA512, + SignatureAlgorithm.ECDSA_SHA256, + SignatureAlgorithm.ECDSA_SHA384, + SignatureAlgorithm.ECDSA_SHA512, +]; + +export type DeprecatedAlgorithmBehavior = "reject" | "warn" | "allow"; + +export interface AlgorithmValidationOptions { + onDeprecated?: DeprecatedAlgorithmBehavior; + allowedSignatureAlgorithms?: string[]; + allowedDigestAlgorithms?: string[]; + allowedKeyEncryptionAlgorithms?: string[]; + allowedDataEncryptionAlgorithms?: string[]; +} + +const xmlParser = new XMLParser({ + ignoreAttributes: false, + attributeNamePrefix: "@_", + removeNSPrefix: true, +}); + +function findNode(obj: unknown, nodeName: string): unknown { + if (!obj || typeof obj !== "object") return null; + + const record = obj as Record; + + if (nodeName in record) { + return record[nodeName]; + } + + for (const value of Object.values(record)) { + if (Array.isArray(value)) { + for (const item of value) { + const found = findNode(item, nodeName); + if (found) return found; + } + } else if (typeof value === "object" && value !== null) { + const found = findNode(value, nodeName); + if (found) return found; + } + } + + return null; +} + +function extractEncryptionAlgorithms(xml: string): { + keyEncryption: string | null; + dataEncryption: string | null; +} { + try { + const parsed = xmlParser.parse(xml); + + const encryptedKey = findNode(parsed, "EncryptedKey") as Record< + string, + unknown + > | null; + const keyEncMethod = encryptedKey?.EncryptionMethod as Record< + string, + unknown + > | null; + const keyAlg = keyEncMethod?.["@_Algorithm"] as string | undefined; + + const encryptedData = findNode(parsed, "EncryptedData") as Record< + string, + unknown + > | null; + const dataEncMethod = encryptedData?.EncryptionMethod as Record< + string, + unknown + > | null; + const dataAlg = dataEncMethod?.["@_Algorithm"] as string | undefined; + + return { + keyEncryption: keyAlg || null, + dataEncryption: dataAlg || null, + }; + } catch { + return { + keyEncryption: null, + dataEncryption: null, + }; + } +} + +function hasEncryptedAssertion(xml: string): boolean { + try { + const parsed = xmlParser.parse(xml); + return findNode(parsed, "EncryptedAssertion") !== null; + } catch { + return false; + } +} + +function handleDeprecatedAlgorithm( + message: string, + behavior: DeprecatedAlgorithmBehavior, + errorCode: string, +): void { + switch (behavior) { + case "reject": + throw new APIError("BAD_REQUEST", { + message, + code: errorCode, + }); + case "warn": + console.warn(`[SAML Security Warning] ${message}`); + break; + case "allow": + break; + } +} + +function validateSignatureAlgorithm( + algorithm: string | null | undefined, + options: AlgorithmValidationOptions = {}, +): void { + if (!algorithm) { + return; + } + + const { onDeprecated = "warn", allowedSignatureAlgorithms } = options; + + if (allowedSignatureAlgorithms) { + if (!allowedSignatureAlgorithms.includes(algorithm)) { + throw new APIError("BAD_REQUEST", { + message: `SAML signature algorithm not in allow-list: ${algorithm}`, + code: "SAML_ALGORITHM_NOT_ALLOWED", + }); + } + return; + } + + if (DEPRECATED_SIGNATURE_ALGORITHMS.includes(algorithm)) { + handleDeprecatedAlgorithm( + `SAML response uses deprecated signature algorithm: ${algorithm}. Please configure your IdP to use SHA-256 or stronger.`, + onDeprecated, + "SAML_DEPRECATED_ALGORITHM", + ); + return; + } + + if (!SECURE_SIGNATURE_ALGORITHMS.includes(algorithm)) { + throw new APIError("BAD_REQUEST", { + message: `SAML signature algorithm not recognized: ${algorithm}`, + code: "SAML_UNKNOWN_ALGORITHM", + }); + } +} + +function validateEncryptionAlgorithms( + algorithms: { keyEncryption: string | null; dataEncryption: string | null }, + options: AlgorithmValidationOptions = {}, +): void { + const { + onDeprecated = "warn", + allowedKeyEncryptionAlgorithms, + allowedDataEncryptionAlgorithms, + } = options; + + const { keyEncryption, dataEncryption } = algorithms; + + if (keyEncryption) { + if (allowedKeyEncryptionAlgorithms) { + if (!allowedKeyEncryptionAlgorithms.includes(keyEncryption)) { + throw new APIError("BAD_REQUEST", { + message: `SAML key encryption algorithm not in allow-list: ${keyEncryption}`, + code: "SAML_ALGORITHM_NOT_ALLOWED", + }); + } + } else if (DEPRECATED_KEY_ENCRYPTION_ALGORITHMS.includes(keyEncryption)) { + handleDeprecatedAlgorithm( + `SAML response uses deprecated key encryption algorithm: ${keyEncryption}. Please configure your IdP to use RSA-OAEP.`, + onDeprecated, + "SAML_DEPRECATED_ALGORITHM", + ); + } + } + + if (dataEncryption) { + if (allowedDataEncryptionAlgorithms) { + if (!allowedDataEncryptionAlgorithms.includes(dataEncryption)) { + throw new APIError("BAD_REQUEST", { + message: `SAML data encryption algorithm not in allow-list: ${dataEncryption}`, + code: "SAML_ALGORITHM_NOT_ALLOWED", + }); + } + } else if (DEPRECATED_DATA_ENCRYPTION_ALGORITHMS.includes(dataEncryption)) { + handleDeprecatedAlgorithm( + `SAML response uses deprecated data encryption algorithm: ${dataEncryption}. Please configure your IdP to use AES-GCM.`, + onDeprecated, + "SAML_DEPRECATED_ALGORITHM", + ); + } + } +} + +export function validateSAMLAlgorithms( + response: { sigAlg?: string | null; samlContent: string }, + options?: AlgorithmValidationOptions, +): void { + validateSignatureAlgorithm(response.sigAlg, options); + + if (hasEncryptedAssertion(response.samlContent)) { + const encAlgs = extractEncryptionAlgorithms(response.samlContent); + validateEncryptionAlgorithms(encAlgs, options); + } +} diff --git a/packages/sso/src/saml/index.ts b/packages/sso/src/saml/index.ts new file mode 100644 index 0000000000..757dd3c622 --- /dev/null +++ b/packages/sso/src/saml/index.ts @@ -0,0 +1,9 @@ +export { + type AlgorithmValidationOptions, + DataEncryptionAlgorithm, + type DeprecatedAlgorithmBehavior, + DigestAlgorithm, + KeyEncryptionAlgorithm, + SignatureAlgorithm, + validateSAMLAlgorithms, +} from "./algorithms"; diff --git a/packages/sso/src/types.ts b/packages/sso/src/types.ts index ffb067ba32..eddb752126 100644 --- a/packages/sso/src/types.ts +++ b/packages/sso/src/types.ts @@ -1,5 +1,6 @@ import type { OAuth2Tokens, User } from "better-auth"; import type { AuthnRequestStore } from "./authn-request-store"; +import type { AlgorithmValidationOptions } from "./saml/algorithms"; export interface OIDCMapping { id?: string | undefined; @@ -339,5 +340,19 @@ export interface SSOOptions { * @default false */ requireTimestamps?: boolean; + /** + * Algorithm validation options for SAML responses. + * + * Controls behavior when deprecated algorithms (SHA-1, RSA1_5, 3DES) + * are detected in SAML responses. + * + * @example + * ```ts + * algorithms: { + * onDeprecated: "reject" // Reject deprecated algorithms + * } + * ``` + */ + algorithms?: AlgorithmValidationOptions; }; }