Enforce or remove algorithm Params classes

[thelunararmy]

Every instance of an algorithm class uses an imported 'algorithm parameter' class object for use in various functions as a parameter.

For example in RSASSA_PKCS1_v1_5's generateKey uses a RsaHashedKeyGenParams.
And...
In AES_CBC's decrypt uses a AesKeyAlgorithm.

The problem is that there is absolutely no type checking within these function calls and it is perfectly acceptable to pass in a new object with any attributes. This causes an intrinsic need for type checking and validation at the start of every one of these function calls since there is surety of these Param objects.

At some point we should investigate writing some supported system to ensure that these param objects are indeed created and passed in correctly; or go back to all the existing algorithms and ensure there are parameter type checks present at every function call. Currently only newer algorithms (AES-CTR onward) have these checks in place.

Take a look at AES_CBC's decrypt call:

    decrypt (algorithm, key, data) {
      // 1. Ensure correct iv length
      if (algorithm.iv.byteLength !== 16){
        throw new OperationError('IV Length must be exactly 16 bytes')
      }
      ...
    }

This would require either:

• The algorithm parameter be ensured to be an AesKeyAlgorithm object with correct fields present, or
• The if statement to have algorithm.iv instanceof ArrayBuffer before doing the byteLength check, with corresponding check with every first time use of an algorithm parameter

@christiansmith Your input would be greatly appreciated

[EternalDeiwos]

I agree that this needs to be considered.

Regarding a way that we can easily use the dictionaries to help us validate input is to use json-document.

I was thinking something like this could help?:

'use strict'

/**
 * Dependencies
 * @ignore
 */
const { JSONSchema, JSONDocument } = require('@trust/json-document')

/**
 * Dictionary
 */
class Dictionary extends JSONDocument {

  static get schema () {
    return new JSONSchema({
      type: 'object',
      properties: {
        name: { type: 'string' },
      }
      required: ['name']
    })
  }

}

class ExtendedDictionary extends Dictionary {

  static get schema () {
    return super.schema.extend({
      properties: {
        counter: { type: 'object' },
        length: { type: 'number' },
      },
      required: ['name', 'counter', 'length']
    })
  }

  constructor (alg, options) {
    super(alg, options)
    // Extra checks go here
  }
}

class Other {

  doSomething () {
    let alg = new ExtendedDictionary(data)
    let validation = alg.validate()

    if (!validation.valid) {
      let error = new Error('AesCtrParams validation failed')
      error.validation = validation
      throw error
    }

    if (!alg instanceof MyParamClass) {
      throw new Error('alg parameter is not an instance of AesCtrParams')
    }
  }
}

[dmitrizagidulin]

👍 to using json-document to check the parameters; was thinking the same thing.

[christiansmith]

@thelunararmy and @EternalDeiwos, the dictionaries that are stubbed out are intended to be used for type checking. @dmitrizagidulin and I discussed using JSON Document for this back at the very beginning of sketching out this code and decided against it. It's very heavy for this purpose.

The intention was initially to try and have the implemented algorithms match the control flow expressed in the W3C spec. But those algorithm descriptions make no reference of the dictionaries defined in IDL. So we haven't glued them together.

What we need to do is refactor the specific algorithm classes to use the dictionaries for type checking, which will satisfy some of the same steps explicitly in the specs, but it makes the control flow less explicit in comparison. I avoided this refactoring for a while to see if we were going the right direction.

Perhaps now is the right time to follow through. Let's sort it out in a pairing session.

[christiansmith]

@dmitrizagidulin – still thinking we should use these dictionary classes, but for further consideration and record of what's transpired in conversation, what was the library you mentioned that compiles WebIDL into js "types"?

[dmitrizagidulin]

@christiansmith I was thinking of this one - https://github.com/jsdom/webidl2js