Not work well with K-256

[monkeylord]

CryptoEngine.prototype.getAlgorithmByOID does not recognize K-256 ECPublicKey does not recognized it, either.

They use a switch-case to recognize algorithm, which make them incompatible with algorithm not listed, even though webcrypto-liner is used.

like

  switch(this.namedCurve)
  {
      case "1.2.840.10045.3.1.7": // P-256
          crvName = "P-256";
          break;
      case "1.3.132.0.34": // P-384
          crvName = "P-384";
          break;
      case "1.3.132.0.35": // P-521
          crvName = "P-521";
          break;
      default:
  }

This make ECPublicKey incompatible with K-256

Could you change the switch-case structure to a query from a table? So that new algorithm can be added simply by adding elements to the table?

[YuryStrozhevsky]

@monkeylord It is not so simple. So, "regular" PKI.js crypto engine supports only algorithms supported by WebCrypto API and AFAIK K-256 is not there. And if you need to support new algorithms you need to make your own crypto engine. And there make any switches, whatever you want.

[rmhrisk]

@monkeylord here is a sample that creates an engine: https://github.com/PeculiarVentures/PKI.js/tree/master/examples/NodePKCS12Example

With that and webcrypto-liner you should be able to use k1.

[monkeylord]

Oh, it's not about engine. It's about class define.

For example, ECPublicKey does not support K-256 because algorithm is hardcoded. It does not support K-256 even though you use webcrypto-liner.

So, what I suggested is do not make algorithm hardcoded in those classes.

[YuryStrozhevsky]

Seems I neee to explain how PKIjs works. So the lib has a core layer where main magic happens. The layer is “algorithm independent “. And there is a specific abstraction calling “Crypto engine” where there are all algorithm dependent parts. The second layer is a specific JavaScript class with specific interface. So the lib in fact has a potential to work with any crypto interfaces: WebCrypto API, Node.js crypto interface or any other APIs. For example webcrypto-liner just introduces some additional algorithms above WebCrypto API. And in order to make all algorithms to be working with PKIjs user need to make a specialized instance of CryptoEngine class. And again: if you need to implement a support for K-256 algorithm in PKIjs you need to also make specialized instance of CryptoEngine class.

[monkeylord]

The design is wonderful, but some implements are hardcoded. For example, in ./src/ECPublicKey.js It does only recognized 3 named curve, and I cannot add my curve without rewrite it. And I have to compromise this because it's used in a lot of classes, like certificate. It's true that pkijs can generate certificate with K-256, but it cannot deserialize nor verify properly.

    fromSchema(schema)
    {
        //region Check the schema is valid
        if((schema instanceof ArrayBuffer) === false)
            throw new Error("Object's schema was not verified against input data for ECPublicKey");

        const view = new Uint8Array(schema);
        if(view[0] !== 0x04)
            throw new Error("Object's schema was not verified against input data for ECPublicKey");
        //endregion

        //region Get internal properties from parsed schema
        let coordinateLength;

        switch(this.namedCurve)
        {
            case "1.2.840.10045.3.1.7": // P-256
                coordinateLength = 32;
                break;
            case "1.3.132.0.34": // P-384
                coordinateLength = 48;
                break;
            case "1.3.132.0.35": // P-521
                coordinateLength = 66;
                break;
            default:
                throw new Error(`Incorrect curve OID: ${this.namedCurve}`);
        }

        if(schema.byteLength !== (coordinateLength * 2 + 1))
            throw new Error("Object's schema was not verified against input data for ECPublicKey");

        this.x = schema.slice(1, coordinateLength + 1);
        this.y = schema.slice(1 + coordinateLength, coordinateLength * 2 + 1);
        //endregion
    }

    toJSON()
    {
        let crvName = "";

        switch(this.namedCurve)
        {
            case "1.2.840.10045.3.1.7": // P-256
                crvName = "P-256";
                break;
            case "1.3.132.0.34": // P-384
                crvName = "P-384";
                break;
            case "1.3.132.0.35": // P-521
                crvName = "P-521";
                break;
            default:
        }

        return {
            crv: crvName,
            x: toBase64(arrayBufferToString(this.x), true, true, false),
            y: toBase64(arrayBufferToString(this.y), true, true, false)
        };
    }

    fromJSON(json)
    {
        let coodinateLength = 0;

        if("crv" in json)
        {
            switch(json.crv.toUpperCase())
            {
                case "P-256":
                    this.namedCurve = "1.2.840.10045.3.1.7";
                    coodinateLength = 32;
                    break;
                case "P-384":
                    this.namedCurve = "1.3.132.0.34";
                    coodinateLength = 48;
                    break;
                case "P-521":
                    this.namedCurve = "1.3.132.0.35";
                    coodinateLength = 66;
                    break;
                default:
            }
        }
        else
            throw new Error("Absent mandatory parameter \"crv\"");

        if("x" in json)
        {
            const convertBuffer = stringToArrayBuffer(fromBase64(json.x, true));

            if(convertBuffer.byteLength < coodinateLength)
            {
                this.x = new ArrayBuffer(coodinateLength);
                const view = new Uint8Array(this.x);
                const convertBufferView = new Uint8Array(convertBuffer);
                view.set(convertBufferView, 1);
            }
            else
                this.x = convertBuffer.slice(0, coodinateLength);
        }
        else
            throw new Error("Absent mandatory parameter \"x\"");

        if("y" in json)
        {
            const convertBuffer = stringToArrayBuffer(fromBase64(json.y, true));

            if(convertBuffer.byteLength < coodinateLength)
            {
                this.y = new ArrayBuffer(coodinateLength);
                const view = new Uint8Array(this.y);
                const convertBufferView = new Uint8Array(convertBuffer);
                view.set(convertBufferView, 1);
            }
            else
                this.y = convertBuffer.slice(0, coodinateLength);
        }
        else
            throw new Error("Absent mandatory parameter \"y\"");
    }

[YuryStrozhevsky]

@monkeylord Great, this is the comment you should be started from. I will try to move the curve selection part into CryptoEngine class.