BOLT 8: use an alternate ECDH function

[sstone]

If we change the definition of our ECDH function to return the actual point in compressed format, I believe that we should be able to reuse existing Noise implementations without any modifications.

The actual code that I've used is there: https://github.com/sstone/noise/tree/wip-secp256k1.

[cdecker]

Is there actually any advantage in using the point/coordinate in an unhashed fashion? libsecp256k1 returns the sha256 hash of the compressed representation of the point instead, so maybe we could use that?

[sstone]

I think so: if we return the actual point then we can just use the output of the Write() construct as defined in the Noise specs and feed it to the Read() construct.

[rustyrussell]

Yech, I implemented this wrong too, and used libsecp's ECDH function, which as @cdecker points out, doesn't do either!

Since everyone has should be probably eventually be using libsecp256k1, even if via language wrappers, my preference would be to use its ECDH function, which is:

 SHA256(compressed-point)

Let's see if I can pull @apoelstra into this conversation...

[rustyrussell]

gmaxwell says on sepc256k1 (https://botbot.me/freenode/secp256k1/2016-12-09/?msg=77695797&page=1 )

gmaxwell: Returning the point is philosophically at odds with the design of our interface. Doing so would be unsafe by default and we've seen broken cryptography created by people directly using the point arising from ECDH for things. (e.g. the electrum original 'message encryption' feature).
gmaxwell: I believe we discussed taking a hasher function, similar to how we do nonce validation, to enable alternative constructions while remaining secure by default.

Implementations of Noise need to define ECDH over sepc256k1 anyway, so from that POV why does it matter what the definition is? What am I missing?

[sstone]

It does not matter very much. I was just being lazy and it seems that the Noise specs assume that dhLen == pubkeyLen (i.e. when processing "e" Write produces e.pub || xxx and Read says re = first dhLen bytes etc...), and I've seen ECDH libraries return either compressed point, point.x or sha256(point.x).
I'm happy with secp256k1's ECDH and changing the description of ECDH in bolt #8 to match it.

[Roasbeef]

Yeah, so in either case, implementers which are instantiating new noise schemes will need to define the semantics of their ECDH function. You should be able to use existing noise implementations adding in one minor modification: adding our version byte which is the prefix of each handshake sent.

I agree with Rusty, I think we should redefine our ECDH function within BOLT0008 to better align ourselves with libsecp256k1 as that's a popular, well reviewed crypto library in the space. Modifying my implementation to use SHA(compressedPoint) is a pretty trivial change, which should resolve this issue.

[sstone]

Fine with me, closing this PR. Thanks!