refactor!: removed optional "none" JWS algorithm support

BREAKING CHANGE: The "none" JWS algorithm, which was previously disabled by default, is now completely removed.
panva · Dec 1, 2022 · e654fe6 · e654fe6
1 parent 868ab2f
commit e654fe6
Show file tree

Hide file tree

Showing 32 changed files with 142 additions and 448 deletions.
diff --git a/certification/oidc/configuration.js b/certification/oidc/configuration.js
@@ -3,13 +3,6 @@ const crypto = require('crypto');
 const pkg = require('../../package.json');
 const enabledJWA = JSON.parse(JSON.stringify(require('../../lib/consts/jwa')));
 
-function filterOutNone(conf, prop) {
-  // eslint-disable-next-line no-param-reassign
-  conf[prop] = conf[prop].filter((alg) => alg !== 'none');
-}
-
-Object.keys(enabledJWA).forEach(filterOutNone.bind(undefined, enabledJWA));
-
 const timeout = parseInt(process.env.TIMEOUT, 10);
 const clientAuthMethods = [
   'none',

diff --git a/docs/README.md b/docs/README.md
@@ -3241,7 +3241,6 @@ _**default value**_:
   'ES256', 'ES256K', 'ES384', 'ES512',
   'EdDSA',
   'HS256', 'HS384', 'HS512',
-  'none',
 ]
 ```
 </details>
@@ -3329,7 +3328,6 @@ _**default value**_:
   'ES256', 'ES256K', 'ES384', 'ES512',
   'EdDSA',
   'HS256', 'HS384', 'HS512',
-  'none',
 ]
 ```
 </details>
@@ -3418,7 +3416,6 @@ _**default value**_:
   'ES256', 'ES256K', 'ES384', 'ES512',
   'EdDSA',
   'HS256', 'HS384', 'HS512',
-  'none',
 ]
 ```
 </details>
@@ -3506,7 +3503,6 @@ _**default value**_:
   'ES256', 'ES256K', 'ES384', 'ES512',
   'EdDSA',
   'HS256', 'HS384', 'HS512',
-  'none',
 ]
 ```
 </details>

diff --git a/lib/actions/authorization/process_request_object.js b/lib/actions/authorization/process_request_object.js
@@ -212,7 +212,7 @@ module.exports = async function processRequestObject(PARAM_LIST, rejectDupesMidd
     throw new InvalidRequestObject('Request Object claims are invalid', err.message);
   }
 
