Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1.3] restrict chromium requests #424

Merged
merged 6 commits into from
Aug 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions .github/workflows/dashboards-reports-test-and-build-workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,21 @@ jobs:
with:
repository: opensearch-project/Opensearch-Dashboards
ref: ${{ env.OPENSEARCH_VERSION }}
path: dashboards-reports/OpenSearch-Dashboards
path: OpenSearch-Dashboards

- name: Setup Node
uses: actions/setup-node@v1
with:
node-version: "10.24.1"

- 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
Expand All @@ -44,25 +44,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
Expand Down Expand Up @@ -96,16 +96,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
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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');
Expand Down
2 changes: 2 additions & 0 deletions dashboards-reports/server/routes/utils/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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))$/;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this have aws.a2z.com? This is an Amazon url.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replied in #425 (comment)


export const replaceBlockedKeywords = (htmlString: string) => {
// replace <ipv4>:<port>
htmlString = htmlString.replace(ipv4Regex, BLOCKED_KEYWORD);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
SELECTOR,
CHROMIUM_PATH,
SECURITY_CONSTANTS,
ALLOWED_HOSTS,
} from '../constants';
import { getFileName } from '../helpers';
import { CreateReportResultType } from '../types';
Expand All @@ -27,7 +28,8 @@ export const createVisualReport = async (
queryUrl: string,
logger: Logger,
extraHeaders: Headers,
timezone?: string
timezone?: string,
validRequestProtocol = /^(data:image)/,
): Promise<CreateReportResultType> => {
const {
core_params,
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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()}`);

Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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 = {
Expand Down
2 changes: 1 addition & 1 deletion dashboards-reports/server/utils/validationHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down