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

First crypto ECDH sign() failes after convertKey() call. #26133

Closed
wdenbakker opened this issue Feb 15, 2019 · 1 comment
Closed

First crypto ECDH sign() failes after convertKey() call. #26133

wdenbakker opened this issue Feb 15, 2019 · 1 comment
Labels
confirmed-bug Issues with confirmed bugs. crypto Issues and PRs related to the crypto subsystem.

Comments

@wdenbakker
Copy link

  • Version: v11.10.0
  • Platform: 64-bit Windows 10
  • Subsystem: crypto

After calling ECDH.convertKey() (on an invalid public key) the next call to sign() fails, but subsequent calls succeed.
Example:

const crypto = require("crypto");

const publicKey = Buffer.from("02ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", "hex");
try {
	crypto.ECDH.convertKey(publicKey, "secp256k1", undefined, undefined, "compressed");
} catch (error) {
	//Lies outside curve, so it should throw.
}

const secp256k1 = crypto.createECDH("secp256k1");
secp256k1.generateKeys();

//Pem format private key
let privateKey = secp256k1.getPrivateKey();
if (privateKey.length < 32) {
	privateKey = Buffer.concat([Buffer.alloc(32 - privateKey.length, 0), privateKey]);
}
const privateStart = Buffer.from("302e0201010420", "hex");
const privateEnd = Buffer.from("a00706052b8104000a", "hex");
const privateKeyPem = "-----BEGIN EC PRIVATE KEY-----\n" +
	Buffer.concat([privateStart, privateKey, privateEnd]).toString("base64") +
	"\n-----END EC PRIVATE KEY-----";

const toSign = "whatever";
try {
	crypto.createSign("SHA256").update(toSign).sign(privateKeyPem);
} catch (error) {
	console.log(error);
	console.log("That threw an error, lets try the same thing again.");
	crypto.createSign("SHA256").update(toSign).sign(privateKeyPem);
	console.log("This time it threw no error.");
}

Resulting error:

Error: error:10067066:elliptic curve routines:ec_GFp_simple_oct2point:invalid encoding
    at Sign.sign (internal/crypto/sig.js:84:29)
bnoordhuis added a commit to bnoordhuis/io.js that referenced this issue Feb 16, 2019
Failed ConvertKey() operations should not leave errors on OpenSSL's
error stack because that's observable by subsequent cryptographic
operations. More to the point, it makes said operations fail with
an unrelated error.

Fixes: nodejs#26133
@bnoordhuis bnoordhuis added confirmed-bug Issues with confirmed bugs. crypto Issues and PRs related to the crypto subsystem. labels Feb 16, 2019
@bnoordhuis
Copy link
Member

Thanks for the good bug report and test case. A fix is in #26153.

@danbev danbev closed this as completed in 042e264 Feb 21, 2019
addaleax pushed a commit that referenced this issue Feb 21, 2019
Failed ConvertKey() operations should not leave errors on OpenSSL's
error stack because that's observable by subsequent cryptographic
operations. More to the point, it makes said operations fail with
an unrelated error.

PR-URL: #26153
Fixes: #26133
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
rvagg pushed a commit that referenced this issue Feb 28, 2019
Failed ConvertKey() operations should not leave errors on OpenSSL's
error stack because that's observable by subsequent cryptographic
operations. More to the point, it makes said operations fail with
an unrelated error.

PR-URL: #26153
Fixes: #26133
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. crypto Issues and PRs related to the crypto subsystem.
Projects
None yet
Development

No branches or pull requests

2 participants