## Issue:
This is an approach to fixing secret connection that is more noise-ish than actually noise.
but it essentially fixes the problem that #3315 is trying to solve by making the secret connection handshake non-malleable. It's easy to understand and I think will be acceptable to @jaekwon
.. the formal reasoning is basically, if the "view" of the transcript between diverges between the sender and the receiver at any point in the protocol, the handshake would terminate.
The base protocol of Station to Station mistakenly assumes that if the sender and receiver arrive at shared secret they have the same view. This is only true for a DH on prime order groups.
This robustly solves the problem by having each cryptographic operation commit to operators view of the protocol.
Another nice thing about a transcript is it provides the basis for "secure" (barring cryptographic breakages, horrible design flaws, or implementation bugs) downgrades, where a backwards compatible handshake can be used to offer newer protocol features/extensions, peers agree to the common subset of what they support, and both sides have to agree on what the other offered for the transcript MAC to verify.
With something like Protos/Amino you already get "extensions" for free (TLS uses a simple TLV format https://tools.ietf.org/html/rfc8446#section-4.2 for extensions not too far off from Protos/Amino), so as long as you cryptographically commit to what they contain in the transcript, it should be possible to extend the protocol in a backwards-compatible manner.
## Commits:
* Minimal changes to remove malleability of secret connection removes the need to check for lower order points.
Breaks compatibility. Secret connections that have no been updated will fail
* Remove the redundant blacklist
* remove remainders of blacklist in tests to make the code compile again
Signed-off-by: Ismail Khoffi <Ismail.Khoffi@gmail.com>
* Apply suggestions from code review
Apply Ismail's error handling
Co-Authored-By: Ismail Khoffi <Ismail.Khoffi@gmail.com>
* fix error check for io.ReadFull
Signed-off-by: Ismail Khoffi <Ismail.Khoffi@gmail.com>
* Update p2p/conn/secret_connection.go
Co-Authored-By: Ismail Khoffi <Ismail.Khoffi@gmail.com>
* Update p2p/conn/secret_connection.go
Co-Authored-By: Bot from GolangCI <42910462+golangcibot@users.noreply.github.com>
* update changelog and format the code
* move hkdfInit closer to where it's used
* Fix long line errors in abci, crypto, and libs packages
* Fix long lines in p2p and rpc packages
* Fix long lines in abci, state, and tools packages
* Fix long lines in behaviour and blockchain packages
* Fix long lines in cmd and config packages
* Begin fixing long lines in consensus package
* Finish fixing long lines in consensus package
* Add lll exclusion for lines containing URLs
* Fix long lines in crypto package
* Fix long lines in evidence package
* Fix long lines in mempool and node packages
* Fix long lines in libs package
* Fix long lines in lite package
* Fix new long line in node package
* Fix long lines in p2p package
* Ignore gocritic warning
* Fix long lines in privval package
* Fix long lines in rpc package
* Fix long lines in scripts package
* Fix long lines in state package
* Fix long lines in tools package
* Fix long lines in types package
* Enable lll linter
* Remove unnecessary type conversions
* Consolidate repeated strings into consts
* Clothe return statements
* Update blockchain/v1/reactor_fsm_test.go
Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com>
This PR repairs linter errors seen when running the following commands:
golangci-lint run --no-config --disable-all=true --enable=unconvert
golangci-lint run --no-config --disable-all=true --enable=goconst
golangci-lint run --no-config --disable-all=true --enable=nakedret
Contributes to #3262
* reject the shared secret if is all zeros in case the blacklist was not
sufficient
* Add test that verifies lower order pub-keys are rejected at the DH step
* Update changelog
* fix typo in test-comment
> Implement a check for the blacklisted low order points, ala the X25519 has_small_order() function in libsodium
(#3010 (comment))
resolves first half of #3010
* crypto: revert to mainline Go crypto lib
We used to use a fork for a modified bcrypt so we could pass our own
randomness but this was largely unecessary, unused, and a burden.
So now we just use the mainline Go crypto lib.
* changelog
* fix tests
* version and changelog
* update secret connection to use a little endian encoded nonce
* update encoding of chunk length to be little endian, too
* update comment
* Change comment slightly to trigger circelci
This now uses one hkdf on the X25519 shared secret to create
a key for the sender and receiver.
The hkdf call is now just called upon the computed shared
secret, since the shared secret is a function of the pubkeys.
The nonces now start at 0, as we are using chacha as a stream
cipher, and the sender and receiver now have different keys.
Generate keys with HKDF instead of hash functions, which provides better security properties.
Add xchacha20poly1305 to secret connection. (Due to rebasing, this code has been removed)
* use increment and decrement operators.
* remove unnecessary else branches.
* fix package comment with leading space.
* fix receiver names.
* fix error strings.
* remove omittable code.
* remove redundant return statement.
* Revert changes (code is generated.)
* use cfg as receiver name for all config-related types.
* use lsi as the receiver name for the LastSignedInfo type.
Updates https://github.com/tendermint/tendermint/issues/850
My security alarms falsely blarred when I skimmed and noticed
keys being compared with `==`, without the proper context
so I mistakenly filed an issue, yet the purpose of that
comparison was to check if the local ephemeral public key
was just the least, sorted lexicographically.
Anyways, let's use the proper bytes.Equal check, to save future labor.
Anton Kaliaev
Zaki Manian
Phil Salant
Zaki Manian
zjubfd
Ivan Kushmantsev
Ismail Khoffi
Ethan Buchman
ValarDragon
Zach Ramsay
Liamsi
Thomas Corbière
Jae Kwon
Emmanuel Odeke
zramsay