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 <deepak@boxyhq.com>
2022-03-03 02:36:04 +05:30 · 2022-03-03 02:36:04 +05:30 · 9bc55ea7f0
commit 9bc55ea7f0
parent eb0a0fe518
10 changed files with 175 additions and 39 deletions
--- 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;
--- 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": {
--- 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",
--- 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);
--- 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');
--- 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<string>) {
  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()}`);
--- 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 {
--- 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,
 };
--- 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 };
--- 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<Record<string, any>> => {
  });
 };

+// 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<boolean> => {
+  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 '<X509Data></X509Data>';
+      },
+    };
+
+    signed.loadSignature(signature.toString());
+
+    const response = signed.checkSignature(xml);
+
+    return !response ? reject(false) : resolve(true);
+  });
+};
+
+export { extractSAMLRequestAttributes, hasValidSignature, decodeBase64 };