homebridge / HAP-NodeJS

Node.js implementation of the HomeKit Accessory Protocol (HAP)
Apache License 2.0
2.69k stars 630 forks source link

Address ChaCha20-Poly1305 Encryption Issue in Electron #1024

Closed seydx closed 8 months ago

seydx commented 8 months ago

:recycle: Current situation

Currently, the hap-nodejs project utilizes Node.js's native crypto module for cryptographic operations, including ChaCha20-Poly1305 encryption. However, when attempting to use hap-nodejs within an Electron environment, developers encounter limitations due to Electron's use of BoringSSL, not OpenSSL.

Electron issue: electron/electron#24024

:bulb: Proposed solution

To address the incompatibility with Electron/BoringSSL, I propose switching from Node.js's crypto module to the @noble/ciphers library for ChaCha20-Poly1305 encryption and decryption. @noble/ciphers is a pure JavaScript implementation of various cryptographic algorithms, making it platform-agnostic and suitable for environments like Electron. This change involves adjusting the chacha20_poly1305_encryptAndSeal and chacha20_poly1305_decryptAndVerify functions within the project to use @noble/ciphers instead of the native crypto module.

Further information about @noble/ciphers can be found here: paulmillr/noble-ciphers.

:gear: Release Notes

This update enables hap-nodejs users to deploy their projects within an Electron environment without encountering issues related to ChaCha20-Poly1305 encryption compatibility with BoringSSL.

:heavy_plus_sign: Additional Information

//

Testing

 PASS  src/lib/util/hapCrypto.spec.ts
  hapCrypto
    chacha20-poly1305
      ✓ should encrypt chacha20-poly1305 correctly (3 ms)
      ✓ should decrypt chacha20-poly1305 correctly
      ✓ should fail verification on manipulated data (13 ms)
    HKDF
      ✓ should calculate test vector correctly
    layer encryption
      ✓ simple encryption and decryption (1 ms)
      ✓ multiple frames encryption and decryption
      ✓ multiple frames encryption and decryption intertwined (1 ms)
      ✓ incomplete frames decryption; first (1 ms)

Test Suites: 1 passed, 1 total
Tests:       8 passed, 8 total
Snapshots:   0 total
Time:        1.252 s, estimated 2 s

Reviewer Nudging

Reviewers should start by examining the changes in the cryptographic functions within hapCrypto.ts, focusing on the integration of @noble/ciphers. A good entry point would be the modified chacha20_poly1305_encryptAndSeal and chacha20_poly1305_decryptAndVerify functions

hjdhjd commented 8 months ago

I would be disinclined to incorporate something like this - moving off of native crypto libraries where possible for the many environments that have them available is a net loss in efficiency, CPU utilization, etc...all to enable a lowest-common-denominator solution like Electron in HAP.

I'm okay with making non-native crypto libraries as an option users can choose to plug in, potentially, but certainly not to default to it. Additionally, using a security library with less than a year's track record in a core library like HAP-NodeJS seems less than wise without a very compelling reason, and I don't think the stated reasons rise to the level of consideration for HAP-NodeJS. It should be noted that the proposed library has not been independently audited or validated by a credible third party.

TL;DR of my view: changing crypto libraries should have a much higher bar than providing support for Electron environments. If users of HAP-NodeJS want to do so, they can create their own private implementations, but we should not rely on unproven/newish/unaudited libraries to provide core cryptography capabilities.

Supereg commented 8 months ago

Couldn't agree more. Thank you @hjdhjd 🚀

seydx commented 8 months ago

@hjdhjd

Thanks for the feedback! I see your points about sticking with native crypto for efficiency and security. Makes sense to me. I'll go ahead and close this PR then 👍🏼

hjdhjd commented 8 months ago

Thanks @seydx - appreciate the thought and contribution to broadening to a wider audience...we just need to work through the best ways to enable those use cases that balance the other tradeoffs.

seydx commented 8 months ago

I'll leave this here in case someone wants to use hap-nodejs in an electron project :)

// crypto.ts

import chacha from 'chacha';
import crypto from 'crypto';

if (!crypto.getCiphers().includes('chacha20-poly1305')) {
  const originalGetCiphers = crypto.getCiphers;
  const originalCreateCipheriv = crypto.createCipheriv;
  const originalCreateDecipheriv = crypto.createDecipheriv;

  crypto.getCiphers = (): string[] => {
    const ciphers = originalGetCiphers();
    ciphers.push('chacha20-poly1305');
    return ciphers;
  };

  crypto.createCipheriv = (algorithm: any, key: any, iv: any, options: any) => {
    if (algorithm !== 'chacha20-poly1305') {
      return originalCreateCipheriv(algorithm, key, iv, options);
    } else {
      return chacha.createCipher(key, iv);
    }
  };

  crypto.createDecipheriv = (algorithm: any, key: any, iv: any, options?: any) => {
    if (algorithm !== 'chacha20-poly1305') {
      return originalCreateDecipheriv(algorithm, key, iv, options);
    } else {
      return chacha.createDecipher(key, iv);
    }
  };
}

And then simply import at the start of the project

// main.ts

import './crypto';

...

So it is up to everyone to change the crypto module without modifying the hap-nodejs module directly