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

encryption: false option? #21

Open
gmaclennan opened this issue May 30, 2023 · 11 comments · May be fixed by #25
Open

encryption: false option? #21

gmaclennan opened this issue May 30, 2023 · 11 comments · May be fixed by #25

Comments

@gmaclennan
Copy link
Contributor

Any interest in adding a { encrypted: false } option? I could work on a PR.

Use cases:

  • Inspecting network traffic to debug code
  • Using secret-stream over an existing encrypted connection

My specific use-case is hypercore replication over a secure websocket (e.g. TLS). The secret-stream handshake to confirm public keys is still useful, but the encryption/decryption adds CPU overhead and is not needed in this use-case.

I understand that this is an edge-case, so I can implement as a project-specific patch if you don't want to add to the API here.

@mafintosh
Copy link
Contributor

if trivial, sure, go for it. Note that hypercore relies on the noise-handshake setup so the only thing you can safely disable is the transport encryption. Also the option should be something like unsafeDisableEncryption: true

@mafintosh
Copy link
Contributor

You would need to impl noops of Push and Pull here so it doesnt impact any of the other code at runtime

@mafintosh
Copy link
Contributor

@av8ta
Copy link

av8ta commented Jan 17, 2024

I have the second use case mentioned above:

Using secret-stream over an existing encrypted connection

It's in a browser environment trying to replicate hypercores and is erroring on the noise handshake.

@gmaclennan did you work on this / advice to give?

@mafintosh is there another solution for replicating hypercores without noise?

@gmaclennan
Copy link
Contributor Author

@gmaclennan did you work on this / advice to give?

No, I haven't yet. So far I've just been piping the whole encrypted noise stream over websocket, but it's still at the prototype stage so I haven't looked at the performance gains from removing the encryption (because the websocket is over TLS).

@gmaclennan gmaclennan linked a pull request Jan 17, 2024 that will close this issue
@gmaclennan
Copy link
Contributor Author

Created a PR to implement this: #25

@av8ta
Copy link

av8ta commented Jan 18, 2024

Thanks @gmaclennan that was quick! I've tried it out your PR in node and it works great. Still not working for me in webview though - it's the noise handshake that's failing as noted by others in this issue #13

@av8ta
Copy link

av8ta commented Jan 18, 2024

Found an issue tracking browser compatibility. There's just one task remaining of 22 tasks!

hypercore-protocol/hypercore-next#53

sodium-friends/sodium-javascript#65

@chm-diederichs is there much involved to get this across the line? 🙏

@chm-diederichs
Copy link

Hey, possibly not too much work. I will take a look when I can, but I'm a little swamped at the moment

I have the methods implemented on this branch here though, if you want to test in the meantime. Just be aware it's an experimental branch.

@gmaclennan
Copy link
Contributor Author

Sorry for the noise, just checking the view of the maintainers on this issue and the implementation in #25. I understand there is the separate question of browser compatibility, but this feature and solution does not need to depend on that.

We can always just patch / fork, but it's easier to maintain into the future for us if this is integrated here. See the PR, there is significant overhead saved by turning off encryption.

@mafintosh
Copy link
Contributor

Hi @gmaclennan . For now rely on the branch. We are pushing a lot of changes through the stack atm with rocks, so dont wanna rock the boat too much and land this while that happens. Feature wise, ok to have the flag and it will land eventually

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

Successfully merging a pull request may close this issue.

4 participants