Skip to content

Commit

Permalink
Deprecate using elasticsearch.ssl.certificate without `elasticsearc…
Browse files Browse the repository at this point in the history 
…h.ssl.key` and vice versa (elastic#54392)
  • Loading branch information
@jportner
jportner authored Jan 11, 2020
1 parent 8ef5609 commit 80b6dd8
Show file tree
Hide file tree
Showing 7 changed files with 208 additions and 187 deletions.
31 changes: 0 additions & 31 deletions src/core/server/config/deprecation/core_deprecations.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,35 +208,4 @@ describe('core deprecations', () => {
).toEqual([`worker-src blob:`]);
});
});

describe('elasticsearchUsernameDeprecation', () => {
it('logs a warning if elasticsearch.username is set to "elastic"', () => {
const { messages } = applyCoreDeprecations({
elasticsearch: {
username: 'elastic',
},
});
expect(messages).toMatchInlineSnapshot(`
Array [
"Setting elasticsearch.username to \\"elastic\\" is deprecated. You should use the \\"kibana\\" user instead.",
]
`);
});

it('does not log a warning if elasticsearch.username is set to something besides "elastic"', () => {
const { messages } = applyCoreDeprecations({
elasticsearch: {
username: 'otheruser',
},
});
expect(messages).toHaveLength(0);
});

it('does not log a warning if elasticsearch.username is unset', () => {
const { messages } = applyCoreDeprecations({
elasticsearch: {},
});
expect(messages).toHaveLength(0);
});
});
});
11 changes: 0 additions & 11 deletions src/core/server/config/deprecation/core_deprecations.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,16 +91,6 @@ const cspRulesDeprecation: ConfigDeprecation = (settings, fromPath, log) => {
return settings;
};

const elasticsearchUsernameDeprecation: ConfigDeprecation = (settings, _fromPath, log) => {
const username: string | undefined = get(settings, 'elasticsearch.username');
if (username === 'elastic') {
log(
`Setting elasticsearch.username to "elastic" is deprecated. You should use the "kibana" user instead.`
);
}
return settings;
};

