feat: octet string builder

mastermatt · mastermatt · commit 647bce9beec2 · 2024-01-22T08:22:39.000-07:00
Adds `octetStringBuilder` to libsaml to DRY up the signature alg code. The octet string is needed in four places: creating and verifying the signature for the Redirect and SimpleSign bindings. Previously, both creations were done adhoc while both verifications where not done by the samlify at all and instead left to user code. #308 #360 The new util is now used during verification if the request object does not include a `octetString` key.
diff --git a/src/binding-redirect.ts b/src/binding-redirect.ts
@@ -25,58 +25,59 @@ export interface BuildRedirectConfig {
 
 /**
 * @private
-* @desc Helper of generating URL param/value pair
-* @param  {string} param     key
-* @param  {string} value     value of key
-* @param  {boolean} first    determine whether the param is the starting one in order to add query header '?'
-* @return {string}
-*/
-function pvPair(param: string, value: string, first?: boolean): string {
-  return (first === true ? '?' : '&') + param + '=' + value;
-}
-/**
-* @private
-* @desc Refractored part of URL generation for login/logout request
+* @desc Refactored part of URL generation for login/logout request
 * @param  {string} type
 * @param  {boolean} isSigned
 * @param  {string} rawSamlRequest
 * @param  {object} entitySetting
 * @return {string}
 */
-function buildRedirectURL(opts: BuildRedirectConfig) {
+function buildRedirectURL(opts: BuildRedirectConfig): string {
   const {
     baseUrl,
     type,
-    isSigned,
     context,
-    entitySetting,
+    relayState,
+    isSigned,
+    entitySetting
   } = opts;
-  let { relayState = '' } = opts;
-  const noParams = (url.parse(baseUrl).query || []).length === 0;
-  const queryParam = libsaml.getQueryParamByType(type);
-  // In general, this xmlstring is required to do deflate -> base64 -> urlencode
-  const samlRequest = encodeURIComponent(utility.base64Encode(utility.deflateString(context)));
-  if (relayState !== '') {
-    relayState = pvPair(urlParams.relayState, encodeURIComponent(relayState));
+
+  const redirectUrl = new url.URL(baseUrl)
+
+  const direction = libsaml.getQueryParamByType(type);
+  // In general, this XML string is required to do deflate -> base64 -> URL encode
+  const encodedContext = utility.base64Encode(utility.deflateString(context));
+  redirectUrl.searchParams.set(direction, encodedContext);
+
+  if (relayState) {
+    redirectUrl.searchParams.set(urlParams.relayState, relayState);
   }
+
   if (isSigned) {
-    const sigAlg = pvPair(urlParams.sigAlg, encodeURIComponent(entitySetting.requestSignatureAlgorithm));
-    const octetString = samlRequest + relayState + sigAlg;
-    return baseUrl
-      + pvPair(queryParam, octetString, noParams)
-      + pvPair(urlParams.signature, encodeURIComponent(
-        libsaml.constructMessageSignature(
-          queryParam + '=' + octetString,
-          entitySetting.privateKey,
-          entitySetting.privateKeyPass,
-          undefined,
-          entitySetting.requestSignatureAlgorithm
-        ).toString()
-      )
-      );
+    const octetString = libsaml.octetStringBuilder(
+      binding.redirect,
+      direction,
+      { // can just be `Object.fromEntries(redirectUrl.searchParams)` when the targeted Node version is updated.
+        [direction]: encodedContext,
+        [urlParams.relayState]: relayState,
+        [urlParams.sigAlg]: entitySetting.requestSignatureAlgorithm,
+      }
+    );
+    const signature = libsaml.constructMessageSignature(
+      octetString,
+      entitySetting.privateKey,
+      entitySetting.privateKeyPass,
+      true,
+      entitySetting.requestSignatureAlgorithm
+    ).toString();
+
+    redirectUrl.searchParams.set(urlParams.signature, signature);
+    redirectUrl.searchParams.set(urlParams.sigAlg, entitySetting.requestSignatureAlgorithm);
   }
-  return baseUrl + pvPair(queryParam, samlRequest + relayState, noParams);
+
+  return redirectUrl.toString();
 }
+
 /**
 * @desc Redirect URL for login request
 * @param  {object} entity                       object includes both idp and sp
diff --git a/src/binding-simplesign.ts b/src/binding-simplesign.ts
@@ -26,17 +26,6 @@ export interface BindingSimpleSignContext {
   sigAlg: string;
 }
 
-/**
-* @private
-* @desc Helper of generating URL param/value pair
-* @param  {string} param     key
-* @param  {string} value     value of key
-* @param  {boolean} first    determine whether the param is the starting one in order to add query header '?'
-* @return {string}
-*/
-function pvPair(param: string, value: string, first?: boolean): string {
-  return (first === true ? '?' : '&') + param + '=' + value;
-}
 /**
 * @private
 * @desc Refactored part of simple signature generation for login/logout request
@@ -51,17 +40,16 @@ function buildSimpleSignature(opts: BuildSimpleSignConfig) : string {
     context,
     entitySetting,
   } = opts;
-  let { relayState = '' } = opts;
-  const queryParam = libsaml.getQueryParamByType(type);
-
-  if (relayState !== '') {
-    relayState = pvPair(urlParams.relayState, relayState);
-  }
+  // ?SAMLRequest= or ?SAMLResponse=
+  const direction = libsaml.getQueryParamByType(type);
+  const octetString =  libsaml.octetStringBuilder(binding.simpleSign, direction, {
+    [direction]: context,
+    [urlParams.relayState]: opts.relayState,
+    [urlParams.sigAlg]: entitySetting.requestSignatureAlgorithm,
+  });
 
-  const sigAlg = pvPair(urlParams.sigAlg, entitySetting.requestSignatureAlgorithm);
-  const octetString = context + relayState + sigAlg;
   return libsaml.constructMessageSignature(
-    queryParam + '=' + octetString,
+    octetString,
     entitySetting.privateKey,
     entitySetting.privateKeyPass,
     undefined,
@@ -117,7 +105,7 @@ function base64LoginRequest(entity: any, customTagReplacement?: (template: strin
           sigAlg: spSetting.requestSignatureAlgorithm,
         };
     }
-    // No need to embeded XML signature
+    // No need to embed XML signature
     return {
       id,
       context: utility.base64Encode(rawSamlRequest),
diff --git a/src/flow.ts b/src/flow.ts
@@ -53,7 +53,7 @@ function getDefaultExtractorFields(parserType: ParserType, assertion?: any): Ext
 async function redirectFlow(options): Promise<FlowResult>  {
 
   const { request, parserType, self, checkSignature = true, from } = options;
-  const { query, octetString } = request;
+  const { query } = request;
   const { SigAlg: sigAlg, Signature: signature } = query;
 
   const targetEntityMetadata = from.entityMeta;
@@ -109,6 +109,9 @@ async function redirectFlow(options): Promise<FlowResult>  {
       return Promise.reject('ERR_MISSING_SIG_ALG');
     }
 
+    // Look for the octet string on the request object first as a backwards compat feature
+    const octetString = request.octetString || libsaml.octetStringBuilder(bindDict.redirect, direction, query)
+
     // put the below two assignments into verifyMessageSignature function
     const base64Signature = Buffer.from(decodeURIComponent(signature), 'base64');
     const decodeSigAlg = decodeURIComponent(sigAlg);
@@ -296,9 +299,7 @@ async function postFlow(options): Promise<FlowResult> {
 async function postSimpleSignFlow(options): Promise<FlowResult> {
 
   const { request, parserType, self, checkSignature = true, from } = options;
-
-  const { body, octetString } = request;
-
+  const { body } = request;
   const targetEntityMetadata = from.entityMeta;
 
   // ?SAMLRequest= or ?SAMLResponse=
@@ -354,6 +355,19 @@ async function postSimpleSignFlow(options): Promise<FlowResult> {
       return Promise.reject('ERR_MISSING_SIG_ALG');
     }
 
+    // Look for the octet string on the request object first as a backwards compat feature
+    const octetString = request.octetString || libsaml.octetStringBuilder(
+      bindDict.simpleSign,
+      direction,
+      {
+        ...body,
+        // SimpleSign wants the XML already base64-decoded before computing the octet string.
+        // For encapsulation, it would be nice to have the helper decode it,
+        // but for optimization, lets not decode it twice.
+        [direction]: xmlString
+      }
+    )
+
     // put the below two assignments into verifyMessageSignature function
     const base64Signature = Buffer.from(signature, 'base64');
 
diff --git a/src/libsaml.ts b/src/libsaml.ts
@@ -238,6 +238,55 @@ const libSaml = () => {
     return prefix + camelContent.charAt(0).toUpperCase() + camelContent.slice(1);
   }
 
+  /**
+   * Create the octet string for the signature algorithms.
+   *
+   * Used in both Redirect and POST-SimpleSign bindings.
+   * HTTP-Redirect Binding as defined in the Bindings OASIS Standard, Section 3.4.4.1.
+   * http://docs.oasis-open.org/security/saml/v2.0/saml-bindings-2.0-os.pdf
+   * HTTP-POST-SimpleSign Binding as defined in the "SimpleSign" Binding OASIS Committee Draft.
+   * https://www.oasis-open.org/committees/download.php/30234/sstc-saml-binding-simplesign-cd-04.pdf
+   *
+   * @public
+   * @param {string} binding "redirect" or "simpleSign"
+   * @param {string} direction "SAMLRequest" or "SAMLResponse"
+   * @param {object} values Object that includes SAMLRequest/SAMLResponse, SigAlg, and optional RelayState. All other values are ignored.
+   * @return {string}
+   */
+  function octetStringBuilder(binding: string, direction: string, values: Record<string, unknown>): string {
+    const content = values[direction]
+    const sigAlg = values[urlParams.sigAlg]
+    const relayState = values[urlParams.relayState]
+
+    if (typeof content !== 'string') {
+      throw Error('ERR_OCTET_BAD_ARGS_CONTENT')
+    }
+
+    if (typeof sigAlg !== 'string') {
+      throw Error('ERR_MISSING_SIG_ALG')
+    }
+
+    const params: string[][] = [[direction, content]]
+
+    if (typeof relayState === 'string' && relayState.length !== 0) {
+      params.push([urlParams.relayState, relayState])
+    }
+
+    params.push([urlParams.sigAlg, sigAlg])
+
+    // Redirect binding needs each param URL encoded, `URLSearchParams` gives us this out of the box and maintains order.
+    if (binding === wording.binding.redirect){
+      return new URLSearchParams(params).toString()
+    }
+
+    // The SimpleSign octet string combines the params with &,= but doesn't encode for URLs.
+    if (binding === wording.binding.simpleSign){
+      return params.map(([k,v]) => `${k}=${v}`).join('&')
+    }
+
+    throw Error('ERR_OCTET_UNDEFINED_BINDING')
+  }
+
   return {
 
     createXPath,
@@ -248,6 +297,7 @@ const libSaml = () => {
     defaultAttributeTemplate,
     defaultLogoutRequestTemplate,
     defaultLogoutResponseTemplate,
+    octetStringBuilder,
 
     /**
     * @desc Replace the tag (e.g. {tag}) inside the raw XML
diff --git a/test/flow.ts b/test/flow.ts
diff --git a/test/issues.ts b/test/issues.ts
