From 5ede578d5c1f84399ba8a544da42276ca52520cb Mon Sep 17 00:00:00 2001 From: Joshua Li Date: Thu, 18 Aug 2022 16:49:29 -0700 Subject: [PATCH] [2.2] Restrict chromium requests (#431) * Fix regex validation, detect iframe, embed, object tags Signed-off-by: Joshua Li * Disallow redirection to non-localhost urls Signed-off-by: Joshua Li * Disallow connection to non-allowlisted urls Signed-off-by: Joshua Li * Disable JIT Signed-off-by: Joshua Li * Fix workflow Signed-off-by: Joshua Li * Try to fix CI Signed-off-by: Joshua Li * Fix localstorage logic Signed-off-by: Joshua Li Signed-off-by: Joshua Li --- ...boards-reports-test-and-build-workflow.yml | 22 +++--- .../__tests__/visualReportHelper.test.ts | 8 +- server/routes/utils/constants.ts | 2 + server/routes/utils/visual_report/style.css | 4 + .../utils/visual_report/visualReportHelper.ts | 75 ++++++++++++++++--- .../utils/__tests__/validationHelper.test.ts | 36 ++++++++- server/utils/validationHelper.ts | 2 +- 7 files changed, 125 insertions(+), 24 deletions(-) diff --git a/.github/workflows/dashboards-reports-test-and-build-workflow.yml b/.github/workflows/dashboards-reports-test-and-build-workflow.yml index 5bb574fd..8f4cd6b3 100644 --- a/.github/workflows/dashboards-reports-test-and-build-workflow.yml +++ b/.github/workflows/dashboards-reports-test-and-build-workflow.yml @@ -20,12 +20,12 @@ jobs: with: repository: opensearch-project/Opensearch-Dashboards ref: ${{ env.OPENSEARCH_VERSION }} - path: dashboards-reports/OpenSearch-Dashboards + path: OpenSearch-Dashboards - name: Get node version id: versions_step run: - echo "::set-output name=node_version::$(node -p "(require('./OpenSearch-Dashboards/package.json').engines.node).match(/[.0-9]+/)[0]")" + echo "::set-output name=node_version::$(node -p "(require('../OpenSearch-Dashboards/package.json').engines.node).match(/[.0-9]+/)[0]")" - name: Setup Node uses: actions/setup-node@v1 @@ -35,13 +35,13 @@ jobs: - name: Move Dashboards Reports to Plugins Dir - run: mv dashboards-reports OpenSearch-Dashboards/plugins/${{ env.PLUGIN_NAME }} + run: mv dashboards-reports ../OpenSearch-Dashboards/plugins/${{ env.PLUGIN_NAME }} - name: Add Chromium Binary to Reporting for Testing run: | sudo apt update sudo apt install -y libnss3-dev fonts-liberation libfontconfig1 - cd OpenSearch-Dashboards/plugins/${{ env.PLUGIN_NAME }} + cd ../OpenSearch-Dashboards/plugins/${{ env.PLUGIN_NAME }} wget https://github.com/opendistro-for-elasticsearch/kibana-reports/releases/download/chromium-1.12.0.0/chromium-linux-x64.zip unzip chromium-linux-x64.zip rm chromium-linux-x64.zip @@ -51,25 +51,25 @@ jobs: with: timeout_minutes: 30 max_attempts: 3 - command: cd OpenSearch-Dashboards/plugins/${{ env.PLUGIN_NAME }}; yarn osd bootstrap + command: cd ../OpenSearch-Dashboards/plugins/${{ env.PLUGIN_NAME }}; yarn osd bootstrap - name: Test uses: nick-invision/retry@v1 with: timeout_minutes: 30 max_attempts: 3 - command: cd OpenSearch-Dashboards/plugins/${{ env.PLUGIN_NAME }}; yarn test --coverage + command: cd ../OpenSearch-Dashboards/plugins/${{ env.PLUGIN_NAME }}; yarn test --coverage - name: Upload coverage uses: codecov/codecov-action@v1 with: flags: dashboards-reports - directory: OpenSearch-Dashboards/plugins/ + directory: ../OpenSearch-Dashboards/plugins/ token: ${{ secrets.CODECOV_TOKEN }} - name: Build Artifact run: | - cd OpenSearch-Dashboards/plugins/${{ env.PLUGIN_NAME }} + cd ../OpenSearch-Dashboards/plugins/${{ env.PLUGIN_NAME }} yarn build cd build @@ -103,16 +103,16 @@ jobs: uses: actions/upload-artifact@v1 with: name: dashboards-reports-linux-x64 - path: OpenSearch-Dashboards/plugins/${{ env.PLUGIN_NAME }}/build/${{ env.ARTIFACT_NAME }}-${{ env.OPENSEARCH_PLUGIN_VERSION }}-linux-x64.zip + path: ../OpenSearch-Dashboards/plugins/${{ env.PLUGIN_NAME }}/build/${{ env.ARTIFACT_NAME }}-${{ env.OPENSEARCH_PLUGIN_VERSION }}-linux-x64.zip - name: Upload Artifact For Linux arm64 uses: actions/upload-artifact@v1 with: name: dashboards-reports-linux-arm64 - path: OpenSearch-Dashboards/plugins/${{ env.PLUGIN_NAME }}/build/${{ env.ARTIFACT_NAME }}-${{ env.OPENSEARCH_PLUGIN_VERSION }}-linux-arm64.zip + path: ../OpenSearch-Dashboards/plugins/${{ env.PLUGIN_NAME }}/build/${{ env.ARTIFACT_NAME }}-${{ env.OPENSEARCH_PLUGIN_VERSION }}-linux-arm64.zip - name: Upload Artifact For Windows uses: actions/upload-artifact@v1 with: name: dashboards-reports-windows-x64 - path: OpenSearch-Dashboards/plugins/${{ env.PLUGIN_NAME }}/build/${{ env.ARTIFACT_NAME }}-${{ env.OPENSEARCH_PLUGIN_VERSION }}-windows-x64.zip + path: ../OpenSearch-Dashboards/plugins/${{ env.PLUGIN_NAME }}/build/${{ env.ARTIFACT_NAME }}-${{ env.OPENSEARCH_PLUGIN_VERSION }}-windows-x64.zip diff --git a/server/routes/utils/__tests__/visualReportHelper.test.ts b/server/routes/utils/__tests__/visualReportHelper.test.ts index 81595979..4bf3cd0b 100644 --- a/server/routes/utils/__tests__/visualReportHelper.test.ts +++ b/server/routes/utils/__tests__/visualReportHelper.test.ts @@ -55,7 +55,9 @@ describe('test create visual report', () => { reportParams as ReportParamsSchemaType, mockHtmlPath, mockLogger, - mockHeader + mockHeader, + undefined, + /^(data:image|file:\/\/)/ ); expect(fileName).toContain(`${reportParams.report_name}`); expect(fileName).toContain('.png'); @@ -71,7 +73,9 @@ describe('test create visual report', () => { reportParams as ReportParamsSchemaType, mockHtmlPath, mockLogger, - mockHeader + mockHeader, + undefined, + /^(data:image|file:\/\/)/ ); expect(fileName).toContain(`${reportParams.report_name}`); expect(fileName).toContain('.pdf'); diff --git a/server/routes/utils/constants.ts b/server/routes/utils/constants.ts index dffb0cd1..6af81fd2 100644 --- a/server/routes/utils/constants.ts +++ b/server/routes/utils/constants.ts @@ -93,6 +93,8 @@ const ipv6Regex = /(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:) const localhostRegex = /localhost:([1-9][0-9]{0,3}|[1-5][0-9]{4}|6[0-4][0-9]{3}|65[0-4][0-9]{2}|655[0-2][0-9]|6553[0-5])/g; const iframeRegex = /iframe/g; +export const ALLOWED_HOSTS = /^(0|0.0.0.0|127.0.0.1|localhost|(.*\.)?(opensearch.org|aws.a2z.com))$/; + export const replaceBlockedKeywords = (htmlString: string) => { // replace : htmlString = htmlString.replace(ipv4Regex, BLOCKED_KEYWORD); diff --git a/server/routes/utils/visual_report/style.css b/server/routes/utils/visual_report/style.css index 58628427..c329e281 100644 --- a/server/routes/utils/visual_report/style.css +++ b/server/routes/utils/visual_report/style.css @@ -4,6 +4,10 @@ body { padding: 0; } +iframe, embed, object { + display: none !important; +} + /* nice padding + matches Kibana default UI colors you could also set this to inherit if the wrapper gets inserted inside a kibana section. I might also remove the manual text color here as well, potentially */ .reportWrapper { diff --git a/server/routes/utils/visual_report/visualReportHelper.ts b/server/routes/utils/visual_report/visualReportHelper.ts index d98d1824..21b52c20 100644 --- a/server/routes/utils/visual_report/visualReportHelper.ts +++ b/server/routes/utils/visual_report/visualReportHelper.ts @@ -14,6 +14,7 @@ import { SELECTOR, CHROMIUM_PATH, SECURITY_CONSTANTS, + ALLOWED_HOSTS, } from '../constants'; import { getFileName } from '../helpers'; import { CreateReportResultType } from '../types'; @@ -27,7 +28,8 @@ export const createVisualReport = async ( queryUrl: string, logger: Logger, extraHeaders: Headers, - timezone?: string + timezone?: string, + validRequestProtocol = /^(data:image)/, ): Promise => { const { core_params, @@ -76,6 +78,8 @@ export const createVisualReport = async ( '--no-zygote', '--single-process', '--font-render-hinting=none', + '--js-flags="--jitless --no-opt"', + '--disable-features=V8OptimizeJavascript', ], executablePath: CHROMIUM_PATH, ignoreHTTPSErrors: true, @@ -84,6 +88,32 @@ export const createVisualReport = async ( }, }); const page = await browser.newPage(); + + await page.setRequestInterception(true); + let localStorageAvailable = true; + page.on('request', (req) => { + // disallow non-allowlisted connections. urls with valid protocols do not need ALLOWED_HOSTS check + if ( + !validRequestProtocol.test(req.url()) && + !ALLOWED_HOSTS.test(new URL(req.url()).hostname) + ) { + if (req.isNavigationRequest() && req.redirectChain().length > 0) { + localStorageAvailable = false; + logger.error( + 'Reporting does not allow redirections to outside of localhost, aborting. URL received: ' + + req.url() + ); + } else { + logger.warn( + 'Disabled connection to non-allowlist domains: ' + req.url() + ); + } + req.abort(); + } else { + req.continue(); + } + }); + page.setDefaultNavigationTimeout(0); page.setDefaultTimeout(100000); // use 100s timeout instead of default 30s // Set extra headers that are needed @@ -93,13 +123,25 @@ export const createVisualReport = async ( logger.info(`original queryUrl ${queryUrl}`); await page.goto(queryUrl, { waitUntil: 'networkidle0' }); // should add to local storage after page.goto, then access the page again - browser must have an url to register local storage item on it - await page.evaluate( - /* istanbul ignore next */ - (key) => { - localStorage.setItem(key, 'false'); - }, - SECURITY_CONSTANTS.TENANT_LOCAL_STORAGE_KEY - ); + try { + await page.evaluate( + /* istanbul ignore next */ + (key) => { + try { + if ( + localStorageAvailable && + typeof localStorage !== 'undefined' && + localStorage !== null + ) { + localStorage.setItem(key, 'false'); + } + } catch (err) {} + }, + SECURITY_CONSTANTS.TENANT_LOCAL_STORAGE_KEY + ); + } catch (err) { + logger.error(err); + } await page.goto(queryUrl, { waitUntil: 'networkidle0' }); logger.info(`page url ${page.url()}`); @@ -162,9 +204,24 @@ export const createVisualReport = async ( // wait for dynamic page content to render await waitForDynamicContent(page); - await addReportStyle(page); await addReportHeader(page, keywordFilteredHeader); await addReportFooter(page, keywordFilteredFooter); + await addReportStyle(page); + + // this causes UT to fail in github CI but works locally + try { + const numDisallowedTags = await page.evaluate( + () => + document.getElementsByTagName('iframe').length + + document.getElementsByTagName('embed').length + + document.getElementsByTagName('object').length + ); + if (numDisallowedTags > 0) { + throw Error('Reporting does not support "iframe", "embed", or "object" tags, aborting'); + } + } catch (error) { + logger.error(error); + } // create pdf or png accordingly if (reportFormat === FORMAT.pdf) { diff --git a/server/utils/__tests__/validationHelper.test.ts b/server/utils/__tests__/validationHelper.test.ts index bc3ca73d..9bdb4fe0 100644 --- a/server/utils/__tests__/validationHelper.test.ts +++ b/server/utils/__tests__/validationHelper.test.ts @@ -10,7 +10,7 @@ import { REPORT_TYPE, TRIGGER_TYPE, } from '../../routes/utils/constants'; -import { validateReport, validateReportDefinition } from '../validationHelper'; +import { isValidRelativeUrl, validateReport, validateReportDefinition } from '../validationHelper'; const SAMPLE_SAVED_OBJECT_ID = '3ba638e0-b894-11e8-a6d9-e546fe2bba5f'; const createReportDefinitionInput: ReportDefinitionSchemaType = { @@ -152,7 +152,41 @@ describe('test input validation', () => { `saved object with id dashboard:${SAMPLE_SAVED_OBJECT_ID} does not exist` ); }); + + test('validation against query_url', async () => { + const urls: [string, boolean][] = [ + ['/app/dashboards#/view/7adfa750-4c81-11e8-b3d7-01146121b73d?_g=', true], + [ + '/_plugin/kibana/app/dashboards#/view/7adfa750-4c81-11e8-b3d7-01146121b73d?_g=', + true, + ], + [ + '/_dashboards/app/dashboards#/view/7adfa750-4c81-11e8-b3d7-01146121b73d?_g=', + true, + ], + [ + '/_dashboards/app/dashboards#/edit/7adfa750-4c81-11e8-b3d7-01146121b73d?_g=', + true, + ], + [ + '/app/observability-dashboards?security_tenant=private#/notebooks/NYdlPIIB0-fJ8Bh1nLdW?view=output_only', + true, + ], + [ + '/app/notebooks-dashboards?view=output_only&security_tenant=private#/M4dlPIIB0-fJ8Bh1nLc7?security_tenant=private', + true, + ], + [ + '/_dashboards/app/visualize&security_tenant=/.%2e/.%2e/.%2e/.%2e/_dashboards?#/view/id', + false, + ], + ]; + expect(urls.map((url) => isValidRelativeUrl(url[0]))).toEqual( + urls.map((url) => url[1]) + ); + }); }); + // TODO: merge this with other mock clients used in testing, to create some mock helpers file const mockOpenSearchClient = (mockSavedObjectIds: string[]) => { const client = { diff --git a/server/utils/validationHelper.ts b/server/utils/validationHelper.ts index aff53d41..2597ee4b 100644 --- a/server/utils/validationHelper.ts +++ b/server/utils/validationHelper.ts @@ -37,7 +37,7 @@ export const isValidRelativeUrl = (relativeUrl: string) => { export const regexDuration = /^(-?)P(?=\d|T\d)(?:(\d+)Y)?(?:(\d+)M)?(?:(\d+)([DW]))?(?:T(?:(\d+)H)?(?:(\d+)M)?(?:(\d+(?:\.\d+)?)S)?)?$/; export const regexEmailAddress = /\S+@\S+\.\S+/; export const regexReportName = /^[\w\-\s\(\)\[\]\,\_\-+]+$/; -export const regexRelativeUrl = /^\/(_plugin\/kibana\/|_dashboards\/)?app\/(dashboards|visualize|discover|observability-dashboards|notebooks-dashboards\?view=output_only)([?&]security_tenant=.+|)#\/(notebooks\/|view\/|edit\/)?[^\/]+$/; +export const regexRelativeUrl = /^\/(_plugin\/kibana\/|_dashboards\/)?app\/(dashboards|visualize|discover|observability-dashboards|notebooks-dashboards\?view=output_only(&security_tenant=.+)?)(\?security_tenant=.+)?#\/(notebooks\/|view\/|edit\/)?[^\/]+$/; export const validateReport = async ( client: ILegacyScopedClusterClient,