From 1da3ec44034441e73f02d7f4bebc5cf381371c25 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Thu, 31 May 2018 16:01:27 -0700 Subject: [PATCH] [Beats Management] APIs: Remove tag(s) from beat(s) (#19440) * Using crypto.timingSafeEqual() for comparing auth tokens * Introduce random delay after we try to find token in ES to mitigate timing attack * Remove random delay * Starting to implement POST /api/beats/beats_tags API * Changing API * Updating tests for changes to API * Renaming * Use destructuring * Using crypto.timingSafeEqual() for comparing auth tokens * Introduce random delay after we try to find token in ES to mitigate timing attack * Implementing `POST /api/beats/agents_tags/removals` API * Updating ES archive * Use destructuring * Moving start of script to own line to increase readability * Nothing to remove if there are no existing tags! * Updating tests to match changes in bulk update painless script * Use destructuring --- .../plugins/beats/server/routes/api/index.js | 2 + .../routes/api/register_enroll_beat_route.js | 9 +- .../register_remove_tags_from_beats_route.js | 166 ++++++++++++ .../apis/beats/assign_tags_to_beats.js | 8 +- .../test/api_integration/apis/beats/index.js | 1 + .../apis/beats/remove_tags_from_beats.js | 246 ++++++++++++++++++ .../es_archives/beats/list/data.json.gz | Bin 436 -> 447 bytes 7 files changed, 427 insertions(+), 5 deletions(-) create mode 100644 x-pack/plugins/beats/server/routes/api/register_remove_tags_from_beats_route.js create mode 100644 x-pack/test/api_integration/apis/beats/remove_tags_from_beats.js diff --git a/x-pack/plugins/beats/server/routes/api/index.js b/x-pack/plugins/beats/server/routes/api/index.js index aa1be44cd96ca..6ec0ad737352a 100644 --- a/x-pack/plugins/beats/server/routes/api/index.js +++ b/x-pack/plugins/beats/server/routes/api/index.js @@ -11,6 +11,7 @@ import { registerVerifyBeatsRoute } from './register_verify_beats_route'; import { registerUpdateBeatRoute } from './register_update_beat_route'; import { registerSetTagRoute } from './register_set_tag_route'; import { registerAssignTagsToBeatsRoute } from './register_assign_tags_to_beats_route'; +import { registerRemoveTagsFromBeatsRoute } from './register_remove_tags_from_beats_route'; export function registerApiRoutes(server) { registerCreateEnrollmentTokensRoute(server); @@ -20,4 +21,5 @@ export function registerApiRoutes(server) { registerUpdateBeatRoute(server); registerSetTagRoute(server); registerAssignTagsToBeatsRoute(server); + registerRemoveTagsFromBeatsRoute(server); } diff --git a/x-pack/plugins/beats/server/routes/api/register_enroll_beat_route.js b/x-pack/plugins/beats/server/routes/api/register_enroll_beat_route.js index 77742c16cd401..bad28c0ab9be5 100644 --- a/x-pack/plugins/beats/server/routes/api/register_enroll_beat_route.js +++ b/x-pack/plugins/beats/server/routes/api/register_enroll_beat_route.js @@ -24,7 +24,14 @@ async function getEnrollmentToken(callWithInternalUser, enrollmentToken) { }; const response = await callWithInternalUser('get', params); - return get(response, '_source.enrollment_token', {}); + const token = get(response, '_source.enrollment_token', {}); + + // Elasticsearch might return fast if the token is not found. OR it might return fast + // if the token *is* found. Either way, an attacker could using a timing attack to figure + // out whether a token is valid or not. So we introduce a random delay in returning from + // this function to obscure the actual time it took for Elasticsearch to find the token. + const randomDelayInMs = 25 + Math.round(Math.random() * 200); // between 25 and 225 ms + return new Promise(resolve => setTimeout(() => resolve(token), randomDelayInMs)); } function deleteUsedEnrollmentToken(callWithInternalUser, enrollmentToken) { diff --git a/x-pack/plugins/beats/server/routes/api/register_remove_tags_from_beats_route.js b/x-pack/plugins/beats/server/routes/api/register_remove_tags_from_beats_route.js new file mode 100644 index 0000000000000..b5e66267b2ea4 --- /dev/null +++ b/x-pack/plugins/beats/server/routes/api/register_remove_tags_from_beats_route.js @@ -0,0 +1,166 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import Joi from 'joi'; +import { + get, + flatten, + uniq +} from 'lodash'; +import { INDEX_NAMES } from '../../../common/constants'; +import { callWithRequestFactory } from '../../lib/client'; +import { wrapEsError } from '../../lib/error_wrappers'; + +async function getDocs(callWithRequest, ids) { + const params = { + index: INDEX_NAMES.BEATS, + type: '_doc', + body: { ids }, + _source: false + }; + + const response = await callWithRequest('mget', params); + return get(response, 'docs', []); +} + +function getBeats(callWithRequest, beatIds) { + const ids = beatIds.map(beatId => `beat:${beatId}`); + return getDocs(callWithRequest, ids); +} + +function getTags(callWithRequest, tags) { + const ids = tags.map(tag => `tag:${tag}`); + return getDocs(callWithRequest, ids); +} + +async function findNonExistentItems(callWithRequest, items, getFn) { + const itemsFromEs = await getFn.call(null, callWithRequest, items); + return itemsFromEs.reduce((nonExistentItems, itemFromEs, idx) => { + if (!itemFromEs.found) { + nonExistentItems.push(items[idx]); + } + return nonExistentItems; + }, []); +} + +function findNonExistentBeatIds(callWithRequest, beatIds) { + return findNonExistentItems(callWithRequest, beatIds, getBeats); +} + +function findNonExistentTags(callWithRequest, tags) { + return findNonExistentItems(callWithRequest, tags, getTags); +} + +async function persistRemovals(callWithRequest, removals) { + const body = flatten(removals.map(({ beatId, tag }) => { + const script = '' + + 'def beat = ctx._source.beat; ' + + 'if (beat.tags != null) { ' + + ' beat.tags.removeAll([params.tag]); ' + + '}'; + + return [ + { update: { _id: `beat:${beatId}` } }, + { script: { source: script, params: { tag } } } + ]; + })); + + const params = { + index: INDEX_NAMES.BEATS, + type: '_doc', + body, + refresh: 'wait_for' + }; + + const response = await callWithRequest('bulk', params); + return get(response, 'items', []) + .map((item, resultIdx) => ({ + status: item.update.status, + result: item.update.result, + idxInRequest: removals[resultIdx].idxInRequest + })); +} + +function addNonExistentItemRemovalsToResponse(response, removals, nonExistentBeatIds, nonExistentTags) { + removals.forEach(({ beat_id: beatId, tag }, idx) => { + const isBeatNonExistent = nonExistentBeatIds.includes(beatId); + const isTagNonExistent = nonExistentTags.includes(tag); + + if (isBeatNonExistent && isTagNonExistent) { + response.removals[idx].status = 404; + response.removals[idx].result = `Beat ${beatId} and tag ${tag} not found`; + } else if (isBeatNonExistent) { + response.removals[idx].status = 404; + response.removals[idx].result = `Beat ${beatId} not found`; + } else if (isTagNonExistent) { + response.removals[idx].status = 404; + response.removals[idx].result = `Tag ${tag} not found`; + } + }); +} + +function addRemovalResultsToResponse(response, removalResults) { + removalResults.forEach(removalResult => { + const { idxInRequest, status, result } = removalResult; + response.removals[idxInRequest].status = status; + response.removals[idxInRequest].result = result; + }); +} + +// TODO: add license check pre-hook +// TODO: write to Kibana audit log file +export function registerRemoveTagsFromBeatsRoute(server) { + server.route({ + method: 'POST', + path: '/api/beats/agents_tags/removals', + config: { + validate: { + payload: Joi.object({ + removals: Joi.array().items(Joi.object({ + beat_id: Joi.string().required(), + tag: Joi.string().required() + })) + }).required() + } + }, + handler: async (request, reply) => { + const callWithRequest = callWithRequestFactory(server, request); + + const { removals } = request.payload; + const beatIds = uniq(removals.map(removal => removal.beat_id)); + const tags = uniq(removals.map(removal => removal.tag)); + + const response = { + removals: removals.map(() => ({ status: null })) + }; + + try { + // Handle removals containing non-existing beat IDs or tags + const nonExistentBeatIds = await findNonExistentBeatIds(callWithRequest, beatIds); + const nonExistentTags = await findNonExistentTags(callWithRequest, tags); + + addNonExistentItemRemovalsToResponse(response, removals, nonExistentBeatIds, nonExistentTags); + + const validRemovals = removals + .map((removal, idxInRequest) => ({ + beatId: removal.beat_id, + tag: removal.tag, + idxInRequest // so we can add the result of this removal to the correct place in the response + })) + .filter((removal, idx) => response.removals[idx].status === null); + + if (validRemovals.length > 0) { + const removalResults = await persistRemovals(callWithRequest, validRemovals); + addRemovalResultsToResponse(response, removalResults); + } + } catch (err) { + return reply(wrapEsError(err)); + } + + reply(response); + } + }); +} diff --git a/x-pack/test/api_integration/apis/beats/assign_tags_to_beats.js b/x-pack/test/api_integration/apis/beats/assign_tags_to_beats.js index a8f542239d22e..88b7b7c3feb3a 100644 --- a/x-pack/test/api_integration/apis/beats/assign_tags_to_beats.js +++ b/x-pack/test/api_integration/apis/beats/assign_tags_to_beats.js @@ -63,7 +63,7 @@ export default function ({ getService }) { }); beat = esResponse._source.beat; - expect(beat.tags).to.eql(tags); + expect(beat.tags).to.eql([...tags, 'qa']); // Adding the existing tag const { body: apiResponse } = await supertest @@ -90,7 +90,7 @@ export default function ({ getService }) { }); beat = esResponse._source.beat; - expect(beat.tags).to.eql(tags); + expect(beat.tags).to.eql([...tags, 'qa']); }); it('should add a single tag to a multiple beats', async () => { @@ -123,7 +123,7 @@ export default function ({ getService }) { }); beat = esResponse._source.beat; - expect(beat.tags).to.eql(['production', 'development']); // as beat 'foo' already had 'production' tag attached to it + expect(beat.tags).to.eql(['production', 'qa', 'development']); // as beat 'foo' already had 'production' and 'qa' tags attached to it // Beat bar esResponse = await es.get({ @@ -195,7 +195,7 @@ export default function ({ getService }) { }); beat = esResponse._source.beat; - expect(beat.tags).to.eql(['production', 'development']); // as beat 'foo' already had 'production' tag attached to it + expect(beat.tags).to.eql(['production', 'qa', 'development']); // as beat 'foo' already had 'production' and 'qa' tags attached to it // Beat bar esResponse = await es.get({ diff --git a/x-pack/test/api_integration/apis/beats/index.js b/x-pack/test/api_integration/apis/beats/index.js index 76f712c05de44..f8956d3e498ba 100644 --- a/x-pack/test/api_integration/apis/beats/index.js +++ b/x-pack/test/api_integration/apis/beats/index.js @@ -24,5 +24,6 @@ export default function ({ getService, loadTestFile }) { loadTestFile(require.resolve('./update_beat')); loadTestFile(require.resolve('./set_tag')); loadTestFile(require.resolve('./assign_tags_to_beats')); + loadTestFile(require.resolve('./remove_tags_from_beats')); }); } diff --git a/x-pack/test/api_integration/apis/beats/remove_tags_from_beats.js b/x-pack/test/api_integration/apis/beats/remove_tags_from_beats.js new file mode 100644 index 0000000000000..19583f9279732 --- /dev/null +++ b/x-pack/test/api_integration/apis/beats/remove_tags_from_beats.js @@ -0,0 +1,246 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import expect from 'expect.js'; +import { + ES_INDEX_NAME, + ES_TYPE_NAME +} from './constants'; + +export default function ({ getService }) { + const supertest = getService('supertest'); + const esArchiver = getService('esArchiver'); + const es = getService('es'); + const chance = getService('chance'); + + describe('remove_tags_from_beats', () => { + const archive = 'beats/list'; + + beforeEach('load beats archive', () => esArchiver.load(archive)); + afterEach('unload beats archive', () => esArchiver.unload(archive)); + + it('should remove a single tag from a single beat', async () => { + const { body: apiResponse } = await supertest + .post( + '/api/beats/agents_tags/removals' + ) + .set('kbn-xsrf', 'xxx') + .send({ + removals: [ + { beat_id: 'foo', tag: 'production' } + ] + }) + .expect(200); + + expect(apiResponse.removals).to.eql([ + { status: 200, result: 'updated' } + ]); + + const esResponse = await es.get({ + index: ES_INDEX_NAME, + type: ES_TYPE_NAME, + id: `beat:foo` + }); + + const beat = esResponse._source.beat; + expect(beat.tags).to.eql(['qa']); + }); + + it('should remove a single tag from a multiple beats', async () => { + const { body: apiResponse } = await supertest + .post( + '/api/beats/agents_tags/removals' + ) + .set('kbn-xsrf', 'xxx') + .send({ + removals: [ + { beat_id: 'foo', tag: 'development' }, + { beat_id: 'bar', tag: 'development' } + ] + }) + .expect(200); + + expect(apiResponse.removals).to.eql([ + { status: 200, result: 'updated' }, + { status: 200, result: 'updated' } + ]); + + let esResponse; + let beat; + + // Beat foo + esResponse = await es.get({ + index: ES_INDEX_NAME, + type: ES_TYPE_NAME, + id: `beat:foo` + }); + + beat = esResponse._source.beat; + expect(beat.tags).to.eql(['production', 'qa' ]); // as beat 'foo' already had 'production' and 'qa' tags attached to it + + // Beat bar + esResponse = await es.get({ + index: ES_INDEX_NAME, + type: ES_TYPE_NAME, + id: `beat:bar` + }); + + beat = esResponse._source.beat; + expect(beat).to.not.have.property('tags'); + }); + + it('should remove multiple tags from a single beat', async () => { + const { body: apiResponse } = await supertest + .post( + '/api/beats/agents_tags/removals' + ) + .set('kbn-xsrf', 'xxx') + .send({ + removals: [ + { beat_id: 'foo', tag: 'development' }, + { beat_id: 'foo', tag: 'production' } + ] + }) + .expect(200); + + expect(apiResponse.removals).to.eql([ + { status: 200, result: 'updated' }, + { status: 200, result: 'updated' } + ]); + + const esResponse = await es.get({ + index: ES_INDEX_NAME, + type: ES_TYPE_NAME, + id: `beat:foo` + }); + + const beat = esResponse._source.beat; + expect(beat.tags).to.eql(['qa']); // as beat 'foo' already had 'production' and 'qa' tags attached to it + }); + + it('should remove multiple tags from a multiple beats', async () => { + const { body: apiResponse } = await supertest + .post( + '/api/beats/agents_tags/removals' + ) + .set('kbn-xsrf', 'xxx') + .send({ + removals: [ + { beat_id: 'foo', tag: 'production' }, + { beat_id: 'bar', tag: 'development' } + ] + }) + .expect(200); + + expect(apiResponse.removals).to.eql([ + { status: 200, result: 'updated' }, + { status: 200, result: 'updated' } + ]); + + let esResponse; + let beat; + + // Beat foo + esResponse = await es.get({ + index: ES_INDEX_NAME, + type: ES_TYPE_NAME, + id: `beat:foo` + }); + + beat = esResponse._source.beat; + expect(beat.tags).to.eql(['qa']); // as beat 'foo' already had 'production' and 'qa' tags attached to it + + // Beat bar + esResponse = await es.get({ + index: ES_INDEX_NAME, + type: ES_TYPE_NAME, + id: `beat:bar` + }); + + beat = esResponse._source.beat; + expect(beat).to.not.have.property('tags'); + }); + + it('should return errors for non-existent beats', async () => { + const nonExistentBeatId = chance.word(); + + const { body: apiResponse } = await supertest + .post( + '/api/beats/agents_tags/removals' + ) + .set('kbn-xsrf', 'xxx') + .send({ + removals: [ + { beat_id: nonExistentBeatId, tag: 'production' } + ] + }) + .expect(200); + + expect(apiResponse.removals).to.eql([ + { status: 404, result: `Beat ${nonExistentBeatId} not found` } + ]); + }); + + it('should return errors for non-existent tags', async () => { + const nonExistentTag = chance.word(); + + const { body: apiResponse } = await supertest + .post( + '/api/beats/agents_tags/removals' + ) + .set('kbn-xsrf', 'xxx') + .send({ + removals: [ + { beat_id: 'bar', tag: nonExistentTag } + ] + }) + .expect(200); + + expect(apiResponse.removals).to.eql([ + { status: 404, result: `Tag ${nonExistentTag} not found` } + ]); + + const esResponse = await es.get({ + index: ES_INDEX_NAME, + type: ES_TYPE_NAME, + id: `beat:bar` + }); + + const beat = esResponse._source.beat; + expect(beat).to.not.have.property('tags'); + }); + + it('should return errors for non-existent beats and tags', async () => { + const nonExistentBeatId = chance.word(); + const nonExistentTag = chance.word(); + + const { body: apiResponse } = await supertest + .post( + '/api/beats/agents_tags/removals' + ) + .set('kbn-xsrf', 'xxx') + .send({ + removals: [ + { beat_id: nonExistentBeatId, tag: nonExistentTag } + ] + }) + .expect(200); + + expect(apiResponse.removals).to.eql([ + { status: 404, result: `Beat ${nonExistentBeatId} and tag ${nonExistentTag} not found` } + ]); + + const esResponse = await es.get({ + index: ES_INDEX_NAME, + type: ES_TYPE_NAME, + id: `beat:bar` + }); + + const beat = esResponse._source.beat; + expect(beat).to.not.have.property('tags'); + }); + }); +} diff --git a/x-pack/test/functional/es_archives/beats/list/data.json.gz b/x-pack/test/functional/es_archives/beats/list/data.json.gz index 48094292337780ce8c10a685ddda5f9446160c5c..b33ecf434c104716138ed08200cf49feaa9b73c7 100644 GIT binary patch literal 447 zcmV;w0YLsAiwFqO9SBQOZ*BnHRLgGTFbur=D-53-!Ez);@^|#qVqs7c zoi!R;sUJahgZ%qSP6E5wCOveK0DCb&a)zLW93P@MPWoS4O!7Ff&LmGEv4hPJG6x^{ zuxc#s1Ax@fz#408`h`a5yAeL?P+VFBmJOKz%io9nCEK~7HB;{yHz3cb_#92B8Lq50 z_yOx{KV8=s)i#tV$;gthzp4$?C%SV)LraXS=a|#9)1YG#jKQuediRD+C@T;~DZsR} zlIc`PN!2Q)R48L5FrA#K$LR7sM#m^R+(l#!zyF=cHTG>~ZpfT@m6wE4!mdm0C%P{6 zH14NTQz$e5lm^hVCf!w=Q}b_4A8f8V1bTRC2)#l?qm9 z7_u^DLaP;@b9dNQWrenIBQ;B@T%>$K`7;%H#`C2lDq}BmN)PMKV_)NB+d4a#zVh5B z?=wi^ACEOld%r)DU*PDL8&(NByke8~*8aX&pL&!{{NnTZ%DQOZ*Bm^RLgGTFbur=D-53-!Ez);@^|d1MPX19 zoi!R;sUJbQLH>OuCxKmTlODQAE(S=>5Y!CmLlnnJ|FOj+j}z-m@)Qy~*bE_a@PQAj z#^OEzNDU3FvBsufXoS8S;j<3KrA1)bkO{E-eb`^Jof}#+^`3D9@{Eel(S(}e%4&n3 zu)g-&b$wB7Lz$9{ED8Ik+CY7xJ4ZCM#JGBnIZZnaIwrvw?7E_NZ`g#g0%4Q_OiL@7 zPKA_Itx`&bGFAf9$(eeLF5hExjH1I`MAq=<|A|~<-&W>^%$ZktNhl@ky3~H6>rzkS zeknDTGUFUI7b!5tq+Vr$3&T+<11QReO6_;(j#B?XbU$|vy{q3$`_RXq9V_DzLZ2|?0HV)S!C2LJ#d`O}C1