Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Built-in webclient #79

Closed
vladimiry opened this issue Nov 29, 2018 · 29 comments
Closed

Built-in webclient #79

vladimiry opened this issue Nov 29, 2018 · 29 comments

Comments

@vladimiry
Copy link
Owner

vladimiry commented Nov 29, 2018

I was thinking before about enabling this feature for different reasons but was focusing on other things. @KAepora has recently made a good point in the An Analysis of the ProtonMail Cryptographic Architecture paper about a need to run webclient locally in order to enhance code integrity. Given that point and that the local store feature is substantially completed I think the time has come to address this issue.

I've already got a proof of concept running on my laptop and it appears to be working well at first glance. So the app is already capable to work with local webclient. I will need to integrate the webclient building into the CI build pipeline. It will clone the code by specifc hash from https://github.com/ProtonMail/WebClient, build it and embed to the app. For now local webclient will be able to talk only to a single API endpoint https://mail.protonmail.com/api, the default one.

There are no plans for enabling built-in webclient automatic updates separately form the app itself. So there will be a need to build a new desktop version in order to update a local webclient. It would be quite easy though to allow advanced users to re-build the webclient in their own and make the app use it since I don't hack/preprocess the original webclient in any way but use it as a completely given static thing.

This feature is going to be enabled initially for Protonmail only. Tutanota has a more complex web client architecture that involves Service Workers and WebSockets logic which makes it more complex for integrating with Electron. Besides, it has been evolving quite rapidly which means there would be a need to release desktop versions continuously to catch up with the web client.

I'd say there is a good chance to include this feature into the final v2 release.

img

CC @thiswillbeyourgithub

@vladimiry
Copy link
Owner Author

vladimiry commented Dec 1, 2018

The feature is released with just published v2.0.0-beta.8 version. You will notice that the installation package size has been significantly increased with this release. The uncompressed WebClient dist size is 26.7 MB.

See some implementations details:

  • The app takes the web client code from the official repository based on this constant.
  • This script does all the work of cloning and building the web client.
  • The logic of this file tricks the CORS thing, so local web client is able to communicate with remote API via XHR requests.
  • The original WebClient code remains completely unchanged, the app uses it as a completely given static thing.
  • It's impossible to build the WebClient project on Windows despite it being a Node.Js project, sort of shame. So I had to split the Appveyor CI build process to the 2 jobs. The first job builds the WebClient project executing this script and the second job picks the prepared web client dist and does the rest of the building. Things are simpler with Travis CI which prepares a Linux and macOS packages.
  • You can review the web client cloning and building logs here, here and there (links are entry points, so scroll down the log).

There will be separate issue opened in case of implementing this feature for Tutanota.

@joshirio
Copy link
Contributor

joshirio commented Dec 1, 2018

Amazing feature, great work!
You just made using protonmail a lot more secure, thanks 👏

@vladimiry
Copy link
Owner Author

Aside from the security matters, the app is considered to be more stable in case of built-in web client used since it doesn't depend on the web client being independently updated and deployed by protonmail dev team. So if someone faces issues like for example not working auto login of local store features using online web client, I'd recommend trying switching to the built-in client. I might want to deprecate and maybe even drop the online web client use after a while.

@nil0x42
Copy link

nil0x42 commented Dec 2, 2018

Amazing feature, @vladimiry do you also plan to enable tor endpoint for Webclient ?

@vladimiry
Copy link
Owner Author

@nil0x42 I initially designed the feature in the way so it's easy to embed addition local web client bundles, like enabling feature for all the entry domains. But since my goal is to embed the untouched original official web client static bundle, the size of the app package will be increased by the exact same static bundle size that's already used with only a few code lines changed - domain configuration. Is it what you would expect to happen? The compressed webclient's bundle size is about 6MB, not so big as for me.

I'm reopening the issue, so we could collect different opinions.

@vladimiry vladimiry reopened this Dec 2, 2018
@jeroenev
Copy link

jeroenev commented Dec 2, 2018

what about tutanota? does that also have a builtin web version?

@vladimiry
Copy link
Owner Author

vladimiry commented Dec 3, 2018

@jeroen7s see messages above in this thread

This feature is going to be enabled initially for Protonmail only. Tutanota has a more complex web client architecture that involves Service Workers and WebSockets logic which makes it more complex for integrating with Electron. Besides, it has been evolving quite rapidly which means there would be a need to release desktop versions continuously to catch up with the web client.

There will be separate issue opened in case of implementing this feature for Tutanota.

But let's keep the Tutanota related discussion here since issue got reopened.

