Skip to content

Commit

Permalink
Additional validation for elasticsearch username (#48247) (#50920)
Browse files Browse the repository at this point in the history
* Additional validation for elasticsearch username

If "elastic" user is set in config:
* In dev mode, throws an error
* In prod mode, logs a deprecation warning

* Fix user for functional tests

* Revert last two commits

Revert "Fix user for functional tests" and
"Fix user for plugin functional tests in Jenkinsfile"

* Change elasticsearch creds for test server

Now uses "kibana" user instead of "elastic" user

* Fix plugin API functional tests

* Fix PKI API integration test

* Change log messages, now conditional on `dist: false` not `dev: true`
  • Loading branch information
jportner committed Nov 18, 2019
1 parent bf75969 commit 10d93a9
Show file tree
Hide file tree
Showing 10 changed files with 54 additions and 17 deletions.

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

8 changes: 8 additions & 0 deletions src/core/server/elasticsearch/elasticsearch_config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,3 +107,11 @@ test('#ssl.certificateAuthorities accepts both string and array of strings', ()
);
expect(configValue.ssl.certificateAuthorities).toEqual(['some-path', 'another-path']);
});

test('#username throws if equal to "elastic", only while running from source', () => {
const obj = {
username: 'elastic',
};
expect(() => config.schema.validate(obj, { dist: false })).toThrowErrorMatchingSnapshot();
expect(() => config.schema.validate(obj, { dist: true })).not.toThrow();
});
30 changes: 28 additions & 2 deletions src/core/server/elasticsearch/elasticsearch_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import { schema, TypeOf } from '@kbn/config-schema';
import { Duration } from 'moment';
import { Logger } from '../logging';

const hostURISchema = schema.uri({ scheme: ['http', 'https'] });

