Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Webpack Dev Server host check issue with 0.0.0.0 #187

Closed
constgen opened this issue Apr 23, 2017 · 9 comments
Closed

Webpack Dev Server host check issue with 0.0.0.0 #187

constgen opened this issue Apr 23, 2017 · 9 comments
Assignees
Milestone

Comments

@constgen
Copy link
Contributor

constgen commented Apr 23, 2017

Starting from v2.4.3 'webpack-dev-server' has additional security measures. It checks that listening hostname matches the hostname in the provided 'host' header (devServer.headers = {host: '192.168.1.100'}). As exceptions it always allows localhost and 127.0.0.1.
Currently the Neutrino will always fail to start the server of host is reconfigured to something different then localhost and 127.0.0.1, e.g. 0.0.0.0 for local network access. The page will be empty with the only error message 'Invalid Host header'.
What are options to resolve this issue.

  1. Disable the Dev Server security. We need to set devServer.disableHostCheck = false. But this flag is not supported in the webpack-chain. Have to add it.
  2. Smartly synchronize host with headers.host. Need to consider here that consumer also may redefine options in different ways.
  3. Make a pull request to the 'webpack-dev-server' to check at list 0.0.0.0 as allowed. Another addresses will be handled by the option 2 above or manually by the consumer.

The Webpack Dev Server commit with these changes:
webpack/webpack-dev-server@2957853

Actually I see a lot of problems in the future if we will keep the dev server completely configurable by developers. In my opinion it is better to provide an abstraction with one-two flags. Ideally there should be one ultimate solution for everyone without any configuration.

@eliperelman
Copy link
Member

Hmm, I'm not sure that any of those 3 solutions are the correct answer. Choosing any of those sounds like they could leave a local app vulnerable, and if we are opting for smart defaults, I would prefer to err on the side of secure over convenient.

@edmorley any thoughts?

@barraponto
Copy link
Contributor

barraponto commented Apr 24, 2017

I guess option 4, implied by @constgen, would be to allow the user to set devserver.allowConnectionsFrom to either local or global, with proper implementation provided by Neutrino. I like it.

@edmorley
Copy link
Member

edmorley commented Apr 24, 2017

I've filed a retrospective GitHub issue with the original private disclosure email wording, which should hopefully make things a bit clearer: webpack/webpack-dev-server#887 - happy to answer any additional questions.

I've added some comments to webpack/webpack-dev-server#882 as to ways that the security check can be adjusted to require less configuration in common cases.

@constgen
Copy link
Contributor Author

Ok. I will try the suggestions and report here

@eliperelman
Copy link
Member

Any chance we can get this in for v6?

@eliperelman eliperelman added this to the v6 milestone May 4, 2017
@constgen
Copy link
Contributor Author

constgen commented May 6, 2017

I would like to implement it in this version. Yes

@constgen
Copy link
Contributor Author

constgen commented May 6, 2017

Have done a significant refactoring of the Webpack Dev Server usage in Neutrino. Now we have neutrino-middleware-dev-server. The documentation is not ready yet. All changes are commited to this branch https://github.com/mozilla-neutrino/neutrino-dev/tree/feature/dev-server-ip
The issue is resolved. I used the '--public' flag of the webpack-dev-server internaly as a solution.
So what are changes in Neutrino:

  1. neutrino-preset-web now uses neutrino-middleware-dev-server
  2. host option for server is gone. Now use public: true or public: false (default). If true the localhost works any way.
  3. open option was added. false by default. If true a default browser is opened with public or localhost URL depending on the public option. I noticed that Webpack Dev Server has this option too, but we can't utilize it because webpack-chain API doesn't support it.
  4. neutrino.options.config.devServer configuration path was changed to neutrino.options.server. At first it seems to be broken in current version. At least I couldn't customize my server in this path. At second the new path is more universal and gives some compatibility with another alternatives in the future, e.g. BrowserSync middlware.

Want to discuss one moment. process.env.HOST is still used internally for sharing local IP between modules. That means that somebody still can change it through CLI. This will replace detected computer IP with what you pass to this parameter. I am looking for the best way to handle all these things. May be somebody can explain me in which cases you want to change --env.HOST. I am little lost.

@eliperelman
Copy link
Member

To give an update here, the implementation from @constgen has resolved this, and I tweaked it a little since at the moment there aren't any environment variables being used.

public takes precedence over host, and without host, it is set to localhost. If host isn't localhost an no public value is set, public is set to host, and host is set to 0.0.0.0, which I believe is safe since it's still bound to whatever host is (unless host is set to 0.0.0.0, in which case the user would need to set disableHostCheck or have proper proxy in place).

We will come back to environment variables in a follow up issue.

@eliperelman
Copy link
Member

If there are issues with this, please raise them in another issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

4 participants