diff options
author | Jakob Borg <jakob@kastelo.net> | 2023-06-14 09:24:31 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-06-14 09:24:31 +0200 |
commit | 6b475bdb78eb671866419ff84a4cda6285c90244 (patch) | |
tree | 8e7d4f609375e22373b570357b47d9ab1dfb719a | |
parent | 7d56fba3217b94415ceced1b5041de34290f32d5 (diff) |
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 <freisim93@gmail.com>
-rw-r--r-- | gui/default/assets/css/overrides.css | 12 | ||||
-rwxr-xr-x | gui/default/syncthing/core/syncthingController.js | 7 | ||||
-rw-r--r-- | gui/default/syncthing/device/editDeviceModalView.html | 10 | ||||
-rw-r--r-- | lib/config/config.go | 37 | ||||
-rw-r--r-- | lib/config/config_test.go | 59 | ||||
-rw-r--r-- | lib/config/deviceconfiguration.go | 13 | ||||
-rw-r--r-- | lib/config/folderconfiguration.go | 4 | ||||
-rw-r--r-- | lib/config/testdata/untrustedintroducer.xml | 17 |
8 files changed, 143 insertions, 16 deletions
diff --git a/gui/default/assets/css/overrides.css b/gui/default/assets/css/overrides.css index 8b0e9fb459..865aa41684 100644 --- a/gui/default/assets/css/overrides.css +++ b/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; diff --git a/gui/default/syncthing/core/syncthingController.js b/gui/default/syncthing/core/syncthingController.js index 20c3ec3841..a681eb8bc9 100755 --- a/gui/default/syncthing/core/syncthingController.js +++ b/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"; diff --git a/gui/default/syncthing/device/editDeviceModalView.html b/gui/default/syncthing/device/editDeviceModalView.html index e74e1692c2..972d37684c 100644 --- a/gui/default/syncthing/device/editDeviceModalView.html +++ b/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> diff --git a/lib/config/config.go b/lib/config/config.go index 21020c4073..0534d5f91e 100644 --- a/lib/config/config.go +++ b/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) diff --git a/lib/config/config_test.go b/lib/config/config_test.go index b70aab91c4..e0d6ef22d6 100644 --- a/lib/config/config_test.go +++ b/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) { diff --git a/lib/config/deviceconfiguration.go b/lib/config/deviceconfiguration.go index d2751002ea..54a30affc4 100644 --- a/lib/config/deviceconfiguration.go +++ b/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 { diff --git a/lib/config/folderconfiguration.go b/lib/config/folderconfiguration.go index a65df1177a..d6e076727d 100644 --- a/lib/config/folderconfiguration.go +++ b/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 diff --git a/lib/config/testdata/untrustedintroducer.xml b/lib/config/testdata/untrustedintroducer.xml new file mode 100644 index 0000000000..122f9a4591 --- /dev/null +++ b/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> |