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

[Feature Request] export sigma32_32 from _arx.ts or simply the constant 'expand 32-byte k' that it's derived from #35

Closed
ainsly opened this issue May 6, 2024 · 7 comments

Comments

@ainsly
Copy link

ainsly commented May 6, 2024

Ok hear me out,

I know you mention that you don't provide NaCl crypto_box, understandable since crypto_box / scalarMult is deprecated, but I'm currently working with interoperability with a codebase that is pretty set in stone for the time being.

That codebase uses plain old NaCl crypto_box/sealedbox for it's e2ee, which uses hsalsa20 to derive the shared secret; I noticed you implemented hsalsa so deriving this myself was pretty easy, but hardcoding the sigma constant left a nasty taste in my mouth as I'm sure you'd understand.

I'm currently using your (top-notch btw) noble packages for a bunch of cryptographic operations and a single source of truth given that they've already been audited (bar the ciphers package if I'm correct), and I'm not looking to bloat the repo with an unnecessary implementation of hsalsa20 when it's easily doable myselft and ill-advised. BUT, you'd be doing my ocd a favour with just one little export 😆

If you or anybody stumbling onto this is interested here's how I implemented hsalsa20 using what your suite exports:

/**
   * Returns hashed x25519 sharedSecret / scalarMult for use with encryption methods
   * and interoperability with NaCl box construct
   * Implements crypto_box_beforenm
   * @param {Uint8Array} sharedSecret x25519 sharedSecret / scalarMult
   * @returns {Uint8Array} hashed shared secret using hsalsa20
   */
  getHsalsa20SharedSecret(sharedSecret: Uint8Array): Uint8Array {
    // todo: pull-request to get sigma32_32 or const exported from @noble/ciphers - for now it's safe to hard code
    const sigma = u32(utf8ToBytes('expand 32-byte k'));
    const hashedSharedSecret = new Uint32Array(8);
    const i = new Uint32Array(16);
    hsalsa(sigma, u32(sharedSecret), i, hashedSharedSecret);

    return u8(hashedSharedSecret);
  }

For the record this is purely for the sake of interoperability. And using that snippet I can just plug in the shared secret from x25519.getSharedSecret() and enable communicating with any system still using NaCl boxes and not using a custom HKDF without a hitch.

Please don't ask my why hardcoding 'expand 32-byte k' is such a gripe for me 😅. But a simple export of the constant or sigma32_32 would be a lifesaver.

Happy to put in a quick pull request if either of those options is ok with you? If not it's not the end of the world and I'd totally understand. Just let me know your thoughts!

Also, a little aside here.... have you ever though about implementing libsodium's secretstream?

One of xchacha20-poly1305's shortcomings (whilst also a benefit) is that it is a stream cipher, so when it comes to large files... browser memory becomes an issue. I'm currently looking for a way to encrypt / decrypt in chunks (as secretstream does) and pipe it to a writable stream (for encrypting & uploading on-the-fly), or (currently only supported by Chrome it seems with their FileSystemAPI) download and decrypt in chunks directly to the browsers download queue; just like a regular download, but without holding an entire file in memory and having to decrypt the whole thing and potentially wreaking all kinds of havoc... Currently just using WebCryptoAPI AES-GCM for file encryption, which works fine in most cases, but as files get larger so does the memory requirements; had to limit mobile devices max file size already.

I did start working with libsodium.js specifically for that method but the ugly emscripten build was waaaay too big for production and not at all tree-shakeable.

Anyways, if it's something that you'd be interested in, just let me know, I'd be more than happy discuss and to contribute! Looks like your noble suite has all the right primitives in place already to implement such a cool feature - even if not specifically in this package (I totally understand unwanted bloat). Just let me know what you think 🙏🏽

@paulmillr
Copy link
Owner

5bce492

@paulmillr
Copy link
Owner

0.5.3

@paulmillr
Copy link
Owner

as for secretstream - i don't think it's very popular, but documenting a way to encrypt huge files would be useful for README.

@ainsly
Copy link
Author

ainsly commented May 6, 2024

Quick turnaround dude, thanks!

And you're right about secretstream, only discovered it not too long ago. The motivation was less about huge files and more about configurable chunking to counteract the limitations of the stream cipher, although that's definitely a plus.

But I'll hopefully be able to get around to an implementation of it using noble for an entry in the readme. Will keep you posted 🙏🏽 thanks again for that quick release

@ainsly
Copy link
Author

ainsly commented May 17, 2024

@paulmillr not sure how esbuild didn't pick this up before, but _arx isn't set in the package exports...

The path "./_arx" is not exported by package "@noble/ciphers":

    node_modules/@noble/ciphers/package.json:35:13:
      35 │   "exports": {
         ╵              ^

  You can mark the path "@noble/ciphers/_arx" as external to exclude it from the bundle, which will remove this error and leave the unresolved path in the bundle.

Not sure you're too keen on exposing too much to the public api, seeing as though the file's marked as pseudo private with the _ prefix.

But on the other hand it would expose the internal createCipher method to enable more customised usage.

Or could just re-export from the utils export, maybe aliased to not cause confusion.

Either way is fine with me, but depends what you think is best.

If neither option is good it's fine I can just switch back to constant, seeing as though it literally is just a constant.. guess it was just a QOL improvement.

LMK your thoughts

@paulmillr
Copy link
Owner

«But on the other hand it would expose the internal createCipher method to enable more customised usage»

what’s the use case?

@ainsly
Copy link
Author

ainsly commented Aug 17, 2024

Sorry for delayed response. I can't remember my initial thoughts on this, but I think I was trying to suggest that it would allow people to implement customised ciphers, but looking at it now that was a stupid idea.

You got it all covered; you can revert this change now since it's inaccessible from exports anyway and totally a bad idea to tamper with these internals.

Appreciate your help nonetheless though 🙏 but yea, best to keep the minimal amount of internals exposed.

My use case was purely for interoperability. I bet you were even hesitant to create a wrapper for secretbox in the first place, but that's surely made adoption easier for people.

And always best to make the usage of these complex structs foolproof to minimise damage people can inflict on themselves, I see where your head's at. Props to you for creating an awesome set of libraries, they deserve way more attention IMO.

If people were interested in a way for this lib to interact with older crypto_box libs I'm sure they'll stumble on this thread. But for anyone reading in future -- best to avoid using hsalsa20 and use HKDF for unique keys; and at the very least use the "sealed box" implementation using ephemeral keys, which prevents a static shared secret and both parties being able to decrypt data, but then you sacrifice the ability to verify the sender..

Feel free to correct any inaccuracies... I'm still learning 😓

I could always make a small PR for the readme detailing interop with other libs.. but again I don't want to bloat anything

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

No branches or pull requests

2 participants