From 9bc55ea7f0b82f66ae435de156083a7515c4cc1b Mon Sep 17 00:00:00 2001 From: Kiran K Date: Thu, 3 Mar 2022 02:36:04 +0530 Subject: [PATCH] Validate AuthnRequest signature (#11) * Validate AuthnRequest signature skelton * Code refactor: Move the base64decode to common method * wip * Add signature validation * Read the keys from config * Lock dep version Co-authored-by: Deepak Prabhakara --- lib/env.ts | 6 ++ package-lock.json | 90 ++++++++++++++++++++++++++--- package.json | 2 + pages/api/saml/auth.ts | 14 +---- pages/api/saml/metadata/download.ts | 8 +-- pages/api/saml/sso.ts | 14 +++-- pages/index.tsx | 3 +- utils/certificate.ts | 15 ++++- utils/idp.ts | 2 +- utils/request.ts | 60 +++++++++++++++++-- 10 files changed, 175 insertions(+), 39 deletions(-) diff --git a/lib/env.ts b/lib/env.ts index ffdd2f0..16c6610 100644 --- a/lib/env.ts +++ b/lib/env.ts @@ -1,11 +1,17 @@ +import { fetchPrivateKey, fetchPublicKey } from 'utils'; + const appUrl = process.env.APP_URL || 'http://localhost:4000'; const entityId = process.env.ENTITY_ID || 'https://saml.example.com/entityid'; const ssoUrl = `${appUrl}/api/saml/sso`; +const privateKey = fetchPrivateKey(); +const publicKey = fetchPublicKey(); const config = { appUrl, entityId, ssoUrl, + privateKey, + publicKey, }; export default config; diff --git a/package-lock.json b/package-lock.json index 56958c1..d1a0de4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6,12 +6,14 @@ "": { "name": "fake", "dependencies": { + "@boxyhq/saml20": "0.2.0", + "@xmldom/xmldom": "^0.8.1", "next": "12.1.0", "node-forge": "1.2.1", "react": "17.0.2", "react-dom": "17.0.2", "react-gtm-module": "2.0.11", - "xml-crypto": "2.1.3", + "xml-crypto": "^2.1.3", "xml2js": "0.4.23", "xmlbuilder": "15.1.1" }, @@ -92,6 +94,26 @@ "node": ">=6.9.0" } }, + "node_modules/@boxyhq/saml20": { + "version": "0.2.0", + "resolved": "https://registry.npmjs.org/@boxyhq/saml20/-/saml20-0.2.0.tgz", + "integrity": "sha512-octyllYuCD//N8DagXB5BMpDQ4B1aA6wTDC0XI72z2E+GJMwPzwYLSvzwKpSetsaXRUYPiIexxqyPYRqA+Uqnw==", + "dependencies": { + "@xmldom/xmldom": "0.7.5", + "lodash": "^4.17.21", + "thumbprint": "^0.0.1", + "xml-crypto": "^2.1.3", + "xml2js": "^0.4.23" + } + }, + "node_modules/@boxyhq/saml20/node_modules/@xmldom/xmldom": { + "version": "0.7.5", + "resolved": "https://registry.npmjs.org/@xmldom/xmldom/-/xmldom-0.7.5.tgz", + "integrity": "sha512-V3BIhmY36fXZ1OtVcI9W+FxQqxVLsPKcNjWigIaa81dLC9IolJl5Mt4Cvhmr0flUnjSpTdrbMTSbXqYqV5dT6A==", + "engines": { + "node": ">=10.0.0" + } + }, "node_modules/@eslint/eslintrc": { "version": "1.0.5", "resolved": "https://registry.npmjs.org/@eslint/eslintrc/-/eslintrc-1.0.5.tgz", @@ -672,9 +694,9 @@ } }, "node_modules/@xmldom/xmldom": { - "version": "0.7.5", - "resolved": "https://registry.npmjs.org/@xmldom/xmldom/-/xmldom-0.7.5.tgz", - "integrity": "sha512-V3BIhmY36fXZ1OtVcI9W+FxQqxVLsPKcNjWigIaa81dLC9IolJl5Mt4Cvhmr0flUnjSpTdrbMTSbXqYqV5dT6A==", + "version": "0.8.1", + "resolved": "https://registry.npmjs.org/@xmldom/xmldom/-/xmldom-0.8.1.tgz", + "integrity": "sha512-4wOae+5N2RZ+CZXd9ZKwkaDi55IxrSTOjHpxTvQQ4fomtOJmqVxbmICA9jE1jvnqNhpfgz8cnfFagG86wV/xLQ==", "engines": { "node": ">=10.0.0" } @@ -2660,6 +2682,11 @@ "integrity": "sha512-7ylylesZQ/PV29jhEDl3Ufjo6ZX7gCqJr5F7PKrqc93v7fzSymt1BpwEU8nAUXs8qzzvqhbjhK5QZg6Mt/HkBg==", "dev": true }, + "node_modules/lodash": { + "version": "4.17.21", + "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz", + "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==" + }, "node_modules/lodash.merge": { "version": "4.6.2", "resolved": "https://registry.npmjs.org/lodash.merge/-/lodash.merge-4.6.2.tgz", @@ -3738,6 +3765,11 @@ "integrity": "sha1-f17oI66AUgfACvLfSoTsP8+lcLQ=", "dev": true }, + "node_modules/thumbprint": { + "version": "0.0.1", + "resolved": "https://registry.npmjs.org/thumbprint/-/thumbprint-0.0.1.tgz", + "integrity": "sha1-VehvmpsU77RbFcA5ZF1HtiJrt3c=" + }, "node_modules/to-regex-range": { "version": "5.0.1", "resolved": "https://registry.npmjs.org/to-regex-range/-/to-regex-range-5.0.1.tgz", @@ -3925,6 +3957,14 @@ "node": ">=0.4.0" } }, + "node_modules/xml-crypto/node_modules/@xmldom/xmldom": { + "version": "0.7.5", + "resolved": "https://registry.npmjs.org/@xmldom/xmldom/-/xmldom-0.7.5.tgz", + "integrity": "sha512-V3BIhmY36fXZ1OtVcI9W+FxQqxVLsPKcNjWigIaa81dLC9IolJl5Mt4Cvhmr0flUnjSpTdrbMTSbXqYqV5dT6A==", + "engines": { + "node": ">=10.0.0" + } + }, "node_modules/xml2js": { "version": "0.4.23", "resolved": "https://registry.npmjs.org/xml2js/-/xml2js-0.4.23.tgz", @@ -4032,6 +4072,25 @@ "regenerator-runtime": "^0.13.4" } }, + "@boxyhq/saml20": { + "version": "0.2.0", + "resolved": "https://registry.npmjs.org/@boxyhq/saml20/-/saml20-0.2.0.tgz", + "integrity": "sha512-octyllYuCD//N8DagXB5BMpDQ4B1aA6wTDC0XI72z2E+GJMwPzwYLSvzwKpSetsaXRUYPiIexxqyPYRqA+Uqnw==", + "requires": { + "@xmldom/xmldom": "0.7.5", + "lodash": "^4.17.21", + "thumbprint": "^0.0.1", + "xml-crypto": "^2.1.3", + "xml2js": "^0.4.23" + }, + "dependencies": { + "@xmldom/xmldom": { + "version": "0.7.5", + "resolved": "https://registry.npmjs.org/@xmldom/xmldom/-/xmldom-0.7.5.tgz", + "integrity": "sha512-V3BIhmY36fXZ1OtVcI9W+FxQqxVLsPKcNjWigIaa81dLC9IolJl5Mt4Cvhmr0flUnjSpTdrbMTSbXqYqV5dT6A==" + } + } + }, "@eslint/eslintrc": { "version": "1.0.5", "resolved": "https://registry.npmjs.org/@eslint/eslintrc/-/eslintrc-1.0.5.tgz", @@ -4407,9 +4466,9 @@ } }, "@xmldom/xmldom": { - "version": "0.7.5", - "resolved": "https://registry.npmjs.org/@xmldom/xmldom/-/xmldom-0.7.5.tgz", - "integrity": "sha512-V3BIhmY36fXZ1OtVcI9W+FxQqxVLsPKcNjWigIaa81dLC9IolJl5Mt4Cvhmr0flUnjSpTdrbMTSbXqYqV5dT6A==" + "version": "0.8.1", + "resolved": "https://registry.npmjs.org/@xmldom/xmldom/-/xmldom-0.8.1.tgz", + "integrity": "sha512-4wOae+5N2RZ+CZXd9ZKwkaDi55IxrSTOjHpxTvQQ4fomtOJmqVxbmICA9jE1jvnqNhpfgz8cnfFagG86wV/xLQ==" }, "acorn": { "version": "8.5.0", @@ -5889,6 +5948,11 @@ "integrity": "sha512-7ylylesZQ/PV29jhEDl3Ufjo6ZX7gCqJr5F7PKrqc93v7fzSymt1BpwEU8nAUXs8qzzvqhbjhK5QZg6Mt/HkBg==", "dev": true }, + "lodash": { + "version": "4.17.21", + "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz", + "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==" + }, "lodash.merge": { "version": "4.6.2", "resolved": "https://registry.npmjs.org/lodash.merge/-/lodash.merge-4.6.2.tgz", @@ -6612,6 +6676,11 @@ "integrity": "sha1-f17oI66AUgfACvLfSoTsP8+lcLQ=", "dev": true }, + "thumbprint": { + "version": "0.0.1", + "resolved": "https://registry.npmjs.org/thumbprint/-/thumbprint-0.0.1.tgz", + "integrity": "sha1-VehvmpsU77RbFcA5ZF1HtiJrt3c=" + }, "to-regex-range": { "version": "5.0.1", "resolved": "https://registry.npmjs.org/to-regex-range/-/to-regex-range-5.0.1.tgz", @@ -6756,6 +6825,13 @@ "requires": { "@xmldom/xmldom": "^0.7.0", "xpath": "0.0.32" + }, + "dependencies": { + "@xmldom/xmldom": { + "version": "0.7.5", + "resolved": "https://registry.npmjs.org/@xmldom/xmldom/-/xmldom-0.7.5.tgz", + "integrity": "sha512-V3BIhmY36fXZ1OtVcI9W+FxQqxVLsPKcNjWigIaa81dLC9IolJl5Mt4Cvhmr0flUnjSpTdrbMTSbXqYqV5dT6A==" + } } }, "xml2js": { diff --git a/package.json b/package.json index 022880a..a32e200 100644 --- a/package.json +++ b/package.json @@ -8,6 +8,8 @@ "lint": "next lint" }, "dependencies": { + "@boxyhq/saml20": "0.2.0", + "@xmldom/xmldom": "0.8.1", "next": "12.1.0", "node-forge": "1.2.1", "react": "17.0.2", diff --git a/pages/api/saml/auth.ts b/pages/api/saml/auth.ts index 2472b5e..f68e0e4 100644 --- a/pages/api/saml/auth.ts +++ b/pages/api/saml/auth.ts @@ -2,13 +2,7 @@ import { createHash } from 'crypto'; import config from 'lib/env'; import type { NextApiRequest, NextApiResponse } from 'next'; import type { User } from 'types'; -import { - createResponseForm, - createResponseXML, - fetchPrivateKey, - fetchPublicKey, - signResponseXML, -} from 'utils'; +import { createResponseForm, createResponseXML, signResponseXML } from 'utils'; export default async function handler(req: NextApiRequest, res: NextApiResponse) { if (req.method === 'POST') { @@ -35,12 +29,8 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse) user: user, }); - const signingKey = fetchPrivateKey(); - const publicKey = fetchPublicKey(); - const xmlSigned = await signResponseXML(xml, signingKey, publicKey); - + const xmlSigned = await signResponseXML(xml, config.privateKey, config.publicKey); const encodedSamlResponse = Buffer.from(xmlSigned).toString('base64'); - const html = createResponseForm(req.body.relayState, encodedSamlResponse, req.body.acsUrl); res.send(html); diff --git a/pages/api/saml/metadata/download.ts b/pages/api/saml/metadata/download.ts index 25e38a7..0f39b4b 100644 --- a/pages/api/saml/metadata/download.ts +++ b/pages/api/saml/metadata/download.ts @@ -1,9 +1,9 @@ +import config from 'lib/env'; import type { NextApiRequest, NextApiResponse } from 'next'; -import { fetchPublicKey, createIdPMetadataXML } from '../../../../utils'; -import { IdPMetadata } from '../../../../types'; import stream from 'stream'; +import { IdPMetadata } from 'types'; import { promisify } from 'util'; -import config from '../../../../lib/env'; +import { createIdPMetadataXML, stripCertHeaderAndFooter } from 'utils'; const pipeline = promisify(stream.pipeline); @@ -20,7 +20,7 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse< const xml = await createIdPMetadataXML({ idpEntityId: config.entityId, idpSsoUrl: config.ssoUrl, - certificate: fetchPublicKey(), + certificate: stripCertHeaderAndFooter(config.publicKey), }); res.setHeader('Content-type', 'text/xml'); diff --git a/pages/api/saml/sso.ts b/pages/api/saml/sso.ts index 63d545f..ff8103b 100644 --- a/pages/api/saml/sso.ts +++ b/pages/api/saml/sso.ts @@ -1,5 +1,5 @@ import type { NextApiRequest, NextApiResponse } from 'next'; -import { extractSAMLRequestAttributes } from 'utils'; +import { decodeBase64, extractSAMLRequestAttributes, hasValidSignature } from 'utils'; export default async function handler(req: NextApiRequest, res: NextApiResponse) { switch (req.method) { @@ -14,6 +14,7 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse< async function processSAMLRequest(req: NextApiRequest, res: NextApiResponse, isPost: boolean) { let samlRequest, relayState, isDeflated; + if (isPost) { relayState = req.body.RelayState; samlRequest = req.body.SAMLRequest; @@ -23,11 +24,14 @@ async function processSAMLRequest(req: NextApiRequest, res: NextApiResponse, isP samlRequest = req.query.SAMLRequest; isDeflated = true; } + try { - const { id, audience, acsUrl, providerName } = await extractSAMLRequestAttributes( - samlRequest, - isDeflated - ); + const rawRequest = await decodeBase64(samlRequest, isDeflated); + + const { id, audience, acsUrl, providerName, publicKey } = await extractSAMLRequestAttributes(rawRequest); + + await hasValidSignature(rawRequest, publicKey); + const params = new URLSearchParams({ id, audience, acsUrl, providerName, relayState }); res.redirect(302, `/saml/login?${params.toString()}`); diff --git a/pages/index.tsx b/pages/index.tsx index 46ac312..c3000f2 100644 --- a/pages/index.tsx +++ b/pages/index.tsx @@ -3,13 +3,12 @@ import Link from 'next/link'; import React from 'react'; import config from '../lib/env'; import { IdPMetadata } from '../types'; -import { fetchPublicKey } from '../utils'; export const getStaticProps: GetStaticProps = async () => { const metadata: IdPMetadata = { ssoUrl: config.ssoUrl, entityId: config.entityId, - certificate: fetchPublicKey(), + certificate: config.publicKey, }; return { diff --git a/utils/certificate.ts b/utils/certificate.ts index 8f0135c..f6ed0fd 100644 --- a/utils/certificate.ts +++ b/utils/certificate.ts @@ -8,13 +8,13 @@ const fetchPrivateKey = (): string => { return Buffer.from(process.env.PRIVATE_KEY!, 'base64').toString('ascii'); }; -function getPublicKeyPemFromCertificate(x509Certificate: string) { +const getPublicKeyPemFromCertificate = (x509Certificate: string) => { const certDerBytes = util.decode64(x509Certificate); const obj = asn1.fromDer(certDerBytes); const cert = pki.certificateFromAsn1(obj); return pki.publicKeyToPem(cert.publicKey); -} +}; const stripCertHeaderAndFooter = (cert: string): string => { cert = cert.replace(/-+BEGIN CERTIFICATE-+\r?\n?/, ''); @@ -24,6 +24,16 @@ const stripCertHeaderAndFooter = (cert: string): string => { return cert; }; +const certToPEM = (certificate: string) => { + if (certificate.indexOf('BEGIN CERTIFICATE') === -1 && certificate.indexOf('END CERTIFICATE') === -1) { + certificate = certificate.match(/.{1,64}/g)!.join('\n'); + certificate = '-----BEGIN CERTIFICATE-----\n' + certificate; + certificate = certificate + '\n-----END CERTIFICATE-----\n'; + } + + return certificate; +}; + function GetKeyInfo(this: any, x509Certificate: string, signatureConfig: any = {}) { x509Certificate = stripCertHeaderAndFooter(x509Certificate); @@ -43,4 +53,5 @@ export { stripCertHeaderAndFooter, getPublicKeyPemFromCertificate, GetKeyInfo, + certToPEM, }; diff --git a/utils/idp.ts b/utils/idp.ts index 0aba1f9..45296af 100644 --- a/utils/idp.ts +++ b/utils/idp.ts @@ -48,7 +48,7 @@ const createIdPMetadataXML = async ({ }, }; - return xmlbuilder.create(nodes, { encoding: 'UTF-8', standalone: false }).end(); + return xmlbuilder.create(nodes, { encoding: 'UTF-8', standalone: false }).end({ pretty: true }); }; export { createIdPMetadataXML }; diff --git a/utils/request.ts b/utils/request.ts index 4eaf5ed..29bda61 100644 --- a/utils/request.ts +++ b/utils/request.ts @@ -1,4 +1,7 @@ +import { DOMParser as Dom } from '@xmldom/xmldom'; import { promisify } from 'util'; +import { certToPEM } from 'utils'; +import { SignedXml, xpath as select } from 'xml-crypto'; import xml2js from 'xml2js'; import { inflateRaw } from 'zlib'; @@ -17,21 +20,66 @@ const parseXML = (xml: string): Promise> => { }); }; +// Decode the base64 string +const decodeBase64 = async (string: string, isDeflated: boolean) => { + return isDeflated + ? (await inflateRawAsync(Buffer.from(string, 'base64'))).toString() + : Buffer.from(string, 'base64').toString(); +}; + // Parse SAMLRequest attributes -const extractSAMLRequestAttributes = async (samlRequest: string, isDeflated: boolean) => { - const request = isDeflated - ? (await inflateRawAsync(Buffer.from(samlRequest, 'base64'))).toString() - : Buffer.from(samlRequest, 'base64').toString(); - const result = await parseXML(request); +const extractSAMLRequestAttributes = async (rawRequest: string) => { + const result = await parseXML(rawRequest); const attributes = result['samlp:AuthnRequest']['$']; const issuer = result['samlp:AuthnRequest']['saml:Issuer']; + return { id: attributes.ID, acsUrl: attributes.AssertionConsumerServiceURL, providerName: attributes.ProviderName, audience: issuer[0]['_'], + publicKey: + result['samlp:AuthnRequest']['Signature'][0]['KeyInfo'][0]['X509Data'][0]['X509Certificate'][0], }; }; -export { extractSAMLRequestAttributes }; +// Validate signature +const hasValidSignature = async (xml: string, certificate: string): Promise => { + return new Promise((resolve, reject) => { + const doc = new Dom().parseFromString(xml); + + const signature = + select( + doc, + "/*/*/*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']" + )[0] || + select( + doc, + "/*/*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']" + )[0] || + select( + doc, + "/*/*/*/*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']" + )[0]; + + const signed = new SignedXml(); + + signed.keyInfoProvider = { + getKey: function getKey(keyInfo: any) { + return certToPEM(certificate); + }, + getKeyInfo: function getKeyInfo(key: any) { + return ''; + }, + }; + + signed.loadSignature(signature.toString()); + + const response = signed.checkSignature(xml); + + return !response ? reject(false) : resolve(true); + }); +}; + +export { extractSAMLRequestAttributes, hasValidSignature, decodeBase64 };