Expand All @@ -39,7 +40,23 @@ export const config = {
defaultValue: 'http://localhost:9200',
}),
preserveHost: schema.boolean({ defaultValue: true }),
username: schema.maybe(schema.string()),
username: schema.maybe(
schema.conditional(
schema.contextRef('dist'),
false,
schema.string({
validate: rawConfig => {
if (rawConfig === 'elastic') {
return (
'value of "elastic" is forbidden. This is a superuser account that can obfuscate ' +
'privilege-related issues. You should use the "kibana" user instead.'
);
}
},
}),
schema.string()
)
),
password: schema.maybe(schema.string()),
requestHeadersWhitelist: schema.oneOf([schema.string(), schema.arrayOf(schema.string())], {
defaultValue: ['authorization'],
Expand Down Expand Up @@ -166,7 +183,7 @@ export class ElasticsearchConfig {
*/
public readonly customHeaders: ElasticsearchConfigType['customHeaders'];

constructor(rawConfig: ElasticsearchConfigType) {
constructor(rawConfig: ElasticsearchConfigType, log?: Logger) {
this.ignoreVersionMismatch = rawConfig.ignoreVersionMismatch;
this.apiVersion = rawConfig.apiVersion;
this.logQueries = rawConfig.logQueries;
Expand Down Expand Up @@ -195,5 +212,14 @@ export class ElasticsearchConfig {
...rawConfig.ssl,
certificateAuthorities,
};

if (this.username === 'elastic' && log !== undefined) {
// logger is optional / not used during tests
// TODO: logger can be removed when issue #40255 is resolved to support deprecations in NP config service
log.warn(
`Setting the elasticsearch username to "elastic" is deprecated. You should use the "kibana" user instead.`,
{ tags: ['deprecation'] }
);
}
}
}
2 changes: 1 addition & 1 deletion src/core/server/elasticsearch/elasticsearch_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export class ElasticsearchService implements CoreService<InternalElasticsearchSe
this.log = coreContext.logger.get('elasticsearch-service');
this.config$ = coreContext.configService
.atPath<ElasticsearchConfigType>('elasticsearch')
.pipe(map(rawConfig => new ElasticsearchConfig(rawConfig)));
.pipe(map(rawConfig => new ElasticsearchConfig(rawConfig, coreContext.logger.get('config'))));
}

public async setup(deps: SetupDeps): Promise<InternalElasticsearchServiceSetup> {
Expand Down
8 changes: 4 additions & 4 deletions src/test_utils/kbn_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,17 +267,17 @@ export function createTestServers({
// Override provided configs, we know what the elastic user is now
kbnSettings.elasticsearch = {
hosts: [esTestConfig.getUrl()],
username: esTestConfig.getUrlParts().username,
password: esTestConfig.getUrlParts().password,
username: kibanaServerTestUser.username,
password: kibanaServerTestUser.password,
};
}

return {
stop: async () => await es.cleanup(),
es,
hosts: [esTestConfig.getUrl()],
username: esTestConfig.getUrlParts().username,
password: esTestConfig.getUrlParts().password,
username: kibanaServerTestUser.username,
password: kibanaServerTestUser.password,
};
},
startKibana: async () => {
Expand Down
6 changes: 3 additions & 3 deletions test/common/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import path from 'path';
import { format as formatUrl } from 'url';
import { OPTIMIZE_BUNDLE_DIR, esTestConfig, kbnTestConfig } from '@kbn/test';
import { OPTIMIZE_BUNDLE_DIR, esTestConfig, kbnTestConfig, kibanaServerTestUser } from '@kbn/test';
import { services } from './services';

export default function () {
Expand Down Expand Up @@ -53,8 +53,8 @@ export default function () {
'--status.allowAnonymous=true',
'--optimize.enabled=true',
`--elasticsearch.hosts=${formatUrl(servers.elasticsearch)}`,
`--elasticsearch.username=${servers.elasticsearch.username}`,
`--elasticsearch.password=${servers.elasticsearch.password}`,
`--elasticsearch.username=${kibanaServerTestUser.username}`,
`--elasticsearch.password=${kibanaServerTestUser.password}`,
`--kibana.disableWelcomeScreen=true`,
'--telemetry.banner=false',
`--server.maxPayloadBytes=1679958`,
Expand Down
4 changes: 2 additions & 2 deletions x-pack/test/pki_api_integration/apis/security/pki_auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ export default function({ getService }: FtrProviderContext) {
enabled: true,
metadata: {
pki_delegated_by_realm: 'reserved',
pki_delegated_by_user: 'elastic',
pki_delegated_by_user: 'kibana',
pki_dn: 'CN=first_client',
},
authentication_realm: { name: 'pki1', type: 'pki' },
Expand Down Expand Up @@ -155,7 +155,7 @@ export default function({ getService }: FtrProviderContext) {
enabled: true,
metadata: {
pki_delegated_by_realm: 'reserved',
pki_delegated_by_user: 'elastic',
pki_delegated_by_user: 'kibana',
pki_dn: 'CN=second_client',
},
authentication_realm: { name: 'pki1', type: 'pki' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export default function TaskTestingAPI(kibana) {

const callCluster = server.plugins.elasticsearch.getCluster('admin').callWithInternalUser;
await callCluster('index', {
index: '.task_manager_test_result',
index: '.kibana_task_manager_test_result',
body: {
type: 'task',
taskId: taskInstance.id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export default function ({ getService }) {
const log = getService('log');
const retry = getService('retry');
const config = getService('config');
const testHistoryIndex = '.task_manager_test_result';
const testHistoryIndex = '.kibana_task_manager_test_result';
const supertest = supertestAsPromised(url.format(config.get('servers.kibana')));

describe('scheduling and running tasks', () => {
Expand Down
6 changes: 3 additions & 3 deletions x-pack/test/reporting/configs/generate_api.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { esTestConfig, kbnTestConfig } from '@kbn/test';
import { esTestConfig, kbnTestConfig, kibanaServerTestUser } from '@kbn/test';
import { format as formatUrl } from 'url';
import { getApiIntegrationConfig } from '../../api_integration/config';
import { getReportingApiConfig } from './api';
Expand Down Expand Up @@ -35,8 +35,8 @@ export default async function ({ readConfigFile }) {
`--server.maxPayloadBytes=1679958`,
`--server.port=${kbnTestConfig.getPort()}`,
`--elasticsearch.hosts=${formatUrl(servers.elasticsearch)}`,
`--elasticsearch.password=${servers.elasticsearch.password}`,
`--elasticsearch.username=${servers.elasticsearch.username}`,
`--elasticsearch.username=${kibanaServerTestUser.username}`,
`--elasticsearch.password=${kibanaServerTestUser.password}`,
`--xpack.reporting.csv.enablePanelActionDownload=true`,
`--xpack.reporting.csv.maxSizeBytes=2850`,
`--xpack.reporting.queue.pollInterval=3000`,
Expand Down

0 comments on commit 10d93a9

Please sign in to comment.