lib/connections, lib/model, gui: Specify allowed networks per device (fixes #219)

This adds a new config AllowedNetworks per device, which when set should
contain a list of network prefixes (192.168.0.0/126 etc) that are
allowed for the given device. The connection service will not attempt
connections to addresses outside of the given networks and incoming
connections will be rejected as well.

I've added the config to the normal device editor and shown it (when
set) in the device summary on the main screen.

There's a unit test for the IsAllowedNetwork method, I've done some
manual sanity testing on top of that.

GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4073

gui/default/assets/lang/lang-en.json

@@ -18,6 +18,7 @@
    "Advanced settings": "Advanced settings",
    "All Data": "All Data",
    "Allow Anonymous Usage Reporting?": "Allow Anonymous Usage Reporting?",
+   "Allowed Networks": "Allowed Networks",
    "Alphabetic": "Alphabetic",
    "An external command handles the versioning. It has to remove the file from the shared folder.": "An external command handles the versioning. It has to remove the file from the shared folder.",
    "An external command handles the versioning. It has to remove the file from the synced folder.": "An external command handles the versioning. It has to remove the file from the synced folder.",

gui/default/index.html

@@ -593,6 +593,12 @@
                       <th><span class="fa fa-fw fa-warning text-danger"></span>&nbsp;<span translate>Connection Type</span></th>
                       <td class="text-right">{{connections[deviceCfg.deviceID].type}}</td>
                     </tr>
+                    <tr ng-if="deviceCfg.allowedNetworks">
+                      <th><span class="fa fa-fw fa-filter"></span>&nbsp;<span translate>Allowed Networks</span></th>
+                      <td class="text-right">
+                        <span>{{deviceCfg.allowedNetworks.join(", ")}}</span>
+                      </td>
+                    </tr>
                     <tr ng-if="deviceCfg.compression != 'metadata'">
                       <th><span class="fa fa-fw fa-compress"></span>&nbsp;<span translate>Compression</span></th>
                       <td class="text-right">

lib/config/config.go

@@ -315,12 +315,8 @@ func (cfg *Configuration) clean() error {
 		sort.Sort(FolderDeviceConfigurationList(cfg.Folders[i].Devices))
 	}
 
-	// An empty address list is equivalent to a single "dynamic" entry
 	for i := range cfg.Devices {
-		n := &cfg.Devices[i]
+		cfg.Devices[i].prepare()
-		if len(n.Addresses) == 0 || len(n.Addresses) == 1 && n.Addresses[0] == "" {
-			n.Addresses = []string{"dynamic"}
-		}
 	}
 
 	// Very short reconnection intervals are annoying

lib/config/config_test.go

