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

Force SSL by default #1768

Closed
ywecur opened this issue Dec 28, 2015 · 24 comments
Closed

Force SSL by default #1768

ywecur opened this issue Dec 28, 2015 · 24 comments

Comments

@ywecur
Copy link

ywecur commented Dec 28, 2015

Currently there is no way to force clients to connect via SSL, making communication insecure by default.

I'm not sure how this would be implemented, though I know of this meteor package that forces SSL, which might help.

And as @geekgonecrazy has pointed out to me: A toggle might be useful during the initial setup.

@geekgonecrazy
Copy link
Contributor

Also as discussed maybe if the site url has https:// in it then we could force ssl by default. But for sure we need a toggle

@ywecur
Copy link
Author

ywecur commented Dec 28, 2015

@geekgonecrazy Maybe add the contrib: easy lable?

@geekgonecrazy
Copy link
Contributor

Going to tack on intermediate. Because I assume easy, but it might throw a curve ball :)

@engelgabriel
Copy link
Member

Meteor won't handle the SSL directly, the NGINX or other reverse proxy will do that. I think it makes more sense to configure the redirect there (like we do on the demo server) rather than on the app level. It's far more effective/secure and much faster.

@Megatronic79
Copy link

@engelgabriel agree with you there, the reverse proxy should handle the SSL connection termination and can force all http to https easily. (as well as secure numerous other web security issues.)

@ywecur
Copy link
Author

ywecur commented Dec 28, 2015

@Megatronic79 Will this also apply to the mobile apps?

@engelgabriel
Copy link
Member

It will apply to ALL connections.

@Megatronic79
Copy link

The mobile apps connect to the same endpoint as the web services, if the reverse proxy is set to force all http to https then all traffic will be encrypted.

@ywecur
Copy link
Author

ywecur commented Dec 28, 2015

Nevertheless, this is something that should be apparent in the tutorial. I found no mention of this before I deployed my instance to Heroku. Not forcing SSL is a big privacy and security risk IMO.

@vargasbo
Copy link

Agree. Maybe you can document this and issue the PR.

@ywecur
Copy link
Author

ywecur commented Dec 28, 2015

@vargasbo How would one document this? I'd prefer a script that does this automatically, if this is possible to achieve that is.

@vargasbo
Copy link

You can add create a new wiki page (how to force SSL) add your 2 cents, and then link to the existing wiki page.

@Megatronic79
Copy link

SSL and reverse proxy is already part of the wiki here:

https://github.com/RocketChat/Rocket.Chat/wiki/Run-Rocket.Chat-behind-a-SSL-Reverse-Proxy

I guess a step on the Heroku page needs to point to this with a security note.

@engelgabriel
Copy link
Member

I am not sure how you'd configure it at Heroku. They relay on the app to do that. So we may have to implement something on our side to support it at all.

@Sing-Li
Copy link
Member

Sing-Li commented Dec 30, 2015

On Heroku ...

https://devcenter.heroku.com/articles/ssl-endpoint

... might have websocket or sticky sesssion problems ?

@engelgabriel
Copy link
Member

@Sing-Li I've seen the article, but didn't find anything about forcing the SSL... from what I understand, they expect the app to do the redirect.

@mquandalle
Copy link

I also think that it is not the role of a Meteor application to handle http to https redirection. The force-ssl package from MDG was almost a hack to configure this redirection on Galaxy since at the time they didn’t have any Galaxy admin UI but this is kind of a weird package honestly.

Anyway even if the force-ssl package you still need to configure an server proxy like Nginx to handle the SSL certificate, and so it makes perfect sense to configure the redirection at Nginx level as well. Sure not everyone is configuring Nginx on top of Rocket.chat but these users will not be able to use https anyway.

@Sing-Li
Copy link
Member

Sing-Li commented Jan 3, 2016

Perhaps add Nginx to front-end Rocket.Chat on Heroku? https://github.com/ryandotsmith/nginx-buildpack

@mquandalle
Copy link

And maybe the docker-compose could also have a Nginx proxy? Although IIRC docker-compose doesn't advertise itself as ready for production use, so I’m not sure if there is a clean way to install and update a full stack of containers (database, application, Nginx proxy) on production with it.

@MuhClaren
Copy link

@mquandalle A deployment guide exists for rocketchat using docker-compose and nginx (SSL): https://github.com/RocketChat/Rocket.Chat/wiki/Docker---Ubuntu-with-Nginx-SSL-and-Hubot although you're right about the non-production warning with docker-compose at the moment. I've banged on my docker-compose deployments as much as possible, never a problem.

As for nginx https rewrites, In general, the official method for having nginx force http connections to https is to use the return directive in the http server block:

server {
    listen       80;
    server_name  example.org;
    return       301 https://www.example.org$request_uri;
}

source: http://nginx.org/en/docs/http/converting_rewrite_rules.html

edit: removed cloudflare info

@engelgabriel
Copy link
Member

Just because Rocket.Chat is the most popular button at Heroku (see https://elements.heroku.com/), should we try to implement something like https://github.com/kfatehi/meteor-my-force-ssl but make it ONLY triggered/installed if the administrator turns it on the the admin panel?

What do you think @RocketChat/core ?

image

@mquandalle
Copy link

(Ah, Wekan is on the list! I wasn’t aware of that at all!)

@engelgabriel
Copy link
Member

From the top 5, 3 are Meteor apps :)

@ywecur
Copy link
Author

ywecur commented Jan 14, 2016

@engelgabriel I agree with this. Heroku users will have no idea about this issue otherwise and will deploy without knowledge.

engelgabriel added a commit that referenced this issue Jan 18, 2016
engelgabriel added a commit that referenced this issue Jan 18, 2016
* develop: (67 commits)
  Remove toUpperCase from emojione popup config; Fixes #1955
  Make oembed parse title in ungreedy form
  Display time based on locale instead of using fixed 24h format
  Fix audio-recorder not stoping. Closes #1941.
  Sort room files by uploadedAt. Closes #1932
  Allow multi-line title on oembed
  Add some logs
  Fallback LDAP login to local account if LDAP fails
  Add connections status bar to login page
  Created and pushed by LingoHub. Project: 'Rocket.Chat' by User: '[email protected]'.
  send correct content-type for livechat. fixes #1951
  Fix redirect exceptions
  Add i18n strings
  Close #1768; Force SSL
  Close #1925; Add options to enable TLS on LDAP
  Improve admin disbleQuery
  Close #1923; Prevent erros update outgoing webhooks with empty channel
  Fix overlapping windows
  Add Raspberry Pi support announcement
  Updated base.less to use 480x270
  ...
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

9 participants