Skip to content

Commit 58fb588

Browse files
authored
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]>
1 parent 1c5ad6c commit 58fb588

File tree

12 files changed

+528
-1
lines changed

12 files changed

+528
-1
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
99
### Deprecations
1010

1111
### 🛡 Security
12+
- Support dynamic CSP rules to mitigate Clickjacking https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5641
1213

1314
### 📈 Features/Enhancements
1415
- [MD]Change cluster selector component name to data source selector ([#6042](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6042))

config/opensearch_dashboards.yml

+4
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@
3636
# Set the value of this setting to true to enable plugin application config. By default it is disabled.
3737
# application_config.enabled: false
3838

39+
# Set the value of this setting to true to enable plugin CSP handler. By default it is disabled.
40+
# It requires the application config plugin as its dependency.
41+
# csp_handler.enabled: false
42+
3943
# The default application to load.
4044
#opensearchDashboards.defaultAppId: "home"
4145

src/plugins/application_config/server/index.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,8 @@ export function plugin(initializerContext: PluginInitializerContext) {
2020
return new ApplicationConfigPlugin(initializerContext);
2121
}
2222

23-
export { ApplicationConfigPluginSetup, ApplicationConfigPluginStart } from './types';
23+
export {
24+
ApplicationConfigPluginSetup,
25+
ApplicationConfigPluginStart,
26+
ConfigurationClient,
27+
} from './types';

src/plugins/csp_handler/README.md

+51
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
# CspHandler
2+
3+
A OpenSearch Dashboards plugin
4+
5+
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.
6+
7+
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.
8+
9+
---
10+
11+
## Configuration
12+
13+
The plugin can be enabled by adding this line in OSD YML.
14+
15+
```
16+
csp_handler.enabled: true
17+
18+
```
19+
20+
Since it has a required dependency `applicationConfig`, make sure that the dependency is also enabled.
21+
22+
```
23+
application_config.enabled: true
24+
```
25+
26+
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 `'`**
27+
28+
```
29+
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}"}'
30+
31+
```
32+
33+
Below is the CURL command to delete CSP rules.
34+
35+
```
36+
curl '{osd endpoint}/api/appconfig/csp.rules' -X DELETE -H 'osd-xsrf: osd-fetch' -H 'Sec-Fetch-Dest: empty'
37+
```
38+
39+
Below is the CURL command to get the CSP rules.
40+
41+
```
42+
curl '{osd endpoint}/api/appconfig/csp.rules'
43+
44+
```
45+
46+
---
47+
## Development
48+
49+
See the [OpenSearch Dashboards contributing
50+
guide](https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/CONTRIBUTING.md) for instructions
51+
setting up your development environment.
+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
export const PLUGIN_ID = 'cspHandler';
7+
export const PLUGIN_NAME = 'CspHandler';

src/plugins/csp_handler/config.ts

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
import { schema, TypeOf } from '@osd/config-schema';
7+
8+
export const configSchema = schema.object({
9+
enabled: schema.boolean({ defaultValue: false }),
10+
});
11+
12+
export type CspHandlerConfigSchema = TypeOf<typeof configSchema>;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"id": "cspHandler",
3+
"version": "opensearchDashboards",
4+
"opensearchDashboardsVersion": "opensearchDashboards",
5+
"server": true,
6+
"ui": false,
7+
"requiredPlugins": [
8+
"applicationConfig"
9+
],
10+
"optionalPlugins": []
11+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,273 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
import { coreMock, httpServerMock } from '../../../core/server/mocks';
7+
import { createCspRulesPreResponseHandler } from './csp_handlers';
8+
import { MockedLogger, loggerMock } from '@osd/logging/target/mocks';
9+
10+
const ERROR_MESSAGE = 'Service unavailable';
11+
12+
describe('CSP handlers', () => {
13+
let toolkit: ReturnType<typeof httpServerMock.createToolkit>;
14+
let logger: MockedLogger;
15+
16+
beforeEach(() => {
17+
toolkit = httpServerMock.createToolkit();
18+
logger = loggerMock.create();
19+
});
20+
21+
it('adds the CSP headers provided by the client', async () => {
22+
const coreSetup = coreMock.createSetup();
23+
const cspRulesFromIndex = "frame-ancestors 'self'";
24+
const cspRulesFromYML = "script-src 'unsafe-eval' 'self'";
25+
26+
const configurationClient = {
27+
getEntityConfig: jest.fn().mockReturnValue(cspRulesFromIndex),
28+
};
29+
30+
const getConfigurationClient = jest.fn().mockReturnValue(configurationClient);
31+
32+
const handler = createCspRulesPreResponseHandler(
33+
coreSetup,
34+
cspRulesFromYML,
35+
getConfigurationClient,
36+
logger
37+
);
38+
const request = {
39+
method: 'get',
40+
headers: { 'sec-fetch-dest': 'document' },
41+
};
42+
43+
toolkit.next.mockReturnValue('next' as any);
44+
45+
const result = await handler(request, {} as any, toolkit);
46+
47+
expect(result).toEqual('next');
48+
49+
expect(toolkit.next).toHaveBeenCalledTimes(1);
50+
51+
expect(toolkit.next).toHaveBeenCalledWith({
52+
headers: {
53+
'content-security-policy': cspRulesFromIndex,
54+
},
55+
});
56+
57+
expect(configurationClient.getEntityConfig).toBeCalledTimes(1);
58+
});
59+
60+
it('do not add CSP headers when the client returns empty and CSP from YML already has frame-ancestors', async () => {
61+
const coreSetup = coreMock.createSetup();
62+
const emptyCspRules = '';
63+
const cspRulesFromYML = "script-src 'unsafe-eval' 'self'; frame-ancestors 'self'";
64+
65+
const configurationClient = {
66+
getEntityConfig: jest.fn().mockReturnValue(emptyCspRules),
67+
};
68+
69+
const getConfigurationClient = jest.fn().mockReturnValue(configurationClient);
70+
71+
const handler = createCspRulesPreResponseHandler(
72+
coreSetup,
73+
cspRulesFromYML,
74+
getConfigurationClient,
75+
logger
76+
);
77+
const request = {
78+
method: 'get',
79+
headers: { 'sec-fetch-dest': 'document' },
80+
};
81+
82+
toolkit.next.mockReturnValue('next' as any);
83+
84+
const result = await handler(request, {} as any, toolkit);
85+
86+
expect(result).toEqual('next');
87+
88+
expect(toolkit.next).toHaveBeenCalledTimes(1);
89+
expect(toolkit.next).toHaveBeenCalledWith({});
90+
91+
expect(configurationClient.getEntityConfig).toBeCalledTimes(1);
92+
});
93+
94+
it('add frame-ancestors CSP headers when the client returns empty and CSP from YML has no frame-ancestors', async () => {
95+
const coreSetup = coreMock.createSetup();
96+
const emptyCspRules = '';
97+
const cspRulesFromYML = "script-src 'unsafe-eval' 'self'";
98+
99+
const configurationClient = {
100+
getEntityConfig: jest.fn().mockReturnValue(emptyCspRules),
101+
};
102+
103+
const getConfigurationClient = jest.fn().mockReturnValue(configurationClient);
104+
105+
const handler = createCspRulesPreResponseHandler(
106+
coreSetup,
107+
cspRulesFromYML,
108+
getConfigurationClient,
109+
logger
110+
);
111+
112+
const request = {
113+
method: 'get',
114+
headers: { 'sec-fetch-dest': 'document' },
115+
};
116+
117+
toolkit.next.mockReturnValue('next' as any);
118+
119+
const result = await handler(request, {} as any, toolkit);
120+
121+
expect(result).toEqual('next');
122+
123+
expect(toolkit.next).toHaveBeenCalledTimes(1);
124+
expect(toolkit.next).toHaveBeenCalledWith({
125+
headers: {
126+
'content-security-policy': "frame-ancestors 'self'; " + cspRulesFromYML,
127+
},
128+
});
129+
130+
expect(configurationClient.getEntityConfig).toBeCalledTimes(1);
131+
});
132+
133+
it('do not add CSP headers when the configuration does not exist and CSP from YML already has frame-ancestors', async () => {
134+
const coreSetup = coreMock.createSetup();
135+
const cspRulesFromYML = "script-src 'unsafe-eval' 'self'; frame-ancestors 'self'";
136+
137+
const configurationClient = {
138+
getEntityConfig: jest.fn().mockImplementation(() => {
139+
throw new Error(ERROR_MESSAGE);
140+
}),
141+
};
142+
143+
const getConfigurationClient = jest.fn().mockReturnValue(configurationClient);
144+
145+
const handler = createCspRulesPreResponseHandler(
146+
coreSetup,
147+
cspRulesFromYML,
148+
getConfigurationClient,
149+
logger
150+
);
151+
152+
const request = {
153+
method: 'get',
154+
headers: { 'sec-fetch-dest': 'document' },
155+
};
156+
157+
toolkit.next.mockReturnValue('next' as any);
158+
159+
const result = await handler(request, {} as any, toolkit);
160+
161+
expect(result).toEqual('next');
162+
163+
expect(toolkit.next).toBeCalledTimes(1);
164+
expect(toolkit.next).toBeCalledWith({});
165+
166+
expect(configurationClient.getEntityConfig).toBeCalledTimes(1);
167+
});
168+
169+
it('add frame-ancestors CSP headers when the configuration does not exist and CSP from YML has no frame-ancestors', async () => {
170+
const coreSetup = coreMock.createSetup();
171+
const cspRulesFromYML = "script-src 'unsafe-eval' 'self'";
172+
173+
const configurationClient = {
174+
getEntityConfig: jest.fn().mockImplementation(() => {
175+
throw new Error(ERROR_MESSAGE);
176+
}),
177+
};
178+
179+
const getConfigurationClient = jest.fn().mockReturnValue(configurationClient);
180+
181+
const handler = createCspRulesPreResponseHandler(
182+
coreSetup,
183+
cspRulesFromYML,
184+
getConfigurationClient,
185+
logger
186+
);
187+
const request = { method: 'get', headers: { 'sec-fetch-dest': 'document' } };
188+
189+
toolkit.next.mockReturnValue('next' as any);
190+
191+
const result = await handler(request, {} as any, toolkit);
192+
193+
expect(result).toEqual('next');
194+
195+
expect(toolkit.next).toBeCalledTimes(1);
196+
expect(toolkit.next).toBeCalledWith({
197+
headers: {
198+
'content-security-policy': "frame-ancestors 'self'; " + cspRulesFromYML,
199+
},
200+
});
201+
202+
expect(configurationClient.getEntityConfig).toBeCalledTimes(1);
203+
});
204+
205+
it('do not add CSP headers when request dest exists and shall skip', async () => {
206+
const coreSetup = coreMock.createSetup();
207+
const cspRulesFromYML = "script-src 'unsafe-eval' 'self'";
208+
209+
const configurationClient = {
210+
getEntityConfig: jest.fn(),
211+
};
212+
213+
const getConfigurationClient = jest.fn().mockReturnValue(configurationClient);
214+
215+
const handler = createCspRulesPreResponseHandler(
216+
coreSetup,
217+
cspRulesFromYML,
218+
getConfigurationClient,
219+
logger
220+
);
221+
222+
const cssSecFetchDest = 'css';
223+
const request = {
224+
method: 'get',
225+
headers: { 'sec-fetch-dest': cssSecFetchDest },
226+
};
227+
228+
toolkit.next.mockReturnValue('next' as any);
229+
230+
const result = await handler(request, {} as any, toolkit);
231+
232+
expect(result).toEqual('next');
233+
234+
expect(toolkit.next).toBeCalledTimes(1);
235+
expect(toolkit.next).toBeCalledWith({});
236+
237+
expect(configurationClient.getEntityConfig).toBeCalledTimes(0);
238+
});
239+
240+
it('do not add CSP headers when request dest does not exist', async () => {
241+
const coreSetup = coreMock.createSetup();
242+
const cspRulesFromYML = "script-src 'unsafe-eval' 'self'";
243+
244+
const configurationClient = {
245+
getEntityConfig: jest.fn(),
246+
};
247+
248+
const getConfigurationClient = jest.fn().mockReturnValue(configurationClient);
249+
250+
const handler = createCspRulesPreResponseHandler(
251+
coreSetup,
252+
cspRulesFromYML,
253+
getConfigurationClient,
254+
logger
255+
);
256+
257+
const request = {
258+
method: 'get',
259+
headers: {},
260+
};
261+
262+
toolkit.next.mockReturnValue('next' as any);
263+
264+
const result = await handler(request, {} as any, toolkit);
265+
266+
expect(result).toEqual('next');
267+
268+
expect(toolkit.next).toBeCalledTimes(1);
269+
expect(toolkit.next).toBeCalledWith({});
270+
271+
expect(configurationClient.getEntityConfig).toBeCalledTimes(0);
272+
});
273+
});

0 commit comments

Comments
 (0)