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.