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

[Beats Management] APIs: Update beat #19148

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
5f367e7
WIP checkin
ycombinator May 11, 2018
60cfd96
Add API integration test
ycombinator May 12, 2018
d40042a
Converting to Jest test
ycombinator May 12, 2018
ed3e29d
WIP checkin
ycombinator May 11, 2018
19d893d
Fixing API for default case + adding test for it
ycombinator May 12, 2018
ccc117b
Fixing copy pasta typos
ycombinator May 14, 2018
f73bf3b
Fixing variable name
ycombinator May 14, 2018
ff35c6f
Using a single index
ycombinator May 14, 2018
be53b05
Implementing GET /api/beats/agents API
ycombinator May 15, 2018
965a4e3
Updating mapping
ycombinator May 16, 2018
481f96f
Creating POST /api/beats/agents/verify API
ycombinator May 16, 2018
71a4d83
Refactoring: extracting out helper functions
ycombinator May 16, 2018
12d908c
Expanding TODO note so I won't forget :)
ycombinator May 16, 2018
ab2a0ec
Fixing file name
ycombinator May 16, 2018
d14642c
Add API tests
ycombinator May 16, 2018
35d44d1
Update template to allow version field for beat
ycombinator May 16, 2018
e09f587
Implement PUT /api/beats/agent/{beat ID} API
ycombinator May 16, 2018
63747a3
Make enroll beat code consistent with update beat code
ycombinator May 16, 2018
6940d3d
Fixing minor typo in TODO comment
ycombinator May 16, 2018
b565aec
Allow version in request payload
ycombinator May 16, 2018
dcbeec6
Make sure beat is not updated in ES in error scenarios
ycombinator May 16, 2018
08a46b6
Adding version as required field in Enroll Beat API payload
ycombinator May 18, 2018
1b54984
Using destructuring
ycombinator May 18, 2018
add6da3
Fixing rename that was accidentally reversed in conflict fixing
ycombinator May 18, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@
"type": {
"type": "keyword"
},
"version": {
"type": "keyword"
},
"host_ip": {
"type": "ip"
},
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/beats/server/routes/api/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ import { registerCreateEnrollmentTokensRoute } from './register_create_enrollmen
import { registerEnrollBeatRoute } from './register_enroll_beat_route';
import { registerListBeatsRoute } from './register_list_beats_route';
import { registerVerifyBeatsRoute } from './register_verify_beats_route';
import { registerUpdateBeatRoute } from './register_update_beat_route';

