lib/config, gui: Disallow some options in combination with "untrusted" (fixes #8920) (#8921)

This prevents combining untrusted with introducer and auto-accept, and
also verifies that folders shared with untrusted devices have passwords
at config loading time.

Co-authored-by: Simon Frei <[email protected]>

gui/default/assets/css/overrides.css

@@ -549,6 +549,18 @@ ul.three-columns li, ul.two-columns li {
     opacity: 1;
 }
 
+.checkbox[disabled] {
+    background-color: #eeeeee;
+    opacity: 1;
+    margin-left: -5px;
+    padding-left: 5px;
+}
+
+.checkbox[disabled] *, .checkbox[disabled] .help-block {
+    color: #999999;
+    cursor: not-allowed;
+}
+
 /* Make a "well" look more like a readonly text input when grouped with a button */
 .input-group .well-sm {
   padding-top: 6px;

gui/default/syncthing/core/syncthingController.js

@@ -1709,6 +1709,13 @@ angular.module('syncthing.core')
             return $scope.currentDevice._editing == 'new';
         }
 
+        $scope.editDeviceUntrustedChanged = function () {
+            if (currentDevice.untrusted) {
+                currentDevice.introducer = false;
+                currentDevice.autoAcceptFolders = false;
+            }
+        }
+
         $scope.editDeviceExisting = function (deviceCfg) {
             $scope.currentDevice = $.extend({}, deviceCfg);
             $scope.currentDevice._editing = "existing";

gui/default/syncthing/device/editDeviceModalView.html

@@ -62,9 +62,9 @@
           <div class="row">
             <div class="col-md-6">
               <div class="form-group">
-                <div class="checkbox">
+                <div ng-disabled="currentDevice.untrusted" class="checkbox" ng-attr-tooltip="{{currentDevice.untrusted ? null : undefined}}" ng-attr-data-original-title="{{currentDevice.untrusted ? ('Always disabled for untrusted devices' | translate) : undefined}}">
                   <label>
-                    <input type="checkbox" ng-model="currentDevice.introducer">
+                    <input ng-disabled="currentDevice.untrusted" type="checkbox" ng-model="currentDevice.introducer">
                     <span translate>Introducer</span>
                     <p translate class="help-block">Add devices from the introducer to our device list, for mutually shared folders.</p>
                   </label>
@@ -73,9 +73,9 @@
             </div>
             <div class="col-md-6">
               <div class="form-group">
-                <div class="checkbox">
+                <div ng-disabled="currentDevice.untrusted" class="checkbox" ng-attr-tooltip="{{currentDevice.untrusted ? null : undefined}}" ng-attr-data-original-title="{{currentDevice.untrusted ? ('Always disabled for untrusted devices' | translate) : undefined}}">
                   <label>
-                    <input type="checkbox" ng-model="currentDevice.autoAcceptFolders">
+                    <input ng-disabled="currentDevice.untrusted" type="checkbox" ng-model="currentDevice.autoAcceptFolders">
                     <span translate>Auto Accept</span>
                     <p translate class="help-block">Automatically create or share folders that this device advertises at the default path.</p>
                   </label>
@@ -164,7 +164,7 @@
           </div>
           <div class="row">
             <div class="form-group col-md-6">
-              <input type="checkbox" id="untrusted" ng-model="currentDevice.untrusted" />
+              <input type="checkbox" id="untrusted" ng-model="currentDevice.untrusted" ng-change="editDeviceUntrustedChanged()"/>
               <label for="untrusted" translate>Untrusted</label>
               <p translate class="help-block">All folders shared with this device must be protected by a password, such that all sent data is unreadable without the given password.</p>
             </div>

lib/config/config.go

@@ -284,7 +284,7 @@ func (cfg *Configuration) ensureMyDevice(myID protocol.DeviceID) {
 	})
 }
 
