From c5e0c479893c4c988ce83ad3cd07361a227a0dc3 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Sat, 1 Apr 2017 09:52:31 +0000 Subject: [PATCH] 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 | 1 + gui/default/index.html | 6 ++ lib/config/config.go | 6 +- lib/config/config_test.go | 96 ++++++++++++++++------------ lib/config/deviceconfiguration.go | 24 +++++-- lib/connections/connections_test.go | 66 +++++++++++++++++++ lib/connections/service.go | 32 ++++++++++ lib/model/model.go | 31 +++++---- 8 files changed, 199 insertions(+), 63 deletions(-) diff --git a/gui/default/assets/lang/lang-en.json b/gui/default/assets/lang/lang-en.json index ed3aeaa0e..26db4838a 100644 --- a/gui/default/assets/lang/lang-en.json +++ b/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.", diff --git a/gui/default/index.html b/gui/default/index.html index 7f0027458..d918a069d 100644 --- a/gui/default/index.html +++ b/gui/default/index.html @@ -593,6 +593,12 @@  Connection Type {{connections[deviceCfg.deviceID].type}} + +  Allowed Networks + + {{deviceCfg.allowedNetworks.join(", ")}} + +  Compression diff --git a/lib/config/config.go b/lib/config/config.go index 2884b0597..677ea721c 100644 --- a/lib/config/config.go +++ b/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] - if len(n.Addresses) == 0 || len(n.Addresses) == 1 && n.Addresses[0] == "" { - n.Addresses = []string{"dynamic"} - } + cfg.Devices[i].prepare() } // Very short reconnection intervals are annoying diff --git a/lib/config/config_test.go b/lib/config/config_test.go index fda9ba078..c3131c791 100644 --- a/lib/config/config_test.go +++ b/lib/config/config_test.go @@ -133,16 +133,18 @@ func TestDeviceConfig(t *testing.T) { expectedDevices := []DeviceConfiguration{ { - DeviceID: device1, - Name: "node one", - Addresses: []string{"tcp://a"}, - Compression: protocol.CompressMetadata, + DeviceID: device1, + Name: "node one", + Addresses: []string{"tcp://a"}, + Compression: protocol.CompressMetadata, + AllowedNetworks: []string{}, }, { - DeviceID: device4, - Name: "node two", - Addresses: []string{"tcp://b"}, - Compression: protocol.CompressMetadata, + DeviceID: device4, + Name: "node two", + Addresses: []string{"tcp://b"}, + 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, - Addresses: []string{"dynamic"}, + DeviceID: device1, + Addresses: []string{"dynamic"}, + AllowedNetworks: []string{}, }, device2: { - DeviceID: device2, - Addresses: []string{"dynamic"}, + DeviceID: device2, + Addresses: []string{"dynamic"}, + AllowedNetworks: []string{}, }, device3: { - DeviceID: device3, - Addresses: []string{"dynamic"}, + DeviceID: device3, + Addresses: []string{"dynamic"}, + AllowedNetworks: []string{}, }, device4: { - DeviceID: device4, - Name: name, // Set when auto created - Addresses: []string{"dynamic"}, - Compression: protocol.CompressMetadata, + DeviceID: device4, + Name: name, // Set when auto created + Addresses: []string{"dynamic"}, + 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, - Addresses: []string{"dynamic"}, - Compression: protocol.CompressMetadata, + DeviceID: device1, + Addresses: []string{"dynamic"}, + Compression: protocol.CompressMetadata, + AllowedNetworks: []string{}, }, device2: { - DeviceID: device2, - Addresses: []string{"dynamic"}, - Compression: protocol.CompressMetadata, + DeviceID: device2, + Addresses: []string{"dynamic"}, + Compression: protocol.CompressMetadata, + AllowedNetworks: []string{}, }, device3: { - DeviceID: device3, - Addresses: []string{"dynamic"}, - Compression: protocol.CompressNever, + DeviceID: device3, + Addresses: []string{"dynamic"}, + Compression: protocol.CompressNever, + AllowedNetworks: []string{}, }, device4: { - DeviceID: device4, - Name: name, // Set when auto created - Addresses: []string{"dynamic"}, - Compression: protocol.CompressMetadata, + DeviceID: device4, + Name: name, // Set when auto created + Addresses: []string{"dynamic"}, + 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, - Addresses: []string{"tcp://192.0.2.1", "tcp://192.0.2.2"}, + DeviceID: device1, + Addresses: []string{"tcp://192.0.2.1", "tcp://192.0.2.2"}, + AllowedNetworks: []string{}, }, device2: { - DeviceID: device2, - Addresses: []string{"tcp://192.0.2.3:6070", "tcp://[2001:db8::42]:4242"}, + DeviceID: device2, + Addresses: []string{"tcp://192.0.2.3:6070", "tcp://[2001:db8::42]:4242"}, + AllowedNetworks: []string{}, }, device3: { - DeviceID: device3, - Addresses: []string{"tcp://[2001:db8::44]:4444", "tcp://192.0.2.4:6090"}, + DeviceID: device3, + Addresses: []string{"tcp://[2001:db8::44]:4444", "tcp://192.0.2.4:6090"}, + AllowedNetworks: []string{}, }, device4: { - DeviceID: device4, - Name: name, // Set when auto created - Addresses: []string{"dynamic"}, - Compression: protocol.CompressMetadata, + DeviceID: device4, + Name: name, // Set when auto created + Addresses: []string{"dynamic"}, + Compression: protocol.CompressMetadata, + AllowedNetworks: []string{}, }, } diff --git a/lib/config/deviceconfiguration.go b/lib/config/deviceconfiguration.go index ac468601d..c2e2a42f4 100644 --- a/lib/config/deviceconfiguration.go +++ b/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 { - c := orig - c.Addresses = make([]string, len(orig.Addresses)) - copy(c.Addresses, orig.Addresses) +func (cfg DeviceConfiguration) Copy() DeviceConfiguration { + c := cfg + c.Addresses = make([]string, len(cfg.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 { diff --git a/lib/connections/connections_test.go b/lib/connections/connections_test.go index 36c4f99e8..32e2a092d 100644 --- a/lib/connections/connections_test.go +++ b/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) + } + } +} diff --git a/lib/connections/service.go b/lib/connections/service.go index eb174733c..ff62396f6 100644 --- a/lib/connections/service.go +++ b/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 +} diff --git a/lib/model/model.go b/lib/model/model.go index 8eb967e8b..e38c04791 100644 --- a/lib/model/model.go +++ b/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 { - // The device exists - if cfg.Paused { - return errDevicePaused - } - return nil + cfg, ok := m.cfg.Device(remoteID) + if !ok { + events.Default.Log(events.DeviceRejected, map[string]string{ + "name": hello.DeviceName, + "device": remoteID.String(), + "address": addr.String(), + }) + return errDeviceUnknown } - events.Default.Log(events.DeviceRejected, map[string]string{ - "name": hello.DeviceName, - "device": remoteID.String(), - "address": addr.String(), - }) + if cfg.Paused { + return errDevicePaused + } - 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.