As I wrote above, the tech stack of Tutanota's web client is more complicated than Protonmail's. I will need to make sure first that for example, Tutanota doesn't update the client despite it being started locally. If I'm not wrong, they have the local client auto update thing, with notification - all such stuff will need to be optionally disabled by the app, depending on running local/online client. Besides their web client doesn't look for me fully stable yet, they keep updating it quite actively but I'd like it gets settled. Nevertheless, I'm going to look into the case.

@nil0x42
Copy link

nil0x42 commented Dec 3, 2018

@nil0x42 I initially designed the feature in the way so it's easy to embed addition local web client bundles, like enabling feature for all the entry domains. But since my goal is to embed the untouched original official web client static bundle, the size of the app package will be increased by the exact same static bundle size that's already used with only a few code lines changed - domain configuration. Is it what you would expect to happen? The compressed webclient's bundle size is about 6MB, not so big as for me.

I'm reopening the issue, so we could collect different opinions.

Okay, the use of webclient for tor endpoint is probably a must have for .onion domain users, as onion network behaves slowly enough to need to prevent cache files download.

@vladimiry
Copy link
Owner Author

vladimiry commented Dec 3, 2018

Amazing feature, @vladimiry do you also plan to enable tor endpoint for Webclient ?

@nil0x42 can you try the beta 9 build and confirm that built-in web client mapped to the Tor domain works as expected?

domains

I guess I need to sort the items by domain for the next release 😄

@nil0x42
Copy link

nil0x42 commented Dec 4, 2018

@vladimiry , i just tested the .onion webclient in many ways and could not connect even once through it.

Sometimes, the autofill does not set the user/pass, sometimes it sets it but doesn't click on Login.
And when everything is correctly autofilled & form is submitted, the client receives the error: Could not connect to server.. Note that this error is not logged in log.log, it only appears in the interface of webclient.
Also, i tried to manually connect through webclient when the autofill doesn't works, and i get the same connection error.

Note that connection through normal .onion domain works, so this is not related to a resolution error of .onion domains on my computer.

@vladimiry
Copy link
Owner Author

vladimiry commented Dec 4, 2018

Thanks for testing, as I don't know yet how to properly cook the Tor stuff, will probably have to learn it in order to test the feature on my own. I just hoped I just build the official web client configured to use the Tor domain and it will be working, but things seem to be more complicated. The Protonmail team might be using internal git branch with some hidden changes/commits for preparing the Tor build. If that's so, then it's not going to be possible to pack the configured to use Tor web client into the app. Going to take another look.

@vladimiry
Copy link
Owner Author

it only appears in the interface of webclient.

This error is thrown by Protonmail's web client. At this time I tend to mess with Protonmail only to the extent needed to fulfill the specific tasks, as for example reactive database syncing logic. Which means I don't set up the errors interceptor for Protonmail web client, so by design.

@vladimiry
Copy link
Owner Author

vladimiry commented Dec 5, 2018

@nil0x42 can you take another look at the updated beta9 build? I enabled some enhancements of calling remote API by built-in web clients (CORS requests).

It worked for me with Tor + Privoxy setup. Don't know how you make the app handle the .onion domains. For testing, I simply hardcoded the proxy configuration to the source code starting the app in dev mode.

@nil0x42
Copy link

nil0x42 commented Dec 6, 2018

Ahhhh if you hardcoded 127.0.0.1:9050 it explains why it will never work on my machine, which is already jailed in a tor gateway without needing proxy. So i'll run a fake socks proxy in this port that simply does nothing in order to be able to test

@nil0x42
Copy link

nil0x42 commented Dec 6, 2018

Edit: I just tested tor built-in webclient from latest beta9 and it works better, more accounts seem to be able to log-in simultaneously. Therefore i still have the erros mentioned here: #77 (comment)

Also, there is a new bug that i never saw before: when i was adding my accounts with "tor webclient", the accounts were connecting one after another, as i added them. But after i restarted the app, most instances connected, but the padlock icon stays as locked, as if not account were connected.

@vladimiry
Copy link
Owner Author

Ahhhh if you hardcoded 127.0.0.1:9050 it explains why it will never work on my machine

I hardcoded stuff on my machine only to test the Tor scenario. I don't push hardcoding to the repository nor to the installation packages.

@nil0x42
Copy link

nil0x42 commented Dec 6, 2018

With built-in webclient, the behavior, even i still probably related to #77 (comment), i sometimes different.

With built-in webclient, when an instance faild to connect i have 2 kinds of behaviors:

  1. The interface says: Problem loading your account, and if i click on "Login" after some time, sometimes it logs-in correctly, and sometimes it just triggers behavior 2:
    behavior_1

  2. The credentials prompt is not filled, and there is no way to log-in without restarting email-securely-app:
    behavior_2

It instead of using built-in webclient i use online tor domain, only behavior 2 happens.

@vladimiry
Copy link
Owner Author

vladimiry commented Dec 6, 2018

