Juan Leni f7f034a8be 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
2019-08-05 19:09:10 +04:00

195 lines
4.7 KiB
Go

package internal
import (
"fmt"
"io/ioutil"
"os"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/tendermint/tendermint/crypto"
"github.com/tendermint/tendermint/crypto/ed25519"
"github.com/tendermint/tendermint/libs/log"
"github.com/tendermint/tendermint/privval"
"github.com/tendermint/tendermint/types"
)
const (
keyFileContents = `{
"address": "D08FCA3BA74CF17CBFC15E64F9505302BB0E2748",
"pub_key": {
"type": "tendermint/PubKeyEd25519",
"value": "ZCsuTjaczEyon70nmKxwvwu+jqrbq5OH3yQjcK0SFxc="
},
"priv_key": {
"type": "tendermint/PrivKeyEd25519",
"value": "8O39AkQsoe1sBQwud/Kdul8lg8K9SFsql9aZvwXQSt1kKy5ONpzMTKifvSeYrHC/C76Oqturk4ffJCNwrRIXFw=="
}
}`
stateFileContents = `{
"height": "0",
"round": "0",
"step": 0
}`
genesisFileContents = `{
"genesis_time": "2019-01-15T11:56:34.8963Z",
"chain_id": "test-chain-0XwP5E",
"consensus_params": {
"block": {
"max_bytes": "22020096",
"max_gas": "-1",
"time_iota_ms": "1000"
},
"evidence": {
"max_age": "100000"
},
"validator": {
"pub_key_types": [
"ed25519"
]
}
},
"validators": [
{
"address": "D08FCA3BA74CF17CBFC15E64F9505302BB0E2748",
"pub_key": {
"type": "tendermint/PubKeyEd25519",
"value": "ZCsuTjaczEyon70nmKxwvwu+jqrbq5OH3yQjcK0SFxc="
},
"power": "10",
"name": ""
}
],
"app_hash": ""
}`
defaultConnDeadline = 100
)
func TestRemoteSignerTestHarnessMaxAcceptRetriesReached(t *testing.T) {
cfg := makeConfig(t, 1, 2)
defer cleanup(cfg)
th, err := NewTestHarness(log.TestingLogger(), cfg)
require.NoError(t, err)
th.Run()
assert.Equal(t, ErrMaxAcceptRetriesReached, th.exitCode)
}
func TestRemoteSignerTestHarnessSuccessfulRun(t *testing.T) {
harnessTest(
t,
func(th *TestHarness) *privval.SignerServer {
return newMockSignerServer(t, th, th.fpv.Key.PrivKey, false, false)
},
NoError,
)
}
func TestRemoteSignerPublicKeyCheckFailed(t *testing.T) {
harnessTest(
t,
func(th *TestHarness) *privval.SignerServer {
return newMockSignerServer(t, th, ed25519.GenPrivKey(), false, false)
},
ErrTestPublicKeyFailed,
)
}
func TestRemoteSignerProposalSigningFailed(t *testing.T) {
harnessTest(
t,
func(th *TestHarness) *privval.SignerServer {
return newMockSignerServer(t, th, th.fpv.Key.PrivKey, true, false)
},
ErrTestSignProposalFailed,
)
}
func TestRemoteSignerVoteSigningFailed(t *testing.T) {
harnessTest(
t,
func(th *TestHarness) *privval.SignerServer {
return newMockSignerServer(t, th, th.fpv.Key.PrivKey, false, true)
},
ErrTestSignVoteFailed,
)
}
func newMockSignerServer(t *testing.T, th *TestHarness, privKey crypto.PrivKey, breakProposalSigning bool, breakVoteSigning bool) *privval.SignerServer {
mockPV := types.NewMockPVWithParams(privKey, breakProposalSigning, breakVoteSigning)
dialerEndpoint := privval.NewSignerDialerEndpoint(
th.logger,
privval.DialTCPFn(
th.addr,
time.Duration(defaultConnDeadline)*time.Millisecond,
ed25519.GenPrivKey(),
),
)
return privval.NewSignerServer(dialerEndpoint, th.chainID, mockPV)
}
// For running relatively standard tests.
func harnessTest(t *testing.T, signerServerMaker func(th *TestHarness) *privval.SignerServer, expectedExitCode int) {
cfg := makeConfig(t, 100, 3)
defer cleanup(cfg)
th, err := NewTestHarness(log.TestingLogger(), cfg)
require.NoError(t, err)
donec := make(chan struct{})
go func() {
defer close(donec)
th.Run()
}()
ss := signerServerMaker(th)
require.NoError(t, ss.Start())
assert.True(t, ss.IsRunning())
defer ss.Stop()
<-donec
assert.Equal(t, expectedExitCode, th.exitCode)
}
func makeConfig(t *testing.T, acceptDeadline, acceptRetries int) TestHarnessConfig {
return TestHarnessConfig{
BindAddr: privval.GetFreeLocalhostAddrPort(),
KeyFile: makeTempFile("tm-testharness-keyfile", keyFileContents),
StateFile: makeTempFile("tm-testharness-statefile", stateFileContents),
GenesisFile: makeTempFile("tm-testharness-genesisfile", genesisFileContents),
AcceptDeadline: time.Duration(acceptDeadline) * time.Millisecond,
ConnDeadline: time.Duration(defaultConnDeadline) * time.Millisecond,
AcceptRetries: acceptRetries,
SecretConnKey: ed25519.GenPrivKey(),
ExitWhenComplete: false,
}
}
func cleanup(cfg TestHarnessConfig) {
os.Remove(cfg.KeyFile)
os.Remove(cfg.StateFile)
os.Remove(cfg.GenesisFile)
}
func makeTempFile(name, content string) string {
tempFile, err := ioutil.TempFile("", fmt.Sprintf("%s-*", name))
if err != nil {
panic(err)
}
if _, err := tempFile.Write([]byte(content)); err != nil {
tempFile.Close()
panic(err)
}
if err := tempFile.Close(); err != nil {
panic(err)
}
return tempFile.Name()
}