-  if (alg !== 'none') {
+  if (!pushedRequestObject) {
     try {
       if (alg.startsWith('HS')) {
         client.checkClientSecretExpiration('could not validate the Request Object - the client secret used for its signature is expired', 'invalid_request_object');
@@ -228,18 +228,16 @@ module.exports = async function processRequestObject(PARAM_LIST, rejectDupesMidd
 
       throw new InvalidRequestObject('could not validate Request Object', err.message);
     }
+  }
 
-    if (route !== 'pushed_authorization_request' && payload.jti && payload.exp && payload.iss) {
-      const unique = await ctx.oidc.provider.ReplayDetection.unique(
-        payload.iss, payload.jti, payload.exp + conf.clockTolerance,
-      );
+  if (route !== 'pushed_authorization_request' && payload.jti && payload.exp && payload.iss) {
+    const unique = await ctx.oidc.provider.ReplayDetection.unique(
+      payload.iss, payload.jti, payload.exp + conf.clockTolerance,
+    );
 
-      if (!unique) {
-        throw new InvalidRequestObject(`request replay detected (jti: ${payload.jti})`);
-      }
+    if (!unique) {
+      throw new InvalidRequestObject(`request replay detected (jti: ${payload.jti})`);
     }
-  } else if (client.requireSignedRequestObject) {
-    throw new InvalidRequestObject('Request Object must not be unsigned for this client');
   }
 
   if (pushedRequestObject) {

diff --git a/lib/actions/discovery.js b/lib/actions/discovery.js
@@ -124,7 +124,7 @@ module.exports = function discovery(ctx, next) {
     ctx.body.backchannel_authentication_endpoint = ctx.oidc.urlFor('backchannel_authentication');
     ctx.body.backchannel_token_delivery_modes_supported = [...features.ciba.deliveryModes];
     ctx.body.backchannel_user_code_parameter_supported = true;
-    ctx.body.backchannel_authentication_request_signing_alg_values_supported = config.requestObjectSigningAlgValues.filter((alg) => alg !== 'none' && !alg.startsWith('HS'));
+    ctx.body.backchannel_authentication_request_signing_alg_values_supported = config.requestObjectSigningAlgValues.filter((alg) => !alg.startsWith('HS'));
   }
 
   defaults(ctx.body, config.discovery);

diff --git a/lib/consts/jwa.js b/lib/consts/jwa.js
@@ -25,10 +25,10 @@ const encryptionEncValues = [
 module.exports = {
   clientAuthSigningAlgValues: [...signingAlgValues],
 
-  idTokenSigningAlgValues: [...signingAlgValues, 'none'],
-  requestObjectSigningAlgValues: [...signingAlgValues, 'none'],
-  userinfoSigningAlgValues: [...signingAlgValues, 'none'],
-  introspectionSigningAlgValues: [...signingAlgValues, 'none'],
+  idTokenSigningAlgValues: [...signingAlgValues],
+  requestObjectSigningAlgValues: [...signingAlgValues],
+  userinfoSigningAlgValues: [...signingAlgValues],
+  introspectionSigningAlgValues: [...signingAlgValues],
   authorizationSigningAlgValues: [...signingAlgValues],
 
   idTokenEncryptionAlgValues: [...encryptionAlgValues],

diff --git a/lib/helpers/_/without.js b/lib/helpers/_/without.js
diff --git a/lib/helpers/client_schema.js b/lib/helpers/client_schema.js
@@ -23,7 +23,6 @@ const sectorIdentifier = require('./sector_identifier');
 const instance = require('./weak_cache');
 const formatters = require('./formatters');
 const pick = require('./_/pick');
-const without = require('./_/without');
 const omitBy = require('./_/omit_by');
 
 const W3CEmailRegExp = /^[a-zA-Z0-9.!#$%&’*+/=?^_`{|}~-]+@[a-zA-Z0-9-]+(?:\.[a-zA-Z0-9-]+)*$/;
@@ -154,15 +153,10 @@ module.exports = function getSchema(provider) {
     grant_types: () => configuration.grantTypes,
     id_token_encrypted_response_alg: () => configuration.idTokenEncryptionAlgValues,
     id_token_encrypted_response_enc: () => configuration.idTokenEncryptionEncValues,
-    id_token_signed_response_alg: (metadata) => {
-      if (!metadata.response_types.join(' ').includes('id_token')) {
-        return configuration.idTokenSigningAlgValues;
-      }
-      return without(configuration.idTokenSigningAlgValues, (alg) => alg === 'none');
-    },
+    id_token_signed_response_alg: () => configuration.idTokenSigningAlgValues,
     request_object_signing_alg: () => configuration.requestObjectSigningAlgValues,
     backchannel_token_delivery_mode: () => features.ciba.deliveryModes,
-    backchannel_authentication_request_signing_alg: () => configuration.requestObjectSigningAlgValues.filter((alg) => alg !== 'none' && !alg.startsWith('HS')),
+    backchannel_authentication_request_signing_alg: () => configuration.requestObjectSigningAlgValues.filter((alg) => !alg.startsWith('HS')),
     request_object_encryption_alg: () => configuration.requestObjectEncryptionAlgValues,
     request_object_encryption_enc: () => configuration.requestObjectEncryptionEncValues,
     response_types: () => configuration.responseTypes,
@@ -230,7 +224,6 @@ module.exports = function getSchema(provider) {
       this.redirectUris();
       this.webMessageUris();
       this.checkContacts();
-      this.backchannelLogoutNeedsIdTokenAlg();
       this.jarPolicy();
       this.parPolicy();
 
@@ -595,9 +588,6 @@ module.exports = function getSchema(provider) {
         if (requestObjects.requireSignedRequestObject) {
           this.require_signed_request_object = true;
         }
-        if (this.require_signed_request_object && this.request_object_signing_alg === 'none') {
-          this.invalidate('request_object_signing_alg must not be "none" when require_signed_request_object is true');
-        }
       }
     }
 
@@ -610,12 +600,6 @@ module.exports = function getSchema(provider) {
       });
     }
 
-    backchannelLogoutNeedsIdTokenAlg() {
-      if (this.backchannel_logout_uri && this.id_token_signed_response_alg === 'none') {
-        this.invalidate('id_token_signed_response_alg must not be "none" when backchannel_logout_uri is used');
-      }
-    }
-
     scopes() {
       if (this.scope) {
         const parsed = new Set(this.scope.split(' '));

diff --git a/lib/helpers/configuration.js b/lib/helpers/configuration.js
@@ -27,10 +27,6 @@ function filterHS(alg) {
 
 const filterAsymmetricSig = RegExp.prototype.test.bind(/^(?:PS(?:256|384|512)|RS(?:256|384|512)|ES(?:256K?|384|512)|EdDSA)$/);
 
-function filterHSandNone(alg) {
-  return alg.startsWith('HS') || alg === 'none';
-}
-
 const supportedResponseTypes = new Set(['none', 'code', 'id_token', 'token']);
 const requestObjectStrategies = new Set(['lax', 'strict']);
 const fapiProfiles = new Set(['1.0 Final', '1.0 ID2']);
@@ -246,19 +242,19 @@ class Configuration {
   defaultSigAlg() {
     const allowList = this.enabledJWA;
 
-    this.setAlgs('idTokenSigningAlgValues', allowList.idTokenSigningAlgValues.filter(filterHSandNone));
+    this.setAlgs('idTokenSigningAlgValues', allowList.idTokenSigningAlgValues.filter(filterHS));
     this.setAlgs('idTokenEncryptionAlgValues', allowList.idTokenEncryptionAlgValues.slice());
     this.setAlgs('idTokenEncryptionEncValues', allowList.idTokenEncryptionEncValues.slice(), 'encryption.enabled');
 
     this.setAlgs('requestObjectSigningAlgValues', allowList.requestObjectSigningAlgValues.slice(), ['requestObjects.request', 'requestObjects.requestUri', 'pushedAuthorizationRequests.enabled', 'ciba.enabled']);
     this.setAlgs('requestObjectEncryptionAlgValues', allowList.requestObjectEncryptionAlgValues.filter(RegExp.prototype.test.bind(/^(A|dir$)/)), 'encryption.enabled', ['requestObjects.request', 'requestObjects.requestUri', 'pushedAuthorizationRequests.enabled']);
     this.setAlgs('requestObjectEncryptionEncValues', allowList.requestObjectEncryptionEncValues.slice(), 'encryption.enabled', ['requestObjects.request', 'requestObjects.requestUri', 'pushedAuthorizationRequests.enabled']);
 
-    this.setAlgs('userinfoSigningAlgValues', allowList.userinfoSigningAlgValues.filter(filterHSandNone), 'jwtUserinfo.enabled');
+    this.setAlgs('userinfoSigningAlgValues', allowList.userinfoSigningAlgValues.filter(filterHS), 'jwtUserinfo.enabled');
     this.setAlgs('userinfoEncryptionAlgValues', allowList.userinfoEncryptionAlgValues.slice(), 'jwtUserinfo.enabled', 'encryption.enabled');
     this.setAlgs('userinfoEncryptionEncValues', allowList.userinfoEncryptionEncValues.slice(), 'jwtUserinfo.enabled', 'encryption.enabled');
 
-    this.setAlgs('introspectionSigningAlgValues', allowList.introspectionSigningAlgValues.filter(filterHSandNone), 'jwtIntrospection.enabled');
+    this.setAlgs('introspectionSigningAlgValues', allowList.introspectionSigningAlgValues.filter(filterHS), 'jwtIntrospection.enabled');
     this.setAlgs('introspectionEncryptionAlgValues', allowList.introspectionEncryptionAlgValues.slice(), 'jwtIntrospection.enabled', 'encryption.enabled');
     this.setAlgs('introspectionEncryptionEncValues', allowList.introspectionEncryptionEncValues.slice(), 'jwtIntrospection.enabled', 'encryption.enabled');
 

diff --git a/lib/helpers/defaults.js b/lib/helpers/defaults.js
@@ -2398,7 +2398,6 @@ function getDefaults() {
        *   'ES256', 'ES256K', 'ES384', 'ES512',
        *   'EdDSA',
        *   'HS256', 'HS384', 'HS512',
-       *   'none',
        * ]
        * ```
        */
@@ -2419,7 +2418,6 @@ function getDefaults() {
        *   'ES256', 'ES256K', 'ES384', 'ES512',
        *   'EdDSA',
        *   'HS256', 'HS384', 'HS512',
-       *   'none',
        * ]
        * ```
        */
@@ -2440,7 +2438,6 @@ function getDefaults() {
        *   'ES256', 'ES256K', 'ES384', 'ES512',
        *   'EdDSA',
        *   'HS256', 'HS384', 'HS512',
-       *   'none',
        * ]
        * ```
        */
@@ -2461,7 +2458,6 @@ function getDefaults() {
        *   'ES256', 'ES256K', 'ES384', 'ES512',
        *   'EdDSA',
        *   'HS256', 'HS384', 'HS512',
-       *   'none',
        * ]
        * ```
        */

diff --git a/lib/helpers/jwt.js b/lib/helpers/jwt.js
@@ -45,10 +45,6 @@ class JWT {
       sub: options.subject !== undefined ? options.subject : payload.sub,
     });
 
-    if (alg === 'none') {
-      return [base64url.encode(JSON.stringify(header)), base64url.encode(JSON.stringify(payload)), ''].join('.');
-    }
-
     return new CompactSign(Buffer.from(JSON.stringify(payload)))
       .setProtectedHeader(header)
       .sign(key);

diff --git a/lib/models/id_token.js b/lib/models/id_token.js
@@ -153,14 +153,14 @@ module.exports = function getIdToken(provider) {
           }
           [jwk] = client.symmetricKeyStore.selectForSign({ alg, use: 'sig' });
           key = await client.symmetricKeyStore.getKeyObject(jwk, alg);
-        } else if (alg !== 'none') {
+        } else {
           [jwk] = instance(provider).keystore.selectForSign({ alg, use: 'sig' });
           key = await instance(provider).keystore.getKeyObject(jwk, alg).catch(() => {
             throw new Error(`provider key (kid: ${jwk.kid}) is invalid`);
           });
         }
 
-        if (alg !== 'none') {
+        if (use === 'idtoken') {
           hashes.forEach((claim) => {
             if (payload[claim]) {
               payload[claim] = tokenHash(payload[claim], alg, jwk.crv);
@@ -224,7 +224,7 @@ module.exports = function getIdToken(provider) {
       if (alg.startsWith('HS')) {
         client.checkClientSecretExpiration('client secret is expired - cannot validate ID Token Hint');
         keyOrStore = client.symmetricKeyStore;
-      } else if (alg !== 'none') {
+      } else {
         keyOrStore = instance(provider).keystore;
       }
 

diff --git a/test/ciba/ciba.test.js b/test/ciba/ciba.test.js
@@ -22,7 +22,7 @@ describe('configuration features.ciba', () => {
       .expect(200)
       .expect((response) => {
         expect(response.body).to.have.property('backchannel_authentication_endpoint').matches(/\/backchannel$/);
-        expect(response.body).to.have.property('backchannel_authentication_request_signing_alg_values_supported').not.contains('none').not.contains('HS256');
+        expect(response.body).to.have.property('backchannel_authentication_request_signing_alg_values_supported').not.contains('HS256');
         expect(response.body).to.have.property('backchannel_token_delivery_modes_supported').deep.equal(['poll', 'ping']);
         expect(response.body).to.have.property('backchannel_user_code_parameter_supported', true);
       });

diff --git a/test/client_auth/client_auth.config.js b/test/client_auth/client_auth.config.js
@@ -73,7 +73,7 @@ module.exports = {
   }, {
     token_endpoint_auth_method: 'client_secret_jwt',
     client_id: 'client-jwt-secret',
-    client_secret: 'its64bytes_____________________________________________________!',
+    client_secret: 'secret',
     grant_types: ['foo'],
     response_types: [],
     redirect_uris: [],

diff --git a/test/configuration/client_keystore.test.js b/test/configuration/client_keystore.test.js
@@ -34,7 +34,7 @@ describe('client keystore refresh', () => {
       client_secret: 'secret',
       redirect_uris: ['https://client.example.com/cb'],
       jwks_uri: 'https://client.example.com/jwks',
-      id_token_signed_response_alg: 'none',
+      id_token_signed_response_alg: 'HS256',
       id_token_encrypted_response_alg: 'ECDH-ES+A128KW',
       id_token_encrypted_response_enc: 'A128CBC-HS256',
     });