From 598706c7d1d171bf7012e91d1389ade9734e8b35 Mon Sep 17 00:00:00 2001 From: Elena Shostak <165678770+elena-shostak@users.noreply.github.com> Date: Wed, 23 Oct 2024 09:39:49 +0200 Subject: [PATCH] [Authz] Superuser privileges (#196586) ## Summary This PR adds support for explicit indication whether endpoint is restricted to superusers only. Moved `api/encrypted_saved_objects/_rotate_key` endpoint to the new configuration. __Relates: https://github.com/elastic/kibana/issues/196271__ ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ## Release note Introduced explicit configuration for routes that require superuser access. Moved `api/encrypted_saved_objects/_rotate_key` endpoint to the new configuration. --------- Co-authored-by: Elastic Machine --- .../security_route_config_validator.test.ts | 28 ++++ .../src/security_route_config_validator.ts | 10 ++ packages/core/http/core-http-server/index.ts | 1 + .../http/core-http-server/src/router/index.ts | 2 +- .../http/core-http-server/src/router/route.ts | 8 + src/core/server/index.ts | 6 +- .../server/routes/key_rotation.test.ts | 11 +- .../server/routes/key_rotation.ts | 8 +- x-pack/plugins/security/common/constants.ts | 11 ++ .../authorization/api_authorization.test.ts | 156 +++++++++++++----- .../server/authorization/api_authorization.ts | 66 ++++++-- 11 files changed, 244 insertions(+), 63 deletions(-) diff --git a/packages/core/http/core-http-router-server-internal/src/security_route_config_validator.test.ts b/packages/core/http/core-http-router-server-internal/src/security_route_config_validator.test.ts index d130bfdce9fb5..f10d2cb3b3ac4 100644 --- a/packages/core/http/core-http-router-server-internal/src/security_route_config_validator.test.ts +++ b/packages/core/http/core-http-router-server-internal/src/security_route_config_validator.test.ts @@ -8,6 +8,7 @@ */ import { validRouteSecurity } from './security_route_config_validator'; +import { ReservedPrivilegesSet } from '@kbn/core-http-server'; describe('RouteSecurity validation', () => { it('should pass validation for valid route security with authz enabled and valid required privileges', () => { @@ -276,4 +277,31 @@ describe('RouteSecurity validation', () => { `"[authz.requiredPrivileges]: anyRequired privileges must contain unique values"` ); }); + + it('should fail validation when anyRequired has superuser privileges set', () => { + const invalidRouteSecurity = { + authz: { + requiredPrivileges: [ + { anyRequired: ['privilege1', 'privilege1'], allRequired: ['privilege4'] }, + { anyRequired: ['privilege5', ReservedPrivilegesSet.superuser] }, + ], + }, + }; + + expect(() => validRouteSecurity(invalidRouteSecurity)).toThrowErrorMatchingInlineSnapshot( + `"[authz.requiredPrivileges]: Combining superuser with other privileges is redundant, superuser privileges set can be only used as a standalone privilege."` + ); + }); + + it('should fail validation when allRequired combines superuser privileges set with other privileges', () => { + const invalidRouteSecurity = { + authz: { + requiredPrivileges: [ReservedPrivilegesSet.superuser, 'privilege1'], + }, + }; + + expect(() => validRouteSecurity(invalidRouteSecurity)).toThrowErrorMatchingInlineSnapshot( + `"[authz.requiredPrivileges]: Combining superuser with other privileges is redundant, superuser privileges set can be only used as a standalone privilege."` + ); + }); }); diff --git a/packages/core/http/core-http-router-server-internal/src/security_route_config_validator.ts b/packages/core/http/core-http-router-server-internal/src/security_route_config_validator.ts index d74f41d3157b4..65073f9a66ec6 100644 --- a/packages/core/http/core-http-router-server-internal/src/security_route_config_validator.ts +++ b/packages/core/http/core-http-router-server-internal/src/security_route_config_validator.ts @@ -9,6 +9,7 @@ import { schema } from '@kbn/config-schema'; import type { RouteSecurity, RouteConfigOptions } from '@kbn/core-http-server'; +import { ReservedPrivilegesSet } from '@kbn/core-http-server'; import type { DeepPartial } from '@kbn/utility-types'; const privilegeSetSchema = schema.object( @@ -49,6 +50,15 @@ const requiredPrivilegesSchema = schema.arrayOf( } }); + // Combining superuser with other privileges is redundant. + // If user is a superuser, they inherently have access to all the privileges that may come with other roles. + if ( + anyRequired.includes(ReservedPrivilegesSet.superuser) || + (allRequired.includes(ReservedPrivilegesSet.superuser) && allRequired.length > 1) + ) { + return 'Combining superuser with other privileges is redundant, superuser privileges set can be only used as a standalone privilege.'; + } + if (anyRequired.length && allRequired.length) { for (const privilege of anyRequired) { if (allRequired.includes(privilege)) { diff --git a/packages/core/http/core-http-server/index.ts b/packages/core/http/core-http-server/index.ts index 7fe125c6aa9a7..9c12f6a09ac45 100644 --- a/packages/core/http/core-http-server/index.ts +++ b/packages/core/http/core-http-server/index.ts @@ -128,6 +128,7 @@ export { getResponseValidation, isFullValidatorContainer, isKibanaResponse, + ReservedPrivilegesSet, } from './src/router'; export type { ICspConfig } from './src/csp'; diff --git a/packages/core/http/core-http-server/src/router/index.ts b/packages/core/http/core-http-server/src/router/index.ts index 8384ed838d5fc..8e2b9373c43bd 100644 --- a/packages/core/http/core-http-server/src/router/index.ts +++ b/packages/core/http/core-http-server/src/router/index.ts @@ -66,7 +66,7 @@ export type { PrivilegeSet, } from './route'; -export { validBodyOutput } from './route'; +export { validBodyOutput, ReservedPrivilegesSet } from './route'; export type { RouteValidationFunction, RouteValidationResultFactory, diff --git a/packages/core/http/core-http-server/src/router/route.ts b/packages/core/http/core-http-server/src/router/route.ts index 6696babce41e1..17fecd1c48b17 100644 --- a/packages/core/http/core-http-server/src/router/route.ts +++ b/packages/core/http/core-http-server/src/router/route.ts @@ -224,6 +224,14 @@ export interface RouteSecurity { authc?: RouteAuthc; } +/** + * A set of reserved privileges that can be used to check access to the route. + */ +export enum ReservedPrivilegesSet { + operator = 'operator', + superuser = 'superuser', +} + /** * Additional route options. * @public diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 1ac38b1d44157..52149cd611be3 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -242,7 +242,11 @@ export type { } from '@kbn/core-http-server'; export type { IExternalUrlPolicy } from '@kbn/core-http-common'; -export { validBodyOutput, OnPostAuthResultType } from '@kbn/core-http-server'; +export { + validBodyOutput, + OnPostAuthResultType, + ReservedPrivilegesSet, +} from '@kbn/core-http-server'; export type { HttpResourcesRenderOptions, diff --git a/x-pack/plugins/encrypted_saved_objects/server/routes/key_rotation.test.ts b/x-pack/plugins/encrypted_saved_objects/server/routes/key_rotation.test.ts index f387e94e80990..b0bdc9b98a86d 100644 --- a/x-pack/plugins/encrypted_saved_objects/server/routes/key_rotation.test.ts +++ b/x-pack/plugins/encrypted_saved_objects/server/routes/key_rotation.test.ts @@ -7,7 +7,7 @@ import { Type } from '@kbn/config-schema'; import type { IRouter, RequestHandler, RequestHandlerContext, RouteConfig } from '@kbn/core/server'; -import { kibanaResponseFactory } from '@kbn/core/server'; +import { kibanaResponseFactory, ReservedPrivilegesSet } from '@kbn/core/server'; import { httpServerMock } from '@kbn/core/server/mocks'; import { routeDefinitionParamsMock } from './index.mock'; @@ -43,9 +43,14 @@ describe('Key rotation routes', () => { }); it('correctly defines route.', () => { + expect(routeConfig.security).toEqual({ + authz: { + requiredPrivileges: [ReservedPrivilegesSet.superuser], + }, + }); expect(routeConfig.options).toEqual({ access: 'public', - tags: ['access:rotateEncryptionKey', 'oas-tag:saved objects'], + tags: ['oas-tag:saved objects'], summary: `Rotate a key for encrypted saved objects`, description: `If a saved object cannot be decrypted using the primary encryption key, Kibana attempts to decrypt it using the specified decryption-only keys. In most of the cases this overhead is negligible, but if you're dealing with a large number of saved objects and experiencing performance issues, you may want to rotate the encryption key. NOTE: Bulk key rotation can consume a considerable amount of resources and hence only user with a superuser role can trigger it.`, @@ -96,7 +101,7 @@ describe('Key rotation routes', () => { expect(config.options).toEqual({ access: 'internal', - tags: ['access:rotateEncryptionKey', 'oas-tag:saved objects'], + tags: ['oas-tag:saved objects'], summary: `Rotate a key for encrypted saved objects`, description: `If a saved object cannot be decrypted using the primary encryption key, Kibana attempts to decrypt it using the specified decryption-only keys. In most of the cases this overhead is negligible, but if you're dealing with a large number of saved objects and experiencing performance issues, you may want to rotate the encryption key. NOTE: Bulk key rotation can consume a considerable amount of resources and hence only user with a superuser role can trigger it.`, diff --git a/x-pack/plugins/encrypted_saved_objects/server/routes/key_rotation.ts b/x-pack/plugins/encrypted_saved_objects/server/routes/key_rotation.ts index 272e74c3a69cb..46df83a187c3b 100644 --- a/x-pack/plugins/encrypted_saved_objects/server/routes/key_rotation.ts +++ b/x-pack/plugins/encrypted_saved_objects/server/routes/key_rotation.ts @@ -6,6 +6,7 @@ */ import { schema } from '@kbn/config-schema'; +import { ReservedPrivilegesSet } from '@kbn/core/server'; import type { RouteDefinitionParams } from '.'; @@ -39,9 +40,14 @@ export function defineKeyRotationRoutes({ type: schema.maybe(schema.string()), }), }, + security: { + authz: { + requiredPrivileges: [ReservedPrivilegesSet.superuser], + }, + }, options: { - tags: ['access:rotateEncryptionKey', 'oas-tag:saved objects'], access: buildFlavor === 'serverless' ? 'internal' : 'public', + tags: ['oas-tag:saved objects'], summary: `Rotate a key for encrypted saved objects`, description: `If a saved object cannot be decrypted using the primary encryption key, Kibana attempts to decrypt it using the specified decryption-only keys. In most of the cases this overhead is negligible, but if you're dealing with a large number of saved objects and experiencing performance issues, you may want to rotate the encryption key. NOTE: Bulk key rotation can consume a considerable amount of resources and hence only user with a superuser role can trigger it.`, diff --git a/x-pack/plugins/security/common/constants.ts b/x-pack/plugins/security/common/constants.ts index 3a9b20bbb0bd7..c75ab77a7de98 100644 --- a/x-pack/plugins/security/common/constants.ts +++ b/x-pack/plugins/security/common/constants.ts @@ -127,3 +127,14 @@ export const API_VERSIONS = { }, }, }; + +/** + * Privileges that define the superuser role or the role equivalent to the superuser role. + */ +export const SUPERUSER_PRIVILEGES = { + kibana: ['*'], + elasticsearch: { + cluster: ['all'], + index: { '*': ['all'] }, + }, +}; diff --git a/x-pack/plugins/security/server/authorization/api_authorization.test.ts b/x-pack/plugins/security/server/authorization/api_authorization.test.ts index e928d73220274..0181c98d6f1b1 100644 --- a/x-pack/plugins/security/server/authorization/api_authorization.test.ts +++ b/x-pack/plugins/security/server/authorization/api_authorization.test.ts @@ -6,6 +6,7 @@ */ import type { RouteSecurity } from '@kbn/core/server'; +import { ReservedPrivilegesSet } from '@kbn/core/server'; import { coreMock, httpServerMock, @@ -149,7 +150,10 @@ describe('initAPIAuthorization', () => { asserts, }: { security?: RouteSecurity; - kibanaPrivilegesResponse?: Array<{ privilege: string; authorized: boolean }>; + kibanaPrivilegesResponse?: { + privileges: { kibana: Array<{ privilege: string; authorized: boolean }> }; + hasAllRequested?: boolean; + }; kibanaPrivilegesRequestActions?: string[]; asserts: { forbidden?: boolean; @@ -180,11 +184,7 @@ describe('initAPIAuthorization', () => { const mockResponse = httpServerMock.createResponseFactory(); const mockPostAuthToolkit = httpServiceMock.createOnPostAuthToolkit(); - const mockCheckPrivileges = jest.fn().mockReturnValue({ - privileges: { - kibana: kibanaPrivilegesResponse, - }, - }); + const mockCheckPrivileges = jest.fn().mockReturnValue(kibanaPrivilegesResponse); mockAuthz.mode.useRbacForRequest.mockReturnValue(true); mockAuthz.checkPrivilegesDynamicallyWithRequest.mockImplementation((request) => { // hapi conceals the actual "request" from us, so we make sure that the headers are passed to @@ -194,6 +194,12 @@ describe('initAPIAuthorization', () => { return mockCheckPrivileges; }); + mockAuthz.checkPrivilegesWithRequest.mockImplementation((request) => { + expect(request.headers).toMatchObject(headers); + + return { globally: () => kibanaPrivilegesResponse }; + }); + await postAuthHandler(mockRequest, mockResponse, mockPostAuthToolkit); expect(mockAuthz.mode.useRbacForRequest).toHaveBeenCalledWith(mockRequest); @@ -207,11 +213,13 @@ describe('initAPIAuthorization', () => { return; } - expect(mockCheckPrivileges).toHaveBeenCalledWith({ - kibana: kibanaPrivilegesRequestActions!.map((action: string) => - mockAuthz.actions.api.get(action) - ), - }); + if (kibanaPrivilegesRequestActions) { + expect(mockCheckPrivileges).toHaveBeenCalledWith({ + kibana: kibanaPrivilegesRequestActions!.map((action: string) => + mockAuthz.actions.api.get(action) + ), + }); + } if (asserts.forbidden) { expect(mockResponse.forbidden).toHaveBeenCalled(); @@ -239,11 +247,15 @@ describe('initAPIAuthorization', () => { ], }, }, - kibanaPrivilegesResponse: [ - { privilege: 'api:privilege1', authorized: true }, - { privilege: 'api:privilege2', authorized: true }, - { privilege: 'api:privilege3', authorized: false }, - ], + kibanaPrivilegesResponse: { + privileges: { + kibana: [ + { privilege: 'api:privilege1', authorized: true }, + { privilege: 'api:privilege2', authorized: true }, + { privilege: 'api:privilege3', authorized: false }, + ], + }, + }, kibanaPrivilegesRequestActions: ['privilege1', 'privilege2', 'privilege3'], asserts: { authzResult: { @@ -267,10 +279,14 @@ describe('initAPIAuthorization', () => { ], }, }, - kibanaPrivilegesResponse: [ - { privilege: 'api:privilege1', authorized: true }, - { privilege: 'api:privilege2', authorized: true }, - ], + kibanaPrivilegesResponse: { + privileges: { + kibana: [ + { privilege: 'api:privilege1', authorized: true }, + { privilege: 'api:privilege2', authorized: true }, + ], + }, + }, kibanaPrivilegesRequestActions: ['privilege1', 'privilege2'], asserts: { authzResult: { @@ -293,11 +309,15 @@ describe('initAPIAuthorization', () => { ], }, }, - kibanaPrivilegesResponse: [ - { privilege: 'api:privilege1', authorized: false }, - { privilege: 'api:privilege2', authorized: true }, - { privilege: 'api:privilege3', authorized: false }, - ], + kibanaPrivilegesResponse: { + privileges: { + kibana: [ + { privilege: 'api:privilege1', authorized: false }, + { privilege: 'api:privilege2', authorized: true }, + { privilege: 'api:privilege3', authorized: false }, + ], + }, + }, kibanaPrivilegesRequestActions: ['privilege1', 'privilege2', 'privilege3'], asserts: { authzResult: { @@ -317,10 +337,14 @@ describe('initAPIAuthorization', () => { requiredPrivileges: ['privilege1', 'privilege2'], }, }, - kibanaPrivilegesResponse: [ - { privilege: 'api:privilege1', authorized: true }, - { privilege: 'api:privilege2', authorized: true }, - ], + kibanaPrivilegesResponse: { + privileges: { + kibana: [ + { privilege: 'api:privilege1', authorized: true }, + { privilege: 'api:privilege2', authorized: true }, + ], + }, + }, kibanaPrivilegesRequestActions: ['privilege1', 'privilege2'], asserts: { authzResult: { @@ -344,18 +368,54 @@ describe('initAPIAuthorization', () => { ], }, }, - kibanaPrivilegesResponse: [ - { privilege: 'api:privilege1', authorized: true }, - { privilege: 'api:privilege2', authorized: false }, - { privilege: 'api:privilege3', authorized: false }, - ], kibanaPrivilegesRequestActions: ['privilege1', 'privilege2', 'privilege3'], + kibanaPrivilegesResponse: { + privileges: { + kibana: [ + { privilege: 'api:privilege1', authorized: true }, + { privilege: 'api:privilege2', authorized: false }, + { privilege: 'api:privilege3', authorized: false }, + ], + }, + }, asserts: { forbidden: true, }, } ); + testSecurityConfig( + `protected route restricted to only superusers returns forbidden if user not a superuser`, + { + security: { + authz: { + requiredPrivileges: [ReservedPrivilegesSet.superuser], + }, + }, + kibanaPrivilegesResponse: { privileges: { kibana: [] }, hasAllRequested: false }, + asserts: { + forbidden: true, + }, + } + ); + + testSecurityConfig( + `protected route allowed only for superuser access returns "authzResult" if user is superuser`, + { + security: { + authz: { + requiredPrivileges: [ReservedPrivilegesSet.superuser], + }, + }, + kibanaPrivilegesResponse: { privileges: { kibana: [] }, hasAllRequested: true }, + asserts: { + authzResult: { + [ReservedPrivilegesSet.superuser]: true, + }, + }, + } + ); + testSecurityConfig( `protected route returns forbidden if user doesn't have at least one from allRequired privileges requested`, { @@ -369,12 +429,16 @@ describe('initAPIAuthorization', () => { ], }, }, - kibanaPrivilegesResponse: [ - { privilege: 'api:privilege1', authorized: true }, - { privilege: 'api:privilege2', authorized: false }, - { privilege: 'api:privilege3', authorized: false }, - { privilege: 'api:privilege4', authorized: true }, - ], + kibanaPrivilegesResponse: { + privileges: { + kibana: [ + { privilege: 'api:privilege1', authorized: true }, + { privilege: 'api:privilege2', authorized: false }, + { privilege: 'api:privilege3', authorized: false }, + { privilege: 'api:privilege4', authorized: true }, + ], + }, + }, kibanaPrivilegesRequestActions: ['privilege1', 'privilege2', 'privilege3', 'privilege4'], asserts: { forbidden: true, @@ -390,10 +454,14 @@ describe('initAPIAuthorization', () => { requiredPrivileges: ['privilege1', 'privilege2'], }, }, - kibanaPrivilegesResponse: [ - { privilege: 'api:privilege1', authorized: true }, - { privilege: 'api:privilege2', authorized: false }, - ], + kibanaPrivilegesResponse: { + privileges: { + kibana: [ + { privilege: 'api:privilege1', authorized: true }, + { privilege: 'api:privilege2', authorized: false }, + ], + }, + }, kibanaPrivilegesRequestActions: ['privilege1', 'privilege2'], asserts: { forbidden: true, diff --git a/x-pack/plugins/security/server/authorization/api_authorization.ts b/x-pack/plugins/security/server/authorization/api_authorization.ts index 9c67ff8bdff8b..2b99c2a176ac1 100644 --- a/x-pack/plugins/security/server/authorization/api_authorization.ts +++ b/x-pack/plugins/security/server/authorization/api_authorization.ts @@ -14,18 +14,28 @@ import type { PrivilegeSet, RouteAuthz, } from '@kbn/core/server'; +import { ReservedPrivilegesSet } from '@kbn/core/server'; import type { AuthorizationServiceSetup } from '@kbn/security-plugin-types-server'; import type { RecursiveReadonly } from '@kbn/utility-types'; -import { API_OPERATION_PREFIX } from '../../common/constants'; +import { API_OPERATION_PREFIX, SUPERUSER_PRIVILEGES } from '../../common/constants'; const isAuthzDisabled = (authz?: RecursiveReadonly): authz is AuthzDisabled => { return (authz as AuthzDisabled)?.enabled === false; }; +const isReservedPrivilegeSet = (privilege: string): privilege is ReservedPrivilegesSet => { + return Object.hasOwn(ReservedPrivilegesSet, privilege); +}; + export function initAPIAuthorization( http: HttpServiceSetup, - { actions, checkPrivilegesDynamicallyWithRequest, mode }: AuthorizationServiceSetup, + { + actions, + checkPrivilegesDynamicallyWithRequest, + checkPrivilegesWithRequest, + mode, + }: AuthorizationServiceSetup, logger: Logger ) { http.registerOnPostAuth(async (request, response, toolkit) => { @@ -47,24 +57,54 @@ export function initAPIAuthorization( const authz = security.authz as AuthzEnabled; - const requestedPrivileges = authz.requiredPrivileges.flatMap((privilegeEntry) => { - if (typeof privilegeEntry === 'object') { - return [...(privilegeEntry.allRequired ?? []), ...(privilegeEntry.anyRequired ?? [])]; + const { requestedPrivileges, requestedReservedPrivileges } = authz.requiredPrivileges.reduce( + (acc, privilegeEntry) => { + const privileges = + typeof privilegeEntry === 'object' + ? [...(privilegeEntry.allRequired ?? []), ...(privilegeEntry.anyRequired ?? [])] + : [privilegeEntry]; + + for (const privilege of privileges) { + if (isReservedPrivilegeSet(privilege)) { + acc.requestedReservedPrivileges.push(privilege); + } else { + acc.requestedPrivileges.push(privilege); + } + } + + return acc; + }, + { + requestedPrivileges: [] as string[], + requestedReservedPrivileges: [] as string[], } + ); - return privilegeEntry; - }); - - const apiActions = requestedPrivileges.map((permission) => actions.api.get(permission)); const checkPrivileges = checkPrivilegesDynamicallyWithRequest(request); - const checkPrivilegesResponse = await checkPrivileges({ kibana: apiActions }); - const privilegeToApiOperation = (privilege: string) => privilege.replace(API_OPERATION_PREFIX, ''); + const kibanaPrivileges: Record = {}; - for (const kbPrivilege of checkPrivilegesResponse.privileges.kibana) { - kibanaPrivileges[privilegeToApiOperation(kbPrivilege.privilege)] = kbPrivilege.authorized; + if (requestedPrivileges.length > 0) { + const checkPrivilegesResponse = await checkPrivileges({ + kibana: requestedPrivileges.map((permission) => actions.api.get(permission)), + }); + + for (const kbPrivilege of checkPrivilegesResponse.privileges.kibana) { + kibanaPrivileges[privilegeToApiOperation(kbPrivilege.privilege)] = kbPrivilege.authorized; + } + } + + for (const reservedPrivilege of requestedReservedPrivileges) { + if (reservedPrivilege === ReservedPrivilegesSet.superuser) { + const checkSuperuserPrivilegesResponse = await checkPrivilegesWithRequest( + request + ).globally(SUPERUSER_PRIVILEGES); + + kibanaPrivileges[ReservedPrivilegesSet.superuser] = + checkSuperuserPrivilegesResponse.hasAllRequested; + } } const hasRequestedPrivilege = (kbPrivilege: Privilege | PrivilegeSet) => {