Support dynamic CSP rules to mitigate clickjacking (#5641)

* support dynamic csp rules to mitigate clickjacking Signed-off-by: Tianle Huang <[email protected]> * add unit tests for the provider class Signed-off-by: Tianle Huang <[email protected]> * move request handler to its own class Signed-off-by: Tianle Huang <[email protected]> * add license headers Signed-off-by: Tianle Huang <[email protected]> * fix failed unit tests Signed-off-by: Tianle Huang <[email protected]> * add unit tests for the handler Signed-off-by: Tianle Huang <[email protected]> * add content to read me Signed-off-by: Tianle Huang <[email protected]> * fix test error Signed-off-by: Tianle Huang <[email protected]> * update readme Signed-off-by: Tianle Huang <[email protected]> * update CHANGELOG.md Signed-off-by: Tianle Huang <[email protected]> * update snap tests Signed-off-by: Tianle Huang <[email protected]> * update snapshots Signed-off-by: Tianle Huang <[email protected]> * fix a wrong import Signed-off-by: Tianle Huang <[email protected]> * undo changes in listing snap Signed-off-by: Tianle Huang <[email protected]> * improve wording Signed-off-by: Tianle Huang <[email protected]> * set client after default client is created Signed-off-by: Tianle Huang <[email protected]> * update return value and add a unit test Signed-off-by: Tianle Huang <[email protected]> * remove unnecessary dependency Signed-off-by: Tianle Huang <[email protected]> * make the name of the index configurable Signed-off-by: Tianle Huang <[email protected]> * expose APIs and update file structures Signed-off-by: Tianle Huang <[email protected]> * add header Signed-off-by: Tianle Huang <[email protected]> * fix link error Signed-off-by: Tianle Huang <[email protected]> * fix link error Signed-off-by: Tianle Huang <[email protected]> * add more unit tests Signed-off-by: Tianle Huang <[email protected]> * add more unit tests Signed-off-by: Tianle Huang <[email protected]> * update api path Signed-off-by: Tianle Huang <[email protected]> * remove logging Signed-off-by: Tianle Huang <[email protected]> * update path Signed-off-by: Tianle Huang <[email protected]> * rename index name Signed-off-by: Tianle Huang <[email protected]> * update wording Signed-off-by: Tianle Huang <[email protected]> * make the new plugin disabled by default Signed-off-by: Tianle Huang <[email protected]> * do not update defaults to avoid breaking change Signed-off-by: Tianle Huang <[email protected]> * update readme to reflect new API path Signed-off-by: Tianle Huang <[email protected]> * update handler to append frame-ancestors conditionally Signed-off-by: Tianle Huang <[email protected]> * update readme Signed-off-by: Tianle Huang <[email protected]> * clean up code to prepare for application config Signed-off-by: Tianle Huang <[email protected]> * reset change log Signed-off-by: Tianle Huang <[email protected]> * reset change log again Signed-off-by: Tianle Huang <[email protected]> * update accordingly to new changes in applicationConfig Signed-off-by: Tianle Huang <[email protected]> * update changelog Signed-off-by: Tianle Huang <[email protected]> * rename to a new plugin name Signed-off-by: Tianle Huang <[email protected]> * rename Signed-off-by: Tianle Huang <[email protected]> * rename more Signed-off-by: Tianle Huang <[email protected]> * sync changelog from main Signed-off-by: Tianle Huang <[email protected]> * onboard to app config Signed-off-by: Tianle Huang <[email protected]> * fix comment Signed-off-by: Tianle Huang <[email protected]> * update yml Signed-off-by: Tianle Huang <[email protected]> * update readme Signed-off-by: Tianle Huang <[email protected]> * update change log Signed-off-by: Tianle Huang <[email protected]> * call out single quotes in readme Signed-off-by: Tianle Huang <[email protected]> * update yml Signed-off-by: Tianle Huang <[email protected]> * update default Signed-off-by: Tianle Huang <[email protected]> * add reference link Signed-off-by: Tianle Huang <[email protected]> * update js doc Signed-off-by: Tianle Huang <[email protected]> * rename Signed-off-by: Tianle Huang <[email protected]> * use new name Signed-off-by: Tianle Huang <[email protected]> * redo changelog update Signed-off-by: Tianle Huang <[email protected]> * remove link Signed-off-by: Tianle Huang <[email protected]> * better name Signed-off-by: Tianle Huang <[email protected]> --------- Signed-off-by: Tianle Huang <[email protected]> (cherry picked from commit 58fb588) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-project · Mar 8, 2024 · 881668a · 881668a
1 parent 8ccc90e
commit 881668a
Show file tree

Hide file tree

Showing 11 changed files with 527 additions and 1 deletion.
diff --git a/config/opensearch_dashboards.yml b/config/opensearch_dashboards.yml
@@ -36,6 +36,10 @@
 # Set the value of this setting to true to enable plugin application config. By default it is disabled.
 # application_config.enabled: false
 
+# Set the value of this setting to true to enable plugin CSP handler. By default it is disabled.
+# It requires the application config plugin as its dependency.
+# csp_handler.enabled: false
+
 # The default application to load.
 #opensearchDashboards.defaultAppId: "home"
 

diff --git a/src/plugins/application_config/server/index.ts b/src/plugins/application_config/server/index.ts
@@ -20,4 +20,8 @@ export function plugin(initializerContext: PluginInitializerContext) {
   return new ApplicationConfigPlugin(initializerContext);
 }
 
-export { ApplicationConfigPluginSetup, ApplicationConfigPluginStart } from './types';
+export {
+  ApplicationConfigPluginSetup,
+  ApplicationConfigPluginStart,
+  ConfigurationClient,
+} from './types';
diff --git a/src/plugins/csp_handler/README.md b/src/plugins/csp_handler/README.md
@@ -0,0 +1,51 @@
+# CspHandler
+
+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.
+
+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.
+
+---
+
+## Configuration
+
+The plugin can be enabled by adding this line in OSD YML.
+
+```
+csp_handler.enabled: true
+
+```
+
+Since it has a required dependency `applicationConfig`, make sure that the dependency is also enabled.
+
+```
+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 `'`**
+
+```
+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}"}'
+
+```
+
+Below is the CURL command to delete CSP rules.
+
+```
+curl '{osd endpoint}/api/appconfig/csp.rules' -X DELETE -H 'osd-xsrf: osd-fetch' -H 'Sec-Fetch-Dest: empty'
+```
+
+Below is the CURL command to get the CSP rules.
+
+```
+curl '{osd endpoint}/api/appconfig/csp.rules'
+
+```
+
+---
+## Development
+
+See the [OpenSearch Dashboards contributing
+guide](https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/CONTRIBUTING.md) for instructions
+setting up your development environment.
diff --git a/src/plugins/csp_handler/common/index.ts b/src/plugins/csp_handler/common/index.ts
@@ -0,0 +1,7 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+export const PLUGIN_ID = 'cspHandler';
+export const PLUGIN_NAME = 'CspHandler';
diff --git a/src/plugins/csp_handler/config.ts b/src/plugins/csp_handler/config.ts
@@ -0,0 +1,12 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { schema, TypeOf } from '@osd/config-schema';
+
+export const configSchema = schema.object({
+  enabled: schema.boolean({ defaultValue: false }),
+});
+
+export type CspHandlerConfigSchema = TypeOf<typeof configSchema>;
diff --git a/src/plugins/csp_handler/opensearch_dashboards.json b/src/plugins/csp_handler/opensearch_dashboards.json
@@ -0,0 +1,11 @@
+{
+  "id": "cspHandler",
+  "version": "opensearchDashboards",
+  "opensearchDashboardsVersion": "opensearchDashboards",
+  "server": true,
+  "ui": false,
+  "requiredPlugins": [
+    "applicationConfig"
+  ],
+  "optionalPlugins": []
+}
diff --git a/src/plugins/csp_handler/server/csp_handlers.test.ts b/src/plugins/csp_handler/server/csp_handlers.test.ts
@@ -0,0 +1,273 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { coreMock, httpServerMock } from '../../../core/server/mocks';
+import { createCspRulesPreResponseHandler } from './csp_handlers';
+import { MockedLogger, loggerMock } from '@osd/logging/target/mocks';
+
+const ERROR_MESSAGE = 'Service unavailable';
+
+describe('CSP handlers', () => {
+  let toolkit: ReturnType<typeof httpServerMock.createToolkit>;
+  let logger: MockedLogger;
+
+  beforeEach(() => {
+    toolkit = httpServerMock.createToolkit();
+    logger = loggerMock.create();
+  });
+
+  it('adds the CSP headers provided by the client', async () => {
+    const coreSetup = coreMock.createSetup();
+    const cspRulesFromIndex = "frame-ancestors 'self'";
+    const cspRulesFromYML = "script-src 'unsafe-eval' 'self'";
+
+    const configurationClient = {
+      getEntityConfig: jest.fn().mockReturnValue(cspRulesFromIndex),
+    };
+
+    const getConfigurationClient = jest.fn().mockReturnValue(configurationClient);
+
+    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': cspRulesFromIndex,
+      },
+    });
+
+    expect(configurationClient.getEntityConfig).toBeCalledTimes(1);
+  });
+
+  it('do not add CSP headers 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 configurationClient = {
+      getEntityConfig: jest.fn().mockReturnValue(emptyCspRules),
+    };
+
+    const getConfigurationClient = jest.fn().mockReturnValue(configurationClient);
+
+    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({});
+
+    expect(configurationClient.getEntityConfig).toBeCalledTimes(1);
+  });
+
+  it('add frame-ancestors CSP headers when the client returns empty and CSP from YML has no frame-ancestors', async () => {
+    const coreSetup = coreMock.createSetup();
+    const emptyCspRules = '';
+    const cspRulesFromYML = "script-src 'unsafe-eval' 'self'";
+
+    const configurationClient = {
+      getEntityConfig: jest.fn().mockReturnValue(emptyCspRules),
+    };
+
+    const getConfigurationClient = jest.fn().mockReturnValue(configurationClient);
+
+    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': "frame-ancestors 'self'; " + cspRulesFromYML,
+      },
+    });
+
+    expect(configurationClient.getEntityConfig).toBeCalledTimes(1);
+  });
+
+  it('do not add CSP headers 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 configurationClient = {
+      getEntityConfig: jest.fn().mockImplementation(() => {
+        throw new Error(ERROR_MESSAGE);
+      }),
+    };
+
+    const getConfigurationClient = jest.fn().mockReturnValue(configurationClient);
+
+    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).toBeCalledTimes(1);
+    expect(toolkit.next).toBeCalledWith({});
+
+    expect(configurationClient.getEntityConfig).toBeCalledTimes(1);
+  });
+
+  it('add frame-ancestors CSP headers 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'";
+
+    const configurationClient = {
+      getEntityConfig: jest.fn().mockImplementation(() => {
+        throw new Error(ERROR_MESSAGE);
+      }),
+    };
+
+    const getConfigurationClient = jest.fn().mockReturnValue(configurationClient);
+
+    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).toBeCalledTimes(1);
+    expect(toolkit.next).toBeCalledWith({
+      headers: {
+        'content-security-policy': "frame-ancestors 'self'; " + cspRulesFromYML,
+      },
+    });
+
+    expect(configurationClient.getEntityConfig).toBeCalledTimes(1);
+  });
+
+  it('do not add CSP headers when request dest exists and shall skip', async () => {
+    const coreSetup = coreMock.createSetup();
+    const cspRulesFromYML = "script-src 'unsafe-eval' 'self'";
+
+    const configurationClient = {
+      getEntityConfig: jest.fn(),
+    };
+
+    const getConfigurationClient = jest.fn().mockReturnValue(configurationClient);
+
+    const handler = createCspRulesPreResponseHandler(
+      coreSetup,
+      cspRulesFromYML,
+      getConfigurationClient,
+      logger
+    );
+
+    const cssSecFetchDest = 'css';
+    const request = {
+      method: 'get',
+      headers: { 'sec-fetch-dest': cssSecFetchDest },
+    };
+
+    toolkit.next.mockReturnValue('next' as any);
+
+    const result = await handler(request, {} as any, toolkit);
+
+    expect(result).toEqual('next');
+
+    expect(toolkit.next).toBeCalledTimes(1);
+    expect(toolkit.next).toBeCalledWith({});
+
+    expect(configurationClient.getEntityConfig).toBeCalledTimes(0);
+  });
+
+  it('do not add CSP headers when request dest does not exist', async () => {
+    const coreSetup = coreMock.createSetup();
+    const cspRulesFromYML = "script-src 'unsafe-eval' 'self'";
+
+    const configurationClient = {
+      getEntityConfig: jest.fn(),
+    };
+
+    const getConfigurationClient = jest.fn().mockReturnValue(configurationClient);
+
+    const handler = createCspRulesPreResponseHandler(
+      coreSetup,
+      cspRulesFromYML,
+      getConfigurationClient,
+      logger
+    );
+
+    const request = {
+      method: 'get',
+      headers: {},
+    };
+
+    toolkit.next.mockReturnValue('next' as any);
+
+    const result = await handler(request, {} as any, toolkit);
+
+    expect(result).toEqual('next');
+
+    expect(toolkit.next).toBeCalledTimes(1);
+    expect(toolkit.next).toBeCalledWith({});
+
+    expect(configurationClient.getEntityConfig).toBeCalledTimes(0);
+  });
+});