| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
For the record, I'm okay with that. |
Sorry, something went wrong.
|
Are there alternative suggestions for the option name (dsaEncoding) or its values (der / ieee-p1363)? Has someone been able to find a different standard that has been around for some time and mentions this format? (WebCrypto / JOSE / COSE mention it, but they are rather new and certainly unrelated to older frameworks that use this format.) |
Sorry, something went wrong.
|
@tniessen is there any room for refactoring to improve the performance? From my benchmark testing using something like this yields a couple hundred more ops/s than the built-in implementation from this PR. The difference is small and possibly within the margin of error but my expectation was that built in would perform better. sign ieee-p1363 via dsaEncoding x 30,310 ops/sec ±0.74% (90 runs sampled) sign ieee-p1363 via module x 30,898 ops/sec ±0.95% (91 runs sampled) Fastest is sign ieee-p1363 via module verify ieee-p1363 via dsaEncoding x 12,875 ops/sec ±0.43% (92 runs sampled) verify ieee-p1363 via module x 12,956 ops/sec ±0.40% (94 runs sampled) Fastest is verify ieee-p1363 via module |
Sorry, something went wrong.
|
@panva I'm looking into it. I can see a < 5% performance loss with dsaEncoding: 'ieee-p1363' when compared to dsaEncoding: 'der' for very small payloads. The new function ConvertSignatureToP1363 seems to use about 5% of the CPU time, with the majority (> 3%) coming from ASN1_item_d2i. My best guess right now is that the performance loss is due to the OpenSSL ASN.1 implementation and a couple of extra allocations. If that is true, I am not sure if there is anything we can do. |
Sorry, something went wrong.
Sorry, something went wrong.
|
I don't see much room for improvement right now. @bnoordhuis, @danbev, @jasnell are you in favor of landing this as it is, or would you prefer to investigate further before adding this semver-minor feature? We could most likely improve the performance by implementing the conversion ourselves instead of relying on OpenSSL, but I don't see much value in that. If a manual JS implementation is faster than OpenSSL, that might be an argument against landing this in core. |
Sorry, something went wrong.
There was a problem hiding this comment.
It'd be nice to use automatic memory management for this. (Grep for DeleteFnPtr in node_crypto.h for examples.)
Ditto line 4913.
Sorry, something went wrong.
There was a problem hiding this comment.
I'm not super fond of type coercion. You save a few keystrokes but introduce new opportunities for subtle bugs.
Sorry, something went wrong.
|
Should this remain open? |
Sorry, something went wrong.
|
I am not sure, I didn't get a reply to #29292 (comment). |
Sorry, something went wrong.
|
@tniessen That question is about the performance of the implementation, not the API? If that's the case, I'm okay with landing it as-is. |
Sorry, something went wrong.
This adds a test case for the dsaEncoding option using data generated in Chrome 76.
Sorry, something went wrong.
Sorry, something went wrong.
PR-URL: #29292 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #29292 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #29292 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #29292 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Notable changes: New assert APIs The `assert` module now provides experimental `assert.match()` and `assert.doesNotMatch()` methods. They will validate that the first argument is a string and matches (or does not match) the provided regular expression This is an experimental feature. Ruben Bridgewater [#30929](#30929). Advanced serialization for IPC The `child_process` and `cluster` modules now support a `serialization` option to change the serialization mechanism used for IPC. The option can have one of two values: * `'json'` (default): `JSON.stringify()` and `JSON.parse()` are used. This is how message serialization was done before. * `'advanced'`: The serialization API of the `v8` module is used. It is based on the HTML structured clone algorithm. and is able to serialize more built-in JavaScript object types, such as `BigInt`, `Map`, `Set` etc. as well as circular data structures. Anna Henningsen [#30162](#30162). CLI flags The new `--trace-exit` CLI flag makes Node.js print a stack trace whenever the Node.js environment is exited proactively (i.e. by invoking the `process.exit()` function or pressing Ctrl+C). legendecas [#30516](#30516). ___ The new `--trace-uncaught` CLI flag makes Node.js print a stack trace at the time of throwing uncaught exceptions, rather than at the creation of the `Error` object, if there is any. This option is not enabled by default because it may affect garbage collection behavior negatively. Anna Henningsen [#30025](#30025). ___ The `--disallow-code-generation-from-strings` V8 CLI flag is now whitelisted in the `NODE_OPTIONS` environment variable. Shelley Vohr [#30094](#30094). New crypto APIs For DSA and ECDSA, a new signature encoding is now supported in addition to the existing one (DER). The `verify` and `sign` methods accept a `dsaEncoding` option, which can have one of two values: * `'der'` (default): DER-encoded ASN.1 signature structure encoding `(r, s)`. * `'ieee-p1363'`: Signature format `r || s` as proposed in IEEE-P1363. Tobias Nießen [#29292](#29292). ___ A new method was added to `Hash`: `Hash.prototype.copy`. It makes it possible to clone the internal state of a `Hash` object into a new `Hash` object, allowing to compute the digest between updates. Ben Noordhuis [#29910](#29910). Dependency updates libuv was updated to 1.34.0. This includes fixes to `uv_fs_copyfile()` and `uv_interface_addresses()` and adds two new functions: `uv_sleep()` and `uv_fs_mkstemp()`. Colin Ihrig [#30783](#30783). ___ V8 was updated to 7.8.279.23. This includes performance improvements to object destructuring, RegExp match failures and WebAssembly startup time. The official release notes are available at https://v8.dev/blog/v8-release-78. Michaël Zasso [#30109](#30109). New EventEmitter APIs The new `EventEmitter.on` static method allows to async iterate over events. Matteo Collina [#27994](#27994). ___ It is now possible to monitor `'error'` events on an `EventEmitter` without consuming the emitted error by installing a listener using the symbol `EventEmitter.errorMonitor`. Gerhard Stoebich [#30932](#30932). ___ Using `async` functions with event handlers is problematic, because it can lead to an unhandled rejection in case of a thrown exception. The experimental `captureRejections` option in the `EventEmitter` constructor or the global setting change this behavior, installing a `.then(undefined, handler)` handler on the `Promise`. This handler routes the exception asynchronously to the `Symbol.for('nodejs.rejection')` method if there is one, or to the `'error'` event handler if there is none. Setting `EventEmitter.captureRejections = true` will change the default for all new instances of `EventEmitter`. This is an experimental feature. Matteo Collina [#27867](#27867). Performance Hooks are no longer experimental The `perf_hooks` module is now considered a stable API. legendecas [#31101](#31101). Introduction of experimental WebAssembly System Interface (WASI) support A new core module, `wasi`, is introduced to provide an implementation of the [WebAssembly System Interface](https://wasi.dev/) specification. WASI gives sandboxed WebAssembly applications access to the underlying operating system via a collection of POSIX-like functions. This is an experimental feature. Colin Ihrig [#30258](#30258). PR-URL: #31691
Notable changes: New assert APIs The `assert` module now provides experimental `assert.match()` and `assert.doesNotMatch()` methods. They will validate that the first argument is a string and matches (or does not match) the provided regular expression This is an experimental feature. Ruben Bridgewater [#30929](#30929). Advanced serialization for IPC The `child_process` and `cluster` modules now support a `serialization` option to change the serialization mechanism used for IPC. The option can have one of two values: * `'json'` (default): `JSON.stringify()` and `JSON.parse()` are used. This is how message serialization was done before. * `'advanced'`: The serialization API of the `v8` module is used. It is based on the HTML structured clone algorithm. and is able to serialize more built-in JavaScript object types, such as `BigInt`, `Map`, `Set` etc. as well as circular data structures. Anna Henningsen [#30162](#30162). CLI flags The new `--trace-exit` CLI flag makes Node.js print a stack trace whenever the Node.js environment is exited proactively (i.e. by invoking the `process.exit()` function or pressing Ctrl+C). legendecas [#30516](#30516). ___ The new `--trace-uncaught` CLI flag makes Node.js print a stack trace at the time of throwing uncaught exceptions, rather than at the creation of the `Error` object, if there is any. This option is not enabled by default because it may affect garbage collection behavior negatively. Anna Henningsen [#30025](#30025). ___ The `--disallow-code-generation-from-strings` V8 CLI flag is now whitelisted in the `NODE_OPTIONS` environment variable. Shelley Vohr [#30094](#30094). New crypto APIs For DSA and ECDSA, a new signature encoding is now supported in addition to the existing one (DER). The `verify` and `sign` methods accept a `dsaEncoding` option, which can have one of two values: * `'der'` (default): DER-encoded ASN.1 signature structure encoding `(r, s)`. * `'ieee-p1363'`: Signature format `r || s` as proposed in IEEE-P1363. Tobias Nießen [#29292](#29292). ___ A new method was added to `Hash`: `Hash.prototype.copy`. It makes it possible to clone the internal state of a `Hash` object into a new `Hash` object, allowing to compute the digest between updates. Ben Noordhuis [#29910](#29910). Dependency updates libuv was updated to 1.34.0. This includes fixes to `uv_fs_copyfile()` and `uv_interface_addresses()` and adds two new functions: `uv_sleep()` and `uv_fs_mkstemp()`. Colin Ihrig [#30783](#30783). ___ V8 was updated to 7.8.279.23. This includes performance improvements to object destructuring, RegExp match failures and WebAssembly startup time. The official release notes are available at https://v8.dev/blog/v8-release-78. Michaël Zasso [#30109](#30109). New EventEmitter APIs The new `EventEmitter.on` static method allows to async iterate over events. Matteo Collina [#27994](#27994). ___ It is now possible to monitor `'error'` events on an `EventEmitter` without consuming the emitted error by installing a listener using the symbol `EventEmitter.errorMonitor`. Gerhard Stoebich [#30932](#30932). ___ Using `async` functions with event handlers is problematic, because it can lead to an unhandled rejection in case of a thrown exception. The experimental `captureRejections` option in the `EventEmitter` constructor or the global setting change this behavior, installing a `.then(undefined, handler)` handler on the `Promise`. This handler routes the exception asynchronously to the `Symbol.for('nodejs.rejection')` method if there is one, or to the `'error'` event handler if there is none. Setting `EventEmitter.captureRejections = true` will change the default for all new instances of `EventEmitter`. This is an experimental feature. Matteo Collina [#27867](#27867). Performance Hooks are no longer experimental The `perf_hooks` module is now considered a stable API. legendecas [#31101](#31101). Introduction of experimental WebAssembly System Interface (WASI) support A new core module, `wasi`, is introduced to provide an implementation of the [WebAssembly System Interface](https://wasi.dev/) specification. WASI gives sandboxed WebAssembly applications access to the underlying operating system via a collection of POSIX-like functions. This is an experimental feature. Colin Ihrig [#30258](#30258). PR-URL: #31691
| Back | FazBrowse Home | New Git URL |
This patch adds support for "IEEE-P1363 signatures" for DSA and ECDSA. If you are thinking "What is that and why doesn't it have a proper name?", you are right to think that.
Currently, OpenSSL produces ASN.1 signatures which are encoded as DER, basically encoding a SEQUENCE of two INTEGER components, r and s. That works nicely and is supported by most crypto APIs including OpenSSL, pycryptodome, Crypto++, JCE, and BouncyCastle. Except, of course, WebCrypto (and .NET).
WebCrypto (and .NET) use an alternative signature format that does not have a name as far as I can tell. It was, however, suggested in IEEE P1363 in 2000, so it is not an entirely new invention, and some frameworks such as Crypto++ and pycryptodome support it. It is stupidly simple, basically just the concatenation of r and s after some padding.
The conversion can also be done in JavaScript and it isn't difficult, an example implementation is in nodejs/webcrypto#18, but since it involves some simple DER / ASN.1 parsing, it would be convenient and more efficient to put it into core. There are other frameworks that support both formats, e.g., pycryptodome and Crypto++.
After giving this a lot of thought, I have a slight preference towards adding this to core, but I would be happy to be convinced of the opposite. And I am totally open to other names for the format than ieee-p1363.
cc @nodejs/crypto @nodejs/security-wg @panva
Checklist