diff --git a/src/plugins/csp_handler/README.md b/src/plugins/csp_handler/README.md index 04a6ca34f0dd..f3b846bc3b97 100755 --- a/src/plugins/csp_handler/README.md +++ b/src/plugins/csp_handler/README.md @@ -2,7 +2,7 @@ A OpenSearch Dashboards plugin -This plugin is to support updating Content Security Policy (CSP) rules dynamically without requiring a server restart. It registers a pre-response handler to `HttpServiceSetup` which can get CSP rules from a dependent plugin `applicationConfig` and then rewrite to CSP header. Users are able to call the API endpoint exposed by the `applicationConfig` plugin directly, e.g through CURL. Currently there is no new OSD page for ease of user interactions with the APIs. Updates to the CSP rules will take effect immediately. As a comparison, modifying CSP rules through the key `csp.rules` in OSD YAML file would require a server restart. +This plugin is to support updating the `frame-ancestors` directive in Content Security Policy (CSP) rules dynamically without requiring a server restart. It registers a pre-response handler to `HttpServiceSetup` which can get the `frame-ancestors` directive from a dependent plugin `applicationConfig` and then rewrite to CSP header. It will not change other directives. Users are able to call the API endpoint exposed by the `applicationConfig` plugin directly, e.g through CURL. The configuration key is `csp.rules.frame-ancestors`. Currently there is no new OSD page for ease of user interactions with the APIs. Updates to the `frame-ancestors` directive will take effect immediately. As a comparison, modifying CSP rules through the key `csp.rules` in OSD YAML file would require a server restart. By default, this plugin is disabled. Once enabled, the plugin will first use what users have configured through `applicationConfig`. If not configured, it will check whatever CSP rules aggregated by the values of `csp.rules` from OSD YAML file and default values. If the aggregated CSP rules don't contain the CSP directive `frame-ancestors` which specifies valid parents that may embed OSD page, then the plugin will append `frame-ancestors 'self'` to prevent Clickjacking. @@ -23,23 +23,23 @@ Since it has a required dependency `applicationConfig`, make sure that the depen application_config.enabled: true ``` -For OSD users who want to make changes to allow a new site to embed OSD pages, they can update CSP rules through CURL. (See the README of `applicationConfig` for more details about the APIs.) **Please note that use backslash as string wrapper for single quotes inside the `data-raw` parameter. E.g use `'\''` to represent `'`** +For OSD users who want to make changes to allow a new site to embed OSD pages, they can update the `frame-ancestors` directive through CURL. (See the README of `applicationConfig` for more details about the APIs.) **Please note that use backslash as string wrapper for single quotes inside the `data-raw` parameter. E.g use `'\''` to represent `'`** ``` -curl '{osd endpoint}/api/appconfig/csp.rules' -X POST -H 'Accept: application/json' -H 'Content-Type: application/json' -H 'osd-xsrf: osd-fetch' -H 'Sec-Fetch-Dest: empty' --data-raw '{"newValue":"script-src '\''unsafe-eval'\'' '\''self'\''; worker-src blob: '\''self'\''; style-src '\''unsafe-inline'\'' '\''self'\''; frame-ancestors '\''self'\'' {new site}"}' +curl '{osd endpoint}/api/appconfig/csp.rules.frame-ancestors' -X POST -H 'Accept: application/json' -H 'Content-Type: application/json' -H 'osd-xsrf: osd-fetch' -H 'Sec-Fetch-Dest: empty' --data-raw '{"newValue":"{new value}"}' ``` -Below is the CURL command to delete CSP rules. +Below is the CURL command to delete the `frame-ancestors` directive. ``` -curl '{osd endpoint}/api/appconfig/csp.rules' -X DELETE -H 'osd-xsrf: osd-fetch' -H 'Sec-Fetch-Dest: empty' +curl '{osd endpoint}/api/appconfig/csp.rules.frame-ancestors' -X DELETE -H 'osd-xsrf: osd-fetch' -H 'Sec-Fetch-Dest: empty' ``` -Below is the CURL command to get the CSP rules. +Below is the CURL command to get the `frame-ancestors` directive. ``` -curl '{osd endpoint}/api/appconfig/csp.rules' +curl '{osd endpoint}/api/appconfig/csp.rules.frame-ancestors' ``` diff --git a/src/plugins/csp_handler/server/csp_handlers.test.ts b/src/plugins/csp_handler/server/csp_handlers.test.ts index b185410f6174..9a4b708fc49b 100644 --- a/src/plugins/csp_handler/server/csp_handlers.test.ts +++ b/src/plugins/csp_handler/server/csp_handlers.test.ts @@ -8,6 +8,7 @@ import { createCspRulesPreResponseHandler } from './csp_handlers'; import { MockedLogger, loggerMock } from '@osd/logging/target/mocks'; const ERROR_MESSAGE = 'Service unavailable'; +const CSP_RULES_FRAME_ANCESTORS_CONFIG_KEY = 'csp.rules.frame-ancestors'; describe('CSP handlers', () => { let toolkit: ReturnType; @@ -18,13 +19,13 @@ describe('CSP handlers', () => { logger = loggerMock.create(); }); - it('adds the CSP headers provided by the client', async () => { + it('adds the frame-ancestors provided by the client', async () => { const coreSetup = coreMock.createSetup(); - const cspRulesFromIndex = "frame-ancestors 'self'"; + const frameAncestorsFromIndex = "'self' localsystem"; const cspRulesFromYML = "script-src 'unsafe-eval' 'self'"; const configurationClient = { - getEntityConfig: jest.fn().mockReturnValue(cspRulesFromIndex), + getEntityConfig: jest.fn().mockReturnValue(frameAncestorsFromIndex), }; const getConfigurationClient = jest.fn().mockReturnValue(configurationClient); @@ -50,21 +51,26 @@ describe('CSP handlers', () => { expect(toolkit.next).toHaveBeenCalledWith({ headers: { - 'content-security-policy': cspRulesFromIndex, + 'content-security-policy': + "script-src 'unsafe-eval' 'self'; frame-ancestors 'self' localsystem", }, }); expect(configurationClient.getEntityConfig).toBeCalledTimes(1); + expect(configurationClient.getEntityConfig).toBeCalledWith( + CSP_RULES_FRAME_ANCESTORS_CONFIG_KEY, + { headers: { 'sec-fetch-dest': 'document' } } + ); expect(getConfigurationClient).toBeCalledWith(request); }); - it('do not add CSP headers when the client returns empty and CSP from YML already has frame-ancestors', async () => { + it('do not modify frame-ancestors when the client returns empty and CSP from YML already has frame-ancestors', async () => { const coreSetup = coreMock.createSetup(); - const emptyCspRules = ''; - const cspRulesFromYML = "script-src 'unsafe-eval' 'self'; frame-ancestors 'self'"; + const emptyFrameAncestors = ''; + const cspRulesFromYML = "script-src 'unsafe-eval' 'self'; frame-ancestors localsystem"; const configurationClient = { - getEntityConfig: jest.fn().mockReturnValue(emptyCspRules), + getEntityConfig: jest.fn().mockReturnValue(emptyFrameAncestors), }; const getConfigurationClient = jest.fn().mockReturnValue(configurationClient); @@ -87,19 +93,29 @@ describe('CSP handlers', () => { expect(result).toEqual('next'); expect(toolkit.next).toHaveBeenCalledTimes(1); - expect(toolkit.next).toHaveBeenCalledWith({}); + expect(toolkit.next).toHaveBeenCalledWith({ + headers: { + 'content-security-policy': cspRulesFromYML, + }, + }); expect(configurationClient.getEntityConfig).toBeCalledTimes(1); + + expect(configurationClient.getEntityConfig).toBeCalledWith( + CSP_RULES_FRAME_ANCESTORS_CONFIG_KEY, + { headers: { 'sec-fetch-dest': 'document' } } + ); + expect(getConfigurationClient).toBeCalledWith(request); }); - it('add frame-ancestors CSP headers when the client returns empty and CSP from YML has no frame-ancestors', async () => { + it('add frame-ancestors when the client returns empty and CSP from YML has no frame-ancestors', async () => { const coreSetup = coreMock.createSetup(); - const emptyCspRules = ''; + const emptyFrameAncestors = ''; const cspRulesFromYML = "script-src 'unsafe-eval' 'self'"; const configurationClient = { - getEntityConfig: jest.fn().mockReturnValue(emptyCspRules), + getEntityConfig: jest.fn().mockReturnValue(emptyFrameAncestors), }; const getConfigurationClient = jest.fn().mockReturnValue(configurationClient); @@ -125,17 +141,23 @@ describe('CSP handlers', () => { expect(toolkit.next).toHaveBeenCalledTimes(1); expect(toolkit.next).toHaveBeenCalledWith({ headers: { - 'content-security-policy': "frame-ancestors 'self'; " + cspRulesFromYML, + 'content-security-policy': "script-src 'unsafe-eval' 'self'; frame-ancestors 'self'", }, }); expect(configurationClient.getEntityConfig).toBeCalledTimes(1); + + expect(configurationClient.getEntityConfig).toBeCalledWith( + CSP_RULES_FRAME_ANCESTORS_CONFIG_KEY, + { headers: { 'sec-fetch-dest': 'document' } } + ); + expect(getConfigurationClient).toBeCalledWith(request); }); - it('do not add CSP headers when the configuration does not exist and CSP from YML already has frame-ancestors', async () => { + it('do not modify frame-ancestors when the configuration does not exist and CSP from YML already has frame-ancestors', async () => { const coreSetup = coreMock.createSetup(); - const cspRulesFromYML = "script-src 'unsafe-eval' 'self'; frame-ancestors 'self'"; + const cspRulesFromYML = "script-src 'unsafe-eval' 'self'; frame-ancestors localsystem"; const configurationClient = { getEntityConfig: jest.fn().mockImplementation(() => { @@ -164,13 +186,23 @@ describe('CSP handlers', () => { expect(result).toEqual('next'); expect(toolkit.next).toBeCalledTimes(1); - expect(toolkit.next).toBeCalledWith({}); + expect(toolkit.next).toBeCalledWith({ + headers: { + 'content-security-policy': cspRulesFromYML, + }, + }); expect(configurationClient.getEntityConfig).toBeCalledTimes(1); + + expect(configurationClient.getEntityConfig).toBeCalledWith( + CSP_RULES_FRAME_ANCESTORS_CONFIG_KEY, + { headers: { 'sec-fetch-dest': 'document' } } + ); + expect(getConfigurationClient).toBeCalledWith(request); }); - it('add frame-ancestors CSP headers when the configuration does not exist and CSP from YML has no frame-ancestors', async () => { + it('add frame-ancestors when the configuration does not exist and CSP from YML has no frame-ancestors', async () => { const coreSetup = coreMock.createSetup(); const cspRulesFromYML = "script-src 'unsafe-eval' 'self'"; @@ -199,15 +231,21 @@ describe('CSP handlers', () => { expect(toolkit.next).toBeCalledTimes(1); expect(toolkit.next).toBeCalledWith({ headers: { - 'content-security-policy': "frame-ancestors 'self'; " + cspRulesFromYML, + 'content-security-policy': "script-src 'unsafe-eval' 'self'; frame-ancestors 'self'", }, }); expect(configurationClient.getEntityConfig).toBeCalledTimes(1); + + expect(configurationClient.getEntityConfig).toBeCalledWith( + CSP_RULES_FRAME_ANCESTORS_CONFIG_KEY, + { headers: { 'sec-fetch-dest': 'document' } } + ); + expect(getConfigurationClient).toBeCalledWith(request); }); - it('do not add CSP headers when request dest exists and shall skip', async () => { + it('do not add frame-ancestors when request dest exists and shall skip', async () => { const coreSetup = coreMock.createSetup(); const cspRulesFromYML = "script-src 'unsafe-eval' 'self'"; @@ -243,7 +281,7 @@ describe('CSP handlers', () => { expect(getConfigurationClient).toBeCalledTimes(0); }); - it('do not add CSP headers when request dest does not exist', async () => { + it('do not add frame-ancestors when request dest does not exist', async () => { const coreSetup = coreMock.createSetup(); const cspRulesFromYML = "script-src 'unsafe-eval' 'self'"; @@ -277,4 +315,76 @@ describe('CSP handlers', () => { expect(configurationClient.getEntityConfig).toBeCalledTimes(0); expect(getConfigurationClient).toBeCalledTimes(0); }); + + it('use default values when getting client throws error and CSP from YML has no frame-ancestors', async () => { + const coreSetup = coreMock.createSetup(); + const cspRulesFromYML = "script-src 'unsafe-eval' 'self'"; + + const getConfigurationClient = jest.fn().mockImplementation(() => { + throw new Error(ERROR_MESSAGE); + }); + + const handler = createCspRulesPreResponseHandler( + coreSetup, + cspRulesFromYML, + getConfigurationClient, + logger + ); + + const request = { + method: 'get', + headers: { 'sec-fetch-dest': 'document' }, + }; + + toolkit.next.mockReturnValue('next' as any); + + const result = await handler(request, {} as any, toolkit); + + expect(result).toEqual('next'); + + expect(toolkit.next).toHaveBeenCalledTimes(1); + expect(toolkit.next).toHaveBeenCalledWith({ + headers: { + 'content-security-policy': "script-src 'unsafe-eval' 'self'; frame-ancestors 'self'", + }, + }); + + expect(getConfigurationClient).toBeCalledWith(request); + }); + + it('do not modify when getting client throws error and CSP from YML has frame-ancestors', async () => { + const coreSetup = coreMock.createSetup(); + const cspRulesFromYML = "script-src 'unsafe-eval' 'self'; frame-ancestors localsystem"; + + const getConfigurationClient = jest.fn().mockImplementation(() => { + throw new Error(ERROR_MESSAGE); + }); + + const handler = createCspRulesPreResponseHandler( + coreSetup, + cspRulesFromYML, + getConfigurationClient, + logger + ); + + const request = { + method: 'get', + headers: { 'sec-fetch-dest': 'document' }, + }; + + toolkit.next.mockReturnValue('next' as any); + + const result = await handler(request, {} as any, toolkit); + + expect(result).toEqual('next'); + + expect(toolkit.next).toHaveBeenCalledTimes(1); + expect(toolkit.next).toHaveBeenCalledWith({ + headers: { + 'content-security-policy': cspRulesFromYML, + }, + }); + + expect(getConfigurationClient).toBeCalledWith(request); + }); }); diff --git a/src/plugins/csp_handler/server/csp_handlers.ts b/src/plugins/csp_handler/server/csp_handlers.ts index 1a76ed942460..f59cf3f18e56 100644 --- a/src/plugins/csp_handler/server/csp_handlers.ts +++ b/src/plugins/csp_handler/server/csp_handlers.ts @@ -6,15 +6,27 @@ import { ConfigurationClient } from '../../application_config/server'; import { CoreSetup, - IScopedClusterClient, Logger, OnPreResponseHandler, OnPreResponseInfo, OnPreResponseToolkit, OpenSearchDashboardsRequest, } from '../../../core/server'; +import { parseCspHeader, stringifyCspHeader } from './csp_header_utils'; -const CSP_RULES_CONFIG_KEY = 'csp.rules'; +const FRAME_ANCESTORS_DIRECTIVE = 'frame-ancestors'; +const CSP_RULES_FRAME_ANCESTORS_CONFIG_KEY = 'csp.rules.frame-ancestors'; + +// add new directives to this Map when onboarding. +const SUPPORTED_DIRECTIVES = new Map([ + [ + CSP_RULES_FRAME_ANCESTORS_CONFIG_KEY, + { + directiveName: FRAME_ANCESTORS_DIRECTIVE, + defaultValue: ["'self'"], + }, + ], +]); /** * This function creates a pre-response handler to dynamically set the CSP rules. @@ -38,6 +50,8 @@ export function createCspRulesPreResponseHandler( response: OnPreResponseInfo, toolkit: OnPreResponseToolkit ) => { + const parsedCspHeader = parseCspHeader(cspHeader); + try { const shouldCheckDest = ['document', 'frame', 'iframe', 'embed', 'object']; @@ -49,36 +63,62 @@ export function createCspRulesPreResponseHandler( const client = getConfigurationClient(request); - const cspRules = await client.getEntityConfig(CSP_RULES_CONFIG_KEY, { + await updateDirectivesFromConfigurationClient(parsedCspHeader, client, request, logger); + + return updateNext(parsedCspHeader, toolkit); + } catch (e) { + logger.error(`Failure happened in CSP rules pre response handler due to ${e}`); + + updateDirectivesFromDefault(parsedCspHeader); + return updateNext(parsedCspHeader, toolkit); + } + }; +} + +async function updateDirectivesFromConfigurationClient( + parsedCspHeader: Map, + client: ConfigurationClient, + request: OpenSearchDashboardsRequest, + logger: Logger +) { + for (const [configKey, directive] of SUPPORTED_DIRECTIVES) { + try { + const value = await client.getEntityConfig(configKey, { headers: request.headers, }); - if (!cspRules) { - return appendFrameAncestorsWhenMissing(cspHeader, toolkit); + if (!value || !value.trim()) { + return addDirectiveWhenMissing(parsedCspHeader, directive); } - const additionalHeaders = { - 'content-security-policy': cspRules, - }; - - return toolkit.next({ headers: additionalHeaders }); + parsedCspHeader.set(directive.directiveName, value.trim().split(' ')); } catch (e) { - logger.error(`Failure happened in CSP rules pre response handler due to ${e}`); - return appendFrameAncestorsWhenMissing(cspHeader, toolkit); + logger.error( + `Failure happened when handling CSP directive ${directive.directiveName} due to ${e}` + ); + + addDirectiveWhenMissing(parsedCspHeader, directive); } - }; + } } -/** - * Append frame-ancestors with default value 'self' when it is missing. - */ -function appendFrameAncestorsWhenMissing(cspHeader: string, toolkit: OnPreResponseToolkit) { - if (cspHeader.includes('frame-ancestors')) { - return toolkit.next({}); +function updateDirectivesFromDefault(parsedCspHeader: Map) { + SUPPORTED_DIRECTIVES.forEach(async (directive) => { + addDirectiveWhenMissing(parsedCspHeader, directive); + }); +} + +function addDirectiveWhenMissing(parsedCspHeader: Map, directive) { + if (parsedCspHeader.has(directive.directiveName)) { + return; } + parsedCspHeader.set(directive.directiveName, directive.defaultValue); +} + +function updateNext(parsedCspHeader: Map, toolkit: OnPreResponseToolkit) { const additionalHeaders = { - 'content-security-policy': "frame-ancestors 'self'; " + cspHeader, + 'content-security-policy': stringifyCspHeader(parsedCspHeader), }; return toolkit.next({ headers: additionalHeaders }); diff --git a/src/plugins/csp_handler/server/csp_header_utils.test.ts b/src/plugins/csp_handler/server/csp_header_utils.test.ts new file mode 100644 index 000000000000..f1f88f5a8726 --- /dev/null +++ b/src/plugins/csp_handler/server/csp_header_utils.test.ts @@ -0,0 +1,46 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { parseCspHeader, stringifyCspHeader } from './csp_header_utils'; + +describe('CSP header utils', () => { + describe('parseCspHeader', () => { + it('parses multiple directives', () => { + const parsed = parseCspHeader( + "script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'; frame-ancestors 'self' localsystem" + ); + + expect(parsed.size).toBe(4); + expect(parsed.get('script-src')).toEqual(["'unsafe-eval'", "'self'"]); + expect(parsed.get('worker-src')).toEqual(['blob:', "'self'"]); + expect(parsed.get('style-src')).toEqual(["'unsafe-inline'", "'self'"]); + expect(parsed.get('frame-ancestors')).toEqual(["'self'", 'localsystem']); + }); + + it('parses single directive', () => { + const parsed = parseCspHeader("frame-ancestors 'self' localsystem"); + + expect(parsed.size).toBe(1); + expect(parsed.get('frame-ancestors')).toEqual(["'self'", 'localsystem']); + }); + }); + + describe('stringifyCspHeader', () => { + it('stringify multiple directives', () => { + const cspHeader = new Map([ + ['script-src', ["'unsafe-eval'", "'self'"]], + ['worker-src', ['blob:', "'self'"]], + ['style-src', ["'unsafe-inline'", "'self'"]], + ['frame-ancestors', ["'self'", 'localsystem']], + ]); + + const stringified = stringifyCspHeader(cspHeader); + + expect(stringified).toBe( + "script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'; frame-ancestors 'self' localsystem" + ); + }); + }); +}); diff --git a/src/plugins/csp_handler/server/csp_header_utils.ts b/src/plugins/csp_handler/server/csp_header_utils.ts new file mode 100644 index 000000000000..0fa5351ca5ca --- /dev/null +++ b/src/plugins/csp_handler/server/csp_header_utils.ts @@ -0,0 +1,28 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +// parse an input CSP header string into a Map where the key of the Map is the directive name +// and the value of the key is a string array of the directive values +export function parseCspHeader(cspHeader: string) { + const directives: string[] = cspHeader.split(';'); + + return directives.reduce((accumulator, directive) => { + const trimmed = directive.trim().split(' '); + + accumulator.set(trimmed[0], trimmed.slice(1)); + + return accumulator; + }, new Map()); +} + +// stringify a CSP header Map to a string +export function stringifyCspHeader(parsedCspHeader: Map) { + const directives: string[] = []; + parsedCspHeader.forEach((values: string[], directive: string) => { + directives.push(directive + ' ' + values.join(' ')); + }); + + return directives.join('; '); +}