You can not select more than 25 topics Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.

464 lines
13 KiB

crypto: Use a different library for ed25519/sr25519 (#6526) At Oasis we have spend some time writing a new Ed25519/X25519/sr25519 implementation called curve25519-voi. This PR switches the import from ed25519consensus/go-schnorrkel, which should lead to performance gains on most systems. Summary of changes: * curve25519-voi is now used for Ed25519 operations, following the existing ZIP-215 semantics. * curve25519-voi's public key cache is enabled (hardcoded size of 4096 entries, should be tuned, see the code comment) to accelerate repeated Ed25519 verification with the same public key(s). * (BREAKING) curve25519-voi is now used for sr25519 operations. This is a breaking change as the current sr25519 support does something decidedly non-standard when going from a MiniSecretKey to a SecretKey and or PublicKey (The expansion routine is called twice). While I believe the new behavior (that expands once and only once) to be more "correct", this changes the semantics as implemented. * curve25519-voi is now used for merlin since the included STROBE implementation produces much less garbage on the heap. Side issues fixed: * The version of go-schnorrkel that is currently imported by tendermint has a badly broken batch verification implementation. Upstream has fixed the issue after I reported it, so the version should be bumped in the interim. Open design questions/issues: * As noted, the public key cache size should be tuned. It is currently backed by a trivial thread-safe LRU cache, which is not scan-resistant, but replacing it with something better is a matter of implementing an interface. * As far as I can tell, the only reason why serial verification on batch failure is necessary is to provide more detailed error messages (that are only used in some unit tests). If you trust the batch verification to be consistent with serial verification then the fallback can be eliminated entirely (the BatchVerifier provided by the new library supports an option that omits the fallback if this is chosen as the way forward). * curve25519-voi's sr25519 support could use more optimization and more eyes on the code. The algorithm unfortunately is woefully under-specified, and the implementation was done primarily because I got really sad when I actually looked at go-schnorrkel, and we do not use the algorithm at this time.
3 years ago
cleanup: Reduce and normalize import path aliasing. (#6975) The code in the Tendermint repository makes heavy use of import aliasing. This is made necessary by our extensive reuse of common base package names, and by repetition of similar names across different subdirectories. Unfortunately we have not been very consistent about which packages we alias in various circumstances, and the aliases we use vary. In the spirit of the advice in the style guide and https://github.com/golang/go/wiki/CodeReviewComments#imports, his change makes an effort to clean up and normalize import aliasing. This change makes no API or behavioral changes. It is a pure cleanup intended o help make the code more readable to developers (including myself) trying to understand what is being imported where. Only unexported names have been modified, and the changes were generated and applied mechanically with gofmt -r and comby, respecting the lexical and syntactic rules of Go. Even so, I did not fix every inconsistency. Where the changes would be too disruptive, I left it alone. The principles I followed in this cleanup are: - Remove aliases that restate the package name. - Remove aliases where the base package name is unambiguous. - Move overly-terse abbreviations from the import to the usage site. - Fix lexical issues (remove underscores, remove capitalization). - Fix import groupings to more closely match the style guide. - Group blank (side-effecting) imports and ensure they are commented. - Add aliases to multiple imports with the same base package name.
3 years ago
crypto: Use a different library for ed25519/sr25519 (#6526) At Oasis we have spend some time writing a new Ed25519/X25519/sr25519 implementation called curve25519-voi. This PR switches the import from ed25519consensus/go-schnorrkel, which should lead to performance gains on most systems. Summary of changes: * curve25519-voi is now used for Ed25519 operations, following the existing ZIP-215 semantics. * curve25519-voi's public key cache is enabled (hardcoded size of 4096 entries, should be tuned, see the code comment) to accelerate repeated Ed25519 verification with the same public key(s). * (BREAKING) curve25519-voi is now used for sr25519 operations. This is a breaking change as the current sr25519 support does something decidedly non-standard when going from a MiniSecretKey to a SecretKey and or PublicKey (The expansion routine is called twice). While I believe the new behavior (that expands once and only once) to be more "correct", this changes the semantics as implemented. * curve25519-voi is now used for merlin since the included STROBE implementation produces much less garbage on the heap. Side issues fixed: * The version of go-schnorrkel that is currently imported by tendermint has a badly broken batch verification implementation. Upstream has fixed the issue after I reported it, so the version should be bumped in the interim. Open design questions/issues: * As noted, the public key cache size should be tuned. It is currently backed by a trivial thread-safe LRU cache, which is not scan-resistant, but replacing it with something better is a matter of implementing an interface. * As far as I can tell, the only reason why serial verification on batch failure is necessary is to provide more detailed error messages (that are only used in some unit tests). If you trust the batch verification to be consistent with serial verification then the fallback can be eliminated entirely (the BatchVerifier provided by the new library supports an option that omits the fallback if this is chosen as the way forward). * curve25519-voi's sr25519 support could use more optimization and more eyes on the code. The algorithm unfortunately is woefully under-specified, and the implementation was done primarily because I got really sad when I actually looked at go-schnorrkel, and we do not use the algorithm at this time.
3 years ago
p2p: make SecretConnection non-malleable (#3668) ## 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
5 years ago
p2p: make SecretConnection non-malleable (#3668) ## 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
5 years ago
p2p: make SecretConnection non-malleable (#3668) ## 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
5 years ago
p2p: make SecretConnection non-malleable (#3668) ## 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
5 years ago
p2p: make SecretConnection non-malleable (#3668) ## 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
5 years ago
p2p: make SecretConnection non-malleable (#3668) ## 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
5 years ago
p2p: make SecretConnection non-malleable (#3668) ## 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
5 years ago
crypto: Use a different library for ed25519/sr25519 (#6526) At Oasis we have spend some time writing a new Ed25519/X25519/sr25519 implementation called curve25519-voi. This PR switches the import from ed25519consensus/go-schnorrkel, which should lead to performance gains on most systems. Summary of changes: * curve25519-voi is now used for Ed25519 operations, following the existing ZIP-215 semantics. * curve25519-voi's public key cache is enabled (hardcoded size of 4096 entries, should be tuned, see the code comment) to accelerate repeated Ed25519 verification with the same public key(s). * (BREAKING) curve25519-voi is now used for sr25519 operations. This is a breaking change as the current sr25519 support does something decidedly non-standard when going from a MiniSecretKey to a SecretKey and or PublicKey (The expansion routine is called twice). While I believe the new behavior (that expands once and only once) to be more "correct", this changes the semantics as implemented. * curve25519-voi is now used for merlin since the included STROBE implementation produces much less garbage on the heap. Side issues fixed: * The version of go-schnorrkel that is currently imported by tendermint has a badly broken batch verification implementation. Upstream has fixed the issue after I reported it, so the version should be bumped in the interim. Open design questions/issues: * As noted, the public key cache size should be tuned. It is currently backed by a trivial thread-safe LRU cache, which is not scan-resistant, but replacing it with something better is a matter of implementing an interface. * As far as I can tell, the only reason why serial verification on batch failure is necessary is to provide more detailed error messages (that are only used in some unit tests). If you trust the batch verification to be consistent with serial verification then the fallback can be eliminated entirely (the BatchVerifier provided by the new library supports an option that omits the fallback if this is chosen as the way forward). * curve25519-voi's sr25519 support could use more optimization and more eyes on the code. The algorithm unfortunately is woefully under-specified, and the implementation was done primarily because I got really sad when I actually looked at go-schnorrkel, and we do not use the algorithm at this time.
3 years ago
p2p: make SecretConnection non-malleable (#3668) ## 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
5 years ago
cleanup: Reduce and normalize import path aliasing. (#6975) The code in the Tendermint repository makes heavy use of import aliasing. This is made necessary by our extensive reuse of common base package names, and by repetition of similar names across different subdirectories. Unfortunately we have not been very consistent about which packages we alias in various circumstances, and the aliases we use vary. In the spirit of the advice in the style guide and https://github.com/golang/go/wiki/CodeReviewComments#imports, his change makes an effort to clean up and normalize import aliasing. This change makes no API or behavioral changes. It is a pure cleanup intended o help make the code more readable to developers (including myself) trying to understand what is being imported where. Only unexported names have been modified, and the changes were generated and applied mechanically with gofmt -r and comby, respecting the lexical and syntactic rules of Go. Even so, I did not fix every inconsistency. Where the changes would be too disruptive, I left it alone. The principles I followed in this cleanup are: - Remove aliases that restate the package name. - Remove aliases where the base package name is unambiguous. - Move overly-terse abbreviations from the import to the usage site. - Fix lexical issues (remove underscores, remove capitalization). - Fix import groupings to more closely match the style guide. - Group blank (side-effecting) imports and ensure they are commented. - Add aliases to multiple imports with the same base package name.
3 years ago
cleanup: Reduce and normalize import path aliasing. (#6975) The code in the Tendermint repository makes heavy use of import aliasing. This is made necessary by our extensive reuse of common base package names, and by repetition of similar names across different subdirectories. Unfortunately we have not been very consistent about which packages we alias in various circumstances, and the aliases we use vary. In the spirit of the advice in the style guide and https://github.com/golang/go/wiki/CodeReviewComments#imports, his change makes an effort to clean up and normalize import aliasing. This change makes no API or behavioral changes. It is a pure cleanup intended o help make the code more readable to developers (including myself) trying to understand what is being imported where. Only unexported names have been modified, and the changes were generated and applied mechanically with gofmt -r and comby, respecting the lexical and syntactic rules of Go. Even so, I did not fix every inconsistency. Where the changes would be too disruptive, I left it alone. The principles I followed in this cleanup are: - Remove aliases that restate the package name. - Remove aliases where the base package name is unambiguous. - Move overly-terse abbreviations from the import to the usage site. - Fix lexical issues (remove underscores, remove capitalization). - Fix import groupings to more closely match the style guide. - Group blank (side-effecting) imports and ensure they are commented. - Add aliases to multiple imports with the same base package name.
3 years ago
  1. package conn
  2. import (
  3. "bytes"
  4. "crypto/cipher"
  5. crand "crypto/rand"
  6. "crypto/sha256"
  7. "encoding/binary"
  8. "errors"
  9. "fmt"
  10. "io"
  11. "math"
  12. "net"
  13. "time"
  14. gogotypes "github.com/gogo/protobuf/types"
  15. pool "github.com/libp2p/go-buffer-pool"
  16. "github.com/oasisprotocol/curve25519-voi/primitives/merlin"
  17. "golang.org/x/crypto/chacha20poly1305"
  18. "golang.org/x/crypto/curve25519"
  19. "golang.org/x/crypto/hkdf"
  20. "golang.org/x/crypto/nacl/box"
  21. "github.com/tendermint/tendermint/crypto"
  22. "github.com/tendermint/tendermint/crypto/ed25519"
  23. "github.com/tendermint/tendermint/crypto/encoding"
  24. "github.com/tendermint/tendermint/internal/libs/protoio"
  25. tmsync "github.com/tendermint/tendermint/internal/libs/sync"
  26. "github.com/tendermint/tendermint/libs/async"
  27. tmp2p "github.com/tendermint/tendermint/proto/tendermint/p2p"
  28. )
  29. // 4 + 1024 == 1028 total frame size
  30. const (
  31. dataLenSize = 4
  32. dataMaxSize = 1024
  33. totalFrameSize = dataMaxSize + dataLenSize
  34. aeadSizeOverhead = 16 // overhead of poly 1305 authentication tag
  35. aeadKeySize = chacha20poly1305.KeySize
  36. aeadNonceSize = chacha20poly1305.NonceSize
  37. labelEphemeralLowerPublicKey = "EPHEMERAL_LOWER_PUBLIC_KEY"
  38. labelEphemeralUpperPublicKey = "EPHEMERAL_UPPER_PUBLIC_KEY"
  39. labelDHSecret = "DH_SECRET"
  40. labelSecretConnectionMac = "SECRET_CONNECTION_MAC"
  41. )
  42. var (
  43. ErrSmallOrderRemotePubKey = errors.New("detected low order point from remote peer")
  44. secretConnKeyAndChallengeGen = []byte("TENDERMINT_SECRET_CONNECTION_KEY_AND_CHALLENGE_GEN")
  45. )
  46. // SecretConnection implements net.Conn.
  47. // It is an implementation of the STS protocol.
  48. // See https://github.com/tendermint/tendermint/blob/0.1/docs/sts-final.pdf for
  49. // details on the protocol.
  50. //
  51. // Consumers of the SecretConnection are responsible for authenticating
  52. // the remote peer's pubkey against known information, like a nodeID.
  53. // Otherwise they are vulnerable to MITM.
  54. // (TODO(ismail): see also https://github.com/tendermint/tendermint/issues/3010)
  55. type SecretConnection struct {
  56. // immutable
  57. recvAead cipher.AEAD
  58. sendAead cipher.AEAD
  59. remPubKey crypto.PubKey
  60. conn io.ReadWriteCloser
  61. // net.Conn must be thread safe:
  62. // https://golang.org/pkg/net/#Conn.
  63. // Since we have internal mutable state,
  64. // we need mtxs. But recv and send states
  65. // are independent, so we can use two mtxs.
  66. // All .Read are covered by recvMtx,
  67. // all .Write are covered by sendMtx.
  68. recvMtx tmsync.Mutex
  69. recvBuffer []byte
  70. recvNonce *[aeadNonceSize]byte
  71. sendMtx tmsync.Mutex
  72. sendNonce *[aeadNonceSize]byte
  73. }
  74. // MakeSecretConnection performs handshake and returns a new authenticated
  75. // SecretConnection.
  76. // Returns nil if there is an error in handshake.
  77. // Caller should call conn.Close()
  78. // See docs/sts-final.pdf for more information.
  79. func MakeSecretConnection(conn io.ReadWriteCloser, locPrivKey crypto.PrivKey) (*SecretConnection, error) {
  80. var (
  81. locPubKey = locPrivKey.PubKey()
  82. )
  83. // Generate ephemeral keys for perfect forward secrecy.
  84. locEphPub, locEphPriv := genEphKeys()
  85. // Write local ephemeral pubkey and receive one too.
  86. // NOTE: every 32-byte string is accepted as a Curve25519 public key (see
  87. // DJB's Curve25519 paper: http://cr.yp.to/ecdh/curve25519-20060209.pdf)
  88. remEphPub, err := shareEphPubKey(conn, locEphPub)
  89. if err != nil {
  90. return nil, err
  91. }
  92. // Sort by lexical order.
  93. loEphPub, hiEphPub := sort32(locEphPub, remEphPub)
  94. transcript := merlin.NewTranscript("TENDERMINT_SECRET_CONNECTION_TRANSCRIPT_HASH")
  95. transcript.AppendMessage(labelEphemeralLowerPublicKey, loEphPub[:])
  96. transcript.AppendMessage(labelEphemeralUpperPublicKey, hiEphPub[:])
  97. // Check if the local ephemeral public key was the least, lexicographically
  98. // sorted.
  99. locIsLeast := bytes.Equal(locEphPub[:], loEphPub[:])
  100. // Compute common diffie hellman secret using X25519.
  101. dhSecret, err := computeDHSecret(remEphPub, locEphPriv)
  102. if err != nil {
  103. return nil, err
  104. }
  105. transcript.AppendMessage(labelDHSecret, dhSecret[:])
  106. // Generate the secret used for receiving, sending, challenge via HKDF-SHA2
  107. // on the transcript state (which itself also uses HKDF-SHA2 to derive a key
  108. // from the dhSecret).
  109. recvSecret, sendSecret := deriveSecrets(dhSecret, locIsLeast)
  110. const challengeSize = 32
  111. var challenge [challengeSize]byte
  112. transcript.ExtractBytes(challenge[:], labelSecretConnectionMac)
  113. sendAead, err := chacha20poly1305.New(sendSecret[:])
  114. if err != nil {
  115. return nil, errors.New("invalid send SecretConnection Key")
  116. }
  117. recvAead, err := chacha20poly1305.New(recvSecret[:])
  118. if err != nil {
  119. return nil, errors.New("invalid receive SecretConnection Key")
  120. }
  121. sc := &SecretConnection{
  122. conn: conn,
  123. recvBuffer: nil,
  124. recvNonce: new([aeadNonceSize]byte),
  125. sendNonce: new([aeadNonceSize]byte),
  126. recvAead: recvAead,
  127. sendAead: sendAead,
  128. }
  129. // Sign the challenge bytes for authentication.
  130. locSignature, err := signChallenge(&challenge, locPrivKey)
  131. if err != nil {
  132. return nil, err
  133. }
  134. // Share (in secret) each other's pubkey & challenge signature
  135. authSigMsg, err := shareAuthSignature(sc, locPubKey, locSignature)
  136. if err != nil {
  137. return nil, err
  138. }
  139. remPubKey, remSignature := authSigMsg.Key, authSigMsg.Sig
  140. if _, ok := remPubKey.(ed25519.PubKey); !ok {
  141. return nil, fmt.Errorf("expected ed25519 pubkey, got %T", remPubKey)
  142. }
  143. if !remPubKey.VerifySignature(challenge[:], remSignature) {
  144. return nil, errors.New("challenge verification failed")
  145. }
  146. // We've authorized.
  147. sc.remPubKey = remPubKey
  148. return sc, nil
  149. }
  150. // RemotePubKey returns authenticated remote pubkey
  151. func (sc *SecretConnection) RemotePubKey() crypto.PubKey {
  152. return sc.remPubKey
  153. }
  154. // Writes encrypted frames of `totalFrameSize + aeadSizeOverhead`.
  155. // CONTRACT: data smaller than dataMaxSize is written atomically.
  156. func (sc *SecretConnection) Write(data []byte) (n int, err error) {
  157. sc.sendMtx.Lock()
  158. defer sc.sendMtx.Unlock()
  159. for 0 < len(data) {
  160. if err := func() error {
  161. var sealedFrame = pool.Get(aeadSizeOverhead + totalFrameSize)
  162. var frame = pool.Get(totalFrameSize)
  163. defer func() {
  164. pool.Put(sealedFrame)
  165. pool.Put(frame)
  166. }()
  167. var chunk []byte
  168. if dataMaxSize < len(data) {
  169. chunk = data[:dataMaxSize]
  170. data = data[dataMaxSize:]
  171. } else {
  172. chunk = data
  173. data = nil
  174. }
  175. chunkLength := len(chunk)
  176. binary.LittleEndian.PutUint32(frame, uint32(chunkLength))
  177. copy(frame[dataLenSize:], chunk)
  178. // encrypt the frame
  179. sc.sendAead.Seal(sealedFrame[:0], sc.sendNonce[:], frame, nil)
  180. incrNonce(sc.sendNonce)
  181. // end encryption
  182. _, err = sc.conn.Write(sealedFrame)
  183. if err != nil {
  184. return err
  185. }
  186. n += len(chunk)
  187. return nil
  188. }(); err != nil {
  189. return n, err
  190. }
  191. }
  192. return n, err
  193. }
  194. // CONTRACT: data smaller than dataMaxSize is read atomically.
  195. func (sc *SecretConnection) Read(data []byte) (n int, err error) {
  196. sc.recvMtx.Lock()
  197. defer sc.recvMtx.Unlock()
  198. // read off and update the recvBuffer, if non-empty
  199. if 0 < len(sc.recvBuffer) {
  200. n = copy(data, sc.recvBuffer)
  201. sc.recvBuffer = sc.recvBuffer[n:]
  202. return
  203. }
  204. // read off the conn
  205. var sealedFrame = pool.Get(aeadSizeOverhead + totalFrameSize)
  206. defer pool.Put(sealedFrame)
  207. _, err = io.ReadFull(sc.conn, sealedFrame)
  208. if err != nil {
  209. return
  210. }
  211. // decrypt the frame.
  212. // reads and updates the sc.recvNonce
  213. var frame = pool.Get(totalFrameSize)
  214. defer pool.Put(frame)
  215. _, err = sc.recvAead.Open(frame[:0], sc.recvNonce[:], sealedFrame, nil)
  216. if err != nil {
  217. return n, fmt.Errorf("failed to decrypt SecretConnection: %w", err)
  218. }
  219. incrNonce(sc.recvNonce)
  220. // end decryption
  221. // copy checkLength worth into data,
  222. // set recvBuffer to the rest.
  223. var chunkLength = binary.LittleEndian.Uint32(frame) // read the first four bytes
  224. if chunkLength > dataMaxSize {
  225. return 0, errors.New("chunkLength is greater than dataMaxSize")
  226. }
  227. var chunk = frame[dataLenSize : dataLenSize+chunkLength]
  228. n = copy(data, chunk)
  229. if n < len(chunk) {
  230. sc.recvBuffer = make([]byte, len(chunk)-n)
  231. copy(sc.recvBuffer, chunk[n:])
  232. }
  233. return n, err
  234. }
  235. // Implements net.Conn
  236. func (sc *SecretConnection) Close() error { return sc.conn.Close() }
  237. func (sc *SecretConnection) LocalAddr() net.Addr { return sc.conn.(net.Conn).LocalAddr() }
  238. func (sc *SecretConnection) RemoteAddr() net.Addr { return sc.conn.(net.Conn).RemoteAddr() }
  239. func (sc *SecretConnection) SetDeadline(t time.Time) error { return sc.conn.(net.Conn).SetDeadline(t) }
  240. func (sc *SecretConnection) SetReadDeadline(t time.Time) error {
  241. return sc.conn.(net.Conn).SetReadDeadline(t)
  242. }
  243. func (sc *SecretConnection) SetWriteDeadline(t time.Time) error {
  244. return sc.conn.(net.Conn).SetWriteDeadline(t)
  245. }
  246. func genEphKeys() (ephPub, ephPriv *[32]byte) {
  247. var err error
  248. // TODO: Probably not a problem but ask Tony: different from the rust implementation (uses x25519-dalek),
  249. // we do not "clamp" the private key scalar:
  250. // see: https://github.com/dalek-cryptography/x25519-dalek/blob/34676d336049df2bba763cc076a75e47ae1f170f/src/x25519.rs#L56-L74
  251. ephPub, ephPriv, err = box.GenerateKey(crand.Reader)
  252. if err != nil {
  253. panic("Could not generate ephemeral key-pair")
  254. }
  255. return
  256. }
  257. func shareEphPubKey(conn io.ReadWriter, locEphPub *[32]byte) (remEphPub *[32]byte, err error) {
  258. // Send our pubkey and receive theirs in tandem.
  259. var trs, _ = async.Parallel(
  260. func(_ int) (val interface{}, abort bool, err error) {
  261. lc := *locEphPub
  262. _, err = protoio.NewDelimitedWriter(conn).WriteMsg(&gogotypes.BytesValue{Value: lc[:]})
  263. if err != nil {
  264. return nil, true, err // abort
  265. }
  266. return nil, false, nil
  267. },
  268. func(_ int) (val interface{}, abort bool, err error) {
  269. var bytes gogotypes.BytesValue
  270. _, err = protoio.NewDelimitedReader(conn, 1024*1024).ReadMsg(&bytes)
  271. if err != nil {
  272. return nil, true, err // abort
  273. }
  274. var _remEphPub [32]byte
  275. copy(_remEphPub[:], bytes.Value)
  276. return _remEphPub, false, nil
  277. },
  278. )
  279. // If error:
  280. if trs.FirstError() != nil {
  281. err = trs.FirstError()
  282. return
  283. }
  284. // Otherwise:
  285. var _remEphPub = trs.FirstValue().([32]byte)
  286. return &_remEphPub, nil
  287. }
  288. func deriveSecrets(
  289. dhSecret *[32]byte,
  290. locIsLeast bool,
  291. ) (recvSecret, sendSecret *[aeadKeySize]byte) {
  292. hash := sha256.New
  293. hkdf := hkdf.New(hash, dhSecret[:], nil, secretConnKeyAndChallengeGen)
  294. // get enough data for 2 aead keys, and a 32 byte challenge
  295. res := new([2*aeadKeySize + 32]byte)
  296. _, err := io.ReadFull(hkdf, res[:])
  297. if err != nil {
  298. panic(err)
  299. }
  300. recvSecret = new([aeadKeySize]byte)
  301. sendSecret = new([aeadKeySize]byte)
  302. // bytes 0 through aeadKeySize - 1 are one aead key.
  303. // bytes aeadKeySize through 2*aeadKeySize -1 are another aead key.
  304. // which key corresponds to sending and receiving key depends on whether
  305. // the local key is less than the remote key.
  306. if locIsLeast {
  307. copy(recvSecret[:], res[0:aeadKeySize])
  308. copy(sendSecret[:], res[aeadKeySize:aeadKeySize*2])
  309. } else {
  310. copy(sendSecret[:], res[0:aeadKeySize])
  311. copy(recvSecret[:], res[aeadKeySize:aeadKeySize*2])
  312. }
  313. return
  314. }
  315. // computeDHSecret computes a Diffie-Hellman shared secret key
  316. // from our own local private key and the other's public key.
  317. func computeDHSecret(remPubKey, locPrivKey *[32]byte) (*[32]byte, error) {
  318. shrKey, err := curve25519.X25519(locPrivKey[:], remPubKey[:])
  319. if err != nil {
  320. return nil, err
  321. }
  322. var shrKeyArray [32]byte
  323. copy(shrKeyArray[:], shrKey)
  324. return &shrKeyArray, nil
  325. }
  326. func sort32(foo, bar *[32]byte) (lo, hi *[32]byte) {
  327. if bytes.Compare(foo[:], bar[:]) < 0 {
  328. lo = foo
  329. hi = bar
  330. } else {
  331. lo = bar
  332. hi = foo
  333. }
  334. return
  335. }
  336. func signChallenge(challenge *[32]byte, locPrivKey crypto.PrivKey) ([]byte, error) {
  337. signature, err := locPrivKey.Sign(challenge[:])
  338. if err != nil {
  339. return nil, err
  340. }
  341. return signature, nil
  342. }
  343. type authSigMessage struct {
  344. Key crypto.PubKey
  345. Sig []byte
  346. }
  347. func shareAuthSignature(sc io.ReadWriter, pubKey crypto.PubKey, signature []byte) (recvMsg authSigMessage, err error) {
  348. // Send our info and receive theirs in tandem.
  349. var trs, _ = async.Parallel(
  350. func(_ int) (val interface{}, abort bool, err error) {
  351. pbpk, err := encoding.PubKeyToProto(pubKey)
  352. if err != nil {
  353. return nil, true, err
  354. }
  355. _, err = protoio.NewDelimitedWriter(sc).WriteMsg(&tmp2p.AuthSigMessage{PubKey: pbpk, Sig: signature})
  356. if err != nil {
  357. return nil, true, err // abort
  358. }
  359. return nil, false, nil
  360. },
  361. func(_ int) (val interface{}, abort bool, err error) {
  362. var pba tmp2p.AuthSigMessage
  363. _, err = protoio.NewDelimitedReader(sc, 1024*1024).ReadMsg(&pba)
  364. if err != nil {
  365. return nil, true, err // abort
  366. }
  367. pk, err := encoding.PubKeyFromProto(pba.PubKey)
  368. if err != nil {
  369. return nil, true, err // abort
  370. }
  371. _recvMsg := authSigMessage{
  372. Key: pk,
  373. Sig: pba.Sig,
  374. }
  375. return _recvMsg, false, nil
  376. },
  377. )
  378. // If error:
  379. if trs.FirstError() != nil {
  380. err = trs.FirstError()
  381. return
  382. }
  383. var _recvMsg = trs.FirstValue().(authSigMessage)
  384. return _recvMsg, nil
  385. }
  386. //--------------------------------------------------------------------------------
  387. // Increment nonce little-endian by 1 with wraparound.
  388. // Due to chacha20poly1305 expecting a 12 byte nonce we do not use the first four
  389. // bytes. We only increment a 64 bit unsigned int in the remaining 8 bytes
  390. // (little-endian in nonce[4:]).
  391. func incrNonce(nonce *[aeadNonceSize]byte) {
  392. counter := binary.LittleEndian.Uint64(nonce[4:])
  393. if counter == math.MaxUint64 {
  394. // Terminates the session and makes sure the nonce would not re-used.
  395. // See https://github.com/tendermint/tendermint/issues/3531
  396. panic("can't increase nonce without overflow")
  397. }
  398. counter++
  399. binary.LittleEndian.PutUint64(nonce[4:], counter)
  400. }