indutny / asn1.js

ASN.1 Decoder/Encoder/DSL
MIT License
181 stars 64 forks source link

Should/not encode optional null component? #112

Open jablko opened 5 years ago

jablko commented 5 years ago

Should the following output 30020500 or 3000?

console.log(
  asn1
    .define('Foo', function() {
      this.seq().obj(
        this.key('none')
          .null_()
          .optional()
      );
    })
    .encode()
    .toString('hex')
);

OpenSSL (on my machine) chokes on the following:

const crypto = require('crypto');

const asn1 = require('parse-asn1/asn1');

const { privateKey } = crypto.generateKeyPairSync('ec', {
  namedCurve: 'P-256',
  publicKeyEncoding: {
    type: 'spki',
    format: 'pem'
  },
  privateKeyEncoding: {
    type: 'pkcs8',
    format: 'pem'
  }
});
const reencoded = asn1.PrivateKey.encode(
  asn1.PrivateKey.decode(privateKey, 'pem', {
    label: 'PRIVATE KEY'
  }),
  'pem',
  {
    label: 'PRIVATE KEY'
  }
);
crypto
  .createSign('sha256')
  .update('Hello')
  .sign(reencoded);
Error: error:0D078094:asn1 encoding routines:asn1_item_embed_d2i:sequence length mismatch

Substituting .sign(privateKey); for .sign(reencoded); doesn't error. reencoded differs from the original privateKey in an additional null component, which comes from this line in the parse-asn1 AlgorithmIdentifier definition. Because the none component is optional, asn1.js decodes the original privateKey with or without it, but OpenSSL will only decode it without.

If I change asn1.js/lib/asn1/base/node.js line 534 from return child._encode(null, reporter, data); to return child._encode(undefined, reporter, data); then asn1.js no longer outputs the optional null component and OpenSSL accepts the output.

Should asn1.js encode the optional null component?

In actuality I have private keys that aren't ASN.1, e.g. RFC 6605 Section 6.1 P-256 Example and I'm trying to adapt them to crypto.createSign().

Probably Irrelevant

I think the OpenSSL elliptic curve AlgorithmIdentifier encoding/decoding takes place in eckey_priv_encode()/eckey_priv_decode()/eckey_param2type()/eckey_type2param().

I looked around for a standard to see if the parse-asn1 AlgorithmIdentifier definition or OpenSSL implementation are correct. I found lots of them, covering PrivateKeyInfo and AlgorithmIdentifier, but failed to find the definitive ASN.1 for elliptic curves.

s3curitybug commented 5 years ago

The null is necessary for RSA keys, but for ECC keys, most parsers will fail if the null is present. Is there a way we can choose if we want it?

asn1.PublicKey.encode({
    'algorithm': {
        'algorithm': [1,2,840,10045,2,1],
        'curve': [1,2,840,10045,3,1,7]
    },
    'subjectPublicKey': {
        data: [4, 8, 221, 83, 84, 13, 251, 210, 126, 96, 69, 94, 251, 249, 212, 101, 30, 69, 225, 145, 40, 235, 42, 127, 145, 192, 75, 129, 172, 14, 5, 98, 168, 236, 223, 208, 152, 225, 221, 123, 120, 87, 40, 125, 229, 186, 92, 106, 172, 129, 27, 82, 185, 60, 152, 239, 19, 61, 49, 194, 14, 240, 18, 116, 53],
        unused: 0
    }
}, 'pem', { label: 'PUBLIC KEY' })

This outputs this key, which is not parseable by OpenSSL or Java BouncyCastle because of the null:

-----BEGIN PUBLIC KEY-----
MFswFQYHKoZIzj0CAQUABggqhkjOPQMBBwNCAAQI3VNUDfvSfmBFXvv51GUeReGR
KOsqf5HAS4GsDgViqOzf0Jjh3Xt4Vyh95bpcaqyBG1K5PJjvEz0xwg7wEnQ1
-----END PUBLIC KEY-----
java.lang.Exception: Error Performing Parsing java.lang.Exception: java.lang.IllegalArgumentException: Bad sequence size: 3

This generates exactly the same key even if the field 'none' is specified as undefined:

asn1.PublicKey.encode({
    'algorithm': {
        'algorithm': [1,2,840,10045,2,1],
        'none': undefined,
        'curve': [1,2,840,10045,3,1,7]
    },
    'subjectPublicKey': {
        data: [4, 8, 221, 83, 84, 13, 251, 210, 126, 96, 69, 94, 251, 249, 212, 101, 30, 69, 225, 145, 40, 235, 42, 127, 145, 192, 75, 129, 172, 14, 5, 98, 168, 236, 223, 208, 152, 225, 221, 123, 120, 87, 40, 125, 229, 186, 92, 106, 172, 129, 27, 82, 185, 60, 152, 239, 19, 61, 49, 194, 14, 240, 18, 116, 53],
        unused: 0
    }
}, 'pem', { label: 'PUBLIC KEY' })

Is there a way I can choose not to have the null?