-func (cfg *Configuration) prepareFoldersAndDevices(myID protocol.DeviceID) (map[protocol.DeviceID]bool, error) {
+func (cfg *Configuration) prepareFoldersAndDevices(myID protocol.DeviceID) (map[protocol.DeviceID]*DeviceConfiguration, error) {
 	existingDevices := cfg.prepareDeviceList()
 
 	sharedFolders, err := cfg.prepareFolders(myID, existingDevices)
@@ -297,7 +297,7 @@ func (cfg *Configuration) prepareFoldersAndDevices(myID protocol.DeviceID) (map[
 	return existingDevices, nil
 }
 
-func (cfg *Configuration) prepareDeviceList() map[protocol.DeviceID]bool {
+func (cfg *Configuration) prepareDeviceList() map[protocol.DeviceID]*DeviceConfiguration {
 	// Ensure that the device list is
 	// - free from duplicates
 	// - no devices with empty ID
@@ -309,14 +309,14 @@ func (cfg *Configuration) prepareDeviceList() map[protocol.DeviceID]bool {
 	})
 
 	// Build a list of available devices
-	existingDevices := make(map[protocol.DeviceID]bool, len(cfg.Devices))
-	for _, device := range cfg.Devices {
-		existingDevices[device.DeviceID] = true
+	existingDevices := make(map[protocol.DeviceID]*DeviceConfiguration, len(cfg.Devices))
+	for i, device := range cfg.Devices {
+		existingDevices[device.DeviceID] = &cfg.Devices[i]
 	}
 	return existingDevices
 }
 
-func (cfg *Configuration) prepareFolders(myID protocol.DeviceID, existingDevices map[protocol.DeviceID]bool) (map[protocol.DeviceID][]string, error) {
+func (cfg *Configuration) prepareFolders(myID protocol.DeviceID, existingDevices map[protocol.DeviceID]*DeviceConfiguration) (map[protocol.DeviceID][]string, error) {
 	// Prepare folders and check for duplicates. Duplicates are bad and
 	// dangerous, can't currently be resolved in the GUI, and shouldn't
 	// happen when configured by the GUI. We return with an error in that
@@ -359,13 +359,13 @@ func (cfg *Configuration) prepareDevices(sharedFolders map[protocol.DeviceID][]s
 	}
 }
 
-func (cfg *Configuration) prepareIgnoredDevices(existingDevices map[protocol.DeviceID]bool) map[protocol.DeviceID]bool {
+func (cfg *Configuration) prepareIgnoredDevices(existingDevices map[protocol.DeviceID]*DeviceConfiguration) map[protocol.DeviceID]bool {
 	// The list of ignored devices should not contain any devices that have
 	// been manually added to the config.
 	newIgnoredDevices := cfg.IgnoredDevices[:0]
 	ignoredDevices := make(map[protocol.DeviceID]bool, len(cfg.IgnoredDevices))
 	for _, dev := range cfg.IgnoredDevices {
-		if !existingDevices[dev.ID] {
+		if _, ok := existingDevices[dev.ID]; !ok {
 			ignoredDevices[dev.ID] = true
 			newIgnoredDevices = append(newIgnoredDevices, dev)
 		}
@@ -502,7 +502,7 @@ func ensureDevicePresent(devices []FolderDeviceConfiguration, myID protocol.Devi
 	return devices
 }
 
-func ensureExistingDevices(devices []FolderDeviceConfiguration, existingDevices map[protocol.DeviceID]bool) []FolderDeviceConfiguration {
+func ensureExistingDevices(devices []FolderDeviceConfiguration, existingDevices map[protocol.DeviceID]*DeviceConfiguration) []FolderDeviceConfiguration {
 	count := len(devices)
 	i := 0
 loop:
@@ -553,6 +553,23 @@ loop:
 	return devices[0:count]
 }
 
+func ensureNoUntrustedTrustingSharing(f *FolderConfiguration, devices []FolderDeviceConfiguration, existingDevices map[protocol.DeviceID]*DeviceConfiguration) []FolderDeviceConfiguration {
+	for i := 0; i < len(devices); i++ {
+		dev := devices[i]
+		if dev.EncryptionPassword != "" {
+			// There's a password set, no check required
+			continue
+		}
+		if devCfg := existingDevices[dev.DeviceID]; devCfg.Untrusted {
+			l.Warnf("Folder %s (%s) is shared in trusted mode with untrusted device %s (%s); unsharing.", f.ID, f.Label, dev.DeviceID.Short(), devCfg.Name)
+			copy(devices[i:], devices[i+1:])
+			devices = devices[:len(devices)-1]
+			i--
+		}
+	}
+	return devices
+}
+
 func cleanSymlinks(filesystem fs.Filesystem, dir string) {
 	if build.IsWindows {
 		// We don't do symlinks on Windows. Additionally, there may
@@ -611,7 +628,7 @@ func getFreePort(host string, ports ...int) (int, error) {
 	return addr.Port, nil
 }
 
-func (defaults *Defaults) prepare(myID protocol.DeviceID, existingDevices map[protocol.DeviceID]bool) {
+func (defaults *Defaults) prepare(myID protocol.DeviceID, existingDevices map[protocol.DeviceID]*DeviceConfiguration) {
 	ensureZeroForNodefault(&FolderConfiguration{}, &defaults.Folder)
 	ensureZeroForNodefault(&DeviceConfiguration{}, &defaults.Device)
 	defaults.Folder.prepare(myID, existingDevices)

lib/config/config_test.go

@@ -1489,6 +1489,65 @@ func TestXattrFilter(t *testing.T) {
 	}
 }
 
+func TestUntrustedIntroducer(t *testing.T) {
+	fd, err := os.Open("testdata/untrustedintroducer.xml")
+	if err != nil {
+		t.Fatal(err)
+	}
+	cfg, _, err := ReadXML(fd, device1)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(cfg.Devices) != 2 {
+		// ourselves and the remote in the config
+		t.Fatal("Expected two devices")
+	}
+
+	// Check that the introducer and auto-accept flags were negated
+	var foundUntrusted protocol.DeviceID
+	for _, d := range cfg.Devices {
+		if d.Name == "untrusted" {
+			foundUntrusted = d.DeviceID
+			if !d.Untrusted {
+				t.Error("untrusted device should be untrusted")
+			}
+			if d.Introducer {
+				t.Error("untrusted device should not be an introducer")
+			}
+			if d.AutoAcceptFolders {
+				t.Error("untrusted device should not auto-accept folders")
+			}
+		}
+	}
+	if foundUntrusted.Equals(protocol.EmptyDeviceID) {
+		t.Error("untrusted device not found")
+	}
+
+	// Folder A has the device added without a password, which is not permitted
+	folderA := cfg.FolderMap()["a"]
+	for _, dev := range folderA.Devices {
+		if dev.DeviceID == foundUntrusted {
+			t.Error("untrusted device should not be in folder A")
+		}
+	}
+
+	// Folder B has the device added with a password, this is just a sanity check
+	folderB := cfg.FolderMap()["b"]
+	found := false
+	for _, dev := range folderB.Devices {
+		if dev.DeviceID == foundUntrusted {
+			found = true
+			if dev.EncryptionPassword == "" {
+				t.Error("untrusted device should have a password in folder B")
+			}
+		}
+	}
+	if !found {
+		t.Error("untrusted device not found in folder B")
+	}
+}
+
 // Verify that opening a config with myID == protocol.EmptyDeviceID doesn't add that ID to the config.
 // Done in various places where config is needed, but the device ID isn't known.
 func TestLoadEmptyDeviceID(t *testing.T) {

lib/config/deviceconfiguration.go

@@ -34,6 +34,19 @@ func (cfg *DeviceConfiguration) prepare(sharedFolders []string) {
 	}
 
 	cfg.IgnoredFolders = sortedObservedFolderSlice(ignoredFolders)
+
+	// A device cannot be simultaneously untrusted and an introducer, nor
+	// auto accept folders.
+	if cfg.Untrusted {
+		if cfg.Introducer {
+			l.Warnf("Device %s (%s) is both untrusted and an introducer, removing introducer flag", cfg.DeviceID.Short(), cfg.Name)
+			cfg.Introducer = false
+		}
+		if cfg.AutoAcceptFolders {
+			l.Warnf("Device %s (%s) is both untrusted and auto-accepting folders, removing auto-accept flag", cfg.DeviceID.Short(), cfg.Name)
+			cfg.AutoAcceptFolders = false
+		}
+	}
 }
 
 func (cfg *DeviceConfiguration) IgnoredFolder(folder string) bool {

lib/config/folderconfiguration.go

@@ -181,14 +181,16 @@ func (f *FolderConfiguration) DeviceIDs() []protocol.DeviceID {
 	return deviceIDs
 }
 
-func (f *FolderConfiguration) prepare(myID protocol.DeviceID, existingDevices map[protocol.DeviceID]bool) {
+func (f *FolderConfiguration) prepare(myID protocol.DeviceID, existingDevices map[protocol.DeviceID]*DeviceConfiguration) {
 	// Ensure that
 	// - any loose devices are not present in the wrong places
 	// - there are no duplicate devices
 	// - we are part of the devices
+	// - folder is not shared in trusted mode with an untrusted device
 	f.Devices = ensureExistingDevices(f.Devices, existingDevices)
 	f.Devices = ensureNoDuplicateFolderDevices(f.Devices)
 	f.Devices = ensureDevicePresent(f.Devices, myID)
+	f.Devices = ensureNoUntrustedTrustingSharing(f, f.Devices, existingDevices)
 
 	sort.Slice(f.Devices, func(a, b int) bool {
 		return f.Devices[a].DeviceID.Compare(f.Devices[b].DeviceID) == -1

lib/config/testdata/untrustedintroducer.xml

@@ -0,0 +1,17 @@
+<configuration version="37">
+    <folder id="a" label="a" path="a">
+        <device id="6564BQV-R2WYPMN-5OLXMII-CDJUKFD-BHNNCRA-WLQPAIV-ELSGAD2-RMFBFQU" introducedBy="">
+            <encryptionPassword></encryptionPassword>
+        </device>
+    </folder>
+    <folder id="b" label="b" path="b">
+        <device id="6564BQV-R2WYPMN-5OLXMII-CDJUKFD-BHNNCRA-WLQPAIV-ELSGAD2-RMFBFQU" introducedBy="">
+            <encryptionPassword>a complex password</encryptionPassword>
+        </device>
+    </folder>
+    <device id="6564BQV-R2WYPMN-5OLXMII-CDJUKFD-BHNNCRA-WLQPAIV-ELSGAD2-RMFBFQU" name="untrusted" compression="metadata" introducer="true" skipIntroductionRemovals="false" introducedBy="">
+        <address>dynamic</address>
+        <autoAcceptFolders>true</autoAcceptFolders>
+        <untrusted>true</untrusted>
+    </device>
+</configuration>