Skip to content

Commit

Permalink
Properly close connections in the proxy.
Browse files Browse the repository at this point in the history
The proxy was doing a bunch of work to properly call ClientClose on streams when clients had no more messages, but it neglected to close the underlying ClientConn used by the stream. This leaked the stream on each outwards dial to a sansshell server, eventually accumulating many client connections all doing nothing but keepalive.

To fix this, I'm closing the connection to a server after we've finished all send/recv calls with the server and we're about to return the final ServerClose message back to the client.

This is reproducable by launching the proxy+server and making a bunch of calls

```
% go run ./cmd/sansshell-server
% go run ./cmd/proxy-server
% for f in $(seq 20); do ./sanssh --proxy=localhost:50043 --targets=localhost:50042 file read /etc/hosts& done
```

Then watch the keepalive goroutine count at http://localhost:50044/debug/pprof/goroutine?debug=1 go up and up.

I've updated the README a bit so that it has better instructions for running the proxy.
  • Loading branch information
sfc-gh-srhodes committed Sep 18, 2023
1 parent 0bda475 commit 3ff397a
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 13 deletions.
39 changes: 29 additions & 10 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,32 @@ the project root directory.
Building SansShell requires a recent version of Go (check the go.mod file for
the current version).

## Build and run

You need to populate ~/.sansshell with certificates before running.

```
$ cp -r auth/mtls/testdata ~/.sanshell
```

Or copy the test certificates from auth/mtls/testdata to ~/.sanshell

Then you can build and run the server, in separate terminal windows:
```
$ go run ./cmd/sansshell-server
$ go run ./cmd/sanssh --targets=localhost file read /etc/hosts
```

You can also run the proxy to try the full flow:

```
$ go run ./cmd/sansshell-server
$ go run ./cmd/proxy-server
$ go run ./cmd/sanssh --proxy=localhost:50043 --targets=localhost:50042 file read /etc/hosts
```

Minimal debugging UIs are available at http://localhost:50044 for the server and http://localhost:50046 for the proxy by default.

## Environment setup : protoc

When making any change to the protocol buffers, you'll also need the protocol
Expand Down Expand Up @@ -100,8 +126,9 @@ do this for you, as well as re-generating the service proto files.
$ go generate tools.go
```

## Build and run
You only need to do these steps once to configure example mTLS certs:
## Creating your own certificates

As an alternative to copying auth/mtls/testdata, you can create your onwn example mTLS certs:
```
$ go install github.com/meterup/generate-cert@latest
$ mkdir -m 0700 certs
Expand All @@ -111,14 +138,6 @@ $ cd ../
$ ln -s $(pwd)/certs ~/.sansshell
```

Or copy the test certificates from auth/mtls/testdata to ~/.sanshell

Then you can build and run the server, in separate terminal windows:
```
$ cd cmd/sansshell-server && go build && ./sansshell-server
$ cd cmd/sanssh && go build && ./sanssh --targets=localhost file read /etc/hosts
```

## Debugging
Reflection is included in the RPC servers (proxy and sansshell-server)
allowing for the use of [grpc_cli](https://github.com/grpc/grpc/blob/master/doc/command_line_tool.md).
Expand Down
10 changes: 8 additions & 2 deletions proxy/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,13 @@ var (
// connections (such as client credentials, deadlines, etc) which
// the proxy can use without needing to understand them.
type TargetDialer interface {
DialContext(ctx context.Context, target string, dialOpts ...grpc.DialOption) (grpc.ClientConnInterface, error)
DialContext(ctx context.Context, target string, dialOpts ...grpc.DialOption) (ClientConnCloser, error)
}

// ClientConnCloser is a closeable ClientConnInterface
type ClientConnCloser interface {
grpc.ClientConnInterface
Close() error
}

// an optionsDialer implements TargetDialer using native grpc.Dial
Expand All @@ -54,7 +60,7 @@ type optionsDialer struct {
}

// See TargetDialer.DialContext
func (o *optionsDialer) DialContext(ctx context.Context, target string, dialOpts ...grpc.DialOption) (grpc.ClientConnInterface, error) {
func (o *optionsDialer) DialContext(ctx context.Context, target string, dialOpts ...grpc.DialOption) (ClientConnCloser, error) {
opts := o.opts
opts = append(opts, dialOpts...)
return grpc.DialContext(ctx, target, opts...)
Expand Down
8 changes: 7 additions & 1 deletion proxy/server/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ type TargetStream struct {
serviceMethod *ServiceMethod

// The underlying grpc.ClientConnInterface to the target server
grpcConn grpc.ClientConnInterface
grpcConn ClientConnCloser

// The underlying grpc.ClientStream to the target server.
grpcStream grpc.ClientStream
Expand Down Expand Up @@ -323,6 +323,12 @@ func (s *TargetStream) Run(nonce uint32, replyChan chan *pb.ProxyReply) {
// a server-close call
err := group.Wait()

// Once all calls are complete, we need to close our network connection
// to the server.
if closeErr := s.grpcConn.Close(); err == nil && closeErr != nil {
err = closeErr
}

// The error status may by set/overidden if CloseWith was used to
// terminate the stream.
select {
Expand Down
Binary file added sanssh
Binary file not shown.

0 comments on commit 3ff397a

Please sign in to comment.