export function registerApiRoutes(server) {
registerCreateEnrollmentTokensRoute(server);
registerEnrollBeatRoute(server);
registerListBeatsRoute(server);
registerVerifyBeatsRoute(server);
registerUpdateBeatRoute(server);
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,16 @@ function deleteUsedEnrollmentToken(callWithInternalUser, enrollmentToken) {
return callWithInternalUser('delete', params);
}

function persistBeat(callWithInternalUser, beat, beatId, accessToken, remoteAddress) {
function persistBeat(callWithInternalUser, beat) {
const body = {
type: 'beat',
beat: {
...omit(beat, 'enrollment_token'),
id: beatId,
access_token: accessToken,
host_ip: remoteAddress
}
beat
};

const params = {
index: INDEX_NAMES.BEATS,
type: '_doc',
id: `beat:${beatId}`,
id: `beat:${beat.id}`,
body,
refresh: 'wait_for'
};
Expand All @@ -69,13 +64,15 @@ export function registerEnrollBeatRoute(server) {
payload: Joi.object({
enrollment_token: Joi.string().required(),
type: Joi.string().required(),
version: Joi.string().required(),
host_name: Joi.string().required()
}).required()
},
auth: false
},
handler: async (request, reply) => {
const callWithInternalUser = callWithInternalUserFactory(server);
const { beatId } = request.params;
let accessToken;

try {
Expand All @@ -90,7 +87,12 @@ export function registerEnrollBeatRoute(server) {

accessToken = uuid.v4().replace(/-/g, "");
const remoteAddress = request.info.remoteAddress;
await persistBeat(callWithInternalUser, request.payload, request.params.beatId, accessToken, remoteAddress);
await persistBeat(callWithInternalUser, {
...omit(request.payload, 'enrollment_token'),
id: beatId,
access_token: accessToken,
host_ip: remoteAddress
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@exekias In this PR I'm implementing the "Update Beat" API (PUT /api/beats/agent/{beat UUID}). That API can take any of the following Beat information in its request body:

  • type: filebeat, metricbeat, etc.
  • version: beat version
  • host_name
  • ephemeral_id
  • local_configuration_yml
  • metadata: arbitrary extra metadata, if we want any

Which of these above properties do you expect to send at enrollment time as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

The ones from this list we should be sending for sure are type & hostname. I'm divided on version, but probably it makes sense to include it during enrolment too (also here, as it may change)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, thanks. I'll add version as a required field to the payload for the enrollment API (it already takes type and host_name as required fields).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in 5e63721.


await deleteUsedEnrollmentToken(callWithInternalUser, enrollmentToken);
} catch (err) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/*
* 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 } from 'lodash';
import { INDEX_NAMES } from '../../../common/constants';
import { callWithInternalUserFactory } from '../../lib/client';
import { wrapEsError } from '../../lib/error_wrappers';

async function getBeat(callWithInternalUser, beatId) {
const params = {
index: INDEX_NAMES.BEATS,
type: '_doc',
id: `beat:${beatId}`,
ignore: [ 404 ]
};

const response = await callWithInternalUser('get', params);
if (!response.found) {
return null;
}

return get(response, '_source.beat');
}

function persistBeat(callWithInternalUser, beat) {
const body = {
type: 'beat',
beat
};

const params = {
index: INDEX_NAMES.BEATS,
type: '_doc',
id: `beat:${beat.id}`,
body,
refresh: 'wait_for'
};
return callWithInternalUser('index', params);
}

// TODO: add license check pre-hook
// TODO: write to Kibana audit log file (include who did the verification as well)
export function registerUpdateBeatRoute(server) {
server.route({
method: 'PUT',
path: '/api/beats/agent/{beatId}',
config: {
validate: {
payload: Joi.object({
access_token: Joi.string().required(),
type: Joi.string(),
version: Joi.string(),
host_name: Joi.string(),
ephemeral_id: Joi.string(),
local_configuration_yml: Joi.string(),
metadata: Joi.object()
}).required()
},
auth: false
},
handler: async (request, reply) => {
const callWithInternalUser = callWithInternalUserFactory(server);
const { beatId } = request.params;

try {
const beat = await getBeat(callWithInternalUser, beatId);
if (beat === null) {
return reply({ message: 'Beat not found' }).code(404);
}

const isAccessTokenValid = beat.access_token === request.payload.access_token;
Copy link
Contributor

Choose a reason for hiding this comment

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

This comparison is susceptible to timing attacks: https://codahale.com/a-lesson-in-timing-attacks/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I just found https://nodejs.org/api/crypto.html#crypto_crypto_timingsafeequal_a_b and will use that here (and other similar places) instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in #19363.

if (!isAccessTokenValid) {
return reply({ message: 'Invalid access token' }).code(401);
}

const isBeatVerified = beat.hasOwnProperty('verified_on');
if (!isBeatVerified) {
return reply({ message: 'Beat has not been verified' }).code(400);
}

const remoteAddress = request.info.remoteAddress;
await persistBeat(callWithInternalUser, {
...beat,
...request.payload,
host_ip: remoteAddress
});
} catch (err) {
return reply(wrapEsError(err));
}

reply().code(204);
}
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ function findVerifiedBeatIds(verifications, toBeVerifiedBeatIds) {
}

// TODO: add license check pre-hook
// TODO: write to Kibana audit log file (include who did the verification as well)
// TODO: write to Kibana audit log file
export function registerVerifyBeatsRoute(server) {
server.route({
method: 'POST',
Expand Down
7 changes: 7 additions & 0 deletions x-pack/test/api_integration/apis/beats/enroll_beat.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,17 @@ export default function ({ getService }) {
beforeEach(async () => {
validEnrollmentToken = chance.word();
beatId = chance.word();
const version = chance.integer({ min: 1, max: 10 })
+ '.'
+ chance.integer({ min: 1, max: 10 })
+ '.'
+ chance.integer({ min: 1, max: 10 });

beat = {
enrollment_token: validEnrollmentToken,
type: 'filebeat',
host_name: 'foo.bar.com',
version
};

await es.index({
Expand Down
1 change: 1 addition & 0 deletions x-pack/test/api_integration/apis/beats/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,6 @@ export default function ({ getService, loadTestFile }) {
loadTestFile(require.resolve('./enroll_beat'));
loadTestFile(require.resolve('./list_beats'));
loadTestFile(require.resolve('./verify_beats'));
loadTestFile(require.resolve('./update_beat'));
});
}
130 changes: 130 additions & 0 deletions x-pack/test/api_integration/apis/beats/update_beat.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
/*
* 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 chance = getService('chance');
const es = getService('es');
const esArchiver = getService('esArchiver');

describe('update_beat', () => {
let beat;
const archive = 'beats/list';

beforeEach('load beats archive', () => esArchiver.load(archive));
beforeEach(() => {
const version = chance.integer({ min: 1, max: 10 })
+ '.'
+ chance.integer({ min: 1, max: 10 })
+ '.'
+ chance.integer({ min: 1, max: 10 });

beat = {
access_token: '93c4a4dd08564c189a7ec4e4f046b975',
type: `${chance.word()}beat`,
host_name: `www.${chance.word()}.net`,
version,
ephemeral_id: chance.word()
};
});

afterEach('unload beats archive', () => esArchiver.unload(archive));

it('should update an existing verified beat', async () => {
const beatId = 'foo';
await supertest
.put(
`/api/beats/agent/${beatId}`
)
.set('kbn-xsrf', 'xxx')
.send(beat)
.expect(204);

const beatInEs = await es.get({
index: ES_INDEX_NAME,
type: ES_TYPE_NAME,
id: `beat:${beatId}`
});

expect(beatInEs._source.beat.id).to.be(beatId);
expect(beatInEs._source.beat.type).to.be(beat.type);
expect(beatInEs._source.beat.host_name).to.be(beat.host_name);
expect(beatInEs._source.beat.version).to.be(beat.version);
expect(beatInEs._source.beat.ephemeral_id).to.be(beat.ephemeral_id);
});

it('should return an error for an invalid access token', async () => {
const beatId = 'foo';
beat.access_token = chance.word();
const { body } = await supertest
.put(
`/api/beats/agent/${beatId}`
)
.set('kbn-xsrf', 'xxx')
.send(beat)
.expect(401);

expect(body.message).to.be('Invalid access token');

const beatInEs = await es.get({
index: ES_INDEX_NAME,
type: ES_TYPE_NAME,
id: `beat:${beatId}`
});

expect(beatInEs._source.beat.id).to.be(beatId);
expect(beatInEs._source.beat.type).to.not.be(beat.type);
expect(beatInEs._source.beat.host_name).to.not.be(beat.host_name);
expect(beatInEs._source.beat.version).to.not.be(beat.version);
expect(beatInEs._source.beat.ephemeral_id).to.not.be(beat.ephemeral_id);
});

it('should return an error for an existing but unverified beat', async () => {
const beatId = 'bar';
beat.access_token = '3c4a4dd08564c189a7ec4e4f046b9759';
const { body } = await supertest
.put(
`/api/beats/agent/${beatId}`
)
.set('kbn-xsrf', 'xxx')
.send(beat)
.expect(400);

expect(body.message).to.be('Beat has not been verified');

const beatInEs = await es.get({
index: ES_INDEX_NAME,
type: ES_TYPE_NAME,
id: `beat:${beatId}`
});

expect(beatInEs._source.beat.id).to.be(beatId);
expect(beatInEs._source.beat.type).to.not.be(beat.type);
expect(beatInEs._source.beat.host_name).to.not.be(beat.host_name);
expect(beatInEs._source.beat.version).to.not.be(beat.version);
expect(beatInEs._source.beat.ephemeral_id).to.not.be(beat.ephemeral_id);
});

it('should return an error for a non-existent beat', async () => {
const beatId = chance.word();
const { body } = await supertest
.put(
`/api/beats/agent/${beatId}`
)
.set('kbn-xsrf', 'xxx')
.send(beat)
.expect(404);

expect(body.message).to.be('Beat not found');
});
});
}
3 changes: 3 additions & 0 deletions x-pack/test/functional/es_archives/beats/list/mappings.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@
"type": {
"type": "keyword"
},
"version": {
"type": "keyword"
},
"host_ip": {
"type": "ip"
},
Expand Down