gui: add advance config port mapping to gui

[rjpruitt16]

Purpose

#4824

Gives the user the opportunity to display a link to web gui remote machines. The user can choose the port and set if the link will display.

Testing

I just check to see if the link was shown depending on the setting.

Screenshots

Documentation

syncthing/docs#573

Authorship

Your name and email will be added automatically to the AUTHORS file
based on the commit metadata.

[calmh]

Thanks for working on this! I have some specific comments below

[calmh]

There's a tradeoff here between having to configure the web_ui boolean and having the UI auto probe reachability. The latter is more convenient and more precise, but can also be a lot of work for the browser when loading a config with many devices...

[tomasz1986]

I think that it may be better to have a more descriptive name for this in the Advanced Configuration. "Web Ui" as such does not really mean anything, as we are in the Web UI right now, are we not 😉? Maybe WebUIAddress or ShowWebAddress?

I would also suggest using capitals for UI, as it is the case with most other acronyms like URL, TLS, LDAP, etc.

[AudriusButkevicius]

To be honest, I'd prefer if this used probing, as otherwise, I can't see this being a feature that will get any use, because I can't see people finding it and enabling it.

[rjpruitt16]

I'm open to port scanning, but I'm just a little lost on where to start. I see libraries with javascript implementation of port sweeping, but I did not know if adding a library is the best decision. Does anybody now of any javascript port sweeping articles? If I had to write it from scratch, I would send a GET request to a range of ports until a success header with X-Syncthing-Id return.

[AudriusButkevicius]

I don't think we want port scan, but we probably want to check if we can reach a web server on the other ip on the right port before showing it in the ui. No point showing something that is unreachable.

[rjpruitt16]

i thought port scanning and probing were similar. My mistake. Does anyone have any suggestion about the probing technique. I am suggesting doing a http get request on a range of ports until the 200 response with X-Syncthing-id

[AudriusButkevicius]

Ok, this is how I would do it, I guess it's up for other maintainers to agree/disagree.

You don't need to do it on a range of ports.
You still have a settings entry with the port.
It defaults to 8384, but people can change it.
In the UI, you check if you can reach :<preconfigured port which is 8384 by default>, if you can - you show the link it in the UI, if you can't you don't need to show anything in the UI.

For LAN scenarios where the GUI is listening on 0.0.0.0, this will "Just work", for cases where people are behind a NAT and what not, this won't work, but we don't expect it to without additional human effort.

There is still the whole "I have a million devices, I don't want a million requests upon accessing the UI", so there should probably be a switch to disable this all together.

[calmh]

Yes. Probe, with a default port. A large off switch to not do it at all.

[rjpruitt16]

What range of ports should it look for the gui in?

[AudriusButkevicius]

You don't need to look for a range. It's a single port preconfigured like you have now, defaulting to 8384

[rjpruitt16]

Im confused. I thought @calmh just said to probe. I thought probe meant to look for the gui over a range of ports.

[calmh]

No. Probe the configured port. If it works, show the link.

[rjpruitt16]

Understood.

[rjpruitt16]

I am getting a cors error when trying to make a head request to connected machines. I am sure this could be fix by me adding remote devices to the CORS policy, but it seems like a big security design decision that I would like to make sure it okay before continuing.

[AudriusButkevicius]

I guess getting a cors error is a form of success, it's not a timeout.

Not sure if you have a way to tell from what the browser returns tho.

[rjpruitt16]

Here is a look at the current state of my probe function. The ip address has authentication setup.

$scope.probeAddress = function (address) {
            $http.head(address, {
                header: {
                    "Access-Control-Allow-Origin": "*"
                }
            }).success(function (data) {
                console.log(data)
            }).error($scope.emitHTTPError);
        }

This is the error I am getting.

I tried it again in insomnia (it similar to postman), and it sends me back that authentication is required.

[rjpruitt16]

it seems I need to set username and password to make request to machines with an authorization set up as well.

[AudriusButkevicius]

So we don't need the request to succeed, we just need to know if there is anything on the other side or not, hence we're fine to get a 401 or a cors error, as long as it's not a timeout.

This might have some relevant info:
https://stackoverflow.com/questions/19325314/how-to-detect-cross-origin-cors-error-vs-other-types-of-errors-for-xmlhttpreq

[rjpruitt16]

I am not finished.

Sometimes the link shows up correctly. Other times I think it is showing the address of a machine port forwarding to the device. it seems the $scope.connections address is not always the real address of the device.

[AudriusButkevicius]

If it's connected via relay etc, it doesn't make sense to do the port check. so I'd check what connection type you have.

[rjpruitt16]

I did not see any documentation on the connectioninfo.type. it seems to me if the connectionInfo.type is "tcp-server", than this means the connection type is the actually the device. If the type is "tcp-client", it represent a relay. Is this feature only going to work when port forwarding is not on. Is there a way to track down the final destination?

[AudriusButkevicius]

