PeculiarVentures / asn1-schema

asn1-schema is a collection of TypeScript schemas that make working with common ASN.1 objects easy
33 stars 11 forks source link

AsnParser.parse() fails to recognise some `ArrayBuffer`s #71

Closed gnarea closed 2 years ago

gnarea commented 2 years ago

The problem

Upgrading @​peculiar/webcrypto from 1.3.3 to 1.4.0 causes the following error:

TypeError: Wrong type of 'data' argument

    at Function.parse (/home/gus/repos/awala-keystore-gcp-js/node_modules/@peculiar/asn1-schema/build/cjs/parser.js:23:19)
    at EcPrivateKey.getKey (/home/gus/repos/awala-keystore-gcp-js/node_modules/@peculiar/webcrypto/build/webcrypto.js:1404:37)
    at EcPrivateKey.toJSON (/home/gus/repos/awala-keystore-gcp-js/node_modules/@peculiar/webcrypto/build/webcrypto.js:1407:26)
    at Function.toJSON (/home/gus/repos/awala-keystore-gcp-js/node_modules/@peculiar/json-schema/build/index.js:254:24)
    at Function.exportKey (/home/gus/repos/awala-keystore-gcp-js/node_modules/@peculiar/webcrypto/build/webcrypto.js:1616:50)
    at EcdhProvider.onExportKey (/home/gus/repos/awala-keystore-gcp-js/node_modules/@peculiar/webcrypto/build/webcrypto.js:1777:25)
    at EcdhProvider.exportKey (/home/gus/repos/awala-keystore-gcp-js/node_modules/webcrypto-core/build/webcrypto-core.js:223:33)
    at SubtleCrypto.exportKey (/home/gus/repos/awala-keystore-gcp-js/node_modules/webcrypto-core/build/webcrypto-core.js:1465:39)
    at CryptoEngine.exportKey (/home/gus/repos/awala-keystore-gcp-js/node_modules/pkijs/build/CryptoEngine.js:650:32)
    at derSerializePrivateKey (/home/gus/repos/awala-keystore-gcp-js/node_modules/@relaycorp/relaynet-core/src/lib/crypto_wrappers/keys.ts:97:38)

The root cause

The following check fails sometimes even though data is indeed an ArrayBuffer:

https://github.com/PeculiarVentures/asn1-schema/blob/988241d5c3c8fa4ef6f581b55b7354330ead0156/packages/schema/src/parser.ts#L23

Here's what the WebStorm debugger shows (which I've double-checked with console.log()):

asn1-1 asn1-2 asn1-3

This behaviour is valid according to:

So the culprit seems to be JS realms, which I'm guessing in the case of Node.js can only happen when asn1-schema is used by multiple libraries in the same process, along with some of the recent changes to @peculiar/webcrypto.

How to reproduce

Unfortunately, it's quite hard to reproduce this as you need a relatively complex dependency tree to reproduce it. But if you want to reliably reproduce this locally, you can run npm test in https://github.com/relaycorp/awala-keystore-cloud-js/pull/13 and check the broken tests.

This is the relevant dependency sub-tree for the repo above:

Potential solution

Check .toString() if instanceof returns false: https://github.com/fengyuanchen/is-array-buffer/blob/9ea7fb638e79f8938161b3b7370cb965d8e93a8b/index.ts#L15

gnarea commented 2 years ago

FYI, I tested the workaround above by patching node_modules and then everything worked as expected:

$ diff node_modules/@peculiar/asn1-schema/build/cjs/parser.js node_modules/@peculiar/asn1-schema/build/cjs/parser.js.backup 
13c13
<         if (data instanceof ArrayBuffer || data.toString() === "[object ArrayBuffer]") {
---
>         if (data instanceof ArrayBuffer) {

So I implemented it properly in #74.

Please let me know what you think.

microshine commented 2 years ago

We have pvtsutils module that implements BufferSourceConverter. It uses isArrayBuffer function which works via string comparison.

I think we should use it. I'll publish update today.

gnarea commented 2 years ago

Thanks @microshine! Yeah that sounds even better.

microshine commented 2 years ago

I've published the latest version of asn1-schema

Successfully published:
 - @peculiar/asn1-adobe-acrobat@2.1.9
 - @peculiar/asn1-android@2.1.9
 - @peculiar/asn1-cert-transparency@2.1.9
 - @peculiar/asn1-cms@2.1.9
 - @peculiar/asn1-csr@2.1.9
 - @peculiar/asn1-ecc@2.1.9
 - @peculiar/asn1-ess@2.1.9
 - @peculiar/asn1-lei@2.1.9
 - @peculiar/asn1-ntqwac@2.1.9
 - @peculiar/asn1-ocsp@2.1.9
 - @peculiar/asn1-pfx@2.1.9
 - @peculiar/asn1-pkcs8@2.1.9
 - @peculiar/asn1-pkcs9@2.1.9
 - @peculiar/asn1-rfc8226@2.1.9
 - @peculiar/asn1-rsa@2.1.9
 - @peculiar/asn1-schema@2.1.9
 - @peculiar/asn1-tsp@2.1.9
 - @peculiar/asn1-x509-attr@2.1.9
 - @peculiar/asn1-x509-logotype@2.1.9
 - @peculiar/asn1-x509-microsoft@2.1.9
 - @peculiar/asn1-x509-netscape@2.1.9
 - @peculiar/asn1-x509-qualified-etsi@2.1.9
 - @peculiar/asn1-x509-qualified@2.1.9
 - @peculiar/asn1-x509-trustanchor@2.1.9
 - @peculiar/asn1-x509@2.1.9
gnarea commented 2 years ago

Thank you so much @microshine! I've just tested the latest version and it all works now. 🎉