Skip to content

Commit

Permalink
Change HTTP servers to support P12 key stores and trust stores
Browse files Browse the repository at this point in the history
  • Loading branch information
jportner committed Dec 30, 2019
1 parent 625c45d commit 0593259
Show file tree
Hide file tree
Showing 8 changed files with 263 additions and 7 deletions.
2 changes: 2 additions & 0 deletions src/cli/serve/serve.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ function applyConfigOverrides(rawConfig, opts, extraCliOptions) {
ensureNotDefined('server.ssl.certificate');
ensureNotDefined('server.ssl.certificateAuthorities');
ensureNotDefined('server.ssl.key');
ensureNotDefined('server.ssl.keystore.path');
ensureNotDefined('server.ssl.truststore.path');
ensureNotDefined('elasticsearch.ssl.certificateAuthorities');

const elasticsearchHosts = (
Expand Down
2 changes: 2 additions & 0 deletions src/core/server/http/__snapshots__/http_config.test.ts.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions src/core/server/http/ssl_config.test.mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,10 @@ export const mockReadFileSync = jest.fn();
jest.mock('fs', () => {
return { readFileSync: mockReadFileSync };
});

export const mockReadPkcs12Keystore = jest.fn();
export const mockReadPkcs12Truststore = jest.fn();
jest.mock('../../utils', () => ({
readPkcs12Keystore: mockReadPkcs12Keystore,
readPkcs12Truststore: mockReadPkcs12Truststore,
}));
129 changes: 126 additions & 3 deletions src/core/server/http/ssl_config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@
* under the License.
*/

import { mockReadFileSync } from './ssl_config.test.mocks';
import {
mockReadFileSync,
mockReadPkcs12Keystore,
mockReadPkcs12Truststore,
} from './ssl_config.test.mocks';

import { sslSchema, SslConfig } from './ssl_config';

Expand All @@ -26,12 +30,27 @@ const createConfig = (obj: any) => new SslConfig(sslSchema.validate(obj));
beforeEach(() => {
mockReadFileSync.mockReset();
mockReadFileSync.mockImplementation((path: string) => `content-of-${path}`);
mockReadPkcs12Keystore.mockReset();
mockReadPkcs12Keystore.mockImplementation((path: string) => ({
key: `content-of-${path}.key`,
cert: `content-of-${path}.cert`,
ca: [`content-of-${path}.ca`],
}));
mockReadPkcs12Truststore.mockReset();
mockReadPkcs12Truststore.mockImplementation((path: string) => [`content-of-${path}`]);
});

describe('throws when config is invalid', () => {
beforeEach(() => {
const realFs = jest.requireActual('fs');
mockReadFileSync.mockImplementation((path: string) => realFs.readFileSync(path));
const utils = jest.requireActual('../../utils');
mockReadPkcs12Keystore.mockImplementation((path: string, password?: string) =>
utils.readPkcs12Keystore(path, password)
);
mockReadPkcs12Truststore.mockImplementation((path: string, password?: string) =>
utils.readPkcs12Truststore(path, password)
);
});

test('throws if `key` is invalid', () => {
Expand All @@ -56,13 +75,73 @@ describe('throws when config is invalid', () => {
);
});

test('throws if `keystore.path` is invalid', () => {
const obj = { keystore: { path: '/invalid/keystore' } };
expect(() => createConfig(obj)).toThrowErrorMatchingInlineSnapshot(
`"ENOENT: no such file or directory, open '/invalid/keystore'"`
);
});

test('throws if `keystore.path` does not contain a private key', () => {
mockReadPkcs12Keystore.mockImplementation((path: string, password?: string) => ({
key: undefined,
certificate: 'foo',
}));
const obj = { keystore: { path: 'some-path' } };
expect(() => createConfig(obj)).toThrowErrorMatchingInlineSnapshot(
`"Did not find private key in keystore at [keystore.path]."`
);
});

test('throws if `keystore.path` does not contain a certificate', () => {
mockReadPkcs12Keystore.mockImplementation((path: string, password?: string) => ({
key: 'foo',
certificate: undefined,
}));
const obj = { keystore: { path: 'some-path' } };
expect(() => createConfig(obj)).toThrowErrorMatchingInlineSnapshot(
`"Did not find certificate in keystore at [keystore.path]."`
);
});

test('throws if `truststore.path` is invalid', () => {
const obj = { truststore: { path: '/invalid/truststore' } };
expect(() => createConfig(obj)).toThrowErrorMatchingInlineSnapshot(
`"ENOENT: no such file or directory, open '/invalid/truststore'"`
);
});

test('throws if both `key` and `keystore.path` are specified', () => {
const obj = {
key: '/path/to/key',
keystore: {
path: 'path/to/keystore',
},
};
expect(() => sslSchema.validate(obj)).toThrowErrorMatchingInlineSnapshot(
`"cannot use [key] when [keystore.path] is specified"`
);
});

test('throws if both `certificate` and `keystore.path` are specified', () => {
const obj = {
certificate: '/path/to/certificate',
keystore: {
path: 'path/to/keystore',
},
};
expect(() => sslSchema.validate(obj)).toThrowErrorMatchingInlineSnapshot(
`"cannot use [certificate] when [keystore.path] is specified"`
);
});

test('throws if TLS is enabled but `certificate` is specified and `key` is not', () => {
const obj = {
certificate: '/path/to/certificate',
enabled: true,
};
expect(() => sslSchema.validate(obj)).toThrowErrorMatchingInlineSnapshot(
`"must specify [certificate] and [key] when ssl is enabled"`
`"must specify [certificate] and [key] -- or [keystore.path] -- when ssl is enabled"`
);
});

Expand All @@ -72,7 +151,16 @@ describe('throws when config is invalid', () => {
key: '/path/to/key',
};
expect(() => sslSchema.validate(obj)).toThrowErrorMatchingInlineSnapshot(
`"must specify [certificate] and [key] when ssl is enabled"`
`"must specify [certificate] and [key] -- or [keystore.path] -- when ssl is enabled"`
);
});

test('throws if TLS is enabled but `key`, `certificate`, and `keystore.path` are not specified', () => {
const obj = {
enabled: true,
};
expect(() => sslSchema.validate(obj)).toThrowErrorMatchingInlineSnapshot(
`"must specify [certificate] and [key] -- or [keystore.path] -- when ssl is enabled"`
);
});

Expand All @@ -98,6 +186,18 @@ describe('throws when config is invalid', () => {
});

describe('reads files', () => {
it('reads certificate authorities when `keystore.path` is specified', () => {
const configValue = createConfig({ keystore: { path: 'some-path' } });
expect(mockReadPkcs12Keystore).toHaveBeenCalledTimes(1);
expect(configValue.certificateAuthorities).toEqual(['content-of-some-path.ca']);
});

it('reads certificate authorities when `truststore.path` is specified', () => {
const configValue = createConfig({ truststore: { path: 'some-path' } });
expect(mockReadPkcs12Truststore).toHaveBeenCalledTimes(1);
expect(configValue.certificateAuthorities).toEqual(['content-of-some-path']);
});

it('reads certificate authorities when `certificateAuthorities` is specified', () => {
let configValue = createConfig({ certificateAuthorities: 'some-path' });
expect(mockReadFileSync).toHaveBeenCalledTimes(1);
Expand All @@ -117,6 +217,29 @@ describe('reads files', () => {
]);
});

it('reads certificate authorities when `keystore.path`, `truststore.path`, and `certificateAuthorities` are specified', () => {
const configValue = createConfig({
keystore: { path: 'some-path' },
truststore: { path: 'another-path' },
certificateAuthorities: 'yet-another-path',
});
expect(mockReadPkcs12Keystore).toHaveBeenCalledTimes(1);
expect(mockReadPkcs12Truststore).toHaveBeenCalledTimes(1);
expect(mockReadFileSync).toHaveBeenCalledTimes(1);
expect(configValue.certificateAuthorities).toEqual([
'content-of-some-path.ca',
'content-of-another-path',
'content-of-yet-another-path',
]);
});

it('reads a private key and certificate when `keystore.path` is specified', () => {
const configValue = createConfig({ keystore: { path: 'some-path' } });
expect(mockReadPkcs12Keystore).toHaveBeenCalledTimes(1);
expect(configValue.key).toEqual('content-of-some-path.key');
expect(configValue.certificate).toEqual('content-of-some-path.cert');
});

it('reads a private key and certificate when `key` and `certificate` are specified', () => {
const configValue = createConfig({ key: 'some-path', certificate: 'another-path' });
expect(mockReadFileSync).toHaveBeenCalledTimes(2);
Expand Down
48 changes: 44 additions & 4 deletions src/core/server/http/ssl_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import { schema, TypeOf } from '@kbn/config-schema';
import crypto from 'crypto';
import { readFileSync } from 'fs';
import { readPkcs12Keystore, readPkcs12Truststore } from '../../utils';

// `crypto` type definitions doesn't currently include `crypto.constants`, see
// https://github.com/DefinitelyTyped/DefinitelyTyped/blob/fa5baf1733f49cf26228a4e509914572c1b74adf/types/node/v6/index.d.ts#L3412
Expand All @@ -45,6 +46,14 @@ export const sslSchema = schema.object(
}),
key: schema.maybe(schema.string()),
keyPassphrase: schema.maybe(schema.string()),
keystore: schema.object({
path: schema.maybe(schema.string()),
password: schema.maybe(schema.string()),
}),
truststore: schema.object({
path: schema.maybe(schema.string()),
password: schema.maybe(schema.string()),
}),
redirectHttpFromPort: schema.maybe(schema.number()),
supportedProtocols: schema.arrayOf(
schema.oneOf([schema.literal('TLSv1'), schema.literal('TLSv1.1'), schema.literal('TLSv1.2')]),
Expand All @@ -57,8 +66,16 @@ export const sslSchema = schema.object(
},
{
validate: ssl => {
if (ssl.enabled && (!ssl.key || !ssl.certificate)) {
return 'must specify [certificate] and [key] when ssl is enabled';
if (ssl.key && ssl.keystore.path) {
return 'cannot use [key] when [keystore.path] is specified';
}

if (ssl.certificate && ssl.keystore.path) {
return 'cannot use [certificate] when [keystore.path] is specified';
}

if (ssl.enabled && (!ssl.key || !ssl.certificate) && !ssl.keystore.path) {
return 'must specify [certificate] and [key] -- or [keystore.path] -- when ssl is enabled';
}

if (!ssl.enabled && ssl.clientAuthentication !== 'none') {
Expand Down Expand Up @@ -94,12 +111,35 @@ export class SslConfig {
this.requestCert = config.clientAuthentication !== 'none';
this.rejectUnauthorized = config.clientAuthentication === 'required';

if (config.key && config.certificate) {
const addCAs = (ca: string[] | undefined) => {
if (ca && ca.length) {
this.certificateAuthorities = this.certificateAuthorities
? this.certificateAuthorities.concat(ca)
: ca;
}
};

if (config.keystore?.path) {
const { key, cert, ca } = readPkcs12Keystore(config.keystore.path, config.keystore.password);
if (!key) {
throw new Error(`Did not find private key in keystore at [keystore.path].`);
} else if (!cert) {
throw new Error(`Did not find certificate in keystore at [keystore.path].`);
}
this.key = key;
this.certificate = cert;
addCAs(ca);
} else if (config.key && config.certificate) {
this.key = readFile(config.key);
this.keyPassphrase = config.keyPassphrase;
this.certificate = readFile(config.certificate);
}

if (config.truststore?.path) {
const ca = readPkcs12Truststore(config.truststore.path, config.truststore.password);
addCAs(ca);
}

const ca = config.certificateAuthorities;
if (ca) {
const parsed: string[] = [];
Expand All @@ -108,7 +148,7 @@ export class SslConfig {
for (const path of paths) {
parsed.push(readFile(path));
}
this.certificateAuthorities = parsed;
addCAs(parsed);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ kibana_vars=(
server.ssl.enabled
server.ssl.key
server.ssl.keyPassphrase
server.ssl.keystore.path
server.ssl.keystore.password
server.ssl.truststore.path
server.ssl.truststore.password
server.ssl.redirectHttpFromPort
server.ssl.supportedProtocols
server.xsrf.whitelist
Expand Down
50 changes: 50 additions & 0 deletions test/server_integration/http/ssl_with_p12/config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { KBN_P12_PATH, KBN_P12_PASSWORD, CA_CERT_PATH } from '@kbn/dev-utils';

export default async function({ readConfigFile }) {
const httpConfig = await readConfigFile(require.resolve('../../config'));

return {
testFiles: [require.resolve('./')],
services: httpConfig.get('services'),
servers: {
...httpConfig.get('servers'),
kibana: {
...httpConfig.get('servers.kibana'),
protocol: 'https',
},
},
junit: {
reportName: 'Http SSL Integration Tests',
},
esTestCluster: httpConfig.get('esTestCluster'),
kbnTestServer: {
...httpConfig.get('kbnTestServer'),
serverArgs: [
...httpConfig.get('kbnTestServer.serverArgs'),
'--server.ssl.enabled=true',
`--server.ssl.certificateAuthorities=${CA_CERT_PATH}`, // this is needed for the test runner to trust the server certificate
`--server.ssl.keystore.path=${KBN_P12_PATH}`,
`--server.ssl.keystore.password=${KBN_P12_PASSWORD}`,
],
},
};
}
28 changes: 28 additions & 0 deletions test/server_integration/http/ssl_with_p12/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

export default function({ getService }) {
const supertest = getService('supertest');

describe('kibana server with ssl', () => {
it('handles requests using ssl', async () => {
await supertest.get('/').expect(302);
});
});
}

0 comments on commit 0593259

Please sign in to comment.