There should be no difference for you between tcp server and tcp client. To be honest, the only time when you should not bother is when the connection type starts with relay

[acolomb]

What we should try to avoid is an error about a malformed URL from the Angular $http library, though.

In my opinion an already known potential error needs to be dealt with before merging an optional enhancement like this. Either catch the error somehow to ignore it or filter problematic addresses before to not even try them.

Not handling link-local IPv6 addresses correctly is definitely a bug in AngularJS and also hard to catch in our code. The error message stack trace is contains only entries from the file angular.js:

Error: The URI is malformed.
createHttpBackend/<@http://127.0.0.1:8385/vendor/angular/angular.js:9851:11
sendReq@http://127.0.0.1:8385/vendor/angular/angular.js:9713:21
serverRequest@http://127.0.0.1:8385/vendor/angular/angular.js:9425:16
processQueue@http://127.0.0.1:8385/vendor/angular/angular.js:13337:27
scheduleProcessQueue/<@http://127.0.0.1:8385/vendor/angular/angular.js:13353:39
$eval@http://127.0.0.1:8385/vendor/angular/angular.js:14589:28
$digest@http://127.0.0.1:8385/vendor/angular/angular.js:14405:31
$apply@http://127.0.0.1:8385/vendor/angular/angular.js:14694:24
scheduleApplyAsync/applyAsyncId<@http://127.0.0.1:8385/vendor/angular/angular.js:14984:22
completeOutstandingRequest@http://127.0.0.1:8385/vendor/angular/angular.js:4959:10
Browser/self.defer/timeoutId<@http://127.0.0.1:8385/vendor/angular/angular.js:5347:33
setTimeout handler*Browser/self.defer@http://127.0.0.1:8385/vendor/angular/angular.js:5345:17
scheduleApplyAsync@http://127.0.0.1:8385/vendor/angular/angular.js:14983:33
$applyAsync@http://127.0.0.1:8385/vendor/angular/angular.js:14722:9
done@http://127.0.0.1:8385/vendor/angular/angular.js:9741:22
completeRequest@http://127.0.0.1:8385/vendor/angular/angular.js:9934:15
requestLoaded@http://127.0.0.1:8385/vendor/angular/angular.js:9875:24
EventHandlerNonNull*createHttpBackend/<@http://127.0.0.1:8385/vendor/angular/angular.js:9858:7
sendReq@http://127.0.0.1:8385/vendor/angular/angular.js:9713:21
serverRequest@http://127.0.0.1:8385/vendor/angular/angular.js:9425:16
processQueue@http://127.0.0.1:8385/vendor/angular/angular.js:13337:27
scheduleProcessQueue/<@http://127.0.0.1:8385/vendor/angular/angular.js:13353:39
$eval@http://127.0.0.1:8385/vendor/angular/angular.js:14589:28
$digest@http://127.0.0.1:8385/vendor/angular/angular.js:14405:31
$apply@http://127.0.0.1:8385/vendor/angular/angular.js:14694:24
scheduleApplyAsync/applyAsyncId<@http://127.0.0.1:8385/vendor/angular/angular.js:14984:22
completeOutstandingRequest@http://127.0.0.1:8385/vendor/angular/angular.js:4959:10
Browser/self.defer/timeoutId<@http://127.0.0.1:8385/vendor/angular/angular.js:5347:33
setTimeout handler*Browser/self.defer@http://127.0.0.1:8385/vendor/angular/angular.js:5345:17
scheduleApplyAsync@http://127.0.0.1:8385/vendor/angular/angular.js:14983:33
$applyAsync@http://127.0.0.1:8385/vendor/angular/angular.js:14722:9
done@http://127.0.0.1:8385/vendor/angular/angular.js:9741:22
completeRequest@http://127.0.0.1:8385/vendor/angular/angular.js:9934:15
requestLoaded@http://127.0.0.1:8385/vendor/angular/angular.js:9875:24
EventHandlerNonNull*createHttpBackend/<@http://127.0.0.1:8385/vendor/angular/angular.js:9858:7
sendReq@http://127.0.0.1:8385/vendor/angular/angular.js:9713:21
serverRequest@http://127.0.0.1:8385/vendor/angular/angular.js:9425:16
processQueue@http://127.0.0.1:8385/vendor/angular/angular.js:13337:27
scheduleProcessQueue/<@http://127.0.0.1:8385/vendor/angular/angular.js:13353:39
$eval@http://127.0.0.1:8385/vendor/angular/angular.js:14589:28
$digest@http://127.0.0.1:8385/vendor/angular/angular.js:14405:31
$apply@http://127.0.0.1:8385/vendor/angular/angular.js:14694:24
scheduleApplyAsync/applyAsyncId<@http://127.0.0.1:8385/vendor/angular/angular.js:14984:22
completeOutstandingRequest@http://127.0.0.1:8385/vendor/angular/angular.js:4959:10
Browser/self.defer/timeoutId<@http://127.0.0.1:8385/vendor/angular/angular.js:5347:33
setTimeout handler*Browser/self.defer@http://127.0.0.1:8385/vendor/angular/angular.js:5345:17
scheduleApplyAsync@http://127.0.0.1:8385/vendor/angular/angular.js:14983:33
$applyAsync@http://127.0.0.1:8385/vendor/angular/angular.js:14722:9
done@http://127.0.0.1:8385/vendor/angular/angular.js:9741:22
completeRequest@http://127.0.0.1:8385/vendor/angular/angular.js:9934:15
requestLoaded@http://127.0.0.1:8385/vendor/angular/angular.js:9875:24
EventHandlerNonNull*createHttpBackend/<@http://127.0.0.1:8385/vendor/angular/angular.js:9858:7
sendReq@http://127.0.0.1:8385/vendor/angular/angular.js:9713:21
serverRequest@http://127.0.0.1:8385/vendor/angular/angular.js:9425:16
processQueue@http://127.0.0.1:8385/vendor/angular/angular.js:13337:27
scheduleProcessQueue/<@http://127.0.0.1:8385/vendor/angular/angular.js:13353:39
$eval@http://127.0.0.1:8385/vendor/angular/angular.js:14589:28
$digest@http://127.0.0.1:8385/vendor/angular/angular.js:14405:31

