From ba8de9315a696a2bf476cde4a77b1c33494c7b04 Mon Sep 17 00:00:00 2001 From: Nicolas Chaulet Date: Thu, 10 Dec 2020 13:50:54 -0500 Subject: [PATCH] [7.10] [Fleet] Enforce superuser role for all fleet APIs (#85136) (#85577) --- .../plugins/ingest_manager/server/plugin.ts | 28 +-- .../server/routes/agent/index.ts | 180 +++++++++--------- .../ingest_manager/server/routes/index.ts | 2 +- .../server/routes/install_script/index.ts | 2 +- .../ingest_manager/server/routes/security.ts | 43 +++++ .../apis/fleet/agents/acks.ts | 23 +-- .../apis/fleet/agents/delete.ts | 20 +- .../apis/fleet/agents/list.ts | 28 +-- 8 files changed, 190 insertions(+), 136 deletions(-) create mode 100644 x-pack/plugins/ingest_manager/server/routes/security.ts diff --git a/x-pack/plugins/ingest_manager/server/plugin.ts b/x-pack/plugins/ingest_manager/server/plugin.ts index e3757b46ed715..14f65a0e80668 100644 --- a/x-pack/plugins/ingest_manager/server/plugin.ts +++ b/x-pack/plugins/ingest_manager/server/plugin.ts @@ -42,7 +42,8 @@ import { registerDataStreamRoutes, registerAgentPolicyRoutes, registerSetupRoutes, - registerAgentRoutes, + registerAgentAPIRoutes, + registerElasticAgentRoutes, registerEnrollmentApiKeyRoutes, registerInstallScriptRoutes, registerOutputRoutes, @@ -69,6 +70,7 @@ import { CloudSetup } from '../../cloud/server'; import { agentCheckinState } from './services/agents/checkin/state'; import { registerIngestManagerUsageCollector } from './collectors/register'; import { getInstallation } from './services/epm/packages'; +import { makeRouterEnforcingSuperuser } from './routes/security'; export interface IngestManagerSetupDeps { licensing: LicensingPluginSetup; @@ -219,16 +221,18 @@ export class IngestManagerPlugin // Always register app routes for permissions checking registerAppRoutes(router); + // For all the routes we enforce the user to have role superuser + const routerSuperuserOnly = makeRouterEnforcingSuperuser(router); // Register rest of routes only if security is enabled if (this.security) { - registerSetupRoutes(router, config); - registerAgentPolicyRoutes(router); - registerPackagePolicyRoutes(router); - registerOutputRoutes(router); - registerSettingsRoutes(router); - registerDataStreamRoutes(router); - registerEPMRoutes(router); + registerSetupRoutes(routerSuperuserOnly, config); + registerAgentPolicyRoutes(routerSuperuserOnly); + registerPackagePolicyRoutes(routerSuperuserOnly); + registerOutputRoutes(routerSuperuserOnly); + registerSettingsRoutes(routerSuperuserOnly); + registerDataStreamRoutes(routerSuperuserOnly); + registerEPMRoutes(routerSuperuserOnly); // Conditional config routes if (config.agents.enabled) { @@ -244,12 +248,14 @@ export class IngestManagerPlugin // we currently only use this global interceptor if fleet is enabled // since it would run this func on *every* req (other plugins, CSS, etc) registerLimitedConcurrencyRoutes(core, config); - registerAgentRoutes(router, config); - registerEnrollmentApiKeyRoutes(router); + registerAgentAPIRoutes(routerSuperuserOnly, config); + registerEnrollmentApiKeyRoutes(routerSuperuserOnly); registerInstallScriptRoutes({ - router, + router: routerSuperuserOnly, basePath: core.http.basePath, }); + // Do not enforce superuser role for Elastic Agent routes + registerElasticAgentRoutes(router, config); } } } diff --git a/x-pack/plugins/ingest_manager/server/routes/agent/index.ts b/x-pack/plugins/ingest_manager/server/routes/agent/index.ts index 2f97a6bcde42c..e99efb1ea056e 100644 --- a/x-pack/plugins/ingest_manager/server/routes/agent/index.ts +++ b/x-pack/plugins/ingest_manager/server/routes/agent/index.ts @@ -81,7 +81,7 @@ function makeValidator(jsonSchema: any) { }; } -export const registerRoutes = (router: IRouter, config: IngestManagerConfigType) => { +export const registerAPIRoutes = (router: IRouter, config: IngestManagerConfigType) => { // Get one router.get( { @@ -119,6 +119,96 @@ export const registerRoutes = (router: IRouter, config: IngestManagerConfigType) getAgentsHandler ); + // Agent actions + router.post( + { + path: AGENT_API_ROUTES.ACTIONS_PATTERN, + validate: PostNewAgentActionRequestSchema, + options: { tags: [`access:${PLUGIN_ID}-all`] }, + }, + postNewAgentActionHandlerBuilder({ + getAgent: AgentService.getAgent, + createAgentAction: AgentService.createAgentAction, + }) + ); + + router.post( + { + path: AGENT_API_ROUTES.UNENROLL_PATTERN, + validate: PostAgentUnenrollRequestSchema, + options: { tags: [`access:${PLUGIN_ID}-all`] }, + }, + postAgentUnenrollHandler + ); + + router.put( + { + path: AGENT_API_ROUTES.REASSIGN_PATTERN, + validate: PutAgentReassignRequestSchema, + options: { tags: [`access:${PLUGIN_ID}-all`] }, + }, + putAgentsReassignHandler + ); + + // Get agent events + router.get( + { + path: AGENT_API_ROUTES.EVENTS_PATTERN, + validate: GetOneAgentEventsRequestSchema, + options: { tags: [`access:${PLUGIN_ID}-read`] }, + }, + getAgentEventsHandler + ); + + // Get agent status for policy + router.get( + { + path: AGENT_API_ROUTES.STATUS_PATTERN, + validate: GetAgentStatusRequestSchema, + options: { tags: [`access:${PLUGIN_ID}-read`] }, + }, + getAgentStatusForAgentPolicyHandler + ); + // upgrade agent + router.post( + { + path: AGENT_API_ROUTES.UPGRADE_PATTERN, + validate: PostAgentUpgradeRequestSchema, + options: { tags: [`access:${PLUGIN_ID}-all`] }, + }, + postAgentUpgradeHandler + ); + // bulk upgrade + router.post( + { + path: AGENT_API_ROUTES.BULK_UPGRADE_PATTERN, + validate: PostBulkAgentUpgradeRequestSchema, + options: { tags: [`access:${PLUGIN_ID}-all`] }, + }, + postBulkAgentsUpgradeHandler + ); + // Bulk reassign + router.post( + { + path: AGENT_API_ROUTES.BULK_REASSIGN_PATTERN, + validate: PostBulkAgentReassignRequestSchema, + options: { tags: [`access:${PLUGIN_ID}-all`] }, + }, + postBulkAgentsReassignHandler + ); + + // Bulk unenroll + router.post( + { + path: AGENT_API_ROUTES.BULK_UNENROLL_PATTERN, + validate: PostBulkAgentUnenrollRequestSchema, + options: { tags: [`access:${PLUGIN_ID}-all`] }, + }, + postBulkAgentsUnenrollHandler + ); +}; + +export const registerElasticAgentRoutes = (router: IRouter, config: IngestManagerConfigType) => { const pollingRequestTimeout = config.agents.pollingRequestTimeout; // Agent checkin router.post( @@ -226,92 +316,4 @@ export const registerRoutes = (router: IRouter, config: IngestManagerConfigType) saveAgentEvents: AgentService.saveAgentEvents, }) ); - - // Agent actions - router.post( - { - path: AGENT_API_ROUTES.ACTIONS_PATTERN, - validate: PostNewAgentActionRequestSchema, - options: { tags: [`access:${PLUGIN_ID}-all`] }, - }, - postNewAgentActionHandlerBuilder({ - getAgent: AgentService.getAgent, - createAgentAction: AgentService.createAgentAction, - }) - ); - - router.post( - { - path: AGENT_API_ROUTES.UNENROLL_PATTERN, - validate: PostAgentUnenrollRequestSchema, - options: { tags: [`access:${PLUGIN_ID}-all`] }, - }, - postAgentUnenrollHandler - ); - - router.put( - { - path: AGENT_API_ROUTES.REASSIGN_PATTERN, - validate: PutAgentReassignRequestSchema, - options: { tags: [`access:${PLUGIN_ID}-all`] }, - }, - putAgentsReassignHandler - ); - - // Get agent events - router.get( - { - path: AGENT_API_ROUTES.EVENTS_PATTERN, - validate: GetOneAgentEventsRequestSchema, - options: { tags: [`access:${PLUGIN_ID}-read`] }, - }, - getAgentEventsHandler - ); - - // Get agent status for policy - router.get( - { - path: AGENT_API_ROUTES.STATUS_PATTERN, - validate: GetAgentStatusRequestSchema, - options: { tags: [`access:${PLUGIN_ID}-read`] }, - }, - getAgentStatusForAgentPolicyHandler - ); - // upgrade agent - router.post( - { - path: AGENT_API_ROUTES.UPGRADE_PATTERN, - validate: PostAgentUpgradeRequestSchema, - options: { tags: [`access:${PLUGIN_ID}-all`] }, - }, - postAgentUpgradeHandler - ); - // bulk upgrade - router.post( - { - path: AGENT_API_ROUTES.BULK_UPGRADE_PATTERN, - validate: PostBulkAgentUpgradeRequestSchema, - options: { tags: [`access:${PLUGIN_ID}-all`] }, - }, - postBulkAgentsUpgradeHandler - ); - // Bulk reassign - router.post( - { - path: AGENT_API_ROUTES.BULK_REASSIGN_PATTERN, - validate: PostBulkAgentReassignRequestSchema, - options: { tags: [`access:${PLUGIN_ID}-all`] }, - }, - postBulkAgentsReassignHandler - ); - - // Bulk unenroll - router.post( - { - path: AGENT_API_ROUTES.BULK_UNENROLL_PATTERN, - validate: PostBulkAgentUnenrollRequestSchema, - options: { tags: [`access:${PLUGIN_ID}-all`] }, - }, - postBulkAgentsUnenrollHandler - ); }; diff --git a/x-pack/plugins/ingest_manager/server/routes/index.ts b/x-pack/plugins/ingest_manager/server/routes/index.ts index 0743270fd7121..19e4d10e04578 100644 --- a/x-pack/plugins/ingest_manager/server/routes/index.ts +++ b/x-pack/plugins/ingest_manager/server/routes/index.ts @@ -8,7 +8,7 @@ export { registerRoutes as registerPackagePolicyRoutes } from './package_policy' export { registerRoutes as registerDataStreamRoutes } from './data_streams'; export { registerRoutes as registerEPMRoutes } from './epm'; export { registerRoutes as registerSetupRoutes } from './setup'; -export { registerRoutes as registerAgentRoutes } from './agent'; +export { registerAPIRoutes as registerAgentAPIRoutes, registerElasticAgentRoutes } from './agent'; export { registerRoutes as registerEnrollmentApiKeyRoutes } from './enrollment_api_key'; export { registerRoutes as registerInstallScriptRoutes } from './install_script'; export { registerRoutes as registerOutputRoutes } from './output'; diff --git a/x-pack/plugins/ingest_manager/server/routes/install_script/index.ts b/x-pack/plugins/ingest_manager/server/routes/install_script/index.ts index c767d3e80d2b7..8cb9478776b5d 100644 --- a/x-pack/plugins/ingest_manager/server/routes/install_script/index.ts +++ b/x-pack/plugins/ingest_manager/server/routes/install_script/index.ts @@ -27,7 +27,7 @@ export const registerRoutes = ({ { path: INSTALL_SCRIPT_API_ROUTES, validate: InstallScriptRequestSchema, - options: { tags: [], authRequired: false }, + options: { tags: [], authRequired: true }, }, async function getInstallScriptHandler( context, diff --git a/x-pack/plugins/ingest_manager/server/routes/security.ts b/x-pack/plugins/ingest_manager/server/routes/security.ts new file mode 100644 index 0000000000000..c2348c313e583 --- /dev/null +++ b/x-pack/plugins/ingest_manager/server/routes/security.ts @@ -0,0 +1,43 @@ +/* + * 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 { IRouter, RequestHandler } from 'src/core/server'; +import { appContextService } from '../services'; + +export function enforceSuperUser( + handler: RequestHandler +): RequestHandler { + return function enforceSuperHandler(context, req, res) { + const security = appContextService.getSecurity(); + const user = security.authc.getCurrentUser(req); + if (!user) { + return res.unauthorized(); + } + + const userRoles = user.roles || []; + if (!userRoles.includes('superuser')) { + return res.forbidden({ + body: { + message: 'Access to Fleet API require the superuser role.', + }, + }); + } + return handler(context, req, res); + }; +} + +export function makeRouterEnforcingSuperuser(router: IRouter): IRouter { + return { + get: (options, handler) => router.get(options, enforceSuperUser(handler)), + delete: (options, handler) => router.delete(options, enforceSuperUser(handler)), + post: (options, handler) => router.post(options, enforceSuperUser(handler)), + put: (options, handler) => router.put(options, enforceSuperUser(handler)), + patch: (options, handler) => router.patch(options, enforceSuperUser(handler)), + handleLegacyErrors: (handler) => router.handleLegacyErrors(handler), + getRoutes: () => router.getRoutes(), + routerPath: router.routerPath, + }; +} diff --git a/x-pack/test/ingest_manager_api_integration/apis/fleet/agents/acks.ts b/x-pack/test/ingest_manager_api_integration/apis/fleet/agents/acks.ts index 119efa92d2327..3948089c124ec 100644 --- a/x-pack/test/ingest_manager_api_integration/apis/fleet/agents/acks.ts +++ b/x-pack/test/ingest_manager_api_integration/apis/fleet/agents/acks.ts @@ -15,7 +15,8 @@ export default function (providerContext: FtrProviderContext) { const esClient = getService('es'); const kibanaServer = getService('kibanaServer'); - const supertest = getSupertestWithoutAuth(providerContext); + const supertest = getService('supertest'); + const supertestWithoutAuth = getSupertestWithoutAuth(providerContext); let apiKey: { id: string; api_key: string }; describe('fleet_agents_acks', () => { @@ -49,7 +50,7 @@ export default function (providerContext: FtrProviderContext) { }); it('should return a 401 if this a not a valid acks access', async () => { - await supertest + await supertestWithoutAuth .post(`/api/fleet/agents/agent1/acks`) .set('kbn-xsrf', 'xx') .set('Authorization', 'ApiKey NOT_A_VALID_TOKEN') @@ -60,7 +61,7 @@ export default function (providerContext: FtrProviderContext) { }); it('should return a 200 if this a valid acks request', async () => { - const { body: apiResponse } = await supertest + const { body: apiResponse } = await supertestWithoutAuth .post(`/api/fleet/agents/agent1/acks`) .set('kbn-xsrf', 'xx') .set( @@ -94,10 +95,6 @@ export default function (providerContext: FtrProviderContext) { const { body: eventResponse } = await supertest .get(`/api/fleet/agents/agent1/events`) .set('kbn-xsrf', 'xx') - .set( - 'Authorization', - `ApiKey ${Buffer.from(`${apiKey.id}:${apiKey.api_key}`).toString('base64')}` - ) .expect(200); const expectedEvents = eventResponse.list.filter( (item: Record) => @@ -120,7 +117,7 @@ export default function (providerContext: FtrProviderContext) { }); it('should return a 400 when request event list contains event for another agent id', async () => { - const { body: apiResponse } = await supertest + const { body: apiResponse } = await supertestWithoutAuth .post(`/api/fleet/agents/agent1/acks`) .set('kbn-xsrf', 'xx') .set( @@ -147,7 +144,7 @@ export default function (providerContext: FtrProviderContext) { }); it('should return a 400 when request event list contains action that does not belong to agent current actions', async () => { - const { body: apiResponse } = await supertest + const { body: apiResponse } = await supertestWithoutAuth .post(`/api/fleet/agents/agent1/acks`) .set('kbn-xsrf', 'xx') .set( @@ -181,7 +178,7 @@ export default function (providerContext: FtrProviderContext) { }); it('should return a 400 when request event list contains action types that are not allowed for acknowledgement', async () => { - const { body: apiResponse } = await supertest + const { body: apiResponse } = await supertestWithoutAuth .post(`/api/fleet/agents/agent1/acks`) .set('kbn-xsrf', 'xx') .set( @@ -211,10 +208,6 @@ export default function (providerContext: FtrProviderContext) { const { body: actionRes } = await supertest .post(`/api/fleet/agents/agent1/actions`) .set('kbn-xsrf', 'xx') - .set( - 'Authorization', - `ApiKey ${Buffer.from(`${apiKey.id}:${apiKey.api_key}`).toString('base64')}` - ) .send({ action: { type: 'UPGRADE', @@ -223,7 +216,7 @@ export default function (providerContext: FtrProviderContext) { }) .expect(200); const actionId = actionRes.item.id; - await supertest + await supertestWithoutAuth .post(`/api/fleet/agents/agent1/acks`) .set('kbn-xsrf', 'xx') .set( diff --git a/x-pack/test/ingest_manager_api_integration/apis/fleet/agents/delete.ts b/x-pack/test/ingest_manager_api_integration/apis/fleet/agents/delete.ts index c3b0a3086e35e..a482702f09790 100644 --- a/x-pack/test/ingest_manager_api_integration/apis/fleet/agents/delete.ts +++ b/x-pack/test/ingest_manager_api_integration/apis/fleet/agents/delete.ts @@ -9,6 +9,7 @@ import { FtrProviderContext } from '../../../../api_integration/ftr_provider_con export default function ({ getService }: FtrProviderContext) { const esArchiver = getService('esArchiver'); + const supertestAsSuperuser = getService('supertest'); const supertest = getService('supertestWithoutAuth'); const security = getService('security'); const users: { [rollName: string]: { username: string; password: string; permissions?: any } } = { @@ -60,7 +61,7 @@ export default function ({ getService }: FtrProviderContext) { await esArchiver.unload('fleet/agents'); }); - it('should return a 403 if user lacks fleet-write permissions', async () => { + it('should return a 403 if user lacks fleet-write permissions: only fleet read', async () => { const { body: apiResponse } = await supertest .delete(`/api/fleet/agents/agent1`) .auth(users.fleet_user.username, users.fleet_user.password) @@ -71,19 +72,28 @@ export default function ({ getService }: FtrProviderContext) { action: 'deleted', }); }); + it('should return a 403 if user lacks fleet-write permissions: fleet all no superuser', async () => { + const { body: apiResponse } = await supertest + .delete(`/api/fleet/agents/agent1`) + .auth(users.fleet_admin.username, users.fleet_admin.password) + .set('kbn-xsrf', 'xx') + .expect(403); + + expect(apiResponse).not.to.eql({ + action: 'deleted', + }); + }); it('should return a 404 if there is no agent to delete', async () => { - await supertest + await supertestAsSuperuser .delete(`/api/fleet/agents/i-do-not-exist`) - .auth(users.fleet_admin.username, users.fleet_admin.password) .set('kbn-xsrf', 'xx') .expect(404); }); it('should return a 200 after deleting an agent', async () => { - const { body: apiResponse } = await supertest + const { body: apiResponse } = await supertestAsSuperuser .delete(`/api/fleet/agents/agent1`) - .auth(users.fleet_admin.username, users.fleet_admin.password) .set('kbn-xsrf', 'xx') .expect(200); expect(apiResponse).to.eql({ diff --git a/x-pack/test/ingest_manager_api_integration/apis/fleet/agents/list.ts b/x-pack/test/ingest_manager_api_integration/apis/fleet/agents/list.ts index e4e7b4b6be54e..661c2e3f2d420 100644 --- a/x-pack/test/ingest_manager_api_integration/apis/fleet/agents/list.ts +++ b/x-pack/test/ingest_manager_api_integration/apis/fleet/agents/list.ts @@ -10,7 +10,8 @@ import { FtrProviderContext } from '../../../../api_integration/ftr_provider_con export default function ({ getService }: FtrProviderContext) { const esArchiver = getService('esArchiver'); - const supertest = getService('supertestWithoutAuth'); + const supertest = getService('supertest'); + const supertestWithoutAuth = getService('supertestWithoutAuth'); const security = getService('security'); const users: { [rollName: string]: { username: string; password: string; permissions?: any } } = { kibana_basic_user: { @@ -72,25 +73,26 @@ export default function ({ getService }: FtrProviderContext) { await esArchiver.unload('fleet/agents'); }); - it('should return the list of agents when requesting as a user with fleet write permissions', async () => { - const { body: apiResponse } = await supertest - .get(`/api/fleet/agents`) - .auth(users.fleet_admin.username, users.fleet_admin.password) - .expect(200); + it('should return the list of agents when requesting as a user superser', async () => { + const { body: apiResponse } = await supertest.get(`/api/fleet/agents`).expect(200); expect(apiResponse).to.have.keys('page', 'total', 'list'); expect(apiResponse.total).to.eql(4); }); - it('should return the list of agents when requesting as a user with fleet read permissions', async () => { - const { body: apiResponse } = await supertest + it('should not return the list of agents when requesting as a user with fleet read permissions', async () => { + await supertestWithoutAuth .get(`/api/fleet/agents`) .auth(users.fleet_user.username, users.fleet_user.password) - .expect(200); - expect(apiResponse).to.have.keys('page', 'total', 'list'); - expect(apiResponse.total).to.eql(4); + .expect(403); + }); + it('should not return the list of agents when requesting as a user with fleet all permissions', async () => { + await supertestWithoutAuth + .get(`/api/fleet/agents`) + .auth(users.fleet_admin.username, users.fleet_admin.password) + .expect(403); }); it('should not return the list of agents when requesting as a user without fleet permissions', async () => { - await supertest + await supertestWithoutAuth .get(`/api/fleet/agents`) .auth(users.kibana_basic_user.username, users.kibana_basic_user.password) .expect(403); @@ -98,14 +100,12 @@ export default function ({ getService }: FtrProviderContext) { it('should return a 400 when given an invalid "kuery" value', async () => { await supertest .get(`/api/fleet/agents?kuery=m`) // missing saved object type - .auth(users.fleet_user.username, users.fleet_user.password) .expect(400); }); it('should accept a valid "kuery" value', async () => { const filter = encodeURIComponent('fleet-agents.shared_id : "agent2_filebeat"'); const { body: apiResponse } = await supertest .get(`/api/fleet/agents?kuery=${filter}`) - .auth(users.fleet_user.username, users.fleet_user.password) .expect(200); expect(apiResponse.total).to.eql(1);