Encryption, merging scary changes, feature flags

Development
#1

I think the encryption change is mostly ready to land. There are surely things we can improve, probably bugs, and likely things that don’t work when confronted with reality, but we’re not going to figure those things out by staring more at the code from a distance. It needs testing in various scenarios and to accomplish that it really needs to be merged and available somehow, so that we can play with it easier.

At the same time I’m not entirely comfortable with calling it done and letting it loose on everyones data right away.

Ideally I’d like some sort of feature flag gating. That is, the encryption feature would only be available if you’re running the nightly, or if you’ve set a specific STEXPERIMENTAL_FEATURES=encryption variable or something like that. We don’t have a framework for doing that kind of thing, and it would anyway be tricky in this case since the change involves a hundred changes to the GUI.

The GUI is however mostly overridable so one possible way of doing this could be to:

  • Merge the backend changes
  • Move the GUI changes to a separate theme, or a separate downloadable .zip file to drop in ~/.config/syncthing/gui

The code is then out there, I can set up and run my devices with it using a GUI patch or manual config hackery, the very interested can do the same without having to jump through hoops like building a custom version themselves. But the average user will remain protected from enabling it in the belief that it’s well tested and stable – until such a point we think it is stable.

Thoughts? Both on how to handle this specifically, and good ways of handling similar things in the future.

1 Like
#2

Yeah not sure.

It’s a lot of effort to try it out that way. I’d rather we had the ui code gated behind the feature flag (which we at some point remove), I guess it’s not possible to just add (alpha) to the folder type as this affects existing folder types/ui…

#3

Yeah I’m just not sure how to practically do that for a UI change that introduces new elements all over the place in a bunch of files and changes the underlying JS code etc. Unless of course it’s still two copies of the UI files (like a separate theme) but that theme gets automatically switched in based on a flag instead of the theme selector. Of course, having two sets of (some) UI files complicates development in the meantime as well…

#4

I would like to do some stress testing with encryption, including the obligatory torture test of the GUI in IE11. Should I just compile the encryption branch and do the testing with it?

1 Like
#5

I have compiled from the encryption branch and I am now trying to do some testing.

I am not sure where I should report the findings and potential issues though. The GitHub pull request is already super long, so please let me know whether I should add comments there, or rather use the forum (i.e. this thread?) for this.

Right now, I am seeing the following JS errors on start. This is Chromium 85.

logbar.js:11 Error: [$parse:syntax] Syntax Error: Token 'folder' is an unexpected token at column 27 of the expression [folder.type != 'sendonly' folder.type != 'receiveencrypted' && folderStats[folder.id].lastFile && folderStats[folder.id].lastFile.filename] starting at [folder.type != 'receiveencrypted' && folderStats[folder.id].lastFile && folderStats[folder.id].lastFile.filename].
http://errors.angularjs.org/1.3.20/$parse/syntax?p0=folder&p1=is%20an%20unexpected%20token&p2=27&p3=folder.type%20!%3D%20'sendonly'%20folder.type%20!%3D%20'receiveencrypted'%20%26%26%20folderStats%5Bfolder.id%5D.lastFile%20%26%26%20folderStats%5Bfolder.id%5D.lastFile.filename&p4=folder.type%20!%3D%20'receiveencrypted'%20%26%26%20folderStats%5Bfolder.id%5D.lastFile%20%26%26%20folderStats%5Bfolder.id%5D.lastFile.filename
    at http://localhost:8387/vendor/angular/angular.js:82:12
    at Parser.throwError (http://localhost:8387/vendor/angular/angular.js:12159:11)
    at Parser.parse (http://localhost:8387/vendor/angular/angular.js:12112:12)
    at $parse (http://localhost:8387/vendor/angular/angular.js:12826:39)
    at Scope.$watch (http://localhost:8387/vendor/angular/angular.js:14020:19)
    at link (http://localhost:8387/vendor/angular/angular.js:22228:16)
    at invokeLinkFn (http://localhost:8387/vendor/angular/angular.js:8300:9)
    at nodeLinkFn (http://localhost:8387/vendor/angular/angular.js:7810:11)
    at compositeLinkFn (http://localhost:8387/vendor/angular/angular.js:7159:13)
    at compositeLinkFn (http://localhost:8387/vendor/angular/angular.js:7162:13) <!-- ngIf: folder.type != 'sendonly' folder.type != 'receiveencrypted' && folderStats[folder.id].lastFile && folderStats[folder.id].lastFile.filename -->```

I have also encountered some other problems/bugs, but I will restrain from the details until I know where to actually report all of this stuff.

#6

Probably a merging error. I found a few more bugs in the GUI changes and pushed the fixes.

As to the feature flag:
Lets do the “theme” hack, i.e. switching in copies with the untrusted code based on a feature flag. Quite a few of the changes in the UI code are refactoring to simplify changes and deduplicate code. That can be merged to the “main GUI” making the diff much smaller. I can take care of replicating changes to the GUI. I thought about tracking the untrusted GUI code as patches and generating the code copies as a build step, to ensure it stays up-to-date. Not sure that’s worth the hassle though.

3 Likes