export const coreDeprecationProvider: ConfigDeprecationProvider = ({
unusedFromRoot,
renameFromRoot,
Expand All @@ -120,5 +110,4 @@ export const coreDeprecationProvider: ConfigDeprecationProvider = ({
dataPathDeprecation,
rewriteBasePathDeprecation,
cspRulesDeprecation,
elasticsearchUsernameDeprecation,
];
153 changes: 94 additions & 59 deletions src/core/server/elasticsearch/elasticsearch_config.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,32 @@ import {
mockReadPkcs12Truststore,
} from './elasticsearch_config.test.mocks';

import { ElasticsearchConfig, config, ElasticsearchConfigType } from './elasticsearch_config';
import { loggingServiceMock } from '../mocks';
import { Logger } from '../logging';

const createElasticsearchConfig = (rawConfig: ElasticsearchConfigType, log?: Logger) => {
if (!log) {
log = loggingServiceMock.create().get('config');
}
return new ElasticsearchConfig(rawConfig, log);
import { ElasticsearchConfig, config } from './elasticsearch_config';
import { applyDeprecations, configDeprecationFactory } from '../config/deprecation';

const CONFIG_PATH = 'elasticsearch';

const applyElasticsearchDeprecations = (settings: Record<string, any> = {}) => {
const deprecations = config.deprecations!(configDeprecationFactory);
const deprecationMessages: string[] = [];
const _config: any = {};
_config[CONFIG_PATH] = settings;
const migrated = applyDeprecations(
_config,
deprecations.map(deprecation => ({
deprecation,
path: CONFIG_PATH,
})),
msg => deprecationMessages.push(msg)
);
return {
messages: deprecationMessages,
migrated,
};
};

test('set correct defaults', () => {
const configValue = createElasticsearchConfig(config.schema.validate({}));
const configValue = new ElasticsearchConfig(config.schema.validate({}));
expect(configValue).toMatchInlineSnapshot(`
ElasticsearchConfig {
"apiVersion": "master",
Expand Down Expand Up @@ -70,17 +83,17 @@ test('set correct defaults', () => {
});

test('#hosts accepts both string and array of strings', () => {
let configValue = createElasticsearchConfig(
let configValue = new ElasticsearchConfig(
config.schema.validate({ hosts: 'http://some.host:1234' })
);
expect(configValue.hosts).toEqual(['http://some.host:1234']);

configValue = createElasticsearchConfig(
configValue = new ElasticsearchConfig(
config.schema.validate({ hosts: ['http://some.host:1234'] })
);
expect(configValue.hosts).toEqual(['http://some.host:1234']);

configValue = createElasticsearchConfig(
configValue = new ElasticsearchConfig(
config.schema.validate({
hosts: ['http://some.host:1234', 'https://some.another.host'],
})
Expand All @@ -89,17 +102,17 @@ test('#hosts accepts both string and array of strings', () => {
});

test('#requestHeadersWhitelist accepts both string and array of strings', () => {
let configValue = createElasticsearchConfig(
let configValue = new ElasticsearchConfig(
config.schema.validate({ requestHeadersWhitelist: 'token' })
);
expect(configValue.requestHeadersWhitelist).toEqual(['token']);

configValue = createElasticsearchConfig(
configValue = new ElasticsearchConfig(
config.schema.validate({ requestHeadersWhitelist: ['token'] })
);
expect(configValue.requestHeadersWhitelist).toEqual(['token']);

configValue = createElasticsearchConfig(
configValue = new ElasticsearchConfig(
config.schema.validate({
requestHeadersWhitelist: ['token', 'X-Forwarded-Proto'],
})
Expand All @@ -122,37 +135,37 @@ describe('reads files', () => {
});

it('reads certificate authorities when ssl.keystore.path is specified', () => {
const configValue = createElasticsearchConfig(
const configValue = new ElasticsearchConfig(
config.schema.validate({ ssl: { keystore: { path: 'some-path' } } })
);
expect(mockReadPkcs12Keystore).toHaveBeenCalledTimes(1);
expect(configValue.ssl.certificateAuthorities).toEqual(['content-of-some-path.ca']);
});

it('reads certificate authorities when ssl.truststore.path is specified', () => {
const configValue = createElasticsearchConfig(
const configValue = new ElasticsearchConfig(
config.schema.validate({ ssl: { truststore: { path: 'some-path' } } })
);
expect(mockReadPkcs12Truststore).toHaveBeenCalledTimes(1);
expect(configValue.ssl.certificateAuthorities).toEqual(['content-of-some-path']);
});

it('reads certificate authorities when ssl.certificateAuthorities is specified', () => {
let configValue = createElasticsearchConfig(
let configValue = new ElasticsearchConfig(
config.schema.validate({ ssl: { certificateAuthorities: 'some-path' } })
);
expect(mockReadFileSync).toHaveBeenCalledTimes(1);
expect(configValue.ssl.certificateAuthorities).toEqual(['content-of-some-path']);

mockReadFileSync.mockClear();
configValue = createElasticsearchConfig(
configValue = new ElasticsearchConfig(
config.schema.validate({ ssl: { certificateAuthorities: ['some-path'] } })
);
expect(mockReadFileSync).toHaveBeenCalledTimes(1);
expect(configValue.ssl.certificateAuthorities).toEqual(['content-of-some-path']);

mockReadFileSync.mockClear();
configValue = createElasticsearchConfig(
configValue = new ElasticsearchConfig(
config.schema.validate({
ssl: { certificateAuthorities: ['some-path', 'another-path'] },
})
Expand All @@ -165,7 +178,7 @@ describe('reads files', () => {
});

it('reads certificate authorities when ssl.keystore.path, ssl.truststore.path, and ssl.certificateAuthorities are specified', () => {
const configValue = createElasticsearchConfig(
const configValue = new ElasticsearchConfig(
config.schema.validate({
ssl: {
keystore: { path: 'some-path' },
Expand All @@ -185,7 +198,7 @@ describe('reads files', () => {
});

it('reads a private key and certificate when ssl.keystore.path is specified', () => {
const configValue = createElasticsearchConfig(
const configValue = new ElasticsearchConfig(
config.schema.validate({ ssl: { keystore: { path: 'some-path' } } })
);
expect(mockReadPkcs12Keystore).toHaveBeenCalledTimes(1);
Expand All @@ -194,15 +207,15 @@ describe('reads files', () => {
});

it('reads a private key when ssl.key is specified', () => {
const configValue = createElasticsearchConfig(
const configValue = new ElasticsearchConfig(
config.schema.validate({ ssl: { key: 'some-path' } })
);
expect(mockReadFileSync).toHaveBeenCalledTimes(1);
expect(configValue.ssl.key).toEqual('content-of-some-path');
});

it('reads a certificate when ssl.certificate is specified', () => {
const configValue = createElasticsearchConfig(
const configValue = new ElasticsearchConfig(
config.schema.validate({ ssl: { certificate: 'some-path' } })
);
expect(mockReadFileSync).toHaveBeenCalledTimes(1);
Expand All @@ -225,52 +238,58 @@ describe('throws when config is invalid', () => {

it('throws if key is invalid', () => {
const value = { ssl: { key: '/invalid/key' } };
expect(() =>
createElasticsearchConfig(config.schema.validate(value))
expect(
() => new ElasticsearchConfig(config.schema.validate(value))
).toThrowErrorMatchingInlineSnapshot(
`"ENOENT: no such file or directory, open '/invalid/key'"`
);
});

it('throws if certificate is invalid', () => {
const value = { ssl: { certificate: '/invalid/cert' } };
expect(() =>
createElasticsearchConfig(config.schema.validate(value))
expect(
() => new ElasticsearchConfig(config.schema.validate(value))
).toThrowErrorMatchingInlineSnapshot(
`"ENOENT: no such file or directory, open '/invalid/cert'"`
);
});

it('throws if certificateAuthorities is invalid', () => {
const value = { ssl: { certificateAuthorities: '/invalid/ca' } };
expect(() =>
createElasticsearchConfig(config.schema.validate(value))
expect(
() => new ElasticsearchConfig(config.schema.validate(value))
).toThrowErrorMatchingInlineSnapshot(`"ENOENT: no such file or directory, open '/invalid/ca'"`);
});

it('throws if keystore path is invalid', () => {
const value = { ssl: { keystore: { path: '/invalid/keystore' } } };
expect(() =>
createElasticsearchConfig(config.schema.validate(value))
expect(
() => new ElasticsearchConfig(config.schema.validate(value))
).toThrowErrorMatchingInlineSnapshot(
`"ENOENT: no such file or directory, open '/invalid/keystore'"`
);
});

it('throws if keystore does not contain a key or certificate', () => {
it('throws if keystore does not contain a key', () => {
mockReadPkcs12Keystore.mockReturnValueOnce({});
const value = { ssl: { keystore: { path: 'some-path' } } };
expect(() =>
createElasticsearchConfig(config.schema.validate(value))
).toThrowErrorMatchingInlineSnapshot(
`"Did not find key or certificate in Elasticsearch keystore."`
);
expect(
() => new ElasticsearchConfig(config.schema.validate(value))
).toThrowErrorMatchingInlineSnapshot(`"Did not find key in Elasticsearch keystore."`);
});

it('throws if keystore does not contain a certificate', () => {
mockReadPkcs12Keystore.mockReturnValueOnce({ key: 'foo' });
const value = { ssl: { keystore: { path: 'some-path' } } };
expect(
() => new ElasticsearchConfig(config.schema.validate(value))
).toThrowErrorMatchingInlineSnapshot(`"Did not find certificate in Elasticsearch keystore."`);
});

it('throws if truststore path is invalid', () => {
const value = { ssl: { keystore: { path: '/invalid/truststore' } } };
expect(() =>
createElasticsearchConfig(config.schema.validate(value))
expect(
() => new ElasticsearchConfig(config.schema.validate(value))
).toThrowErrorMatchingInlineSnapshot(
`"ENOENT: no such file or directory, open '/invalid/truststore'"`
);
Expand All @@ -291,31 +310,47 @@ describe('throws when config is invalid', () => {
});
});

describe('logs warnings', () => {
let logger: ReturnType<typeof loggingServiceMock.create>;
let log: Logger;
describe('deprecations', () => {
it('logs a warning if elasticsearch.username is set to "elastic"', () => {
const { messages } = applyElasticsearchDeprecations({ username: 'elastic' });
expect(messages).toMatchInlineSnapshot(`
Array [
"Setting [${CONFIG_PATH}.username] to \\"elastic\\" is deprecated. You should use the \\"kibana\\" user instead.",
]
`);
});

beforeAll(() => {
mockReadFileSync.mockResolvedValue('foo');
it('does not log a warning if elasticsearch.username is set to something besides "elastic"', () => {
const { messages } = applyElasticsearchDeprecations({ username: 'otheruser' });
expect(messages).toHaveLength(0);
});

beforeEach(() => {
logger = loggingServiceMock.create();
log = logger.get('config');
it('does not log a warning if elasticsearch.username is unset', () => {
const { messages } = applyElasticsearchDeprecations({});
expect(messages).toHaveLength(0);
});

it('warns if ssl.key is set and ssl.certificate is not', () => {
createElasticsearchConfig(config.schema.validate({ ssl: { key: 'some-path' } }), log);
expect(loggingServiceMock.collect(logger).warn[0][0]).toMatchInlineSnapshot(
`"Detected a key without a certificate; mutual TLS authentication is disabled."`
);
it('logs a warning if ssl.key is set and ssl.certificate is not', () => {
const { messages } = applyElasticsearchDeprecations({ ssl: { key: '' } });
expect(messages).toMatchInlineSnapshot(`
Array [
"Setting [${CONFIG_PATH}.ssl.key] without [${CONFIG_PATH}.ssl.certificate] is deprecated. This has no effect, you should use both settings to enable TLS client authentication to Elasticsearch.",
]
`);
});

it('warns if ssl.certificate is set and ssl.key is not', () => {
createElasticsearchConfig(config.schema.validate({ ssl: { certificate: 'some-path' } }), log);
expect(loggingServiceMock.collect(logger).warn[0][0]).toMatchInlineSnapshot(
`"Detected a certificate without a key; mutual TLS authentication is disabled."`
);
it('logs a warning if ssl.certificate is set and ssl.key is not', () => {
const { messages } = applyElasticsearchDeprecations({ ssl: { certificate: '' } });
expect(messages).toMatchInlineSnapshot(`
Array [
"Setting [${CONFIG_PATH}.ssl.certificate] without [${CONFIG_PATH}.ssl.key] is deprecated. This has no effect, you should use both settings to enable TLS client authentication to Elasticsearch.",
]
`);
});

it('does not log a warning if both ssl.key and ssl.certificate are set', () => {
const { messages } = applyElasticsearchDeprecations({ ssl: { key: '', certificate: '' } });
expect(messages).toEqual([]);
});
});

Expand Down
Loading

0 comments on commit 80b6dd8

Please sign in to comment.