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

Revert same port http -> https redirect #10930

Merged
merged 13 commits into from
Jul 7, 2017
16 changes: 2 additions & 14 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -202,19 +202,7 @@ In development mode, Kibana runs a customized version of [Webpack](http://webpac

#### Setting Up SSL

When Kibana runs in development mode it will automatically use bundled SSL certificates. These certificates won't be trusted by your OS by default which will likely cause your browser to complain about the certificate.

If you run into this issue, visit the development server and configure your OS to trust the certificate.

- OSX: https://www.accuweaver.com/2014/09/19/make-chrome-accept-a-self-signed-certificate-on-osx/
- Windows: http://stackoverflow.com/a/1412118
- Linux: http://unix.stackexchange.com/a/90607

There are a handful of other options, although we enthusiastically recommend that you trust our development certificate.

- Click through the warning and accept future warnings
- Supply your own certificate using the `config/kibana.dev.yml` file
- Disable SSL in Kibana by starting the application with `npm start -- --no-ssl`
Kibana includes a self-signed certificate that can be used for development purposes: `npm start -- --ssl`.

### Linting

Expand Down Expand Up @@ -307,7 +295,7 @@ npm run test:browser -- --dev # remove the --dev flag to run them once and close
* Open VMWare and go to Window > Virtual Machine Library. Unzip the virtual machine and drag the .vmx file into your Virtual Machine Library.
* Right-click on the virtual machine you just added to your library and select "Snapshots...", and then click the "Take" button in the modal that opens. You can roll back to this snapshot when the VM expires in 90 days.
* In System Preferences > Sharing, change your computer name to be something simple, e.g. "computer".
* Run Kibana with `npm start -- --no-ssl --host=computer.local` (substituting your computer name).
* Run Kibana with `npm start -- --host=computer.local` (substituting your computer name).
* Now you can run your VM, open the browser, and navigate to `http://computer.local:5601` to test Kibana.

#### Running Browser Automation Tests
Expand Down
9 changes: 8 additions & 1 deletion docs/migration/migrate_6_0.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,11 @@ This is no longer the case. Now, only commas are a valid query separator: e.g. `
=== Time-interval based index patterns are no longer supported
*Details:* Starting in Kibana 6.0.0 we removed the ability to create index patterns that use a date-pattern and interval to identify Elasticsearch indices. Index patterns must now use wildcards which are more performant in most cases.

*Impact:* Existing index patterns and saved objects will continue to function without issue, and in a subsequent release we will provide utilities to migrate your index patterns/saved objects.
*Impact:* Existing index patterns and saved objects will continue to function without issue, and in a subsequent release we will provide utilities to migrate your index patterns/saved objects.


[float]
=== Removed same port http to https redirect behavior
*Details:* Kibana 5.x redirected requests from http to https on the same port if TLS was configured. Starting in Kibana 6.0.0, Kibana no longer redirects basic http traffic to https.

*Impact:* With the new configuration setting `server.ssl.redirectHttpFromPort` you can specify a port that will redirect from http to https. This cannot be the same port as the https port.
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@
"dependencies": {
"@bigfunger/jsondiffpatch": "0.1.38-webpack",
"@elastic/datemath": "2.3.0",
"@elastic/httpolyglot": "0.1.2-elasticpatch1",
"@elastic/webpack-directory-name-as-main": "2.0.2",
"@spalger/filesaver": "1.1.2",
"@spalger/leaflet-draw": "0.2.3",
Expand Down
2 changes: 1 addition & 1 deletion src/cli/serve/serve.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export default function (program) {
if (canCluster) {
command
.option('--dev', 'Run the server with development mode defaults')
.option('--no-ssl', 'Don\'t run the dev server using HTTPS')
.option('--ssl', 'Run the dev server using HTTPS')
.option('--no-base-path', 'Don\'t put a proxy in front of the dev server, which adds a random basePath')
.option('--no-watch', 'Prevents automatic restarts of the server in --dev mode');
}
Expand Down
1 change: 1 addition & 0 deletions src/server/config/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export default () => Joi.object({
basePath: Joi.string().default('').allow('').regex(/(^$|^\/.*[^\/]$)/, `start with a slash, don't end with one`),
ssl: Joi.object({
enabled: Joi.boolean().default(false),
redirectHttpFromPort: Joi.number(),
certificate: Joi.string().when('enabled', {
is: true,
then: Joi.required(),
Expand Down
2 changes: 2 additions & 0 deletions src/server/http/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { handleShortUrlError } from './short_url_error';
import { shortUrlAssertValid } from './short_url_assert_valid';
import shortUrlLookupProvider from './short_url_lookup';
import setupConnectionMixin from './setup_connection';
import setupRedirectMixin from './setup_redirect_server';
import registerHapiPluginsMixin from './register_hapi_plugins';
import xsrfMixin from './xsrf';

Expand All @@ -17,6 +18,7 @@ export default async function (kbnServer, server, config) {

const shortUrlLookup = shortUrlLookupProvider(server);
await kbnServer.mixin(setupConnectionMixin);
await kbnServer.mixin(setupRedirectMixin);
await kbnServer.mixin(registerHapiPluginsMixin);

// provide a simple way to expose static directories
Expand Down
21 changes: 1 addition & 20 deletions src/server/http/setup_connection.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import { readFileSync } from 'fs';
import { format as formatUrl } from 'url';
import httpolyglot from '@elastic/httpolyglot';
import { map } from 'lodash';
import secureOptions from './secure_options';

Expand Down Expand Up @@ -40,8 +38,7 @@ export default function (kbnServer, server, config) {

server.connection({
...connectionOptions,
tls: true,
listener: httpolyglot.createServer({
tls: {
key: readFileSync(config.get('server.ssl.key')),
cert: readFileSync(config.get('server.ssl.certificate')),
ca: map(config.get('server.ssl.certificateAuthorities'), readFileSync),
Expand All @@ -51,22 +48,6 @@ export default function (kbnServer, server, config) {
// We use the server's cipher order rather than the client's to prevent the BEAST attack
honorCipherOrder: true,
secureOptions: secureOptions(config.get('server.ssl.supportedProtocols'))
})
});

server.ext('onRequest', function (req, reply) {
// A request sent through a HapiJS .inject() doesn't have a socket associated with the request
// which causes a failure.
if (!req.raw.req.socket || req.raw.req.socket.encrypted) {
reply.continue();
} else {
reply.redirect(formatUrl({
port,
protocol: 'https',
hostname: host,
pathname: req.url.pathname,
search: req.url.search,
}));
}
});
}
40 changes: 40 additions & 0 deletions src/server/http/setup_redirect_server.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { format as formatUrl } from 'url';
import Hapi from 'hapi';

// If a redirect port is specified, we need to start a server a non-ssl
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to start a server a non-ssl server at this doesn't english good.

// server at this port, and redirect all requests to the ssl port.
export default function (kbnServer, server, config) {
const isSslEnabled = config.get('server.ssl.enabled');
const portToRedirectFrom = config.get('server.ssl.redirectHttpFromPort');

// Both ssl and port to redirect from must be specified
if (!isSslEnabled || portToRedirectFrom === undefined) {
return;
}

const host = config.get('server.host');
const sslPort = config.get('server.port');

const redirectServer = new Hapi.Server();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't block on this, but I really wish we just used a native node http server for this. I have much greater confidence in that abstraction in general than I do in hapi, both in terms of how lightweight it is, and how stable the API is.

This works though.


redirectServer.connection({
host,
port: portToRedirectFrom
});

redirectServer.ext('onRequest', (req, reply) => {
reply.redirect(formatUrl({
protocol: 'https',
hostname: host,
port: sslPort,
pathname: req.url.pathname,
search: req.url.search,
}));
});

redirectServer.start((err) => {
if (err) {
throw err;
}
});
}
2 changes: 0 additions & 2 deletions tasks/config/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ module.exports = function (grunt) {
'--elasticsearch.url=' + format(esTestServerUrlParts),
'--dev',
'--no-base-path',
'--no-ssl',
'--optimize.lazyPort=5611',
'--optimize.lazyPrebuild=true',
'--optimize.bundleDir=optimize/testUiServer',
Expand Down Expand Up @@ -165,7 +164,6 @@ module.exports = function (grunt) {
...buildTestsArgs,
'--dev',
'--no-watch',
'--no-ssl',
'--no-base-path',
'--server.port=5610',
'--optimize.lazyPort=5611',
Expand Down
10 changes: 5 additions & 5 deletions tasks/lib/notice/__tests__/notice.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ const NODE_MODULES = resolve(__dirname, '../../../../node_modules');
const NODE_DIR = resolve(process.execPath, '../..');
const PACKAGES = [
{
name: '@elastic/httpolyglot',
version: '0.1.2-elasticpatch1',
name: '@spalger/filesaver',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's switch this to being a controlled package that we know will always exist because we've created it specifically for this purpose. We can create an npm package under the @elastic org that does nothing except provide the license we want to verify against, and then we can depend on that package in our devDependencies. We should never need to update this test just because we decided to no longer depend on a given package in our source.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okey if I do it in a separate PR? (Just want to make sure this gets ready for 6.0)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah

version: '1.1.2',
licenses: ['MIT'],
directory: resolve(NODE_MODULES, '@elastic/httpolyglot'),
relative: 'node_modules/@elastic/httpolyglot',
directory: resolve(NODE_MODULES, '@spalger/filesaver'),
relative: 'node_modules/@spalger/filesaver',
},
{
name: 'aws-sdk',
Expand Down Expand Up @@ -41,7 +41,7 @@ describe('tasks/lib/notice', () => {
});

it('includes *LICENSE* files from packages', () => {
expect(notice).to.contain(readFileSync(resolve(NODE_MODULES, '@elastic/httpolyglot/LICENSE'), 'utf8'));
expect(notice).to.contain(readFileSync(resolve(NODE_MODULES, '@spalger/filesaver/LICENSE.md'), 'utf8'));
});

it('includes the LICENSE file from node', () => {
Expand Down