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

fix: SocketIO HTTPs bug #982

Closed
wants to merge 1 commit into from
Closed

Conversation

SaiFi0102
Copy link
Contributor

Socket IO messages are not received on HTTPs when the configuration:
ssl_ecdh_curve is set to secp384r1

Setting it to auto solves it according to Telegram Developer's Group message by Shadoyip

@Thunderbottom
Copy link
Contributor

mistakenly clicked on the close button before commenting. i think this really isnt an issue as of now. this defaults to prime256v1, which even though secure is not as fast as secp384r1.

there is some other misconfiguration that causes socketio to fail and not this, since most (all modern?) clients do support secp384r1, and is recommended to be used as the default cipher for both speed and security.

@Thunderbottom
Copy link
Contributor

just checked with @revant, socketio works without these changes too.

could you tell us what exactly did not work for you?

@Thunderbottom
Copy link
Contributor

Thunderbottom commented May 4, 2020

could you try setting the socketio block to this (without making the changes from this PR):

location /socket.io {
		proxy_http_version 1.1;
		proxy_set_header Upgrade $http_upgrade;
		proxy_set_header Connection "upgrade";
		proxy_set_header X-Frappe-Site-Name $http_host;
		proxy_set_header Origin $scheme://$http_host;
		proxy_set_header Host $host;

		proxy_pass http://{{ bench_name }}-socketio-server;
	}

and let me know if it works?

basically setting the X-Frappe-Site-Name header to $http_host

@pm-at
Copy link

pm-at commented May 4, 2020

I'm facing this issue too. My results as below:

basically setting the X-Frappe-Site-Name header to $http_host

Works after nginx reload but breaks after supervisorctl reload

Setting ssl_ecdh_curve to auto works after nginx reload as well as supervisorctl reload

This thing broke silently so didn't notice until a user complained.

@revant
Copy link
Collaborator

revant commented Jul 5, 2020

just checked with @revant, socketio works without these changes too.

I only checked chat. It was working.

Opening same doctype by two users doesn't show message anymore. (v12)

on develop branch everything is working as expected in manual production installation.

@sagarvora
Copy link
Collaborator

Here's what worked for me. Node JS v8 is what gets installed with easy install script. I just upgraded to v12 (latest stable) and the SSL error went away.

@sagarvora
Copy link
Collaborator

Here's what worked for me. Node JS v8 is what gets installed with easy install script. I just upgraded to v12 (latest stable) and the SSL error went away.

@SaiFi0102 can you try this please?

@SaiFi0102
Copy link
Contributor Author

SaiFi0102 commented Jul 28, 2020

Fortunately, with Node v12 and HTTPS and Frappe v12 it's working just fine.
Unfortunately, I cannot test both Node v8 and Node v12...

There was another problem that caused issue with reloading document when the document was modified on another session which I solved here frappe/frappe#11137

@adityahase
Copy link
Member

@SaiFi0102 Can we close this now?

@SaiFi0102
Copy link
Contributor Author

Please go ahead

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

Successfully merging this pull request may close these issues.

6 participants