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

SSL handling #99

Closed
josephdenne opened this issue Nov 21, 2016 · 11 comments
Closed

SSL handling #99

josephdenne opened this issue Nov 21, 2016 · 11 comments
Assignees
Milestone

Comments

@josephdenne
Copy link
Member

At the moment configuration appears to enable SSL as being either on or off. SSL needs to be supported in a variety of configurations:

  • Just SSL
  • SSL, with support for routing traffic from HTTP to HTTPS
  • HTTPS alongside HTTP (either/or support)

In addition the method of recognising/enforcing an SSL request needs attention. In a load balanced setup the load balancer will usually handle SSL, piping requests as HTTP to the end instances. This architecture doesn't currently work with DADI Web.

@adamkdean
Copy link
Contributor

Happy to look at this, @jimlambie

This was referenced Nov 30, 2016
@adamkdean adamkdean assigned adamkdean and unassigned jimlambie Dec 9, 2016
@adamkdean
Copy link
Contributor

Hmm. Making progress on this. In essence it's fairly simple to implement, but there is a knock on effect when we're running on both HTTP + HTTPS.

I see the modes as HTTP, HTTP + HTTPS, HTTP => HTTPS, and HTTPS.

When running in HTTP, HTTP => HTTPS, and HTTPS modes, we're only really dealing with a single server instance, which the app then adds handlers and events to etc. When we're running with HTTP + HTTPS mode, we now have to separate instances, and so as we consume them we'll need to handle this duality.

I'm going to get the HTTP, HTTP => HTTPS, and HTTPS modes working for now, and I'll have a chat with @jimlambie on Monday to see what the best course of action is for HTTP + HTTPS.

@adamkdean
Copy link
Contributor

adamkdean commented Dec 9, 2016

Ok here is what I have so far: d5b4bb0

Tests still need to be updated but don't want to do that until we have decided this is the way we want to go. Getting a strange issue though while testing (https://github.com/adamkdean/dadi-demo-stack) where when you're on https://127.0.0.1 for example, if you click a link that is <a href="/test">test</a> which shows up in Chrome/Firefox status bar as being https://127.0.0.1/test, for some reason, it strips the https protocol off the request and sends you to http://127.0.0.1/test which then fails.

Need to discuss with @jimlambie before taking this any further.

@adamkdean
Copy link
Contributor

I've now fixed the issue with redirecting to http. There was a hardcoded protocol redirect adding trailing slashes but removing the current protocol.

I have also reverted some of the config changes, so config will no longer undergo breaking changes – which is good. Picking up where I left off, I've been running into numerous issues, and the easiest way has been to use the original approach, which is now all working great.

The only thing which this doesn't address is HTTPS alongside HTTP (either/or support) due to the need to have multiple server instances.

@adamkdean adamkdean mentioned this issue Jan 10, 2017
@adamkdean
Copy link
Contributor

@jimlambie if you're happy with this approach I can roll it out to api and cdn

@josephdenne
Copy link
Member Author

Can you explain what you mean by "the need to have multiple server instances" in relation to HTTPS alongside HTTP (either/or support)? Is this a blocker for enabling this functionality? And if so, why?

@adamkdean
Copy link
Contributor

adamkdean commented Jan 12, 2017

In dadi/lib/index.js we take the api instance (https://github.com/dadi/web/blob/master/dadi/lib/index.js#L75) and add all the middleware to it, and then later on (https://github.com/dadi/web/blob/master/dadi/lib/index.js#L186) we take the http/https instance which we then add events and handlers to etc.

As we can't listen to http & https with the same instance, it means if we serve the site up on both, we'll need to return two instances, which means altering the code to work in duality. It is possible but it does have knock on effects everywhere where only once instance is assumed to exist.

@adamkdean
Copy link
Contributor

adamkdean commented Jan 12, 2017

We could look at encapsulating http + https with something like https://github.com/mscdex/httpolyglot which combines them, but I haven't tested it and it looks quite old. Will report back later with update.

Jim & I have decided to go with the dual listener route instead.

@josephdenne
Copy link
Member Author

Do you have the routing of one to the other in place? HTTP>HTTPS, HTTPS>HTTP? If so, dual support isn't really needed.

@adamkdean
Copy link
Contributor

We have HTTP>HTTPS in place, yes, but not the vice versa, as that would still require certificates setup, which if you have them setup, you're probably going to want SSL anyway.

@josephdenne
Copy link
Member Author

This makes sense to me. I'm happy.

@jimlambie jimlambie removed this from Release in Web roadmap Sep 23, 2017
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

3 participants