Open gnarea opened 2 years ago
gnarea
commented
2 years ago I'm not having much luck with the Electron or BoringSSL teams. If I can't convince them to add/enable AES-KW, would you guys be open to a PR that uses a pure JS implementation of AES-KW when crypto.getCiphers() doesn't include this AES mode?
This would require adding a dependency on @stablelib/aes-kw or aes-kw, or forking one of them to embed the implementation in @peculiar/webcrypto.
microshine
commented
2 years ago @gnarea I think the simplest way is creating and registering the new AES-KW provider with extra dependency for Electron.
I don't think it's a good solution to import such dependencies to the package if they are required for Electron runtime only
import { Crypto } from "@peculiar/webcrypto";
import * as core from "webcrypto-core";
class AesKwProvider extends core.AesKwProvider {
async onGenerateKey(algorithm, extractable, keyUsages) {
throw new Error("AES-KW: generateKey: Method not implemented");
}
async onExportKey(format, key) {
throw new Error("Method not implemented");
}
async onImportKey(format, keyData, algorithm, extractable, keyUsages) {
throw new Error("Method not implemented");
}
async onEncrypt(algorithm, key, data) {
throw new Error("Method not implemented");
}
async onDecrypt(algorithm, key, data) {
throw new Error("Method not implemented");
}
}
// Register the new AES-KW provider
const crypto = new Crypto();
crypto.subtle.providers.set(new AesKwProvider())
await crypto.subtle.generateKey({ name: "AES-KW", length: 128 }, false, ["wrapKey", "unwrapKey"]);
// Error: AES-KW: generateKey: Method not implementedBut SubtleCrypto.providers is protected field and TypeScript doesn't show it. Looks like we need to make it public.
gnarea
commented
2 years ago @microshine, thanks, that'd work for me. We could even just extend the AesKwProvider class from this project and only override onEncrypt()/onDecrypt(), right?
I can put a PR together to make SubtleCrypto.providers public if you're happy with that.
microshine
commented
2 years ago We could even just extend the AesKwProvider class from this project and only override onEncrypt()/onDecrypt(), right?
It's impossible. @peculiar/webcrypto doesn't export any providers.
I can put a PR together to make SubtleCrypto.providers public if you're happy with that
it would be nice
gnarea
commented
2 years ago It's impossible. @peculiar/webcrypto doesn't export any providers.
True, I didn't realise that class wasn't exported. Would you consider exporting it? Otherwise, I'd have to duplicate most of the code in mechs/aes/aes_kw.ts plus other files like mechs/aes/crypto.ts, etc.
microshine
commented
2 years ago @peculiar/webcrypto uses internal getCryptoKey/setCryptoKey functions which allow to protect private key blobs of NodeJS keys. Without those blobs, you can't export NodeJS keys to the extra crypto module. This is why I think provider exporting is useless
gnarea
commented
2 years ago Sorry, I don't understand why getCryptoKey/setCryptoKey would be problematic with this approach or in Electron. From my testing on Electron, the current AesKwProvider from @peculiar/webcrypto can generate, import and export keys without issues. I also just looked at the code and can't think of a reason why getCryptoKey/setCryptoKey wouldn't work on Electron.
gnarea
commented
2 years ago Actually, I don't think I'd need AesKwProvider to be exported anyway because I could get the original implementation with something like SubtleCrypto.providers.get('aes-kw').
Then I could just proxy onGenerateKey()/onExportKey()/onImportKey() from my own AesKwProvider, so I'd just override onEncrypt() and onDecrypt().
microshine
commented
2 years ago Here are two representations of the one key.
CryptoKey doesn't keep a private blob of NodeJS secret key.
getCryptoKey function allows getting AesCryptoKey with the private NodeJS secret key blob which you can use in AesCrypto class. See AesCryptoKey.data field
Does it make sense?
microshine
commented
2 years ago Here is the package structure
gnarea
commented
2 years ago I see. Thanks @microshine! That's really helpful. I'll get back to this tomorrow or Monday, and I'll make the PR then.
gnarea
commented
2 years ago I don't think it's a good solution to import such dependencies to the package if they are required for Electron runtime only
Coming back to this, what about a PR that only used the pure JS implementation of AES-KW if Node.js doesn't support this cipher?
This is what I did in my project:
export class AwalaCrypto extends Crypto {
constructor() {
super();
const doesNodejsSupportAesKw = getCiphers().includes('id-aes128-wrap');
if (!doesNodejsSupportAesKw) {
// This must be running on Electron, so let's use a pure JavaScript implementation of AES-KW:
// https://github.com/relaycorp/relaynet-core-js/issues/367
const providers = (this.subtle as SubtleCrypto).providers;
const nodejsAesKwProvider = providers.get('AES-KW') as AesKwProvider;
providers.set(new AwalaAesKwProvider(nodejsAesKwProvider));
}
}
}
You'd get an error like this:
This is due to https://github.com/electron/electron/issues/31874
This means that you can't use PKI.js with
EnvelopedDataand DH, for example.I don't think there's anything to change in this repo, but I wanted to create this issue to save some time to anyone that comes across this issue. Feel free to close it.