-
Notifications
You must be signed in to change notification settings - Fork 447
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
[WIP] Updated early stage of websocket proxy development #2275
Conversation
Any feedback would be greatly appreciated so that implementation can proceed. |
I will be able to take a look at this by end of day Monday. Sorry that I missed this earlier. |
I attempted to build this from your repo/branch with the following commands: Using those commands I can build from BOINC master and produce the client. However, when building your branch I got the following error: CXX boinc_client-gui_rpc_server_ops.o If you could let me know what is needed to build, I'll try again. Thanks! |
The μWebSockets library (https://github.com/uNetworking/uWebSockets) has to be downloaded and installed (by installation I mean moving the libuWS.so file resulting from compilation of the library to the /usr/lib/ -- or any of the directories in the search path) so that the header file can be found. I have not included it in my branch and this is the reason the checks do not pass. I believe it would be better if we concluded that this is the library we want to use before including it. |
This also should be done for Windows build. |
One of the most important criteria when choosing the library was the cross platform support. uWebsockets supports Windows among others. But at this stage it is only tested on Linux and in no way plug n play. When the choice is finalised it can be fully integrated. |
Still having troubles getting this to run. I got uWebSockets installed and got the client to compile. However, I had to compile with with this option: I created the SSL certificate in the client directory and started boinc and got: [knreed@oc7811586721 boinc]$ ./boinc Exiting... Running it under gdb gave this stack trace: (gdb) backtrace Not sure what is going on. The compiler reported the c++11 was experimental in my version of g++ so perhaps that is the issue. I'm on RedHat Desktop 7 running g++ (GCC) 4.8.5 20150623 (Red Hat 4.8.5-16). What did you build and test on? |
I have tested both on Ubuntu 16.04.3 LTS and CentOS 7.4. I should have pointed I used: |
I apologize for the very overdue response. I was able to get this example working. For those who want to try this is what I did (summarizing various responses from @nikolasgianna above). Setup uWebSockets Build Client
I copied the following binaries built in the previous step and placed them into /home/knreed/test-boinc
Create self-signed ssl*
Start BOINC Accept Self-Signed Certificate Complete Test |
Having this ability to communicate from a BOINC project website to the BOINC core client could be useful in a number of ways. Some that I can think of are as follows:
However, I also see a number of concerns:
This is an intriguing idea, but there are some important security concerns that need to be figured out. |
This ticket still breaks Windows build: |
#include <thread> | ||
#include <iostream> | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need all these empty lines?
@@ -406,6 +411,81 @@ int boinc_main_loop() { | |||
return finalize(); | |||
} | |||
|
|||
// | |||
// | |||
// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty lines
|
||
// | ||
// | ||
// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty lines
} | ||
// | ||
// | ||
// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty lines
// | ||
int connect_sock () { | ||
|
||
int portno = 31416; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this should be a parameter somewhere in cc_config.xml rather than just hardcoded number?
// | ||
void init_websocket() { | ||
|
||
std::vector<std::thread *> threads(/*std::thread::hardware_concurrency()*/1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no c++11 support on Windows client now and this function can fail while building on Visual Studio 2010. I know that we plan to support Visual Studio 2017 but there is no plans (as far as I know) to drop support of Visual Studio 2010. If we still want to support Visual Studio 2010 this must be compiled using it (unfortunately, current AppVeyor script use Visual Studio 2013 for building) or use boinc own implementation of threads.
int sockfd = connect_sock(); | ||
char buf[50000]; | ||
memset(buf, 0, sizeof(buf)); | ||
snprintf(buf, /*sizeof(buf)*/ (int) length + 1, "%s", /*(int) msg.len,*/ message); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
possible buffer overflow. you buf variable is 50000 bytes long but you use input variable length that can be more than 50000
snprintf(buf, /*sizeof(buf)*/ (int) length + 1, "%s", /*(int) msg.len,*/ message); | ||
int num = write(sockfd, buf, (int) length + 1); | ||
if (num < 0) | ||
printf("The write error is: %s\n", strerror(errno)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Windows has no errno constant (take a look at my comment above)
char recv_buf[50000]; | ||
memset(recv_buf, 0, sizeof(recv_buf)); | ||
int r = recv(sockfd, recv_buf, 50000, 0); | ||
if (r < 0) printf("\n\nThe error of recv is: %s", strerror(errno)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Windows has no errno constant (take a look at my comment above)
|
||
}); | ||
if (!h.listen(3000, uS::TLS::createContext("server.pem", | ||
"server.key")/*, uS::ListenOptions::REUSE_PORT*/)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it ok to have key names hardcoded?
@AenBleidd - It is my understanding that this is a very early proof of concept rather than a serious pull request. I don't believe that the @nikolasgianna expected to have this merged in its current form but rather wanted to explore the concept and let people see it in real life and the discuss if the concept as a whole should be pursued. |
In any case I've pointed places that will not definitely work on Windows. I guess it is important to keep in mind this while working on final implementation |
Definitely useful - thanks for doing it. I just wanted to make sure you knew that this wasn't immediately bound for real world use. |
Here is some initial feed back to the concerns:
The only sensitive operation is connecting to a project as all other operations require the authenticator. By default only connecting to official projects in the white list will be allowed. Additional projects can be manually added to the client configuration to support development activities.
As far as I understand all operations require the authenticator. This is stored in a cookie after doing the Web login.
This can be a self-signed certificate generated during the client installation. A clean method is required to add this to the trusted certificates in the browser configuration.
The RPC already exists so the attack surface is has not increased significantly. |
What's the intended target audience for the recipients of this service? I think there are some fundamental networking questions which may be beyond some audience segments. Users in corporate environments - may be behind a centrally-managed firewall. |
Hi folks, just discovered this PR, very very nice one, although i have familiarity with WebRTC, i have no such thing for BOINC, line wire protocol, I will need to read into this part of BOINC if i want to contribute |
It is my opinion that a user must explicitly consent within the BOINC client installed on their machine before an attach can complete. If we don't require this, then if (for example, and he wouldn't actually) @brevilo goes rogue and modifies E@H so that the website automatically attaches anyone who visits the website. Or, someone could hack the BOINC website and add a new malicious project to the approved project list. They could then hack a site popular with volunteers and start attaching them to his malicious project. Doing any form of automatic attach based on requests from a website opens up the opportunity for some very bad behavior.
The use of the authenticator in the cookie as the authorization token for the website is a really insecure practice and is on my list of things to fix (I do not know when I can get to it though). We should not be relying on this mechanism to obtain the authenticator. Additionally, in the long run, it is my opinion that the authenticator should be reduced so that it is only used to indicate that a specific computer is owned by a specific user account and should have no other meaning or use in the system. Having said that, I believe that all the client RPC commands require the use of the value in the gui_rpc_auth.cfg file not the authenticator. Since the value of the gui_rpc_auth.cfg is not available to the browser, it would probably be necessary to implement a API on the website to get a nonce that is used to authorize browser-client communications. When the browser calls the client with the nonce, the client will have to call the server to validate that the nonce is associated with the user. There might be another way of doing this, but that was my first thought.
I think that this is dangerous because it weakens the security of the browsers. For example, if the private key for the self-signed certificate was obtained by an attacker it would make the browsers on that computer vulnerable to man in the middle attacks. Since we have no way to revoke the self-signed certificate, once this happened there would be nothing we could do to fix the situation. Here is an example of where someone tried an approach similar (although much worse - I realize that we would generate the self-signed root certificate unique to each machine and then once the https certificate is generated I presume we would delete the private key of the root certificate) then this: https://www.digicert.com/blog/lenovos-superfish-adware-perils-self-signed-certificates/ Further - browsers like Firefox deploy with their own store of certificates and those get replaced on each update (I don't know what the other browsers do). Once the browser was updated, it wouldn't have the self-signed root certificate anymore and thus the mechanism would stop working. |
This should be fairly straight forward to do and is very similar to the auto-attach mechanism already in place.
Now I am confused. I looked at the protocol specification to refresh my memory but it didn't help.
There is a good summary which states that http://127.0.0.1 can be considered secure so we may not need a certificate. |
Great. As long as the BOINC client requires the user acknowledge the attach request before performing it, then I think it will be good.
I looked at the code which lead me to this output from the boinccmd tool. The password referred to in the link you provided is directly associated with the contents of the gui_rpc_auth.cfg file.
That would be good. I hope that ws://127.0.0.1 and http://127.0.0.1 are considered secure domains. The discussion around that on links off the article you posted are extensive and somewhat confusing. Please let us know what your testing unveils. |
The feedback has been added to issue #2113. A new PR will be provided once the issues have been resolved. |
(Update of #2246)
This is a first approach and attempt at creating a proxy to allow the BOINC client to understand commands issued via websockets. The proxy should run within the same executable so that the whole procedure is transparent to the volunteers. As it stands now this is a prototype and should not be expected to function in any meaningful way. For that reason this pull request is not expected or intended to be merged. It is rather a way to start a conversation around the issue and to get more eyes on such an important piece of the potential future BOINC client.
This is a small test page: https://lhcathomedev.cern.ch/lhcathome-dev/websock_test.html that will return the version of a running client that implements these changes. The communication is done solely through websockets.
Since Firefox expects Secure Websockets (because the javascript is served from an https server) in this stage you will have to create your own certificate and key with (run from client directory):
openssl req -new -newkey rsa:2048 -days 365 -nodes -x509 -keyout server.key -out server.pem
and then add it by visiting https://localhost:3000 and following the prompts (add exception).
Prior to implementing changes this library has to be downloaded and installed:
https://github.com/uNetworking/uWebSockets
Note: This is only tested on Linux (Debian and CentOS) and should not work on Windows or macOs