Skip to content

Instantly share code, notes, and snippets.

Embed
What would you like to do?
@LLFourn
Copy link

LLFourn commented Aug 18, 2021

Comments on the new text (which are mostly the same as the old but less speculative):

ODD secp256k1 public keys MUST be used (public keys starting with 0x03). If the public key from the generated ephemeral key is an EVEN public key (starting with 0x02), its private key SHOULD be negated and then recalculated. Only using ODD public keys makes it more complex to identify the handshake based by analyzing the traffic and looking for 33 bytes that start with 0x02 or 0x03. This in turn increases the cost of censorship.

I maintain it would be better if this was EVEN instead of ODD because that's what's done in BIP340 and it doesn't make sense to introduce a new x-only key that behaves differently to the existing one. BIP340 should be referenced but unfortunately it doesn't look like you can explicitly reference any particular bit of it because it doesn't seem to have a notion of key-pair generation like this document does (which works well here IMO so I would keep it). For me, the justification for only sending 32 byte keys can safely be omitted since it sounds odd to justify why you're not sending a useless byte when that is now more established practice with BIP340.

Two separate cipher instances are used here so as to keep the packet lengths confidential (best effort; for passive observing) but not create an oracle for the packet payload cipher by decrypting and using the packet length prior to checking the MAC. By using an independently-keyed cipher instance to encrypt the length, an active attacker seeking to exploit the packet input handling as a decryption oracle can learn nothing about the payload contents or its MAC (assuming key derivation, ChaCha20 and Poly1305 are secure). Active observers can still obtain the message length (ex. active ciphertext bit flipping or traffic semantics analysis)

I still find this explanation very unhelpful. The only purpose of the length field is to find the end of the payload and check the MAC so why would you use the payload length for anything "prior to checking the MAC". If we didn't have MACs there would be no need for length fields at the transport encryption layer at all afaict -- checking the MAC is its sole purpose. I think the dual stream design is justifiable because it's easier to implement with two streams and easier to get an intuition for the security without having to think carefully about it.

Thanks for working on this @dhruv I think I will have to implement myself to offer further feedback.

@dhruv
Copy link
Author

dhruv commented Aug 23, 2021

Thank you for the review @LLFourn!

I maintain it would be better if this was EVEN instead of ODD because that's what's done in BIP340 and it doesn't make sense to introduce a new x-only key that behaves differently to the existing one.

Agreed. Updated.

For me, the justification for only sending 32 byte keys can safely be omitted

I'd like to keep that in case any reader is unaware of BIP340 specifics.

I still find this explanation very unhelpful.

I completely agree. It's the one paragraph I trip over again and again. I've updated it to:

Even with end-to-end encryption, the Bitcoin p2p protocol can be fingerprinted (using public information like new blocks) and an MITM attacker should be assumed to have knowledge of the plaintext corresponding to the encrypted length. This can lead to malleability in the encrypted length portion of the ciphertext since it is used prior to checking the MAC. We use two cipher instance to avoid any consequences of such malleability leaking into the payload portion of the ciphertext. The two cipher instances make it easier to reason about the security of the suite. The cost in creating additional ChaCha20 and Poly1305 instances is very low.

@real-or-random
Copy link

I think there are still some bigger open questions and this needs a long way. Unfortunately, noone really had the time to contribute, so it's really nice to have @dhruv push the BIP forward now. :)

Here are some points:

  • Is it a goal to hide that this is the Bitcoin protocol? It's not stated as a goal but the text uses this as justification for 32-byte keys. But anyway, this is not effective: Even an attacker who ignores ports, timing etc, can just check for evenness of the 32-bytes. If we want to counter the guaranteed evenness, we could do so using the Elligator Squared techniques. But before we can design a proper protocol that achieves the desired objectives, we need to be clear about the objectives.
  • Post-quantum key exchange: this was discussed a few years ago. We could add a post-quantum key exchange to make sure confidentiality holds up even after an attacker has a quantum computer and can break old recorded connections. That may be overkill (this was my position a few years ago) but connections are long-lived and we don't really care about performance of connection establishment, so this may be a valid thing to do.
  • No matter if we want to integrate the post-quantum key exchange, we should think about extensibility. If we don't integrate that now, what is the proper way to add this or other features in the future -- without having fallback attacks? (TLS has done a lot here and I discussed this a while ago with @sipa but I don't think I can remember all of the details).
  • This BIP is about encryption only. We should design it carefully such that authentication is possible in an established encryption session. I think the BIP achieves this with the session ID but we need to look at this. (This is stated as a goal: "Peer operators can compare encryption session IDs").
  • (I'm sure there's more.)

I suggest we create a repo for it (can be private for now) because this would make discussion much easier: We can have separate issues. And even more important, we can have PRs, which makes it much easier to make updates and improve wording etc.

@sipa
Copy link

sipa commented Aug 24, 2021

I agree with creating a repository, so separate aspects can be discussed separately.

FWIW, I've discussed the idea of a generic extension/feature negotiation mechanism after the ECDH step, and before starting the application level protocol with @dhruv. It's probably easiest to work on that as part of a repository too.

I have some thoughts on @real-or-random's discussion points, but I'll keep them for discussing them as issues if we go that route, to avoid spreading everything out.

@dhruv
Copy link
Author

dhruv commented Aug 25, 2021

I've created a repo for now to get the large outstanding questions under discussion. Once we are in reasonable shape, I will update again here or make that repo public.

@dhruv
Copy link
Author

dhruv commented Oct 8, 2021

Pushed revision 92 removing the BIP 61 REJECT p2p message which is no longer used.

@GeneFerneau
Copy link

GeneFerneau commented Nov 5, 2021

On rekey, it may help strengthen entropy to feed the last 32 bytes of keystream into the HKDF:

... The IV is initialized to 0 and incremented on every re-key event.

k0 = key
iv = 0
k0 = HKDF_EXPAND(prk=k0, hash=SHA256, info="BitcoinK_Rekey", L=32)
ks0, k1 = ChaCha20DRBG(k0, iv)[0:4064], ChaCha20DRBG(k0, iv)[4064:4096]
iv = iv + 1
k1 = HKDF_EXPAND(prk=k1, hash=SHA256, info="BitcoinK_Rekey", L=32)
ks1, k2 = ChaCha20DRBG(k1, iv)[0:4064], ChaCha20DRBG(k1, iv)[4064:4096]
iv = iv + 1
k2 = HKDF_EXPAND(prk=k2, hash=SHA256, info="BitcoinK_Rekey", L=32)
ChaCha20Forward4064DRBG(key) = ks0 || ks1 || ks2 || ...

@dhruv
Copy link
Author

dhruv commented Oct 7, 2022

We now have a bips repo PR and will continue community engagement there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment