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

Cryptographical choices for protocol security - recap & rationale #109

Closed
jakubtrnka opened this issue Nov 15, 2024 · 10 comments
Closed

Cryptographical choices for protocol security - recap & rationale #109

jakubtrnka opened this issue Nov 15, 2024 · 10 comments

Comments

@jakubtrnka
Copy link
Collaborator

This is not necessarily an issue about something broken, but I'd like to recap and validate the core ideas:

The sv2 uses noise protocol for encryption
https://github.com/stratum-mining/sv2-spec/blob/main/04-Protocol-Security.md

Namely it uses Noise_NX_Secp256k1+EllSwift_ChaChaPoly_SHA256 variant.

I know it should've happened earlier, but I'd like to challenge this choice.

First, this is a nonstandard curve choice. Noise standard suggest using curves either x25519 or x448.
Those are curves that have library support in frameworks such as snow in rust (https://github.com/mcginty/snow), dissononce in python (https://github.com/tgalal/dissononce), noise-c in c (https://github.com/rweather/noise-c)

We decided to use secp256k1. It's not forbidden, but we needed to implement it manually. And anyone who wants to use sv2 needs to do as well.

Also there comes a problem with encoding the public keys that are communicated over a network. We decided that we use Elligator swift technology. Again, it's an advanced nonstandard feature. And since everybody needs to implement it themselves. It also requires a deep understanding of the noise protocol and underlying cryptography in order to be done correctly. Things like key parity, correct order of elligators in order to calculate ECDH correctly. I think it's pretty difficult to get. What is an elligator anyways? We all used the rust wrapper for the c-library secp256k1. But that's not a pure implementation. It bears signature of BIP324 here. So what should for example people building the pool backend in python use? secp256k1-py? There is no elligator support yet. If I try to put myself into somebodys place who just wants to try it out. I think I would give up pretty quickly.

I know we had some discussions about roles being implemented in bitcoind and wanted to make things somehow aligned with bitcoin core. But on the other hand I don't think this is a relevant and objective argument. Core doesn't decide what's best for stratum v2 and its adoption. Their situation is different.
Or what was the reason for this choise exactly? There was an argument that elligators may make initial handshake public key look uniformly random -> privacy benefit.
Although it looks nice at first, I don't think it's necessary. Making mining private by making the initial handshake public keys harder to parse as a EC pubkey is in my opinion absurd. The entire session has a clear mining footprint - even though it's encrypted. Obscuring the mining entirely is beyond the original mission of SV2.

To sum it up, I just wanted to ask:
Is this what we all still agree on? Is this what we think is the best for stratum-v2?

What if I mildly suggested to just step back a little bit and use normally srandard noise NX protocol with x25519.
Wouldn't it make life easier?
For example writing an session initializer (handshake) for stratum-v2 client is then matter 50 lines of code. half of this example - which implements both sides

for python also quite easy: https://github.com/tgalal/dissononce/blob/master/examples/patterns/Noise_XX_25519_AESGCM_SHA256.py

Can SRI afford to reconsider this core property of the specification and make a breaking change at this point?

I don't think it would be too difficult. Mostly work on the specification - rewriting all the tiny details. Implementation should be just a simplification, I believe.

What if we gave it at least a try? And then choose the better option?

Thanks for your opinions.
Jakub T.

@Fi3
Copy link
Contributor

Fi3 commented Nov 15, 2024

And anyone who wants to use sv2 needs to do as well.

That's not true it can use the SRI library.

@Fi3
Copy link
Contributor

Fi3 commented Nov 15, 2024

So what should for example people building the pool backend in python use? secp256k1-py? There is no elligator support yet. If I try to put myself into somebodys place who just wants to try it out. I think I would give up pretty quickly.

We can offer binding to SRI instead of changing again it.

@Fi3
Copy link
Contributor

Fi3 commented Nov 15, 2024

Sadly I don't have time now to go into the merits of this proposal. The only thing that I can say is that if we keep changing the protocol, we will never have something to use.

@TheBlueMatt
Copy link

First of all I think its much too late to be rethinking this. Things are out in the wild where we'll have to consider compatibility and making everyone overhaul their code is yet more delay on getting things actually used, which is at a critical juncutre today.

As to the questions about use of Elligator, I don't really see the argument that secp256k1-py hasn't upgraded yet as a strong one. Indeed, maybe the use of Elligator forces people to use secp256k1 bindings for their language rather than some native crap, but that's universally a good thing - no other secp256k1 curve implementation is anywhere near the quality of the secp256k1 library.

As for whether uniformly random data is worth it, I strongly, strongly disagree. Censorship technology differs greatly from country to country, and simple DPI which can block traffic with specific bytes is fairly common, while blocking traffic based on more advanced heuristics (like traffic patterns) or async IP detection much less common. While you're correct that the traffic pattern has a fairly detectable fingerprint, that is something we can and should fix later with padding, something which shouldn't break existing code and allows us to make bitcoin mining much more censorship-resistant, a very important property.

If we do want to change the algorithm used for availability of implementation reasons, I'd strongly, strongly suggest we simply use the BIP 324 transport. That is likely to have many implementations built over the course of the next few years, was designed explicitly to allow for block-evasion, and also has the nice side-effect of Bitcoin Core already supporting it.

@jakubtrnka
Copy link
Collaborator Author

Thanks for responses.

It's not that I want to change it again. Indeed I don't. I get that it would be quite hard to change at this point.

I'm trying to look at the reality. Most people use ordinary plaintext stratum because it's simple. Sometimes stratum with TLS.

And we are arguing whether the initial message is a serialized elligator or serialized curve point. I'd like to see where this makes any difference.

"no other secp256k1 curve implementation is anywhere near the quality of the secp256k1" - How do you know that?
Using secp256k1 lib caused my executable binary to inflate by 1 MB. Perhaps I'd prefer a native crap that doesn't do that on embedded devices.

I think simplicity, ease of use and entry barrier matters quite a lot.
I'd like to see any 3rd party firmware developer reading this document and implement native sv2-support.
Also I think it's overly optimistic to think that everybody will just take SRI code and use that somehow.
Has anybody ever done that? Does any miner anywhere have a native sv2 support using SRI code? Does any commercial pool have that?

@Fi3
Copy link
Contributor

Fi3 commented Nov 15, 2024

Using secp256k1 lib caused my executable binary to inflate by 1 MB

no idea of what is in there but maybe you can open an issue to make it smaller if this is a blocker?

@jakubtrnka
Copy link
Collaborator Author

This with the executable size was actually the original motivation for opening this issue.

I'll try looking for solutions. But it costs me time and energy.

I just wanted to make sure what the consensus is. It's fair to say "we are fine with this current form of sv2-spec". We can then close this issue as irrelevant.

@Fi3
Copy link
Contributor

Fi3 commented Nov 15, 2024

This with the executable size was actually the original motivation for opening this issue.

I'll try looking for solutions. But it costs me time and energy.

I just wanted to make sure what the consensus is. It's fair to say "we are fine with this current form of sv2-spec". We can then close this issue as irrelevant.

yep I understand, I would vote for closing it, but let's wait a little bit. Maybe if in the meantime you can open an issue on the relevant repo for the binary size who know maybe is something trivial

@TheBlueMatt
Copy link

I'm trying to look at the reality. Most people use ordinary plaintext stratum because it's simple. Sometimes stratum with TLS.

Yes, building things the right way to ensure that properties that matter are honored is hard. We're not gonna take stupid shortcuts just because its hard.

"no other secp256k1 curve implementation is anywhere near the quality of the secp256k1" - How do you know that?

Because I know the amount of hours that went into building secp256k1....and I've seen time and time and time again other ones have stupid bugs. There's other good crypto libraries, sure, but definitely nothing with even 1/10th the hours put into just the secp256k1 implementation in it, let alone from great engineers.

Using secp256k1 lib caused my executable binary to inflate by 1 MB. Perhaps I'd prefer a native crap that doesn't do that on embedded devices.

Have you looked into LTO?

I'd like to see any 3rd party firmware developer reading this document and implement native sv2-support.

Luckily it doesn't really matter all that much whether firmware supports Sv2 or not. The advantages of Sv2 come about when you're using a proxy anyway, so the downstream protocol basically doesn't matter. But, yes, hopefully SRI can provide this, and its not crazy to think it can...

@Sjors
Copy link
Contributor

Sjors commented Dec 16, 2024

First, this is a nonstandard curve choice

IIUC the cryptographic primitives for sv2 were initially chosen to make life easier for direct Bitcoin Core integration of the Template Provider. Since most likely this will use a sidecar approach instead, that's not really an argument anymore. However it seems a bit late to change this.

Noise standard suggest using curves either x25519 or x448.
Those are curves that have library support in frameworks such as snow in rust

Libsecp is already available in Rust through projects like rust-bitcoin, and SRI uses it. c and c++ can also use the libsecp library easily. Micropython support is there as well IIUC, see https://github.com/cryptoadvance/specter-diy

It's true that you still need to implement the rest of the noise framework, although the SRI crates are reusable for Rust folks, see e.g. stratum-mining/stratum#1238

The c++ noise code I wrote for the Template Provider could be extracted into a library if someone else needs it (and knows how to make a library).

Also there comes a problem with encoding the public keys that are communicated over a network. We decided that we use Elligator swift technology. Again, it's an advanced nonstandard feature. And since everybody needs to implement it themselves. It also requires a deep understanding of the noise protocol and underlying cryptography in order to be done correctly.

We all used the rust wrapper for the c-library secp256k1. But that's not a pure implementation.

People should use libsecp for this, unless there's a really good reason not to. Why does it need to be "pure"? Do the noise libraries you mention above have pure rust implementation for the curves they use? And well are those reviewed?

@TheBlueMatt wrote:

As to the questions about use of Elligator, I don't really see the argument that secp256k1-py hasn't upgraded yet as a strong one. Indeed, maybe the use of Elligator forces people to use secp256k1 bindings for their language rather than some native crap, but that's universally a good thing - no other secp256k1 curve implementation is anywhere near the quality of the secp256k1 library.

Completely agree.

If we do want to change the algorithm used for availability of implementation reasons, I'd strongly, strongly suggest we simply use the BIP 324 transport.

Unfortunately there's still authentication extension for BIP 324.

Using secp256k1 lib caused my executable binary to inflate by 1 MB. Perhaps I'd prefer a native crap that doesn't do that on embedded devices.

That sounds like something that can be improved upstream. Can you open issues for that? libsecp is written in C and there's other projects like Core Lightning that rely on it and want to function on low end hardware.

Keeping the footprint small also sounds like a good reason to manually implement the noise protocol rather than rely on an omnibus library.

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

4 participants