@@ -133,16 +133,18 @@ func TestDeviceConfig(t *testing.T) {
 
 		expectedDevices := []DeviceConfiguration{
 			{
-				DeviceID:    device1,
+				DeviceID:        device1,
-				Name:        "node one",
+				Name:            "node one",
-				Addresses:   []string{"tcp://a"},
+				Addresses:       []string{"tcp://a"},
-				Compression: protocol.CompressMetadata,
+				Compression:     protocol.CompressMetadata,
+				AllowedNetworks: []string{},
 			},
 			{
-				DeviceID:    device4,
+				DeviceID:        device4,
-				Name:        "node two",
+				Name:            "node two",
-				Addresses:   []string{"tcp://b"},
+				Addresses:       []string{"tcp://b"},
-				Compression: protocol.CompressMetadata,
+				Compression:     protocol.CompressMetadata,
+				AllowedNetworks: []string{},
 			},
 		}
 		expectedDeviceIDs := []protocol.DeviceID{device1, device4}
@@ -236,22 +238,26 @@ func TestDeviceAddressesDynamic(t *testing.T) {
 	name, _ := os.Hostname()
 	expected := map[protocol.DeviceID]DeviceConfiguration{
 		device1: {
-			DeviceID:  device1,
+			DeviceID:        device1,
-			Addresses: []string{"dynamic"},
+			Addresses:       []string{"dynamic"},
+			AllowedNetworks: []string{},
 		},
 		device2: {
-			DeviceID:  device2,
+			DeviceID:        device2,
-			Addresses: []string{"dynamic"},
+			Addresses:       []string{"dynamic"},
+			AllowedNetworks: []string{},
 		},
 		device3: {
-			DeviceID:  device3,
+			DeviceID:        device3,
-			Addresses: []string{"dynamic"},
+			Addresses:       []string{"dynamic"},
+			AllowedNetworks: []string{},
 		},
 		device4: {
-			DeviceID:    device4,
+			DeviceID:        device4,
-			Name:        name, // Set when auto created
+			Name:            name, // Set when auto created
-			Addresses:   []string{"dynamic"},
+			Addresses:       []string{"dynamic"},
-			Compression: protocol.CompressMetadata,
+			Compression:     protocol.CompressMetadata,
+			AllowedNetworks: []string{},
 		},
 	}
 
@@ -270,25 +276,29 @@ func TestDeviceCompression(t *testing.T) {
 	name, _ := os.Hostname()
 	expected := map[protocol.DeviceID]DeviceConfiguration{
 		device1: {
-			DeviceID:    device1,
+			DeviceID:        device1,
-			Addresses:   []string{"dynamic"},
+			Addresses:       []string{"dynamic"},
-			Compression: protocol.CompressMetadata,
+			Compression:     protocol.CompressMetadata,
+			AllowedNetworks: []string{},
 		},
 		device2: {
-			DeviceID:    device2,
+			DeviceID:        device2,
-			Addresses:   []string{"dynamic"},
+			Addresses:       []string{"dynamic"},
-			Compression: protocol.CompressMetadata,
+			Compression:     protocol.CompressMetadata,
+			AllowedNetworks: []string{},
 		},
 		device3: {
-			DeviceID:    device3,
+			DeviceID:        device3,
-			Addresses:   []string{"dynamic"},
+			Addresses:       []string{"dynamic"},
-			Compression: protocol.CompressNever,
+			Compression:     protocol.CompressNever,
+			AllowedNetworks: []string{},
 		},
 		device4: {
-			DeviceID:    device4,
+			DeviceID:        device4,
-			Name:        name, // Set when auto created
+			Name:            name, // Set when auto created
-			Addresses:   []string{"dynamic"},
+			Addresses:       []string{"dynamic"},
-			Compression: protocol.CompressMetadata,
+			Compression:     protocol.CompressMetadata,
+			AllowedNetworks: []string{},
 		},
 	}
 
@@ -307,22 +317,26 @@ func TestDeviceAddressesStatic(t *testing.T) {
 	name, _ := os.Hostname()
 	expected := map[protocol.DeviceID]DeviceConfiguration{
 		device1: {
-			DeviceID:  device1,
+			DeviceID:        device1,
-			Addresses: []string{"tcp://192.0.2.1", "tcp://192.0.2.2"},
+			Addresses:       []string{"tcp://192.0.2.1", "tcp://192.0.2.2"},
+			AllowedNetworks: []string{},
 		},
 		device2: {
-			DeviceID:  device2,
+			DeviceID:        device2,
-			Addresses: []string{"tcp://192.0.2.3:6070", "tcp://[2001:db8::42]:4242"},
+			Addresses:       []string{"tcp://192.0.2.3:6070", "tcp://[2001:db8::42]:4242"},
+			AllowedNetworks: []string{},
 		},
 		device3: {
-			DeviceID:  device3,
+			DeviceID:        device3,
-			Addresses: []string{"tcp://[2001:db8::44]:4444", "tcp://192.0.2.4:6090"},
+			Addresses:       []string{"tcp://[2001:db8::44]:4444", "tcp://192.0.2.4:6090"},
+			AllowedNetworks: []string{},
 		},
 		device4: {
-			DeviceID:    device4,
+			DeviceID:        device4,
-			Name:        name, // Set when auto created
+			Name:            name, // Set when auto created
-			Addresses:   []string{"dynamic"},
+			Addresses:       []string{"dynamic"},
-			Compression: protocol.CompressMetadata,
+			Compression:     protocol.CompressMetadata,
+			AllowedNetworks: []string{},
 		},
 	}
 

lib/config/deviceconfiguration.go

@@ -18,22 +18,36 @@ type DeviceConfiguration struct {
 	SkipIntroductionRemovals bool                 `xml:"skipIntroductionRemovals,attr" json:"skipIntroductionRemovals"`
 	IntroducedBy             protocol.DeviceID    `xml:"introducedBy,attr" json:"introducedBy"`
 	Paused                   bool                 `xml:"paused" json:"paused"`
+	AllowedNetworks          []string             `xml:"allowedNetwork,omitempty" json:"allowedNetworks"`
 }
 
 func NewDeviceConfiguration(id protocol.DeviceID, name string) DeviceConfiguration {
-	return DeviceConfiguration{
+	d := DeviceConfiguration{
 		DeviceID: id,
 		Name:     name,
 	}
+	d.prepare()
+	return d
 }
 
-func (orig DeviceConfiguration) Copy() DeviceConfiguration {
+func (cfg DeviceConfiguration) Copy() DeviceConfiguration {
-	c := orig
+	c := cfg
-	c.Addresses = make([]string, len(orig.Addresses))
+	c.Addresses = make([]string, len(cfg.Addresses))
-	copy(c.Addresses, orig.Addresses)
+	copy(c.Addresses, cfg.Addresses)
+	c.AllowedNetworks = make([]string, len(cfg.AllowedNetworks))
+	copy(c.AllowedNetworks, cfg.AllowedNetworks)
 	return c
 }
 
+func (cfg *DeviceConfiguration) prepare() {
+	if len(cfg.Addresses) == 0 || len(cfg.Addresses) == 1 && cfg.Addresses[0] == "" {
+		cfg.Addresses = []string{"dynamic"}
+	}
+	if len(cfg.AllowedNetworks) == 0 {
+		cfg.AllowedNetworks = []string{}
+	}
+}
+
 type DeviceConfigurationList []DeviceConfiguration
 
 func (l DeviceConfigurationList) Less(a, b int) bool {

lib/connections/connections_test.go

@@ -24,3 +24,69 @@ func TestFixupPort(t *testing.T) {
 		}
 	}
 }
+
+func TestAllowedNetworks(t *testing.T) {
+	cases := []struct {
+		host    string
+		allowed []string
+		ok      bool
+	}{
+		{
+			"192.168.0.1",
+			nil,
+			false,
+		},
+		{
+			"192.168.0.1",
+			[]string{},
+			false,
+		},
+		{
+			"fe80::1",
+			nil,
+			false,
+		},
+		{
+			"fe80::1",
+			[]string{},
+			false,
+		},
+		{
+			"192.168.0.1",
+			[]string{"fe80::/48", "192.168.0.0/24"},
+			true,
+		},
+		{
+			"fe80::1",
+			[]string{"192.168.0.0/24", "fe80::/48"},
+			true,
+		},
+		{
+			"192.168.0.1",
+			[]string{"192.168.1.0/24", "fe80::/48"},
+			false,
+		},
+		{
+			"fe80::1",
+			[]string{"fe82::/48", "192.168.1.0/24"},
+			false,
+		},
+		{
+			"192.168.0.1:4242",
+			[]string{"fe80::/48", "192.168.0.0/24"},
+			true,
+		},
+		{
+			"[fe80::1]:4242",
+			[]string{"192.168.0.0/24", "fe80::/48"},
+			true,
+		},
+	}
+
+	for _, tc := range cases {
+		res := IsAllowedNetwork(tc.host, tc.allowed)
+		if res != tc.ok {
+			t.Errorf("allowedNetwork(%q, %q) == %v, want %v", tc.host, tc.allowed, res, tc.ok)
+		}
+	}
+}

lib/connections/service.go

@@ -371,6 +371,13 @@ func (s *Service) connect() {
 					continue
 				}
 
+				if len(deviceCfg.AllowedNetworks) > 0 {
+					if !IsAllowedNetwork(uri.Host, deviceCfg.AllowedNetworks) {
+						l.Debugln("Network for", uri, "is disallowed")
+						continue
+					}
+				}
+
 				dialerFactory, err := s.getDialerFactory(cfg, uri)
 				if err == errDisabled {
 					l.Debugln("Dialer for", uri, "is disabled")
@@ -641,3 +648,28 @@ func tlsTimedHandshake(tc *tls.Conn) error {
 	defer tc.SetDeadline(time.Time{})
 	return tc.Handshake()
 }
+
+// IsAllowedNetwork returns true if the given host (IP or resolvable
+// hostname) is in the set of allowed networks (CIDR format only).
+func IsAllowedNetwork(host string, allowed []string) bool {
+	if hostNoPort, _, err := net.SplitHostPort(host); err == nil {
+		host = hostNoPort
+	}
+
+	addr, err := net.ResolveIPAddr("ip", host)
+	if err != nil {
+		return false
+	}
+
+	for _, n := range allowed {
+		_, cidr, err := net.ParseCIDR(n)
+		if err != nil {
+			continue
+		}
+		if cidr.Contains(addr.IP) {
+			return true
+		}
+	}
+
+	return false
+}

lib/model/model.go

@@ -119,6 +119,7 @@ var (
 	errNotRelative         = errors.New("not a relative path")
 	errFolderPaused        = errors.New("folder is paused")
 	errFolderMissing       = errors.New("no such folder")
+	errNetworkNotAllowed   = errors.New("network not allowed")
 )
 
 // NewModel creates and starts a new model. The model starts in read-only mode,
@@ -1321,21 +1322,27 @@ func (m *Model) OnHello(remoteID protocol.DeviceID, addr net.Addr, hello protoco
 		return errDeviceIgnored
 	}
 
-	if cfg, ok := m.cfg.Device(remoteID); ok {
+	cfg, ok := m.cfg.Device(remoteID)
-		// The device exists
+	if !ok {
-		if cfg.Paused {
+		events.Default.Log(events.DeviceRejected, map[string]string{
-			return errDevicePaused
+			"name":    hello.DeviceName,
-		}
+			"device":  remoteID.String(),
-		return nil
+			"address": addr.String(),
+		})
+		return errDeviceUnknown
 	}
 
-	events.Default.Log(events.DeviceRejected, map[string]string{
+	if cfg.Paused {
-		"name":    hello.DeviceName,
+		return errDevicePaused
-		"device":  remoteID.String(),
+	}
-		"address": addr.String(),
-	})
 
-	return errDeviceUnknown
+	if len(cfg.AllowedNetworks) > 0 {
+		if !connections.IsAllowedNetwork(addr.String(), cfg.AllowedNetworks) {
+			return errNetworkNotAllowed
+		}
+	}
+
+	return nil
 }
 
 // GetHello is called when we are about to connect to some remote device.