With built-in webclient, when an instance faild to connect i have 2 kinds of behaviors:

Do all these issues occur with Tor connection only?

@vladimiry
Copy link
Owner Author

what about tutanota? does that also have a builtin web version?

@jeroen7s, built-in web client feature has been enabled for Tutanota with updated beta9 build. Please confirm that it works as expected. @nil0x42, it might be helpful for you too.

local-tuta

@vladimiry
Copy link
Owner Author

Also, there is a new bug that i never saw before

I think this issue is gone with the updated beta9 build, at least I fixed a similar recently introduced issue. Thanks for noticing the issue and giving a feedback.

@vladimiry
Copy link
Owner Author

I tested the Tor/.onion case with Tor+Privoxy setup (default configs) and it worked. App was started on Linux like this (proxy-server and host-resolver-rules are basically google chrome arguments):

`(which email-securely-app)` --proxy-server="http://127.0.0.1:8118" --host-resolver-rules="MAP * 0.0.0.0 , EXCLUDE 127.0.0.1"

Beta 9 release has been just published. It includes built-in web client support for tutanota and enables the Tor/.onion entry point for use with protonmail's built-in web client.

Closing the issue as resolved.

@jeroenev
Copy link

jeroenev commented Dec 8, 2018

tested both Tutanota and ProtonMail
seems to work well
great work!

@ghost
Copy link

ghost commented Mar 20, 2019

Great work! This feature is the only thing that makes Protonmail worthy of endorsement.

Regarding nomenclature in the UI

I think "web client: built-in" and "web client: online" are ambiguous. I'm not sure that most users would intuitively know what it means, particularly because "built-in" makes people wonder "built-in to what".. from which viewpoint?" It could be referring to the code that is built-in to protonmail.com's implementation (the remote copy), or it could be built-in to ElectronMail. And use of the term "web" kind of skews it to suggest it's the code built-in w.r.t. the website, as well as the idea that the PM hostname appears next to it (implying that the code will come from that host).

So I suggest killing off ambiguity and guesswork by using some or all of this language on the setup page:

  • local static application (updates controlled by you or your admin)
  • remote server-distributed dynamic application (live version pushed by Protonmail on-the-fly)

And possibly make the target hostname of choice separate from the software choice.

weird browser launch

When clicking "documentation" on that setup page (for proxy rules), firefox is launched but this instance of firefox does not correspond with any local firefox profiles. So it's astonishing to the user and leaves them wondering which profile was used and whether that was launched on the Tor network. Luckily in my case i've firejailed Electronmail so it must be over tor, but most other users may be left wondering how firefox was launched. (or not, perhaps the fact that i ran in a firejail caused firefox to be denied access to the local settings).

Tor sandbox works

@nil0x42

Ahhhh if you hardcoded 127.0.0.1:9050 it explains why it will never work on my machine, which is already jailed in a tor gateway without needing proxy. So i'll run a fake socks proxy in this port that simply does nothing in order to be able to test

Not sure what your setup is but It works in a firejail sandbox with the uplink isolated on a tor middlebox. e.g. something like:

$ firejail --net=tornet0 --dns=<DNSPort from torrc> --whitelist="$HOME"/.config/electron-mail electron-mail

(edit) seems the firejail whitelisting is broken; the config does not persist. (filed netblue30/firejail#2617)

@vladimiry
Copy link
Owner Author

I think "web client: built-in" and "web client: online" are ambiguous.

I tend to keep web client prefix. Web client basically means the official web client, ie the thing you open in browser, code / live and code / live. So does something like web client: prepackaged and web client: online/live sound better?

@vladimiry
Copy link
Owner Author

weird browser launch

The app simply opens a link in a default browser which you can configure system-wide using xdg-open on linux and other means on win/mac platforms.

@ghost
Copy link

ghost commented Mar 21, 2019

I think "prepackaged" is also ambiguous.. could mean prepackaged from Electronmail or from the target server. "prepackaged in Electronmail", "built into Electronmail", "Electronmail version", or "Electronmail embedded" would be clear. I imagine you're trying to keep it short but that's all I can think of ATM. Maybe I'm overthinking it but this is not the sort of option where ambiguity is harmless.

@vladimiry
Copy link
Owner Author

v3.0.1 is released with renamed badges. I tend to think that an explanation of built-in/prepackaged web clients meaning is described in sufficient details in the readme file which includes a link to the original issue with more details.

@vladimiry
Copy link
Owner Author

vladimiry commented Mar 21, 2019

Maybe static / prepackaged and live / online? 😄 Though online version is also supposed to be static just loaded via the internet. This this is also ambiguous.

This was referenced Mar 22, 2019
@vladimiry
Copy link
Owner Author

I'm adding description links block near to the field so users could get here and get familiar with technical stuff if needed:

domain

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants