mimblewimble / grin

Minimal implementation of the Mimblewimble protocol.
https://grin.mw/
Apache License 2.0
5.04k stars 991 forks source link

Kernel offsets must *not* be commitments #1060

Closed anargle closed 6 years ago

anargle commented 6 years ago

I believe that #1020 completely destroys Grin's security properties and should be reverted. Although the kernel offset was always just turned into a commitment, the fact that it was stored and transmitted as a blinding factor serves as a (trivial) proof of knowledge of the discrete logarithm or the kernel offset commitment. By storing a commitment instead of a blinding factor, the signature that makes up the kernel is useless.

As an example, I could, in a single transaction, steal every UTXO in existence and forge as much money as I wanted: if the sum of all existing UTXOs is P, then I can post a transaction with

ignopeverell commented 6 years ago

Very nice catch, thanks for keeping an eye on us!

antiochp commented 6 years ago

🤦‍♀️ Took me a couple of reads of this bug report to actually get my head around what's being said here. But basically (please correct my thinking here if wrong) this distills down to - If we sum a signed kernel excess and an unsigned kernel offset (and we know both commitments) then it is trivial to replace them with any pair of commitments. i.e. we can sign anything and trivially provide the necessary offset commitment.

Working on reverting this change on the consensus_breaking branch.

Thanks @anargle for this.

Got so deep into the "everything in Grin is just summing commitments" that I forgot to fully consider the kernel signatures here...

mably commented 6 years ago

Tried to re-read the intro page of the Grin docs without succeeding to match with what is being said in this issue:

https://github.com/mimblewimble/grin/blob/master/doc/intro.md

Are these commitments / signed kernel excess / unsigned kernel offset described in details somewhere?

quentinlesceller commented 6 years ago

@mably You can read more about it here https://github.com/mimblewimble/grin/pull/681.

ignopeverell commented 6 years ago

The commitmens and the kernel excess are described in that doc. Offsets are an addition on top of the excess and its signature to improve privacy. They're not formally documented (yet) but #671 covers a lot of it.

ignopeverell commented 6 years ago

And @quentinlesceller beats me to it by throwing #681 in the mix as well :-)

antiochp commented 6 years ago

Using a kernel offset to maintain privacy of individual transactions in a block - good. Using a kernel offset that allows all the funds to be stolen - bad.

ignopeverell commented 6 years ago

Fixed by #1062