I could only avoid that by removing the zone identifier before the probing request. That yields an invalid address, which makes the browser's CORS check fail. That in turn avoids displaying a dysfunctional link.

Ideally, if the Angular "malformed URL" bug was fixed, we should stop doing that, so at least compliant browsers without that limitation can actually work with the (properly encoded) address.

[imsodin]

I could only avoid that by removing the zone identifier before the probing request. That yields an invalid address, which makes the browser's CORS check fail. That in turn avoids displaying a dysfunctional link.

Does the check fail all the time? Then we could just skip if there is a zone identifier without probing.

[acolomb]

Does the check fail all the time? Then we could just skip if there is a zone identifier without probing.

Depends on the browser, so really hard to say.

[imsodin]

lgtm (except for a tiny merge conflict). With the untrusted PR merged, the changes here need to be copied over to the file in gui/default/untrusted. Someone can either do it in the PR or otherwise I'll take care of that in a followup commit once merged.

[acolomb]

I will merge in main tonight. Hope that's still in time for the next rc?

[acolomb]

Merge conflicts fixed and incorporated the HTML changes to the untrusted GUI.

[acolomb]

Sorry, overlooked that syncthingController.js was also duplicated. Ready for merging from my point of view. @imsodin

[acolomb]

No good doing things in a hurry. That was some kind of very strange merge conflict from the untrusted branch I suppose. Now the HTML code inserted by this PR is in the right spot again, so the link actually shows up correctly.

Sorry again for this confusion.

[calmh]

This doesn't work when the GUI is loaded over HTTPS, as the request is blocked by the browser.

[rjpruitt16]

Should we just handle the error?

We can set upgrade insecure url in the options request header. Is this acceptable?

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Upgrade-Insecure-Requests

[rjpruitt16]

I will see if it solves it tomorrow.

[acolomb]

This seems to be a Really Hard Problem.

Content-Security-Policy basically always disallows mixed content (HTTP request from a script loaded through HTTPS). There are some additional headers to control that, but apparently can only further restrict access.

On the other hand, doing probe requests to HTTPS directly will fail almost always. Either the remote GUI uses the auto-generated self-signed TLS certificate, or it has a proper TLS cert, which does not match the IP address we're using to construct the remote GUI address (only the cert domain name). Either case leads to the browser blocking the request because of an insecure connection.

One solution on the browser side would be to bypass CSP for OPTIONS requests completely, since these provide no displayed content and therefore pose no security threat. Still a possible privacy problem though.

What we could do on the Syncthing side is looking at the argument of the request error callback. If the request was blocked by the browser already, that err argument is null. Generating an http:// link in that case still works because clicking on that is not blocked by CSP (as tested under Firefox). Such a check produces some false-positives, though, as there are many reasons why the browser could have blocked. The OPTIONS method not being allowed in the preflight request (on versions before #7079 was implemented) exhibits the same err === null condition for example.

[calmh]

Perhaps we should remove the probing in its entirety and just generate an url with the hope of it being useful. I very much wanted the probing approach to work but it seems to be low value...

[acolomb]

Indeed, ironically browsers have become a rather difficult environment for network programming. I'm also quite disappointed by all these restrictions, but they do make sense in the name of security within a WWW browsing world.

Especially the problem with link-local IPv6 addresses being unsupported is very unfortunate. They are very common as a Syncthing connection on LANs, but often unpredictable. If using Bonjour / mDNS or similar, browsers will happily make such connections, but not with IPv6 literals :-( That's still a problem even without trying to probe, but maybe more bearable.