Skip to content

Commit

Permalink
Add listener that allows remote connections without mTLS (#5418)
Browse files Browse the repository at this point in the history
## Motivation
Adds a new listener to allow remote post services to connect if they are within the same private network without having to setup mTLS.

## Changes
- add new `grpcPostServer` that allows remote connection to a post service without setting up mTLS
  - Post is now available on `grpcPostServer` (default: 0.0.0.0:9094) to allow exposing it without mTLS and
  - on `grpcTLSServer` (no default listening address) to allow exposing over insecure networks (to allow transmitting keys securely between node and post service in the future)
- GRPC services are now true singletons - simplified code to instantiate and register in `node.go` during startup
- updated GRPC `PostService` to allow connections from multiple post services. (prep. work for multiple smeshers)
  - `nipostBuilder` / `atxBuilder` can select the required client based on the node Id the post service reports after connecting
  - only one post service can connect per Node ID
- Merged similar functionality of `grpc.NewPublic` and `grpc.NewPrivate` into one `grpc.NewWithServices`
  - function checks if listening IP is in a public network and prints a waring (displayed during startup) to ensure to be careful not to expose it over public networks
- Expanded system tests to automatically add nodes with remote smeshing setups to all tests.
  - By default 25% of all nodes will be deployed in a remote setup (i.e. with a separate pod running the post service remotely and connecting to the node)
  - The node running the post service has an init container where `postcli` is used to create the post data with the same setting as the node would use in a supervised setup. Those settings are picked from the `smesherConfig` that is deployed with every node and translated into parameters for `postcli` / `post-service`
  - systest Makefile has been updated with image names and tags for `post-service` and `postcli`
  - systest Dockerfile has been expanded to include `libpost`. This has unfortunately become necessary to parse the config used during system tests with the same code as we use during the startup of the node (to ensure compatibility with changing configs)
- Fixed a few minor issues with existing system tests
  - tests now generate the same number of keys as nodes are used during the test (1 per node)
  - `AddBootnodes` didn't keep track of the correct number of bootnodes after deployment

## Test Plan
- Extended existing system tests to deploy 1/4 of the nodes during the test with a remote post service.

## TODO
<!-- This section should be removed when all items are complete -->
- [x] Explain motivation or link existing issue(s)
- [x] Test changes and document test plan
- [x] Update documentation as needed
- [x] Update [changelog](../CHANGELOG.md) as needed
  • Loading branch information
fasmat committed Jan 22, 2024
1 parent dd1fa87 commit 0e4dba2
Show file tree
Hide file tree
Showing 28 changed files with 688 additions and 176 deletions.
19 changes: 19 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,18 @@ See [RELEASE](./RELEASE.md) for workflow instructions.

### Upgrade information

#### Post service endpoint

The post service now has its own endpoint separate from `grpc-private-listener`. The default for `grpc-post-listener`
is `127.0.0.1:9094`. In contrast to `grpc-tls-listener` this endpoint does not require setting up mTLS.

The post service cannot connect to `grpc-private-listener` anymore. If you are using a remote smeshing setup please
adjust your configuration accordingly. If you are using a remote setup with mTLS over a private network you can switch
to using `grpc-post-listener` to not require the overhead of mTLS. We however strongly recommend using an mTLS
encrypted connection between the post service and the node over insecure connections (e.g. over the Internet).

Smeshers using the default setup with a supervised post service do not need to make changes to their node configuration.

#### New poets configuration

Upgrading requires changes in config and in CLI flags (if not using the default).
Expand Down Expand Up @@ -111,6 +123,13 @@ configuration is as follows:
Also, remove unnecessary wait for ATXs to be synced before beginning initialization if
the commitment ATX is already selected.

* [#5418](https://github.com/spacemeshos/go-spacemesh/pull/5418) Add `grpc-post-listener` to separate post service from
`grpc-private-listener` and not require mTLS for the post service.

If you are not using a remote post service you do not need to adjust anything. If you are using a remote setup
make sure your post service now connects to `grpc-post-listener` instead of `grpc-private-listener`. If you are
connecting to a remote post service over the internet we strongly recommend using mTLS via `grpc-tls-listener`.

## Release v1.3.2

### Improvements
Expand Down
2 changes: 1 addition & 1 deletion activation/post_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
type PowDifficulty [32]byte

func (d PowDifficulty) String() string {
return fmt.Sprintf("%X", d[:])
return fmt.Sprintf("%x", d[:])
}

// Set implements pflag.Value.Set.
Expand Down
23 changes: 14 additions & 9 deletions api/grpcserver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,16 @@ type Config struct {
PublicListener string `mapstructure:"grpc-public-listener"`
PrivateServices []Service
PrivateListener string `mapstructure:"grpc-private-listener"`
TLSServices []Service
TLSListener string `mapstructure:"grpc-tls-listener"`
TLSCACert string `mapstructure:"grpc-tls-ca-cert"`
TLSCert string `mapstructure:"grpc-tls-cert"`
TLSKey string `mapstructure:"grpc-tls-key"`
GrpcSendMsgSize int `mapstructure:"grpc-send-msg-size"`
GrpcRecvMsgSize int `mapstructure:"grpc-recv-msg-size"`
JSONListener string `mapstructure:"grpc-json-listener"`
PostServices []Service
PostListener string `mapstructure:"grpc-post-listener"`
TLSServices []Service `mapstructure:"grpc-tls-services"`
TLSListener string `mapstructure:"grpc-tls-listener"`
TLSCACert string `mapstructure:"grpc-tls-ca-cert"`
TLSCert string `mapstructure:"grpc-tls-cert"`
TLSKey string `mapstructure:"grpc-tls-key"`
GrpcSendMsgSize int `mapstructure:"grpc-send-msg-size"`
GrpcRecvMsgSize int `mapstructure:"grpc-recv-msg-size"`
JSONListener string `mapstructure:"grpc-json-listener"`

SmesherStreamInterval time.Duration
}
Expand All @@ -41,8 +43,10 @@ func DefaultConfig() Config {
return Config{
PublicServices: []Service{GlobalState, Mesh, Transaction, Node, Activation},
PublicListener: "0.0.0.0:9092",
PrivateServices: []Service{Admin, Smesher, Debug, Post},
PrivateServices: []Service{Admin, Smesher, Debug},
PrivateListener: "127.0.0.1:9093",
PostServices: []Service{Post},
PostListener: "127.0.0.1:9094",
TLSServices: []Service{Post},
TLSListener: "",
JSONListener: "",
Expand All @@ -57,6 +61,7 @@ func DefaultTestConfig() Config {
conf := DefaultConfig()
conf.PublicListener = "127.0.0.1:0"
conf.PrivateListener = "127.0.0.1:0"
conf.PostListener = "127.0.0.1:0"
conf.JSONListener = ""
conf.TLSListener = ""
return conf
Expand Down
24 changes: 10 additions & 14 deletions api/grpcserver/grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,28 +58,24 @@ func streamingGrpcLogStart(
return handler(srv, stream)
}

// NewPublic creates a new Server listening on the PublicListener address with the given logger and config.
// NewWithServices creates a new Server listening on the provided address with the given logger and config.
// Services passed in the svc slice are registered with the server.
func NewPublic(logger *zap.Logger, config Config, svc []ServiceAPI) (*Server, error) {
func NewWithServices(listener string, logger *zap.Logger, config Config, svc []ServiceAPI) (*Server, error) {
if len(svc) == 0 {
return nil, errors.New("no services to register")
}

server := New(config.PublicListener, logger, config)
for _, s := range svc {
s.RegisterService(server.GrpcServer)
// check if listener IP is in private network range
host, _, err := net.SplitHostPort(listener)
if err != nil {
return nil, fmt.Errorf("split local listener: %w", err)
}
return server, nil
}

// NewPrivate creates new Server listening on the PrivateListener address with the given logger and config.
// Services passed in the svc slice are registered with the server.
func NewPrivate(logger *zap.Logger, config Config, svc []ServiceAPI) (*Server, error) {
if len(svc) == 0 {
return nil, errors.New("no services to register")
ip := net.ParseIP(host)
if host != "localhost" && !ip.IsPrivate() && !ip.IsLoopback() {
logger.Warn("unsecured grpc server is listening on a public IP address", zap.String("address", listener))
}

server := New(config.PrivateListener, logger, config)
server := New(listener, logger, config)
for _, s := range svc {
s.RegisterService(server.GrpcServer)
}
Expand Down
63 changes: 60 additions & 3 deletions api/grpcserver/grpcserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"go.uber.org/zap/zaptest"
"go.uber.org/zap/zaptest/observer"
"golang.org/x/sync/errgroup"
"google.golang.org/genproto/googleapis/rpc/code"
"google.golang.org/grpc"
Expand Down Expand Up @@ -450,9 +453,7 @@ func newChallenge(sequence uint64, prevAtxID, posAtxID types.ATXID, epoch types.

func launchServer(tb testing.TB, services ...ServiceAPI) (Config, func()) {
cfg := DefaultTestConfig()
cfg.PublicListener = "127.0.0.1:0" // run on random port

grpcService, err := NewPublic(zaptest.NewLogger(tb).Named("grpc"), cfg, services)
grpcService, err := NewWithServices(cfg.PublicListener, zaptest.NewLogger(tb).Named("grpc"), cfg, services)
require.NoError(tb, err)

// start gRPC server
Expand Down Expand Up @@ -490,6 +491,62 @@ func TestNewServersConfig(t *testing.T) {
require.Contains(t, jsonService.listener, strconv.Itoa(port2), "Expected same port")
}

func TestNewLocalServer(t *testing.T) {
tt := []struct {
name string
listener string
warn bool
}{
{
name: "valid",
listener: "192.168.1.1:1234",
warn: false,
},
{
name: "valid random port",
listener: "10.0.0.1:0",
warn: false,
},
{
name: "invalid",
listener: "0.0.0.0:1234",
warn: true,
},
{
name: "invalid random port",
listener: "88.77.66.11:0",
warn: true,
},
}

for _, tc := range tt {
t.Run(tc.name, func(t *testing.T) {
observer, observedLogs := observer.New(zapcore.WarnLevel)
logger := zap.New(observer)

ctrl := gomock.NewController(t)
peerCounter := NewMockpeerCounter(ctrl)
meshApi := NewMockmeshAPI(ctrl)
genTime := NewMockgenesisTimeAPI(ctrl)
syncer := NewMocksyncer(ctrl)

cfg := DefaultTestConfig()
cfg.PostListener = tc.listener
svc := NewNodeService(peerCounter, meshApi, genTime, syncer, "v0.0.0", "cafebabe")
grpcService, err := NewWithServices(cfg.PostListener, logger, cfg, []ServiceAPI{svc})
if tc.warn {
require.Equal(t, observedLogs.Len(), 1, "Expected a warning log")
require.Equal(t, observedLogs.All()[0].Message, "unsecured grpc server is listening on a public IP address")
require.Equal(t, observedLogs.All()[0].ContextMap()["address"], tc.listener)
return
}

require.NoError(t, err)
require.Equal(t, grpcService.listener, tc.listener, "expected same listener")
})
}
}

type smesherServiceConn struct {
pb.SmesherServiceClient

Expand Down
51 changes: 36 additions & 15 deletions api/grpcserver/post_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ type PostService struct {
log *zap.Logger

clientMtx sync.Mutex
client *postClient
client map[types.NodeID]*postClient
}

type postCommand struct {
Expand All @@ -46,7 +46,8 @@ func (s *PostService) String() string {
// NewPostService creates a new grpc service using config data.
func NewPostService(log *zap.Logger) *PostService {
return &PostService{
log: log,
log: log,
client: make(map[types.NodeID]*postClient),
}
}

Expand All @@ -55,11 +56,32 @@ func NewPostService(log *zap.Logger) *PostService {
// The other functions on this service are called by services of the node to send
// requests to the PoST node and receive responses.
func (s *PostService) Register(stream pb.PostService_RegisterServer) error {
err := stream.SendMsg(&pb.NodeRequest{
Kind: &pb.NodeRequest_Metadata{
Metadata: &pb.MetadataRequest{},
},
})
if err != nil {
return fmt.Errorf("failed to send metadata request: %w", err)
}
resp, err := stream.Recv()
if err != nil {
return fmt.Errorf("failed to receive response: %w", err)
}
metadataResp := resp.GetMetadata()
if metadataResp == nil {
return fmt.Errorf("expected metadata response, got %T", resp.Kind)
}
meta := metadataResp.GetMeta()
if meta == nil {
return fmt.Errorf("expected metadata, got empty response")
}

con := make(chan postCommand)
if err := s.setConnection(con); err != nil {
if err := s.setConnection(types.BytesToNodeID(meta.NodeId), con); err != nil {
return err
}
defer s.dropConnection()
defer s.dropConnection(types.BytesToNodeID(meta.NodeId))

for {
select {
Expand All @@ -81,36 +103,35 @@ func (s *PostService) Register(stream pb.PostService_RegisterServer) error {
}
}

func (s *PostService) setConnection(con chan postCommand) error {
func (s *PostService) setConnection(nodeId types.NodeID, con chan postCommand) error {
s.clientMtx.Lock()
defer s.clientMtx.Unlock()

if s.client != nil {
if _, ok := s.client[nodeId]; ok {
return fmt.Errorf("post service already registered")
}

s.client = newPostClient(con)
s.log.Info("post service registered")
s.client[nodeId] = newPostClient(con)
s.log.Info("post service registered", zap.Stringer("node_id", nodeId))
return nil
}

func (s *PostService) dropConnection() error {
func (s *PostService) dropConnection(nodeId types.NodeID) error {
s.clientMtx.Lock()
defer s.clientMtx.Unlock()

err := s.client.Close()
s.client = nil
err := s.client[nodeId].Close()
delete(s.client, nodeId)
return err
}

func (s *PostService) Client(nodeId types.NodeID) (activation.PostClient, error) {
s.clientMtx.Lock()
defer s.clientMtx.Unlock()

// TODO(mafa): select correct client based on node id
if s.client == nil {
client, ok := s.client[nodeId]
if !ok {
return nil, fmt.Errorf("post service not registered")
}

return s.client, nil
return client, nil
}
6 changes: 5 additions & 1 deletion cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,12 @@ func AddCommands(cmd *cobra.Command) {
cfg.API.PublicListener, "Socket for grpc services that are save to expose publicly.")
cmd.PersistentFlags().StringVar(&cfg.API.PrivateListener, "grpc-private-listener",
cfg.API.PrivateListener, "Socket for grpc services that are not safe to expose publicly.")
cmd.PersistentFlags().StringVar(&cfg.API.PostListener, "grpc-post-listener", cfg.API.PostListener,
"Socket on which the node listens for post service connections.")
cmd.PersistentFlags().StringSliceVar(&cfg.API.TLSServices, "grpc-tls-services",
cfg.API.TLSServices, "List of services that to be exposed via TLS Listener.")
cmd.PersistentFlags().StringVar(&cfg.API.TLSListener, "grpc-tls-listener",
cfg.API.TLSListener, "Socket for the grpc services that need to be accessible via mTLS.")
cfg.API.TLSListener, "Socket for the grpc services using mTLS.")
cmd.PersistentFlags().StringVar(&cfg.API.TLSCACert, "gprc-tls-ca-cert",
cfg.API.TLSCACert, "Path to the file containing the CA certificate for mTLS.")
cmd.PersistentFlags().StringVar(&cfg.API.TLSCert, "grpc-tls-cert",
Expand Down
1 change: 1 addition & 0 deletions config/presets/standalone.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,5 +87,6 @@ func standalone() config.Config {

conf.API.PublicListener = "0.0.0.0:10092"
conf.API.PrivateListener = "127.0.0.1:10093"
conf.API.PostListener = "127.0.0.1:10094"
return conf
}
2 changes: 1 addition & 1 deletion node/adminservice_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func TestPeerInfoApi(t *testing.T) {

cfg.API.PublicListener = "0.0.0.0:0"
cfg.API.PrivateServices = nil
cfg.API.PublicServices = []string{grpcserver.Admin}
cfg.API.PublicServices = []grpcserver.Service{grpcserver.Admin}
l := logtest.New(t)
networkSize := 3
network := NewTestNetwork(t, cfg, l, networkSize)
Expand Down
Loading

0 comments on commit 0e4dba2

Please sign in to comment.