Skip to content

Commit

Permalink
fix: side-effect for testing by allowing any connection if remote add…
Browse files Browse the repository at this point in the history
…ress is empty
  • Loading branch information
jaeseung-bae committed Aug 8, 2023
1 parent 041f37a commit 59838a0
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 12 deletions.
2 changes: 2 additions & 0 deletions cmd/ostracon/commands/show_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package commands
import (
"bytes"
"os"
"strings"
"sync"
"testing"

Expand Down Expand Up @@ -79,6 +80,7 @@ func TestShowValidatorWithKMS(t *testing.T) {
}
privval.WithMockKMS(t, dir, chainID, func(addr string, privKey crypto.PrivKey) {
config.PrivValidatorListenAddr = addr
config.PrivValidatorRemoteAddr = addr[:strings.Index(addr, ":")]
require.NoFileExists(t, config.PrivValidatorKeyFile())
output, err := captureStdout(func() {
err := showValidator(ShowValidatorCmd, nil, config)
Expand Down
25 changes: 15 additions & 10 deletions privval/signer_listener_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ type SignerListenerEndpoint struct {

instanceMtx tmsync.Mutex // Ensures instance public methods access, i.e. SendRequest

allowAddr string
allowAddr string // empty value allows all
}

// NewSignerListenerEndpoint returns an instance of SignerListenerEndpoint.
Expand Down Expand Up @@ -195,14 +195,13 @@ func (sl *SignerListenerEndpoint) serviceLoop() {
case <-sl.connectRequestCh:
{
conn, err := sl.acceptNewConnection()
remoteAddr := conn.RemoteAddr()
if !sl.isAllowedAddr(remoteAddr) {
sl.Logger.Info(fmt.Sprintf("deny a connection request from remote address=%s", remoteAddr))
conn.Close()
continue
}

if err == nil {
remoteAddr := conn.RemoteAddr()
if !sl.isAllowedAddr(remoteAddr) {
sl.Logger.Info(fmt.Sprintf("deny a connection request from remote address=%s", remoteAddr))
conn.Close()
continue

Check warning on line 203 in privval/signer_listener_endpoint.go

View check run for this annotation

Codecov / codecov/patch

privval/signer_listener_endpoint.go#L201-L203

Added lines #L201 - L203 were not covered by tests
}
sl.Logger.Info("SignerListener: Connected")

// We have a good connection, wait for someone that needs one otherwise cancellation
Expand All @@ -225,8 +224,14 @@ func (sl *SignerListenerEndpoint) serviceLoop() {
}

func (sl *SignerListenerEndpoint) isAllowedAddr(addr net.Addr) bool {
addrOnly := addr.String()[:strings.Index(addr.String(), ":")]
return sl.allowAddr == addrOnly
if len(sl.allowAddr) == 0 {
return true
}
if strings.Contains(addr.String(), ":") {
addrOnly := addr.String()[:strings.Index(addr.String(), ":")]
return sl.allowAddr == addrOnly
}
return sl.allowAddr == addr.String()

Check warning on line 234 in privval/signer_listener_endpoint.go

View check run for this annotation

Codecov / codecov/patch

privval/signer_listener_endpoint.go#L234

Added line #L234 was not covered by tests
}

func (sl *SignerListenerEndpoint) pingLoop() {
Expand Down
4 changes: 2 additions & 2 deletions privval/signer_listener_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,12 +184,12 @@ func TestFilterRemoteConnectionByIP(t *testing.T) {
}{"127.0.0.1", addrStub{"10.0.0.2:45678"}, false},
},
{
"empty allowIP should deny all",
"empty allowIP should allow all",
struct {
allowIP string
remoteAddr net.Addr
expected bool
}{"", addrStub{"127.0.0.1:45678"}, false},
}{"", addrStub{"127.0.0.1:45678"}, true},
},
}
for _, tt := range tests {
Expand Down

0 comments on commit 59838a0

Please sign in to comment.