Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

Should CryptoKeyStorage really be required in v2? #161

Closed
gnarea opened this issue Apr 23, 2020 · 7 comments
Closed

Should CryptoKeyStorage really be required in v2? #161

gnarea opened this issue Apr 23, 2020 · 7 comments
Assignees

Comments

@gnarea
Copy link

gnarea commented Apr 23, 2020

I'm trying to upgrade from v1 to v2 but the following lines in v2 are problematic because they'll create a directory:

constructor(options?: CryptoOptions) {
super();
this.keyStorage = new CryptoKeyStorage(this, options?.directory ?? path.join(os.homedir(), ".node-webcrypto-ossl"));

public constructor(crypto: Crypto, directory: string) {
this.crypto = crypto;
this.directory = path.normalize(directory);
if (!fs.existsSync(this.directory)) {
mkdirp.sync(this.directory);

I'm using this library in a Node.js server where each process should be stateless, so it isn't ideal to have state in the Docker containers. Especially if that state is potentially sensitive.

I'm using this library via PKI.js and I don't have any need to persist data on disk. This worked fine on v1 because the KeyStorage was only created if requested:

constructor(options?: WebCryptoOptions) {
this.subtle = new subtle.SubtleCrypto();
if (options && options.directory) {
this.keyStorage = new KeyStorage(options.directory);
}

I discovered this issue because the upgrade broke the functional tests, since the process shouldn't be allowed to write to the home directory (or any other directory). I can certainly work around that, but before doing that -- are you open to a PR that would only initialise CryptoKeyStorage if a directory is passed to Crypto, like in v1 of this library?

@microshine
Copy link
Contributor

@gnarea thank you for pointing to this issue. I updated crypto and made keyStorage optional.

New version node-webcrypto-ossl@2.0.7 is available

@gnarea
Copy link
Author

gnarea commented Apr 28, 2020

Thank you so much!

@microshine
Copy link
Contributor

Have you seen @peculiar/webcrypto module? It's alternative of node-webcrypto-ossl which is based on NodeJS Crypto API and it doesn't use C++ Addons

@gnarea
Copy link
Author

gnarea commented Apr 28, 2020

I have, but PeculiarVentures/webcrypto#1 is blocking me: The library I wrote to wrap PKI.js/node-webcrypto-ossl is used on the server and will soon be used on an Electron app too.

@rmhrisk
Copy link
Contributor

rmhrisk commented Apr 28, 2020

@gnarea its not clear to me how PeculiarVentures/webcrypto#1 is blocking; it is a discussion thread?

@microshine
Copy link
Contributor

@gnarea We've got a project which uses Electron and @peculiar/webcrypto works fine there

@gnarea
Copy link
Author

gnarea commented Apr 28, 2020

Apologies, I misread that thread. I thought the upshot of the conversation was that @peculiar/webcrypto didn't work in Electron apps. I'll try migrating to @peculiar/webcrypto in that case. Thanks!

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

No branches or pull requests

3 participants