User-manual ‘disconnect’ adds a peer to a blacklist

modules/gateway.go

@@ -162,9 +162,15 @@ type (
 		// Connect establishes a persistent connection to a peer.
 		Connect(NetAddress) error
 
+		// ConnectManual is a Connect wrapper for a user-initiated Connect
+		ConnectManual(NetAddress) error
+
 		// Disconnect terminates a connection to a peer.
 		Disconnect(NetAddress) error
 
+		// DisconnectManual is a Disconnect wrapper for a user-initiated disconnect
+		DisconnectManual(NetAddress) error
+
 		// Address returns the Gateway's address.
 		Address() NetAddress
 

modules/gateway/gateway.go

@@ -126,6 +126,8 @@ type Gateway struct {
 	handlers map[rpcID]modules.RPCFunc
 	initRPCs map[string]modules.RPCFunc
 
+	// blacklist are peers that the gateway shouldn't connect to
+	//
 	// nodes is the set of all known nodes (i.e. potential peers).
 	//
 	// peers are the nodes that the gateway is currently connected to.
@@ -141,9 +143,10 @@ type Gateway struct {
 	// and would block any threads.Flush() calls. So a second threadgroup is
 	// added which handles clean-shutdown for the peers, without blocking
 	// threads.Flush() calls.
-	nodes  map[modules.NetAddress]*node
-	peers  map[modules.NetAddress]*peer
-	peerTG siasync.ThreadGroup
+	blacklist map[string]struct{}
+	nodes     map[modules.NetAddress]*node
+	peers     map[modules.NetAddress]*peer
+	peerTG    siasync.ThreadGroup
 
 	// Utilities.
 	log        *persist.Logger
@@ -198,8 +201,9 @@ func New(addr string, bootstrap bool, persistDir string) (*Gateway, error) {
 		handlers: make(map[rpcID]modules.RPCFunc),
 		initRPCs: make(map[string]modules.RPCFunc),
 
-		nodes: make(map[modules.NetAddress]*node),
-		peers: make(map[modules.NetAddress]*peer),
+		blacklist: make(map[string]struct{}),
+		nodes:     make(map[modules.NetAddress]*node),
+		peers:     make(map[modules.NetAddress]*peer),
 
 		persistDir: persistDir,
 	}
@@ -245,6 +249,10 @@ func New(addr string, bootstrap bool, persistDir string) (*Gateway, error) {
 	if loadErr := g.load(); loadErr != nil && !os.IsNotExist(loadErr) {
 		return nil, loadErr
 	}
+	// Do the same for the blacklist.
+	if loadErr := g.loadBlacklist(); loadErr != nil && !os.IsNotExist(loadErr) {
+		return nil, loadErr
+	}
 	// Spawn the thread to periodically save the gateway.
 	go g.threadedSaveLoop()
 	// Make sure that the gateway saves after shutdown.

modules/gateway/gateway_test.go

@@ -8,6 +8,7 @@ import (
 	"strconv"
 	"sync"
 	"testing"
+	"time"
 
 	"github.com/NebulousLabs/Sia/build"
 	"github.com/NebulousLabs/Sia/modules"
@@ -209,3 +210,132 @@ func TestParallelClose(t *testing.T) {
 	}
 	wg.Wait()
 }
+
+// TestManualConnectDisconnect checks if a user initiated connect and
+// disconnect works as expected.
+func TestManualConnectDisconnect(t *testing.T) {
+	if testing.Short() {
+		t.SkipNow()
+	}
+	t.Parallel()
+	g1 := newNamedTestingGateway(t, "1")
+	defer g1.Close()
+	g2 := newNamedTestingGateway(t, "2")
+	defer g2.Close()
+
+	// g1 should be able to connect to g2
+	err := g1.Connect(g2.Address())
+	if err != nil {
+		t.Fatal("failed to connect:", err)
+	}
+
+	// g2 manually disconnects from g1 and therefore blacklists it
+	err = g2.DisconnectManual(g1.Address())
+	if err != nil {
+		t.Fatal("failed to disconnect:", err)
+	}
+	time.Sleep(time.Second)
+
+	// Neither g1 nor g2 can connect after g1 being blacklisted
+	err = g1.Connect(g2.Address())
+	if err == nil {
+		t.Fatal("shouldn't be able to connect")
+	}
+	err = g1.ConnectManual(g2.Address())
+	if err == nil {
+		t.Fatal("shouldn't be able to connect")
+	}
+	err = g2.Connect(g1.Address())
+	if err == nil {
+		t.Fatal("shouldn't be able to connect")
+	}
+
+	// g2 manually connects and therefore removes g1 from the blacklist again
+	err = g2.ConnectManual(g1.Address())
+	if err != nil {
+		t.Fatal("failed to connect:", err)
+	}
+
+	// g2 disconnects and lets g1 connect which should also be possible now
+	err = g2.Disconnect(g1.Address())
+	if err != nil {
+		t.Fatal("failed to disconnect:", err)
+	}
+	time.Sleep(time.Second)
+	err = g1.Connect(g2.Address())
+	if err != nil {
+		t.Fatal("failed to connect:", err)
+	}
+
+	// same thing again but the other way round
+	err = g2.Disconnect(g1.Address())
+	if err != nil {
+		t.Fatal("failed to disconnect:", err)
+	}
+	time.Sleep(time.Second)
+	err = g2.Connect(g1.Address())
+	if err != nil {
+		t.Fatal("failed to connect:", err)
+	}
+}
+
+// TestManualConnectDisconnectPersist checks if the blacklist is persistet on
+// disk
+func TestManualConnectDisconnectPersist(t *testing.T) {
+	if testing.Short() {
+		t.SkipNow()
+	}
+	t.Parallel()
+	g1 := newNamedTestingGateway(t, "1")
+	defer g1.Close()
+	g2 := newNamedTestingGateway(t, "2")
+
+	// g1 should be able to connect to g2
+	err := g1.Connect(g2.Address())
+	if err != nil {
+		t.Fatal("failed to connect:", err)
+	}
+
+	// g2 manually disconnects from g1 and therefore blacklists it
+	err = g2.DisconnectManual(g1.Address())
+	if err != nil {
+		t.Fatal("failed to disconnect:", err)
+	}
+	time.Sleep(time.Second)
+
+	// Neither g1 nor g2 can connect after g1 being blacklisted
+	err = g1.Connect(g2.Address())
+	if err == nil {
+		t.Fatal("shouldn't be able to connect")
+	}
+	err = g1.ConnectManual(g2.Address())
+	if err == nil {
+		t.Fatal("shouldn't be able to connect")
+	}
+	err = g2.Connect(g1.Address())
+	if err == nil {
+		t.Fatal("shouldn't be able to connect")
+	}
+
+	// Restart g2 without deleting the tmp dir
+	g2.Close()
+	g2, err = New("localhost:0", false, g2.persistDir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer g2.Close()
+
+	// Neither g1 nor g2 can connect. Since g1 is still blacklisted
+	err = g1.Connect(g2.Address())
+	if err == nil {
+		t.Fatal("shouldn't be able to connect")
+	}
+	err = g1.ConnectManual(g2.Address())
+	if err == nil {
+		t.Fatal("shouldn't be able to connect")
+	}
+	err = g2.Connect(g1.Address())
+	if err == nil {
+		t.Fatal("shouldn't be able to connect")
+	}
+}

modules/gateway/peers.go

@@ -128,6 +128,14 @@ func (g *Gateway) threadedAcceptConn(conn net.Conn) {
 	addr := modules.NetAddress(conn.RemoteAddr().String())
 	g.log.Debugf("INFO: %v wants to connect", addr)
 
+	g.mu.RLock()
+	_, exists := g.blacklist[addr.Host()]
+	g.mu.RUnlock()
+	if exists {
+		g.log.Debugf("INFO: %v was rejected. (blacklisted)")
+		conn.Close()
+		return
+	}
 	remoteVersion, err := acceptVersionHandshake(conn, build.Version)
 	if err != nil {
 		g.log.Debugf("INFO: %v wanted to connect but version handshake failed: %v", addr, err)
@@ -394,6 +402,9 @@ func (g *Gateway) managedConnect(addr modules.NetAddress) error {
 	if net.ParseIP(addr.Host()) == nil {
 		return errors.New("address must be an IP address")
 	}
+	if _, exists := g.blacklist[addr.Host()]; exists {
+		return errors.New("can't connect to blacklisted address")
+	}
 	g.mu.RLock()
 	_, exists := g.peers[addr]
 	g.mu.RUnlock()
@@ -505,15 +516,32 @@ func (g *Gateway) Disconnect(addr modules.NetAddress) error {
 	return nil
 }
 
-// Peers returns the addresses currently connected to the Gateway.
-func (g *Gateway) Peers() []modules.Peer {
-	g.mu.RLock()
-	defer g.mu.RUnlock()
-	var peers []modules.Peer
-	for _, p := range g.peers {
-		peers = append(peers, p.Peer)
+// ConnectManual is a wrapper for the Connect function. It is specifically used
+// if a user wants to connect to a node manually. This also removes the node
+// from the blacklist.
+func (g *Gateway) ConnectManual(addr modules.NetAddress) error {
+	g.mu.Lock()
+	var err error
+	if _, exists := g.blacklist[addr.Host()]; exists {
+		delete(g.blacklist, addr.Host())
+		err = g.saveBlacklist()
 	}
-	return peers
+	g.mu.Unlock()
+	return build.ComposeErrors(err, g.Connect(addr))
+}
+
+// DisconnectManual is a wrapper for the Disconnect function. It is
+// specifically used if a user wants to connect to a node manually. This also
+// adds the node to the blacklist.
+func (g *Gateway) DisconnectManual(addr modules.NetAddress) error {
+	err := g.Disconnect(addr)
+	if err == nil {
+		g.mu.Lock()
+		g.blacklist[addr.Host()] = struct{}{}
+		err = g.saveBlacklist()
+		g.mu.Unlock()
+	}
+	return err
 }
 
 // Online returns true if the node is connected to the internet. During testing
@@ -532,3 +560,14 @@ func (g *Gateway) Online() bool {
 	}
 	return false
 }
+
+// Peers returns the addresses currently connected to the Gateway.
+func (g *Gateway) Peers() []modules.Peer {
+	g.mu.RLock()
+	defer g.mu.RUnlock()
+	var peers []modules.Peer
+	for _, p := range g.peers {
+		peers = append(peers, p.Peer)
+	}
+	return peers
+}

modules/gateway/persist.go

@@ -14,6 +14,9 @@ const (
 
 	// nodesFile is the name of the file that contains all seen nodes.
 	nodesFile = "nodes.json"
+
+	// blacklistFile is the name of the file that contains all blacklisted nodes.
+	blacklistFile = "blacklist.json"
 )
 
 // persistMetadata contains the header and version strings that identify the
@@ -23,6 +26,13 @@ var persistMetadata = persist.Metadata{
 	Version: "1.3.0",
 }
 
+// persistMetadataBlacklist contains the header and version strings that
+// identify the gateway persist file.
+var persistMetadataBlacklist = persist.Metadata{
+	Header:  "Sia Node Blacklist",
+	Version: "1.3.2",
+}
+
 // persistData returns the data in the Gateway that will be saved to disk.
 func (g *Gateway) persistData() (nodes []*node) {
 	for _, node := range g.nodes {
@@ -31,6 +41,15 @@ func (g *Gateway) persistData() (nodes []*node) {
 	return
 }
 
+// persistDataBlacklist returns the data of the blacklist that will be saved to
+// disk.
+func (g *Gateway) persistDataBlacklist() (ips []string) {
+	for ip := range g.blacklist {
+		ips = append(ips, ip)
+	}
+	return
+}
+
 // load loads the Gateway's persistent data from disk.
 func (g *Gateway) load() error {
 	var nodes []*node
@@ -45,6 +64,25 @@ func (g *Gateway) load() error {
 	return nil
 }
 
+// load loads the Gateway's blacklisted ips from disk.
+func (g *Gateway) loadBlacklist() error {
+	var ips []string
+	err := persist.LoadJSON(persistMetadataBlacklist, &ips, filepath.Join(g.persistDir, blacklistFile))
+	if err != nil {
+		return err
+	}
+	for _, ip := range ips {
+		g.blacklist[ip] = struct{}{}
+	}
+	return nil
+}
+
+// saveBlacklist stores the Gateway's blacklisted ips on disk and then syncs to
+// disk to minimize the possibility of data loss.
+func (g *Gateway) saveBlacklist() error {
+	return persist.SaveJSON(persistMetadataBlacklist, g.persistDataBlacklist(), filepath.Join(g.persistDir, blacklistFile))
+}
+
 // saveSync stores the Gateway's persistent data on disk, and then syncs to
 // disk to minimize the possibility of data loss.
 func (g *Gateway) saveSync() error {

node/api/gateway.go

@@ -29,7 +29,7 @@ func (api *API) gatewayHandler(w http.ResponseWriter, req *http.Request, _ httpr
 // gatewayConnectHandler handles the API call to add a peer to the gateway.
 func (api *API) gatewayConnectHandler(w http.ResponseWriter, req *http.Request, ps httprouter.Params) {
 	addr := modules.NetAddress(ps.ByName("netaddress"))
-	err := api.gateway.Connect(addr)
+	err := api.gateway.ConnectManual(addr)
 	if err != nil {
 		WriteError(w, Error{err.Error()}, http.StatusBadRequest)
 		return
@@ -41,7 +41,7 @@ func (api *API) gatewayConnectHandler(w http.ResponseWriter, req *http.Request,
 // gatewayDisconnectHandler handles the API call to remove a peer from the gateway.
 func (api *API) gatewayDisconnectHandler(w http.ResponseWriter, req *http.Request, ps httprouter.Params) {
 	addr := modules.NetAddress(ps.ByName("netaddress"))
-	err := api.gateway.Disconnect(addr)
+	err := api.gateway.DisconnectManual(addr)
 	if err != nil {
 		WriteError(w, Error{err.Error()}, http.StatusBadRequest)
 		return