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

[2.1] Restrict chromium requests #430

Merged
merged 8 commits into from
Aug 18, 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
22 changes: 11 additions & 11 deletions .github/workflows/dashboards-reports-test-and-build-workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ jobs:
echo "Running backwards compatibility tests ..."
./gradlew bwcTestSuite


- name: Build with Gradle
run: |
cd reports-scheduler
Expand Down
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))$/;

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
16 changes: 8 additions & 8 deletions reports-scheduler/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -315,18 +315,18 @@ testClusters.integTest {
setting 'path.repo', repo.absolutePath
}

// For job-scheduler and reports-scheduler, the latest opendistro releases appear to be 1.13.0.0.
String bwcVersion = "1.13.0.0"
// For job-scheduler and reports-scheduler, the latest opensearch releases appear to be 1.1.0.0.
String bwcVersion = "1.1.0.0"
String baseName = "reportsSchedulerBwcCluster"
String bwcFilePath = "src/test/resources/bwc"
String bwcJobSchedulerURL = "https://d3g5vo6xdbdb9a.cloudfront.net/downloads/elasticsearch-plugins/opendistro-job-scheduler/opendistro-job-scheduler-" + bwcVersion + ".zip"
String bwcReportsSchedulerURL = "https://d3g5vo6xdbdb9a.cloudfront.net/downloads/elasticsearch-plugins/opendistro-reports-scheduler/opendistro-reports-scheduler-" + bwcVersion + ".zip"
String bwcJobSchedulerURL = "https://ci.opensearch.org/ci/dbc/bundle-build/1.1.0/20210930/linux/x64/builds/opensearch/plugins/opensearch-job-scheduler-1.1.0.0.zip"
String bwcReportsSchedulerURL = "https://ci.opensearch.org/ci/dbc/bundle-build/1.1.0/20210930/linux/x64/builds/opensearch/plugins/opensearch-reports-scheduler-1.1.0.0.zip"

2.times {i ->
testClusters {
"${baseName}$i" {
testDistribution = "ARCHIVE"
versions = ["7.10.2", opensearch_version]
versions = ["1.1.0", opensearch_version]
numberOfNodes = 3
plugin(provider(new Callable<RegularFile>(){
@Override
Expand All @@ -338,7 +338,7 @@ String bwcReportsSchedulerURL = "https://d3g5vo6xdbdb9a.cloudfront.net/downloads
if (!dir.exists()) {
dir.mkdirs()
}
File file = new File(dir, "opendistro-job-scheduler-" + bwcVersion + ".zip")
File file = new File(dir, "opensearch-job-scheduler-" + bwcVersion + ".zip")
if (!file.exists()) {
new URL(bwcJobSchedulerURL).withInputStream{ ins -> file.withOutputStream{ it << ins }}
}
Expand All @@ -357,7 +357,7 @@ String bwcReportsSchedulerURL = "https://d3g5vo6xdbdb9a.cloudfront.net/downloads
if (!dir.exists()) {
dir.mkdirs()
}
File file = new File(dir, "opendistro-reports-scheduler-" + bwcVersion + ".zip")
File file = new File(dir, "opensearch-reports-scheduler-" + bwcVersion + ".zip")
if (!file.exists()) {
new URL(bwcReportsSchedulerURL).withInputStream{ ins -> file.withOutputStream{ it << ins }}
}
Expand Down Expand Up @@ -389,7 +389,7 @@ task prepareBwcTests {
if (!dir.exists()) {
dir.mkdirs()
}
File file = new File(dir, "opendistro-reports-scheduler-" + project.version + ".zip")
File file = new File(dir, "opensearch-reports-scheduler-" + project.version + ".zip")
if (!file.exists()) {
new URL(jobSchedulerURL).withInputStream{ ins -> file.withOutputStream{ it << ins }}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ class ReportsSchedulerBackwardsCompatibilityIT : PluginRestTestCase() {
val pluginNames = plugins.map { plugin -> plugin["name"] }.toSet()
when (CLUSTER_TYPE) {
ClusterType.OLD -> {
assertTrue(pluginNames.contains("opendistro-reports-scheduler"))
assertTrue(pluginNames.contains("opendistro-job-scheduler"))
assertTrue(pluginNames.contains("opensearch-reports-scheduler"))
assertTrue(pluginNames.contains("opensearch-job-scheduler"))
createBasicReportDefinition()
}
ClusterType.MIXED -> {
Expand Down