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.

257 lines
6.9 KiB

privval: refactor Remote signers (#3370) This PR is related to #3107 and a continuation of #3351 It is important to emphasise that in the privval original design, client/server and listening/dialing roles are inverted and do not follow a conventional interaction. Given two hosts A and B: Host A is listener/client Host B is dialer/server (contains the secret key) When A requires a signature, it needs to wait for B to dial in before it can issue a request. A only accepts a single connection and any failure leads to dropping the connection and waiting for B to reconnect. The original rationale behind this design was based on security. Host B only allows outbound connections to a list of whitelisted hosts. It is not possible to reach B unless B dials in. There are no listening/open ports in B. This PR results in the following changes: Refactors ping/heartbeat to avoid previously existing race conditions. Separates transport (dialer/listener) from signing (client/server) concerns to simplify workflow. Unifies and abstracts away the differences between unix and tcp sockets. A single signer endpoint implementation unifies connection handling code (read/write/close/connection obj) The signer request handler (server side) is customizable to increase testability. Updates and extends unit tests A high level overview of the classes is as follows: Transport (endpoints): The following classes take care of establishing a connection SignerDialerEndpoint SignerListeningEndpoint SignerEndpoint groups common functionality (read/write/timeouts/etc.) Signing (client/server): The following classes take care of exchanging request/responses SignerClient SignerServer This PR also closes #3601 Commits: * refactoring - work in progress * reworking unit tests * Encapsulating and fixing unit tests * Improve tests * Clean up * Fix/improve unit tests * clean up tests * Improving service endpoint * fixing unit test * fix linter issues * avoid invalid cache values (improve later?) * complete implementation * wip * improved connection loop * Improve reconnections + fixing unit tests * addressing comments * small formatting changes * clean up * Update node/node.go Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_client.go Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_client_test.go Co-Authored-By: jleni <juan.leni@zondax.ch> * check during initialization * dropping connecting when writing fails * removing break * use t.log instead * unifying and using cmn.GetFreePort() * review fixes * reordering and unifying drop connection * closing instead of signalling * refactored service loop * removed superfluous brackets * GetPubKey can return errors * Revert "GetPubKey can return errors" This reverts commit 68c06f19b4650389d7e5ab1659b318889028202c. * adding entry to changelog * Update CHANGELOG_PENDING.md Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_client.go Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_dialer_endpoint.go Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_dialer_endpoint.go Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_dialer_endpoint.go Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_dialer_endpoint.go Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_listener_endpoint_test.go Co-Authored-By: jleni <juan.leni@zondax.ch> * updating node.go * review fixes * fixes linter * fixing unit test * small fixes in comments * addressing review comments * addressing review comments 2 * reverting suggestion * Update privval/signer_client_test.go Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com> * Update privval/signer_client_test.go Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com> * Update privval/signer_listener_endpoint_test.go Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com> * do not expose brokenSignerDialerEndpoint * clean up logging * unifying methods shorten test time signer also drops * reenabling pings * improving testability + unit test * fixing go fmt + unit test * remove unused code * Addressing review comments * simplifying connection workflow * fix linter/go import issue * using base service quit * updating comment * Simplifying design + adjusting names * fixing linter issues * refactoring test harness + fixes * Addressing review comments * cleaning up * adding additional error check
5 years ago
privval: refactor Remote signers (#3370) This PR is related to #3107 and a continuation of #3351 It is important to emphasise that in the privval original design, client/server and listening/dialing roles are inverted and do not follow a conventional interaction. Given two hosts A and B: Host A is listener/client Host B is dialer/server (contains the secret key) When A requires a signature, it needs to wait for B to dial in before it can issue a request. A only accepts a single connection and any failure leads to dropping the connection and waiting for B to reconnect. The original rationale behind this design was based on security. Host B only allows outbound connections to a list of whitelisted hosts. It is not possible to reach B unless B dials in. There are no listening/open ports in B. This PR results in the following changes: Refactors ping/heartbeat to avoid previously existing race conditions. Separates transport (dialer/listener) from signing (client/server) concerns to simplify workflow. Unifies and abstracts away the differences between unix and tcp sockets. A single signer endpoint implementation unifies connection handling code (read/write/close/connection obj) The signer request handler (server side) is customizable to increase testability. Updates and extends unit tests A high level overview of the classes is as follows: Transport (endpoints): The following classes take care of establishing a connection SignerDialerEndpoint SignerListeningEndpoint SignerEndpoint groups common functionality (read/write/timeouts/etc.) Signing (client/server): The following classes take care of exchanging request/responses SignerClient SignerServer This PR also closes #3601 Commits: * refactoring - work in progress * reworking unit tests * Encapsulating and fixing unit tests * Improve tests * Clean up * Fix/improve unit tests * clean up tests * Improving service endpoint * fixing unit test * fix linter issues * avoid invalid cache values (improve later?) * complete implementation * wip * improved connection loop * Improve reconnections + fixing unit tests * addressing comments * small formatting changes * clean up * Update node/node.go Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_client.go Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_client_test.go Co-Authored-By: jleni <juan.leni@zondax.ch> * check during initialization * dropping connecting when writing fails * removing break * use t.log instead * unifying and using cmn.GetFreePort() * review fixes * reordering and unifying drop connection * closing instead of signalling * refactored service loop * removed superfluous brackets * GetPubKey can return errors * Revert "GetPubKey can return errors" This reverts commit 68c06f19b4650389d7e5ab1659b318889028202c. * adding entry to changelog * Update CHANGELOG_PENDING.md Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_client.go Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_dialer_endpoint.go Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_dialer_endpoint.go Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_dialer_endpoint.go Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_dialer_endpoint.go Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_listener_endpoint_test.go Co-Authored-By: jleni <juan.leni@zondax.ch> * updating node.go * review fixes * fixes linter * fixing unit test * small fixes in comments * addressing review comments * addressing review comments 2 * reverting suggestion * Update privval/signer_client_test.go Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com> * Update privval/signer_client_test.go Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com> * Update privval/signer_listener_endpoint_test.go Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com> * do not expose brokenSignerDialerEndpoint * clean up logging * unifying methods shorten test time signer also drops * reenabling pings * improving testability + unit test * fixing go fmt + unit test * remove unused code * Addressing review comments * simplifying connection workflow * fix linter/go import issue * using base service quit * updating comment * Simplifying design + adjusting names * fixing linter issues * refactoring test harness + fixes * Addressing review comments * cleaning up * adding additional error check
5 years ago
privval: refactor Remote signers (#3370) This PR is related to #3107 and a continuation of #3351 It is important to emphasise that in the privval original design, client/server and listening/dialing roles are inverted and do not follow a conventional interaction. Given two hosts A and B: Host A is listener/client Host B is dialer/server (contains the secret key) When A requires a signature, it needs to wait for B to dial in before it can issue a request. A only accepts a single connection and any failure leads to dropping the connection and waiting for B to reconnect. The original rationale behind this design was based on security. Host B only allows outbound connections to a list of whitelisted hosts. It is not possible to reach B unless B dials in. There are no listening/open ports in B. This PR results in the following changes: Refactors ping/heartbeat to avoid previously existing race conditions. Separates transport (dialer/listener) from signing (client/server) concerns to simplify workflow. Unifies and abstracts away the differences between unix and tcp sockets. A single signer endpoint implementation unifies connection handling code (read/write/close/connection obj) The signer request handler (server side) is customizable to increase testability. Updates and extends unit tests A high level overview of the classes is as follows: Transport (endpoints): The following classes take care of establishing a connection SignerDialerEndpoint SignerListeningEndpoint SignerEndpoint groups common functionality (read/write/timeouts/etc.) Signing (client/server): The following classes take care of exchanging request/responses SignerClient SignerServer This PR also closes #3601 Commits: * refactoring - work in progress * reworking unit tests * Encapsulating and fixing unit tests * Improve tests * Clean up * Fix/improve unit tests * clean up tests * Improving service endpoint * fixing unit test * fix linter issues * avoid invalid cache values (improve later?) * complete implementation * wip * improved connection loop * Improve reconnections + fixing unit tests * addressing comments * small formatting changes * clean up * Update node/node.go Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_client.go Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_client_test.go Co-Authored-By: jleni <juan.leni@zondax.ch> * check during initialization * dropping connecting when writing fails * removing break * use t.log instead * unifying and using cmn.GetFreePort() * review fixes * reordering and unifying drop connection * closing instead of signalling * refactored service loop * removed superfluous brackets * GetPubKey can return errors * Revert "GetPubKey can return errors" This reverts commit 68c06f19b4650389d7e5ab1659b318889028202c. * adding entry to changelog * Update CHANGELOG_PENDING.md Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_client.go Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_dialer_endpoint.go Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_dialer_endpoint.go Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_dialer_endpoint.go Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_dialer_endpoint.go Co-Authored-By: jleni <juan.leni@zondax.ch> * Update privval/signer_listener_endpoint_test.go Co-Authored-By: jleni <juan.leni@zondax.ch> * updating node.go * review fixes * fixes linter * fixing unit test * small fixes in comments * addressing review comments * addressing review comments 2 * reverting suggestion * Update privval/signer_client_test.go Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com> * Update privval/signer_client_test.go Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com> * Update privval/signer_listener_endpoint_test.go Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com> * do not expose brokenSignerDialerEndpoint * clean up logging * unifying methods shorten test time signer also drops * reenabling pings * improving testability + unit test * fixing go fmt + unit test * remove unused code * Addressing review comments * simplifying connection workflow * fix linter/go import issue * using base service quit * updating comment * Simplifying design + adjusting names * fixing linter issues * refactoring test harness + fixes * Addressing review comments * cleaning up * adding additional error check
5 years ago
  1. package privval
  2. import (
  3. "fmt"
  4. "testing"
  5. "time"
  6. "github.com/stretchr/testify/assert"
  7. "github.com/stretchr/testify/require"
  8. tmrand "github.com/tendermint/tendermint/libs/rand"
  9. "github.com/tendermint/tendermint/types"
  10. )
  11. type signerTestCase struct {
  12. chainID string
  13. mockPV types.PrivValidator
  14. signerClient *SignerClient
  15. signerServer *SignerServer
  16. }
  17. func getSignerTestCases(t *testing.T) []signerTestCase {
  18. testCases := make([]signerTestCase, 0)
  19. // Get test cases for each possible dialer (DialTCP / DialUnix / etc)
  20. for _, dtc := range getDialerTestCases(t) {
  21. chainID := tmrand.Str(12)
  22. mockPV := types.NewMockPV()
  23. // get a pair of signer listener, signer dialer endpoints
  24. sl, sd := getMockEndpoints(t, dtc.addr, dtc.dialer)
  25. sc, err := NewSignerClient(sl)
  26. require.NoError(t, err)
  27. ss := NewSignerServer(sd, chainID, mockPV)
  28. err = ss.Start()
  29. require.NoError(t, err)
  30. tc := signerTestCase{
  31. chainID: chainID,
  32. mockPV: mockPV,
  33. signerClient: sc,
  34. signerServer: ss,
  35. }
  36. testCases = append(testCases, tc)
  37. }
  38. return testCases
  39. }
  40. func TestSignerClose(t *testing.T) {
  41. for _, tc := range getSignerTestCases(t) {
  42. err := tc.signerClient.Close()
  43. assert.NoError(t, err)
  44. err = tc.signerServer.Stop()
  45. assert.NoError(t, err)
  46. }
  47. }
  48. func TestSignerPing(t *testing.T) {
  49. for _, tc := range getSignerTestCases(t) {
  50. defer tc.signerServer.Stop()
  51. defer tc.signerClient.Close()
  52. err := tc.signerClient.Ping()
  53. assert.NoError(t, err)
  54. }
  55. }
  56. func TestSignerGetPubKey(t *testing.T) {
  57. for _, tc := range getSignerTestCases(t) {
  58. defer tc.signerServer.Stop()
  59. defer tc.signerClient.Close()
  60. pubKey := tc.signerClient.GetPubKey()
  61. expectedPubKey := tc.mockPV.GetPubKey()
  62. assert.Equal(t, expectedPubKey, pubKey)
  63. addr := tc.signerClient.GetPubKey().Address()
  64. expectedAddr := tc.mockPV.GetPubKey().Address()
  65. assert.Equal(t, expectedAddr, addr)
  66. }
  67. }
  68. func TestSignerProposal(t *testing.T) {
  69. for _, tc := range getSignerTestCases(t) {
  70. ts := time.Now()
  71. want := &types.Proposal{Timestamp: ts}
  72. have := &types.Proposal{Timestamp: ts}
  73. defer tc.signerServer.Stop()
  74. defer tc.signerClient.Close()
  75. require.NoError(t, tc.mockPV.SignProposal(tc.chainID, want))
  76. require.NoError(t, tc.signerClient.SignProposal(tc.chainID, have))
  77. assert.Equal(t, want.Signature, have.Signature)
  78. }
  79. }
  80. func TestSignerVote(t *testing.T) {
  81. for _, tc := range getSignerTestCases(t) {
  82. ts := time.Now()
  83. want := &types.Vote{Timestamp: ts, Type: types.PrecommitType}
  84. have := &types.Vote{Timestamp: ts, Type: types.PrecommitType}
  85. defer tc.signerServer.Stop()
  86. defer tc.signerClient.Close()
  87. require.NoError(t, tc.mockPV.SignVote(tc.chainID, want))
  88. require.NoError(t, tc.signerClient.SignVote(tc.chainID, have))
  89. assert.Equal(t, want.Signature, have.Signature)
  90. }
  91. }
  92. func TestSignerVoteResetDeadline(t *testing.T) {
  93. for _, tc := range getSignerTestCases(t) {
  94. ts := time.Now()
  95. want := &types.Vote{Timestamp: ts, Type: types.PrecommitType}
  96. have := &types.Vote{Timestamp: ts, Type: types.PrecommitType}
  97. defer tc.signerServer.Stop()
  98. defer tc.signerClient.Close()
  99. time.Sleep(testTimeoutReadWrite2o3)
  100. require.NoError(t, tc.mockPV.SignVote(tc.chainID, want))
  101. require.NoError(t, tc.signerClient.SignVote(tc.chainID, have))
  102. assert.Equal(t, want.Signature, have.Signature)
  103. // TODO(jleni): Clarify what is actually being tested
  104. // This would exceed the deadline if it was not extended by the previous message
  105. time.Sleep(testTimeoutReadWrite2o3)
  106. require.NoError(t, tc.mockPV.SignVote(tc.chainID, want))
  107. require.NoError(t, tc.signerClient.SignVote(tc.chainID, have))
  108. assert.Equal(t, want.Signature, have.Signature)
  109. }
  110. }
  111. func TestSignerVoteKeepAlive(t *testing.T) {
  112. for _, tc := range getSignerTestCases(t) {
  113. ts := time.Now()
  114. want := &types.Vote{Timestamp: ts, Type: types.PrecommitType}
  115. have := &types.Vote{Timestamp: ts, Type: types.PrecommitType}
  116. defer tc.signerServer.Stop()
  117. defer tc.signerClient.Close()
  118. // Check that even if the client does not request a
  119. // signature for a long time. The service is still available
  120. // in this particular case, we use the dialer logger to ensure that
  121. // test messages are properly interleaved in the test logs
  122. tc.signerServer.Logger.Debug("TEST: Forced Wait -------------------------------------------------")
  123. time.Sleep(testTimeoutReadWrite * 3)
  124. tc.signerServer.Logger.Debug("TEST: Forced Wait DONE---------------------------------------------")
  125. require.NoError(t, tc.mockPV.SignVote(tc.chainID, want))
  126. require.NoError(t, tc.signerClient.SignVote(tc.chainID, have))
  127. assert.Equal(t, want.Signature, have.Signature)
  128. }
  129. }
  130. func TestSignerSignProposalErrors(t *testing.T) {
  131. for _, tc := range getSignerTestCases(t) {
  132. // Replace service with a mock that always fails
  133. tc.signerServer.privVal = types.NewErroringMockPV()
  134. tc.mockPV = types.NewErroringMockPV()
  135. defer tc.signerServer.Stop()
  136. defer tc.signerClient.Close()
  137. ts := time.Now()
  138. proposal := &types.Proposal{Timestamp: ts}
  139. err := tc.signerClient.SignProposal(tc.chainID, proposal)
  140. require.Equal(t, err.(*RemoteSignerError).Description, types.ErroringMockPVErr.Error())
  141. err = tc.mockPV.SignProposal(tc.chainID, proposal)
  142. require.Error(t, err)
  143. err = tc.signerClient.SignProposal(tc.chainID, proposal)
  144. require.Error(t, err)
  145. }
  146. }
  147. func TestSignerSignVoteErrors(t *testing.T) {
  148. for _, tc := range getSignerTestCases(t) {
  149. ts := time.Now()
  150. vote := &types.Vote{Timestamp: ts, Type: types.PrecommitType}
  151. // Replace signer service privval with one that always fails
  152. tc.signerServer.privVal = types.NewErroringMockPV()
  153. tc.mockPV = types.NewErroringMockPV()
  154. defer tc.signerServer.Stop()
  155. defer tc.signerClient.Close()
  156. err := tc.signerClient.SignVote(tc.chainID, vote)
  157. require.Equal(t, err.(*RemoteSignerError).Description, types.ErroringMockPVErr.Error())
  158. err = tc.mockPV.SignVote(tc.chainID, vote)
  159. require.Error(t, err)
  160. err = tc.signerClient.SignVote(tc.chainID, vote)
  161. require.Error(t, err)
  162. }
  163. }
  164. func brokenHandler(privVal types.PrivValidator, request SignerMessage, chainID string) (SignerMessage, error) {
  165. var res SignerMessage
  166. var err error
  167. switch r := request.(type) {
  168. // This is broken and will answer most requests with a pubkey response
  169. case *PubKeyRequest:
  170. res = &PubKeyResponse{nil, nil}
  171. case *SignVoteRequest:
  172. res = &PubKeyResponse{nil, nil}
  173. case *SignProposalRequest:
  174. res = &PubKeyResponse{nil, nil}
  175. case *PingRequest:
  176. err, res = nil, &PingResponse{}
  177. default:
  178. err = fmt.Errorf("unknown msg: %v", r)
  179. }
  180. return res, err
  181. }
  182. func TestSignerUnexpectedResponse(t *testing.T) {
  183. for _, tc := range getSignerTestCases(t) {
  184. tc.signerServer.privVal = types.NewMockPV()
  185. tc.mockPV = types.NewMockPV()
  186. tc.signerServer.SetRequestHandler(brokenHandler)
  187. defer tc.signerServer.Stop()
  188. defer tc.signerClient.Close()
  189. ts := time.Now()
  190. want := &types.Vote{Timestamp: ts, Type: types.PrecommitType}
  191. e := tc.signerClient.SignVote(tc.chainID, want)
  192. assert.EqualError(t, e, "received unexpected response")
  193. }
  194. }