Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should/not encode optional null component? #112

Open
jablko opened this issue Jun 30, 2019 · 1 comment
Open

Should/not encode optional null component? #112

jablko opened this issue Jun 30, 2019 · 1 comment

Comments

@jablko
Copy link

jablko commented Jun 30, 2019

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
Copy link

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants