From f0b5ae9ded5debe06f4312c4d34e7e7619cb807e Mon Sep 17 00:00:00 2001 From: Miki Date: Tue, 21 Mar 2023 11:20:27 -0700 Subject: [PATCH] Omit adding the `osd-version` header when the Fetch request is to an external origin * Making `fetch` requests using core/public/http/fetch, an `osd-version` header is forcefully added, even to external requests. This change examines the destination and only adds the header to relative URLs and those that are to OSD itself. * This change also adds `osd-xsrf` to calls that use `osd-version` incorrectly to satisfy XSRF protection Fixes #3277 Signed-off-by: Miki --- CHANGELOG.md | 2 ++ src/core/public/http/fetch.ts | 13 ++++++++++++- .../integration_tests/lifecycle_handlers.test.ts | 3 +++ src/core/server/http/lifecycle_handlers.test.ts | 7 ++++--- src/core/server/http/lifecycle_handlers.ts | 3 ++- src/plugins/bfetch/public/plugin.ts | 1 + .../public/angular/angular_config.tsx | 4 ++++ 7 files changed, 28 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c01bc19ccaa..15893895513e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Vis Builder] Add metric to metric, bucket to bucket aggregation persistence ([#3495](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3495)) - Use mirrors to download Node.js binaries to escape sporadic 404 errors ([#3619](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3619)) - [Multiple DataSource] Refactor dev tool console to use opensearch-js client to send requests ([#3544](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3544)) +- Add `osd-xsrf` header to all requests that incorrectly used `node-version` to satisfy XSRF protection ([#3643](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3643)) ### 🐛 Bug Fixes @@ -107,6 +108,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Region Maps] Add ui setting to configure custom vector map's size parameter([#3399](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3399)) - [Search Telemetry] Fixes search telemetry's observable object that won't be GC-ed([#3390](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3390)) - Clean up and rebuild `@osd/pm` ([#3570](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3570)) +- Omit adding the `osd-version` header when the Fetch request is to an external origin ([#3643](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3643)) ### 🚞 Infrastructure diff --git a/src/core/public/http/fetch.ts b/src/core/public/http/fetch.ts index cefaa353f7fa..8bbb63fe7448 100644 --- a/src/core/public/http/fetch.ts +++ b/src/core/public/http/fetch.ts @@ -31,6 +31,7 @@ import { omitBy } from 'lodash'; import { format } from 'url'; import { BehaviorSubject } from 'rxjs'; +import { isRelativeUrl } from '@osd/std'; import { IBasePath, @@ -144,7 +145,7 @@ export class Fetch { headers: removedUndefined({ 'Content-Type': 'application/json', ...options.headers, - 'osd-version': this.params.opensearchDashboardsVersion, + 'osd-xsrf': 'osd-fetch', }), }; @@ -158,6 +159,16 @@ export class Fetch { fetchOptions.headers['osd-system-request'] = 'true'; } + /* `osd-version` is used on the server-side to make sure that an incoming request originated from a front-end + * of the same version; see core/server/http/lifecycle_handlers.ts + * + * If url equals `basePath, starts with `basePath` + '/', or is relative, add `osd-version` header. + */ + const basePath = this.params.basePath.get(); + if (isRelativeUrl(url) || url === basePath || url.startsWith(`${basePath}/`)) { + fetchOptions.headers['osd-version'] = this.params.opensearchDashboardsVersion; + } + return new Request(url, fetchOptions as RequestInit); } diff --git a/src/core/server/http/integration_tests/lifecycle_handlers.test.ts b/src/core/server/http/integration_tests/lifecycle_handlers.test.ts index fc13c1ae3fbb..80fd8424f284 100644 --- a/src/core/server/http/integration_tests/lifecycle_handlers.test.ts +++ b/src/core/server/http/integration_tests/lifecycle_handlers.test.ts @@ -231,12 +231,14 @@ describe('core lifecycle handlers', () => { .expect(200, 'ok'); }); + // ToDo: Remove next; `osd-version` incorrectly used for satisfying XSRF protection it('accepts requests with the version header', async () => { await getSupertest(method.toLowerCase(), testPath) .set(versionHeader, actualVersion) .expect(200, 'ok'); }); + // ToDo: Rename next; `osd-version` incorrectly used for satisfying XSRF protection it('rejects requests without either an xsrf or version header', async () => { await getSupertest(method.toLowerCase(), testPath).expect(400, { statusCode: 400, @@ -245,6 +247,7 @@ describe('core lifecycle handlers', () => { }); }); + // ToDo: Rename next; `osd-version` incorrectly used for satisfying XSRF protection it('accepts whitelisted requests without either an xsrf or version header', async () => { await getSupertest(method.toLowerCase(), whitelistedTestPath).expect(200, 'ok'); }); diff --git a/src/core/server/http/lifecycle_handlers.test.ts b/src/core/server/http/lifecycle_handlers.test.ts index 6a49bbfa14fa..ae1d64baf67b 100644 --- a/src/core/server/http/lifecycle_handlers.test.ts +++ b/src/core/server/http/lifecycle_handlers.test.ts @@ -102,6 +102,7 @@ describe('xsrf post-auth handler', () => { expect(result).toEqual('next'); }); + // ToDo: Remove; `osd-version` incorrectly used for satisfying XSRF protection it('accepts requests with version header', () => { const config = createConfig({ xsrf: { whitelist: [], disableProtection: false } }); const handler = createXsrfPostAuthHandler(config); @@ -199,7 +200,7 @@ describe('versionCheck post-auth handler', () => { responseFactory = httpServerMock.createLifecycleResponseFactory(); }); - it('forward the request to the next interceptor if header matches', () => { + it('forward the request to the next interceptor if osd-version header matches the actual version', () => { const handler = createVersionCheckPostAuthHandler('actual-version'); const request = forgeRequest({ headers: { 'osd-version': 'actual-version' } }); @@ -212,7 +213,7 @@ describe('versionCheck post-auth handler', () => { expect(result).toBe('next'); }); - it('returns a badRequest error if header does not match', () => { + it('returns a badRequest error if osd-version header exists but does not match the actual version', () => { const handler = createVersionCheckPostAuthHandler('actual-version'); const request = forgeRequest({ headers: { 'osd-version': 'another-version' } }); @@ -236,7 +237,7 @@ describe('versionCheck post-auth handler', () => { expect(result).toBe('badRequest'); }); - it('forward the request to the next interceptor if header is not present', () => { + it('forward the request to the next interceptor if osd-version header is not present', () => { const handler = createVersionCheckPostAuthHandler('actual-version'); const request = forgeRequest({ headers: {} }); diff --git a/src/core/server/http/lifecycle_handlers.ts b/src/core/server/http/lifecycle_handlers.ts index 636bb8af4522..f17b07942e6a 100644 --- a/src/core/server/http/lifecycle_handlers.ts +++ b/src/core/server/http/lifecycle_handlers.ts @@ -54,8 +54,9 @@ export const createXsrfPostAuthHandler = (config: HttpConfig): OnPostAuthHandler const hasVersionHeader = VERSION_HEADER in request.headers; const hasXsrfHeader = XSRF_HEADER in request.headers; + // ToDo: Remove !hasVersionHeader; `osd-version` incorrectly used for satisfying XSRF protection if (!isSafeMethod(request.route.method) && !hasVersionHeader && !hasXsrfHeader) { - return response.badRequest({ body: `Request must contain a ${XSRF_HEADER} header.` }); + return response.badRequest({ body: `Request must contain the ${XSRF_HEADER} header.` }); } return toolkit.next(); diff --git a/src/plugins/bfetch/public/plugin.ts b/src/plugins/bfetch/public/plugin.ts index 1a3a0bffc821..e115b35a44e1 100644 --- a/src/plugins/bfetch/public/plugin.ts +++ b/src/plugins/bfetch/public/plugin.ts @@ -95,6 +95,7 @@ export class BfetchPublicPlugin url: `${basePath}/${removeLeadingSlash(params.url)}`, headers: { 'Content-Type': 'application/json', + 'osd-xsrf': 'osd-bfetch', 'osd-version': version, ...(params.headers || {}), }, diff --git a/src/plugins/opensearch_dashboards_legacy/public/angular/angular_config.tsx b/src/plugins/opensearch_dashboards_legacy/public/angular/angular_config.tsx index ef01d29e4b14..fbe36a289d70 100644 --- a/src/plugins/opensearch_dashboards_legacy/public/angular/angular_config.tsx +++ b/src/plugins/opensearch_dashboards_legacy/public/angular/angular_config.tsx @@ -159,6 +159,8 @@ export const $setupXsrfRequestInterceptor = (version: string) => { // Configure jQuery prefilter $.ajaxPrefilter(({ osdXsrfToken = true }: any, originalOptions, jqXHR) => { if (osdXsrfToken) { + jqXHR.setRequestHeader('osd-xsrf', 'osd-legacy'); + // ToDo: Remove next; `osd-version` incorrectly used for satisfying XSRF protection jqXHR.setRequestHeader('osd-version', version); } }); @@ -170,6 +172,8 @@ export const $setupXsrfRequestInterceptor = (version: string) => { request(opts) { const { osdXsrfToken = true } = opts as any; if (osdXsrfToken) { + set(opts, ['headers', 'osd-xsrf'], 'osd-legacy'); + // ToDo: Remove next; `osd-version` incorrectly used for satisfying XSRF protection set(opts, ['headers', 'osd-version'], version); } return opts;